From 518ff2b9eddaac8236e5eac8afb7180159b5f570 Mon Sep 17 00:00:00 2001 From: Don Petry Date: Thu, 16 Jul 2026 22:07:15 -0500 Subject: [PATCH 1/7] feat(personas): add the persona @-mention router MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../workflows/persona-mention-reusable.yml | 196 +++++++++++++++ .github/workflows/persona-mention-tests.yml | 74 ++++++ scripts/lib/persona-mention.sh | 208 ++++++++++++++++ standards/canary-rings.json | 63 +++++ standards/ci-standards.md | 1 + standards/persona-standards.md | 15 ++ standards/workflows/persona-mention.yml | 58 +++++ tests/persona_mention.bats | 229 ++++++++++++++++++ 8 files changed, 844 insertions(+) create mode 100644 .github/workflows/persona-mention-reusable.yml create mode 100644 .github/workflows/persona-mention-tests.yml create mode 100644 scripts/lib/persona-mention.sh create mode 100644 standards/workflows/persona-mention.yml create mode 100644 tests/persona_mention.bats diff --git a/.github/workflows/persona-mention-reusable.yml b/.github/workflows/persona-mention-reusable.yml new file mode 100644 index 000000000..d2d05f702 --- /dev/null +++ b/.github/workflows/persona-mention-reusable.yml @@ -0,0 +1,196 @@ +# Reusable Persona Mention Router. +# SOURCE OF TRUTH: petry-projects/.github/.github/workflows/persona-mention-reusable.yml +# Standard: https://github.com/petry-projects/.github/blob/main/standards/persona-standards.md (§4.1) +# +# Routes `@petry-projects/` mentions to the addressed persona. ONE router +# for every persona — personas do NOT each ship a mention workflow (§4.1); that +# per-agent drift is exactly what the manifest exists to prevent. +# +# How a mention resolves, and why there is no index: +# `validate-personas.py` enforces address.handle's slug == id == the persona's +# directory name, so '@petry-projects/qa-lead' resolves to +# personas/qa-lead/persona.yml in .github-private by convention. The manifest +# IS the index-of-record (§1.1). A 404 means "not a persona" — which is also +# how a real, non-persona team like @petry-projects/org-leads falls through +# harmlessly. +# +# The manifest, not this file, decides: +# whether the mention surface is enabled, in what mode, behind which trust +# floor, and under which opt-out label. This router restates none of it. +# +# Requires: +# GH_PAT_WORKFLOWS — org secret, classic PAT (repo scope) for API calls and dispatch +name: Persona Mention Router (Reusable) + +on: + workflow_call: + inputs: + tooling_ref: + description: | + Ref of petry-projects/.github to source scripts/lib/persona-mention.sh + from. Leave empty (the default) to source it from THIS reusable's own + commit (`github.job_workflow_sha`), which guarantees the routing logic + always matches the version of this workflow the caller pinned, with no + skew (the #465/#528 lesson: a fixed `v1` default predates the library + and fails every run with "No such file or directory"). Override only to + test a branch end-to-end. + required: false + default: '' + type: string + persona_ref: + description: | + Ref of petry-projects/.github-private to read persona manifests at. + Defaults to main — a persona's live contract is what is merged, not + what a branch proposes. Override only for end-to-end testing. + required: false + default: 'main' + type: string + secrets: + GH_PAT_WORKFLOWS: + description: 'Classic PAT with repo scope, used for API calls and dispatching the persona' + required: true + +permissions: {} # no default permissions; each job grants only what it needs + +concurrency: + # Serialise per comment, not per thread: two different personas addressed in + # one comment are dispatched by a single run, and cancelling an in-flight route + # would silently drop a mention. + group: persona-mention-${{ github.event.comment.id || github.run_id }} + cancel-in-progress: false + +jobs: + route: + runs-on: ubuntu-latest + permissions: + issues: read # read the item's labels for the opt-out check + pull-requests: read + # The CHEAP pre-filter, mirrored from pm_should_route (tests/persona_mention.bats). + # It runs BEFORE the job starts, so secrets are never exposed to a run that a + # bot, a marked comment, or an untrusted actor triggered. + # + # Two independent recursion axes, per §4.1 and the #860 postmortem (1,481 + # acks in 4.5h). Comments posted via a PAT re-trigger workflows, unlike + # GITHUB_TOKEN, and with N mutually addressable personas the cycles are + # combinatorial, not self-loops: + # 1. bot actor — a comment authored by an agent identity never routes + # 2. marker — a comment carrying ' risk looks low' + [ "$status" -eq 0 ] + run pm_is_agent_comment ' on it' + [ "$status" -eq 0 ] +} + +@test "pm_is_agent_comment does not match a human comment" { + run pm_is_agent_comment 'no marker here' + [ "$status" -ne 0 ] +} + +@test "pm_is_bot_actor recognises the agent identities" { + run pm_is_bot_actor donpetry-bot + [ "$status" -eq 0 ] + run pm_is_bot_actor 'github-actions[bot]' + [ "$status" -eq 0 ] +} + +@test "pm_is_bot_actor does not match a human" { + run pm_is_bot_actor don-petry + [ "$status" -ne 0 ] +} + +@test "pm_should_route blocks a comment authored by an agent (axis 1)" { + run pm_should_route donpetry-bot OWNER "@petry-projects/qa-lead please review" + [ "$status" -ne 0 ] +} + +@test "pm_should_route blocks a comment carrying the agent marker (axis 2)" { + run pm_should_route don-petry OWNER ' @petry-projects/dev-lead over to you' + [ "$status" -ne 0 ] +} + +@test "pm_should_route blocks the combinatorial cross-persona loop" { + # The failure mode §4.1 warns about: qa-lead answering a thread and mentioning + # dev-lead. Marked AND bot-authored — either axis alone must stop it. + run pm_should_route donpetry-bot OWNER ' @petry-projects/dev-lead your turn' + [ "$status" -ne 0 ] +} + +@test "pm_should_route allows a trusted human addressing a persona" { + run pm_should_route don-petry OWNER "@petry-projects/qa-lead please review" + [ "$status" -eq 0 ] +} + +@test "pm_should_route blocks an untrusted commenter" { + run pm_should_route drive-by NONE "@petry-projects/qa-lead please review" + [ "$status" -ne 0 ] +} + +@test "pm_should_route blocks a comment that addresses nobody" { + run pm_should_route don-petry OWNER "looks good to me" + [ "$status" -ne 0 ] +} + +# --- trust ----------------------------------------------------------------- + +@test "pm_trust_ok admits an association in the floor" { + run pm_trust_ok MEMBER OWNER MEMBER COLLABORATOR + [ "$status" -eq 0 ] +} + +@test "pm_trust_ok denies an association outside the floor" { + run pm_trust_ok CONTRIBUTOR OWNER MEMBER COLLABORATOR + [ "$status" -ne 0 ] +} + +@test "pm_trust_ok denies when the floor is empty" { + # A persona that declared no floor must not be MORE permissive than one that did. + run pm_trust_ok OWNER + [ "$status" -ne 0 ] +} + +@test "pm_trust_ok denies an empty association" { + run pm_trust_ok "" OWNER MEMBER + [ "$status" -ne 0 ] +} + +# --- manifest decisions ---------------------------------------------------- + +@test "pm_mention_decision reads an explicit enabled mention surface" { + run pm_mention_decision "$(manifest "$MENTION_ON")" + [ "$output" = "true advisory qa-lead:hands-off" ] +} + +@test "pm_mention_decision honours an explicitly disabled mention surface" { + run pm_mention_decision "$(manifest ' - surface: mention + enabled: false + mode: advisory')" + [ "$output" = "false advisory qa-lead:hands-off" ] +} + +@test "pm_mention_decision falls back to default_mode advisory when unlisted" { + run pm_mention_decision "$(manifest ' - surface: issues + enabled: true + mode: advisory')" + [ "$output" = "true advisory qa-lead:hands-off" ] +} + +@test "pm_mention_decision treats default_mode off as not addressable" { + local m + m="$(manifest ' - surface: issues + enabled: true + mode: advisory')" + run pm_mention_decision "${m/default_mode: advisory/default_mode: off}" + [ "${output% *}" = "false off" ] +} + +@test "pm_mention_decision surfaces a write-mode mention" { + run pm_mention_decision "$(manifest ' - surface: mention + enabled: true + mode: write + gate_label: qa-lead')" + [ "$output" = "true write qa-lead:hands-off" ] +} + +@test "pm_mention_trust_floor defaults to the persona-wide floor" { + run pm_mention_trust_floor "$(manifest "$MENTION_ON")" + [ "$output" = "OWNER MEMBER COLLABORATOR" ] +} + +@test "pm_mention_trust_floor lets a surface tighten the persona-wide floor" { + run pm_mention_trust_floor "$(manifest ' - surface: mention + enabled: true + mode: advisory + trust_floor: [OWNER]')" + [ "$output" = "OWNER" ] +} + +@test "pm_persona_id reads the id the manifest claims" { + run pm_persona_id "$(manifest "$MENTION_ON")" + [ "$output" = "qa-lead" ] +} + +@test "pm_manifest_query fails loudly on unparseable YAML" { + run pm_manifest_query '.id' <<<'this: [is: not: yaml' + [ "$status" -ne 0 ] +} + +# --- urls ------------------------------------------------------------------ + +@test "pm_manifest_url builds the raw manifest path by convention" { + run pm_manifest_url qa-lead + [ "$output" = "https://raw.githubusercontent.com/petry-projects/.github-private/main/personas/qa-lead/persona.yml" ] +} + +@test "pm_manifest_url honours PERSONA_REF for testing against a branch" { + PERSONA_REF=some-branch run pm_manifest_url qa-lead + [[ "$output" == */some-branch/personas/qa-lead/persona.yml ]] +} From f3a6fb06a983e1dcccd881b5a0084a2080d0503f Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Fri, 17 Jul 2026 03:25:46 +0000 Subject: [PATCH 2/7] fix(reviews): address review comments [skip ci-relay] --- .../workflows/persona-mention-reusable.yml | 84 ++++++++++++++----- scripts/lib/persona-mention.sh | 49 ++++++++--- standards/workflows/persona-mention.yml | 5 ++ tests/persona_mention.bats | 55 +++++++++++- 4 files changed, 160 insertions(+), 33 deletions(-) diff --git a/.github/workflows/persona-mention-reusable.yml b/.github/workflows/persona-mention-reusable.yml index d2d05f702..f0a325798 100644 --- a/.github/workflows/persona-mention-reusable.yml +++ b/.github/workflows/persona-mention-reusable.yml @@ -63,6 +63,7 @@ jobs: route: runs-on: ubuntu-latest permissions: + contents: read # actions/checkout of petry-projects/.github tooling issues: read # read the item's labels for the opt-out check pull-requests: read # The CHEAP pre-filter, mirrored from pm_should_route (tests/persona_mention.bats). @@ -81,12 +82,23 @@ jobs: # The trust check here is the conservative §4 default. A persona may TIGHTEN # it in its own manifest (enforced below), never loosen it — so this gate is # always at least as strict as the union of what the personas allow. + # + # review_requested events (§4 "mention" surface, "Reviewer-assignment counts + # here too") are also admitted: only org members with write+ access can assign + # team reviewers, so the recursion guards do not apply. if: | - contains(github.event.comment.body, '@petry-projects/') && - github.event.comment.user.login != 'donpetry-bot' && - github.event.comment.user.login != 'github-actions[bot]' && - !contains(github.event.comment.body, '