ci: post HIL report comment from workflow_run so it works on forked PRs#3723
Conversation
The hil-report job lived in build.yml gated on pull_request.head.repo.fork == false, so the combined HIL sticky comment was skipped on forked PRs even though the HIL jobs themselves ran. Fork pull_request events get a read-only GITHUB_TOKEN, so posting from build.yml would 403. Move the comment into the workflow_run-triggered pr_comment.yml (renamed from metrics_comment.yml), which runs in the base-repo context with a write token and access to the triggering run's artifacts. A new hil-comment job downloads the hil-report-* artifacts, combines one table per rig, and posts the sticky hil-report comment with an explicit PR number read from pr_number.txt in the metrics-comment artifact (workflow_run.pull_requests is empty for forks). build.yml's hil-report job is removed; the HIL artifact uploads are unchanged. workflow_run workflows only trigger from the default branch, so this takes effect after merge to master. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Moves HIL report PR commenting out of the Build workflow and into a workflow_run-triggered workflow so the sticky HIL report comment can be posted with write permissions even for forked PRs (where the build-side GITHUB_TOKEN is read-only).
Changes:
- Add
.github/workflows/pr_comment.yml(workflow_runon “Build”) to post sticky PR comments (metrics + new combined HIL report). - Remove the in-workflow
hil-reportcommenting job from.github/workflows/build.yml. - Remove the old standalone
.github/workflows/metrics_comment.ymlworkflow (superseded bypr_comment.yml).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/pr_comment.yml | New workflow_run workflow that posts metrics and combined HIL sticky PR comments in base-repo context. |
| .github/workflows/metrics_comment.yml | Removed old metrics-only workflow_run commenter workflow (replaced by pr_comment.yml). |
| .github/workflows/build.yml | Deletes the previous hil-report job that posted comments only for non-fork PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b7a507f25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in sizeNo entries. No changes
|
|
- metrics-comment: make the artifact download best-effort so docs-only PRs (where code-metrics is skipped) don't fail the check (Copilot). - HIL PR number: carry pr_number.txt inside each hil-report-* artifact and read it in hil-comment, so the HIL comment is self-contained and still posts when an unrelated build leg fails or code-metrics is skipped, on same-repo and forked PRs alike (Codex). - hil-comment: neutralize @-mentions in the combined report before posting, since fork PRs can influence report content and this job posts with a write token in base-repo context (Copilot). Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3403865b13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex flagged that taking the PR number from a downloaded artifact lets a forked PR (which controls its own Build run) plant an arbitrary number, so the privileged workflow_run comment job could post to any PR/issue. Resolve the number from trusted workflow_run metadata instead: same-repo PRs use github.event.workflow_run.pull_requests[0].number; forked PRs look up the PR by the trusted workflow_run.head_sha. A shared pr_number job feeds both metrics-comment and hil-comment so neither trusts artifact-supplied numbers. Revert the Option B change that carried pr_number.txt in the HIL artifacts, and drop the now-unused pr_number.txt from the code-metrics metrics-comment artifact (build.yml back to baseline + that removal). Addresses Codex review comment on #3723. Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Copilot flagged that the fork lookup 'gh api commits/<sha>/pulls' can 404/fail under bash -e and block both comment jobs. Harden the pr_number job: - parse workflow_run.pull_requests in-shell (jq over toJSON) instead of indexing [0] in an expression; - resolve fork PRs via the reliable targeted pulls?state=open&head=<owner>:<branch> filter, verifying head.sha == head_sha (still trusted metadata); - make every lookup best-effort so a miss skips the comment instead of failing the step. Addresses Copilot review comment on #3723. Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Problem
The
hil-reportjob (the combined HIL sticky PR comment) lived inbuild.ymlgated on:github.event.pull_request.head.repo.fork == falseso it was skipped on forked PRs — even though the
hil-tinyusbjobs themselves run there. Verified live on fork PR #3582: the HIL jobs ran (one failed, one passed) buthil-reportwas skipped. Root cause: apull_requestevent from a fork givesGITHUB_TOKENread-only permissions, so apull-requests: writepost would 403.Fix
Use the standard
workflow_runpattern (already used bymetrics_comment.ymlandmembrowse-comment.yml): post the comment from a separate workflow that runs in the base-repo context with a write token and access to the triggering run's uploaded artifacts..github/workflows/metrics_comment.yml→.github/workflows/pr_comment.yml(name:Metrics Comment→PR Comment).hil-commentjob that downloads thehil-report-*artifacts from the triggering run, combines one markdown table per rig (same logic as the old job), and posts the stickyhil-reportcomment.hil-reportjob frombuild.yml.Gate
hil-commentruns onworkflow_run.event == 'pull_request' && conclusion != 'cancelled'— i.e. it posts even when the build/HIL failed (that's exactly when the report matters; the old job usedalways()). It deliberately does not requireconclusion == 'success'.PR number — resolved from trusted
workflow_runmetadatagithub.event.workflow_run.pull_requestsis empty for fork PRs, so a sharedpr_numberjob resolves the number from trusted sources — never from build artifacts (a forked PR controls its own Build run and could plant an arbitrary number, which the privileged comment jobs would otherwise post to):workflow_run.pull_requests(parsed in-shell);head_repository:head_branch) and verifyhead.sha == workflow_run.head_sha.All lookups are best-effort, so a miss skips the comment rather than failing the job. Both
metrics-commentandhil-commentconsumeneeds.pr_number.outputs.number, so neither trusts artifact-supplied numbers. This also closes the same issue in the pre-existing metrics path; the now-unusedpr_number.txtwas dropped from thecode-metricsartifact.Hardening (review feedback)
metrics-commentartifact download is best-effort (continue-on-error) so docs-only PRs (wherecode-metricsis skipped) don't fail the check.@-mentions in the combined HIL report are neutralized (zero-width space) before posting, since fork PRs can influence report content and this job posts with a write token in base-repo context.Note on testing
workflow_runworkflows only trigger when present on the default branch, sopr_comment.ymlwill not run for this PR — it takes effect only after merge tomaster. Validated locally withactionlint(pr_comment.ymlclean; the twobuild.ymlwarnings —if: falsezephyr job,hifiphileself-hosted label — are pre-existing on master) and a YAML parse.Minor heads-up: the workflow display name changed (
Metrics Comment→PR Comment). If any branch-protection required status check references the old name, it would need updating — unlikely for aworkflow_runcomment job, but flagging it.🤖 Generated with Claude Code