Decouple TBDocs Commenter

[leordev]

Pretty similar to what was done for web5-js repo: TBD54566975/web5-js#323

To post the tbdocs report in a comment we need to have access to secrets. For security reasons forked PRs dont have access to them, and then we get this kind of error: https://github.com/TBD54566975/tbdex-js/actions/runs/6862633634/job/18660762085?pr=79#step:8:248

Now we also have the tbdocs report in the GH Action summary!

• Decouples the docs reporter from the CI tests
• Decouples the PR comments generation from the tbdocs activity because secrets are not exposed to forked PRs
  • we could either create a server app to watch the PR and open a comment from the server (same approach taken by CodeCov reports which are passing in our PRs!)
  • or, per the github workflows security recommendations, use the workflow_run trigger to safely execute code from the main branch, access secrets and be able to create the report comment
    • Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests
    • workflow_run usage example

PS: the tbdocs report comment in the PR will only start to show up when we merge this PR to main. Unfortunately this is the normal behavior for new workflow_run definitions.

Working evidence:

• TBDocs PR with a comment: testing error tbdocs#89 (comment)
• TBDocs Commenter workflow_run triggered action logs: https://github.com/TBD54566975/tbdocs/actions/runs/7039271133/job/19157923330#step:5:12

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9171203

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[KendallWeihe]

lgtm but do we need to remove tbdocs-reporter from the integrity-check.yml workflow?

[leordev]

lgtm but do we need to remove tbdocs-reporter from the integrity-check.yml workflow?

@KendallWeihe Great question! The reason why I did this is because the workflow trigger for the commenter is dependent in the completion of another workflow.

If I left it within the main CI workflow the comment would be added to the PR after all tests are complete, which might take way longer than the tbdocs reporter (in web5 repo the ci tests were taking 10m+, and the tbdocs reporter was finished in less than 2m)

[KendallWeihe]

The reason why I did this is because the workflow trigger for the commenter is dependent in the completion of another workflow.

Make sense to me, but so you want the reporter job to run both during the integrity-check.yml (which, looks like it runs on PR commits and commits on main) and also in the tbdocs-commenter.yml workflow (which, to your point, runs after the completion of the docs-ci.yml workflow, which, also appears to run on PR commits and commits on main)? So basically, you have the job running twice, right?

I raise, because you removed the job from the tests-ci.yml in web5-js but not here

[leordev]

Oh I missed your original question point! Yes we definitely need to remove the job from the integrity check to avoid it running twice and I completely forgot... fixing it! Great catch!

[mistermoe]

should we name this publish-docs.yml? since there's also another workflow related to docs?

[leordev]

Maybe another name because this one is just checking the docs errors and generating the report, not actually publishing the docs anywhere... kind of a docs linting. What do you suggest? docs-check.yml, docs-report.yml?

[mistermoe]

ah gotcha! we can submit another PR when/if we figure out a different name!