feat(pr-pipeline): run the security review on pr-review-cli with key-first funding

.changes/pr-pipeline/feat-security-review-on-pr-review-cli-5b1e9d47.md

@@ -0,0 +1,9 @@
+---
+bump: minor
+---
+
+### Changed
+
+- The security review lane (`pr-review-security-hosted.yml`) runs on ci-workflows' `pr-review-cli.yml` (v0.63.1), lane `security`: the reviews role's API key first, and its subscription token only after the key hands off or when the repository variable `REVIEWS_KEY_EXHAUSTED` is `true`. The check stays `pr-review-security / security-review`; the review now posts as `github-actions[bot]`, each comment ending in its `claude-lane: claude-security-review` footer. A newer push no longer cancels a running review: it waits for it.
+- `address-feedback` and `pr-feedback-dispatch.yml` treat a `github-actions[bot]` comment as the reviewer's when its last non-blank line is a review lane's footer and its review is `github-actions[bot]`'s; `claude[bot]` findings are unchanged.
+- One funding role, `reviews`, backs both review lanes and is keyed; the `reviews-security` role is gone.

.changes/source-control/feat-security-review-on-pr-review-cli-a83c06f2.md

@@ -0,0 +1,9 @@
+---
+bump: minor
+---
+
+### Changed
+
+- The merge gate's review evidence accepts the security lane on ci-workflows' `pr-review-cli.yml`: a completion record ending in `account=fallback`, and a `github-actions[bot]` thread or summary comment whose last non-blank line is the lane's `claude-lane:` footer. A reviewed-head marker is never read a second time as a summary comment.
+- `review-venue.sh` finds the security lane on ci-workflows' `pr-review-cli.yml` when the caller's path contains `security`. It also reads a `<lane>: failure-class=skipped-validation` red as no verdict.
+- A `github-actions` summary comment counts for its lane only when it carries the REST bot identity: login `github-actions[bot]` and type `Bot`.

.github/lane-funding.json

@@ -7,21 +7,11 @@
       "secretPrefix": "REVIEWS",
       "tokenSecret": "REVIEWS_CLAUDE_CODE_OAUTH_TOKEN",
       "keySecret": "REVIEWS_CLAUDE_API_KEY",
-      "keyed": false,
-      "category": "review",
-      "lanes": [
-        { "id": "pr-review", "workflow": "pr-review-hosted.yml", "launcher": "claude-code-action", "currentSecrets": ["CLAUDE_CODE_OAUTH_TOKEN"] }
-      ]
-    },
-    {
-      "id": "reviews-security",
-      "secretPrefix": "REVIEWS_SECURITY",
-      "tokenSecret": "REVIEWS_SECURITY_CLAUDE_CODE_OAUTH_TOKEN",
-      "keySecret": "REVIEWS_SECURITY_CLAUDE_API_KEY",
-      "keyed": false,
+      "keyed": true,
       "category": "review",
       "lanes": [
-        { "id": "pr-review-security", "workflow": "pr-review-security-hosted.yml", "launcher": "claude-code-action", "currentSecrets": ["CLAUDE_CODE_OAUTH_TOKEN"] }
+        { "id": "pr-review", "workflow": "pr-review-hosted.yml", "launcher": "claude-code-action", "currentSecrets": ["CLAUDE_CODE_OAUTH_TOKEN"] },
+        { "id": "pr-review-security", "workflow": "pr-review-security-hosted.yml", "launcher": "run-claude-cli", "currentSecrets": ["REVIEWS_CLAUDE_API_KEY", "REVIEWS_CLAUDE_CODE_OAUTH_TOKEN"] }
       ]
     },
     {

.github/scripts/pr-unblock/address-feedback.mjs

@@ -51,6 +51,20 @@ const FILES_CAP = 3000;
 
 const firstLine = (body) => String(body ?? "").split("\n")[0].replace(/\r$/, "");
 
+// A review lane's own comment as the pr-review-cli reusable posts it: by
+// github-actions[bot] (MARKER_AUTHOR_ID), its last non-blank line the lane's
+// `claude-lane: <lane>` footer. Any other github-actions[bot] comment is not a
+// finding.
+const LANE_FOOTER = /^`?claude-lane: (?:claude-review|claude-security-review)`?$/;
+export const isLaneComment = (comment) =>
+  comment.authorId === MARKER_AUTHOR_ID &&
+  LANE_FOOTER.test(
+    String(comment.body ?? "")
+      .split("\n")
+      .map((line) => line.trim())
+      .findLast((line) => line !== "") ?? "",
+  );
+
 // Round markers the lanes App posted; any other author's copy is ignored.
 export const isRoundMarker = (comment, appId) => comment.user?.id === appId && firstLine(comment.body) === ROUND_MARKER;
 
@@ -258,7 +272,8 @@ export function humanGatedLabelFrom(path) {
 
 // Who wrote each comment of a thread, or null when it is not one to work.
 // `comments` are oldest first, each { authorId, createdAt, body, lastEditedAt,
-// editorId }. The first is the reviewer bot's, or a verifier note
+// editorId }. The first is the reviewer bot's, a lane comment (isLaneComment,
+// which counts as the reviewer bot's throughout), or a verifier note
 // (github-actions[bot]'s, its first line VERIFY_NOTE_MARKER), and one of these
 // holds:
 // - every comment is the reviewer bot's, but a verifier note's own;
@@ -278,10 +293,11 @@ export function humanGatedLabelFrom(path) {
 // escalation when there is one.
 export function threadAuthors(comments, { reviewerId, appId, trustedHumans = new Set(), requesters = null, fixers = requesters ?? new Set() }) {
   const verifier = comments.length > 0 && comments[0].authorId === MARKER_AUTHOR_ID && firstLine(comments[0].body) === VERIFY_NOTE_MARKER;
-  if (comments.length === 0 || (comments[0].authorId !== reviewerId && !verifier)) {
+  const reviewer = (comment) => comment.authorId === reviewerId || isLaneComment(comment);
+  if (comments.length === 0 || (!reviewer(comments[0]) && !verifier)) {
     return null;
   }
-  const finder = (comment, i) => (i === 0 && verifier ? { author: "verifier" } : comment.authorId === reviewerId ? { author: "reviewer" } : null);
+  const finder = (comment, i) => (i === 0 && verifier ? { author: "verifier" } : reviewer(comment) ? { author: "reviewer" } : null);
   if (comments.every((comment, i) => finder(comment, i) !== null)) {
     return requesters ? null : comments.map(finder);
   }
@@ -368,7 +384,8 @@ export function keepThreads({ threads, reviewComments, reviews, heads, reviewerI
     // escalation) asks for the thread, so its review may be on an earlier head.
     const directed = authors.some((a) => a.direction === true);
     const review = reviewsById.get(rows[0].pull_request_review_id);
-    const reviewAuthor = authors[0].author === "verifier" ? MARKER_AUTHOR_ID : reviewerId;
+    // A verifier note's or a lane comment's review is github-actions[bot]'s.
+    const reviewAuthor = authors[0].author === "verifier" || rows[0].user?.id === MARKER_AUTHOR_ID ? MARKER_AUTHOR_ID : reviewerId;
     if (review?.user?.id !== reviewAuthor || (!directed && !heads.has(review.commit_id))) {
       continue;
     }

.github/scripts/pr-unblock/address-feedback.test.mjs

@@ -420,6 +420,40 @@ test("a github-actions[bot] thread without the note marker, or in another accoun
   assert.equal((await run(forged)).decision, "skip");
 });
 
+// The pr-review-cli reusable posts each finding as github-actions[bot], the
+// body ending `\n\nclaude-lane: <lane>` (ci-workflows
+// .github/actions/publish-review/publish.cjs commentBody at v0.63.0).
+const LANE_FINDING = "severity: IMPORTANT\nThe input reaches the shell unquoted.\n\nclaude-lane: claude-security-review";
+const laneFinding = (body, reviewer = ACTIONS_BOT) =>
+  fakeApi({
+    issueComments: [{ id: 1, user: { id: ACTIONS_BOT }, body: marker("claude-security-review", HEAD) }],
+    reviewComments: [comment(100, ACTIONS_BOT, { body })],
+    reviews: [{ id: 10, user: { id: reviewer }, commit_id: HEAD }],
+  });
+
+test("a github-actions[bot] finding ending in the lane footer is kept as the reviewer's", async () => {
+  const result = await run(laneFinding(LANE_FINDING));
+  assert.equal(result.decision, "proceed");
+  assert.deepEqual(result.threads[0].comments, [{ id: 100, author: "reviewer", body: LANE_FINDING }]);
+  // Trailing blank lines do not hide the footer.
+  assert.equal((await run(laneFinding(`${LANE_FINDING}\n\n`))).decision, "proceed");
+});
+
+test("a github-actions[bot] comment whose last line is not a review lane's footer is no finding", async () => {
+  for (const body of [
+    "The input reaches the shell unquoted.",
+    "claude-lane: claude-security-review\nThe footer is not the last line.",
+    "The input reaches the shell.\n\nclaude-lane: pr-verify",
+  ]) {
+    assert.equal((await run(laneFinding(body))).decision, "skip", body);
+  }
+  // The footer on another account's comment, or in a review another account
+  // submitted, opens nothing.
+  const human = fakeApi({ reviewComments: [comment(100, HUMAN, { body: LANE_FINDING })] });
+  assert.equal((await run(human)).decision, "skip");
+  assert.equal((await run(laneFinding(LANE_FINDING, REVIEWER))).decision, "skip");
+});
+
 test("a later github-actions[bot] comment on a verifier note is no finding", async () => {
   const result = await run(verifierNote({ replies: [comment(101, ACTIONS_BOT, { body: VERIFY_NOTE })] }));
   assert.equal(result.decision, "skip");

.github/scripts/pr-unblock/feedback-dispatch.mjs

@@ -73,10 +73,12 @@ export const DEFAULTS = Object.freeze({
     ".github/workflows/pr-review-security-hosted.yml",
   ]),
   // The lanes each review workflow names in its reviewed-head marker
-  // (melodic-software/ci-workflows pr-review.yml and pr-review-security.yml).
+  // (melodic-software/ci-workflows pr-review.yml and pr-review-cli.yml).
   markerLanes: Object.freeze(["claude-review", "claude-security-review"]),
   holdLabels: Object.freeze(["do-not-merge"]),
-  // claude[bot], the reviewer (.github/standards/trusted-actors/trusted-actors.json).
+  // claude[bot], the code lane's reviewer (.github/standards/trusted-actors/trusted-actors.json).
+  // pr-review-cli posts as github-actions[bot]; threadAuthors accepts those
+  // comments only when the lane footer is their last line.
   reviewerId: 209825114,
   // github-actions[bot], which writes the reviewed-head markers.
   markerAuthorId: 41898282,

.github/scripts/pr-unblock/feedback-dispatch.test.mjs

@@ -229,6 +229,26 @@ test("the security review workflow is an upstream too", async () => {
   assert.equal((await decide(api)).result.number, 42);
 });
 
+// A pr-review-cli finding: github-actions[bot]'s, its body ending in the lane
+// footer (ci-workflows publish-review publish.cjs commentBody at v0.63.0).
+const laneThread = (body) => ({
+  isResolved: false,
+  comments: { nodes: [{ author: ACTIONS_BOT_NODE, body }], pageInfo: { hasNextPage: false } },
+});
+
+test("a github-actions[bot] thread ending in the lane footer dispatches like the reviewer bot's", async () => {
+  const api = fakeApi({ threads: [laneThread("severity: IMPORTANT\nUnquoted input.\n\nclaude-lane: claude-security-review")] });
+  assert.equal((await decide(api)).result.number, 42);
+});
+
+test("a github-actions[bot] thread without a review lane's footer last dispatches nothing", async () => {
+  for (const body of ["Unquoted input.", "claude-lane: claude-security-review\nUnquoted input."]) {
+    const api = fakeApi({ threads: [laneThread(body)] });
+    assert.equal((await decide(api)).result, null, body);
+    assert.deepEqual(api.calls.dispatch, []);
+  }
+});
+
 for (const [label, options] of [
   ["the upstream is another workflow", { run: { path: ".github/workflows/ci.yml" } }],
   ["the upstream path names a review workflow at another ref", {

.github/workflows/pr-review-security-hosted.yml

@@ -7,32 +7,31 @@ name: pr-review-security-hosted
 # this copy can add the resolve-model and kill-switch jobs below. It differs
 # from the component in those jobs and these comments.
 #
-# Thin caller for the org's reusable security-review lane (advisory verdict,
-# fail-closed execution check). The reusable workflow owns the action pin,
-# model, and the secret-handling model; per its header, this caller owns the
-# triggers, concurrency, and the GITHUB_TOKEN permission grant.
+# Thin caller for the org's pr-review-cli reusable, lane `security` (advisory
+# verdict, fail-closed execution check). The reusable owns the launcher, the
+# credential order (the reviews role's API key, then its subscription token
+# after a handoff) and the secret-handling model; per its header, this caller
+# owns the triggers, concurrency, and the GITHUB_TOKEN permission grant.
 #
 # There is no workflow-level path gating. The reusable skips, with a green
-# check, a review whose files in scope are all documentation (its
-# `docs-only-paths` default: `docs/**/*.md`, READMEs and changelogs). A
-# `.github/claude-security-paths` file is no longer read.
+# check, a review whose files in scope are all documentation (the
+# `docs-only-paths` passed below). A `.github/claude-security-paths` file is
+# no longer read.
 #
 # NEVER add workflow-level `paths:`/`paths-ignore:` to this caller: a check
 # whose workflow never triggers is missing, not green, so a required check
 # would stay Pending forever, and the babysit merge gate, which waits for
 # `pr-review-security / security-review`, never arms.
 #
-# The reusable skips fork PRs (they receive no secrets and a read-only token)
-# and every `[bot]` actor whose sender id is not in `allowed-bot-sender-ids`
-# (below); a required check does NOT prove a fork PR was reviewed (see the
-# reusable's header). On a public repository the lane reviews only pull
-# requests from branches of this repository: fork and Dependabot pull requests
-# get no secrets.
+# The reusable skips fork PRs (they receive no secrets and a read-only token),
+# Dependabot, and every `[bot]` actor whose sender id is not in
+# `allowed-bot-sender-ids` (below); a required check does NOT prove a fork PR
+# was reviewed (see the reusable's header). On a public repository the lane
+# reviews only pull requests from branches of this repository: fork and
+# Dependabot pull requests get no secrets.
 #
-# RUNNER PLACEMENT: hosted shape, the sibling of
-# components/claude-lanes/claude-security-review.yml, differing from it only
-# in these comments, `runner`, the resolve-model and kill-switch jobs and the workflow `name:`. It is an interim with
-# the same removal trigger as the code-review caller. See that file's header.
+# RUNNER PLACEMENT: every job of the reusable runs on ubuntu-24.04; it takes
+# no runner input.
 
 on:
   pull_request:
@@ -41,25 +40,22 @@ on:
     # pull_request workflow while a PR conflicts with its base), narrowed by
     # the reusable to the files changed since the last completed review. The check
     # stays advisory: github-iac ADR 0011 forbids any organization ruleset
-    # from making an agentic review a merge or deployment prerequisite. With
-    # the per-PR group below cancelling, a push supersedes the in-flight review.
+    # from making an agentic review a merge or deployment prerequisite.
     #
-    # NO `merge_group:`: the pinned action cannot serve that event; see
+    # NO `merge_group:`: the reusable runs only on `pull_request`; see
     # melodic-software/ci-workflows#272 before enabling a merge queue.
     types: [opened, synchronize, ready_for_review, reopened]
 
 permissions:
   contents: read
 
-# Per-PR supersede group: a newer event for the same PR, a push included,
-# cancels the older run, so reviews of superseded heads do not stack. The
-# merge guard judges only the newest run of each check, so a cancelled run on
-# a superseded head does not block merge. The group is the
-# `${{ github.workflow }}`-prefixed canonical form the standards
-# `concurrency-policy` component requires.
+# Per-PR group: a newer event for the same PR waits for the running review
+# instead of cancelling it, because the guardrails merge guard reads a
+# cancelled run as red. The group is the `${{ github.workflow }}`-prefixed
+# canonical form the standards `concurrency-policy` component requires.
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
-  cancel-in-progress: true
+  cancel-in-progress: false
 
 jobs:
   # The review's model, turns and effort: the review-security route of the
@@ -114,7 +110,7 @@ jobs:
           SETTINGS_JSON: ${{ steps.models.outputs.settings-json }}
         run: |
           if [ -n "$SETTINGS_JSON" ]; then
-            echo "::error::the review-security profile sets subagent-model, which pr-review-security.yml does not support"
+            echo "::error::the review-security profile sets subagent-model, which pr-review-cli.yml does not support"
             exit 1
           fi
 
@@ -169,15 +165,16 @@ jobs:
     # A draft is work in progress and does not spend a review; the
     # ready_for_review trigger above is what delivers one when it is ready.
     if: ${{ !cancelled() && github.event.pull_request.draft == false && needs.kill-switch.outputs.proceed == 'true' }}
+    # The model jobs narrow this to contents: read and pull-requests: read;
+    # only the reusable's status job (the security-review check) writes.
     permissions:
-      contents: read         # checkout + read the diff
-      pull-requests: write   # post the security review, tracking comment and
-                             # the lane's reviewed-head marker comment
-      id-token: write        # OIDC, mints the Claude GitHub App token (required
-                             # even with an OAuth/API-key credential)
-    uses: melodic-software/ci-workflows/.github/workflows/pr-review-security.yml@2fe38c031867900824c164e33a140a335ed9ecf0 # v0.62.0
+      contents: read
+      pull-requests: write   # the review, its inline comments and the lane's
+                             # reviewed-head marker comment
+    uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@83b739f625b501ad40c65dee7168d836883195e1 # v0.63.1
     with:
-      runner: ubuntu-24.04
+      lane: security
+      publish: comments
       # Empty unless resolve-model succeeded, so a failed job's partial output
       # never reaches the review.
       model-args: ${{ needs.resolve-model.result == 'success' && needs.resolve-model.outputs.claude-args || '' }}
@@ -186,8 +183,14 @@ jobs:
       # reviewed. Where the repository does not set this variable the value is
       # empty and the reusable reviews no bot.
       allowed-bot-sender-ids: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }}
-    # Pass only the one named secret (least privilege) rather than
-    # `secrets: inherit`, which would forward every parent secret to the
-    # called workflow.
+      # A review whose files in scope all match is skipped with a green check.
+      # Agent instruction files never count as documentation.
+      docs-only-paths: |
+        docs/**/*.md
+        **/README.md
+        **/CHANGELOG.md
+    # The reviews role's key runs first; its subscription token only after the
+    # key hands off. Named secrets only, never `secrets: inherit`.
     secrets:
-      CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
+      CLAUDE_API_KEY: ${{ secrets.REVIEWS_CLAUDE_API_KEY }}
+      CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.REVIEWS_CLAUDE_CODE_OAUTH_TOKEN }}