[#218] feat: dedicated review identity + adoption-gated light-PR auto-approval

[rucka]

PR Information

PR Title: [#218] feat: dedicated review identity + adoption-gated light-PR auto-approval
Story/Epic: #218 — parent epic #212 Supervised automation
Type: Feature
Priority: P0 (Must-Have)
Assignee: rucka
Labels: risk:red · pr-state:to-be-reviewed
Head: 6c0d4828 — pair review APPROVED, 0 open Critical/Major/Minor findings (pair-review check success on this head). At risk:red the state stays to-be-reviewed until a non-author human records an APPROVED review on this exact SHA (pr-states.md synthesis table, row green / APPROVED / 🔴 / absent). Nothing here merges automatically.

Summary

What Changed

A project can now provision a dedicated review identity (GitHub App recommended, bot user alternative) that executes every code-host write of the review flow — the native verdict and the pair-review publication — instead of the session user's token. Plus one adoption-gated row that lets that identity sign the approving review a light-tagged, sub-🔴, already-ready-to-merge PR needs.

The shipped PR state flow (#234/#390, ADR-018 Option 3) is extended, never rebuilt: resolve_pr_state's decision table is untouched and the required-check enforcement is untouched.

Three deltas, plus the one predicate change they force:

#	Delta	Where
1	Identity adapter (seven entry points, enumerated in full on all three wiring surfaces — pr-states.md, github-implementation.md and the ADR — because an adapter wired from a list that omits review_identity_exclusion_ok lets a bot user sign the 🔴 approval) — resolve_identity_mode resolves session (default, not a degradation) / identity / halt. Configured-but-broken HALTs with a setup pointer and never falls back to the session user. review_identity_health is where the healthy flag comes from (see Where healthy comes from below). The <identity_kind> vocabulary is the adoption literal the skills forward verbatim — app, bot-user, and user as its short form — and it is owned by review_identity_kind_ok, so the adoption read is two questions (see The adoption read is two questions below).	.pair/knowledge/assets/review-identity.sh (new)
2	Native verdict + Checks API — with an identity, a blocking verdict is a native REQUEST_CHANGES; on the App path pair-review publishes as a check run instead of a commit status. The default session path is unchanged and still submits the native event; --comment there is the self-review case only.	review Steps 5.3–5.4, publish-pr Phase 5
3	Light row — light_auto_approve_allowed reads a tag, an adoption declaration, the tier and the synthesis. Zero classification criteria (D18). Inert on this repo (Active: risk only).	.pair/knowledge/assets/pr-state.sh
4	human_approval_jq_filter gains one clause — see The one shipped predicate that changed below.	.pair/knowledge/assets/pr-state.sh
5b	The identity's write surface is closed, and so is the HALT's scope — the identity performs exactly three attributed writes (the native review, the pair-review publication, the audit comment), but the 403-mid-write HALT binds only two of them: the review and the audit comment. A refused pair-review publication is the documented advisory-continue case (pair-review: NOT PUBLISHED — advisory, the verdict still lives in the native review) — stated identically in both skills' Step 5.3/step 3, their HALT-condition entries, their Graceful Degradation and the host guide, so one event never carries two mandated behaviors. The pr-state:* label is not an identity write at all: it is a board view nobody is attributed for, written by the session token in every mode, which is why the label's pre-existing non-blocking refusal survives instead of colliding with the HALT — and why no issues grant appears on the App. Symmetrically, the halt mode binds only a phase that actually performs an identity host write: PR publication under Review enforcement: disabled (the default) writes nothing as the identity, so it reports the unusable identity and continues — stated in the model (pr-states.md + the ADR), not only in the consumer skill that implements it.	review Steps 5.3–5.4, pr-states.md halt row, ADR-018 amendment
5	pair-review becomes dual-form, so the enablement transition is ruled — check run on app, commit status otherwise, with the form resolved independently at publish time and at review time. Switching Review identity with PRs already open would leave two producers on one required context, so the guide, both skills and the ADR carry the rule: drain open PRs before switching, or supersede the outgoing form on the head — the second exit needing statuses in the mint payload, not only on the App. See One producer per required context below.	github-implementation.md § Dedicated review identity, review Step 5.4, publish-pr Phase 5

Why This Change

Review actions become auditable per-identity rather than attributed to whoever's token happened to be loaded, and the self-authored --comment degradation retires for the blocking direction (the identity is not the PR author, so the host accepts the real event). This is the approval gate #219's supervisor loop consumes.

The one shipped predicate that changed

human_approval_jq_filter — the 🔴 explicit-human-approval predicate — now also excludes by login:

-.user.type=="User" and .user.login!=env.PR_AUTHOR
+.user.type=="User" and .user.login!=env.PR_AUTHOR and .user.login!=env.REVIEW_IDENTITY_LOGIN

Why it is not redundant with the type clause. Only a GitHub App installation types as user.type == "Bot". A bot-user identity is an ordinary machine account and types as "User" on the reviews API — the type clause does not exclude it. Without the login clause, a risk:red PR with green gates and an APPROVED verdict would reach ready-to-merge on a machine approval alone, with no human involvement.

The exclusion is therefore two clauses, one per account kind, and it is checked rather than asserted: review_identity_exclusion_ok <kind> <login> <acting> makes an unprovisioned REVIEW_IDENTITY_LOGIN a not-healthy identity, so resolve_identity_mode yields halt instead of running with the clause inert.

And the check binds the login that is actually SIGNING (this head, 6c0d4828). The gate used to verify only that some login was provisioned; the 🔴 predicate, however, excludes that login and no other, so a variable naming acme-bot while the flow runs under other-bot left the acting account's approval satisfying the explicit human approval — the exact hole the exclusion exists to close, passing as healthy. The third argument is the login the identity's own credential answers with on this run (gh api user --jq .login), and every non-matching shape now fails closed: <acting> absent ⇒ unknown is never excluded ⇒ not healthy; <acting> ≠ <login> ⇒ not healthy, with the mismatch named on stderr. app is unaffected — it is excluded by user.type == "Bot", before any login comparison.

Additive by construction: with the repository variable unset the clause matches nothing, so every pre-existing outcome is unchanged.

And the clause is an ENV input, so every one of its FOUR evaluators has to be told to populate it. The CI job resolves it from ${{ vars.REVIEW_IDENTITY_LOGIN }}; the agent evaluates the same filter in three places — review Step 5.3 step 2 and Step 5.4 step 5, and /pair-process-implement's post-review merge precondition — and nothing exported it there. Measured on the shipped filter, two APPROVED reviews on head abc (id 1 by acme-bot, type: "User"; id 2 by a human), PR_AUTHOR=rucka:

variable UNSET      →  qualifying review ids  1 2      # the bot's own approval counts as the HUMAN one
variable=acme-bot   →  qualifying review ids  2

.user.login != "" is true for every account, so a bot-user identity's approval cast outside this flow (a maintainer holding the bot PAT, #219's supervisor loop) reached ready-to-merge and merge_allowed on a risk:red PR. The CI pair-explicit-approval job catches that only where Review enforcement: enabled and branch protection is applied — not the default, and not this repo. All three agent-side sites now mandate exporting REVIEW_IDENTITY_LOGIN from the repository variable ($RV, which Step 5.3 step 1 already reads back) into the shell that evaluates the filter, and each states that the login clause is inert without it.

The fourth evaluator is the CI job — and the skill that GENERATES it is /pair-capability-setup-gates. Its Step 4.5 enumerated exactly three properties that must survive generation unmodified ("all three are authorization properties rather than style"): trusted ref, head-SHA pinning, pending-first. The REVIEW_IDENTITY_LOGIN: ${{ vars.REVIEW_IDENTITY_LOGIN }} env thread was not among them, so an agent generating the job "from the host guide's template" on a repo declaring Review identity: bot-user + Review enforcement: enabled reads the dropped line as optional config while three neighbours are called out as load-bearing — and the job then evaluates the filter with the variable unset, on the one surface where the 🔴 gate is actually binding. It is now the fourth unmodified respect, naming the repository variable and what the clause degrades to without it.

The adoption read is two questions, and none is not a default

Review identity is read from way-of-working.md as a markdown bullet. none in that key does not mean "unparsed" — it means no identity is configured, which resolves session, i.e. the review is written with the session token. So an extraction that recognises one markdown shape and degrades everything else to none is the session-user fallback AC4 forbids, reached with no HALT because the flow never learns an identity was configured:

adoption line	before	now
- **Review identity**: \bot-user` — …`	bot-user	bot-user
- **Review identity**: app	app	app
**Review identity**: bot-user (no bullet)	none ⇒ session ⇒ review signed by the session account	HALT
- Review identity: app (no bold)	none ⇒ session ⇒ review signed by the session account	HALT
key genuinely absent	none	none
key absent, but prose says "we use no dedicated review identity — reviews run with the session token"	—	none — the presence probe is anchored to a KEY AT THE START OF A LINE, so prose does not make a project look configured
key absent, but prose says "A note on review identity: we deliberately run none, reviews use the session token."	—	none — a colon mid-sentence is not a key: an unanchored phrase-then-colon match would answer PRESENT here and HALT every review on a project that configured nothing
## Review identity: app (an ATX heading)	none ⇒ session ⇒ review signed by the session account	HALT
> - **Review identity**: app (inside a blockquote)	none ⇒ session ⇒ review signed by the session account	HALT

The read is therefore split: presence is detected by key shape at the start of a line, with the accepted DECORATIONS enumerated rather than assumed — blockquote markers (>, repeatable), an ATX heading (#..######), a list bullet (-/*), bold markers, any of them omitted:

grep -qiE '^[[:space:]]*(>[[:space:]]*)*(#{1,6}[[:space:]]*)?[-*]?[[:space:]]*\*{0,2}Review identity\*{0,2}[[:space:]]*:'

It is deliberately not agnostic about POSITION (prose is not configuration), and the enumeration is stated as such because a shape outside the set reads as ABSENT ⇒ none ⇒ session, which is the silent session-token fallback this whole read exists to prevent — the last two table rows above were exactly that hole until this head. The value is extracted from the shipped bullet, and review_identity_kind_ok decides whether it parsed — but only when the key is present. Ordering the two questions the other way round called the validator with the empty string on the DEFAULT path (the key genuinely absent: every project that has not opted in), so it printed its own HALT-flavoured diagnostic — "empty is not a Review identity value … HALT and fix the key, never treat it as none" — on every review and every publish of a correctly configured repository, contradicting the flow's own Identity: session and telling an operator to fix a key the project deliberately does not have. The absent key now resolves none silently; the present-but-unparseable one still speaks, loudly. The vocabulary lives in the adapter, not in a markdown snippet that can drift from review_identity_exclusion_ok / pair_review_publication_mode. The smoke scenario executes the guide's own lines against the real files, the dataset template and six fixtures, the two unparseable shapes included.

Where healthy comes from

resolve_identity_mode's second argument is the single signal separating identity from halt, so it is computed, never assumed: review_identity_health <kind> <auth_ok> <perms_ok> <login> <acting>, fed by probes run on this run, with the exclusion check folded in so no caller can compute health while forgetting it — and <acting>, the login the identity's own credential answers with, folded in with it so the exclusion is checked against the account that will sign rather than against whatever the variable happens to name.

The probes are split by cost, because the two halves cannot both run per review:

when	probes	what they prove
every run (artifact-free)	App: gh api /installation/repositories --paginate --jq '.repositories[].full_name' | grep -Fqx "$REPO" (MEMBERSHIP — an installation token is org-wide, so a 200 is not scope; -F because a repo name routinely carries a ., a regex metachar) + the installation-token exchange with permissions requested explicitly (GitHub answers 422 when the installation lacks a grant) + $PR_AUTHOR vs both app/${APP_SLUG} and ${APP_SLUG}[bot] · bot user: gh api user for the acting login + gh api "repos/$REPO/actions/variables/REVIEW_IDENTITY_LOGIN" --jq .value (the acting login must equal that repository variable, read back on this run) + repos/{repo}/collaborators/{login}/permission + $ACTING vs the PR author	the credential authenticates, it is scoped to this repository, the grants are present — and the identity is not this PR's author
once, at setup (leaves artifacts)	the pair-identity-probe check run (undeletable) + a posted/deleted scratch comment	the WRITE grants a read probe cannot prove

The bot-user health input is the repository VARIABLE, read back on the run — never the agent's environment. The 🔴 clause this check exists to arm is evaluated in the pair-explicit-approval job, which resolves ${{ vars.REVIEW_IDENTITY_LOGIN }}. So the probe reads that value back into $RV, requires [ -n "$RV" ] && [ "$ACTING" = "$RV" ] before AUTH_OK=1, and passes $RV — not an ambient $REVIEW_IDENTITY_LOGIN — to review_identity_health. An operator who exports the login in the shell/CI env but never runs gh variable set (or stores it as a secret, or scopes the variable to an Environment this pull_request_target job does not use) would otherwise get a healthy identity while the gate reads the empty string:

$ HEAD_SHA=abc PR_AUTHOR=rucka REVIEW_IDENTITY_LOGIN="" \
    jq -r "$(human_approval_jq_filter)" <one APPROVED review by acme-bot, type User> | grep -c .
1     # `.user.login != ""` is true for every account — the bot's own approval counts as the HUMAN one
$ ... REVIEW_IDENTITY_LOGIN=acme-bot ...                                                  →  0

The exploitable path is an approval that account casts outside this flow (a maintainer using the bot PAT, #219's supervisor loop, a second automation) — the review flow itself refuses to approve at 🔴. With the read-back the probe answers AUTH_OK=0 in exactly that state ⇒ health 0 ⇒ halt; a variable naming a different account fails for the same reason.

That read-back also adds a grant to the bot-user PAT — Variables: read — and it is the one grant no WRITE needs, so a PAT provisioned for the writes alone authenticates, passes every other probe and is refused only here. Provisioning succeeds too, because gh variable set runs under the maintainer's token. The probe therefore captures the read's exit status instead of swallowing it with || true: a 403 (grant missing) and a 404 (variable never set) each print their own reason before zeroing the flag, so review_identity_health no longer reports "the credential did not authenticate" about a credential that answered 200 one line above, sending the operator to re-issue a PAT that is fine.

Whatever a read probe cannot cover at run time is covered by one rule, on all three surfaces: a 403/422 met mid-write is a HALT, reported against the artifact that failed — never a retry with the session token, and never a pair-review publication implying a review that did not land. With one named exception, so the rule never collides with a sibling one: a refused pair-review publication is pair-review: NOT PUBLISHED — advisory and CONTINUES, exactly as Step 5.4 step 3 and Graceful Degradation have always said — the verdict lives in the native review, and stopping would leave the PR with neither the state label nor the report. Anything that is not exactly 1 — including "probe not run" — is not healthy, so a repository that skipped the per-run check gets halt, not a silent session fallback.

The identity must not be the PR's author — on BOTH forms

The host rejects a review action on your own pull request (422 Can not request changes on your own pull request). An identity that also opens PRs — the shape an unattended loop falls into when one credential runs the whole pipeline — would therefore have every native event refused while the separate publication step still marked pair-review success: an approving verdict as a green required check on a PR carrying no review body.

It is not a bot-user-only rule. A GitHub App authors pull requests too, so the cheapest setup (one App, one credential to store, publishing the PR and reviewing it) hits it exactly as a shared machine account does. The rule is therefore stated once, for both forms, in the shared Dedicated review identity section rather than under Bot user.

And the comparison carries TWO login shapes, because gh renders one Bot actor differently per API. Measured, not inferred:

$ gh pr view 14276 --repo cli/cli --json author -q .author.login     # GraphQL
app/dependabot
$ gh api repos/cli/cli/pulls/14276 --jq .user.login                  # REST
dependabot[bot]

The probe reads the author with gh pr view --json author, so a gate comparing only against ${APP_SLUG}[bot] is never true — inert on exactly the path it guards. Both shapes are compared now (case "$PR_AUTHOR" in "app/${APP_SLUG:-}" | "${APP_SLUG:-}[bot]") PERMS_OK=0 ;; esac), and the two smoke cases feed the two real values. A bot-user identity is a plain User and compares literally on both paths, so that form was never affected.

Two layers, in the order they fire:

layer	App	bot user	outcome
per-run health probe (before any host write)	$PR_AUTHOR vs both app/${APP_SLUG} (GraphQL) and ${APP_SLUG}[bot] (REST); $APP_SLUG is captured at mint time, since GET /app is a JWT endpoint an installation token cannot read — an unknown slug is itself not-healthy	$ACTING vs the PR author	PERMS_OK=0 ⇒ not healthy ⇒ halt with the setup pointer, nothing written
in-flow fail-safe (adapter, no probe or an unreadable author)	identity_verdict_event reads <self_authored> in both modes	same	COMMENT form — the verdict is published in full instead of being lost to a rejected event

An UNREADABLE author is unknown health, not "not the author". Both probes used to compare an empty $PR_AUTHOR against the identity's login, match neither shape and leave the flag at 1 — while the sibling unknown-input arm in the same block ($APP_SLUG unset) correctly zeroes it. The one-credential pipeline plus a transient author read (network, a wrong $PR, a token that cannot read the PR) therefore resolved healthy, ran as identity, and met the host's 422 Can not request changes on your own pull request mid-write: a HALT with the check left pending and no review submitted — the exact outcome the probe promises to prevent before any host write. Both probes now capture the read's outcome and zero PERMS_OK with their own reason (the pull request's author could not be read … unknown authorship is unknown health), and the smoke scenario drives each under a stub whose author read exits 404.

And both probes PIN the read to $REPO. Every other host call in those two snippets pins the repository (gh api /installation/repositories, gh api "repos/$REPO/collaborators/…", gh api "repos/$REPO/actions/variables/…"); the author read did not. Run from a cwd whose origin is a different repository — pair's own orchestrator runs skills from detached worktrees under ../pair-worktrees/<id>, and an agent harness may invoke from a parent directory — gh pr view 466 resolves PR 466 of that repo and returns an unrelated author. The comparison then finds no match, PERMS_OK stays 1, health resolves identity, and the one-credential misconfiguration reaches the native review and dies on 422 Can not request changes on your own pull request mid-write. The failure is not symmetric with a failed read: a read that fails or returns empty is correctly handled (PERMS_OK=0, above); a read that succeeds against the wrong repository silently passed. Both reads are now gh pr view "$PR" --repo "$REPO" --json author -q .author.login, and the smoke stubs model the cwd — an unpinned call answers a foreign login, so every existing authorship fixture asserts the pin.

Each authorship check prints its own reason. It is a third, distinct failure sharing PERMS_OK — the flag that means the required grants were observed — so a silent zero made review_identity_health emit the grant-shaped diagnostic and send the operator of the one-credential pipeline (the misconfiguration the guide calls the likeliest) to re-inspect App grants that are correct, with nothing in the trail naming authorship. Both forms now emit review-identity: the identity (<login>) is this pull request's AUTHOR — not a grant problem… with the § pointer before zeroing the flag, and the unknown-$APP_SLUG arm carries its own line too.

The adapter's defaults stay per-mode and deliberately asymmetric — unknown ⇒ self-authored in session (the acting account routinely is the author), unknown ⇒ not self-authored in identity (setup forbids it, and the other default would collapse every identity verdict to COMMENT and delete the feature). A wrong guess there is loud (a 422), not silent, and Step 5.3's read-back has a defined action: a review the read does not show is Review: NOT SUBMITTED — <host error>, check left pending, the resolved check is not published, and the pending one stays. That is one of the two values the report's Review: row enumerates — and the Check: row likewise carries NOT PUBLISHED — advisory, <host error>, the outcome Step 5.4 and Graceful Degradation both mandate on a refused publication (both CONTINUE, not HALT). The Output Format block is the literal rendering contract, so a row missing its failure value has an agent assert an artifact that was never written.

One producer per required context — the enablement transition

pair-review is now dual-form: a check run on an app identity, a commit status on every other mode. The form is resolved independently at two moments in a PR's life — by publish-pr when the PR is created, and by review Step 5.4 when the verdict lands — so a Review identity change between them splits one required context across two records:

state	head carries	who updates it after the switch
PR #100 opened while Review identity: none	pair-review commit status, pending	nothing — the App path writes check runs
team provisions an App, sets Review identity: app, #100 is reviewed	pair-review check run, success	review Step 5.4

Which record branch protection honours is host-defined; if it honours the stale pending status, #100 is unmergeable with a manual POST /repos/{owner}/{repo}/statuses/{sha} as the only exit. The guide already stated this rule for the sibling context (pair-explicit-approval is a status, deliberately not a same-named check run) and now states it for this one. Two exits, both documented:

• Drain (recommended, and the only one that always works) — merge or close the open PRs before changing Review identity.
• Supersede the outgoing form — overwrite the other form on the head with the same conclusion. Enabling needs statuses granted on the App AND requested in step 4's installation-token permissions payload: the token carries only the subset that payload asks for, so the grant alone leaves the POST /statuses at 403 Resource not accessible by integration and the stale pending status uncleared — the exact outcome the rule exists to prevent. It is added only while taking this exit, because step 4 requests permissions explicitly and GitHub 422s a permission the installation was never granted, i.e. an unconditional request breaks the mint on every run. Retiring needs the old App's token, which a session run no longer has. Both conditions are why drain is the normative exit rather than this one.

The light row is load-bearing, not decorative

light_auto_approve_allowed is the third argument of identity_verdict_event and the sole authority for an APPROVE event the identity signs:

mode	verdict	light row	event submitted
identity	approved	authorized	APPROVE — the only path to a native approve
identity	approved	not authorized	COMMENT, verdict token leading the body
identity	approved	argument absent/malformed	COMMENT (fail-safe closed)
identity	changes-requested	—	REQUEST_CHANGES, ungated — a block never unlocks a merge
identity	any, identity IS the PR author	not reached	COMMENT — the host rejects the native event; reported as a setup finding
session	approved / changes-requested, not self-authored	not read	the native APPROVE / REQUEST_CHANGES — the shipped default, unchanged by this PR
session	any, self-authored or authorship unknown	not read	COMMENT (the host rejects a self-authored verdict; unknown authorship is fail-safe)

The report row has one value per case, five in total — approved, approved — … audit comment NOT POSTED (<host error>) — HALT, the APPROVE stands, not-authorized — <unmet condition>, n-a — no approving verdict (row not consulted) (identity mode where the verdict is not APPROVED, so the row is never evaluated and there is no stderr condition to name) and n-a — no identity (session mode) — so the row is emitted truthfully on every run, blocking reviews included, and #219's loop can key on it.

The audit comment follows the review, by choice, and the choice carries a stated residual. Every input the comment renders (action, tag, declaration, tier, state) is resolved before the review is submitted, so the order is not a dependency. It is posted after because it attributes an action that has happened: posting it first would leave a permanent comment asserting an APPROVE the host may then refuse (422 on a self-authored event, a 403 mid-write) on a PR carrying no review at all. The cost, recorded rather than assumed away: the two writes are both HALT-bound and seconds apart, and the HALT on the second does not roll the first back — a 403 on the audit comment leaves an identity-mode APPROVE on a ready-to-merge PR with no paired audit comment, mergeable with no human action wherever required_approving_review_count >= 1. It is loud, not silent: the run HALTs and reports the fifth Light row: value above — the one documented exception to "every identity review is paired with its audit comment", stated at Step 5.3 step 5, Step 5.4b step 1, in the HALT Conditions entry (which no longer claims an unqualified "before any host write" for a write interrupted mid-flight) and in the ADR-018 amendment.

On a repository setting required_approving_review_count >= 1, a native APPROVE is what satisfies the host's approvals rule. Gating it on the row is what stops every green/yellow PR with an approving verdict from being approved by the identity, light tag or not, declaration or not. The row governs the review cast on the project's behalf; it is not read in session mode, where the account whose token is loaded signs its own review exactly as it did before this PR.

The tag read is whole-FIELD, because label names contain spaces. Hosts ship label names like good first issue; a UI project equally carries ui: light theme. Collapsing every input shape to spaces made that single label indistinguishable from the light tag — a risk:green PR carrying only it reached APPROVE on a repository with required_approving_review_count >= 1, and the audit comment named a Tag: light the PR does not carry. The two delimited shapes are now matched as whole trimmed fields:

labels (declared, 🟢, ready-to-merge)	before	now
light	APPROVE	APPROVE
lightweight	no-op	no-op
risk:green + ui: light theme, one per line	APPROVE	no-op
risk:green,ui: light theme	APPROVE	no-op
risk:green + light, one per line	APPROVE	APPROVE
ui: light theme — the LINE read's output for a PR carrying that ONE label	APPROVE	no-op
risk:green light — the legacy join(" ") shape	APPROVE	no-op
theme, light on its own line (the newline preserved)	no-op	no-op
theme, light — the same ONE label with its trailing newline stripped	APPROVE	APPROVE — documented residual, see below

The match is exact on every shape, because no shape falls through to a substring test. Newline input splits on newlines alone (a label name may legally contain a comma), comma input on commas, and a string carrying neither delimiter is one whole trimmed field — which is precisely what the documented line read emits for a PR carrying a single label. That last arm used to be a space-substring test, so ui: light theme reached APPROVE through the read the asset itself calls exact.

The cost is bounded and deliberate: the legacy space-joined join(" ") shape now fails closed for a multi-label PR (risk:green light ⇒ no-op) instead of being accepted. Once joined, nothing in the string separates the tag from a label containing the word — good first issue, help wanted, ui: light theme — and an irrecoverably ambiguous input must not authorize a merge-enabling event. The skills' documented read is the line form gh pr view <n> --json labels -q '.labels[].name'.

The residual the last table row names. LABELS="$(gh pr view <n> --json labels -q '.labels[].name')" strips the trailing newline, so a PR carrying the ONE label theme, light reaches the helper with no newline, takes the comma branch, is cut into theme + light and matches. Nothing in that string says whether the comma delimits two names or belongs to one — the same irrecoverable ambiguity as the joined shape, arriving through the read the asset calls exact. It is not reachable through this row today: a sub-🔴 tier requires a risk:* label, hence a second field and a newline. So what changed is the claim, not the behavior — the LINE form is documented as exact "whenever the string actually carries a newline (≥2 labels, or the trailing newline preserved)", the residual is stated next to the joined-shape one, and both shapes are pinned as smoke fixtures. Preserve the trailing newline and the exact branch is taken.

Two containments, both mechanical:

• Adoption is the gate, the label is not. A hand-applied light label on a project whose ## Tag Projection declares no light family authorizes nothing.
• It never touches 🔴. The row requires a tier below red, and the identity's approval cannot satisfy pair-explicit-approval either way (App by type, bot user by login).

And one residual, stated rather than assumed away. The first containment holds only on projects that never declared the family. On one that has declared it — and that sets required_approving_review_count >= 1 — nothing in the flow verifies who applied the light label: any collaborator with write or triage access can tag their own sub-🔴 PR and the identity's authorized APPROVE satisfies the host rule with no second person. That is the row working as designed, so the declaration is an authorization decision: light must be access-controlled (applied from classification; manual application restricted and audited). Recorded in the ADR-018 amendment and on all three consumer surfaces (pr-states.md, github-implementation.md, the docs page) so an adopter meets it before opting in. Inert here — Active: risk only.

Story Context

User Story: As a team gating automated merges, I want a dedicated review identity that executes the already-shipped approval/block flow, plus an adoption-gated auto-approval row restricted to light-tagged PRs, so that review actions are auditable per-identity, light PRs become mergeable without a human native-approve, and nothing red merges without explicit human approval (R5.5, R5.6).

Acceptance Criteria coverage:

AC	Covered by	Evidence
AC1 — identity executes the verdict + pair-review; App path uses the Checks API	review-identity.sh (resolve_identity_mode, identity_verdict_event, pair_review_publication_mode), review Steps 5.3–5.4, publish-pr Phase 5	smoke matrix + conformance
AC2 — declared light + tagged + sub-🔴 + merge-enabling ⇒ native APPROVE + audit comment	light_auto_approve_allowed wired as identity_verdict_event's authority; Step 5.4b audit	event: 🟢 light, declared, all green ⇒ APPROVE
AC3 — no approval outside the row; resolve_pr_state unchanged	the event matrix above, asserted on the resolved event	event: 🟡 approved, NO light tag ⇒ COMMENT
AC4 — configured-but-broken HALTs, never a session fallback; untagged ⇒ fail-safe red	resolve_identity_mode, review_identity_health (per-run probes + exclusion), untouched resolve_tier	review_identity_health app 1 0 '' '' → 0 ⇒ resolve_identity_mode 1 0 → halt; review_identity_health bot-user 1 1 acme-bot other-bot → 0 (acting ≠ provisioned)
AC5 — per-identity attribution + audit comment on every identity action	identity_audit_comment with three actions (approve / comment / block); Step 5.3 verify reads the acting account back; the one exception — a refused audit-comment write after the review landed — HALTs and is reported, never silent	conformance + smoke
AC6 — ADR-018 amendment stating what is and is not changed	adr-018-pr-state-flow-required-checks.md amendment 2026-08-28	—

Changes Made

Implementation Details

• T1 — ADR-018 amendment: adopts the deferred Option 4 (identity + host-agnostic adapter + light row); records the one predicate change and that resolve_pr_state's table is untouched.
• T2 — KB identity setup guide: new Dedicated review identity section in github-implementation.md — App (recommended) and bot user, required permissions, secret-store credential handling, the JWT → installation-token exchange (with permissions requested explicitly), the setup-time write probes and the per-run artifact-free health check as two separate steps, the failure-mode table, and the pair-explicit-approval job wiring for REVIEW_IDENTITY_LOGIN.
• T3/T4 — adapter + native path: review-identity.sh (seven entry points, names no host command); review Steps 5.3–5.4 and publish-pr Phase 5 resolve WHO writes before writing anything.
• T5/T6 — light row + audit: sibling helper in pr-state.sh; identity_audit_comment renders the reason from the same inputs so it is reconstructable from the PR alone.
• T7/T8 — fixture matrix + 🔴 regression: the identity × light × tier × verdict matrix runs offline; the 🔴 predicate is pinned against the committed reviews fixture plus a type:"User" machine-account fixture.
• T9/T10 — mirror + docs: dataset mirror in sync (mirror-equality guard green); new docs page concepts/review-identity, cross-linked from concepts/pr-state-flow, registered in meta.json / e2e / CP5.

Files Changed

• Added: .pair/knowledge/assets/review-identity.sh (+ dataset mirror) · packages/knowledge-hub/src/conformance/review-identity.test.ts · scripts/smoke-tests/scenarios/review-identity.sh · apps/website/content/docs/concepts/review-identity.mdx
• Modified: .pair/knowledge/assets/pr-state.sh (+ mirror) · .claude/skills/pair-process-review/SKILL.md + pair-capability-publish-pr/SKILL.md + pair-capability-setup-gates/SKILL.md + pair-process-implement/post-review-merge.md (+ mirrors) · github-implementation.md + pr-states.md (+ mirrors) · adr-018-pr-state-flow-required-checks.md · .pair/adoption/tech/way-of-working.md (+ mirror) · apps/website/content/docs/concepts/{meta.json,pr-state-flow.mdx} · apps/website/e2e/docs.e2e.test.ts · qa/release-validation/CP5-website-docs-completeness.md · scripts/smoke-tests/lib/ci-tests.sh

Database Changes

n/a — no schema, no migration, no data change.

API Changes

Breaking change to a shipped shell contract, contained by fail-safe defaults:

• identity_verdict_event <mode> <verdict> → identity_verdict_event <mode> <verdict> <approve_authorized> <self_authored>. Both new arguments are fail-safe: approve_authorized defaults to 0 (not authorized), so an unmigrated call never yields an unauthorized APPROVE; self_authored is read in both modes, with per-mode defaults — unknown ⇒ self-authored in session (⇒ COMMENT, exactly what a 3-argument call already produced), unknown ⇒ not self-authored in identity (setup forbids a PR-authoring identity; the other default would collapse every identity verdict to COMMENT). Shipped in the same PR as its only callers.
• human_approval_jq_filter reads one new env var, REVIEW_IDENTITY_LOGIN. Unset ⇒ the clause matches nothing ⇒ every prior outcome unchanged.
• New: review_identity_exclusion_ok <identity_kind> <review_identity_login> <acting_login> — <acting_login> is the account the identity's credential answers with on this run; on the machine-user form it must EQUAL <review_identity_login>, and absent or mismatched is fail-safe NOT excluded ⇒ not healthy ⇒ halt (the 🔴 predicate excludes the provisioned login and no other, so a flow signing under a different account would otherwise satisfy the human approval itself). <identity_kind> accepts app, bot-user (the adoption literal) and user (its short form); any other value is fail-safe NOT excluded ⇒ not healthy ⇒ halt. pair_review_publication_mode shares that vocabulary and matches only app positively, so an unknown kind degrades to the commit status.
• New: review_identity_kind_ok <identity_kind> — the vocabulary check the adoption read validates against (app | user | bot-user | none). It is what makes a present-but-unparseable key a HALT instead of a silent none — and the guide's read calls it only when the key is present, so the default (absent) path resolves none without emitting the validator's HALT-flavoured diagnostic.
• New: review_identity_health <identity_kind> <auth_ok> <perms_ok> <review_identity_login> <acting_login> — echoes 1/0, the healthy argument of resolve_identity_mode. Any probe outcome that is not exactly 1 (including absent) is not healthy, and review_identity_exclusion_ok is folded in so health cannot be computed without it. <review_identity_login> must be the value READ BACK from the host's configuration store on this run — the same store the 🔴 predicate's job resolves it from — never the caller's ambient environment variable of the same name.
• light_auto_approve_allowed <pr-labels> … accepts the label list newline- or comma-delimited, and matches whole trimmed fields on both — a string carrying neither delimiter is one whole field, so the match is exact on every shape and the ambiguous space-joined join(" ") input authorizes nothing beyond the degenerate single-label case. The skills' documented read is the line form gh pr view <n> --json labels -q '.labels[].name'. lightweight is still not light on every shape.

Testing

Test Coverage

• Unit / conformance: packages/knowledge-hub — 5034 passed / 5034, 49 files (mirror-guard included). review-identity.test.ts (139 contracts) pins the adapter, HALT-not-fallback, the two-clause exclusion bound to the acting login (absent and mismatched both fail closed), the runtime health source, the gated APPROVE, the per-mode authorship defaults, the unconditional state synthesis (session + identity — halt ends the review), the light-row contracts in the skill + KB text, the repo-pinned authorship reads, the idempotency contract's audit-comment item, the audit action vocabulary, the generator's fourth unmodified respect, and the audit-comment ordering: the order is justified where it is chosen, its residual is stated where the publication and label residuals are, and the fifth Light row: value exists for the case it produces.
• Shell behavior (smoke, per the gate-tooling ADL — shell assets are never vitest-unit-tested): scripts/smoke-tests/scenarios/review-identity.sh — the full identity × light × tier × verdict matrix, the session × authorship matrix, review_identity_exclusion_ok across every kind/login/acting-login combination (matching, mismatched, absent acting, bot-user form), one pr-state:* label per mode × verdict, the 🔴-predicate regression against both fixtures, the DoD grep guards, the label list in all three separator shapes, the review_identity_health matrix (probe outcomes × kind × login), the identity × authorship matrix, the host guide's own adoption read AND its App and bot-user health probes executed — the adoption read against the real way-of-working.md, the dataset template and twelve fixtures; the App's probes 1 and 3 and the whole bot-user probe slab sliced out of the guide and eval-ed under a stubbed gh (repo in/out of the installation, a prefix near-miss, a regex-metacharacter near-miss, the App as PR author in both login shapes, an unknown slug; and for the bot user: variable matching ⇒ healthy, variable unset while the ambient env var matches ⇒ NOT healthy, variable naming another account, PAT that does not authenticate, read-only collaborator, identity-is-author — plus an assertion that each authorship check names AUTHORSHIP on stderr rather than firing silently, and — on both forms — a stubbed author read that FAILS (404) or returns EMPTY, asserted to zero PERMS_OK and to name itself) — the guide's lines are sliced out of the file and run, not asserted as text; the four unparseable shapes (no bold, no bullet, ATX heading, blockquote) are asserted to HALT and the three prose shapes (the third carrying a mid-sentence colon) to resolve none — each of them with EMPTY stderr, which is what pins the default path as silent. The 🔴-predicate block additionally runs the filter with env -u REVIEW_IDENTITY_LOGIN on the machine-account fixture, asserting the clause is inert unexported and that the resulting state is ready-to-merge — the measurement the agent-side export mandate exists to close.
• E2E: apps/website Playwright — 39 passed (the new docs page is registered and reachable).
• Manual: n/a — no live identity exists for this repository (T11, below).

Test Results

pnpm quality-gate                    ✅ PASS (exit 0)
  ├── turbo ts:check test lint       ✅
  ├── workflows:test                 ✅ 320/320
  ├── format:check                   ✅ prettier + markdownlint clean
  ├── gate:composition               ✅
  ├── hygiene:check                  ✅
  ├── smoke-modes:check              ✅
  ├── docs:staleness                 ✅
  ├── skills:conformance             ✅
  └── dup:check                      ✅
pnpm smoke-tests                     ✅ all scenarios passed
pnpm --filter @pair/website e2e      ✅ 39 passed
knowledge-hub vitest                 ✅ 5034/5034, 49 files
review-identity.sh smoke scenario    ✅ 228 assertions, exit 0

Re-run on this head (6c0d4828) as part of the publish/sync step: pnpm quality-gate exit 0, review-identity.test.ts 139/139, scenarios/review-identity.sh 228 PASS / 0 FAIL. Remote CI on this head: build, smoke, secret-scan, preview — all SUCCESS.

Tiering is disabled on this repo, so the full adopted suite ran regardless of the risk:red tag — all three Custom Gate Registry gates.

Testing Strategy

• Happy path: declared light family + light tag + 🟢/🟡 tier + green gates + APPROVED ⇒ native APPROVE + audit comment.

• Edge cases: light tagged but undeclared ⇒ COMMENT · light at 🔴 ⇒ COMMENT (most restrictive wins) · gates red ⇒ COMMENT · untagged tier ⇒ fail-safe red · identity_verdict_event's authority argument absent or malformed ⇒ COMMENT · session + self-authored or unknown authorship ⇒ COMMENT, session + a second reviewer ⇒ the native event · adoption key absent from way-of-working.md ⇒ none, but present and unparseable ⇒ HALT, never none; prose mentioning the phrase without the key ⇒ none, never a HALT — colon-bearing prose included · identity mode where the identity is the PR author ⇒ the per-run probe answers not-healthy ⇒ halt before any write (App and bot user alike), with COMMENT remaining the in-flow fail-safe where no probe ran or the authorship read failed · an App reachable but never installed on this repository (org-wide installation token) ⇒ AUTH probe 0 ⇒ halt, instead of a full review discarded at the first 404 · $REPO=acme/pair.js against an installation listing acme/pairXjs ⇒ AUTH probe 0 (a . is a regex metachar, and grep -qx matched) · the App as PR author in either login shape (app/<slug> from GraphQL, <slug>[bot] from REST) ⇒ author probe 0.

• Error handling: configured-but-broken identity ⇒ halt with the setup pointer, never a session-user fallback · a per-run probe that failed or never ran ⇒ review_identity_health = 0 ⇒ halt · a 403/422 met mid-write on the review or the audit comment ⇒ HALT on the refused write — the writes that already landed are not rolled back, so a refused audit comment leaves the APPROVE in place and reports it as the fifth Light row: value — while the same refusal on the pair-review publication ⇒ advisory, continue · unprovisioned REVIEW_IDENTITY_LOGIN on a bot-user ⇒ not healthy ⇒ halt · a kind the adapter does not know ⇒ fail-safe not-excluded ⇒ halt (both bot-user and user ARE known, asserted on both entry points).

• Regression, executed on this head:

  fixture [{"state":"APPROVED","commit_id":"HEADSHA",
            "user":{"login":"acme-review-bot","type":"User"}}]
  
  HEAD_SHA=HEADSHA PR_AUTHOR=rucka REVIEW_IDENTITY_LOGIN=acme-review-bot \
    jq -r "$(human_approval_jq_filter)" | grep -c .     →  0
  resolve_pr_state pass approved red 0                  →  to-be-reviewed
  
  # the login clause is exact, not over-broad:
  ... REVIEW_IDENTITY_LOGIN=another-bot ...             →  1
  

Quality Assurance

Code Quality Checklist

• Code follows established style guides and conventions
• Functions and classes have appropriate documentation — every helper carries a header stating why, not just what
• Error handling implemented for edge cases — every unresolved input fails closed (COMMENT / halt / fail-safe red)
• Security best practices followed — no credential material anywhere in the diff
• Performance considerations addressed — n/a, no hot path
• No debugging code or console logs left behind

Review Areas

• Business Logic: the event matrix and the two containments above
• Code Structure: one adapter, one row, zero duplication of resolve_pr_state
• Error Handling: fail-safe closed on every unresolved input
• Performance: n/a
• Security: authz over who can approve — the focus area below
• Testing: offline matrix + the 🔴 regression on two fixtures

Deployment Information

Configuration Changes

Only for a project that adopts an identity (nothing here is required to merge this PR):

• Review identity: app | bot-user in tech/way-of-working.md (default none — unchanged).

• bot-user only, MANDATORY: repository variable REVIEW_IDENTITY_LOGIN = the bot's login, threaded into the pair-explicit-approval job (env: REVIEW_IDENTITY_LOGIN: ${{ vars.REVIEW_IDENTITY_LOGIN }}). It must be a repository variable specifically — the per-run probe reads it back with gh api "repos/$REPO/actions/variables/REVIEW_IDENTITY_LOGIN", so an exported env var, a secret, or a variable scoped to an Environment the pull_request_target job does not use all leave the identity not healthy ⇒ the flow HALTs. This is not advisory: it is the only thing keeping a machine account out of the 🔴 human gate.

• App path: a short-lived installation token per run, minted from the private key (never stored), plus APP_SLUG captured in the same step (GET /app needs the JWT; in a workflow it is actions/create-github-app-token's app-slug output) — the per-run author probe cannot run without it, and an unknown slug is not-healthy. The key lives in the secret store, never the repository.

• Turning it on with PRs already open: drain them first (merge/close), or take the guide's supersede step — which additionally requires statuses both granted on the App and added to the mint payload, for that transition only. Otherwise the pair-review context ends up with a record in each form and only one of them is ever updated again.

On an unusable identity at publish time (publish-pr Phase 5 step 3) the outcome depends on Review enforcement, because that is what decides whether this phase writes as the identity at all:

Review enforcement	Phase 5 identity write	Outcome
enabled	the pair-review registration	HALT — the PR stays created and ready-for-review, steps 4–6 do not run, so it carries no pr-state:* label and no dispatch; the HALT entry says so, since an operator told only "the check did not register" would not think to look at the label
disabled (the default)	none — step 4 publishes nothing	continue — the unusable identity is a finding on the Identity: row and the flow still applies pr-state:to-be-reviewed and dispatches the review. Halting here would strand every story of an enforcement-disabled project unlabelled and un-dispatched over a credential this phase never uses; the review re-resolves the same identity at its own Step 5.4, where its writes actually happen, and HALTs there

Rollback Plan

Revert the PR. Nothing here is stateful: no schema, no migration, no host configuration written by the flow. A project that had adopted an identity reverts to Review identity: none ⇒ session mode ⇒ the pre-#218 behavior exactly.

Breaking Changes

One shell-contract signature change, listed under API Changes above. It is contained by a fail-safe default and ships with its only callers, so no consumer migration is required.

Documentation

Documentation Updates

• Technical Documentation: ADR-018 amendment · pr-states.md § Dedicated review identity + § Adoption-gated light auto-approval · github-implementation.md § Dedicated review identity (setup, probes, failure modes, job wiring)
• User Documentation: new docs-site page concepts/review-identity, cross-linked from concepts/pr-state-flow; docs:staleness green
• README: n/a

Knowledge Sharing

• Technical Decisions: recorded in the ADR-018 amendment (2026-08-28)
• Best Practices: "an exclusion asserted in prose is not an exclusion" — review_identity_exclusion_ok turns the claim into a checked precondition of acting at all

Security Considerations

Security Review

• Authentication: identity credentials (App private key / bot PAT) mandated to the secret store; the guide's minting step is explicit that the installation token is short-lived and never stored
• Authorization: the focus area of this PR. Two independent gates: (a) the identity is excluded from pair-explicit-approval by two clauses, one per account kind, with an unprovisioned login treated as not-healthy; (b) a native APPROVE is authorized only by the adoption-gated light row; and the credential that acts is re-verified per run (review_identity_health) rather than trusted from a setup-time check, with a mid-write refusal resolving to HALT
• Data Protection: no user data touched
• Input Validation: every helper fails closed on unknown/absent input
• Dependency Security: no new dependency

Security Testing

• Security Scan: the diff carries no key, PAT or token (scanned for -----BEGIN, ghp_ / ghs_ / github_pat_, password=, secret= — clean). The repository's deterministic secret scan (D24) is the CI backstop.
• Least privilege: the App permission list explicitly excludes administration — the identity must not be able to edit branch protection.

Risk Assessment

Technical Risks

Risk	Impact	Probability	Mitigation
Identity approval leaking into the 🔴 gate	High	Low	Two clauses (type + login); the login precondition is checked against the repository variable read back on the run — the same value the gate job resolves — so an env-only login cannot pass health with the clause inert; unprovisioned, naming another account, or naming an account other than the one actually acting ⇒ not healthy ⇒ HALT; regression on a type:"User" machine-account fixture
Auto-approval abused via mis-tagging	High	Low	Adoption declaration required (label alone inert); the row is the sole authority for the APPROVE event, asserted on the resolved event across 7 matrix cases; the tag is matched as a whole label FIELD on every shape (no substring fallback), so a multi-word label containing the word (ui: light theme) is not the tag and the ambiguous joined shape fails closed. Residual on a repo that HAS declared the family: label application is not itself access-controlled by the flow — documented as an adopter obligation on all three surfaces + the ADR, not left implicit
Credential leakage of the identity	High	Low	Secret-store mandate + short-lived installation token + deterministic secret scan (D24)
Identity provisioned as a principal that also opens PRs (incl. ONE App used as publisher + reviewer)	Medium	Medium	MANDATORY setup rule lifted to cover both forms + a per-run author probe that GATES on each path (halt before any write), comparing both login shapes gh emits + identity_verdict_event degrading to the COMMENT form as the residual; the check is never published without the review
Review identity switched with pull requests already open ⇒ two producers on the pair-review context	Medium	Medium	Enablement-transition rule on all four surfaces (guide, both skills, ADR) + the adopter-facing note on the docs page: drain the open PRs, or supersede the outgoing form on the head — the supersede exit carrying the mint-payload condition that makes it work rather than 403
Live-host behavior differs from the fixtures	Medium	Low	Open — T11 is blocked on a maintainer-provided App (see below); the ADR amendment records explicitly what is and is not observed on a live host

Reviewer Guide

Review Focus Areas

1. The predicate change, and what feeds it (pr-state.sh human_approval_jq_filter + review-identity.sh review_identity_exclusion_ok) — is the 🔴 gate genuinely closed for both account kinds, and is the additive claim (unset var ⇒ no behavior change) true? Specifically: the precondition is checked against the value the gate job resolves (vars.REVIEW_IDENTITY_LOGIN, read back per run), so a login that exists only in the agent's environment cannot make the identity look healthy while the clause is inert. And symmetrically: every place the agent evaluates that filter (review Step 5.3 step 2, Step 5.4 step 5, /pair-process-implement Step 4.1) now mandates exporting the variable into the evaluating shell, because the CI job's vars.* wiring covers only the enforcement-enabled path. And the fourth evaluator's generator (/pair-capability-setup-gates Step 4.5) now names the env thread as an authorization property the generated job must keep, alongside trusted ref / head-SHA pinning / pending-first.
2. The APPROVE authority (identity_verdict_event's third argument) — is there any path to a native APPROVE that does not go through light_auto_approve_allowed?
   2b. The health flag has exactly one writer — is review_identity_health fed by probes that run on this run, and is every artifact-leaving probe confined to setup? An unwritable healthy would make either every review HALT on a correct setup, or every reviewed head carry a probe check run.
3. The host guide's setup path (github-implementation.md § Dedicated review identity) — can a maintainer follow steps 1–7 end to end and land a working, correctly-scoped identity? Every snippet is self-contained: step 5 (setup probes), step 6 (the per-run health check, which assigns AUTH_OK from repository MEMBERSHIP and PERMS_OK from the mint answer plus the author comparison) and step 7 (publication, which assigns IDENTITY_KEY_PRESENT / IDENTITY_KIND / IDENTITY_CONFIGURED and derives IDENTITY_HEALTHY from review_identity_health). The read is executed by the smoke scenario against the real files, so a shape that silently yields none fails the gate rather than the adopter. Step 4's JWT → installation-token exchange uses the form GitHub itself documents (curl -H "Authorization: Bearer $JWT") rather than relying on gh's own auth scheme for an App JWT — no live App exists to verify that here (T11), and a 401 there is indistinguishable from a bad signature. The mint payload is the token's real permission set, not the App's grant list: the baseline requests pull_requests/checks/contents, and the only documented addition (statuses, for the supersede exit) is conditional in both directions — ungranted-but-requested 422s the mint for every run, granted-but-unrequested 403s the write.
4. The kind vocabulary is end-to-end consistent — review Step 5.3 forwards the Review identity: literal, and the adapter accepts it. A kind the adapter rejects is a permanent HALT on a correctly provisioned repository, so the smoke scenario exercises bot-user and user on both review_identity_exclusion_ok and pair_review_publication_mode.
5. The default path is untouched — with no identity configured, does a second maintainer's CHANGES-REQUESTED still submit a real change request, and an APPROVED still count toward the host's approvals rule? session mode reads <self_authored> and degrades to COMMENT only on a self-authored PR (or an unreadable authorship).
6. One pr-state:* label, always — review Step 5.3 step 2 synthesizes the state unconditionally in both modes that continue (session and identity — halt already ended the review) and for every verdict, and Step 5.4 step 5 publishes that one value; nothing about the label depends on the identity or on the verdict being APPROVED.
7. The idempotency skip cannot skip the audit — Step 5.3 submits a fresh native review on every re-invocation, so a re-review on an unchanged head is a new identity action. Step 5.4's "already published" skip therefore covers steps 3–5 only and lands on Step 5.4b: an identity APPROVE without its paired audit comment would be an approval whose reason is not reconstructable from the PR (AC5). The Idempotent Re-invocation section states this consequence rather than contradicting it: N re-invocations on an unchanged head leave N reviews and N audit comments — deliberate, the pairing invariant holding — and §5's "no duplicate comment artifact" is scoped to the report. The residual is stated too: the comments are byte-identical (the projection renders no head SHA, timestamp or review id), so the pairing is 1-to-1 by count and order, not per-pair from the comment text.

Testing the Changes

git fetch origin feature/US-218-dedicated-review-identity
git worktree add ../pair-218 feature/US-218-dedicated-review-identity
cd ../pair-218 && pnpm install

bash scripts/smoke-tests/scenarios/review-identity.sh    # the full offline matrix
pnpm --filter @pair/knowledge-hub test                   # the contract assertions
pnpm quality-gate

Key Test Scenarios

1. The 🔴 gate holds against a machine USER account — the fixture + count_in assertions in review-identity.sh.
2. No APPROVE outside the light row — the event_for matrix, 7 cases, asserted on the resolved event rather than on prose.
3. HALT, never fallback — resolve_identity_mode 1 0 and review_identity_exclusion_ok user '' ''.
4. The adoption literal resolves — review_identity_exclusion_ok bot-user acme-review-bot acme-review-bot ⇒ excluded (a repository provisioned per the guide must not HALT forever).
5. session mode keeps the native verdict — identity_verdict_event session changes-requested 0 0 ⇒ REQUEST_CHANGES; ... 0 1 and the 3-argument form ⇒ COMMENT.
6. The adoption key is actually parsed, and unparseable HALTs — the guide's own read, run against - **Review identity**: \app`⇒app⇒checks-api; against this repo's file ⇒ none; against Review identity: bot-userand- Review identity: app⇒ HALT, notnone`.
7. Health is computed, not assumed — review_identity_health app 1 1 '' '' ⇒ 1 ⇒ identity; ... app 1 0 '' '' (a 403 on the permission probe) ⇒ 0 ⇒ halt; ... bot-user 1 1 '' '' (login unprovisioned) ⇒ 0; no arguments ⇒ 0.
8. A self-authored identity is caught before it writes, in EITHER login shape — the guide's probe 3, eval-ed with PR_AUTHOR=app/acme-review (what gh pr view --json author actually returns) and with acme-review[bot] (the REST shape) ⇒ PERMS_OK=0 ⇒ halt on both. Before this head only the second was compared, so the gate was inert. And it does not lose the verdict where no probe ran: identity_verdict_event identity approved 1 1 ⇒ COMMENT (not a rejected APPROVE), while ... 1 (authorship unknown) ⇒ APPROVE.
   8b. Reachable is not installed, and membership is LITERAL — the guide's probe 1, eval-ed with the installation holding acme/pair while the run is in acme/late ⇒ AUTH_OK=0 (it answered 1 on a .total_count test); and with $REPO=acme/pair.js against a listed acme/pairXjs ⇒ AUTH_OK=0 (it matched, under grep -qx, because . is any-char).
9. Prose is not configuration, colon or no colon — the guide's adoption read against "we use no dedicated review identity — reviews run with the session token" and against "A note on review identity: we deliberately run none, reviews use the session token." ⇒ none on both, not a HALT. The second is what a line anchor buys over a phrase-then-colon match.
10. The transition rule's second exit is actually executable — the supersede snippet's POST /statuses runs with the step-4 installation token, whose permissions are exactly what that mint requested; the guide, the App permission list, the ADR and review Step 5.4 all now state the payload — not the grant — is the limit.
11. The label list is read in every shape, and whole names are whole — light_auto_approve_allowed "$(printf 'risk:green\nlight\n')" 1 green ready-to-merge ⇒ authorized, while "$(printf 'risk:green\nui: light theme\n')", "risk:green,ui: light theme", the bare "ui: light theme" and the joined "risk:green light" ⇒ no-op. A string with no delimiter used to fall through to a space-substring test, so the exact output of the documented line read for a one-label PR authorized an APPROVE. The one residual is pinned both ways: "theme, light" (newline stripped) ⇒ authorized — documented — and $'theme, light\n' ⇒ no-op.
12. A probe's diagnostic names the failure it found — the App's probe 3 and the bot-user author check print …is this pull request's AUTHOR — not a grant problem… before zeroing PERMS_OK, so review_identity_health's grant-shaped message no longer stands alone for an authorship misconfiguration. And an author read that fails or returns empty zeroes PERMS_OK with its own reason on both forms (app_author_perms acme-review __fail__ ⇒ 0; bot_probe acme-bot acme-bot __fail__ ⇒ 1:0) instead of passing as "not the author".
13. The default path is silent — the guide's own adoption read, eval-ed against a file with no Review identity key, answers none and writes nothing to stderr. Before this head it printed review-identity: 'empty' is not a Review identity value … HALT and fix the key on every review of every unconfigured repository.
14. A successful read against the WRONG repository is caught — the smoke stubs model the cwd: an unpinned gh pr view <n> answers unrelated-human, so app_author_perms acme-review 'app/acme-review' ⇒ 1 (healthy — the defect) and bot_probe acme-bot acme-bot acme-bot ⇒ 1:1 before the pin, 0 and 1:0 after it. Seven fixtures flipped red→green on this head.
15. An unresolved verdict is a modelled audit action — identity_verdict_event identity <unknown> … ⇒ COMMENT, so a COMMENT-form review by the identity lands and is audited; action = comment now covers it explicitly, matching Step 5.4b step 2's fourth value instead of leaving the agent to invent a token.
16. The agent exports what the clause reads — env -u REVIEW_IDENTITY_LOGIN … jq -r "$(human_approval_jq_filter)" on a single machine-account APPROVED review counts 1, and resolve_pr_state pass approved red 1 ⇒ ready-to-merge. That is the state the three agent-side sites now forbid by mandating the export.
17. The exclusion binds the ACTING account, not merely a provisioned one (this head, 6c0d4828) — review_identity_exclusion_ok bot-user acme-bot other-bot ⇒ NOT excluded ⇒ health 0 ⇒ halt, and ... bot-user acme-bot '' (acting unknown) ⇒ the same, each naming its own reason on stderr. Against the previous two-argument contract those cases resolved healthy while the 🔴 predicate excluded a login nobody was signing under — 4 conformance cases were red before the fix, 139/139 after, with the smoke scenario covering matching / mismatched / absent / bot-user-form acting logins.

Dependencies & Related Work

Blocking Dependencies

None. Prerequisites #215, #233, #234 (merged as #390) and #240 are all closed.

Related PRs

• Follows: [US-234] feat: PR state flow (gate≠review) + pair review as required check #390 (PR state flow — the architecture this extends)
• Blocks: Generalized implement-batch shipped in the dataset: N mutex-safe cards to review-approved PRs, never merges #219 (supervisor loop — consumes this approval gate)
• Related: Solo-maintainer explicit-approval token for 🔴 PRs (verified human token instead of a second account) #398 (solo-maintainer approval token — one-way: it needs this identity, this story needs nothing from it)

Follow-up Work

• T11 — end-to-end verification on a real identity: still BLOCKED, and deliberately not filed as a separate card. No App or bot user exists for this repository, so the live-host run cannot be performed here. Everything T11 would observe is asserted offline against fixtures; the ADR-018 amendment records explicitly what is and is not observed on a live host. This is a known, stated gap for the merge gate — not a silent one.
• This repo keeps Review enforcement: disabled and branch protection unapplied; the identity work is independent of that human step.

Pre-Submission Checklist

• All acceptance criteria implemented and tested (AC1–AC6 against fixtures; live-host observation is the blocked T11)
• Code follows team standards and style guides
• All tests passing (unit, conformance, smoke, E2E)
• Documentation updated (ADR, KB guides, docs site)
• Security considerations reviewed
• Breaking changes documented
• No merge conflicts with main

[rucka]

Verdict

risk:red · cost:green — CHANGES-REQUESTED — the shipped bot-user identity mode satisfies the 🔴 human_approval_jq_filter predicate that five artifacts in this diff claim it cannot, so a risk:red PR reaches ready-to-merge with zero human involvement.

Open findings: 8.

PR: #466 · Author: rucka · Reviewer: independent review agent · Date: 2026-08-28 · Story: US-218 · Type: feature

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	green	Criticality Table (tech/risk-matrix.md)	Confirmed — touched deployables all Low
Change/diff risk	yellow	diff footprint	Confirmed — 23 files, additive; resolve_pr_state byte-unchanged (verified)
Business impact	red	subdomain class	Confirmed — Collaborative Workflow / merge gating
Security relevance	red	authz path	Confirmed at review, and materialized: finding C1 is an authorization bypass on the 🔴 gate
Coupling balance	not assessed	no volatility rating for Collaborative Workflow	excluded from max (§3.1, D21)

Tier = max(assessed) = risk:red, unchanged from refinement. Cost = green — no paid SDK, metered call or IaC in the diff.

Assessments

Security — Input validation

Verdict: green — inputs are tags/flags/enums; light_auto_approve_allowed whole-word matches with case " ${labels//,/ } ", lightweight correctly rejected (executed).

Security — Output handling

Verdict: green — identity_audit_comment is a printf projection of its five arguments; no interpolation of untrusted host content.

Security — Authentication

Verdict: yellow — the credential model is sound (secret store, least-privilege, never in repo; secret scan of the diff is clean), but the App path is not followable as written: the health probe is wrong (M1) and the installation-token minting step is missing (m2).

Security — Authorization

Verdict: red — C1. The bot-user mode this PR ships as a supported, documented identity form satisfies pair-explicit-approval. Executed, see finding.

Security — Introduced vulnerabilities

Verdict: red — 1 introduced (C1), 0 pre-existing. The vulnerability is introduced by offering Review identity: bot-user while the 🔴 predicate has no exclusion for it.

Cost

Verdict: cost:green — no metered surface in the diff.

Architecture (Coupling)

Verdict: green — the adapter is a leaf asset; light_auto_approve_allowed is a genuine sibling (resolve_pr_state's body is byte-identical to origin/main, verified).

Bug fix — Red test before fix

Verdict: not applicable — not a bug fix.

Acceptance criteria

AC	Met	Note
AC1 native verdict + Checks API	partial	Adapter + skill wiring correct; the App health probe documented to produce its input returns 403 for the recommended token type (M1)
AC2 light auto-approve	yes (mechanically)	Helper + wiring correct — but its gate is not load-bearing, see M2
AC3 no other auto-approval path	no	Step 5.3 submits APPROVE for every APPROVED verdict in identity mode (M2)
AC4 HALT, never session fallback	yes	7 adapter cases executed, incl. unknown/malformed health ⇒ halt
AC5 per-identity attribution + audit	partial	Read-back is mandated; the Step 5.3 approval on a non-light PR is not audited (M2)
AC6 ADR-018 amendment	yes	Recorded as an amendment, supersession struck through not deleted; repeats the C1 claim and misnames the scenario file (m4)

Details

Findings by severity

Critical (must fix before merge)

• .pair/adoption/tech/way-of-working.md:66 + .pair/knowledge/assets/pr-state.sh:181 — A bot-user review identity satisfies the 🔴 explicit-human-approval gate. A GitHub machine account is an ordinary user: the reviews API returns user.type == "User" for it (only GitHub App identities type as "Bot"). human_approval_jq_filter selects on exactly that field and has no identity exclusion. Concrete failure: repo sets Review identity: bot-user; a risk:red PR has green gates and an APPROVED verdict; review Step 5.3 resolves identity mode → identity_verdict_event identity approved → APPROVE → the bot account submits a native approving review on the head; pair-explicit-approval counts it (state APPROVED, commit_id == HEAD_SHA, type == "User", login != author) → passes → resolve_pr_state pass approved red 1 → ready-to-merge. A red PR is fully mergeable with no human ever looking at it — the exact invariant Assumption 4, the Business Rules and the ADR amendment say is untouched. Evidence (executed on this branch):

  $ source .pair/knowledge/assets/pr-state.sh
  $ cat bu.json
  [{"id":1,"state":"APPROVED","commit_id":"HEADSHA","user":{"login":"acme-review-bot","type":"User"}}]
  $ HEAD_SHA=HEADSHA PR_AUTHOR=rucka jq -r "$(human_approval_jq_filter)" bu.json | grep -c .
  1
  $ resolve_pr_state pass approved red 1
  ready-to-merge
  

  The only containment shipped is prose in github-implementation.md:741 ("keep it out of the repository's human-reviewer set") — advisory, unenforceable, and contradicted by four artifacts that assert the exclusion holds "by construction": .pair/knowledge/assets/review-identity.sh:34, pr-states.md:101, adr-018-…md:199, apps/website/content/docs/concepts/review-identity.mdx:34, plus the audit comment pair-state.sh… review-identity.sh:139 posts publicly on every identity action. The smoke scenario's "🔴 predicate intact" regression is vacuous for this mode: it asserts only that reviews with type != "User" exist in the fixture (review-identity.sh scenario, BOT_TYPES check), so a type:"User" machine approval is never exercised — the test was written from the same false premise and cannot catch this.
  Recommendation: make the exclusion mechanical, not prose. Either (a) drop bot-user from the supported values of Review identity and from github-implementation.md § Bot user, leaving app (which genuinely types as "Bot") as the only identity form; or (b) extend human_approval_jq_filter with and .user.login != env.REVIEW_IDENTITY_LOGIN and thread that env var through the pair-explicit-approval job. Then correct the four "by construction" statements + the audit-comment string to match whichever holds, and add a smoke case with a type:"User" machine approval asserting a count of 0.

Major (must fix before merge)

• .claude/skills/pair-process-review/SKILL.md:316 (+ mirror) — The light row's adoption gate is not load-bearing, so AC3 is not met. Step 5.3 in identity mode maps any APPROVED verdict to event = APPROVE and submits it. On a repo with required_approving_review_count: 1, that review already satisfies the host approvals rule — for every green/yellow PR, tagged light or not, declared or not. Concrete: repo declares no light projection, PR labelled risk:yellow only, gates green, verdict APPROVED → Step 5.3 submits the identity's APPROVE → PR is mergeable with no human action, while Step 5.4b reports Light row: n-a. The story's containment ("adoption is the gate, the label is not") therefore contains nothing, and github-implementation.md:757's claim that the light row's review "is what satisfies the host's approvals rule" is inaccurate — Step 5.3's already did. Two further consequences: Step 5.4b step 4 would submit a second APPROVE on a light PR (duplicate review), and AC5's "every identity approval carries an audit comment" is unmet for the Step 5.3 approval on a non-light PR (Step 5.4b step 4 says "on no-op, do nothing"). The DoD's "grep-verifiable: no auto-approval path outside the light row" passes only lexically — the guard greps for the string auto-approv (scripts/smoke-tests/scenarios/review-identity.sh:291), which an unconditional APPROVE never contains.
  Recommendation: pick one and make the docs match. Either gate Step 5.3's APPROVE on the light row (identity submits APPROVE only when light_auto_approve_allowed passes, COMMENT otherwise — makes AC2/AC3 coherent and the audit universal), or keep the unconditional native verdict and state plainly in pr-states.md, github-implementation.md and the docs page that in identity mode any approving verdict satisfies required-approvals, so light gates nothing additional — and drop the containment framing.

• .pair/knowledge/guidelines/collaboration/project-management-tool/github-implementation.md:706 — The documented health probe for the recommended (App) path fails on a correctly-provisioned App. gh api user requires a user-to-server or PAT credential; with the App installation token the same code block declares GH_TOKEN to be (line 704), GET /user answers 403 Resource not accessible by integration — it is not associated with a user. Concrete: maintainer provisions the App exactly per steps 1–3, runs step 4, sees a 403 on the first probe, and the failure-mode table (line ~749, "Configured, a permission missing (403 on a probe) ⇒ halt") tells them this is a broken identity → resolve_identity_mode 1 0 → HALT on every review, on a setup that is in fact correct. The comment "expect the app slug and 'Bot'" is also wrong for that token.
  Recommendation: replace the probe with one an installation token can serve — e.g. gh api /installation/repositories --jq '.total_count' (200 ⇒ token valid and installed) — and drop the .login/.type expectation, or state explicitly that gh api user applies only to the bot-user path.

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

• github-implementation.md:707-709 — the probes never verify pull_requests: **write**; the third one (…/pulls/$PR/reviews, labelled "200 ⇒ pull_requests read") passes on a read-only grant. Concrete: App granted pull_requests: read + checks: write → all probes pass → healthy=1 → identity mode → pair-review check published → Step 5.3's review submission 403s after the write, mid-flow, which is precisely the case the HALT exists to catch. Recommendation: add a write-touching probe (e.g. post + delete a scratch PR comment) or state that write cannot be probed non-destructively and that the grant must be verified in the App settings UI.
• github-implementation.md:702-711 — the App path never documents how to obtain the installation token it depends on. Step 3 stores the private key; step 4 opens with "GH_TOKEN here is the App INSTALLATION token, minted from the private key" and no minting step exists anywhere in the file (no JWT, no POST /app/installations/{id}/access_tokens — grepped). A maintainer following the guide end-to-end cannot complete the recommended path. Recommendation: add the JWT → installation-token exchange (or name the tool that performs it) as step 3b.
• github-implementation.md:705-708 — the probe block references $HEAD_SHA and $PR, neither of which is set in the snippet (only $REPO is); and it POSTs a real check run named pair-identity-probe onto a real commit as a side effect of a verification step. Recommendation: set both variables in the snippet and note that the probe leaves an artifact (or target a scratch commit).
• .pair/adoption/tech/adr/adr-018-pr-state-flow-required-checks.md:220 — the amendment's Verification-status paragraph says the identity × light × tier × verdict matrix "runs offline in scripts/smoke-tests/scenarios/pr-state-flow.sh". It runs in scripts/smoke-tests/scenarios/review-identity.sh; pr-state-flow.sh contains none of it. A reader auditing what was actually verified follows the pointer and finds nothing. (Line 283's reference to the same file is pre-existing and correct — only the new line 220 is wrong.) Recommendation: one-word filename fix.
• .claude/skills/pair-process-review/SKILL.md:276 vs :320 (+ mirror) — Step 5.3's preamble still asserts "the verdict is the review action; there is no separate PR comment (decision Q5)", while new Step 5.4b makes an audit comment mandatory on every identity approval and block. As written the two rules contradict inside one phase; an implementer honouring Q5 literally will skip the audit AC5 requires. Recommendation: qualify the Q5 sentence — no separate verdict comment; the identity audit comment is a distinct, required artifact.

Questions (informational, never blocking)

• scripts/smoke-tests/scenarios/review-identity.sh:291 — the "no auto-approval outside the light row" guard is a keyword grep over skill prose (grep -in 'auto-approv' | grep -vi light). It passes on any wording that avoids the hyphenated token, so it cannot regress-protect the behavior it names even after M2 is fixed. Worth stating as a documentation guard rather than a behavioral one.
• DoD — "Security review of identity credentials handling (no secrets in code)" — closed by this review for the credential-material half: the diff carries no key, PAT or token (scanned for -----BEGIN, ghp_/ghs_/github_pat_, password=, secret= — clean), and the guide mandates secret-store storage with the deterministic scan as backstop. The authorization half is finding C1 and is not closed.
• T11 (live-host verification) — genuinely blocked on maintainer-provisioned infrastructure, honestly disclosed in the PR body and in the ADR. Not held against this PR; note that M1 means the App setup path has not been executed by anyone, which is how the probe error survived.

What was verified, and how

• scripts/smoke-tests/scenarios/review-identity.sh — executed, PASS (rc=0); registered in scripts/smoke-tests/lib/ci-tests.sh:46.
• packages/knowledge-hub/src/conformance/review-identity.test.ts — not executed here (no node_modules in the throwaway review worktree); read in full.
• resolve_pr_state body compared against origin/main — byte-identical; the light row is genuinely a sibling.
• Dataset mirrors vs. root: new content identical modulo the repo's existing skill-name prefixing convention (/review vs /pair-process-review) — verified per file.
• CP5 page count 84→85 and Concepts 12→13 consistent with meta.json + docs.e2e.test.ts registration.

[rucka]

Verdict

risk:red · cost:green — APPROVED — il gate lega l’identità esclusa a quella che firma davvero e tutte le varianti di mismatch ora falliscono chiuse.

Open findings: 0. Ogni finding è risolto; nessun Major/Minor è aperto su questa PR.

PR: #466 · Author: rucka · Reviewer: rucka (self-review) · Date: 2026-08-30 · Story: US-218 · Type: bug fix

Classification matrix — per dimension

Dimension	Tier	Note
Service/domain criticality	red	Protegge l'identità che può emettere review e cambiare lo stato di merge.
Change/diff risk	red	Adapter shell e contratti host/distribuiti toccano il controllo di identità.
Business impact	red	Un'identità agendo al posto di quella esclusa invalida la separazione reviewer/author.
Security relevance	red	Il check difende una boundary di autorizzazione/review.
Coupling balance	yellow	Adapter, guide, skill e smoke restano sincronizzati dal dataset/generator.

Tier = max dimensione valutata. Cost green: nessun servizio o consumo runtime introdotto.

Assessments

Security — Authentication / authorization

Verdict: green — il controllo ora richiede l'identità effettivamente agente e fallisce su assenza o mismatch; nessun token o comando privilegiato è aggiunto.

Security — Introduced vulnerabilities

Verdict: green — 0 introdotte; il diff restringe il gate, non amplia autorizzazioni.

Cost

Verdict: cost:green — shell locale e documentazione, senza nuovo provider o costo variabile.

Architecture (Coupling)

Verdict: green — il contratto è unico nel dataset e le copie generate/smoke sono verificate insieme.

Bug fix — Red test before fix

Verdict: green — quattro casi di contratto erano rossi sul precedente argomento a due identità; il contratto ora richiede ACTING e i casi sono verdi.

Details

Finding closure and evidence

Severity	Finding	Closure
Major	L'esclusione verificava il login configurato, non chi stava realmente firmando.	review_identity_exclusion_ok riceve ACTING; assenza e mismatch HALT.
Minor	Health/guide/test/smoke usavano il contratto a due argomenti.	Tutti usano la stessa firma e la guida pinna il repo.
Minor	Copie generate potevano divergere dall'adapter canonico.	Dataset rigenerato e smoke copre login uguale, diverso, mancante e forma bot-user.

• Red-first: test con vecchio contratto — 4 failure previste.
• Regressione: pnpm --filter @pair/knowledge-hub exec vitest run src/conformance/review-identity.test.ts — 139 passed.
• Smoke: bash scripts/smoke-tests/scenarios/review-identity.sh — PASS.
• Gate completo: pnpm quality-gate — PASS.
• CI remoto: build, preview, secret-scan e smoke — SUCCESS; merge state CLEAN.

Required human action

La review self-authored viene pubblicata come commento e non soddisfa l'approvazione esplicita. Per risk:red serve una review APPROVED di un umano non-autore sullo SHA 6c0d482822554a50f8b0e22ea19da40987edf4f9; fino ad allora la PR resta pr-state:to-be-reviewed. Nessun auto-merge.

[rucka]

In response to the first review and the round-7 escalation (now minimized, per the convention that flush stated).

Converged — review↔fix loop closed at round 17 (head 6c0d4828)

Verdict: risk:red · cost:green — APPROVED. pair-review on this head reads success — APPROVED — 0 open Major/Minor findings. Open Critical/Major/Minor: 0, with the one documented exception at the bottom of this comment. Nothing was merged and nothing merges automatically — see Required human action.

Round-by-round history (finding → change → commit, plus the red-before/green-after evidence for every round) is the story's own log, .pair/working/reviews/218.md, kept in the persistent authoring worktree ../pair-worktrees/218 — untracked, so that worktree must be preserved until merge.

17 rounds — what each closed

round	commit	findings	closed
1	11c29cb8	1 Critical, 2 Major, 5 Minor	human_approval_jq_filter gained the REVIEW_IDENTITY_LOGIN clause + review_identity_exclusion_ok; identity_verdict_event's APPROVE gated by light_auto_approve_allowed; App-safe installation-repositories probe
2	045fea0f	1 Major, 4 Minor	bot-user case arm added to the exclusion (was excluded by omission); light label access-control obligation recorded
3	ba50238a	3 Major, 2 Minor	identity_verdict_event gained self_authored; resolve_pr_state hoisted to an unconditional step; the adoption read parses the shipped bullet, executed by smoke
4	c077cbdc	2 Major, 3 Minor	review_identity_kind_ok (present-but-unparseable ⇒ HALT, never none); the 5.4 skip scoped to steps 3–5 so 5.4b always runs
5	b090f033	4 Minor	4th Light row: value; ADR entry-point enumeration; the exclusion stated as two clauses
6	2361fa52	1 Major, 2 Minor	review_identity_health introduced — per-run artifact-free probes split from setup-time artifact-leaving ones; mid-write 403/422 HALT
7	—	escalated	3 Major + 2 Minor flagged for a human structural call (closure table below)
8	0dd55506	3 Minor	Review: row enumerates both outcomes; presence probe line-anchored (prose is not configuration)
9	2bb60c84	3 Minor	AUTH probe tests repository membership, not reachability; App author probe gates (a health failure, not a verdict degradation)
10	c7b9795e	3 Major	the author comparison carries both login shapes gh emits for one Bot actor (app/<slug> GraphQL, <slug>[bot] REST) — a single-shape gate was inert
11	15d4cfbe	2 Minor	the supersede exit needs statuses granted and in the mint payload; halt under Review enforcement: disabled reports and continues
12	a3a11882	—	the bot-user health input is the repository variable read back on the run, never the ambient env; membership tests made literal (grep -Fqx); authorship checks print their own reason
13	ed1f9969	—	the light match is exact on every shape (the ambiguous space-joined form fails closed); the halt scope bound to phases that perform an identity write; the presence class enumerates its decorations
14	5b201489	—	every agent-side evaluator must export REVIEW_IDENTITY_LOGIN; an unreadable PR author is unknown health; the adoption-kind read ordered on presence
15	d0161a6e	—	the audit comment stays after the review, its residual stated on four surfaces and reported as a fifth Light row: value
16	0c421402	—	human_approval_jq_filter has four evaluators — the generator (setup-gates) names the env thread as a fourth unmodified respect; both authorship probes pin the read to $REPO
17	6c0d4828	1 Major, 2 Minor	the exclusion binds the acting identity, not merely a provisioned login

Round-7 escalation — closure, one line per finding

#	sev	escalated finding	closed by
1	Major	the login clause was wired into exactly one evaluation environment (the CI job); three agent-side sites evaluated the same filter with nothing populating it	rounds 14 + 16 — all agent-side sites mandate exporting the variable, and the fourth evaluator (the CI job) is covered at its generator: setup-gates now names REVIEW_IDENTITY_LOGIN: ${{ vars.* }} as an authorization property that must survive generation unmodified, alongside trusted ref / head-SHA pinning / pending-first
2	Major	the health snippets read the login from the ambient shell with nothing populating it ⇒ a correctly provisioned repo HALTs on every review	round 12 — the health input is gh api "repos/$REPO/actions/variables/REVIEW_IDENTITY_LOGIN" --jq .value, read back on the run, and the read's exit status is captured so a 403 (grant missing) and a 404 (never set) each name themselves
3	Major	"an App installation is never a PR author" was false, and App probe 3 assigned $PR_AUTHOR without comparing it	rounds 9 + 10 — the parenthetical is deleted, the MANDATORY no-PR-authoring rule is lifted to the shared section binding both forms, and probe 3 gates on both login shapes; unknown slug is itself not-healthy
4	Minor	the case "identity is the PR author + light authorized" fitted none of the four Light row: values	round 15 — a fifth value exists and the case picks it; the report follows the event actually submitted, and round 13/15 pinned the residual it names
5	Minor	pr-states.md's session row reads "unchanged and unreported", against both skills' mandatory Identity: session output row	NOT changed — see below

Round 17 — the last Major, and why it was one

The exclusion gate verified that some login was provisioned; the 🔴 predicate, however, excludes that login and no other. A repository whose variable named acme-bot while the flow actually ran under other-bot therefore resolved healthy, and other-bot's approving review satisfied the explicit human approval — the exact hole the check exists to close, passing as green.

review_identity_exclusion_ok <kind> <login> <acting> now binds the account that will sign. Every non-matching shape fails closed: acting absent ⇒ unknown is never excluded ⇒ not healthy; acting ≠ provisioned ⇒ not healthy, with the mismatch named on stderr. app is untouched — it is excluded by user.type == "Bot" before any login comparison. Test-first: the old two-argument contract produced 4 expected failing conformance cases; after the smallest adapter change, 139/139.

Not changed — one Minor, deliberately left open

[Minor] pr-states.md § actor table, session row — "Today's behavior, unchanged and unreported". Escalation finding 5, filed at round 7 and never picked up by a later round. Both consumer skills mandate an unconditional Identity: session output row and a Light row: n-a — no identity (session mode) line, so "unreported" is at best ambiguous about which surface it describes (the host records nothing; the flow does report). It is a one-phrase documentation wording fix in the model doc with no behavioral consequence — no adapter, no skill step and no test reads that phrase — which is why it did not fail the gate and did not fail conformance.

This publish/sync step is explicitly scoped to no code changes, so it is reported rather than fixed: either fold the phrase into the next commit on this branch — "unchanged — reported as Identity: session, never as a degradation", the wording the docs page already uses — or accept it and let it close with the PR. This is the one finding that is open, and it is named rather than dropped.

Verification on this head (6c0d4828), re-run during this sync

gate	result
pnpm quality-gate	PASS (exit 0) — all three Custom Gate Registry gates; tiering disabled ⇒ full suite, CI parity
review-identity.test.ts	139 / 139
scenarios/review-identity.sh	228 PASS / 0 FAIL, exit 0
knowledge-hub vitest	5034 / 5034, 49 files
remote CI	build · smoke · secret-scan · preview — SUCCESS
pair-review	success — APPROVED — 0 open Major/Minor findings

State synthesized with the shipped adapter against this PR's real reviews, not asserted:

resolve_pr_state pass approved red 0   →  to-be-reviewed
merge_allowed   pass approved red 0    →  NO
  pr-state: tier 'red' requires explicit human approval — staying to-be-reviewed

Required human action

pr-state:to-be-reviewed is the correct synthesized state, not a stale label: at 🔴 with green gates and an APPROVED verdict, the table's green / APPROVED / 🔴 / absent row is to-be-reviewed until an explicit human approval exists on the current head. The pair review here is self-authored, published as a comment-form review, and by construction cannot supply it.

To merge, a non-author human must record an APPROVED review on SHA 6c0d482822554a50f8b0e22ea19da40987edf4f9, and merge by hand. No auto-merge, and nothing in this run merged, closed or deleted anything. The blocked T11 (end-to-end verification on a real identity — no App or bot user exists for this repository) is a stated gap for that human gate, recorded in the ADR-018 amendment and the PR body, deliberately not filed as a separate card.