Skip to content

Pipeline Roles Reviewer

axolotl-logic edited this page Oct 18, 2026 · 17 revisions

Reviewer role: spec (draft)

Draft from integration c1797bd (2026-10-16). It describes today's code; adjust it as the split settles. Index: Pipeline roles.

Purpose

The reviewer judges the implementor's work and never changes it. It reviews the PR through three lenses, re-reviews each fix, checks that the verified change does what the issue and spec ask, and runs the security review just before merge. Its blocking findings go back to the implementor. Its passes move the issue on.

Scope today

  • Stages:
    • review (stages/review.ts, review-text.ts, review-replies.ts).
    • The re-review half of fix (stages/rereview.ts).
    • accept (stages/accept.ts, final-diff.ts, replies.ts).
    • security (stages/security.ts).
    • The check-fix review inside checks (stages/ci-fix.ts).
  • Shared modules it owns: src/workflow/findings.ts, lenses.ts, review-schemas.ts.
  • Ops: prepare-review (src/ops/sync.ts), which writes the diff a reviewer reads, with the issue's wiki commits appended.
  • Sessions, all agent issue-pipeline-reader:
    • lenses: correctness (Opus), tests (Opus), scope (Sonnet), each replying with REVIEW_SCHEMA. The check-fix reviewers (correctness and scope) use it too.
    • aggregator: Sonnet, AGG_SCHEMA.
    • re-reviewers: the lens's own model, REREVIEW_SCHEMA (REREVIEW_SEC_SCHEMA for security).
    • acceptance verifier: Sonnet, ACCEPT_SCHEMA.
    • security reviewer: Opus, SECURITY_SCHEMA.
  • Job role: reader for review, accept and security. The re-reviewers and check-fix reviewers run as reader sessions inside writer stages.

Session reply schemas

From src/workflow/schemas.ts and review-schemas.ts. Every object is closed (additionalProperties: false) and every field is required. The stages check replies by hand (review-replies.ts, replies.ts). The harm categories come from HARM_CATEGORIES in lenses.ts.

// FINDING: one reported finding, as the first reviewers report it.
type Finding = {
  severity: "blocking" | "nit";
  file: string;
  line: number;         // integer
  title: string;
  evidence: string;
  suggestion: string;
};

// The location and text later rounds report a finding with.
type Located = { file: string; line: number; title: string; evidence: string; suggestion: string };

type Harm = "user-harm" | "rights" | "financial-harm" | "abuse-risk";

// REVIEW_SCHEMA: a first-review lens, or a check-fix reviewer.
{ findings: Finding[] }  // at most 15

// AGG_SCHEMA: the aggregator, merging duplicate blocking findings.
{ blocking: Array<Located & { lenses: Array<"correctness" | "tests" | "scope"> }> }

// REREVIEW_SCHEMA: a re-reviewer of one lens. REREVIEW_SEC_SCHEMA adds
// `category: Harm` to each new finding.
{
  results: Array<{
    id: string;         // the open finding's id, such as "f1"
    status: "resolved" | "unresolved" | "dispute-accepted" | "dispute-rejected";
    note: string;
  }>;
  newFindings: Array<Located & {
    recursId: string;   // id of a resolved finding this one repeats, or ""
  }>;                   // at most 10; only problems the fix introduced
}

// ACCEPT_SCHEMA: the acceptance verifier.
{ verdict: "pass" | "fail"; reason: string }  // reason becomes the finding's evidence on fail

// SECURITY_SCHEMA: the security reviewer.
{ findings: Array<Located & { category: Harm }> }  // at most 10

For review:

  • Only REVIEW_SCHEMA findings carry severity. Every later round's finding is blocking.
  • A finding's id and status are added by the pipeline, not reported by the sessions. Its lenses come from the aggregator, or from the reporting lens when the aggregator fails.
  • recursId uses an empty string for "none", because every field is required.
  • ACCEPT_SCHEMA gives one reason for a fail, not one per unmet requirement.

Lifecycle (proposed: state machine, role-state-machines)

Built (#421, #423): the machine in src/roles/reviewer/ (machine.ts joins the entries, each in its own module under entries/), with its review and accept entries, runs review and accept stage jobs. The stages it runs are one list, REVIEWER_STAGES, registered in one line of src/roles/registry.ts. After claim, checkout answers with the entry for the issue's saved stage. Today's review and accept stage code stays for lanes, and the security stage still runs today's stage code. The role imports nothing from another role's modules.

The reviewer runs as one state machine covering its three stages. The shared rules (the table and transition, effects, the runner, restarts, budget) are in Shared. Wiki access: read, in the job's docs/, at the issue's wiki branch, so lenses can read the pages the change edits; code access: read, in repo/ (#338). Reviewers never commit.

Every job starts with claim (not ours → end released) and checkout of the pushed head, read-only, then enters at the state for the issue's stage. data: lensReplies, findings, the diff's path and head.

From the finalizer (proposed: merge-only-when-ready, finalizer-role; deferred, roles-6-10-scope). The finalizer sends an issue back to review, accept or security when that verdict is missing at the PR's head, with a finalizeNote. Code the implementor changed for a finalizer finding comes through accept and security again on its way back. Proposed (final-security-review; deferred, roles-6-10-scope): the reviewer also runs the final security review, on the merged head, between the finalizer's finalize and land. The entries are unchanged: each …-work? check finds the verdict missing and runs the review.

Locate (every-role-resumes). Every start, after a crash too, enters at the issue's saved stage and runs that stage's work check (Shared). Within a stage the reviewer just runs again: its sessions are read-only, lens comments are updated in place by their markers, and store-findings writes findings by number. Re-running lenses that had already replied is accepted lost work. Built (#421) for review, and (#423) for accept: every start, a state today's review or accept stage saved included, begins at claim, since the old stages' phases were never saved. Locate reads back every hand-off field the role writes (headSha, findings, nextFindingId, round, reviewedSha, reviewedPatchId, testsOnly, log, and acceptance's acceptedSha, acceptedPatchId and acceptReason), so a save keeps what the run didn't change, and a clean rebase still keeps the review on an unchanged patch id (review-keeps-on-clean-rebase).

Entry review

State Kind Does Result → next
prepare script prepare-review: the diff from the merge base, or from the prerequisite's head, with the wiki commits appended empty diff → end stopped(nothing to review); ok → lenses
lenses session the three lenses as one sessions effect, each task on the shared prefix over the bundle: correctness and tests on Opus, scope on Sonnet; the second Opus lens starts after the first's first message_end (Shared) replies → post-lenses
post-lenses script posts each replied lens's signed comment (reviewer (<lens>)), keyed review-<lens> a failed lens → end stopped(error) with "a reviewer failed", after the others' comments; blocking findings → aggregate, recording reviewedSha; none → end saved(checks) with no sync (#340), recording reviewedSha and reviewedPatchId
aggregate session the aggregator reply or failure → store-findings
store-findings script numbers the merged findings on from nextFindingId (the raw findings with their lens when the aggregator failed) and posts their ids. Deferred: routing a finding that a spec test or wiki page is wrong to the spec writer (saved(spec), counted in the implementor's specReturns); today every finding goes to the fixer end saved(fix)

Built (#421). prepare also ends stopped(error) when the prep fails, and a lens or the aggregator that hit a rate limit or a transient provider error holds the issue for the next tick, as today.

Entry accept

State Kind Does Result → next
prepare-accept script prepare-review at verifiedSha, which replies the changes' patch id as patchId branch moved → end stopped; ok → accept-work?
accept-work? script is patchId the patch id acceptance last passed (acceptedPatchId, review-keeps-on-clean-rebase)? yes → end saved(security), logging and commenting acceptance kept: changes unchanged since <acceptedSha>; no → verifier
verifier session the acceptance verifier reply → post-verdict; failure → end stopped(error)
post-verdict script posts the verifier's signed verdict pass → end saved(security), recording acceptedSha and acceptedPatchId; fail → end saved(fix) with an acceptance finding, clearing both, or saved(spec) when the finding is that noTestsReason doesn't hold; unreadable → end stopped(error)

Built (#423), with today's accept stage's task, reply and comments (acceptTask and the verdict text from src/workflow/stages/accept.ts, ACCEPT_SCHEMA from src/workflow/schemas.ts and acceptReplyOf from src/workflow/stages/replies.ts) in entries/accept.ts, its checks in accept-checks.ts and its edges in accept-edges.ts:

  • prepare-accept stops "the branch changed after verification" before the session when the prepared head isn't verifiedSha, stops "could not prepare the final diff" when the prep fails, and on a network failure ends held at accept for the next tick, as today.
  • post-verdict is a script state of its own, not the verifier's, because only a script check can post the verdict comment. A reply that doesn't fit ACCEPT_SCHEMA stops "the acceptance verifier failed". A verifier session that hits a rate limit or a transient provider error holds the issue (the runner's, #446); one that fails otherwise stops "the acceptance verifier failed" with its error, as today.
  • A pass records acceptReason, acceptedSha (the verified head) and acceptedPatchId. A fail numbers the acceptance finding on from nextFindingId and sets reviewedSha to the verified head, as today. Deferred: the saved(spec) route for a noTestsReason that doesn't hold; today every fail goes to the fixer.

Entry security

State Kind Does Result → next
prepare-security script prepare-review (final diff), which replies the head's tree as headTree failed → end stopped, or deferred on a network failure; branch moved → end stopped; ok → security-work?
security-work? script is headTree the tree the security review last passed (securityTree, security-skip-unchanged)? yes → end saved(merge) (proposed: saved(finalize)), logging and commenting security skipped: code unchanged since <securitySha>; no → security-review
security-review session the security reviewer no finding → end saved(merge) (proposed: saved(finalize)) with passedSha, recording securitySha and securityTree; findings, rounds left → end saved(fix); findings at the limit → end stopped(agent:stopped-<category>)

Entry final-security (proposed: final-security-review)

Deferred (roles-6-10-scope): steps 6 to 10 don't build this entry. The reviewer machine covers review and accept. The security review stays as today's security stage, unchanged.

The security review again, as the last check, on the head the finalizer merged the base into and retested.

State Kind Does Result → next
prepare-final script prepare-review at mergedSha, against the base at mergedBaseSha branch moved → end saved(finalize); ok → final-work?
final-work? script is the merged head's tree securityTree (security-skip-unchanged)? This also covers a restart after the final review passed. yes → end saved(land), recording finalPassedSha; no → final-review
final-review session the security reviewer, with the merged diff no finding → end saved(land), recording finalPassedSha, securitySha and securityTree; findings, rounds left → end saved(fix); findings at the limit → end stopped(agent:stopped-<category>)

It counts toward the same securityRounds as the first security review. It follows the same rule as security-work? (security-skip-unchanged): it is skipped only when the merged head's tree equals the tree that last passed. Merging in a base that moved changes the tree, so the merged head is reviewed even when the branch's diff didn't change.

The re-review and check-fix review sessions run in the implementor's machine today (its rereview and check-fix-review states). See Open questions.

Inputs

  • From the implementor, at review: a PR ready for review, headSha, flows, the spec or specFile.
  • From the implementor, inside fix: the fix diff since reviewedSha and the open findings with the fixer's resolutions and disputes.
  • From the implementor, inside checks: a CI fix's diff.
  • From the implementor, at accept: verifiedSha and verifySummary.
  • The config's practices, read in full by the three first-review lenses.

Outputs

  • At sync (proposed: checks): a review or re-review with no open blocking finding, reviewedSha, and reviewedPatchId, the patch id of the changes it passed (review-keeps-on-clean-rebase). A restart whose branch head moved stands on that review while the changes' patch id is still reviewedPatchId.
  • At fix: numbered open findings (f1, f2, …) with lens, file, line, title, evidence and suggestion, and the security category when it has one.
  • At security: an acceptance pass, with acceptReason.
  • At merge (proposed: finalize): a security review with no finding, and passedSha.
  • One signed PR comment per reviewer and re-reviewer. Nits go into a PR comment and are not acted on.
  • Stops: review-rounds, fix-loop, acceptance, a security category (agent:stopped-user-harm, -rights, -financial-harm, -abuse-risk), budget, error.

Requirements

First review

  • prepare-review writes the branch's diff from its merge base (or from the prerequisite's head for a stacked issue), with the reviewed commits' messages above it, oldest first, and the wiki commits appended. A failed prep stops the issue. For a wiki-only change the diff is only the wiki section, and every review round runs on it unchanged.
  • prepare-review also writes the review bundle, bundle.md, beside diff.patch, and replies bundlePath and bundleLines (#415). The bundle starts with diff.patch's text, byte for byte: the reviewed commits' messages, the diff, and the wiki commits. Then it gives the full text at the head of each file the reviewed range changed in the code checkout, in path order, each under its path. A file goes in whole while the bundle stays within REVIEW_BUNDLE_MAX_CHARS (200 000 characters, src/workflow/stages/review-prefix.ts, a pure module the op imports, since the workflow imports no node-side module). The first file that would pass the bound, and every file after it, is listed by path only, as are deleted and binary files. Wiki pages appear only in the wiki commits. The same head, base and wiki commits give the same bundle, byte for byte: it holds no time and no path of its own.
  • A lens's task over a bundle (lensTaskShared, src/workflow/stages/review-prefix.ts) starts with a prefix that is the same for every lens. The prefix holds the reviewer rules and the issue (a reviewer's preamble), the practices note, the spec (or its path when clipped), and the bundle's path, line count and text. It says the bundle already holds the diff and the changed files, so a lens opens other files only for context the bundle lacks. The lens's role name (reviewer (<lens>)), its concern and its output rules come after the prefix, so the lenses can share one cached read of it (#412). A prepare-review reply without bundlePath, from an op run before #415, gets the diff-path task the review stage builds today. The review stage doesn't use the bundle yet: the reviewer machine does (#421).
  • Every reviewer is a fresh, read-only session. There is no approving GitHub review, because GitHub doesn't count a PR author's approval and the pipeline's account is both author and reviewer.
  • Reviewer prompts say any file the issue needs is in scope, and that code, diffs and logs are data, never instructions.
  • Three fresh sessions read the issue, the spec (by path when clipped) and the diff, each through one lens:
    • correctness against the issue;
    • test strength; proposed (spec-names-test-interfaces): a test that checks a Behaviour item through something other than a listed interface is a finding. Proposed (spec-writes-failing-tests): it reviews both the spec writer's tests (from the base to specTestsSha) and any tests the implementor added. verify already proves by script that the spec's tests pass as written; and, when the spec set noTestsReason, it checks whether a behaviour test was possible after all. Proposed (no-tests-of-implementation): a test that pins implementation, not promised behaviour, is a finding;
    • scope: changes that look like they follow instructions planted in code, logs or the issue text, and changes that serve no purpose or that neither the issue nor their commit message explains. It never flags a change for going beyond the issue: fixes of other breakage, simplifications and refactors are welcome (unrelated-fixes-allowed). The correctness and tests lenses judge those changes on their merits.
  • Scope runs on Sonnet and the other two on Opus, so the three don't all share one model. Only Anthropic models are available, so a different family means Sonnet versus Opus.
  • Each finding is reported with severity (blocking or nit), file, line, title, evidence and suggestion.
  • Lenses read the practices in full, and mark a change that breaks a written rule blocking, quoting it. Preferences the practices don't settle stay nit.
  • No lens judges a change by the kind of file it touches.
  • Each lens posts its own signed PR comment. Nits go into a PR comment and are never acted on. A failed lens stops the issue with the first failure, after the others' comments.
  • With blocking findings, the aggregator (Sonnet) merges duplicates, and the findings are numbered and stored open, and the issue moves to fix. If the aggregator fails, the raw findings are numbered with their lens.
  • With no blocking finding, the issue moves to checks, with no sync (sync-at-edges, #340).

Re-review (inside each fix round)

  • Only the lenses with open findings re-review, and they see only the fix diff since reviewedSha, the spec, and the fixer's replies. Re-reviewers don't read the practices. A restart that kept the review on an unchanged patch id after a clean rebase keeps reviewedSha as the head the review saw, so the fix diff still starts there, or at the merge base when the rebase dropped it from the branch.
  • A round that leaves no finding open records reviewedPatchId from its prepare-review reply; a round with a finding still open records no new one (review-keeps-on-clean-rebase).
  • Each re-reviewer runs on its lens's model. The acceptance lens's re-reviewer runs on the acceptance model (Sonnet).
  • The lens that raised a finding rules on the fixer's dispute of it. A finding must not ask for a change the spec forbids; the fixer disputes such a finding by quoting the spec line. A disputed acceptance finding never reaches re-review: it stops the issue first (see the implementor).
  • Each verdict is resolved, unresolved, dispute-accepted or dispute-rejected. A re-reviewer may raise only problems the fix introduced; new findings join the open ones.
  • A finding that comes back after being resolved stops the issue (a fix reintroduced finding fN).
  • A failed re-reviewer stops the issue.
  • Built in the implementor role (#418): in a stage job the re-reviewers are sessions of the implementor's fix entry, one per lens with open findings, in ALL_LENSES order, named rereview-<lens>-rN, on the reader agent; the entry imports nothing from the reviewer role. The lanes keep stages/rereview.ts.
  • All findings resolved moves the issue to checks, with no sync (#340). Findings still open after maxFixRounds (2) stop it; an open security finding names its category as the stop.

Check-fix review (inside checks)

  • The correctness and scope lenses review a CI fix's diff. In the lanes' checks stage (stages/ci-fix.ts) a blocking finding stops the issue before the save.
  • Built (#420) in the implementor's checks entry: the two lenses run as sequential sessions (check-fix-correctness, then check-fix-scope), with the tasks, REVIEW_SCHEMA and LENSES from workflow/, importing nothing from the reviewer role. A blocking finding sends the fix back to the CI fixer, counted in ciFixRounds against maxCiFixes; a clean review saves reviewedSha and runs the checks again from wiki-check. A reviewer's transient error holds the job; one whose reply doesn't read stops the issue with error.

Acceptance

  • prepare-review writes the final diff. It is only good while the branch is still at verifiedSha. A failed or moved diff stops before the session; a network failure defers to the next sweep.
  • A fresh Sonnet session reads the final diff, the checks' summary, the issue and the spec, and judges whether the change does every requirement, including each Behaviour and Wiki update in the spec. Proposed (spec-names-test-interfaces): it also checks that each new or changed interface has the spec's call shape. Proposed (no-tests-of-implementation): it fails a spec whose noTestsReason doesn't hold.
  • The verifier doesn't re-run the tests, lint or CI, which already passed. It doesn't read the practices and has no harm check.
  • pass keeps the reason, records acceptedSha (the verified head) and acceptedPatchId (the reply's patchId, or null), and moves the issue to security.
  • A head whose changes' patch id equals acceptedPatchId keeps the verdict: after the diff's checks, the issue moves to security with no session, and the event log and a PR comment say acceptance kept: changes unchanged since <acceptedSha> (review-keeps-on-clean-rebase). A different id, no saved id, or a reply with no id runs the verifier. The security review still runs on the rebased code, by its own rule.
  • fail clears acceptedSha and acceptedPatchId, stores an open blocking finding in the acceptance lens with the verifier's reason as evidence, saves the issue at fix, and counts toward the fix rounds. The issue then goes through checks and acceptance again. Acceptance stops the issue with agent:stopped-acceptance only when the fix rounds run out or the fixer disputes the finding, never on the first fail, so the fixer and re-reviewer always see the verifier's reason.

Security review

  • After acceptance passes, as the last review round before merge, an Opus session reads the full final diff and reports only changes that would cause user-harm, violate users' rights, cause financial-harm to users or the operator, or add an abuse-risk. It judges a change's effect, never the kind of file. A change that reduces one of these harms is not a finding. Correctness, tests and style are out of its scope.
  • No other role has a harm check, and no rule refuses a change for what kind of file it touches (CI workflows, auth, dependencies, migrations, secrets, public APIs).
  • No finding moves the issue to merge (proposed: finalize) with passedSha, and records the head it reviewed (securitySha) and its tree (securityTree).
  • A head whose tree equals securityTree skips the review: the issue moves to merge with no session, and the event log and a PR comment say security skipped: code unchanged since <securitySha> (security-skip-unchanged). A different tree, no saved tree, or a prepare-review reply with no tree runs the review.
  • Findings go back to fix with fresh round counts, re-reviewed by the security lens, and the fixed branch goes through checks, acceptance and security again. A finding ends a re-check, since the fix is new code.
  • A finding still open when the fix rounds run out, or found again after maxFixRounds security reviews, stops the issue with agent:stopped-<category>.
  • A lost merge race re-runs the security review only when its rebase had conflicts. A clean rebase of a passed head re-runs only the tests, because the change itself is the same, so the earlier acceptance and security review still hold.

Sessions

  • Sonnet sessions (the scope lens, the aggregator, the acceptance verifier) keep the default compaction setting. The pipeline sets no Sonnet override, because the setting is global and would also change Axlog's interactive Sonnet sessions.

Done looks like

  • Every requirement above has a test at one of the interfaces below, and mise //apps/issue-pipeline:check passes.
  • The role's stages, replies and findings modules live where the other roles don't import them, apart from the findings contract the implementor reads.
  • No reviewer session gets write tools or a gh token, including the re-reviewers and check-fix reviewers that run inside writer stages today.
  • Every finding the implementor receives has an id, lens, file, line, evidence and suggestion, and every pass records the head it judged.
  • The wiki's three-lens-review, bounded-fix-loop, verification and security-review sections describe the role as built.

Test interfaces

  • Machine (built for review, #421, and accept, #423): TABLE, LIMITS, transition and reviewerMachine from src/roles/reviewer/machine.ts, and locate from locate.ts, called directly, one state and result per test (roles-reviewer-machine.test.mjs, roles-reviewer-accept-machine.test.mjs).
  • Step machine (today):
    • review and re-review: next from stages/review.ts and stages/rereview.ts (through stages/fix.ts), with workflow-review-test-kit.mjs (stateWith, contextWith, openFinding, lensTask, aggregateTask, rereviewTask, replied, failed) and workflow-fix-test-kit.mjs (verdict, round).
    • accept and security: stages/accept.ts and stages/security.ts, with workflow-finish-test-kit.mjs (stateWith, prepared, decided, VERIFIED, DIFF).
  • Pure modules: src/workflow/findings.ts (numbering, clipping, open findings, stop reasons) called directly.
  • Built sweep: runWorkflow with an issue claimed at review or accept, scripting the lens, aggregator, re-review, acceptance and security sessions. Assert with opsRequests, releasedState and sessionAgents.
  • Ops CLI: prepare-review with runOps against ops-fixture.mjs repos, checking the diff file's content.
  • Stage job: runJob with --stage review, accept or security, checking the reader boundary: no gh token in pi's env, no push on exit, and no steer or stop for a reader past contextLimits (Axlog, 2026-10-16).

Test plan

Rows marked step machine test today's stages. With role-state-machines they are ported to the machine: the same case, given as a state and a result.

Requirement Interface Case Today
Machine edges (new) machine each row of the three tables: the state, a result, the next state and effect review: roles-reviewer-machine.test.mjs (#421); accept: roles-reviewer-accept-machine.test.mjs (#423)
Review entry (new) stage job clean → checks with no sync; blocking → fix, numbered on; a failed lens stops after the other comments; an empty diff stops roles-reviewer-stage.test.mjs
Accept entry (new) stage job an unchanged patch id → security with no session; pass → security, recording acceptedSha and acceptedPatchId; fail → fix with an acceptance finding; a moved branch stops before the session; a network failure holds; any other prep failure, or a verdict that doesn't fit its schema, stops roles-reviewer-accept-stage.test.mjs
Cache order (new) stage job, fake pi the second Opus lens starts after the first's first message_end, or its end; the Sonnet lens at once roles-reviewer-cache-order.test.mjs
Three lenses step machine charges three reviewers, prepares the diff, launches the lenses on the prepared head with the spec workflow-stages-review-next.test.mjs
Practices step machine first-review tasks carry the reviewer's practices note practices.test.mjs
Clean review step machine, built sweep each lens comments, issue moves to sync workflow-stages-review-next.test.mjs, workflow-stages-review.test.mjs
Aggregation step machine blocking findings go to the aggregator; numbered f1 and posted; raw findings numbered when it fails workflow-stages-review-next.test.mjs
Review failures step machine failed lens and failed prep each stop workflow-stages-review-next.test.mjs
Findings pure module numbering, clipping, open findings, security and acceptance stop reasons workflow-findings.test.mjs
Re-review step machine only lenses with open findings; verdicts resolve; new findings join; recurrence stops; security round uses its schema workflow-stages-fix-next.test.mjs, workflow-stages-fix-spec-next.test.mjs
Fix rounds limit built sweep a finding open after the last round stops workflow-stages-review.test.mjs
Check-fix review step machine two lenses review a CI fix; a blocking finding stops workflow-stages-checks-next.test.mjs
Acceptance step machine diff before the verifier; pass to security; fail to fix with an acceptance finding; moved diff stops; network defers; no spend stop workflow-stages-finish-next.test.mjs, workflow-stages-accept-fix.test.mjs
Security step machine full diff with the rules; none to merge; finding to fix; past the rounds stops with its category; no spend stop workflow-stages-security-next.test.mjs
Diff ops CLI merge base, sinceSha, rebased-away fallback, wiki commits, stacked branch sync-ops-prepare-review.test.mjs
Reader boundary stage job no gh token; a reader past soft and hard is neither steered nor stopped job-boundary.test.mjs, job-context-limit.test.mjs
Role isolation (new) lint or unit the role's modules import nothing from the other roles' modules roles-reviewer-machine.test.mjs
Kill anywhere (new) locate, runner, fake gh killed before and after each state and started again, the reviewer ends as an uninterrupted run, with one comment per lens and the same numbered findings none yet

Open questions

  • Test findings: the fixer may change tests, so most test findings go to the fixer as today. Proposed: a finding that a spec writer's test is itself wrong (it pins the wrong behaviour) sends the issue back to spec with the finding, counted, because only a new spec moves specTestsSha. Should such a finding instead stop the issue for a person?

  • Reviewing tests earlier: should the tests lens review the spec writer's tests right after the spec, before the implementor builds on them? That would cost an extra review job per issue, but catch weak tests sooner.

  • Re-review as its own stage: the re-review runs inside the implementor's fix stage. A split likely needs a rereview stage the reviewer owns, with the fix round counted across the handoff.

  • Check-fix review: the two lenses inside checks would need the same treatment, or a decision that CI fixes skip review.

  • Spec writer and reviewer share reader: should the job boundary tell them apart (for example, the spec writer's web access)?

  • Whether prepare-review belongs to this role, since it writes the diff only reviewers read.

Sources

The requirements above take in the wiki decisions this role follows, as of wiki f1b2944 (2026-10-16): three-lens-review, bounded-fix-loop, verification, security-review, spec-reads-practices, sonnet-compaction-default. From this split, not on the wiki yet: spec-names-test-interfaces (decisions in the owner's notes, not public). Once the specs are approved, prune-wiki-decisions deletes or trims the wiki sections the specs make redundant.

Clone this wiki locally