Skip to content

Add the persona @-mention router#754

Merged
don-petry merged 9 commits into
mainfrom
feat/persona-mention-router
Jul 17, 2026
Merged

Add the persona @-mention router#754
don-petry merged 9 commits into
mainfrom
feat/persona-mention-router

Conversation

@don-petry

@don-petry don-petry commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Serves the addressing contract #752 defined. One router for every persona — personas do not each ship a mention workflow (§4.1). A stub per agent is exactly the drift the manifest exists to prevent, and it's how the fleet ended up with two divergent registries last time.

There is no persona index, by design

This was the open question when #752 landed, and it dissolved. Because address.handle's slug MUST equal id, and id MUST equal the persona's directory name — both enforced by validate-personas.py@petry-projects/qa-lead resolves to personas/qa-lead/persona.yml by convention. The manifest is the index-of-record (§1.1); nothing derives, caches, or duplicates it.

It also avoids inverting the established derivation direction: canary-rings.json (.github) → release/registry.yml (.github-private). A persona index would have had to flow the other way, putting generated content in the public standards repo.

And a 404 is a meaningful answer — "not a persona". That's also how a real, non-persona team like @petry-projects/org-leads falls through harmlessly instead of erroring.

The manifest decides; the router restates nothing

Whether the mention surface is enabled, in what mode, behind which trust floor, under which opt_out_label — all read from the persona. A persona may tighten the job-level trust default but never loosen it, and an undeclared floor denies, so a persona that forgot to declare trust isn't more permissive than one that did.

Recursion — why the core is a tested library, not inline shell

Comments posted via a PAT re-trigger workflows, unlike GITHUB_TOKEN. .github-private#860 burned 1,481 identical acks in 4.5h from a single self-loop. With N mutually addressable personas the cycles are combinatorial, not self-loops: qa-lead answering a thread that mentions dev-lead is enough.

So the guard is two independent axes — bot actor and agent marker — enforced in both the job-level if (so secrets never reach a bad run) and pm_should_route (so it's testable). The marker match is a prefix: #860's first fix matched one exact ack string and still self-looped through a different agent-authored comment.

Contents

File What
scripts/lib/persona-mention.sh The pure, network-free decision core — fetching is the workflow's job, so the suite pins behaviour with zero API access
tests/persona_mention.bats 31 tests, recursion guards most heavily
.github/workflows/persona-mention-tests.yml The runner — a bats suite with no CI runner is unenforced (the #613/#624 gap canary-rollout-tests.yml exists to close), so it ships with the suite
.github/workflows/persona-mention-reusable.yml The router. Sources the lib at github.job_workflow_sha (the #465/#528 tooling_ref lesson); every event field arrives via env, never inline ${{ }} in a run: body
standards/workflows/persona-mention.yml The single caller stub, pinned persona-mention/v1-stable (major-scoped per #657, matching live stubs)
standards/canary-rings.json Registers the reusable so a release can be cut

Deliberately NOT here

  • ring-pins.sh RING_REUSABLES is untouched. Adding persona-mention would make compliance-audit demand the stub in repos that don't have it, and make the deploy sweep report drift — manufacturing failures for a reusable with no cut release. That belongs to rollout.
  • Nothing routes on merge. No release cut, no repo enrolled, and no persona runtime exists yet. The router dispatches persona-mention to .github-private, where nothing listens — a safe no-op.

Verification (CI's exact invocations, run locally)

  • bats tests/persona_mention.bats31/31
  • shellcheck --severity=warning -x scripts/**/*.sh — clean
  • actionlint -ignore 'SC2129' -ignore '"job_workflow_sha" is not defined' .github/workflows/*.yml — clean. That ignore is pre-existing and correct: job_workflow_sha is a real context property actionlint 1.7.7's schema omits, and the merged auto-rebase-reusable.yml trips the identical false positive.
  • markdownlint-cli2 — clean
  • canary-rings.json — spliced as text, not re-serialized: the file's encoding is genuinely mixed (em-dashes escaped, arrows half-literal) and its agent order is insertion order, not alphabetical, so a JSON round-trip rewrote ~140 untouched lines. Result is 63 insertions / 0 deletions, structurally identical to pr-review-mention's entry but for host/reusable/run_workflow.

One bug the tests caught

pm_manifest_query piped python into jq, so the pipeline reported jq's status — a manifest parse failure returned empty output with exit 0, indistinguishable from "the manifest says no". A malformed or truncated manifest would have silently mis-routed. Now captured and checked.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a reusable GitHub Actions workflow to route @petry-projects/<role> persona mentions across issues, PRs, reviews, and discussions.
    • Added an org-level caller workflow stub that forwards to the reusable router, plus a new canary agent configuration to roll it out.
  • Documentation
    • Updated persona standards and CI template listings to reference the centralized mention router.
  • Tests
    • Added Bats end-to-end tests for mention parsing, recursion safeguards, trust and opt-out behavior, and manifest-driven routing.

Serves the addressing contract that .github#752 defined. ONE router for every
persona — personas do NOT each ship a mention workflow (§4.1); a stub per agent
is exactly the drift the manifest exists to prevent, and it is how the fleet got
two divergent registries the last time.

There is no persona index, by design

Because address.handle's slug MUST equal `id`, and `id` MUST equal the persona's
directory name (both enforced by validate-personas.py), '@petry-projects/qa-lead'
resolves to personas/qa-lead/persona.yml by convention. The manifest is the
index-of-record (§1.1) — nothing derives, caches or duplicates it. This also
avoids inverting the established derivation direction: canary-rings.json
(.github) -> release/registry.yml (.github-private), whereas a persona index
would have had to flow .github-private -> .github.

A 404 is a meaningful answer ("not a persona"), which is also how a real,
non-persona team like @petry-projects/org-leads falls through harmlessly.

What decides what

The manifest, not this router: whether the mention surface is enabled, in what
mode, behind which trust floor, under which opt_out_label. The router restates
none of it. A persona may TIGHTEN the job-level trust default but never loosen
it — an undeclared floor denies, so a persona that forgot to declare trust is
not more permissive than one that did.

Recursion (the reason the core is a tested library, not inline shell)

Comments posted via a PAT re-trigger workflows, unlike GITHUB_TOKEN.
.github-private#860 burned 1,481 identical acks in 4.5h from a SINGLE self-loop.
With N mutually addressable personas the cycles are combinatorial, not
self-loops: qa-lead answering a thread that mentions dev-lead is enough. So the
guard is two independent axes — bot actor AND agent marker — enforced both in
the job-level `if` (so secrets never reach a bad run) and in pm_should_route
(so it is testable). The marker match is a PREFIX: #860's first fix matched one
exact ack string and still self-looped through a different agent comment.

Contents
- scripts/lib/persona-mention.sh — the pure, network-free decision core.
  Fetching is the workflow's job, so the suite pins routing behaviour with no
  API access at all.
- tests/persona_mention.bats — 31 tests, recursion guards most heavily.
- .github/workflows/persona-mention-tests.yml — the runner. A bats suite with no
  CI runner is unenforced (the #613/#624 gap canary-rollout-tests.yml exists to
  close); added WITH the suite rather than after it.
- .github/workflows/persona-mention-reusable.yml — the router. Sources the lib
  at `github.job_workflow_sha` so the logic always matches the pinned workflow
  version (the #465/#528 tooling_ref lesson); every event field arrives via env,
  never inline `${{ }}` in a run: body.
- standards/workflows/persona-mention.yml — the single caller stub, pinned to
  persona-mention/v1-stable (major-scoped per #657, matching live stubs).
- standards/canary-rings.json — registers the reusable so a release can be cut.

Deliberately NOT here
- ring-pins.sh RING_REUSABLES is untouched: adding persona-mention would make
  compliance-audit demand the stub in repos that do not have it and make the
  deploy sweep report drift — manufacturing failures for a reusable with no cut
  release. That belongs to rollout.
- No release cut, no repo enrolled, and no persona runtime exists yet, so
  NOTHING routes on merge. The router dispatches `persona-mention` to
  .github-private, where nothing listens yet — a safe no-op.

Verification (CI's exact invocations, run locally)
- bats tests/persona_mention.bats: 31/31.
- shellcheck --severity=warning -x scripts/**/*.sh: clean.
- actionlint -ignore SC2129 -ignore '"job_workflow_sha" is not defined'
  .github/workflows/*.yml: clean. (That ignore is pre-existing and correct —
  job_workflow_sha is a real context property actionlint 1.7.7's schema omits;
  the merged auto-rebase-reusable.yml trips the same false positive.)
- markdownlint-cli2: clean.
- canary-rings.json: spliced as TEXT, not re-serialized — the file's encoding is
  genuinely mixed (em-dashes escaped, arrows half-literal) and its agent order is
  insertion order, not alphabetical, so a json round-trip rewrote ~140 untouched
  lines. Result is 63 insertions / 0 deletions, and the entry is structurally
  identical to pr-review-mention's but for host/reusable/run_workflow.

One bug the tests caught: pm_manifest_query piped python into jq, so the
pipeline reported JQ's status and a parse failure returned empty output with
exit 0 — indistinguishable from "the manifest says no". Now captured and
checked.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@don-petry
don-petry requested a review from a team as a code owner July 17, 2026 03:07
Copilot AI review requested due to automatic review settings July 17, 2026 03:07
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@don-petry, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 08800004-8e32-43db-92ca-fad5e91246bb

📥 Commits

Reviewing files that changed from the base of the PR and between 02ac387 and b022ed0.

📒 Files selected for processing (2)
  • .github/workflows/persona-mention-reusable.yml
  • standards/workflows/persona-mention.yml
📝 Walkthrough

Walkthrough

Adds guarded persona-mention routing, reusable GitHub Actions execution, manifest-based trust and label handling, private repository dispatches, caller wiring, rollout registration, documentation, and automated Bats/ShellCheck validation.

Changes

Persona mention routing

Layer / File(s) Summary
Routing decision core and validation
scripts/lib/persona-mention.sh, scripts/persona-mention-requirements.txt, tests/persona_mention.bats, .github/workflows/persona-mention-tests.yml
Adds guarded slug extraction, manifest parsing, mention decisions, trust floors, gate labels, pinned PyYAML dependencies, tests, and CI checks.
Reusable routing workflow
.github/workflows/persona-mention-reusable.yml
Defines the reusable interface, filters unroutable comments, evaluates manifests and label/trust rules, and dispatches enabled persona events.
Caller workflow and standards registration
standards/workflows/persona-mention.yml, standards/canary-rings.json, standards/ci-standards.md, standards/persona-standards.md
Adds the pinned event caller, canary configuration, template listing, and routing conventions documentation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Comment
  participant CallerWorkflow
  participant ReusableRouter
  participant PersonaManifest
  participant PrivateRepository
  Comment->>CallerWorkflow: created issue or review comment
  CallerWorkflow->>ReusableRouter: invoke reusable workflow
  ReusableRouter->>PersonaManifest: fetch persona manifest
  PersonaManifest-->>ReusableRouter: routing, labels, and trust settings
  ReusableRouter->>PrivateRepository: dispatch persona-mention event
Loading

Possibly related issues

Possibly related PRs

  • petry-projects/.github#670: Introduces the persona manifest fields and standards consumed by this router for surface modes, labels, and trust settings.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding the persona @-mention router.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/persona-mention-router

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

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a reusable Bash library and workflow for routing persona @-mention decisions, complete with recursion guards, manifest parsing, and BATS unit tests. The feedback recommends enhancing the robustness of the jq queries within the Bash library by incorporating the optional/try operator ? to prevent null-indexing errors, guarding against empty inputs, and expanding the test suite to cover these null-safety paths.

Comment thread scripts/lib/persona-mention.sh
Comment thread scripts/lib/persona-mention.sh
Comment thread tests/persona_mention.bats
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #754
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-07-17T03:41:04Z

@don-petry

Copy link
Copy Markdown
Contributor Author

Note

@don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically.
Next attempt after: 2026-07-17T03:41:04Z

@don-petry
don-petry enabled auto-merge (squash) July 17, 2026 03:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the org-standard “persona mention router” that parses @petry-projects/<role> mentions, consults each persona’s manifest in .github-private, and dispatches to the persona runtime without per-persona workflow stubs (centralized routing per the persona addressing contract).

Changes:

  • Introduces a pure, network-free routing decision library (scripts/lib/persona-mention.sh) plus a bats test suite and CI runner to quality-gate it.
  • Adds a reusable workflow router (persona-mention-reusable.yml) and the single caller stub template (standards/workflows/persona-mention.yml).
  • Registers the new reusable in standards/canary-rings.json and documents/adverts the stub in the standards docs.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
scripts/lib/persona-mention.sh Adds the routing decision core (slug extraction, recursion guards, trust/manifest parsing).
tests/persona_mention.bats Adds unit coverage (31 cases) emphasizing recursion and manifest/trust behaviors.
.github/workflows/persona-mention-tests.yml Adds CI quality gate to run shellcheck + bats for the routing core.
.github/workflows/persona-mention-reusable.yml Adds the reusable router that fetches manifests and dispatches persona runs.
standards/workflows/persona-mention.yml Adds the thin caller stub template for consumer repos.
standards/persona-standards.md Documents the router, its decision core, and the “no persona index” convention.
standards/ci-standards.md Registers the new caller stub in the standards table of workflow templates.
standards/canary-rings.json Registers the reusable in the canary channel/ring registry for future rollout.

Comment thread .github/workflows/persona-mention-reusable.yml
Comment thread .github/workflows/persona-mention-reusable.yml
Comment thread .github/workflows/persona-mention-reusable.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/persona-mention-reusable.yml:
- Around line 126-131: Update the label lookup block around LABELS so discussion
comments retain an empty label set, while issue and pull request lookups
propagate gh api failures instead of converting them to empty output. Remove the
fallback that masks errors and preserve the existing ITEM_NUMBER guard.
- Around line 138-143: Update the manifest fetch in the persona-processing
workflow to authenticate private `.github-private` requests using the existing
GH_TOKEN/PAT, either through the GitHub Contents API or an Authorization header
on the curl request. Preserve the current failure handling and manifest parsing
flow while ensuring valid private manifests are retrieved instead of treated as
missing.

In `@scripts/lib/persona-mention.sh`:
- Around line 169-185: Write-mode mention decisions currently omit the required
gate label, allowing unarmed items to dispatch. Update pm_mention_decision in
scripts/lib/persona-mention.sh to emit the mention surface’s gate_label
alongside enabled, mode, and opt_out_label; update tests/persona_mention.bats at
lines 188-194 to verify gate preservation and allowed/denied label states;
update .github/workflows/persona-mention-reusable.yml at lines 155-192 to block
write-mode dispatch unless the required gate label is present.
- Around line 81-105: The direct slug lookup in pm_manifest_url breaks the
existing persona alias and rename contract. Update the persona mention
resolution flow, including pm_extract_slugs and pm_manifest_url, to resolve each
addressed slug through the established alias mapping and construct the manifest
URL from the canonical persona ID; update standards/persona-standards.md lines
253-258 to document this alias-resolution mechanism, or explicitly remove
aliases from the addressing contract.

In `@standards/workflows/persona-mention.yml`:
- Around line 38-48: Subscribe to pull_request review_requested events in the
workflow trigger configuration in standards/workflows/persona-mention.yml. In
.github/workflows/persona-mention-reusable.yml, update the event normalization
logic to handle reviewer-assignment payloads by normalizing requested_team and
pull-request fields without assuming comment is present; preserve existing
comment-event handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5d4c8e2a-377a-4330-9d97-be017de4be50

📥 Commits

Reviewing files that changed from the base of the PR and between 0a52011 and 518ff2b.

📒 Files selected for processing (8)
  • .github/workflows/persona-mention-reusable.yml
  • .github/workflows/persona-mention-tests.yml
  • scripts/lib/persona-mention.sh
  • standards/canary-rings.json
  • standards/ci-standards.md
  • standards/persona-standards.md
  • standards/workflows/persona-mention.yml
  • tests/persona_mention.bats

Comment thread .github/workflows/persona-mention-reusable.yml Outdated
Comment thread .github/workflows/persona-mention-reusable.yml Outdated
Comment thread scripts/lib/persona-mention.sh
Comment thread scripts/lib/persona-mention.sh
Comment thread standards/workflows/persona-mention.yml
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 17, 2026
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 17, 2026
@don-petry
don-petry disabled auto-merge July 17, 2026 03:26
@don-petry

Copy link
Copy Markdown
Contributor Author

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

No changes were needed for this PR.

don-petry and others added 2 commits July 16, 2026 22:57
Addresses the review findings on this PR (#755) and the bot review.

1. Fail-open trio — errors were read as negative answers

The systemic flaw #755 names as its headline finding. Three instances here:

- Manifest fetch: `curl -f` collapses 404 and 5xx into one exit code, so a
  raw.githubusercontent hiccup silently disabled EVERY persona fleet-wide, with
  no error anywhere and indistinguishable from nobody being addressed. Now
  switched on the status code: 200 routes, 404 is a real answer ("not a persona"
  — a plain team like @petry-projects/org-leads, or a typo), anything else is a
  hard failure.
- Labels: `|| echo ""` turned an API error into "no labels", which would bypass
  an opt-out AND arm an ungated write. Now fails closed. Discussions genuinely
  have no labels surface, so an empty set is the TRUE answer there — that
  difference is load-bearing (see 2).
- pm_manifest_query already fixed in 518ff2b (pipeline reported jq's status).

2. gate_label is now ENFORCED (security)

The schema makes gate_label required for write surfaces, but only enforces that
it is DECLARED — declaring a lock is not locking the door. The router read
`mode`, passed it in the payload, and dispatched. '@petry-projects/dev-lead do X'
would have been an ungated write the moment the first write persona onboarded.
Now: write-mode requires the gate label to be APPLIED; a write persona with no
gate_label is a hard error; and write-mode on a surface where labels are
unavailable (discussions) is refused, because the gate cannot be verified.

gate_label is read by its own function, not a 4th space-separated field on
pm_mention_decision: an empty optional field silently shifts the next one into
its place ("true write  qa-lead" parses the gate AS the opt_out_label). A
security gate is the last place to accept that. Test 42 pins it.

3. contents: read + PyYAML (Copilot)

The job checks out the tooling repo but never granted contents: read. PyYAML is
NOT documented as preinstalled on the runner images, and this reusable runs on
the CALLER's runner, so it cannot be assumed — our own tests install it
explicitly, which proved the gap. Added a cheap no-op-when-present step. Also
persist-credentials: false on the tooling checkout.

4. Mention precision (#755 finding 9)

The router fired where GitHub itself renders NO mention: fenced code, inline
code. Pasting a usage example or documenting the handle summoned the persona.
Blockquotes are a DELIBERATE divergence — GitHub does notify on a quoted
mention, but its one-click "Quote reply" copies a whole prior comment prefixed
'>', and re-running an agent over quoted text is not what the quoter meant.
Neither recursion axis catches it (a human quoting a human is not a bot and
carries no marker). To address a persona, mention it outside the quote.

Verification (CI's exact invocations)
- bats tests/persona_mention.bats: 42/42 (31 + 11 new covering fences, tilde
  fences, inline code, blockquotes, the two must-still-fire cases, and the
  gate_label read-shift trap).
- shellcheck --severity=warning -x scripts/**/*.sh: clean.
- actionlint (CI's ignores): clean.

NOT fixed here, deliberately
- aliases (#755 finding 1): the merged schema promises address.aliases[] but
  index-free routing cannot honour it. Dropping it needs an ORDERED cross-repo
  pair — .github-private must remove `aliases: []` from qa-lead's manifest
  first, or dropping it from the schema (additionalProperties: false) breaks
  validate-personas on main fleet-wide. Tracked in #755; it is a contract change
  and deserves its own PR.
- review_requested routing (#755 finding 10) and the 👀 ack (finding 11) are
  scoped out to keep this PR about correctness and security.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stub subscribed to pull_request/review_requested, but the router has no
path for it — the job's `if` never admits the event, so every reviewer
assignment fleet-wide would start a run that immediately skips.

§4 does say "Reviewer-assignment counts here too", and it is a real gap
(#755 finding 10). But the trigger lands WITH the routing,
not before it: a trigger firing into a router that ignores it is dead weight
that only burns runs and makes the stub lie about what it supports.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Contributor Author

Pushed the review fixes in 1953ccd + 277f6de. Tests 31 → 42, shellcheck and actionlint clean.

Fixed

The fail-open trio (#755's headline finding — errors read as negative answers):

  • Manifest fetch: curl -f collapsed 404 and 5xx into one exit code, so a raw.githubusercontent hiccup silently disabled every persona fleet-wide, with no error and indistinguishable from nobody being addressed. Now switched on status: 200 routes, 404 is a real answer ("not a persona"), anything else is a hard failure.
  • Labels: || echo "" turned an API error into "no labels" — bypassing an opt-out and arming an ungated write. Now fails closed. Discussions genuinely have no labels surface, so empty is the true answer there; that difference is load-bearing.

gate_label is now enforced (security). The schema only guarantees it's declared — declaring a lock isn't locking the door. Write-mode now requires the gate applied; a write persona with no gate_label is a hard error; write-mode where labels are unavailable is refused because the gate can't be verified.

contents: read + PyYAML (Copilot, both correct). PyYAML is not documented as preinstalled on the runner images, and this reusable runs on the caller's runner — our own test workflow installs it explicitly, which proved the gap.

Mention precision: no longer fires inside code fences or inline code, where GitHub itself renders no mention. Blockquotes are a deliberate divergence — GitHub does notify on quoted mentions, but one-click Quote reply copies a whole prior comment prefixed >, and neither recursion axis catches a human quoting a human.

I dropped two things from f3a6fb0 (@donpetry-bot's push) — with reasons

Not silently, and not because they were unwelcome:

1. review_requested routing — fabricated trust.

# Use COLLABORATOR as a synthetic floor-check association; assigning a team
# reviewer requires at least write access, which maps to COLLABORATOR.

This asserts a trust level rather than checking one. The fleet already has the correct pattern for this exact case: pr-review-mention-reusable.yml queries /repos/{repo}/collaborators/{user}/permission precisely because author_association is absent on review_requested. Synthesising COLLABORATOR means the persona's declared trust floor is evaluated against a value nobody verified — on the one path where the payload can't tell us.

The feature is legitimate and wanted (§4, #755 finding 10). It should land with a real permission-API check, in its own PR. I also removed the matching stub trigger: it fired into a router that ignores it — a dead trigger that only burns runs.

2. pm_persona_aliases — unreachable by construction.
It reads aliases from a manifest. To have the manifest you must already have resolved slug → path. If the slug is an alias, personas/<alias>/persona.yml 404s and there is no manifest to read them from. It can never fire for a real alias.

That's not a coding slip — it's the proof that index-free routing cannot honour aliases at all (#755 finding 1). The contract in #752 and the implementation here genuinely contradict, and no amount of code here closes it. It needs a decision: drop aliases (my recommendation) or add a derived personas/aliases.json. Dropping needs an ordered cross-repo pair.github-private must remove aliases: [] from qa-lead's manifest before the schema drops it, or additionalProperties: false breaks validate-personas on main fleet-wide.

Still out of scope here, deliberately

@donpetry-bot

Copy link
Copy Markdown
Contributor

CI checks on this PR are still running. Once they complete, re-mention @donpetry-bot to trigger a fresh review.

Posted by the donpetry-bot PR-review cascade.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/persona-mention-reusable.yml:
- Around line 179-196: Update the manifest fetch in the curl invocation to send
the available GH_TOKEN as an Authorization header when requesting
.github-private URLs. Preserve the existing HTTP status handling, including
treating genuine 404 responses as non-personas and failing other fetch errors.

In `@standards/workflows/persona-mention.yml`:
- Around line 49-53: Fix the indentation of the explanatory comment beginning
“NOTE: reviewer assignment” in the workflow YAML so it matches the surrounding
block structure and satisfies yamllint’s comments-indentation rule. Preserve the
comment text and its placement.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d8d07379-0dcf-407a-a62b-affec23c96b0

📥 Commits

Reviewing files that changed from the base of the PR and between 518ff2b and 02ac387.

📒 Files selected for processing (5)
  • .github/workflows/persona-mention-reusable.yml
  • scripts/lib/persona-mention.sh
  • scripts/persona-mention-requirements.txt
  • standards/workflows/persona-mention.yml
  • tests/persona_mention.bats

Comment thread .github/workflows/persona-mention-reusable.yml
Comment thread standards/workflows/persona-mention.yml Outdated
@don-petry
don-petry disabled auto-merge July 17, 2026 12:02
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 17, 2026
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

@don-petry

Copy link
Copy Markdown
Contributor Author

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

No changes were needed for this PR.

…ent-death path

Reverts the Authorization header added in 9333997, which was made in response to
CodeRabbit's 🔴 Critical "the repo is private, so raw.githubusercontent will 404
and every persona gets skipped".

That premise is false, and acting on it introduces the exact bug this PR exists
to fix.

petry-projects/.github-private is PUBLIC despite its name (private: false —
every one of the org's 11 repos is public). Measured against the real manifest
URL:

    unauthenticated -> 200      valid token -> 200
    expired token   -> 404      empty token -> 404

raw.githubusercontent answers a bad or MISSING token with 404, not 401. Our
router — correctly — treats 404 as a real answer: "not a persona". So with an
Authorization header, the moment GH_PAT_WORKFLOWS expires, or a repo adopts the
stub before the secret exists, or a fork PR runs without secrets, EVERY persona
404s and is silently skipped as "not a persona". Fleet-wide, silent, and
indistinguishable from nobody being addressed.

Unauthenticated cannot fail that way for a public repo. The header buys nothing
(200 either way) and adds a silent-death path — the precise fail-open class this
PR closes. Reverted, with the measurements recorded inline so the next reader
(human or agent) does not re-add it on the same false premise.

The stub's comment re-indent from 9333997 is kept; it is cosmetic and fine.

Verification
- bats tests/persona_mention.bats: 42/42.
- actionlint (CI's ignores): clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@don-petry don-petry left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai your 🔴 Critical (".github-private is private, so raw.githubusercontent returns 404 and every persona gets skipped") is a false positive driven by the repo's name, and acting on it introduced the exact bug this PR exists to fix. Reverted in the latest commit, with the evidence recorded inline.

petry-projects/.github-private is PUBLICprivate: false. Every one of the org's 11 repos is public; the name is a leftover and lies.

Measured against the real manifest URL:

Auth Result
unauthenticated 200
valid token 200
expired token 404
empty token 404

raw.githubusercontent answers a bad or missing token with 404, not 401. This router — correctly — treats 404 as a real answer: "not a persona".

So adding Authorization: Bearer $GH_TOKEN means that the moment GH_PAT_WORKFLOWS expires, or a repo adopts the stub before the secret exists, or a fork PR runs without secrets, every persona 404s and is silently skipped — fleet-wide, with no error, indistinguishable from nobody being addressed. That is precisely the fail-open class this PR closes (#755's headline finding).

Unauthenticated cannot fail that way for a public repo. The header buys nothing — 200 either way — and adds a silent-death path. The measurements are now inline above the case so the next reader doesn't re-add it on the same premise.

@don-petry
don-petry disabled auto-merge July 17, 2026 12:30
@don-petry

Copy link
Copy Markdown
Contributor Author

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

No changes were needed for this PR.

@don-petry
don-petry disabled auto-merge July 17, 2026 12:56
@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Contributor Author

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

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) July 17, 2026 12:57

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: b022ed0fabb5eec1489f7620ba0e14b25554c977
Review mode: triage-approved (single reviewer)

Summary

Adds the org-wide persona @-mention router: a reusable workflow, a pure fail-closed routing library (scripts/lib/persona-mention.sh), a thin caller stub, 42 bats tests, canary-ring registration, and standards updates. Security posture is strong: all event fields reach shell via env (never inline ${{ }}), the trust floor is enforced in the job-level if: before secrets are exposed, actions are SHA-pinned, PyYAML is hash-locked, permissions default to {}, and recursion is guarded on two independent axes (bot actor + persona marker) per the #860 postmortem. Triage's low-risk call is broadly confirmed; I rate it MEDIUM (new workflow trust surface with an org PAT) — still within auto-approve range.

Linked issue analysis

No closing-issue references. The PR implements the addressing contract defined in #752 (persona-standards.md §4.1) and closes out findings carried from #755 (fail-open trio, write-mode gate arming, mention precision) — each is substantively addressed in the diff with matching regression tests.

Findings

Verified disputed finding — resolved in the author's favor. CodeRabbit's critical claim that petry-projects/.github-private is private (so unauthenticated raw fetches would 404 fleet-wide) is a false positive: the repo is public (verified via API: private=false, visibility=public). The unauthenticated fetch is the correct design — an Authorization header would convert an expired/missing PAT into silent 404s for every persona.

Review threads: all 15 resolved; CodeRabbit's two CHANGES_REQUESTED reviews are superseded (latest states all COMMENTED).

Non-blocking nits:

  1. scripts/lib/persona-mention.sh has a duplicated doc-comment block for pm_extract_slugs (the header appears above pm_strip_unaddressable and again above the function).
  2. In the reusable workflow, an unparseable manifest yields an empty pm_mention_decision result and is skipped as "not enabled" rather than erroring loudly (the here-string swallows the query's exit status). Fail-closed, so safe — but slightly at odds with the fail-loud philosophy applied elsewhere.

Secret scan: run_secret_scanning MCP tool unavailable in this run; gitleaks CI check passed, and the only hashes in the diff are pip package integrity hashes.

CI status

All checks green: CodeQL, SonarCloud (quality gate passed, 0 new issues), ShellCheck, bats, gitleaks, npm audit, AgentShield, Agent Security Scan, Lint. Ecosystem audits not applicable were skipped.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

@don-petry
don-petry merged commit 33496b8 into main Jul 17, 2026
23 checks passed
@don-petry
don-petry deleted the feat/persona-mention-router branch July 17, 2026 13:06
don-petry added a commit that referenced this pull request Jul 17, 2026
…thub-private#1289 merges (#769)

* refactor(personas): drop address.aliases from the contract (half 2 of 2)

Second half of the ordered cross-repo pair. petry-projects/.github-private#1289
removed every USE of the field first; this removes the field itself.

MERGE ORDER: #1289 MUST land before this. The schema is
additionalProperties:false, so dropping `aliases` while qa-lead's manifest still
carries `aliases: []` fails validate-personas on main FLEET-WIDE. Verified the
rejection is real: a manifest with `aliases` now yields "Additional properties
are not allowed ('aliases' was unexpected)".

Why aliases are gone (see .github#755 finding 1)

They existed so a RENAMED role kept routing — qa-lead listing
`petry-projects/murat` so old comments still reached it. Two reasons it goes:

1. Structural: index-free routing could never honour it. The router resolves
   '@petry-projects/murat' to personas/murat/persona.yml — a 404 after the
   rename. The alias is declared INSIDE the renamed persona's own manifest,
   which the router never opens, because it does not know to look there.
   Chicken-and-egg; the pm_persona_aliases attempt in #754 was unreachable for
   exactly this reason.

2. Measured: there is nothing live left to route. GitHub stops rendering a
   renamed team's old handle as a mention at all. Probed it directly — created
   a team, rendered its mention, renamed it, re-rendered the OLD handle:

     before rename:  <a class="team-mention" …>@petry-projects/probe</a>
     after rename:   <p>@petry-projects/probe please review</p>

   The old handle becomes PLAIN TEXT. So it is not a silently-broken mention
   that looks live — the reader sees grey text and knows instantly it did not
   resolve, and the fix is local: mention the new handle. Aliases would only
   route text GitHub itself no longer treats as a mention. Not worth a derived
   index, an extra fetch on every 404, and a contract the router cannot honour.

Rule 4 becomes a theorem, not a check

"Handles and aliases MUST be unique across all personas" is replaced by
"Handles are unique BY CONSTRUCTION": the pattern pins the org, slug == id,
id == the persona directory name, and directory names are unique — so two
personas cannot claim the same handle. #1289 removed the corresponding
check_handles_unique for the same reason: it could never fire.

Rule 5 ("a rename keeps the old handle as an alias") is replaced by a §4.1
"Renames" section documenting what to change on a rename and stating plainly
that old mentions stop resolving VISIBLY — which is the correct signal, not a
regression.

Changes
- standards/personas/persona.schema.json — drop address.aliases (edited as TEXT,
  not re-serialized: a json round-trip reformats every inline array and rewrote
  264 untouched lines. Diff is +2/-10.)
- standards/persona-standards.md — §4.1 rules 4-5, the example, and the new
  Renames section
- standards/personas/TEMPLATE/persona.yml — drop the aliases scaffold line

Verification
- persona.schema.json is a valid Draft 2020-12 schema; diff is +2/-10.
- Conditional behaviour verified: qa-lead's post-#1289 shape (handle only)
  PASSES; a manifest carrying `aliases` FAILS — which is exactly why #1289 goes
  first.
- TEMPLATE/persona.yml validates against the updated schema.
- markdownlint-cli2: clean.
- Zero `aliases` references remain outside the deliberate explanatory notes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(reviews): address review comments [skip ci-relay]

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Co-authored-by: Don Petry Bot <donpetry+bot@gmail.com>
don-petry added a commit to petry-projects/.github-private that referenced this pull request Jul 18, 2026
* feat(personas): the persona runtime — dispatch side of the mention framework

Closes the loop the router opened. petry-projects/.github#754 detects an
`@petry-projects/<role>` mention and fires a `persona-mention`
repository_dispatch at this repo; nothing listened. This is what listens.

Architecture: ONE runner, single-homed

The runner is NOT fanned out to the fleet. Every mention across every repo funnels
through the router's dispatch to THIS repo, so the runner lives only here and is
called locally (uses: ./). That is why it needs no channel tag or canary
registration — unlike the router (`persona-mention`), which IS consumed by fleet
repos and rolls out through the rings. One runner serves every persona,
convention-driven (prompts/<id>/advisory.md), never one workflow per persona
(§4.1: per-agent workflows are the drift the framework exists to prevent).

The recursion marker is closed from BOTH sides

The router skips any comment carrying '<!-- persona:'. This runner's prompt and
its pr_comment_has_marker guard REQUIRE every advisory's first line to be
'<!-- persona:<id> -->'. So a persona's own comment can never re-summon it or any
persona named in the thread — the .github-private#860 failure (1,481 acks in
4.5h), closed from the writing side.

Contents
- prompts/qa-lead/advisory.md — qa-lead's advisory behaviour: read the vendored
  bmad-tea skill by path, assess test-risk (risk vs value, no manufactured risk),
  post ONE advisory comment. Advisory only — no writes, no PRs, no labels.
- scripts/lib/persona-runner.sh — the pure core: payload re-validation (this
  runner is a SEPARATE trust boundary — a repository_dispatch can be sent by
  anything with a token, so a malformed persona id must never reach a path
  interpolation), advisory-prompt resolution by convention, and the shared marker.
- .github/workflows/persona-runner-reusable.yml — the runtime: resolve prompt →
  install claude-code → run headless → the prompt posts one marked comment. PAT
  (not github.token) because the item lives in a DIFFERENT repo. Rate-limit is a
  soft skip, matching ci-failure-analyst.
- .github/workflows/persona-runner.yml — thin caller: maps the dispatch payload to
  the reusable's inputs, nothing else.
- tests/persona_runner.bats — 15 tests, weighted to the marker and payload
  re-validation (path traversal, bad ids, missing prompt → rc 3 not a crash).
- lint.yml — wires the suite into CI (a bats suite with no runner is unenforced).

A persona with no prompts/<id>/advisory.md yet is a clean no-op ("no runtime
wired"), not a failure — a persona may declare an address before its runtime
exists.

NOT wired live here. persona-runner-reusable.yml is workflow_call-only, so per
the org standard it must be set disabled_manually after merge (GitHub ignores the
flag for workflow_call). And no mention reaches it until the router release is cut
and .github-private adopts the `persona-mention` stub — the `next`-ring
enablement, which follows in sequence. This PR is the runtime; turning it on is
the next step.

Verification
- bats tests/persona_runner.bats: 15/15.
- shellcheck --severity=warning -x scripts/lib/persona-runner.sh: clean.
- actionlint (CI's ignores) on both workflows: clean.
- prompts/** is a markdownlint ignore (agent instructions, not prose).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(reviews): address review comments [skip ci-relay]

* fix(personas): make the recursion marker MECHANICALLY enforced (review)

Copilot's most important finding: the agent posted its OWN comment, so the
marker was only prompt-REQUESTED, not enforced. An agent that forgot it would
post an unmarked comment → self-summon → #860 (1,481 acks in 4.5h). My
"closed from both sides" claim was false. Restructured, not patched.

Read/write split

- The AGENT now runs with github.token, which reads the PUBLIC source repo but
  CANNOT post to it (scoped to THIS repo). It PRINTS the advisory between
  sentinels; it never receives GH_PAT_WORKFLOWS.
- The WORKFLOW extracts the body, guarantees the marker (pr_ensure_marker —
  prepends if the agent omitted it), and is the ONLY writer, using the PAT.

So an agent that forgets the marker cannot post an unmarked comment because it
cannot post at all. The guard is now mechanical. New helpers pr_extract_advisory
/ pr_ensure_marker, tested to the invariant "the posted body always starts with
the marker".

Other review fixes

- Gemini (high): pr_comment_has_marker kept the trailing '\r' from GitHub's CRLF
  bodies, so the guard silently never matched. Strip it; CRLF test added.
- Copilot: resolve step conflated rc 2 (malformed/hostile id) with rc 3 (no
  runtime). Now rc 2 FAILS loudly, rc 3 is the benign skip.
- Copilot: concurrency group keyed only on item_number, which is empty for
  discussions → all discussion mentions for one persona+repo collapsed into one
  group. Added comment_url to the key.
- Copilot: prompt used $COMMENT_URL (an html_url) with `gh api`, referenced an
  undefined $BODY, and put a parenthetical on the marker line. Rewritten: the
  agent derives the API url, prints between sentinels (no self-post, no $BODY),
  and the marker is alone on the first body line.
- Gemini: `head -400` → `head -n 400 || true` (POSIX + no SIGPIPE under pipefail).

Discussion posting is explicitly a no-op-with-notice for now (the issue-comments
endpoint doesn't serve discussions); wired when the discussion path lands.

Verification
- bats tests/persona_runner.bats: 21/21 (15 + 6: CRLF, sentinel extraction x2,
  marker-prepend, unchanged-when-present, and the post-body invariant).
- shellcheck --severity=warning -x scripts/lib/persona-runner.sh: clean.
- actionlint (CI ignores) on both workflows: clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(reviews): address review comments [skip ci-relay]

* fix(personas): restore the CRLF test; pin the COMMENT_URL contract

Two regressions from the review-agent's push (b6f12fa):

1. It DELETED the CRLF test that Gemini specifically requested and that guards
   the '\r'-stripping fix. Restored — without it, a future revert of the strip
   would pass CI silently.

2. It changed the prompt to `gh api "$COMMENT_URL"` (correct target: an API url),
   but the router (#754, merged) currently sends `.html_url` — and `gh api`
   cannot fetch a github.com browser url, so the comment-body read fails. The
   prompt already degrades gracefully ("if that fails, proceed from the item"),
   so the soak still works; but the contract must be made real.

   The fix is in the ROUTER, not here: it should send the comment's `.url` (API)
   not `.html_url`. Companion PR on petry-projects/.github does that. Documented
   the contract in the prompt so the two sides agree.

Verification
- bats tests/persona_runner.bats: 22/22 (CRLF test back).
- shellcheck --severity=warning -x: clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants