[US-398] feat: solo-maintainer explicit-approval token for 🔴 PRs

[rucka]

PR Information

PR Title: [US-398] feat: solo-maintainer explicit-approval token for 🔴 PRs
Story/Epic: #398 (epic #208)
Type: Feature
Priority: High
Assignee: rucka
Labels: user story, risk:yellow

Summary

What Changed

An alternative satisfaction path for D10 ("🔴 needs a non-author human approving review"), for a repository that cannot produce a second human: a head-bound /approve <head-sha> comment, authorized server-side in two stages that both have to pass.

Stage	What it decides	Where
1 — the comment	host-asserted fields only: user.type=="User", performed_via_github_app==null, author_association ∈ {OWNER,MEMBER,COLLABORATOR} (pre-filter), .user.login != PR_AUTHOR unless the repo opted in, and /approve <HEAD_SHA> owning its line, outside fenced regions and HTML comments	human_token_approval_select in assets/pr-state.sh
2 — the actor	GET /repos/{owner}/{repo}/collaborators/{login}/permission must be admin/maintain/write; a lookup that could not answer is kept distinct from none	token_approver_login / token_permission_sufficient, same asset

Every refusal names itself. One published description per state, because telling a maintainer who just posted a valid token to post one is the failure the whole flow exists to remove:

State	Published description
nothing posted	needs a non-author human approval, or /approve <sha> posted by a human maintainer
posted by the PR author, opt-in not set	token posted by the PR author; set PAIR_SOLO_APPROVAL_TOKEN=true if this repo has one human
posted, actor not write-level	token from <login> not authorized: repository permission is not write-level
posted, permission lookup unanswerable (403/5xx)	a token was posted but could not be authorized: permission lookup failed, see the run log

• The author exclusion is on by default. PAIR_SOLO_APPROVAL_TOKEN=true (repo Actions variable, compared case-insensitively with whitespace ignored — it is typed into a free-text box) is the single-human declaration that suspends it. Nothing in a comment payload can say "this repository has one human", so the repository says it. Unset (every multi-human repo, and the safe default), a 🔴 PR can never be satisfied by its own author — and the fourth description above is what keeps that default from becoming an undiagnosable dead end for the solo maintainer who has not run gh variable set yet.
• author_association is not authorization. MEMBER means "member of the owning organization", not "has push access here" — the permission read is what authorizes.
• The command owns its line, and two invisible-or-code regions are removed first. The anchor (^|\n)/approve <sha>[ \t\r]*(\n|$) rejects > quote-replies, 4-space-indented blocks and inline backticks. It cannot see a fence (a fence puts the command at column 0 of its own line) nor an HTML comment (which renders as nothing at all), so both are stripped before it runs. The fence split is line-anchored ((^|\n) {0,3}```[^\n]*, CommonMark's three-space allowance, info string consumed) and rejoins with "" — a bare backtick-parity split read an inline ```gh``` span as a fence, and joining with "\n" then manufactured the line boundary that made a mid-line mention approve.
• HEAD_SHA is validated ^[0-9a-f]{40}$ before being concatenated into that regex (a length check accepted 40 metacharacters).
• A comment run gets its own concurrency group. concurrency is evaluated at RUN level, before any job if:, and GitHub cancels a group's pending run whenever a newer one queues into it — so a shared group is unsafe in both directions and merely making comment runs non-cancelling fixes one: a /approve waiting behind a synchronize evaluation is dropped by the next ordinary "thanks!". Keying by github.event.comment.id makes a token run neither cancel nor be cancelled; edited/deleted of the same comment share the key, so the newest state of that comment wins. Push/label/review runs stay in the shared eval group (property 4).
• The audit head is abbreviated to 12 chars: the commit-status description is capped at 140 characters, and the full SHA put any login of 24+ characters over it — a rejected POST under set -euo pipefail leaves the pending placeholder standing, i.e. the "permanently unmergeable 🔴 PR" this story exists to remove. The status is still posted on the full SHA.
• The issue_comment job body-filters the created action on /approve; edited/deleted stay unfiltered, because a withdrawal no longer contains the command and is exactly the event that must re-evaluate. Stated with its price: an edit or delete of any comment on any PR runs the job in full, pending-first flip included.
• The review path is queried first and wins wherever an approval exists — but it is not free for a multi-human repo: the token branch runs whenever the reviews query returns zero, which is the state of every 🔴 PR before its first review.
• ADR-018 amended (not superseded): the guarantee, the author-exclusion decision and its opt-in, the two-stage authorization, the concurrency grouping, the four-state reporting, and the corrected Dedicated review identity + adoption-gated light-PR auto-approval #218 claim.

Why This Change

A single-account repo cannot produce a non-author human review, so 🔴 was unusable there — every 🔴 PR permanently unmergeable regardless of quality. This gives that repo a named, weaker, explicitly-labelled alternative instead of silently downgrading the rule.

What the token is worth, stated plainly (and in the ADR): explicit human confirmation, not independent review — deliberateness, an audit trail, invalidation on change. It is not forgery-resistant while the agent runs on the maintainer's credentials, and #218 recovers that property only if the agent ships as a GitHub App or Bot identity: a machine user account holding a PAT is user.type=="User" with no app attribution, so the predicate would accept it, and closing that shape needs an explicit deny-list of the agent login. Residual documented, not hidden.

Story Context

User Story: solo-maintainer explicit-approval token for 🔴 PRs (verified human token instead of a second account).
Acceptance Criteria: AC1–AC7 implemented and tested (see Testing). AC4's business rule — "the token is a fallback for a configuration that cannot produce a second human, not a shortcut for one that can" — is executable as the self-approval exclusion (the opt-in), with the residual looseness argued rather than left implied. AC1's happy case and AC5 are verified OFFLINE only — see Follow-up Work; nothing in this diff has run against a live GitHub host.

Changes Made

Implementation Details

• Comment command, not a label: a label carries no head SHA, so binding it to the current head means comparing a timeline timestamp against the head commit — a comparison a force-push invalidates in either direction. A comment names the head explicitly.
• Two-stage authorization: stage 1's predicate is the ONE text all three projections (count, logins, audit line) are built from, so the count the gate acts on and the line it publishes cannot drift; stage 2 takes a lookup command as its first argument (the job passes a gh api wrapper, the smoke test a fixture reader), so one code path decides in the recipe and in the test.
• Four states, four descriptions: token_approver_login exits 1/2/3 (nothing posted / not write-level / lookup unanswerable) and the job adds a 4th by re-running stage 1 with the opt-in forced on (token_blocked_by_author_exclusion) when the candidate list is empty — the only way to tell "you posted nothing" apart from "you posted, and the exclusion dropped it".
• Invisible and code regions are dropped before the anchor: <!-- … --> (including an unclosed one, which swallows the rest of the body) then line-anchored ```/~~~ fences, keeping the even-indexed segments and rejoining with "". An unclosed fence swallows everything after it, which is both what GitHub renders and the fail-safe direction; a genuine token before or after a closed fence, or beside an HTML comment, still counts.
• Fail-safe everywhere: no lookup command ⇒ no approver; empty PR_AUTHOR with the opt-in off ⇒ nothing counts; an unresolved PR_AUTHOR can never match a candidate (so a failed author resolution is never reported as a self-approval); a non-hex/absent HEAD_SHA ⇒ nothing counts; a permission lookup that cannot answer ⇒ rejected and said so. The audit line is drawn only for the actor stage 2 authorized.
• Fresh API read each run — a withdrawn or edited token stops counting immediately; a quote-reply of a withdrawn token does not resurrect it.
• Repository setting for adopters: PAIR_SOLO_APPROVAL_TOKEN, unset by default. Leaving it unset is the safe configuration and requires no action. This repository must set it (its PR author and its maintainer are one person) — recorded in way-of-working.md and ADR-018 § Adoption Impact.

Files Changed

• Modified: .pair/adoption/tech/adr/adr-018-…md, .pair/adoption/tech/way-of-working.md, .pair/knowledge/assets/pr-state.sh (+ dataset source), …/github-implementation.md (+ dataset source), …/pr-states.md (+ dataset source), apps/website/content/docs/concepts/pr-state-flow.mdx, packages/knowledge-hub/src/conformance/pr-state-flow.test.ts, scripts/smoke-tests/scenarios/pr-state-flow.sh
• Added: scripts/smoke-tests/fixtures/github-pr-comments.json, scripts/smoke-tests/fixtures/github-collaborator-permissions.json
• The root .pair/knowledge/** copies were regenerated through the production transform (buildDatasetSkillNameMap + buildSkillLinkPathMap + applyKnownMirrorTransforms — the same pair pair update runs), never hand-ported; mirror-guard is green.

Testing

Test Coverage

• Smoke (the shipped predicate, executed): scripts/smoke-tests/scenarios/pr-state-flow.sh sources the shipped pr-state.sh and runs the real jq filters and the real shell functions against fixtures — no reimplementation. Accept: human non-author on head; PR author with the opt-in on (true/True/TRUE/true); a genuine token beside a closed fence; a genuine token beside an HTML comment. Reject: PR author with the opt-in off, or set to 1/yes/false; read-only org MEMBER (clears stage 1, refused by stage 2); quote-reply carrying the real head SHA; indented block + inline backtick; ```- and ~~~-fenced tokens; a token inside an HTML comment; a token after an inline ```gh``` span; a token glued to a closing fence; Bot / app-attributed / actor-claiming body; drive-by NONE association; stale head after force-push; withdrawn/edited; 40 metacharacters and 40 non-hex chars as HEAD_SHA; empty HEAD_SHA. Plus every permission value (admin/maintain/write/read/triage/none/empty/bogus), token_approver_login's three failure exit codes, token_blocked_by_author_exclusion's match / non-match / unresolved-author cases, the four denial descriptions asserted pairwise distinct and each under the 140-char cap, and the no-lookup / no-candidate fail-safes. Review-path regression kept.
• Conformance: pr-state-flow.test.ts — the AC-named assertions are scoped to the extracted token predicate text, not the whole file (asserted against the file, /[Bb]ot/ was satisfied by a prose comment and .user.type=="User" by the untouched review predicate, so both stayed green with the bot exclusion deleted). The extraction itself is asserted inside an it(), so a reformatted printf is one named failure, never a collection abort taking the file down. Plus: the strip is asserted to happen before the anchor and to be line-anchored (a bare split("```") and a join("\n") are asserted absent), the per-comment concurrency group asserted present and both shared-group forms absent, and every corrected wording asserted absent on the surfaces that carried it — including the "a two-human repository pays nothing" claim, whose guard now matches the behaviour rather than one draft's phrasing.
• Live-host (T-7, deferred): see Follow-up Work.

Test Results

pnpm quality-gate (ts:check + test + lint + format:check + gate:composition + hygiene
                   + smoke-modes + docs:staleness + skills:conformance + dup): PASS (exit 0)
pnpm --filter @pair/knowledge-hub test:  PASS (4924 tests + link check)
pnpm smoke-tests (full suite):           PASS (exit 0, 0 [FAIL])
pnpm --filter @pair/website build:       PASS (exit 0)
pnpm format:check:                       PASS (exit 0)
bash scripts/smoke-tests/scenarios/pr-state-flow.sh: PASS (188 [PASS], 0 [FAIL])

Workflow recipe re-validated after the concurrency + fourth-state edits:
  yaml.safe_load(<the pair-explicit-approval block>) -> OK
    group: pair-explicit-approval-${{ …number }}-${{ github.event_name == 'issue_comment'
           && github.event.comment.id || 'eval' }}   cancel-in-progress: true
  bash -n <the job's final run: body>                -> OK
  the token branch executed with stubs under `set -euo pipefail`, comments payload =
  the PR author's token only:
    CANDIDATES=[]  FORCED=[pr-author]  DENIED=4
    DESC="risk:red — token posted by the PR author; set PAIR_SOLO_APPROVAL_TOKEN=true
          if this repo has one human (D10)"   (108 chars)
  and the paired states on the same harness, still distinct:
    empty payload         -> DENIED=1, 133 chars, "needs a non-author human approval…"
    read-only org MEMBER  -> DENIED=2, 104 chars, names the login

Mutation check (do the cases detect what they name?):
  restore the previous predicate  -> 10 smoke assertions + 3 conformance RED, printing e.g.
    "hidden-commenter approved head cc1fba122f0c at 2026-08-28T11:21:00Z",
    "tokens accepted ...: expected '5' got '8'",
    "opt-in 'True' declares the repository single-human: expected '1' got '0'",
    "the author-exclusion description is byte-identical to 'no token was posted'".
  restore the previous guide+ADR -> 4 conformance + 3 smoke RED, including the widened
    "pays nothing" guard catching the reworded form the earlier regex missed.

Testing Strategy

• Happy path: on a repo that declared itself single-human, the maintainer posts /approve <head-sha>, is OWNER, human, not via an App, holds admin → the gate counts it and the audit line names the actor, the head and the timestamp.
• Edge cases: author self-approval (blocked unless declared — and told which one it is), read-only org member (stage 1 passes, stage 2 refuses), quote-reply, indented/backticked, fenced, HTML-commented and fence-glued mentions, force-pushed head, withdrawn token, body text claiming another actor.
• Error handling: absent/short/non-hex HEAD_SHA, absent permission lookup, permission API error (its own sentinel and its own published description) — all fail closed, and the closed state says which one it is.

Existing tests modified (flagged for the reviewer)

1. A test asserting a 🔴 merge needs a second human account was retargeted to the review path — still true of that path; what changed is that a solo repo now has a named, weaker alternative.
2. The ref: ${{ github.event.pull_request.base.sha }} assertion (conformance + smoke) now also accepts the || steps.pr.outputs.base fallback the issue_comment trigger forces (that payload has no pull_request object) — plus a new negative assertion that no ref: ever resolves to the head.
3. The AC2/AC5 conformance assertions changed target (whole file → the extracted predicate); (env.HEAD_SHA|length)==40 is now asserted absent.
4. The concurrency assertions moved twice: they now pin the per-comment group and assert both earlier forms (cancel-in-progress: true alone, and cancel-in-progress: ${{ github.event_name != 'issue_comment' }} with a shared group) absent.
5. TOKEN_PREDICATE's extraction guard moved out of module scope into the describe's first it().

Known residual (documented, not hidden)

• Publication race. A token run and a synchronize evaluation can now execute concurrently on the same head, and the later POST wins: an evaluation that read the comments before the token was posted can overwrite the token's success with a failure. Fail-safe direction (blocked, never merged) and self-healing — editing the token comment, or any later push/label/review event, re-evaluates. This is the traded-away half of the per-comment concurrency group: the ordering guarantee, in exchange for a token run that always executes.
• edited/deleted are deliberately unfiltered, so a typo fix on an unrelated comment — on any PR, 🟢 included — runs the job in full and flips the required context to pending until the evaluation completes. A run that dies in between leaves the PR blocked until the next event. Narrowing it would need per-comment state the payload does not carry.
• The strip is targeted, not a CommonMark parser. A fence indented four or more spaces is not recognised as a fence — but its content is indented too, and the anchor forbids leading whitespace, so the token inside it is rejected anyway. The docs say "outside fenced regions and HTML comments", never "outside every code block".
• On an issue_comment event the pending-first step resolves the head via one API read; if that read fails the step aborts and the previous status for the same head stands (a withdrawn token could stay satisfied until the next event). A tier raise arrives as a labeled event carrying the head SHA and never depends on that read.
• Stage 2 costs one extra API read per candidate actor on the token branch — which every 🔴 PR reaches before its first review, not only single-account repositories.
• administration: read is documented as a conditional remedy rather than added to the template: the collaborators endpoint was probed live here with a user token (non-collaborator ⇒ read, admin ⇒ admin, unknown login ⇒ HTTP 404), which says nothing about an Actions GITHUB_TOKEN — and this repo has no pair-explicit-approval.yml to exercise it. Widening an authorization workflow's permissions on an unverified guess is the worse error; the gate now reports the 403 as itself and ordering step 4 sends the adopter to check it.
• Forgery-resistance: see § Why This Change. Dedicated review identity + adoption-gated light-PR auto-approval #218 is a prerequisite only in its App/Bot shape.

Follow-up Work

• T-7 — empirical validation on a throwaway repo with branch protection (P1, ~1.5h). This is the merge-gate judgement, stated as one: every result above is offline. This repository ships no .github/workflows/pair-explicit-approval.yml, so the predicate has been executed against fixtures and the recipe parsed and bash -n-checked, but the authorization path's happy case has never run against GitHub. Two unknowns are answerable only there: (1) whether an Actions GITHUB_TOKEN carrying the four declared permissions: can call collaborators/{login}/permission at all — if it cannot, every token is refused with the lookup-failure description until an adopter adds administration: read; (2) whether GitHub's queued-run semantics behave as the concurrency reasoning above describes. Same precedent as PR state flow (gate≠review) + pair review as required check #234 (recipe shipped as documentation, verified on a throwaway repo separately). Ordering step 4 in github-implementation.md is the adopter-side verification and is written to be run before relying on the token path; the offline PASS in this PR is not live confirmation of AC1.

[rucka]

Verdict

risk:red · cost:green — CHANGES-REQUESTED — the token path is reachable on every repository and by the PR author, so it converts D10's "non-author human approval" into self-approval for all adopters, not just single-account ones.

Open findings: 8 (1 Critical, 3 Major, 4 Minor). One further item is an accepted residual (live-host verification), not blocking.

Classification changed: risk:yellow → risk:red — Security relevance raises from yellow to red (D17, raise-only): the diff introduces a satisfaction path that a PR author can apply to their own 🔴 PR on any repository. This is the "false sense of security" failure mode the story itself named as the one that matters.

PR: #472 · Author: rucka · Reviewer: independent reviewer agent · Date: 2026-08-30 · Story: US-398 · Type: feature

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	green	Criticality Table	unchanged from refinement — packages/knowledge-hub + KB source/mirror pair
Change/diff risk	yellow	diff footprint	unchanged — one module (source + dataset mirror count as one), semantic risk stands
Business impact	yellow	subdomain class	unchanged — Collaborative Workflow (Core)
Security relevance	red	/pair-capability-assess-security	raised (D17): findings 1–3 are introduced authorization weaknesses in the shipped predicate
Coupling balance	green	/pair-capability-assess-coupling	uses existing host surfaces; no new integration

Tier = max(assessed) = red. Review value is a floor: confirmed or raised, never lowered.

Assessments

Security — Input validation

Verdict: yellow — the comment body is parsed with a regex into which HEAD_SHA is interpolated under a length==40 guard only, not a hex-charset guard.

Details

• Inputs touched: .body (applier-controlled, read for command + SHA only — correct), .user.type / .author_association / .performed_via_github_app (host-asserted — correct), env.HEAD_SHA (workflow-controlled).
• The (env.HEAD_SHA|length)==40 fail-safe is genuinely load-bearing and correctly placed; verified empirically that HEAD_SHA='' yields 0 matches.
• Gap: length==40 is not ^[0-9a-f]{40}$, and the value is concatenated into a regex. See finding 5.

Security — Output handling

Verdict: yellow — the audit line is interpolated into a GitHub commit-status description with no length bound. See finding 4.

Details

• DESC="$AUDIT — confirmation, not independent review" is emitted straight into gh api ... -f description=.
• No escaping issue (login/SHA/timestamp are host-asserted and character-restricted); the exposure is length, not injection.

Security — Authentication

Verdict: green — no credential handling is added; identity comes entirely from host-asserted comment fields, never from the payload the applier writes.

Details

• .user.login is never read from the body. Verified with fixture id 3237609009 (a Bot comment whose body embeds a fake "user": {"login": "solo-maintainer"}): rejected.
• persist-credentials: false and the base-pinned checkout are preserved on the new trigger; the negative assertion that no ref: resolves to the head is a good addition.

Security — Authorization

Verdict: red — the token grants 🔴 satisfaction to actors the model does not intend: the PR author (on any repo shape) and read-only org members.

Details

• This is the section the story exists for ("the first card in the backlog making an authorization decision"), and it is where the diff falls short.
• Three distinct actors satisfy pair-explicit-approval who should not: findings 1, 2, 3.
• The Bot / Organization / App-attribution exclusions (AC5) are correct and tested.

Security — Introduced vulnerabilities

Verdict: red — 3 introduced, 0 pre-existing.

Details

Severity	Category	File:location	Introduced / pre-existing	Recommendation
P0	A01 Broken Access Control	.pair/knowledge/assets/pr-state.sh:169 + github-implementation.md:826-848	introduced	gate the token branch on an explicit solo opt-in; exclude PR_AUTHOR unless that opt-in is on
P1	A01 Broken Access Control	.pair/knowledge/assets/pr-state.sh:169	introduced	MEMBER ≠ write access — resolve the actor's repo permission server-side
P1	A01 Broken Access Control	.pair/knowledge/assets/pr-state.sh:169	introduced	anchor /approve to a non-quoted line start so a quote-reply is not an approval

Any introduced red security finding drives CHANGES-REQUESTED.

Cost

Verdict: cost:green — no paid dependency; the only new spend is Actions minutes from the broadened trigger (finding 7).

Details

Signal	Class	Provider	Note
CI job invocations	green	GitHub Actions	issue_comment on every PR comment adds one short job run per comment; bounded, but see finding 7

Architecture (Coupling)

Verdict: green — the "one predicate text, two projections" shape (human_token_approval_select feeding both the count and the audit filter) is the right call; count and audit line cannot drift.

Bug fix — Red test before fix

Verdict: n/a — feature, not a bug fix.

Details

Findings by severity

Critical (must fix before merge)

• .pair/knowledge/assets/pr-state.sh:169 + github-implementation.md:826-848 — The token path has no single-account gate and no author exclusion, so any PR author on any repository can self-satisfy 🔴. The job falls through to the token branch whenever the reviews query returns zero — which is the state of every 🔴 PR before someone reviews it, on a 2-person repo as much as a 1-person one. Nothing in the shell, the predicate or the workflow expresses "this repository cannot produce a second human"; that condition exists only in prose. Concrete failure: a 10-person org, risk:red PR opened by dev A, no approving review submitted yet. Dev A comments /approve <head-sha>. APPROVALS=0 → token branch → dev A is user.type=="User", author_association=="OWNER", performed_via_github_app==null, body names the current head → TOKENS=1 → pair-explicit-approval flips to success and the 🔴 PR merges with zero human review by anyone other than its author. Evidence — probe run against the shipped filter sourced from pr-state.sh, with PR_AUTHOR=pr-author exported: a comment by pr-author (OWNER) on the current head returned its id as a match, and the audit projection printed pr-author approved head cc1fba12… at 2026-08-29T10:00:00Z. Note the review predicate one function above deliberately carries .user.login!=env.PR_AUTHOR; the token predicate carries no equivalent and PR_AUTHOR is exported into that step but never consulted. This directly contradicts AC4's business rule — "the token is a fallback for a configuration that cannot produce a second human, not a shortcut for one that can" — and the fixture/test matrix does not cover it, so the suite reads complete while the case is open. Fix: make the token branch opt-in and author-aware — e.g. reach it only when a repo variable (vars.PAIR_SOLO_APPROVAL_TOKEN == 'true') is set, or when a server-side read shows exactly one human account with push access; where the opt-in is off, require .user.login != env.PR_AUTHOR on the token too. Document the opt-in in github-implementation.md + the ADR amendment, and add both cases (author-with-opt-in-off, author-with-opt-in-on) to github-pr-comments.json and the smoke scenario.

Major (must fix before merge)

• .pair/knowledge/assets/pr-state.sh:169 (and the claim repeated at github-implementation.md:880, pr-states.md:98, ADR-018 § Decision, pr-state-flow.mdx:63) — MEMBER does not mean write access, so on an org repo any org member can satisfy 🔴. All five places justify the association check as "write-level association is the authorization", but GitHub's author_association: MEMBER means "member of the organization that owns the repository" — it says nothing about that person's permission on this repository. Concrete failure: an org with 500 members grants push on this repo to 3 people; member Smoke-test suite (run-all.sh) is not wired into CI — pinned-bug assertions only fire locally #400, who has read-only access here, posts /approve <head-sha> on a 🔴 PR and the gate accepts it. Evidence — probe comment with author_association: "MEMBER", user.type: "User", performed_via_github_app: null on the current head matched the shipped filter and produced the audit line org-member-readonly approved head cc1fba12… at 2026-08-29T10:00:00Z. The fixture tests only OWNER (accept) and NONE (reject), so the one association that is actually ambiguous is untested. Fix: resolve the permission server-side — GET /repos/{owner}/{repo}/collaborators/{login}/permission and require admin|maintain|write — or drop MEMBER from the accepted set; either way correct the "write-level association" wording in all five documents, and add a MEMBER fixture case.

• .pair/knowledge/assets/pr-state.sh:169 — A quoted token counts as an approval, and the quoter is published as the approver. The regex is (^|\s)/approve[ \t]+<sha>(\s|$); \s matches the newline before a markdown blockquote and the space after >, so GitHub's own "Quote reply" output satisfies it. Concrete failure: the maintainer posts the token, then withdraws it by editing it away — the documented withdrawal path. A collaborator had meanwhile quote-replied, producing > /approve <head-sha> followed by "I do NOT approve this, just quoting what you posted." The withdrawal is defeated: the gate still counts 1, the status stays success, and the audit line — the story's property Collaborative Knowledge Base #2 — publishes random-collab approved head <sha> at …, naming as approver a person who explicitly declined in the same comment. Evidence — that exact probe comment (COLLABORATOR, User, no app) matched the shipped filter and produced that audit line. Fixture case 3237609008 gestures at this scenario but deliberately quotes <head-sha> as a literal placeholder rather than a real SHA, so it never exercises the failure. Fix: anchor the command to an unquoted line start — e.g. test("(?m)^[ \\t]*/approve[ \\t]+" + env.HEAD_SHA + "[ \\t]*$") — rejecting >-prefixed, indented-code and inline-backtick occurrences; add a quote-reply-with-real-SHA case to the fixture.

• github-implementation.md:843-845 — The audit description can exceed GitHub's 140-character status description cap, leaving the required check stuck pending — the exact "permanently unmergeable" trap this story exists to remove. DESC="$AUDIT — confirmation, not independent review" where $AUDIT is "\(.user.login) approved head \(env.HEAD_SHA) at \(.created_at)". Fixed cost: 13 (approved head) + 40 (SHA) + 2 (at) + 20 (ISO timestamp) + 4 separators + 38 ( — confirmation, not independent review) = 117 characters plus the login. GitHub logins run to 39 characters, so any login of 24+ characters produces a description over the documented 140-character limit; the gh api .../statuses/$HEAD_SHA POST is then rejected, set -euo pipefail aborts the step, and the pending placeholder written by the first step stands. A valid token on a valid head yields a permanently blocked 🔴 PR, with the cause visible only in the workflow log. Note every pre-existing DESC in this job is short fixed text, so this is the first unbounded one. Fix: bound it — DESC="${DESC:0:140}" before the POST, or shorten the line (short SHA, or drop the fixed suffix which is already stated in the docs).

Minor (must fix before merge — same bar as Major, just lower impact)

• .pair/knowledge/assets/pr-state.sh:169 — HEAD_SHA is concatenated into a regex behind a length check only, not the "40-hex" check the PR describes. The PR description says the binding is a "/approve <40-hex> regex"; the shipped guard is (env.HEAD_SHA|length)==40, which a 40-character string of regex metacharacters satisfies — e.g. 40 . characters would make the predicate match /approve followed by any 40 characters. Not reachable through the documented workflow (HEAD_SHA comes from the payload or headRefOid), but pr-state.sh is a shipped, sourceable asset other consumers call, and the comment above it claims a fail-safe stronger than the code delivers. Fix: (env.HEAD_SHA|test("^[0-9a-f]{40}$")) in place of the length check; the fail-safe comment then matches the code.

• packages/knowledge-hub/src/conformance/pr-state-flow.test.ts:640-650 — The AC5 and AC2 assertions pass on prose alone, so they cannot detect the regression they name. expect(EVALUATOR).toMatch(/[Bb]ot/) is satisfied by the comment block above the predicate ("\"User\" excludes Bot and Organization accounts"), and expect(EVALUATOR).toMatch(/\.user\.type=="User"/) is satisfied by the review predicate on line 121, which is untouched by this story. Concrete failure: delete .user.type=="User" from human_token_approval_select and both "AC5 — bots and app-attributed comments are excluded by construction" and "AC2 — resolves the actor from host-asserted fields" still pass green, while a bot can approve a 🔴 PR. (The behavioural smoke assertion does catch it, which is why this is Minor rather than Major — but the test that carries the AC's name should be the one that fails.) Fix: scope these assertions to the token predicate text, e.g. extract the human_token_approval_select line and assert against that substring.

• github-implementation.md:729-733 — The job now runs on every comment on every pull request, including 🟢 ones that auto-pass. issue_comment: [created, edited, deleted] has no body filter, so an ordinary discussion comment on a 🟢 PR spawns a run that resolves the PR via an API read, checks out the base and re-posts a status. On an active repository this is a steady stream of Actions minutes and a status rewrite per comment; combined with cancel-in-progress: true it also means chatty comment traffic repeatedly cancels in-flight evaluations. Fix: narrow the job if: for that trigger, keeping withdrawal working — gate only created on the body (github.event.action != 'created' || contains(github.event.comment.body, '/approve')), since an edited-away or deleted token must still re-evaluate even though the new body no longer contains the command.

• ADR-018, § "Forgery-resistance is NOT achieved in the default configuration (Dedicated review identity + adoption-gated light-PR auto-approval #218)" — The ADR asserts a property of Dedicated review identity + adoption-gated light-PR auto-approval #218 that only holds for one of its possible shapes. It states that with a dedicated identity "its comments carry .user.type == \"Bot\" (or an App attribution) and are rejected by the very predicate above", and concludes Dedicated review identity + adoption-gated light-PR auto-approval #218 makes the human act "verified rather than assumed". If Dedicated review identity + adoption-gated light-PR auto-approval #218 ships as a machine user account (a second GitHub user holding a PAT — the cheapest and most common way to give an agent its own identity), user.type is "User" and performed_via_github_app is null, so the predicate accepts the agent's /approve and forgery-resistance is not recovered at all. In an ADR whose entire purpose is to claim nothing stronger than the mechanism supports, this is the one sentence that over-claims. Fix: state the condition — forgery-resistance requires Dedicated review identity + adoption-gated light-PR auto-approval #218 to ship the agent as a GitHub App or Bot identity specifically; a machine user account leaves the token exactly as forgeable, and the predicate would need an explicit deny-list of the agent login in that case.

Questions (informational, never blocking)

• DoD — "one empirical run on a real PR with protection applied on a throwaway repo" — Not performed; the PR states this openly and substitutes offline fixture execution plus the live procedure written as ordering step 4 in github-implementation.md. Accepted as a residual: it needs a disposable repo, admin scope and a separate bot identity, none of which exist inside this change. Recording it here so the merge gate sees it, not as a blocker — but note that findings 1–3 are precisely the class of defect an empirical pass would not have caught either, since they need a second actor to reproduce.

Positive feedback

• The one-predicate/two-projections shape (human_token_approval_select → count filter + audit filter) is the right structural call: the number the gate acts on and the line it publishes are provably the same predicate.
• The smoke scenario executes the shipped jq text sourced from pr-state.sh rather than a transliteration, and asserts through the same jq … | grep -c . pipeline the documented job uses. That is the difference between testing the artifact and testing a copy of it, and it is why findings 1–3 were reproducible in minutes.
• The (env.HEAD_SHA|length)==40 fail-safe, and the test that an unset head accepts nothing, anticipate the right failure direction (degrade closed, not open).
• AC6 is met unusually well: the ADR amendment states the forgeability limit in plain words, twice, in a grep-checkable form, and the docs, the shell comments and the website page all repeat "confirmation, not independent review" rather than softening it per surface.
• The negative assertion that no ref: ever resolves to steps.pr.outputs.head is a good instinct — the base-pinned checkout survived a trigger change that could easily have quietly broken it.

Functionality & requirements (AC coverage)

AC	Status	Note
1 — token passes the check on a solo repo	met	executed against the fixture
2 — actor resolved server-side + head-bound	met	host fields only; body never yields an actor
3 — force-push voids the token	met	verified: 0 matches on a different head
4 — review path valid and preferred, token never a replacement	not met	preferred only in query order; with no review yet, the author's own token satisfies 🔴 on any repo (finding 1)
5 — bot/app actors rejected	met	user.type + performed_via_github_app, both tested
6 — ADR records the forgery-resistance limit and the #218 dependency	met, with one over-claim	finding 8
7 — quality-model §4 and D10 unchanged	met	neither file is in the diff
8 — docs point here and state the property honestly	met, with an inaccuracy	"write-level association" is wrong for MEMBER (finding 2)

• Business logic + edge cases correct — except the actor set (findings 1–3)
• Integrates with existing systems
• Error handling appropriate — the status POST can fail on description length (finding 4)

Testing & quality gates

• Coverage: good where it exists — the smoke scenario runs the real predicate over a 9-case fixture. Gaps are the three actor cases that fail: PR author, MEMBER, quote-reply with a real SHA.
• Conformance assertions for AC2/AC5 are grep-over-prose and do not fail when the predicate loses the field they name (finding 6).
• Quality gates: PASS — re-ran scripts/smoke-tests/scenarios/pr-state-flow.sh on the PR head: all assertions green, including the 10 new token assertions. (vitest not runnable in the detached review worktree — no installed deps; the conformance file was read rather than executed.)

Adoption compliance

• Degradation level: 1 — no adoption violation found.
• ADR present and substantive for the new decision (amendment to ADR-018, correctly amending rather than superseding).
• way-of-working.md updated consistently with the amendment.
• Dataset mirror: verified. pr-state.sh is byte-identical to its mirror; the two guideline files differ only by the pre-existing, by-design skill-name de-prefixing transform (/pair-process-review → /review), which is present on origin/main too — the new token content itself is mirrored correctly.
• Conformance-test placement follows the one-file-per-KB-artifact ADL (extends pr-state-flow.test.ts, no new story-named file).

Tech debt

• The "Verified on a throwaway repository" table continues to grow a tail of "added after that session and therefore not in the table" items (now four). The honesty is right; the accumulation means the table covers a steadily smaller share of the mechanism. Surfaced, not blocking.

Documentation

• KB guidelines, ADR, adoption file and website page all updated and mutually consistent.
• One inaccuracy propagated to five surfaces: "write-level association" as the justification for accepting MEMBER (finding 2).

Performance & deployment

• The broadened issue_comment trigger adds a job run per PR comment (finding 7).
• Rollback path is clean: the token branch is additive and reached only when the review path finds nothing, so removing it restores the previous behaviour exactly.
• The documented residual (a failed API read on issue_comment leaves the previous status standing) is correctly scoped and stated in both the guide and the ADR.

[rucka]

Escalation — remediation rounds so far + still-open findings

Three remediation rounds have landed on this PR since the first review:

• Round 1 (f4d26e64) — token predicate opt-in gating (D10), stage-2 write-permission check, single-line command anchoring, audit-line SHA truncation, HEAD_SHA length regex hardening, conformance test scoping, /approve withdrawal re-evaluation, App/Bot identity clarification.
• Round 2 (e9fd3d80) — fence-stripping (```/~~~) before the command anchor, comment-run concurrency (queue instead of drop), stage-2 permission-lookup failure state (`gh_permission`/`token_denied_desc`), "review path always wins" claim corrected, opt-in default rationale recorded.
• Round 3 (cb7426c2) — fourth token-denial state for author-exclusion + opt-in normalization (case/whitespace), comment-run concurrency scoping refined (own comment id, cancel-in-progress: true), HTML-comment + line-anchored fence stripping, retracted cost claim removed from ADR § Decision, "comment costs nothing" claim narrowed to created, IIFE assertion made non-fatal.

A further review pass over cb7426c2 found 6 new issues plus a re-confirmed open decision — escalated, not fixed, this round:

severity	location	summary
Major	pr-state.sh:215,315 + github-implementation.md:918 (job else DESC)	Stage-1 twin of round 3's author-exclusion fix: an author token posted while the opt-in isn't the recognized true form still falls back to the generic "no token was posted" DESC instead of naming the author-exclusion cause.
Major	github-implementation.md:747,976 + adr-018 §§ Amendment/Consequences	cancel-in-progress scoping stops a comment run from cancelling an in-progress evaluation, but GitHub's queued-run semantics can still cancel a pending token run when a newer run queues into the same concurrency group — residual as documented undersells this.
Minor	pr-state.sh:215 fence-strip + docs claim	Backtick-parity heuristic still accepts 3 shapes GitHub renders as code/invisible (HTML-comment variant, inline-span line-break induction, token glued to a malformed/unclosed fence) against an absolute "outside every fenced region" claim.
Minor	adr-018:379 § Consequences	Retracted "pays nothing for a two-human repo" cost claim survives reworded, 5 bullets below its own correction.
Minor	github-implementation.md:972	"An ordinary comment spends no Actions minutes" is broader than the mechanism — only created is filtered; edited/deleted run the job in full.
Minor	pr-state-flow.test.ts:626-630	Module-scope expect(...) inside an IIFE turns a rename/reformat into a whole-file collection error instead of a named test failure.
Questions	PR § Follow-up Work (T-7)	AC1/AC5 verified only offline against fixtures; two live-only unknowns (Actions GITHUB_TOKEN scope for the collaborators-permission endpoint, GitHub's queued-run semantics) remain unexecuted against a live host — this is the open decision: is landing an authorization path whose happy case has never run live acceptable, given the intended adopter is the one who discovers the answer? Merge-gate judgement call for a human, not a defect to fix in code.

Convention: further rework or re-review on this story — including manual, out-of-band rounds — should be appended to the working log at .pair/working/reviews/398.md rather than posted as new standalone PR comments. The next orchestrated run on this story continues the same cycle; its convergence will synthesize ONE final remediation comment and minimize this flush (and any prior synthesis comment from an earlier convergence of this same cycle, if one exists).

Note: the working log is an untracked file that lives only in the persistent authoring worktree ../pair-worktrees/398. That worktree must be preserved until merge — pruning/recreating it loses the audit trail (this comment and the first-review comment remain on the PR regardless, and the first-review comment continues to prevent a duplicate first review on the next run).

Not merging. Awaiting human decision on the Questions item; the 6 Major/Minor findings above are open for the next fix round.