ci(pr-review-poster): add line-anchored review poster and migrate clang-tidy (#27028)

* ci(pr-review-poster): add line-anchored review poster and migrate clang-tidy

Adds a generic PR review-comment poster as a sibling of the issue-comment
poster from #27021. Replaces platisd/clang-tidy-pr-comments@v1 in the
Static Analysis workflow with an in-tree, fork-friendly producer + poster
pair so fork PRs get inline clang-tidy annotations on the Files changed
tab without trusting a third-party action with a write token.

Architecture mirrors pr-comment-poster: a producer (clang-tidy.yml) runs
inside the px4-dev container and writes a `pr-review` artifact containing
manifest.json and a baked comments.json. A separate workflow_run-triggered
poster runs on ubuntu-latest with the base-repo write token, validates the
artifact, dismisses any stale matching review, and posts a fresh review
on the target PR. The poster never checks out PR code and only ever reads
two opaque JSON files from the artifact.

Stale-review dismissal is restricted to reviews authored by
github-actions[bot] AND whose body contains the producer's marker. A fork
cannot impersonate the bot login or inject the marker into a human
reviewer's body, so the poster can never dismiss a human review. APPROVE
events are explicitly forbidden so a bot cannot approve a pull request.

To avoid duplicating ~120 lines of HTTP plumbing between the two posters,
the GitHub REST helpers (single-request, pagination, error handling) are
extracted into Tools/ci/_github_helpers.py with a small GitHubClient
class. The existing pr-comment-poster.py is refactored to use it; net
change is roughly -80 lines on that script. The shared module is
sparse-checked-out alongside each poster script and is stdlib only.

The clang-tidy producer reuses MIT-licensed translation logic from
platisd/clang-tidy-pr-comments (generate_review_comments,
reorder_diagnostics, get_diff_line_ranges_per_file and helpers) under a
preserved attribution header. The HTTP layer is rewritten on top of
_github_helpers so the producer does not pull in `requests`. Conversation
resolution (the GraphQL path) is intentionally dropped for v1.

clang-tidy.yml now produces the pr-review artifact in the same job as
the build, so the cross-runner compile_commands.json hand-off and
workspace-path rewriting are no longer needed and the
post_clang_tidy_comments job is removed.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

* ci(workflows): bump action versions to clear Node 20 deprecation

GitHub has deprecated the Node 20 runtime for Actions as of
September 16, 2026. Bump the pinned action versions in the three poster
workflows to the latest majors, all of which run on Node 24:

  actions/checkout         v4 -> v6
  actions/github-script    v7 -> v8
  actions/upload-artifact  v4 -> v7

No behavior changes on our side: upload-artifact v5/v6/v7 only added an
optional direct-file-upload mode we do not use, and checkout v5/v6 are
runtime-only bumps. The security-invariant comment headers in both
poster workflows are updated to reference the new version so they stay
accurate.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

* ci(pr-posters): skip job when producer was not a pull_request event

Both poster workflows previously ran on every workflow_run completion of
their listed producers and then silently no-oped inside the script when
the triggering producer run was a push-to-main (or any other non-PR
event). That made the UI ambiguous: the job was always green, never
showed the reason it did nothing, and looked like a failure whenever
someone clicked in looking for the comment that was never there.

Gate the job at the workflow level on
github.event.workflow_run.event == 'pull_request'. Non-PR producer runs
now surface as a clean "Skipped" entry in the run list, which is
self-explanatory and needs no in-script summary plumbing.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

---------

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

Author: mrpollo

.github/workflows/clang-tidy.yml

@@ -21,11 +21,12 @@ jobs:
     runs-on: [runs-on, runner=16cpu-linux-x64, "run-id=${{ github.run_id }}", "extras=s3-cache"]
     container:
       image: ghcr.io/px4/px4-dev:v1.17.0-rc2
-    outputs:
-      has_findings: ${{ steps.clang_tidy.outputs.has_findings }}
+    permissions:
+      contents: read
+      pull-requests: read
     steps:
     - uses: runs-on/action@v2
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v6
       with:
         fetch-depth: 0
         fetch-tags: true
@@ -47,96 +48,50 @@ jobs:
       run: |
         if [ "${{ github.event_name }}" != "pull_request" ]; then
           make -j$(nproc) clang-tidy
-          echo "has_findings=false" >> $GITHUB_OUTPUT
         else
-          output=$(python3 Tools/ci/run-clang-tidy-pr.py origin/${{ github.base_ref }} 2>&1)
-          exit_code=$?
-          echo "$output"
-          # Helper prints this message on both early-exit paths
-          # (no changed C++ files, or no matching TUs in the compile DB)
-          if echo "$output" | grep -q "skipping clang-tidy"; then
-            echo "has_findings=false" >> $GITHUB_OUTPUT
-          else
-            echo "has_findings=true" >> $GITHUB_OUTPUT
-          fi
-          exit $exit_code
+          python3 Tools/ci/run-clang-tidy-pr.py origin/${{ github.base_ref }}
         fi
 
-    - name: Upload compile_commands.json
+    # On PRs, also produce a `pr-review` artifact for the PR Review Poster
+    # workflow to consume. clang-tidy-diff-18 emits a unified fixes.yml that
+    # the producer script translates into line-anchored review comments.
+    # Running this inside the same container as the build means there is no
+    # workspace-path rewriting and no cross-runner artifact handoff.
+    - name: Export clang-tidy fixes for PR review
+      if: always() && github.event_name == 'pull_request'
+      run: |
+        mkdir -p pr-review
+        git diff -U0 origin/${{ github.base_ref }}...HEAD \
+          | clang-tidy-diff-18.py -p1 \
+              -path build/px4_sitl_default-clang \
+              -export-fixes pr-review/fixes.yml \
+              -j0 || true
+
+    - name: Build pr-review artifact
       if: always() && github.event_name == 'pull_request'
-      uses: actions/upload-artifact@v4
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      run: |
+        python3 Tools/ci/clang-tidy-fixes-to-review.py \
+          --fixes pr-review/fixes.yml \
+          --repo-root "$GITHUB_WORKSPACE" \
+          --repo "$GITHUB_REPOSITORY" \
+          --pr-number "${{ github.event.pull_request.number }}" \
+          --commit-sha "${{ github.event.pull_request.head.sha }}" \
+          --out-dir pr-review \
+          --event REQUEST_CHANGES
+
+    - name: Upload pr-review artifact
+      if: always() && github.event_name == 'pull_request'
+      uses: actions/upload-artifact@v7
       with:
-        name: compile-commands
-        path: build/px4_sitl_default-clang/compile_commands.json
+        name: pr-review
+        path: |
+          pr-review/manifest.json
+          pr-review/comments.json
         retention-days: 1
 
     - uses: ./.github/actions/save-ccache
       if: always()
       with:
         cache-primary-key: ${{ steps.ccache.outputs.cache-primary-key }}
-
-  post_clang_tidy_comments:
-    name: Clang-Tidy PR Annotations
-    needs: [clang_tidy]
-    runs-on: ubuntu-latest
-    permissions:
-      pull-requests: write
-      contents: read
-    if: >-
-      always()
-      && github.event.pull_request
-      && github.event.pull_request.head.repo.full_name == github.repository
-      && (needs.clang_tidy.result == 'success' || needs.clang_tidy.result == 'failure')
-      && needs.clang_tidy.outputs.has_findings == 'true'
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
-
-      - name: Install clang-tools (for clang-tidy-diff)
-        run: sudo apt-get install -y clang-tools
-
-      - name: Download compile_commands.json
-        uses: actions/download-artifact@v4
-        with:
-          name: compile-commands
-          path: build/px4_sitl_default-clang
-
-      - name: Run clang-tidy-diff and export fixes
-        run: |
-          # WHY WE REWRITE compile_commands.json PATHS
-          #
-          # The clang_tidy job runs on a RunsOn/AWS runner where the workspace
-          # root is "/__w/PX4-Autopilot/PX4-Autopilot". All absolute paths baked
-          # into compile_commands.json (the "file" and "directory" fields, and
-          # every -I include path in "command") use that prefix.
-          #
-          # This annotation job runs on a GitHub-hosted runner where the
-          # workspace root is "/home/runner/work/PX4-Autopilot/PX4-Autopilot".
-          # When clang-tidy-diff invokes clang-tidy, it reads the "directory"
-          # field from compile_commands.json and calls chdir() on it. Since
-          # the AWS-style path does not exist here, clang-tidy crashes with:
-          #   LLVM ERROR: Cannot chdir into "/__w/.../build/px4_sitl_default-clang"
-          # and silently produces an empty fixes.yml, so no annotations are posted.
-          #
-          # Fix: rewrite all occurrences of the AWS workspace prefix to the
-          # current runner workspace ($GITHUB_WORKSPACE) before invoking
-          # clang-tidy-diff. Safe because compile_commands.json is a local
-          # scratch file pulled from the artifact; no source file is modified.
-          sed -i "s|/__w/PX4-Autopilot/PX4-Autopilot|${GITHUB_WORKSPACE}|g" \
-            build/px4_sitl_default-clang/compile_commands.json
-
-          mkdir -p clang-tidy-result
-          git diff -U0 origin/${{ github.base_ref }}...HEAD \
-            | clang-tidy-diff-18.py -p1 \
-                -path build/px4_sitl_default-clang \
-                -export-fixes clang-tidy-result/fixes.yml \
-                -j0 || true
-
-      - name: Annotate PR with clang-tidy findings
-        uses: platisd/clang-tidy-pr-comments@v1
-        with:
-          github_token: ${{ secrets.GITHUB_TOKEN }}
-          clang_tidy_fixes: clang-tidy-result/fixes.yml
-          request_changes: true
-          suggestions_per_comment: 10

.github/workflows/pr-comment-poster.yml

@@ -40,12 +40,13 @@ name: PR Comment Poster
 #      manifest and body itself, NOT copied fork-controlled content into
 #      them. Producer workflows are responsible for that.
 #
-#   6. `actions/checkout@v4` below uses NO ref (so it pulls the base branch,
+#   6. `actions/checkout@v6` below uses NO ref (so it pulls the base branch,
 #      the default-branch commit this workflow file was loaded from) AND uses
-#      sparse-checkout to materialize ONLY Tools/ci/pr-comment-poster.py. The
-#      rest of the repo never touches the workspace. This is safe: the only
-#      file the job executes is a base-repo Python script that was reviewed
-#      through normal code review, never anything from the PR.
+#      sparse-checkout to materialize ONLY Tools/ci/pr-comment-poster.py and
+#      its stdlib-only helper module Tools/ci/_github_helpers.py. The rest of
+#      the repo never touches the workspace. This is safe: the only files the
+#      job executes are base-repo Python scripts that were reviewed through
+#      normal code review, never anything from the PR.
 #
 # ==============================================================================
 # ARTIFACT CONTRACT
@@ -91,22 +92,29 @@ jobs:
   post:
     name: Post PR Comment
     runs-on: ubuntu-latest
-    if: github.event.workflow_run.conclusion != 'cancelled'
+    # Only run for pull_request producer runs. Push-to-main and other
+    # non-PR triggers would have no comment to post, and silently no-oping
+    # inside the script made it look like the poster was broken. Gating at
+    # the job level surfaces those as a clean "Skipped" in the UI instead.
+    if: >-
+      github.event.workflow_run.conclusion != 'cancelled'
+      && github.event.workflow_run.event == 'pull_request'
     steps:
       # Checkout runs first so the poster script is available AND so that
-      # actions/checkout@v4's default clean step does not delete the artifact
+      # actions/checkout@v6's default clean step does not delete the artifact
       # zip that the next step writes into the workspace. Sparse-checkout
       # restricts the materialized tree to just the poster script.
       - name: Checkout poster script only
-        uses: actions/checkout@v4
+        uses: actions/checkout@v6
         with:
           sparse-checkout: |
             Tools/ci/pr-comment-poster.py
+            Tools/ci/_github_helpers.py
           sparse-checkout-cone-mode: false
 
       - name: Download pr-comment artifact
         id: download
-        uses: actions/github-script@v7
+        uses: actions/github-script@v8
         with:
           script: |
             const artifacts = await github.rest.actions.listWorkflowRunArtifacts({

.github/workflows/pr-review-poster.yml

@@ -0,0 +1,177 @@
+name: PR Review Poster
+
+# Generic PR review-comment poster. Sibling of "PR Comment Poster": that
+# workflow posts sticky issue-style comments, this one posts line-anchored
+# review comments on the "Files changed" tab. Any analysis workflow that
+# wants to flag specific lines can produce a `pr-review` artifact and this
+# workflow will dismiss any stale matching review and post a fresh one.
+# Designed so analysis jobs running on untrusted fork PRs can still get
+# their inline annotations posted back to the PR.
+#
+# ==============================================================================
+# SECURITY INVARIANTS
+# ==============================================================================
+# This workflow runs on `workflow_run` which means it runs in the BASE REPO
+# context with a WRITE token, even when the triggering PR comes from a fork.
+# That is the entire reason it exists, and also the reason it is a loaded
+# footgun. Anyone modifying this file MUST preserve the following invariants:
+#
+#   1. NEVER check out PR code. No `actions/checkout` with a ref. No git clone
+#      of a fork branch. No execution of scripts from the downloaded artifact.
+#      The ONLY things read from the artifact are `manifest.json` and
+#      `comments.json`, and both are treated as opaque data (JSON parsed by
+#      the poster script and the comment fields posted via the GitHub API).
+#
+#   2. `pr_number` is validated to be a positive integer before use.
+#      `marker` is validated to be printable ASCII only before use.
+#      `commit_sha` is validated to be 40 lowercase hex characters.
+#      `event` is validated against an allowlist of `COMMENT` and
+#      `REQUEST_CHANGES`. `APPROVE` is intentionally forbidden so a bot
+#      cannot approve a pull request. Validation happens inside
+#      Tools/ci/pr-review-poster.py which is checked out from the base
+#      branch, not from the artifact.
+#
+#   3. Comment bodies and the optional summary are passed to the GitHub API
+#      as JSON fields, never interpolated into a shell command string.
+#
+#   4. This workflow file lives on the default branch. `workflow_run` only
+#      loads workflow files from the default branch, so a fork cannot modify
+#      THIS file as part of a PR. The fork CAN cause this workflow to fire
+#      by triggering a producer workflow that uploads a `pr-review`
+#      artifact. That is intended.
+#
+#   5. The artifact-name filter (`pr-review`) is the only gate on which
+#      workflow runs get processed. Any workflow in this repo that uploads
+#      an artifact named `pr-review` is trusted to have written the
+#      manifest and comments itself, NOT copied fork-controlled content
+#      into them. Producer workflows are responsible for that.
+#
+#   6. `actions/checkout@v6` below uses NO ref (so it pulls the base branch,
+#      the default-branch commit this workflow file was loaded from) AND
+#      uses sparse-checkout to materialize ONLY
+#      Tools/ci/pr-review-poster.py and its stdlib-only helper module
+#      Tools/ci/_github_helpers.py. The rest of the repo never touches the
+#      workspace. This is safe: the only files the job executes are
+#      base-repo Python scripts that were reviewed through normal code
+#      review, never anything from the PR.
+#
+#   7. Stale-review dismissal is restricted to reviews whose AUTHOR is
+#      `github-actions[bot]` AND whose body contains the producer's
+#      marker. A fork PR cannot impersonate the bot login, and cannot
+#      inject the marker into a human reviewer's body without API
+#      access. Both filters together prevent the poster from ever
+#      dismissing a human review.
+#
+# ==============================================================================
+# ARTIFACT CONTRACT
+# ==============================================================================
+# Producers upload an artifact named exactly `pr-review` containing:
+#
+#   manifest.json:
+#     {
+#       "pr_number":  12345,                                      // required, int > 0
+#       "marker":     "<!-- pr-review-poster:clang-tidy -->",     // required, printable ASCII
+#       "event":      "REQUEST_CHANGES",                          // required, "COMMENT" | "REQUEST_CHANGES"
+#       "commit_sha": "0123456789abcdef0123456789abcdef01234567", // required, 40 hex chars
+#       "summary":    "Optional review summary text"              // optional
+#     }
+#
+#   comments.json: JSON array of line-anchored review comment objects:
+#     [
+#       {"path": "src/foo.cpp", "line": 42, "side": "RIGHT", "body": "..."},
+#       {"path": "src/bar.hpp", "start_line": 10, "line": 15,
+#        "side": "RIGHT", "start_side": "RIGHT", "body": "..."}
+#     ]
+#
+# The `marker` string is used to find an existing matching review to
+# dismiss before posting a new one. It MUST be unique per producer (e.g.
+# include the producer name).
+#
+# Producers MUST write `pr_number` and `commit_sha` from their own
+# workflow context (`github.event.pull_request.number` and
+# `github.event.pull_request.head.sha`) and MUST NOT read either from any
+# fork-controlled source.
+
+on:
+  workflow_run:
+    # Producers that may upload a `pr-review` artifact. When a new
+    # producer is wired up, add its workflow name here. Runs of workflows
+    # not in this list will never trigger the poster. Every run of a
+    # listed workflow will trigger the poster, which will no-op if no
+    # `pr-review` artifact exists.
+    workflows:
+      - "Static Analysis"
+    types:
+      - completed
+
+permissions:
+  pull-requests: write
+  actions: read
+  contents: read
+
+jobs:
+  post:
+    name: Post PR Review
+    runs-on: ubuntu-latest
+    # Only run for pull_request producer runs. Push-to-main and other
+    # non-PR triggers have no review to post, so gating at the job level
+    # surfaces those as a clean "Skipped" in the UI instead of a
+    # silent no-op buried inside the script.
+    if: >-
+      github.event.workflow_run.conclusion != 'cancelled'
+      && github.event.workflow_run.event == 'pull_request'
+    steps:
+      # Checkout runs first so the poster scripts are available AND so
+      # that actions/checkout@v6's default clean step does not delete the
+      # artifact zip that the next step writes into the workspace.
+      # Sparse-checkout restricts the materialized tree to just the
+      # poster script and its stdlib helper module.
+      - name: Checkout poster script only
+        uses: actions/checkout@v6
+        with:
+          sparse-checkout: |
+            Tools/ci/pr-review-poster.py
+            Tools/ci/_github_helpers.py
+          sparse-checkout-cone-mode: false
+
+      - name: Download pr-review artifact
+        id: download
+        uses: actions/github-script@v8
+        with:
+          script: |
+            const artifacts = await github.rest.actions.listWorkflowRunArtifacts({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              run_id: context.payload.workflow_run.id,
+            });
+            const match = artifacts.data.artifacts.find(a => a.name === 'pr-review');
+            if (!match) {
+              core.info('No pr-review artifact on this run; nothing to post.');
+              core.setOutput('found', 'false');
+              return;
+            }
+            const download = await github.rest.actions.downloadArtifact({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              artifact_id: match.id,
+              archive_format: 'zip',
+            });
+            const fs = require('fs');
+            fs.writeFileSync('pr-review.zip', Buffer.from(download.data));
+            core.setOutput('found', 'true');
+
+      - name: Unpack artifact
+        if: steps.download.outputs.found == 'true'
+        run: |
+          mkdir -p pr-review
+          unzip -q pr-review.zip -d pr-review
+
+      - name: Validate artifact
+        if: steps.download.outputs.found == 'true'
+        run: python3 Tools/ci/pr-review-poster.py validate pr-review
+
+      - name: Post PR review
+        if: steps.download.outputs.found == 'true'
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: python3 Tools/ci/pr-review-poster.py post pr-review