feat: implement issue #1905 — [qa-lead reach 3/3] persona-runner ignores surface=pull_request — the router-served qa-lead advisory loses its test-surface, budget and already-advised gates

.github/workflows/lint.yml

@@ -232,6 +232,7 @@
             tests/dev-lead/unit/test_gemini_quota.bats \
             tests/test_qa_lead_test_surface.bats \
             tests/test_qa_lead_advisory_gate.bats \
+            tests/test_persona_pr_pregate.bats \
             tests/test_qa_lead_holdout_calibration.bats \
             tests/test_advisory_offline_mode.bats \
             tests/agent_ingress_reference.bats \
@@ -307,7 +308,7 @@
     timeout-minutes: 5
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
         # NOTE: persist-credentials is intentionally left at the default (true) —
         # vci_resolve_reusable needs `git fetch origin` to authenticate against this
         # private repo when resolving same-repo channel tags. Setting
         # persist-credentials: false would silently push every such caller into the

.github/workflows/persona-runner-reusable.yml

@@ -73,6 +73,25 @@
         required: false
         type: string
         default: 'advisory'
+      surface:
+        # Which surface the dispatch came from. Defaults to 'mention' so existing
+        # mention dispatches are unchanged. When 'pull_request', the persona's
+        # event pre-gate runs before the engine (#1905). The local
+        # qa-lead-pr-advisory.yml advise job now sets 'pull_request' too, so both
+        # its run and the router-served run share this workflow's per-(persona,
+        # repo,PR) concurrency group and the same already-advised marker check —
+        # the serialization that prevents the two paths double-posting.
+        required: false
+        type: string
+        default: 'mention'
+      event_action:
+        # The underlying event action the router observed (e.g. opened,
+        # ready_for_review). Forwarded for parity with the router payload and
+        # future per-persona gates; the surface's declared events are bound by
+        # the router (ADR-0009), so the qa-lead decision does not consider it.
+        required: false
+        type: string
+        default: ''
     secrets:
       CLAUDE_CODE_OAUTH_TOKEN:
         required: true
@@ -89,7 +108,7 @@
         # restoring the ADR-0005 position; reverses #1650, fixes #1734). Chosen per
         # persona from runtime.identity.credential, never a repo-wide default.
         required: false
       # GH_PAT_WORKFLOWS removed (#1650): the broadly-scoped workflows PAT is no
       # longer a posting fallback here — least privilege, and posting under it
       # attributed advisories to a human owner account.
 
@@ -107,9 +126,16 @@
     runs-on: ubuntu-latest
     timeout-minutes: 15
     permissions:
-      contents: read   # checkout THIS repo's tooling + prompts. The agent reads
-                       # the public source repo with github.token; the comment is
-                       # posted by the workflow with the persona's manifest PAT.
+      contents: read        # checkout THIS repo's tooling + prompts. The agent
+                            # reads the public source repo with github.token; the
+                            # comment is posted with the persona's manifest PAT.
+      pull-requests: read   # the event pre-gate (#1905) reads the PR's changed
+                            # files, labels, and reviews via github.token — with
+                            # only contents:read those reads 403 on a private
+                            # source repo and every routed pull_request advisory
+                            # fails closed (skips).
+      issues: read          # …and the shared issue-comment stream the gate scans
+                            # for the already-advised marker / automation budget.
     steps:
       - name: Checkout persona tooling + prompts
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
@@ -151,6 +177,51 @@
               ;;
           esac
 
+      - name: Event pre-gate — suppress the pull_request surface before the engine
+        # #1905. The router serves qa-lead's pull_request advisory here with
+        # surface=pull_request, but binds only the GENERIC brakes — it does NOT
+        # carry the persona-specific suppressors (no-test-surface, budget-
+        # exhausted, already-advised) the local qa-lead gate enforces. Run the
+        # persona's event pre-gate BEFORE the engine so a router-served PR
+        # dispatch is suppressed with the SAME fidelity as the local surface;
+        # otherwise the engine runs and posts on every trusted PR. For a mention
+        # dispatch (surface defaults to 'mention') the gate is a no-op ('run'),
+        # so existing mention behaviour is unchanged. Fails CLOSED: an unreadable
+        # signal yields a skip (no engine, no post) with a ::error naming it.
+        id: pregate
+        if: steps.resolve.outputs.skip == 'false'
+        env:
+          # github.token reads the (public) source repo — the same read scope the
+          # agent step uses. The pre-gate only READS PR signals; it never posts.
+          GH_TOKEN: ${{ github.token }}
+          PERSONA: ${{ inputs.persona }}
+          SURFACE: ${{ inputs.surface }}
+          SOURCE_REPO: ${{ inputs.source_repo }}
+          ITEM_NUMBER: ${{ inputs.item_number }}
+          EVENT_ACTION: ${{ inputs.event_action }}
+        run: |
+          set -euo pipefail
+          # shellcheck source=scripts/persona-pr-pregate.sh
+          source scripts/persona-pr-pregate.sh
+
+          decision="$(persona_event_pregate \
+            "$PERSONA" "$SURFACE" "$SOURCE_REPO" "$ITEM_NUMBER" "$EVENT_ACTION" || true)"
+          echo "persona pre-gate for '${PERSONA}' (surface=${SURFACE}) on ${SOURCE_REPO}#${ITEM_NUMBER}: ${decision}"
+          if [ "$decision" = "run" ]; then
+            echo "run=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "run=false" >> "$GITHUB_OUTPUT"
+            # Policy skips (no-test-surface, budget-exhausted, already-advised, ...)
+            # stay green. An unreadable signal, malformed dispatch, or empty
+            # decision must be a retryable failure, not a silently lost advisory.
+            case "$decision" in
+              skip:signal-unavailable | skip:unknown-surface | "")
+                echo "::error::persona pre-gate could not decide (${decision:-empty}) — failing so the dispatch is retryable"
+                exit 1
+                ;;
+            esac
+          fi
+
       - name: Resolve the persona's posting identity from its manifest
         # Source of truth for who a persona posts as: personas/<id>/persona.yml
         # runtime.identity (the #1316/#1317 pattern). The runner honors the
@@ -159,7 +230,7 @@
         # PATs only for the sanctioned posting identities; any other declared
         # credential fails loudly rather than silently borrowing a default.
         id: identity
-        if: steps.resolve.outputs.skip == 'false'
+        if: steps.resolve.outputs.skip == 'false' && steps.pregate.outputs.run == 'true'
         env:
           PERSONA: ${{ inputs.persona }}
         run: |
@@ -199,7 +270,7 @@
         # derived per-persona from the manifest (via the identity step), so every
         # persona is covered without an enumerated list (AC #6).
         id: authz_preflight
-        if: steps.resolve.outputs.skip == 'false'
+        if: steps.resolve.outputs.skip == 'false' && steps.pregate.outputs.run == 'true'
         env:
           # Same manifest-driven PAT selection as the post step — the token that
           # WILL post is the token we probe. The agent step never sees these.
@@ -235,7 +306,7 @@
           bash scripts/persona-authz-preflight.sh
 
       - name: Install Claude Code
-        if: steps.resolve.outputs.skip == 'false'
+        if: steps.resolve.outputs.skip == 'false' && steps.pregate.outputs.run == 'true'
         env:
           CLAUDE_CODE_VERSION: ${{ vars.CLAUDE_CODE_VERSION || 'latest' }}
         run: |
@@ -246,7 +317,7 @@
           npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}"
 
       - name: Produce the advisory (agent — read only, no post)
-        if: steps.resolve.outputs.skip == 'false'
+        if: steps.resolve.outputs.skip == 'false' && steps.pregate.outputs.run == 'true'
         id: advise
         env:
           CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
@@ -323,7 +394,7 @@
 
       - name: Enforce marker and post the comment (workflow — the only writer)
         id: post
-        if: steps.resolve.outputs.skip == 'false' && steps.advise.outputs.skip_post == 'false'
+        if: steps.resolve.outputs.skip == 'false' && steps.pregate.outputs.run == 'true' && steps.advise.outputs.skip_post == 'false'
         env:
           # The PAT lives ONLY here. The agent step above never had it. WHICH
           # PAT is used is driven by the persona manifest

.github/workflows/persona-runner.yml

@@ -4,8 +4,15 @@
 #
 # The mention router (in petry-projects/.github) fires `persona-mention` at this
 # repo with client_payload {persona, mode, source_repo, item_number, comment_url,
-# requested_by}. This stub hands that payload to the reusable, which runs the
-# addressed persona's advisory prompt.
+# requested_by, surface, event_action}. This stub hands that payload to the
+# reusable, which runs the addressed persona's advisory prompt.
+#
+# `surface` selects which suppressors the reusable applies before the engine:
+# it defaults to `mention` (existing mention dispatches unchanged), and a
+# router-served `pull_request` dispatch (surface=pull_request) triggers the
+# persona's event pre-gate — test-surface / budget / already-advised for qa-lead
+# (#1905). This reusable is referenced by a LOCAL `./` path (same commit), so the
+# new inputs are declared in the same change — no channel-skew concern.
 #
 # Thin caller: no logic here beyond mapping the payload to inputs. All behaviour
 # lives in the reusable. This repo is the `next` ring — the first, smallest
@@ -21,7 +28,14 @@ permissions: {}
 jobs:
   run:
     permissions:
+      # A reusable is granted no more than its calling job, so the read scopes the
+      # reusable's event pre-gate needs (#1905) must be granted HERE too. Without
+      # pull-requests/issues:read the shared qa-lead gate 403s on a private source
+      # repo's PR files/labels/reviews/comments and every routed pull_request
+      # advisory fails closed (skips).
       contents: read
+      pull-requests: read
+      issues: read
     uses: ./.github/workflows/persona-runner-reusable.yml
     with:
       persona: ${{ github.event.client_payload.persona }}
@@ -30,4 +44,7 @@ jobs:
       comment_url: ${{ github.event.client_payload.comment_url }}
       requested_by: ${{ github.event.client_payload.requested_by }}
       mode: ${{ github.event.client_payload.mode }}
+      # Absent surface -> 'mention' so existing dispatches are unchanged (#1905).
+      surface: ${{ github.event.client_payload.surface || 'mention' }}
+      event_action: ${{ github.event.client_payload.event_action || '' }}
     secrets: inherit

.github/workflows/qa-lead-pr-advisory.yml

@@ -21,7 +21,11 @@
 #
 # Idempotency (AC #5): the gate scans the PR for an existing marker AND the caller
 # concurrency group serializes opened + ready_for_review for one PR, so the two
-# events cannot both clear the gate and double-post.
+# events cannot both clear the gate and double-post. The advise job also forwards
+# surface=pull_request, so the reusable re-runs the same already-advised check and
+# both the local and router-served qa-lead advisories share ONE reusable
+# concurrency group (per persona+repo+PR) — serializing the two paths so they
+# cannot both observe "no marker" and post duplicate advisories.
 #
 # Soak (AC #9): watch this workflow's runs for the first two weeks. Noise metrics
 # to report — advisories posted vs. PRs seen, skip-reason histogram (the gate
@@ -72,89 +76,16 @@ jobs:
           PR: ${{ github.event.pull_request.number }}
         run: |
           set -euo pipefail
-          # shellcheck source=scripts/qa-lead-advisory-gate.sh
-          source scripts/qa-lead-advisory-gate.sh
+          # shellcheck source=scripts/qa-lead-pr-gate.sh
+          source scripts/qa-lead-pr-gate.sh
 
-          # Fail closed: qa-lead advises only when EVERY signal is available.
-          # Silence is the safe default, so any signal-gathering API failure must
-          # SKIP — never degrade to an empty value ([] labels, [] events, "no
-          # marker") that would bypass a suppressor or duplicate an advisory.
-          skip_closed() {
-            echo "gate: $1 — failing closed (skip) for ${REPO}#${PR}"
-            echo "decision=skip" >> "$GITHUB_OUTPUT"
-            exit 0
-          }
-
-          # AC #3 — changed files. Fetch the raw JSON array of filenames so the
-          # list can be validated BEFORE it is flattened to the newline-delimited
-          # text qa_lead_test_surface consumes. An empty list on success is fine
-          # (an empty PR does not fire); only an API failure skips.
-          if ! files_json="$(gh api --paginate --slurp \
-            "repos/${REPO}/pulls/${PR}/files" --jq '[.[][].filename]')"; then
-            skip_closed "changed-file list unavailable"
-          fi
-
-          # Completeness: the /pulls/{n}/files endpoint caps at 3000 files, so a
-          # larger PR yields a truncated list that could omit a test/source path
-          # and mislabel the PR. Compare received vs. the declared changed_files
-          # count and fail closed (skip) on an incomplete list — never advise off
-          # partial data.
-          if ! declared_files="$(gh api "repos/${REPO}/pulls/${PR}" \
-            --jq '.changed_files')"; then
-            skip_closed "changed-file count unavailable"
-          fi
-          received_files="$(jq 'length' <<< "$files_json")"
-          if ! qa_lead_file_list_complete "$received_files" "$declared_files"; then
-            skip_closed "changed-file list incomplete (${received_files}/${declared_files})"
-          fi
-
-          # Control characters: a filename containing an LF would split into
-          # phantom records once flattened (a docs-only PR could forge a SOURCE
-          # path and trigger the advisory). Reject before flattening.
-          if qa_lead_paths_have_control_char "$files_json"; then
-            skip_closed "changed-file list contains control characters"
-          fi
-
-          changed_paths="$(jq -r '.[]' <<< "$files_json")"
-
-          # AC #4/#6 — label names as a JSON array. A labels API failure must NOT
-          # degrade to [] — that would bypass the opt-out and human-gating
-          # suppressors and advise a PR marked hands-off / needs-human-review.
-          if ! labels_json="$(gh api "repos/${REPO}/pulls/${PR}" \
-            --jq '[.labels[]?.name]')"; then
-            skip_closed "labels unavailable"
-          fi
-
-          # AC #5 — has a qa-lead advisory already been posted? Scan issue comments
-          # (the PR shares the issues comment stream) for the recursion marker. A
-          # comment API failure is indistinguishable from "no marker", so failing
-          # open here would risk a duplicate advisory — fail closed instead.
-          marker="<!-- persona:qa-lead -->"
-          if ! comment_bodies="$(gh api --paginate \
-            "repos/${REPO}/issues/${PR}/comments" --jq '.[].body')"; then
-            skip_closed "existing-advisory scan unavailable"
-          fi
-          if grep -qF "$marker" <<< "$comment_bodies"; then
-            existing_advisory=1
-          else
-            existing_advisory=0
-          fi
-
-          # AC #6 — is the per-PR automation budget exhausted? (#860/#926 breaker)
-          # gather_pr_automation_events returns non-zero when any API call fails;
-          # a failure must NOT degrade to an empty event list (an exhausted PR
-          # would then pass the budget gate during a transient outage), so skip.
-          budget_exhausted=0
-          if events_json="$(gather_pr_automation_events "$PR" "$REPO")"; then
-            if pr_budget_exhausted "$events_json"; then
-              budget_exhausted=1
-            fi
-          else
-            skip_closed "automation-budget events unavailable"
-          fi
-
-          decision="$(qa_lead_gate_decision \
-            "$changed_paths" "$labels_json" "$existing_advisory" "$budget_exhausted" || true)"
+          # The signal gathering + fire/skip decision lives in ONE shared script
+          # (#1905 AC #2/#4): the router-served pull_request surface calls the
+          # SAME qa_lead_pr_gather_and_decide via scripts/persona-pr-pregate.sh,
+          # so the suppressors are identical, not a copy. It fails closed — any
+          # unreadable signal yields a skip with a ::error naming it — so this
+          # workflow keeps advising only when EVERY signal is available.
+          decision="$(qa_lead_pr_gather_and_decide "$REPO" "$PR" || true)"
           echo "gate decision for ${REPO}#${PR}: ${decision}"
           # Normalize to run|skip for the downstream `if`; keep the reason in logs.
           if [ "$decision" = "run" ]; then
@@ -168,6 +99,8 @@ jobs:
     if: needs.gate.outputs.decision == 'run'
     permissions:
       contents: read
+      pull-requests: read   # the reusable re-runs the shared pre-gate below with
+      issues: read          # surface=pull_request, so grant it the same reads.
     uses: ./.github/workflows/persona-runner-reusable.yml
     with:
       persona: qa-lead
@@ -176,4 +109,13 @@ jobs:
       comment_url: ''
       requested_by: ''
       mode: advisory
+      # Drive the reusable's pull_request pre-gate (#1905) so BOTH the local and
+      # the router-served qa-lead advisory run the SAME already-advised marker
+      # check AND share the reusable's per-(persona,repo,PR) concurrency group
+      # (comment_url is '' on both paths). That serialization is what closes the
+      # duplicate-advisory race: the local advise formerly left surface unset
+      # (-> 'mention'), so its reusable run skipped the marker recheck and could
+      # post a second advisory after the router already posted one.
+      surface: pull_request
+      event_action: ${{ github.event.action }}
     secrets: inherit

personas/qa-lead/README.md

@@ -123,9 +123,12 @@ set lives in [`persona.yml`](./persona.yml). This README does not restate either
 
 The whole framework is safe because **the agent cannot post**:
 
-- The **agent** runs with `github.token`. That token reads the (public) source
-  repo but is scoped to *this* repo, so it **cannot comment on the target**. The
-  agent only *prints* its advisory between sentinels.
+- The **agent** runs with `github.token`. That token reads **public** source
+  repos only (scoped to *this* repo, it cannot access private repos). For
+  private source repos, the token has no read access, so the pull_request
+  pre-gate skips the advisory (fail-closed). **Private source repositories are
+  out of scope** for the pull_request event surface today. The agent only *prints*
+  its advisory between sentinels; it never posts.
 - The **workflow** is the only writer. It extracts the advisory, mechanically
   guarantees the `<!-- persona:qa-lead -->` recursion marker, and posts with the
   persona's PAT (declared as `runtime.identity` in the manifest).

personas/qa-lead/interaction.yml

@@ -17,6 +17,9 @@
 #      router so it reaches beyond this repo (#1869). The local workflow is
 #      retired only after that router change lands on persona-mention/v1-next and
 #      the #1745 stop markers are verified on a pull_request-sourced dispatch.
+#      NOTE: Private source repositories are out of scope — the pull_request
+#      pre-gate runs with github.token (scoped to .github-private) which cannot
+#      access private repos, causing the gate to fail closed (skip the advisory).
 # Both paths post a single advisory comment as the persona's runtime.identity and
 # write nothing else. The workflow's read/write split mechanically stamps the
 # <!-- persona:qa-lead --> recursion marker; the prompt is not trusted to.