From 1b7a507f2530c97f761e358b4c0118b6fdf815fe Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 21 Jun 2026 23:06:36 +0700 Subject: [PATCH 1/4] ci: post HIL report comment from workflow_run so it works on forked PRs 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 --- .github/workflows/build.yml | 42 ---------- .github/workflows/metrics_comment.yml | 39 --------- .github/workflows/pr_comment.yml | 112 ++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 81 deletions(-) delete mode 100644 .github/workflows/metrics_comment.yml create mode 100644 .github/workflows/pr_comment.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3f64582850..66da1b07db 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -420,45 +420,3 @@ jobs: path: hil_report.md if-no-files-found: ignore overwrite: true - - # --------------------------------------- - # Combine HIL results from the rigs into a single sticky PR comment (one table per rig) - # --------------------------------------- - hil-report: - needs: [ hil-tinyusb, hil-hfp-iar ] - if: | - always() && - (needs.hil-tinyusb.result != 'skipped' || needs.hil-hfp-iar.result != 'skipped') && - github.event_name == 'pull_request' && - github.repository_owner == 'hathach' && - github.event.pull_request.head.repo.fork == false - runs-on: ubuntu-latest - permissions: - pull-requests: write - steps: - - name: Download HIL reports - uses: actions/download-artifact@v5 - with: - pattern: hil-report-* - path: hil-reports - - - name: Combine rig reports (one table per rig) - run: | - { - echo "## Hardware-in-the-loop (HIL) Test Report" - echo - for d in hil-reports/hil-report-*; do - [ -d "$d" ] || continue - echo "### ${d#hil-reports/hil-report-}" - echo - cat "$d/hil_report.md" 2>/dev/null || echo "_no report produced_" - echo - done - } > hil_combined.md - cat hil_combined.md - - - name: Post HIL report as sticky PR comment - uses: marocchino/sticky-pull-request-comment@v2 - with: - header: hil-report - path: hil_combined.md diff --git a/.github/workflows/metrics_comment.yml b/.github/workflows/metrics_comment.yml deleted file mode 100644 index 5d250211f2..0000000000 --- a/.github/workflows/metrics_comment.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: Metrics Comment - -on: - workflow_run: - workflows: ["Build"] - types: - - completed - -jobs: - post-comment: - runs-on: ubuntu-latest - if: > - github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion == 'success' - permissions: - actions: read - pull-requests: write - steps: - - name: Download Artifacts - uses: actions/download-artifact@v5 - with: - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - name: metrics-comment - - - name: Read PR Number - id: pr_number - run: | - if [ -f pr_number.txt ]; then - echo "number=$(cat pr_number.txt)" >> $GITHUB_OUTPUT - fi - - - name: Post Code Metrics as PR Comment - if: steps.pr_number.outputs.number != '' - uses: marocchino/sticky-pull-request-comment@v2 - with: - header: code-metrics - path: metrics_compare.md - number: ${{ steps.pr_number.outputs.number }} diff --git a/.github/workflows/pr_comment.yml b/.github/workflows/pr_comment.yml new file mode 100644 index 0000000000..ac87909a44 --- /dev/null +++ b/.github/workflows/pr_comment.yml @@ -0,0 +1,112 @@ +name: PR Comment + +on: + workflow_run: + workflows: ["Build"] + types: + - completed + +jobs: + metrics-comment: + runs-on: ubuntu-latest + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + permissions: + actions: read + pull-requests: write + steps: + - name: Download Artifacts + uses: actions/download-artifact@v5 + with: + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + name: metrics-comment + + - name: Read PR Number + id: pr_number + run: | + if [ -f pr_number.txt ]; then + echo "number=$(cat pr_number.txt)" >> $GITHUB_OUTPUT + fi + + - name: Post Code Metrics as PR Comment + if: steps.pr_number.outputs.number != '' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: code-metrics + path: metrics_compare.md + number: ${{ steps.pr_number.outputs.number }} + + # --------------------------------------- + # Combine the rigs' HIL reports into one sticky PR comment (one table per rig). + # Runs here (workflow_run / base-repo context) rather than in build.yml so it also works on + # forked PRs, whose build-side GITHUB_TOKEN is read-only and cannot post comments. Posts even + # on build/HIL failure (when the report matters most); skips only on cancellation. + # The PR number is carried in the metrics-comment artifact (the code-metrics and HIL jobs both + # run only on code changes, so it is present); workflow_run.pull_requests is empty for forks. + # --------------------------------------- + hil-comment: + runs-on: ubuntu-latest + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion != 'cancelled' + permissions: + actions: read + pull-requests: write + steps: + - name: Download HIL reports + uses: actions/download-artifact@v5 + with: + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + pattern: hil-report-* + path: hil-reports + continue-on-error: true + + - name: Download PR number (carried in the metrics-comment artifact) + uses: actions/download-artifact@v5 + with: + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + name: metrics-comment + path: pr-meta + continue-on-error: true + + - name: Read PR Number + id: pr_number + run: | + if [ -f pr-meta/pr_number.txt ]; then + echo "number=$(cat pr-meta/pr_number.txt)" >> $GITHUB_OUTPUT + fi + + - name: Combine rig reports (one table per rig) + id: combine + run: | + shopt -s nullglob + dirs=(hil-reports/hil-report-*) + if [ ${#dirs[@]} -eq 0 ]; then + echo "No HIL reports found" + exit 0 + fi + { + echo "## Hardware-in-the-loop (HIL) Test Report" + echo + for d in "${dirs[@]}"; do + [ -d "$d" ] || continue + echo "### ${d#hil-reports/hil-report-}" + echo + cat "$d/hil_report.md" 2>/dev/null || echo "_no report produced_" + echo + done + } > hil_combined.md + cat hil_combined.md + echo "found=true" >> $GITHUB_OUTPUT + + - name: Post HIL report as sticky PR comment + if: steps.combine.outputs.found == 'true' && steps.pr_number.outputs.number != '' + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: hil-report + path: hil_combined.md + number: ${{ steps.pr_number.outputs.number }} From 3403865b13968eda88f102d0a0e04dbc415690f9 Mon Sep 17 00:00:00 2001 From: hathach Date: Sun, 21 Jun 2026 23:19:18 +0700 Subject: [PATCH 2/4] ci: address review feedback on pr_comment workflow - 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 --- .github/workflows/build.yml | 18 ++++++++++++++++-- .github/workflows/pr_comment.yml | 31 +++++++++++++++++-------------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 66da1b07db..f8ffe28ba5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -348,12 +348,20 @@ jobs: exit 1 fi) + - name: Save PR number (carried with the HIL report so the comment job is self-contained) + if: always() && github.event_name == 'pull_request' + run: | + mkdir -p "${{ env.HIL_REPORT_DIR }}" + echo ${{ github.event.number }} > "${{ env.HIL_REPORT_DIR }}/pr_number.txt" + - name: Upload HIL report if: always() && github.event_name == 'pull_request' uses: actions/upload-artifact@v7 with: name: hil-report-${{ matrix.display }} - path: ${{ env.HIL_REPORT_DIR }}/hil_report.md + path: | + ${{ env.HIL_REPORT_DIR }}/hil_report.md + ${{ env.HIL_REPORT_DIR }}/pr_number.txt if-no-files-found: ignore overwrite: true @@ -412,11 +420,17 @@ jobs: run: | python3 test/hil/hil_test.py hfp.json + - name: Save PR number (carried with the HIL report so the comment job is self-contained) + if: always() && github.event_name == 'pull_request' + run: echo ${{ github.event.number }} > pr_number.txt + - name: Upload HIL report if: always() && github.event_name == 'pull_request' uses: actions/upload-artifact@v7 with: name: hil-report-hfp-iar - path: hil_report.md + path: | + hil_report.md + pr_number.txt if-no-files-found: ignore overwrite: true diff --git a/.github/workflows/pr_comment.yml b/.github/workflows/pr_comment.yml index ac87909a44..4db30762c8 100644 --- a/.github/workflows/pr_comment.yml +++ b/.github/workflows/pr_comment.yml @@ -22,6 +22,8 @@ jobs: run-id: ${{ github.event.workflow_run.id }} github-token: ${{ secrets.GITHUB_TOKEN }} name: metrics-comment + # Best-effort: docs-only PRs skip code-metrics, so the artifact may be absent. + continue-on-error: true - name: Read PR Number id: pr_number @@ -43,8 +45,9 @@ jobs: # Runs here (workflow_run / base-repo context) rather than in build.yml so it also works on # forked PRs, whose build-side GITHUB_TOKEN is read-only and cannot post comments. Posts even # on build/HIL failure (when the report matters most); skips only on cancellation. - # The PR number is carried in the metrics-comment artifact (the code-metrics and HIL jobs both - # run only on code changes, so it is present); workflow_run.pull_requests is empty for forks. + # workflow_run.pull_requests is empty for forks, so each hil-report-* artifact carries its own + # pr_number.txt (written in build.yml); the comment is self-contained and survives an unrelated + # build/metrics failure on both same-repo and forked PRs. # --------------------------------------- hil-comment: runs-on: ubuntu-latest @@ -64,21 +67,17 @@ jobs: path: hil-reports continue-on-error: true - - name: Download PR number (carried in the metrics-comment artifact) - uses: actions/download-artifact@v5 - with: - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - name: metrics-comment - path: pr-meta - continue-on-error: true - - name: Read PR Number id: pr_number run: | - if [ -f pr-meta/pr_number.txt ]; then - echo "number=$(cat pr-meta/pr_number.txt)" >> $GITHUB_OUTPUT - fi + # Each hil-report-* artifact carries its own pr_number.txt (see build.yml), so any rig's + # copy works and they are all the same number. + for f in hil-reports/hil-report-*/pr_number.txt; do + if [ -f "$f" ]; then + echo "number=$(cat "$f")" >> $GITHUB_OUTPUT + break + fi + done - name: Combine rig reports (one table per rig) id: combine @@ -100,6 +99,10 @@ jobs: echo done } > hil_combined.md + # Fork PRs can influence report content and this job posts in base-repo context, so + # neutralize @-mentions (insert a zero-width space) to prevent notification abuse. + zwsp=$(printf '\342\200\213') + sed -i -E "s/@([A-Za-z0-9_-])/@${zwsp}\1/g" hil_combined.md cat hil_combined.md echo "found=true" >> $GITHUB_OUTPUT From d60acb61df3432580f15a73b220a4b4f6a806436 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 22 Jun 2026 13:09:47 +0700 Subject: [PATCH 3/4] ci: resolve PR number from trusted workflow_run metadata (security) 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 --- .github/workflows/build.yml | 23 +--------- .github/workflows/pr_comment.yml | 75 ++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 54 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f8ffe28ba5..c8c597e503 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -171,10 +171,6 @@ jobs: mv metrics_compare.md $COMPARE_FILE gh release upload $CURR_TAG metrics.json $COMPARE_FILE - - name: Save PR number - if: github.event_name == 'pull_request' - run: echo ${{ github.event.number }} > pr_number.txt - - name: Upload Metrics Comment Artifact if: github.event_name == 'pull_request' uses: actions/upload-artifact@v7 @@ -183,7 +179,6 @@ jobs: path: | metrics_compare.md metrics.json - pr_number.txt - name: Post Code Metrics as PR Comment if: (github.event_name == 'workflow_dispatch') || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false) @@ -348,20 +343,12 @@ jobs: exit 1 fi) - - name: Save PR number (carried with the HIL report so the comment job is self-contained) - if: always() && github.event_name == 'pull_request' - run: | - mkdir -p "${{ env.HIL_REPORT_DIR }}" - echo ${{ github.event.number }} > "${{ env.HIL_REPORT_DIR }}/pr_number.txt" - - name: Upload HIL report if: always() && github.event_name == 'pull_request' uses: actions/upload-artifact@v7 with: name: hil-report-${{ matrix.display }} - path: | - ${{ env.HIL_REPORT_DIR }}/hil_report.md - ${{ env.HIL_REPORT_DIR }}/pr_number.txt + path: ${{ env.HIL_REPORT_DIR }}/hil_report.md if-no-files-found: ignore overwrite: true @@ -420,17 +407,11 @@ jobs: run: | python3 test/hil/hil_test.py hfp.json - - name: Save PR number (carried with the HIL report so the comment job is self-contained) - if: always() && github.event_name == 'pull_request' - run: echo ${{ github.event.number }} > pr_number.txt - - name: Upload HIL report if: always() && github.event_name == 'pull_request' uses: actions/upload-artifact@v7 with: name: hil-report-hfp-iar - path: | - hil_report.md - pr_number.txt + path: hil_report.md if-no-files-found: ignore overwrite: true diff --git a/.github/workflows/pr_comment.yml b/.github/workflows/pr_comment.yml index 4db30762c8..0d229e3301 100644 --- a/.github/workflows/pr_comment.yml +++ b/.github/workflows/pr_comment.yml @@ -7,11 +7,41 @@ on: - completed jobs: - metrics-comment: + # Resolve the PR number from trusted workflow_run metadata, NOT from build artifacts: a forked PR + # controls its own Build run and could plant any number, which the privileged jobs below would + # then post to. Same-repo PRs populate workflow_run.pull_requests; for forks it is empty, so look + # the PR up by the trusted head SHA. + pr_number: + if: github.event.workflow_run.event == 'pull_request' runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + outputs: + number: ${{ steps.resolve.outputs.number }} + steps: + - name: Resolve PR number + id: resolve + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SAME_REPO_PR: ${{ github.event.workflow_run.pull_requests[0].number }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + if [ -n "$SAME_REPO_PR" ]; then + echo "number=$SAME_REPO_PR" >> "$GITHUB_OUTPUT" + else + # Fork PR: match the trusted head SHA to an open PR in this repo. + num=$(gh api "repos/${{ github.repository }}/commits/$HEAD_SHA/pulls" \ + --jq '[.[] | select(.state == "open" and .head.sha == env.HEAD_SHA)][0].number // empty') + echo "number=$num" >> "$GITHUB_OUTPUT" + fi + + metrics-comment: + needs: pr_number if: > - github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion == 'success' + github.event.workflow_run.conclusion == 'success' && + needs.pr_number.outputs.number != '' + runs-on: ubuntu-latest permissions: actions: read pull-requests: write @@ -25,35 +55,26 @@ jobs: # Best-effort: docs-only PRs skip code-metrics, so the artifact may be absent. continue-on-error: true - - name: Read PR Number - id: pr_number - run: | - if [ -f pr_number.txt ]; then - echo "number=$(cat pr_number.txt)" >> $GITHUB_OUTPUT - fi - - name: Post Code Metrics as PR Comment - if: steps.pr_number.outputs.number != '' + if: hashFiles('metrics_compare.md') != '' uses: marocchino/sticky-pull-request-comment@v2 with: header: code-metrics path: metrics_compare.md - number: ${{ steps.pr_number.outputs.number }} + number: ${{ needs.pr_number.outputs.number }} # --------------------------------------- # Combine the rigs' HIL reports into one sticky PR comment (one table per rig). # Runs here (workflow_run / base-repo context) rather than in build.yml so it also works on # forked PRs, whose build-side GITHUB_TOKEN is read-only and cannot post comments. Posts even # on build/HIL failure (when the report matters most); skips only on cancellation. - # workflow_run.pull_requests is empty for forks, so each hil-report-* artifact carries its own - # pr_number.txt (written in build.yml); the comment is self-contained and survives an unrelated - # build/metrics failure on both same-repo and forked PRs. # --------------------------------------- hil-comment: - runs-on: ubuntu-latest + needs: pr_number if: > - github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion != 'cancelled' + github.event.workflow_run.conclusion != 'cancelled' && + needs.pr_number.outputs.number != '' + runs-on: ubuntu-latest permissions: actions: read pull-requests: write @@ -67,18 +88,6 @@ jobs: path: hil-reports continue-on-error: true - - name: Read PR Number - id: pr_number - run: | - # Each hil-report-* artifact carries its own pr_number.txt (see build.yml), so any rig's - # copy works and they are all the same number. - for f in hil-reports/hil-report-*/pr_number.txt; do - if [ -f "$f" ]; then - echo "number=$(cat "$f")" >> $GITHUB_OUTPUT - break - fi - done - - name: Combine rig reports (one table per rig) id: combine run: | @@ -104,12 +113,12 @@ jobs: zwsp=$(printf '\342\200\213') sed -i -E "s/@([A-Za-z0-9_-])/@${zwsp}\1/g" hil_combined.md cat hil_combined.md - echo "found=true" >> $GITHUB_OUTPUT + echo "found=true" >> "$GITHUB_OUTPUT" - name: Post HIL report as sticky PR comment - if: steps.combine.outputs.found == 'true' && steps.pr_number.outputs.number != '' + if: steps.combine.outputs.found == 'true' uses: marocchino/sticky-pull-request-comment@v2 with: header: hil-report path: hil_combined.md - number: ${{ steps.pr_number.outputs.number }} + number: ${{ needs.pr_number.outputs.number }} From ebf9d8a12dc9cba8574c33ef9561727dedac4777 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 22 Jun 2026 15:05:04 +0700 Subject: [PATCH 4/4] ci: make PR-number resolution robust (review feedback) Copilot flagged that the fork lookup 'gh api commits//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=: 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 --- .github/workflows/pr_comment.yml | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr_comment.yml b/.github/workflows/pr_comment.yml index 0d229e3301..4d50817b4a 100644 --- a/.github/workflows/pr_comment.yml +++ b/.github/workflows/pr_comment.yml @@ -24,17 +24,24 @@ jobs: id: resolve env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SAME_REPO_PR: ${{ github.event.workflow_run.pull_requests[0].number }} + REPO: ${{ github.repository }} HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + HEAD_REPO: ${{ github.event.workflow_run.head_repository.full_name }} + PRS_JSON: ${{ toJSON(github.event.workflow_run.pull_requests) }} run: | - if [ -n "$SAME_REPO_PR" ]; then - echo "number=$SAME_REPO_PR" >> "$GITHUB_OUTPUT" - else - # Fork PR: match the trusted head SHA to an open PR in this repo. - num=$(gh api "repos/${{ github.repository }}/commits/$HEAD_SHA/pulls" \ - --jq '[.[] | select(.state == "open" and .head.sha == env.HEAD_SHA)][0].number // empty') - echo "number=$num" >> "$GITHUB_OUTPUT" + # Every lookup is best-effort: on any miss the number stays empty and the comment jobs + # below simply skip (never a failed check). + # Same-repo PRs: workflow_run.pull_requests is populated. + num=$(printf '%s' "$PRS_JSON" | jq -r '.[0].number // empty') + # Fork PRs: pull_requests is empty. Find the open PR by its trusted head ref and confirm + # its head SHA matches the built commit. + if [ -z "$num" ] && [ -n "$HEAD_BRANCH" ] && [ -n "$HEAD_REPO" ]; then + num=$(gh api --method GET "repos/$REPO/pulls" \ + -f state=open -f head="${HEAD_REPO%%/*}:$HEAD_BRANCH" \ + --jq '[.[] | select(.head.sha == env.HEAD_SHA)][0].number // empty' 2>/dev/null || true) fi + echo "number=$num" >> "$GITHUB_OUTPUT" metrics-comment: needs: pr_number