Skip to content
Back to skills

Pair Workflow Review Phase

BSecurity

Stage 4 of the delivery workflow — independent final verification of one exact PR head: the deterministic custody check first (red-snapshot.mjs verify / verify-chain: snapshot ancestry, sealed blobs byte-identical, no unlisted test change, fixScope respected, revisions as successors), then the fixer's evidence re-run, the approved witnesses and controls, every prior blocking finding with an explicit transition, the source delta and its directly affected consumers, cross-group interactions, th...

  • 2 stars
  • 0 votes
  • 0 copies
  • 0 views
  • Added November 7, 2026
testingrustgoshellbashnodeazurecode-reviewgitdevopssecurity

Works with

  • terminal

Security analysis

B75/100
  • criticalSends environment variables or credentials to an external URL

Pro scans all 14 files and shows the line behind each finding

Scanned November 7, 2026

npx -y skills add learnn-com/designn --skill pair-workflow-review-phase --agent claude-code

Installs into .claude/skills of the current project.

Are you the author of Pair Workflow Review Phase?

Add the live security badge to your README. It updates with every re-scan.

Security grade badge for Pair Workflow Review Phase
[![Security: B — Skills Directory](https://www.skillsdirectory.com/api/skills/learnn-com-pair-workflow-review-phase/badge)](https://www.skillsdirectory.com/skills/learnn-com-pair-workflow-review-phase)

More formats (shields.io, HTML) on the badges page. Keep it an A: scan every change in CI with Pro.

Download with Pro
SKILL.md
---
name: pair-workflow-review-phase
description: "Stage 4 of the delivery workflow — independent final verification of one exact PR head: the deterministic custody check first (red-snapshot.mjs verify / verify-chain: snapshot ancestry, sealed blobs byte-identical, no unlisted test change, fixScope respected, revisions as successors), then the fixer's evidence re-run, the approved witnesses and controls, every prior blocking finding with an explicit transition, the source delta and its directly affected consumers, cross-group interactions, the adopted risk-tier review passes (general via /pair-process-review, plus security / boundary / architecture lenses as the tier rises) — unioned into one finding set with stable ids, one verdict and the head it reviewed. Publishes the one first review or the one synthesis idempotently (pr-comment.mjs upsert by marker) and states readiness only when the remote head equals the verified head. Read-only on code; never fixes, never merges. Dispatched by the batch engine (pair-implement-batch)."
version: 0.2.0
author: Foomakers
---

# /pair-workflow-review-phase — One Head, One Verdict, From Someone Who Did Not Write It

Judge the whole result on its own merits, adversarially, from the story (acceptance criteria), the PR (diff + description), the code and the cycle's evidence (contracts, seals, ledgers — claims to reproduce, never author context to trust). Custody first, a script decides; then evidence you re-run yourself; then the review passes the tier requires. Nothing the author wrote for themselves reaches you.

## Arguments

| Argument            | Required | Description                                                                                                                             |
| ------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------- |
| `$run`, `$story`, `$pr`, `$branch`, `$worktree`, `$base`, `$stacked`, `$entry`, `$policy`, `$inputs`, `$workflowVersion` | Yes | The cycle arguments. `$worktree` here is a DETACHED throwaway path (`<worktreeRoot>/<story>-review`). Handoffs live under `.pair/working/runs/$run/$story/` in the MAIN checkout. |
| `$phase`            | Yes      | `r<n>`.                                                                                                                                 |
| `$mode`             | Yes      | `first` — the cycle's first review, POSTED with `$marker`. `re-review` — prior findings + the delta since `$head`, silent unless converging. |
| `$head`             | Yes      | The diff base for THIS pass: the head the implementation was published on (`first`), the previous `reviewedHead` (`re-review`). Empty only on a resumed PR whose cycle has no evidence yet — then the diff base is the story's `$base`. |
| `$marker`           | Yes      | `<!-- pair:first-review #<story> PR#<n> run:<run> -->` — line 1 of the first-review comment, matched verbatim by every later resume of THIS run; a later cycle on the same PR carries its own run id and never edits this comment (canary v9). |
| `$synthesisMarker`  | Yes      | `<!-- pair:synthesis #<story> PR#<n> run:<run> -->` — line 1 of the ONE remediation synthesis of this run. Passed for reference only: the synthesis is PUBLISHED by `cycle-runtime.mjs finalize` (the host's, or yours per Step 5.3 when no host runtime is present) — never `upsert` it yourself. |
| `$reviewLog`        | Yes      | The cycle's working log in the MAIN checkout (`.pair/working/reviews/<story>.md`) — rounds appended by the fixers; you read it to synthesize. |
| `$template`         | Yes      | Name of the code-review template whose vocabulary and structure the report follows.                                                    |
| `$severities`, `$verdicts` | Yes | Comma-separated vocabularies from the template contract.                                                                              |
| `$floor`, `$ranks`  | Yes      | The severity floor name (may be absent) and the JSON rank map — the script computes `blocking` from them; the coordinator re-checks it.  |
| `$attempt`          | Yes      | The cycle state's attempt number for THIS review of `$phase` (`1` for the first pass; `2+` after a GREEN retry or for a later reviewer of the same pass). It names the handoff file — never derive it from `$reviewer`. |
| `$reviewer`, `$reviewers` | Yes | Your index among the tier's independent reviewers and their count (`pipeline.reviewers`). `$reviewer > 1`: run your passes blind FIRST, then union with the prior reviewer's handoff. |
| `$reviewSkill`      | No       | The project's review process skill for the general pass (default `/pair-process-review`).                                                            |
| `$writeIssue`       | No       | The project's issue-filing skill (default `/pair-capability-write-issue`) — named only to forbid it.                                                    |
| `$prior`            | re-review | Name of the previous review handoff (`r<n-1>-review-phase`) in `$RUN_DIR`; `$openIds` the ids it left open — every one needs a transition. |
| `$headMoved`, `$inputsChanged` | No | `true` when the remote head moved after the last verification / when the effective inputs changed: re-validate every prior finding plus the delta; never a fresh full pass. |
| `$required`         | No       | JSON: verified prior evidence a card carries in (`observedHead`, oracle, probe, observed) — re-prove it on this exact head.               |
| `$regressionGuards` | No       | JSON: the authoritative ACTIVE regression risks derived from the ledger (US-479 F-RR-03) — execute every closure assertion on the exact head and echo the set as `regressionGuards`; a set that differs from the dispatched one is refused. |
| `$contractSpec`     | No       | JSON `{ name, template, contract, skeleton, mirrors, contractSkill, workflowVersion }` — present on the FIRST review dispatch of a run only (t9d-2, AC-06): resolve the template contract in Step 0 by invoking `/pair-workflow-contract-phase` with exactly those arguments (cache-by-hash: a fresh cache is a file read) and return its output as `templateContract: { status, contract }`. Take `$severities`/`$verdicts` and the ranks for `blocking` from `contract.vocabulary` / `contract.severityRanks` — the prompt's defaults and `$ranks` are placeholders then; `$floor` is the name to apply. |

## Algorithm

**Headless execution (single owner: `.pair/knowledge/guidelines/technical-standards/ai-development/skill-conventions/headless-stage-execution.md`)** — this stage runs headless: never leave a command in the background and never end your turn while a command runs (a push with a pre-push gate, a quality gate): use the maximum tool timeout, poll to completion in the same turn, and ALWAYS end with the stage's structured result — on partial progress `{ status: "failed", reason: "incomplete", detail, branch, outputHead }`.

### Step 0: Resolve the durable state (mandatory)

```bash
SKILL_DIR="$(dirname "<absolute path of this SKILL.md>")"
MAIN="$(pwd)"                                   # the main checkout — you have not cd'd yet
RUN_DIR="$MAIN/.pair/working/runs/$run/$story"
AC_HASH="$(node "$SKILL_DIR/scripts/cycle-state.mjs" ac-hash --story $story --dir "$RUN_DIR" | node -pe 'JSON.parse(require("fs").readFileSync(0,"utf8")).acHash')"   # canonical card hash — the SAME input every stage resolves with
node "$SKILL_DIR/scripts/cycle-state.mjs" resolve --dir "$RUN_DIR" --workflowVersion $workflowVersion \
  --policy '$policy' --entry $entry --story $story --inputs $inputs --acHash "$AC_HASH" --runsRoot "$MAIN/.pair/working/runs" --pr $pr --head "$(git ls-remote origin refs/heads/$branch | cut -c1-40)"
```

- `status: other-run` ⇒ return `{ status: "other-run", runId }`. `incompatible | invalid` ⇒ return `{ status: "redirect", next: { step: "blocked", reason: "failed-resume", detail: <reason> } }`.
- `next` is `blocked` with `reason: awaiting-scope-decision` ⇒ BEFORE redirecting, discover the decision the maintainer may already have posted on the PR (canary v9, B — a fresh run directory does not remember it): `REPO="$(gh repo view --json nameWithOwner -q .nameWithOwner)"; node "$SKILL_DIR/scripts/cycle-state.mjs" apply-scope-decisions --dir "$RUN_DIR" --repo "$REPO" --pr $pr --workflowVersion $workflowVersion` (no `--decision-ref`: the script reads the PR's comments back and applies every decision-shaped one through the same authenticated path, oldest first), then run `resolve` again and act on THAT `next`. You never author a decision.
- `next.step` is not `verify`, or `next.phase` is not `$phase` ⇒ return `{ status: "redirect", next }` verbatim. Spend no judgment. When `next` names THIS dispatch (`verify`, `$phase`) — `inputsChanged` or `headMoved` included — you ARE the step: continue, never return a redirect to yourself.
- `$contractSpec` given ⇒ BEFORE any judgment, invoke `/pair-workflow-contract-phase` with it and keep the returned contract: its `vocabulary` and `severityRanks` are what you review and rank against, and `templateContract` goes into your result (Step 5.6 publish does not need it; the coordinator adopts it and validates THIS review's verdict against it post hoc). Missing or failed ⇒ return `templateContract: { status: "failed" }` and review with the prompt's default vocabulary.
- Otherwise continue; `next.attempt` is your attempt number.
- `next.step: done` on a completed cycle whose remote head still equals its `reviewedHead` is returned as a redirect too: a cheap identity check, no judgment, no publication.

### Step 1: Isolation and pacing

1. Never switch the main checkout's branch. `git worktree remove --force $worktree 2>/dev/null; git fetch origin -q; git worktree add --detach $worktree origin/$branch; cd $worktree`, then run the adopted `worktree-setup-command` (`way-of-working.md`; absent ⇒ none) before any gate — the fresh tree has no dependencies or build; a `worktree-setup-command` non-zero exit HALTs the review — no gate runs on a broken setup (`status: failed`, `reason: worktree-setup-failed`); never symlink another checkout's `node_modules`/`dist` (spurious gate failures). Record `setup: <cmd> exit N` alongside the gates actually run with their exit codes; a gate not run is never reported green. Remove it when done — every handoff you write goes to `$RUN_DIR` in the main checkout, never here.
2. **PACING (mandatory)**: a supervisor kills any agent that goes 180 seconds without a TEXT message. After EVERY file you inspect, write ONE SHORT LINE. List the changed files first (`git diff $head...origin/$branch --name-only`), say the order, go file by file.
3. **Blind (mandatory)**: do NOT read `.pair/working/` except `$RUN_DIR` (contracts, seals, handoffs, prior reviews — cycle evidence) and `$reviewLog` (to synthesize). Checkpoints are the author's private context.

### Step 2: Custody (deterministic — the script decides)

```bash
SKILL_DIR="$(dirname "<absolute path of this SKILL.md>")"
cd $worktree && node "$SKILL_DIR/scripts/red-snapshot.mjs" verify-chain --pr $pr --base <cycle base> --run-dir "$RUN_DIR" --base-ref $base   # ONE call: the chain lists every seal between the base and HEAD under EVERY identity (pr=0 initial chain, pr=$pr remediation groups); `--pr` is informational; `--run-dir` lets the SCRIPT decide whether this cycle sealed a contract
```

`<cycle base>` is the head the cycle's FIRST seal sits on (`inputHead` of the earliest sealed `red-verify` handoff in `$RUN_DIR` — `a0`'s for a story on the sealed `a0` path, the first remediation group's otherwise). **When `$RUN_DIR` holds no sealed `red-verify` handoff at all, this cycle has sealed nothing**: the first review of a FRESH card (its first handoff is `a0-implement-phase`, built test-first above the base with no up-front contract — US-506) or of a PR-entry cycle, before any remediation round. A fresh card's custody starts at the base: there is no `a0` segment, and the implementation's own commits precede every seal. Pass `--base $head` and ALWAYS `--run-dir "$RUN_DIR"`: the script derives the expectation from the run directory itself (t9d-9) — no sealed `red-verify` handoff ⇒ `contract: 'none'`, snapshots left by earlier, CONCLUDED cycles are returned as `historicalSnapshots` and are not breaches of the cycle starting now; a sealed handoff present ⇒ the strict walk, whatever you assert. `--contract-expected false` is a claim the script checks, never a bypass: beside a sealed handoff it is the typed breach `contract-expected-refused`. Do not walk from the branch's merge-base hoping to find the cycle's seals: a finished cycle's snapshot commit lives in the branch forever, and treating it as binding makes every file it sealed untouchable outside the workflow for good — a hand fix, a hotfix or a merge from main then fails the NEXT cycle at its first review, with no way out, because a successor seal needs `validate` and custody blocks at `r0` (canary 481-v5). The chain answers `contract: "none"` and custody is `verified: true`, because a cycle with no contract has no custody to violate. Do NOT omit the call and do NOT invent a base — say out loud that there was nothing to check. Without that flag the chain reports `snapshot-missing` and the coordinator reads it as `failed-custody`, killing the cycle at r0 with nothing wrong (canary 481-v2). The flag is a STATEMENT ABOUT THE CYCLE, never a way past a breach: if `$RUN_DIR` does hold a sealed handoff, the strict default applies and a missing snapshot is the rebase breach it has always been. Run it ONCE: a later seal ends the previous seal's segment whatever identity it carries, so a per-identity run would report the first remediation round's commits as breaches of the `a0` segment (T-9, t9-1); a single-snapshot group with no revision may also be checked with `verify --phase`. Any breach (`snapshot-missing`, `parent-not-base`, `test-blob-changed`, `unlisted-test-changed`, `out-of-scope`, `behavioral-adds-or-moves-module`, `test-mode-production-change`, …) ⇒ `custody: { verified: false, contractBreach: true, breaches }`, no review, publish the handoff, return — a breach is terminal for the attempt and preserves the trusted snapshot.

`--base-ref $base` (US-506 AC-11): when the story branch merged its base (`origin/main`), the files that merge brought in — paths the merge commit itself changed AND the base changed since the fork, byte-identical to the base at the segment end (or deleted by the base, when they predate the branch split) — are the base's changes, not the PR's: the script leaves them out of every segment and lists them under `mergedBase.paths` (copy it into `custody` like `overriddenBreaches`). A file edited after the merge differs from the base and is still a breach; so is a PR-owned file deleted anywhere, and a fixer's own edit that restores base content on a path the merge did not change; a merged branch that is not the base earns no exemption; a sealed blob is checked by identity regardless. **The base moved while the cycle ran (AN):** a path BOTH sides edited in different places merges clean, so its blob is neither the base's nor the story's — the script RECOMPUTES the merge (`git merge-tree`) and attributes the path to the base only when the merge commit carries exactly the recomputed blob (conflict resolution, an evil merge, or any edit before or after the merge stays a breach); a SEALED test changed this way is listed under `mergedBase.sealedPaths` instead of `test-blob-changed`. **Generated files (AN2):** mirrors regenerated after the merge (a path under a mirror root byte-identical to a dataset file of the same name; a mirror deleted because its dataset origin is gone) and the `.pair/llms.txt` index (every changed line an index line, every added link resolving) are not the agent's changes — the script lists them under `generated.paths`; copy it into `custody` like `mergedBase`. A diverging mirror, a mirror-root file with no dataset counterpart, or prose in the index stays a breach. Never write a custody override for a base merge or for generated files.

An override the script HONORED comes back as `overriddenBreaches`: the breach that would have blocked, with `override: { authorizedBy, reason, at, verifyAgainst? }` attached. Whenever `verify-chain` returns that array, copy it into your `custody` VERBATIM — `custody: { verified: true, contractBreach: false, overriddenBreaches }` — and name it in the review body. The published handoff is the ONLY durable record of who authorized which exception and on what evidence; a `custody: { verified: true }` that drops the array erases the attribution the override exists to preserve (observed on PR #494, where the human authorization left no trace in the record). Never omit it because custody came back green, never summarize it, never re-derive it: the script’s array, exactly as printed. The overrides themselves are human-authored in `<runDir>/custody-overrides.json` and are NEVER yours to write, edit or add to — you report what the script honored, you do not authorize anything.

### Step 3: Evidence (re-run, never trust)

**`mode: doc` groups** (`.pair/knowledge/guidelines/collaboration/automation/doc-remediation-groups.md`): there are no witnesses to re-run. Verify each `checklist` item of the sealed contract against the diff AND its authority (the changed text really states the requirement and agrees with the source), re-run the usual conformance/link tests for the touched files, and treat any change outside `fixScope` prose as a custody breach.

0. **Fresh card, `first` pass — the tests, judged per AC (US-506 AC-3).** There is no sealed contract to re-run: the tests the implementer wrote together with the code ARE the evidence, so judge them qualitatively. For EVERY acceptance criterion of the card name the test(s) that prove it and assess them: `proven` (the test would fail without the behavior the AC claims), `weak` (it exists but cannot fail for the defect it should catch — e.g. `toHaveReturnedWith` on an async mock, an assertion on a value the setup fixes), `missing` (no test proves the AC). Record it as `acAssessment: [{ ac, tests, assessment, findingId? }]`. A `weak` or `missing` row is an ORDINARY finding (`kind: defect`, no new class) and names it in `findingId` — `publish` refuses the table without it (`acAssessment-missing`, `acAssessment-invalid:<ac>`, `acAssessment-finding-missing:<ac>`). A **mutation probe** (break the production line an AC depends on in an isolated copy, re-run its test) is a tool you MAY use to confirm a suspicion — never a required step. No evidence of test-first is asked for or checked.
1. Re-run every sealed witness and control command from the manifests: witnesses green, controls green. A witness that FAILS is a finding `kind: approved-test-failing` with `groupId` and `rowId` — it routes the group back to GREEN on the same seal. (Only a group that SEALED a contract has approved tests: on a fresh card `a0` has none, and a test weakness there is an ordinary finding.)
2. Re-run every row of the fixers' evidence ledgers (`$RUN_DIR/*-green-fix.json`, `implement-phase.json`): `observed` must reproduce. A ledger is an input to verify, never proof by assertion. Every new fixture field or table column is consumed by an expectation (trace it).
3. `$required`: re-run its oracle on this head; if the defect is still present it is a finding (`kind: regression`).
4. Full adopted gates on THIS head: run `$reviewSkill`'s gate step (or `/pair-capability-verify-quality` for the tier); a test result is reusable only under an identical `test-identity` recorded in the fixers' handoffs — otherwise rerun. The final gate set is the adopted full set.

### Step 4: Classify, then review passes by tier (declared before running, each once)

**Classify FIRST — the tier is COMPUTED, never merely read.** `/pair-capability-publish-pr` creates the PR without classification tags, so an untagged PR is the NORMAL state at review time, not evidence of risk: it is the cue to run `/pair-capability-classify`, never an answer by itself. Compose `/pair-capability-classify` with `$context: review` against the PR diff at `$head`: the story's `risk:*` label (read from the card — `$story` — through the bound host adapter) is the **floor**; the diff may RAISE it (confirm-or-raise), never lower it. The result is the handoff's `tier`, with `tierSource: "classified"`. Fall back to the fail-safe `risk:red` ONLY when classification itself fails (`/pair-capability-classify` not installed, the card or the diff unreadable, no resolvable story tier): then state it explicitly — `tierSource: "classification-failed"`, plus the reason in the review body ("classification failed: <why> — fail-safe risk:red") — and never present it as a classification of the diff. A docs-only diff, or an untagged PR, never short-circuits this step.

Declare the passes, run each once, never sample a pass again hoping for a different answer:

| Tier | Passes |
| ---- | ------ |
| `risk:green` | **general** — `$reviewSkill` phases 1–4 (validation, technical review, adoption compliance, completeness) |
| `risk:yellow` | general + **security** (`/pair-capability-assess-security $mode=review`) + **boundary**: for every changed parser, state machine, configuration or command-output domain, a finite decision table of supported states plus their invalid/boundary pair, probed at the real producer/consumer |
| `risk:red` | yellow's set + **architecture** (`/pair-capability-assess-coupling $scope=diff`) + an **adversarial second general pass** that starts from the acceptance criteria and tries to break each one |

- `first`: the full PR surface. **Contract inventory**: map each changed observable contract to its authoritative producer, inputs, consumers and representations before reporting the first hole.
- `re-review`: every `$openIds` finding gets a transition — `resolved` (you verified the fix, not the claim), `open` (still present, same id, same severity unless `severityEvidence` names a NEW failure case or changed impact), `superseded` (absorbed by another finding you name). Then ONLY the delta `git diff $head...origin/$branch --name-status`, its directly changed producer/consumer boundaries and the **cross-group interactions** of the groups fixed this round (two individually green groups whose outputs feed each other are checked together). A new finding is `kind: defect` if it is in this delta or a boundary changed by it; a real defect on unchanged code that the earlier review missed still blocks under the unchanged policy, is marked `missedUpstream: true` and gets a regression row in the next preparation; a gap in a sealed contract's coverage is `kind: contract-gap` with `groupId` — it revises that group only (`groupId: "a0"` for a SEALED initial acceptance contract: the cycle prepares `a0-rev<m>`, seals it as a successor and implements again on the same branch; on a fresh card `a0` sealed nothing, so a coverage gap there is an ordinary finding and routes the test-first remediation).
- Union the passes; deduplicate by (owner, location, observable defect) keeping every rationale; on severity disagreement keep the highest evidenced severity.

Every finding is REAL, EVIDENCED and CONCRETE (US-506 AC-3): it carries a **reproducer** — `reproducer: { command }` or `reproducer: { testRef }`, an executable reference with no shell syntax — or a **concrete failure scenario** — `failureScenario: { input, actual, expected }` (this input ⇒ this wrong result). `publish` refuses an OPEN finding with neither (`finding-unevidenced:<id>`); a closure (`resolved`/`superseded`/`human`) transitions a finding evidenced when it was opened, and a `question` claims no defect. **A difference of taste ("I would have done it differently") is not a finding and is not recorded** — not as a Minor, not as a question, not in the report. The handoff records ONLY the activities of this independent review: never the implementer's self-review, never a narrative of how the author worked. Then: `id` (`r<n>-<k>`, or `r<n>-<letter>-<k>` when `$reviewer > 1`; a prior finding keeps its id), `location` (File:Line, or the card / PR body for an `external: true` finding — a location, never acceptance), `severity` ∈ {`$severities`}, `description` = the failure case (inputs/state → wrong output), `recommendation` ending `VERIFY: …; ORACLE: …; ASSERT: …`, `kind`, `transition`, `blocking` = `!nonActionable && transition ∉ {resolved, human} && kind ≠ question && rank(severity) ≥ rank($floor)` computed from `$ranks`. `nonActionable: true` ONLY when fixing would be genuinely wrong (byte-consistent with a source of truth, an existing convention, an ALREADY-EXISTING tracked story — cite its number; do not create one — or something that resolves only after merge), always with `disposition`. An `external` finding resolves only with read-back `evidence` (the exact command and what it showed) or a `transition: human` the card supplied. **DO NOT FILE NEW ISSUES**; never invoke `$writeIssue`. **History rewrite**: `needsHumanDecision: true`, `humanDecisionKind: "history-rewrite"` (any other mandatory human decision sets `needsHumanDecision: true` too — the kind only says why, and the escalation precedes every automatic transition either way). Name EVERY finding that owes a human decision in `humanDecisionIds` (US-506: ids of this review's own findings, `publish` refuses an unknown one) — the cycle stays `escalate` until `cycle-state.mjs decide` has recorded each of them. **A finding carrying an ACTIVE `regressionRisk` is `blocking: true`** (US-479 F-9): an active risk IS an open blocker, `publish` refuses the handoff otherwise, and the severity floor never applies to it — a regression a remediation introduced is not a defect the floor may carry unfixed. The DISCHARGE of that same risk is the mirror image: `transition: resolved`, `blocking: false`, and `publish` refuses it the other way round. State follows the risk, not the finding.

**Stale-claim sweep** (US-582): before writing a finding about documentation (guideline, ADL/ADR, docs page, mirror, changeset) that states behaviour the PR changed, search the whole PR diff (`git diff $base...origin/$branch`) and the repository docs for the same claim — the specific claim, not the topic — and list EVERY location in that one finding's `location`. A generated mirror is listed with its dataset source (`<mirror> ← <dataset path>`) and fixed at the source. One stale claim ⇒ one finding, never N findings over N rounds: a later hit of a claim already reported joins that finding (its id, no new one). The doc group red-spec plans from it must cover every listed path (`finding-location-outside-scope`, `doc-remediation-groups.md`).

**US-479 (ADL 2026-09-12) — `worked`: what was already RIGHT.** When this review leaves any
obligation open or any regression active, also record the decisions you verified were correct in the
work that may be discarded: `worked: [{ id, claim, appliesTo, evidence }]`, where `evidence` has the
same shape as `closureAssertions` (`{ id, command | testRef, expected }`). A design decision no
command can demonstrate is legal as `notVerifiable: true` WITH a `rationale` — the same bargain
`nonActionable`/`disposition` strikes. The contract already says what must work again and what must
not break; nothing says what was already right, so a rebuild from an earlier head repeats the
discarded round's mistakes. You are the only participant who looked at the code closely enough to
know. Ids are stable across rounds, and `appliesTo` (paths or obligation ids) must not be empty: a
note that applies to nothing reaches no rebuild. An `evidence[].command` is an executable reference,
never shell code: `publish` refuses (`worked-evidence-unsafe:<id>`) a command containing `;`, `&`, `|`,
a backtick, `$(` or `../` — the same filter reproducers and counterexamples pass. One plain command per
evidence row; a pipeline or a chain belongs in a `testRef`, not in `command` (canary v9).

**Scope proposals are NEVER findings (US-479 T-22, S2/S5):** something demonstrably absent from the story's baseline/adoption/supported boundary — not a violation of anything approved — is `scopeChanges[]: { id: "sc-<n>", type: "new-requirement" | "scope-extension", proposal, baselineEvidenceRefs: [the evidence it is new, not a defect], discoveredAtReviewId, status: "pending" }`. It NEVER carries `severity`, is NEVER `blocking`, NEVER enters a remediation fix plan, and is never counted as a defect. `sc-<n>` ids are assigned once, under the same lock as finding ids (max existing numeric `sc-` id + 1), and stay stable across rounds. Cite the accepted obligation a finding violates; a proposal has none — if you cannot tell which it is, keep the classification visibly unresolved rather than moving a real defect into `scopeChanges`.

### Step 5: Verdict, readiness, publication (idempotent)

1. `reviewedHead` = `git rev-parse origin/$branch` AFTER inspection, lower-case 40-hex. `readiness.remoteHead` = `git ls-remote origin refs/heads/$branch` read back at the very end, ALWAYS present as lower-case 40-hex; `readiness.ready` = no blocking finding AND `remoteHead == reviewedHead` AND the full adopted gates passed on it. A `ready: true` without that head is unproven: the coordinator refuses it and the cycle state re-verifies (T-9, t9-3).
2. `first` ⇒ write the report to the [skeletal model](../../../.pair/knowledge/guidelines/technical-standards/ai-development/shrink-discipline.md#skeletal-output-model) within the template's `word-limits` and run `node "$SKILL_DIR/scripts/word-limits.mjs" check --template <$template> --body <report.md>` (exit 1 ⇒ shorten the listed sections, re-run); then post the full report as ONE PR comment in the `$template` structure with `$marker` verbatim as line 1: `node "$SKILL_DIR/scripts/pr-comment.mjs" upsert --pr $pr --marker "$marker" --body-file <report.md> --dir "$RUN_DIR"` — the script reads the PR's comments back and edits an existing marker comment in place, so a restart under another invocation never posts a second first review.
2b. **A re-review that opens NEW findings is never silent.** After you publish the handoff, run `node "$SKILL_DIR/scripts/cycle-metrics.mjs" findings --dir "$RUN_DIR" --story $story --pr $pr --runId $run --out <findings.md>`: when it answers `needed: true` (a finding was first reported by a re-review), `upsert` that file with its own marker line `<!-- pair:findings #$story PR#$pr run:$run -->` (`pr-comment.mjs upsert --body-file`) — the "Open findings" comment lists every open finding and those the re-review opened, with severity, round found, status, location and summary. `finalize` upserts the same comment by marker as a guarantee, so a skipped step never leaves an opened finding invisible. Silence is allowed ONLY when the re-review opened nothing new.
3. Converged (`ready`, no blocking finding, you are the last non-partial reviewer) ⇒ your own handoff carries every finding across ALL rounds (via `priorFindings`/the union you performed) and every scope proposal — that is the durable record. **US-479 T-26 (S8), as amended 2026-09-13 (canary v9, A):** the ONE final synthesis at `$synthesisMarker` and `metrics.json`/`metrics.md` are produced by `cycle-runtime.mjs finalize` — deterministic, zero additional model tokens, never a second judgment — and WHO runs it depends on whether a host runtime is present. `$RUN_DIR/.runtime-checkpoint.json` or `$RUN_DIR/.run-terminal.json` exists ⇒ the host's launch recipe owns `finalize`: report `metrics: { owner: "host", written: false }` and `published.synthesis: false`. Neither exists (the Workflow sandbox cannot run the recipe; canary v9 left an APPROVED verdict in the run directory and the PR without a synthesis) ⇒ AFTER step 6's publish, and ONLY when `resolve` then answers `next.step: done`, run it yourself: `REPO="$(gh repo view --json nameWithOwner -q .nameWithOwner)"; node "$SKILL_DIR/scripts/cycle-runtime.mjs" finalize --dir "$RUN_DIR" --repo "$REPO" --story $story --branch $branch --pr $pr --runId $run` — then report its printed output as `metrics: { owner: "review-phase", written, revision, completeness }` and `published.synthesis` = (`publication.state == "confirmed"`). A `published.synthesis: false` from you is `failed-publication` for the coordinator, never `ready-for-merge`. Never hand-write the synthesis with `pr-comment.mjs`; do NOT minimize or delete anything from your OWN handoff — it stays until merge.
4. Blocking findings and the coordinator's budget is exhausted (COMPLETED corrective cycles reach `policy.maxFixRounds` — a metadata-only re-review never spends it, US-479 T-21 S4), or every blocking finding is `external`, or a history rewrite ⇒ `upsert` an escalation with `<!-- pair:escalation #$story PR#$pr run:$run -->` naming the open ids and the human decision required (run-scoped like the first review: a previous cycle's escalation is history, never edited). An escalation for external blockers sets `needsHumanDecision: true` and `humanDecisionIds: [<those ids>]` in your result — `decide` answers exactly those.
4b. Zero blocking findings but one or more `scopeChanges[]` are `status: pending` (the cycle state's `awaiting-scope-decision`, US-479 T-22 S5) ⇒ FIRST run the discovery command of Step 0 (`apply-scope-decisions` with no `--decision-ref`): a decision the maintainer posted on an earlier cycle's packet for the same `sc-` ids is applied, `resolve` moves on (`done`, or a targeted remediation for an approved extension) and no packet is posted — the baseline hash is computed over the pending ids and types, never the wording, so your re-authored proposal text does not stale it. Only when discovery reports `no-decision-comment` (or every candidate was refused with its reason) ⇒ `upsert` the consolidated packet with `<!-- pair:scope-decision #$story PR#$pr -->` (PR-scoped ON PURPOSE, no run id: `sc-` ids and the maintainer's answer outlive cycles, so a later cycle edits the one packet instead of asking twice): for each proposal, its `sc-ID`, the proposal text, the evidence that it is new scope (not a defect), the affected AC/areas, and the three permitted choices (`ignore` / `extend-current-card` / `new-card`) with the exact fenced-JSON shape `apply-scope-decisions` expects (`schemaVersion`, `scopeBaselineHash`, `decisions[]`). **`scopeBaselineHash` is never yours to compute by hand (t9d-8):** quote the `scopeBaselineHash` the blocking `resolve` printed next to `scopeChanges`, or read it from the run directory with `node "$SKILL_DIR/scripts/cycle-state.mjs" scope-baseline --dir "$RUN_DIR"` (the same pending union `apply-scope-decisions` checks against) — a packet that omits it, or carries a hand-made one, makes the maintainer's answer unusable (`scopeBaselineHash-missing` / `stale-baseline`). This is informational only — you never generate a decision; the maintainer replies and the coordinator applies it mechanically via `cycle-state.mjs apply-scope-decisions`.
5. `$reviewers > 1`: EVERY reviewer records `reviewer: $reviewer` and `partial: $reviewer < $reviewers` in the handoff (reviewer 1 included); `$reviewer > 1` unions its findings with the prior reviewer's handoff (`$prior`); only the last reviewer publishes to the PR. The handoff's `attempt` is ALWAYS `$attempt` from the dispatch (the cycle state's number), never `$reviewer` — the second review of a phase would otherwise land on an occupied filename (T-9, t9b-1). The cycle state dispatches reviewer `k+1` of the SAME phase until `$reviewers` non-partial reviews of the same head exist; a partial review can never complete the cycle (T-9, t9-2).
6. Publish the handoff (`skill: "review-phase"`, `inputHead: $head or $base`, `mode`, `tier`, `tierSource`, `passes`, `reviewedHead`, `verdict`, `findings`, `acAssessment` (fresh card, `first` pass — Step 3.0), `scopeChanges`, `custody`, `readiness`, `attempt: $attempt`, `reviewer`, `partial`, `remediationBatchId?: "r<n>"` (`$phase`'s round, when `$phase` is `r<n>` with `n >= 1` — US-479 T-21 S4), `published: { firstReview, escalation, scopeDecisionPacket, reviewCheck?, prState?, classification? }` (the final synthesis is `cycle-runtime.mjs finalize`'s to publish — by the host or, absent one, by you AFTER this publish (step 3) — so it is never in this handoff), `gates: { command, identity, exitCode }`, `inputsDigest: $inputs`, `acHash` (any value: `publish` REPLACES it with the canonical card hash it computes itself and marks `acHashSource: publish`; only script-stamped hashes are ever compared), `severityRanks` (the template contract's severity ranks you reviewed under, for the record — the #514/AC1 revision: `publish` derives each finding's `blocking` from `$policy.blockingFloor`, ranked by RANKS IT RESOLVES ITSELF in order — `policy.severityRanks`, then the on-disk resolved template contract, then pair's own default table — never by name; the ranks you state here are checked ONLY for agreement with whichever `publish` resolved, a disagreement is a typed refusal (`severity-ranks-mismatch`), never an alternate ranking; omit only when the vocabulary is pair's own default table), `reviewer`, `partial`, `elapsedMs`) with `cycle-state.mjs publish … --workflowVersion $workflowVersion --attempt $attempt --predecessor <a0-implement-phase | r<n>-g<k>-green-fix | r<n-1>-review-phase> --pr $pr --policy '$policy'` (`publish` requires `--workflowVersion`), run `resolve` again and return its `next` **verbatim** — copy the whole object the script printed; never retype, summarize or drop a field it carries because another field in your answer already seems to say the same thing (e.g. `contract`, next to a `contractHash` you also state). The orchestrator checks the object's shape, not your restatement of it; a hand-reconstructed `next` missing one field is refused as unusable. When that `next` is `done` and no host runtime marker exists, run `finalize` as step 3 says and add its `metrics` and `published.synthesis` to your result.
7. **Conclude the required check and the state label (t9d-24 — the deterministic half of the review's Phase 5).** You are the last non-partial reviewer (`$reviewer == $reviewers`, `partial: false`) ⇒ AFTER the publish: `REPO="$(gh repo view --json nameWithOwner -q .nameWithOwner)"; node "$SKILL_DIR/scripts/pr-state.mjs" conclude --pr $pr --sha $reviewedHead --verdict <approved | changes-requested> --repo "$REPO" --description "<verdict> on $phase — <n> blocking finding(s)" --dir "$RUN_DIR" --tier $tier [--cost $costClass]`. `--dir` makes the scripts publish through the PM/code-host adapter the coordinator bound for this run (`.host-binding.json`, US-492). `approved` ONLY when `readiness.ready` is true with zero blocking findings (the verdict is the template's approving one); `changes-requested` when any finding is blocking. Anything else — a partial review, an unproven readiness, `awaiting-scope-decision` — publishes NOTHING: the pending `pair-review` status keeps the merge blocked. The script maps the verdict exactly as the KB's `pr-state.sh` does (`success` + `pr-state:ready-to-merge`, `failure` + `pr-state:not-approved`), removes every other `pr-state:*` label and reads the labels back; a status POST the token cannot make (no `repo:status`) is reported as advisory, never faked; the same conclusion already on the head is `unchanged`. Report `published.reviewCheck` (`success | failure`, or omit) and `published.prState` (the label confirmed, or omit). `--tier` is the tier Step 4 COMPUTED (this handoff's own `tier`, with its `tierSource` — never a bare read of an untagged PR) and `--cost` is the cost class only when this review computed one; the script reads the adoption's `## Tag Projection` (`tech/risk-matrix.md`) and applies `risk:*`/`cost:*` only for the active families, reporting `published.classification` alongside `reviewCheck`/`prState` (or its `skipped` reason). **This is the ONLY place a PR's classification tags are written — `/pair-capability-publish-pr` creates the PR without them (untagged = fail-safe red until conclude runs).** Merge stays outside the engine — a human presses it, and `pair-explicit-approval` is the host's job.

**US-479 F-RR-03 — `$regressionGuards`.** When it is given, it is the authoritative set of ACTIVE
regression risks derived from the ledger: execute every one of their closure assertions on the exact
head you are reviewing, and echo the `riskId` set you actually executed as `regressionGuards`. The
coordinator enforces exact set equality — a guard you did not execute cannot be discharged by you,
and one you invent is not in the ledger. This is the set, not a hint: do not narrow it.

**US-479 S11 — regression risks.** A finding is `origin: introduced-by-remediation` only with ALL of: the approved obligation it violates (`obligationIds`), an executable `reproducerRef`, the `lastCleanReviewedHead` where that reproducer passes, the `firstFailingHead` where it fails, the `introducedByRemediationBatchId` that produced the failing head, `closureAssertions` and `affectedBoundaryRefs`. Publish it as `regressionRisk: { ..., state: 'active' }` and name the batch in `invalidatedBatchId`; `publish` stamps the `riskId` itself and refuses the claim when a proof is missing or the failing head did not come from that batch. Anything less is `origin: unknown` — an ordinary finding — and a new or changed requirement is a `scopeChanges` proposal, never a risk. You never write the active matrix: it is derived from the ledger.

To DISCHARGE a risk you must be the independent review bound to the EXACT new head: execute its closure assertions, show the cited obligation passing, confirm the batch's original findings are closed, re-test the affected boundaries, then publish the same `riskId` with `state: 'discharged'`, `dischargedHead` equal to the head you reviewed and `dischargedByReviewId`. A reintroduction reopens the risk on the same stable finding and returns to the same remediation path; it is not a new discovery. `ready`, a converged summary and any scope escalation are impossible while one risk is active.

**US-479 S12 — sample the sealed matrix, do not reinvent it.** The sealed contract already names
the transition's positive and negative rows: execute them and the changed interactions on the exact
head. You are not the first control expected to discover a fundamental illegal transition — that is
the contract's job upstream. A further variant of a root cause already contracted stays that same
finding id; a genuinely distinct defect needs its own deterministic witness, independent cause and
separate impact before it is reported as new.

## Output Format

`{ status: reviewed, verdict, reviewedHead, findings: [{ id, location, severity, description, recommendation, kind, transition, blocking, reproducer?: { command | testRef }, failureScenario?: { input, actual, expected }, nonActionable?, disposition?, external?, evidence?, groupId?, rowId?, severityEvidence?, missedUpstream? }], acAssessment?: [{ ac, tests, assessment: proven | weak | missing, findingId? }], scopeChanges?: [{ id, type, proposal, baselineEvidenceRefs, discoveredAtReviewId, status }], custody: { verified, contractBreach, breaches?, overriddenBreaches?, mergedBase? }, readiness: { ready, remoteHead }, published: { firstReview?, synthesis?, escalation?, scopeDecisionPacket?, reviewCheck?, prState?, classification? }, metrics?: { owner, written, revision?, completeness? }, templateContract?: { status, contract? }, tier, passes, needsHumanDecision?, humanDecisionKind?, humanDecisionIds?, partial?, reviewer?, regressionGuards?, worked?, next }`.

## Notes

- Read-only on code: never edit, commit, push, fix or merge. The only writes are the marker-keyed PR comments, the `pair-review` commit status and the ONE `pr-state:*` label at conclusion (Step 5.7, last non-partial reviewer only — t9d-24) and the handoff.
- The verdict is the template's; control flow keys on `blocking` and `next`, never on a verdict string. Individual green groups never imply approval: only this stage's `ready` on the exact remote head does.
- A resumed cycle whose LAST handoff is a `recordType: decision` record (published by `cycle-state.mjs apply-scope-decisions`, never by this skill) is not yours to re-judge: `resolve`'s `next` already routes it (a targeted remediation for an approved `extend-current-card`, or `done` once every proposal is dispositioned) — you only reach that step through the normal dispatch, same as any other `next`.
- **Host runtime (US-479 T-25, S7) — `entry`/`observe`/`reconcile` are never yours to invoke; `finalize` is yours ONLY when no host runtime is present (Step 5.3, canary v9 A).** The coordinator's launch recipe — NOT this skill, NOT the sandbox — runs `cycle-runtime.mjs entry` before dispatching the first phase and `observe`/`reconcile`/`finalize` around it, using the shell executor it already has; when it does, `$RUN_DIR/.runtime-checkpoint.json` or `.run-terminal.json` is there and you leave `finalize` to it. `observe` tails ONLY the journal/usage sources it is given, on a 5-second default interval, updates `metrics.json`/`metrics.md` at every observed step boundary, and stops after a durable terminal result and reconciliation (or a 30-second grace period leaves it partial, never fabricated). **US-479 B4:** the host journal carries no tokens and no timestamps, so the cost source is PRODUCED from the harness's per-agent transcripts — `usage-extract` (also run inside `observe`/`reconcile`/`finalize` when `--transcripts` is given) joins them to the journal on `agentId`, counts each provider request's fixed fields once and its final block's output, and reads no message content; `dispatch-stats` derives the four admin counters from the engine's own returned result. **US-479 F8:** the observer stops on `mark-terminal`, the host's record of the workflow's OWN returned result — the harness journal has no end-of-run record, is never modified, and the end of a run is never inferred from the agents observed so far; after `finalize` no late tick can overwrite the finalized view. A run directory that CONTINUES an older one is bound to it first, read-only, with `cycle-state.mjs migrate-acknowledge` — the lifetime metrics then fold every predecessor that persisted metrics and name every one that did not, so a new run directory is never read as a clean new PR. Every phase's own handoff publish already updates phase-level observations through `cycle-state.mjs`, so a missing provider usage source never erases durable progress — it only leaves that one dimension partial.

**What the PR shows (AK).** The final `pair:synthesis` comment ALWAYS carries a findings table (id, severity, round found, status/transition, location, one-line summary, resolution evidence); its late-defects line reads `missedUpstream` and `origin` from the review handoffs; and with no host usage the metrics say `not recorded (in-session)` (tokens, models) with the elapsed time taken from the handoff timestamps — never "unknown" repeated. **The verdict is published as the required `pair-review` check and the state label, not as a GitHub PR review** (approve / request-changes): the code-host adapters expose no review/vote operation, GitHub refuses an approval or change-request on one's own pull request (the automation account that opened it is usually the reviewer), and the check already gates the merge. Azure DevOps could carry a reviewer vote, but that needs a new adapter capability and an identity that is not the author — not implemented; documented here so the Reviews tab being empty is understood, not mistaken for a missing review.

Files in this skill

  • SKILL.md47 KB
  • scripts/autonomy-policy.mjs29.9 KB
  • scripts/cycle-metrics.mjs73.1 KB
  • scripts/cycle-runtime.mjs58.8 KB
  • scripts/host/adapter-kit.mjs14.2 KB
  • scripts/host/azure-devops.mjs22.8 KB
  • scripts/host/filesystem.mjs13.9 KB
  • scripts/host/github.mjs21.9 KB
  • scripts/host/index.mjs13.1 KB
  • scripts/host/linear.mjs14.5 KB
  • scripts/pr-comment.mjs5.3 KB
  • scripts/pr-state.mjs14.1 KB
  • scripts/red-snapshot.mjs83 KB
  • scripts/word-limits.mjs2.9 KB

Attribution

Is this your skill, or is something wrong with this listing? Request removal or report an issue. Author removals are honored within 72 hours.

Comments

Loading comments…

Newsletter

Get the weekly list of safe new skills, and the ones we pulled.