Reviewer
Shared contract for every review specialist; each agent's domain and probes live in its own agent file. These workflows run orch scripts and do not stand alone.
| Workflow |
Purpose |
workflows/review.md |
Code review: diff → findings → JSON artifact → verdict |
workflows/codebase-review.md |
Whole-codebase audit, no diff |
workflows/qa-review.md |
QA label-triggered review of one PR |
Ethos
- Verify before reporting: if the repo contains the caller, config, test, or doc that settles a suspicion, read it. Never file "maybe X handles this" when X is in the repo.
- Never trust a green check you have not seen fail: prove each instrument the change adds or modifies once on a control input that must fail, regardless of how many times the suite invokes it, before trusting its pass. A changed test carrying the statement code-quality § Tests takes in place of its control is judged on that statement. Zero samples or a nonzero measuring pipeline = instrument failure: declare the top-level
measurement_failed (schemas/review-finding.md), cite no numbers. A zero RESULT is a result: stability: 0/10 is ten measured runs and a finding.
- Report the class, not the instance. When a finding generalizes (the same missing guard at sibling sites), enumerate every affected site in that one finding.
- Duplicated judgment is a finding. Logic the diff introduces or arms that re-answers a question implemented elsewhere in the repo, or at another site in the same file, is raised even when both copies agree, and so is a rule it restates that another file owns, in prose, config or a table; name the surviving copy.
- A claim needs the line that makes it true. For every sentence the diff adds to a
--help, SKILL.md, CHANGELOG entry, comment, or diagnostic that states an order, a source set, an exit code, or a guarantee, find the code that makes it true. None found is a blocker; the claim is the defect, not the code.
- Plausible by default. Never refute a finding as "speculative" or "depends on runtime state" when the state is realistic, meaning reached by a producer you can name rather than merely conceivable: nil/undefined on a rare-but-reachable path (error handler, cold cache, missing optional field); a falsy zero treated as missing; an off-by-one on a boundary the code does not exclude; retry storms and partial failures; a regex or allowlist that lost an anchor. A finding is refuted only when the refutation is constructible from the code: factually wrong (quote the line), provably impossible (show the type, constant, or invariant), already guarded in the diff (cite the guard), or pure style with no observable effect. Whether a reach clears the filing bar is the dispositioner's call at orch finding-disposition § Filing bar, never the reviewer's.
- Judge Markdown against
../docs-writing/SKILL.md, not taste: a finding cites its standard or its file-type list, and never restates the rule. Source comments stay code-quality § Comments and Prose.
- Fewer high-conviction findings beat lists of nits.
- A reviewer writes nothing but its artifact and leaves the reviewed worktree as it found it: the
reviewer-read-only hook refuses an edit, a write into a repository, a commit, a push and Git discard commands, and the reviewer-stop-check hook refuses a stop that leaves the tree dirty.
- Project decisions and architecture docs outrank generic heuristics. Do not contradict or re-litigate the decisions the delegation lists.
- A hook or gate is judged against the workflow that runs it: name the event it fires on, the state that exists there (committed, staged, on disk), and the flow that reaches it; a trigger the standard flow never meets is a defect.
- A number in prose (a cap, a default, a count, a threshold) is re-derived from the code or the setting that holds it; a stated value the code does not carry is the defect.
- Do not re-verify what deterministic gates already enforce (preflight, doc-limits, project lint/CI); cite gate output only for the property the gate tests, and name that property: a gate's name is not its predicate.
blockers[] = worth stopping the merge: a real domain regression or high-risk uncertainty only the author can resolve. suggestions[] = actionable now (fix) or worth tracking (issue). Cosmetic items belong in neither. pass means your domain has no verified blocker in scope.
Output Contract
Capture the starting fields before reviewing. Findings are a JSON artifact per schemas/review-finding.md, written with the harness file-write tool, never shell redirection, to the delegation's Artifact: path. When the delegation carries no Artifact: line, mint the path yourself ([AGENT] = your full agent name):
.agents/skills/orch/scripts/review-artifact-check --path [WORKTREE_PATH] [AGENT]
Self-validate before returning, on the file you wrote, never the zero-epoch glob form, which falls through to an older sibling. Fix until this prints "ok": true:
.agents/skills/orch/scripts/review-artifact-check --file [ARTIFACT_PATH] [WORKTREE_PATH]
Write a control's files under a mktemp -d of your own, the way scripts/mutation-stability does: stubs, fixtures, mutants, logs. The scratchpad root is shared with the parallel panel, where a sibling overwrites a fixed name mid-review.
Return by sending the workflow's <output_format> block, filled verbatim, nothing added, as an agent-to-agent message; a disk write is never a return. Shell commands follow orch SKILL.md § Harness-Safe Shell.
Re-Review Rounds
Items the delegation lists as resolved are not re-reported, unless you check a Fixed item against the current diff and the defect is still there. Report that one again, copying the listed entry's location and description verbatim and naming its recorded commit sha in your recommendation, or saying it was recorded then dropped in a rebase when the entry carries no sha. A Fixed item you did not check, and every Escalated item, stays suppressed.
The delegation's Diff-range is the fix diff: scope the pass to that range and its blast radius, not a fresh full read. With no range, the line absent or reading unavailable, the pass is unscoped, and workflows/review.md § 1 owns what it reads and what it declares. Sweep every fixed defect's class before passing.
Mutation-Stability Pairing
Mutation proves a test can fail; stability proves it fails only for the right reason. Run both with one command, on a copy, never in the shared tree:
.agents/skills/reviewer/scripts/mutation-stability --worktree [WORKTREE_PATH] --sha [SHA] --test '[TEST_CMD]' --build '[BUILD_CMD]' --mutate '[MUTATE_CMD]'
- Kill the mutant under every selection/invocation mode the changed code exposes, not only the default (one call per mode).
- A kill counts only when the mutated copy compiles. Use the suite's compile-without-running command for
--build.
- Prove a behavior-preserving swap by driving both implementations through the real entry point and diffing every observable; the source diff alone cannot prove equivalence.
- Copy the printed
mutation: … stability: … line into your artifact's summary; that field and qa_metadata are the only carriers read as your own measurement.
- Mutation-pass + any stability-fail is a concurrency-sensitive finding, never a pass. A survived mutant means the test is not evidence.
1---2name: reviewer3description: Load when reviewing a diff, classifying findings, or returning a verdict.4license: MIT5---67# Reviewer89Shared contract for every review specialist; each agent's domain and probes live in its own agent file. These workflows run orch scripts and do not stand alone.1011| Workflow | Purpose |12|----------|---------|13| `workflows/review.md` | Code review: diff → findings → JSON artifact → verdict |14| `workflows/codebase-review.md` | Whole-codebase audit, no diff |15| `workflows/qa-review.md` | QA label-triggered review of one PR |1617## Ethos1819- Verify before reporting: if the repo contains the caller, config, test, or doc that settles a suspicion, read it. Never file "maybe X handles this" when X is in the repo.20- Never trust a green check you have not seen fail: prove each instrument the change adds or modifies once on a control input that must fail, regardless of how many times the suite invokes it, before trusting its pass. A changed test carrying the statement [code-quality § Tests](../code-quality/SKILL.md#tests) takes in place of its control is judged on that statement. Zero samples or a nonzero measuring pipeline = instrument failure: declare the top-level `measurement_failed` ([`schemas/review-finding.md`](./schemas/review-finding.md)), cite no numbers. A zero RESULT is a result: `stability: 0/10` is ten measured runs and a finding.21- **Report the class, not the instance.** When a finding generalizes (the same missing guard at sibling sites), enumerate every affected site in that one finding.22- **Duplicated judgment is a finding.** Logic the diff introduces or arms that re-answers a question implemented elsewhere in the repo, or at another site in the same file, is raised even when both copies agree, and so is a rule it restates that another file owns, in prose, config or a table; name the surviving copy.23- **A claim needs the line that makes it true.** For every sentence the diff adds to a `--help`, SKILL.md, CHANGELOG entry, comment, or diagnostic that states an order, a source set, an exit code, or a guarantee, find the code that makes it true. None found is a blocker; the claim is the defect, not the code.24- **Plausible by default.** Never refute a finding as "speculative" or "depends on runtime state" when the state is realistic, meaning reached by a producer you can name rather than merely conceivable: nil/undefined on a rare-but-reachable path (error handler, cold cache, missing optional field); a falsy zero treated as missing; an off-by-one on a boundary the code does not exclude; retry storms and partial failures; a regex or allowlist that lost an anchor. A finding is refuted only when the refutation is constructible from the code: factually wrong (quote the line), provably impossible (show the type, constant, or invariant), already guarded in the diff (cite the guard), or pure style with no observable effect. Whether a reach clears the filing bar is the dispositioner's call at [orch finding-disposition § Filing bar](../orch/references/finding-disposition.md#filing-bar), never the reviewer's.25- Judge Markdown against [`../docs-writing/SKILL.md`](../docs-writing/SKILL.md), not taste: a finding cites its standard or its file-type list, and never restates the rule. Source comments stay [code-quality § Comments and Prose](../code-quality/SKILL.md#comments-and-prose).26- Fewer high-conviction findings beat lists of nits.27- A reviewer writes nothing but its artifact and leaves the reviewed worktree as it found it: the `reviewer-read-only` hook refuses an edit, a write into a repository, a commit, a push and Git discard commands, and the `reviewer-stop-check` hook refuses a stop that leaves the tree dirty.28- Project decisions and architecture docs outrank generic heuristics. Do not contradict or re-litigate the decisions the delegation lists.29- A hook or gate is judged against the workflow that runs it: name the event it fires on, the state that exists there (committed, staged, on disk), and the flow that reaches it; a trigger the standard flow never meets is a defect.30- A number in prose (a cap, a default, a count, a threshold) is re-derived from the code or the setting that holds it; a stated value the code does not carry is the defect.31- Do not re-verify what deterministic gates already enforce (preflight, doc-limits, project lint/CI); cite gate output only for the property the gate tests, and name that property: a gate's name is not its predicate.32- `blockers[]` = worth stopping the merge: a real domain regression or high-risk uncertainty only the author can resolve. `suggestions[]` = actionable now (`fix`) or worth tracking (`issue`). Cosmetic items belong in neither. `pass` means your domain has no verified blocker in scope.3334## Output Contract3536Capture the starting fields before reviewing. Findings are a JSON artifact per [`schemas/review-finding.md`](./schemas/review-finding.md), written with the harness file-write tool, never shell redirection, to the delegation's `Artifact:` path. When the delegation carries no `Artifact:` line, mint the path yourself (`[AGENT]` = your full agent name):3738```bash39.agents/skills/orch/scripts/review-artifact-check --path [WORKTREE_PATH] [AGENT]40```4142**Self-validate before returning**, on the file you wrote, never the zero-epoch glob form, which falls through to an older sibling. Fix until this prints `"ok": true`:4344```bash45.agents/skills/orch/scripts/review-artifact-check --file [ARTIFACT_PATH] [WORKTREE_PATH]46```4748Write a control's files under a `mktemp -d` of your own, the way [`scripts/mutation-stability`](./scripts/mutation-stability) does: stubs, fixtures, mutants, logs. The scratchpad root is shared with the parallel panel, where a sibling overwrites a fixed name mid-review.4950Return by sending the workflow's `<output_format>` block, filled verbatim, nothing added, as an agent-to-agent message; a disk write is never a return. Shell commands follow orch SKILL.md § Harness-Safe Shell.5152## Re-Review Rounds5354Items the delegation lists as resolved are not re-reported, unless you check a Fixed item against the current diff and the defect is still there. Report that one again, copying the listed entry's location and description verbatim and naming its recorded commit sha in your recommendation, or saying it was recorded then dropped in a rebase when the entry carries no sha. A Fixed item you did not check, and every Escalated item, stays suppressed.5556The delegation's `Diff-range` is the fix diff: scope the pass to that range and its blast radius, not a fresh full read. With no range, the line absent or reading `unavailable`, the pass is unscoped, and [`workflows/review.md`](./workflows/review.md) § 1 owns what it reads and what it declares. Sweep every fixed defect's class before passing.5758## Mutation-Stability Pairing5960Mutation proves a test can fail; stability proves it fails only for the right reason. Run both with one command, on a copy, never in the shared tree:6162```bash63.agents/skills/reviewer/scripts/mutation-stability --worktree [WORKTREE_PATH] --sha [SHA] --test '[TEST_CMD]' --build '[BUILD_CMD]' --mutate '[MUTATE_CMD]'64```6566- Kill the mutant under every selection/invocation mode the changed code exposes, not only the default (one call per mode).67- A kill counts only when the mutated copy compiles. Use the suite's compile-without-running command for `--build`.68- Prove a behavior-preserving swap by driving both implementations through the real entry point and diffing every observable; the source diff alone cannot prove equivalence.69- Copy the printed `mutation: … stability: …` line into your artifact's `summary`; that field and `qa_metadata` are the only carriers read as your own measurement.70- Mutation-pass + any stability-fail is a concurrency-sensitive finding, never a pass. A survived mutant means the test is not evidence.