feat: implement issue #1165 — [qa-lead reach 3/3] Serve the pull_request surface from the persona mention router (architect decision (b), ADR-0007 consolidation)

[don-petry]

Problem

[qa-lead reach 3/3] Serve the pull_request surface from the persona mention router (architect decision (b), ADR-0007 consolidation)

From the issue: As the org, I want the persona mention router to also serve the pull_request event, so that a persona's PR-advisory surface is delivered by the one published ingress path instead of a second, repo-local runtime.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: tests/persona_mention.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Closes #1165

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Summary

Summary by CodeRabbit

• New Features
  • Personas can respond to pull requests when their pull-request surface and event are enabled; a mention is not required.
  • Eligible pull requests are routed when opened, reopened, or marked ready for review—not when new commits are pushed.
  • Routing is limited to qualifying human-authored pull requests and respects trust requirements, opt-out labels, stop markers, and write-mode approval gates.
• Bug Fixes
  • Invalid routing contracts and missing write-mode gates fail closed instead of allowing unsafe routing.

📝 Summary

📝 Summary

Walkthrough

The router now processes selected pull request events. It checks persona manifests and interaction contracts, applies routing and write-gate rules, and dispatches eligible personas.

Changes

Pull request persona routing

Layer / File(s)	Summary
Surface decisions and routing checks scripts/lib/persona-mention.sh, tests/persona_mention.bats	Helpers evaluate explicit surface settings, trust floors, event declarations, pre-filters, and routing verdicts. Tests cover those decisions, retry statuses, and verdict cases.
Pull request event entry point standards/workflows/persona-mention.yml, .github/workflows/persona-mention-reusable.yml	The caller subscribes to opened, ready_for_review, and reopened. The reusable workflow adds a filtered pull request job, setup, and PR-number concurrency fallback.
Persona selection and dispatch .github/workflows/persona-mention-reusable.yml	The job reads labels and persona listings, checks manifests and interaction contracts, and dispatches personas with eligible verdicts. Fetch and dispatch operations retry; routing errors and missing write gates fail the job.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Router as route-pull-request job
  participant GitHub as GitHub API
  participant Verdict as pm_pr_route_verdict
  participant Event as persona-mention event
  Router->>GitHub: Read labels and persona listing
  Router->>GitHub: Fetch persona manifest and interaction contract
  Router->>Verdict: Check stop markers, write gate, and trust floor
  Router->>Event: Dispatch eligible persona with PR details

Loading

---

---

---

---

Merge Risk: 🟡 Moderate · up to 0c654

Pull request events can now trigger persona advisories. A PR with many labels could still receive an advisory despite an opt-out or hold label. A network hiccup during dispatch could also cause a duplicate advisory. Fix both before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4b897

Eligibility is restricted to trusted-author, same-repository pull requests, with explicit hold and write controls. Dispatch retries can nevertheless repeat automation, and the final protections before downstream writes could not be verified.

Retained concerns

• Medium · reliability · inferred: The new PR path retries repository_dispatch POSTs without a visible delivery identity or acknowledgement ledger. If GitHub accepts a request but the client reports failure, another POST can submit the same work. Failure after earlier personas were dispatched also leaves partial fan-out that a rerun can repeat. This may repeat gated write automation unless the receiver deduplicates or makes its operations idempotent; that receiver behavior is unavailable. The base comment path used a single POST, so automatic POST retries are introduced here.

Security review details

Security Blast Radius

• inferred — Each eligible event can reach all personas whose PR policies permit it, rather than only a mentioned persona. The configured privileged sink is one persona repository, with subsequent scope determined by its consumers. Effective PAT repository grants, enabled production personas, downstream tools, credentials, and writable assets were not available, so wider organization or environment exposure cannot be established.

Trust Boundaries and Controls

• observed — Attacker-controllable PR text enters through environment variables and is passed as quoted shell arguments rather than embedded in executable script text. The router executes centrally sourced tooling, not PR code. Same-repository and author-association checks precede token-dependent routing, and local write eligibility requires a declared gate label present in the fetched label set.
• inferred — The receiver must resolve source_repo and item_number into current PR state and determine whether dispatched mode and authorization still apply before sensitive work. The workflow references a runner-side pre-gate, but its enforcement, checkout selection, and secret isolation were unavailable. A missing SHA in this payload alone does not establish an unsafe receiver.

Resilience and Maintainability Implications

• inferred — Human holds, opt-out, and write gates are evaluated from one label snapshot before fan-out. Changes after that read are not rechecked by this router, so revocation containment depends on downstream validation. Snapshot-based label checks already existed on the base comment path; their existence is not treated as a newly introduced bypass. The new automatic PR path makes the receiver's enforcement contract relevant to additional callers.

Hardening Proposals

• proposed — Establish an explicit receiver contract for delivery deduplication and current-state authorization before writes. If equivalent controls do not already exist, use a stable identity for retries and reruns, record per-persona acceptance, and revalidate PR provenance, holds, and write gates at the sensitive operation boundary.

---

---

---

---

---

---

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	[#1165] The PR adds the route-pull-request job, manifest-derived surface and event filtering, stop-marker enforcement, opt-out and write-gate checks, trust checks, and the caller-template trigger. T…	Add tests in tests/persona_mention.bats or the existing workflow test harness that exercise the PR route's contract fetch. Assert that an exhausted 5xx fetch fails the job and that a 404 for a manifested persona refuses dispatch. Keep the…

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: adding pull_request routing to the persona mention router. It includes extra issue and ADR context, but it remains specific and related to the changeset.
Description check	✅ Passed	The description directly explains the pull_request routing objective, affected workflows, tests, risk, rollback, and monitoring plan. It is clearly related to the changeset.
Out of Scope Changes check	✅ Passed	The workflow job, manifest-derived routing, safety gates, retry classification, caller-template trigger, and tests support [#1165]. The grep -- changes harden label matching, and the concurrency cha…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: 1…

---

---

---

Full details: Linked Issues check

Explanation

[#1165] The PR adds the route-pull-request job, manifest-derived surface and event filtering, stop-marker enforcement, opt-out and write-gate checks, trust checks, and the caller-template trigger. The existing comment route remains unchanged apart from safe grep -- handling and the concurrency-key extension. The required fetch-failure tests are not present. tests/persona_mention.bats tests pm_should_retry_status and pure verdict failures, but it does not test an exhausted contract fetch returning 5xx or a manifested persona contract returning 404 and refusing dispatch.

Resolution

Add tests in tests/persona_mention.bats or the existing workflow test harness that exercise the PR route's contract fetch. Assert that an exhausted 5xx fetch fails the job and that a 404 for a manifested persona refuses dispatch. Keep the existing comment-path tests passing.

---

---

---

---

---

---

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

---

---

---

---

---

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request implements event-surface routing for the pull_request surface, allowing PR-advisory personas to be dispatched based on declared manifest surfaces. It introduces several helper functions in persona-mention.sh to evaluate surface decisions, trust floors, and write gates, updates the workflow trigger, and adds extensive BATS tests. The review feedback highlights opportunities to improve robustness and efficiency in the routing script, such as using native Bash parameter splitting instead of spawning awk, declaring local variables to prevent scope leakage, and propagating failures to ensure the system fails closed. Additionally, the feedback recommends improving the BATS tests by explicitly asserting on command exit statuses immediately after execution and verifying exact non-zero exit codes rather than using generic checks.

[don-petry]

Acknowledged — this is a Codex usage-limit notice, not a review finding. No code action is required and there is no follow-up to track.

[don-petry]

Acknowledged — this is a Qodo trial-ended notice (qodo:billing-blocked), not a review finding. No code action is required and there is no follow-up to track.

[don-petry]

Acknowledged — SonarCloud reports the Quality Gate passed. No action required.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed. Requested items addressed:

• scripts/lib/persona-mention.sh:484 — applied
• scripts/lib/persona-mention.sh:494 — applied
• tests/persona_mention.bats:554 — applied
• tests/persona_mention.bats:517 — applied
• tests/persona_mention.bats:523 — applied
• tests/persona_mention.bats:532 — applied
• tests/persona_mention.bats:539 — applied
• tests/persona_mention.bats:547 — applied
• scripts/lib/persona-mention.sh:469 — applied
• tests/persona_mention.bats:562 — applied
• tests/persona_mention.bats:570 — applied
• tests/persona_mention.bats:575 — applied
• tests/persona_mention.bats:590 — applied
• tests/persona_mention.bats:595 — applied
• tests/persona_mention.bats:600 — applied
• tests/persona_mention.bats:623 — applied
• tests/persona_mention.bats:664 — applied

[don-petry]

Resolved — the `needs-human-review` hold was lifted; dev-lead has picked this item up. Click to expand the prior hold notice.

dev-lead is withholding action on this item.

It is labeled needs-human-review (flagged for human review — this label is applied by automation as well as by people, so an item can become held without anyone noticing), so dev-lead will not pick it up while that label is present. This notice is posted once so the withhold is visible rather than looking like a stalled run.

To re-enable automated pickup: remove the needs-human-review label.

[don-petry]

Maintainer review: changes required before merge

The stop-marker binding is right, and it is the architect's hard requirement: pm_pr_route_verdict checks stop markers first, fails closed on an unreadable contract, and is tested per branch. The routing still has three behaviour regressions compared with the surface it replaces (.github-private/.github/workflows/qa-lead-pr-advisory.yml). As written, once this reaches a channel and the stub is swept, qa-lead would post an advisory on every push to every trusted PR in every repo. This review answers the needs-human-review escalation (pr-review tier-3, HIGH); the hold is being cleared so these changes can be applied.

Required (in this order; #1 matters most)

1. Honour the surface's declared events, not only enabled. pm_surface_decision reads enabled/mode and ignores the row's events: list. The stub now subscribes pull_request: [opened, ready_for_review, synchronize, reopened], while qa-lead declares events: [opened, ready_for_review] and its notes say synchronize is deliberately excluded. Nothing in the router compares github.event.action against the declared events, so every synchronize dispatches a fresh advisory, and nothing downstream deduplicates (the runner has no already-advised check; that lived in the local gate).

• Pass github.event.action into the route step via env (PR_ACTION), and make the per-persona decision skip unless PR_ACTION is in the row's events. A row with no events list does not fire: derive from declared surfaces, the same rule as the no-default_mode-fallback you already applied.
• Emit a named skip (skip undeclared-event <action>) so it's visible in the log.
• bats: synchronize against qa-lead's row → skip; opened → dispatch; a row without events → skip.
• Narrow the stub's types: to [opened, ready_for_review, reopened] unless a persona declares synchronize. None does, and each subscribed type is a runner start in every repo on every push.

2. Check github.actor as well as the PR author in both recursion pre-filters (the workflow if: and pm_pr_should_route). This is the codeant/tier-3 finding. pull_request.user.login is the PR's author and never changes; the actor of a synchronize/reopened event is whoever pushed or reopened. A bot push to a human's PR passes the author check. Add github.actor to the if: bot exclusions, and pass it as PR_ACTOR into pm_pr_should_route (checked with pm_is_bot_actor). bats: human author with bot actor → blocked.

3. Forward the event action in the dispatch payload (client_payload[event_action]=$PR_ACTION) next to surface=pull_request. The persona-specific suppressors the local gate enforced (test-surface heuristic, automation-budget, already-advised idempotency) belong on the runner side in .github-private, which is where that policy and its scripts live. That work is tracked in petry-projects/.github-private#1905, and the runner needs surface/event_action to apply it. Do not try to port qa-lead-test-surface.sh or the budget gate into this router: they are persona and .github-private policy, and the router stays generic.

Budget guard

Commit 1 (events filter + narrowed stub types + tests) is the one that matters. If you run short, land that, then file a follow-up issue in petry-projects/.github for items 2–3 and stop. Don't attempt the runner-side gate here; it's in a different repo.

Not required

• Pagination on the contents/personas listing: that endpoint returns up to 1,000 entries, and there are fewer than 20 personas.
• gemini's per-test exit-status suggestions were already applied in 80b23dac.

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: fix-reviews)

PR: #1167
No changes were committed, but the PR still can't be marked done: a reviewer requested changes. The retry cron will re-attempt automatically. Next attempt after: 2026-10-09T17:34:13Z

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: fix-reviews)

PR: #1167
No changes were committed, but the PR still can't be marked done: a reviewer requested changes. The retry cron will re-attempt automatically. Next attempt after: 2026-10-09T23:17:03Z

[don-petry]

QA Lead — test-risk advisory

Risk tier: MEDIUM. This turns on an automatic persona trigger for every trusted PR across the org. The checks that stop or hold a persona (stop markers, opt-out label, write gate, trust floor, bot/marker guards) are well unit-tested one rule at a time. The workflow loop that runs them is not tested at all.

What I'd shore up (highest leverage first):

• Test the loop, not just the rules. Move the route-pull-request run block into a pm_route_pull_request lib function. Test it in bats with fake gh/curl on PATH and a sleep that can be swapped out. Cover:

  • two personas fanned out
  • a 404 manifest being skipped
  • a 5xx that succeeds on retry, and one that keeps failing and aborts
  • the dispatch POST firing exactly once

  Also make a deliberate, tested decision on one case. Today, one PR-enabled persona with no interaction contract hits exit 1 partway through the loop. Every PR in the org then gets a red check, and any persona later in the list never fires.

• Drafts get advised twice. A persona that declares both opened and ready_for_review fires on a draft and again when it is marked ready, and nothing removes the duplicate. Decide whether to skip opened on drafts, then pin that with a test. Separately, the per-PR concurrency group replaces a pending run rather than queueing it, so events in between can be dropped. The code comment should say so.

• Keep the two bot lists in sync. The job if: hardcodes two bot logins, and that check is what keeps secrets away from bot-triggered runs. Add a structural test asserting it matches pm_bot_logins.

• Negative cases: add tests for an unparseable manifest (return 2) and a scalar events: opened in pm_surface_declares_event, and for reopened (the stub subscribes to it). Also confirm on purpose that the mention path lets a surface floor override the persona-wide floor while the PR path intersects them.

Escalate? no. Before moving the published version forward, run an end-to-end smoke test on a sandbox repo with persona_ref set.

Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture
(bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label.

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: fix-reviews)

PR: #1167
No changes were committed, but the PR still can't be marked done: a reviewer requested changes. The retry cron will re-attempt automatically. Next attempt after: 2026-10-10T06:26:35Z

[don-petry]

Maintainer review — two line-anchored items. The September items (declared-events filter, github.actor check, event_action forwarding, stub types) are all in and verified; I am resolving those threads.

[don-petry]

No description provided.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed. Requested items addressed:

• .github/workflows/persona-mention-reusable.yml:582 — applied
• .github/workflows/persona-mention-reusable.yml:620 — applied

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

No description provided.

[don-petry]

No description provided.

[don-petry]

Re-reviewed the edited CodeRabbit summary. Its only finding is the Security Architecture Medium · reliability concern that the PR path retries repository_dispatch POSTs with no idempotency key, so an ambiguous transport failure could double-dispatch a write persona. This is already fixed. Commit 7df3d07 removed gh_retry from the dispatch POST in .github/workflows/persona-mention-reusable.yml, and the call site now carries a comment explaining why it is deliberately not retried. The stale per-persona fan-out is bounded because the loop now continues past a per-persona failure instead of aborting. The Hardening Proposal (a receiver-side dedup / current-state revalidation contract) lives in .github-private, not this repo, so it needs no change here. The rest of the comment is the walkthrough and a paused-review notice, with no other findings.

[don-petry]

CodeAnt's finding is that the workflow omits pull_request/review_requested. This is intentional, so I'm not subscribing it. The router has no routing path for reviewer assignment. standards/workflows/persona-mention.yml documents that a trigger firing into a router that ignores it is a dead trigger that only burns runner minutes. The finding is tracked as finding 10 in #755, and that trigger lands together with its routing. This PR's scope is the opened / ready_for_review / reopened actions that personas can declare today. No code change.

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

Addressed 0 threads: there were no open review threads, and I made no code changes and no commits.
- **Blocking checks:** none that I can fix. No review is in `CHANGES_REQUESTED`. The only non-passing checks are `dev-lead / ci-relay`, `dispatch` and `resume`, which show `cancelled`. They look like superseded dev-lead workflow runs rather than a code failure, though I didn't open the run logs.
- **Issue comments:** two bot comments had been edited after my last dispositions, so I posted a fresh disposition on each.
  - **CodeRabbit summary** (`IC_kwDORyesfc8AAAABWPPyoQ`): `disposition=fixed`, citing `7df3d07b0ac63bfce93fc6bd42de08662f9e0901`. Its one finding was that the pull-request path retries `repository_dispatch` POSTs, which could double-dispatch. That commit already removed `gh_retry` from the dispatch POST in `.github/workflows/persona-mention-reusable.yml`. The receiver-side dedup suggestion belongs to `.github-private`, so I left it.
  - **CodeAnt risk comment** (`IC_kwDORyesfc8AAAABZ2z8SQ`): `disposition=invalid`. It wants `pull_request/review_requested` subscribed, but the router has no path for it, so that trigger would never route anything. It's deliberately omitted per `standards/workflows/persona-mention.yml` and tracked as finding 10 in petry-projects/.github#755.
- **Not rerun:** I did not rerun the test suite, because nothing changed this pass.
Files changed: none.

[don-petry]

pr-review findings for 42c02bd7, posted by the maintainer

pr-review completed a full three-tier review of this head (run 38043713369) and reached approve, risk MEDIUM. It could not post the result itself (petry-projects/.github-private#2169), so the findings are recorded here. This comment is not an approval and carries no pr-review marker.

Verdict (tier 3, security audit): approve. The new route-pull-request job runs no PR-head code: it sources its tooling from the reusable's own pinned commit, passes every event field through env, and binds the same bot, marker, trust-floor, stop-marker, opt-out and write-gate brakes as the comment path. Fork heads are excluded before any token-dependent step.

Tier 1 and tier 2 escalated on an automated safety flag for a pull_request_target trigger. Tier 3 confirmed that as a false positive: the only occurrence is the comment at persona-mention-reusable.yml:406 stating the design is not pull_request_target; the stub subscribes to plain pull_request.

Findings — none blocking:

Severity	Where	Finding
minor	standards/workflows/persona-mention.yml:68	The stub now runs on pull_request, so its definition is read from the PR merge commit, and the org token is present on more fleet runs. No new privilege: changing the stub already requires write access to the repo.
minor	scripts/lib/persona-mention.sh:492	The PR path intersects the surface trust floor with the persona-wide floor (can only tighten); the mention path's doc says the surface floor overrides it. The PR path is the safe direction; the two should be reconciled.
minor	scripts/lib/persona-mention.sh:639	The write gate is checked only when mode is exactly write; any other mode string is dispatched as-is. Relies on validate-personas.py enforcing the enum, as the comment path does.
minor	persona-mention-reusable.yml:532	PERSONA_REF is interpolated unencoded into the contents API query. Trusted caller input, default main.
minor	persona-mention-reusable.yml:515	Labels are read once at event time, so an opt-out label applied just after opened can be missed. Advisory mode only; the write gate fails closed.
info	persona-mention-reusable.yml:476	raw_fetch re-sources the library on every call (redundant). The concurrency comment says rapid events queue, but GitHub keeps one pending run per group.
info	tests/persona_mention.bats	The job-level if: duplicates pm_pr_should_route and is not covered by bats, so the two can drift. The in-script call re-checks the same conditions.

Checks on this head: all four required checks pass (SonarCloud, CodeQL, AgentShield, dependency-audit); 24 pass and 6 are skipped by design. The three dev-lead / * entries shown as failing are one cancelled orchestration run in which no step executed.

Maintainer verification, beyond the suite: 103 bats tests pass; the workflow's routing block was run against real PRs and the live persona manifests with only the dispatch call intercepted — opened routes qa-lead alone, synchronize routes nobody, a PR carrying needs-human-review is skipped at the stop marker, dev-lead and pr-review are skipped as served by their own runtime, and a failed fetch for an earlier persona no longer prevents later ones being evaluated.