// review-contract.ts — THE REVIEWER REPORT CONTRACT (KEY: AUTOPILOT-108). // // WHAT CHANGED AND WHY. The review contract used to imply the reviewer should // re-run the worker's acceptance suite. Evidence from one 2026-09-15 batch says // that is usually impossible AND mostly beside the point: three different // environmental refusals (deps-less worktree, read-only scope, a permission // layer refusing `bun run`), each reviewer improvising a DIFFERENT substitute — // and every substantive finding came from a DIFFERENT method (a Python // re-implementation of a drift check, a byte-for-byte projection compare, a // retained-policy dry-run, per-file typecheck error-site arithmetic, an importer // grep). So the reviewer's job is redefined, NOT weakened: // // (i) AUDIT the worker's acceptance artifact (src/acceptance-evidence.ts): // is it from the reviewed sha, does the stated ENVIRONMENT explain the // numbers, are the load-bearing claims independently derivable? // (ii) RE-DERIVE cheap static facts by a DIFFERENT method where possible // (deleted ranges, symbol liveness, spec-pointer survival, typecheck // error-site arithmetic, byte-comparison against the base). // // and then report a COVERAGE STATEMENT — a REQUIRED field naming every // load-bearing claim as exactly one of three statuses. "The worker already ran // the tests" is NOT a sufficient basis for a verdict unless the evidence has // been audited; a claim with no artifact can only be COULD NOT CHECK. The // coverage statement cannot be omitted silently (validateReviewReport fails it), // so this is a change in WHAT verification means, not permission to stop. // // THE TERRITORY RULE (KEY: AUTOPILOT-114). AUTOPILOT-108 fixed WHAT the // reviewer verifies; it left the PASS/FAIL line itself unprincipled, and the // default drifted to PASS. Reviewers kept producing real findings about THE // DELIVERABLE UNDER REVIEW — a doc claim it rendered false, a status member it // half-rolled-out, a test whose title asserted the opposite of the truth — and // returned PASS with "non-blocking" attached, pushing the fix into a follow-up // that may never be dispatched. The missing line is WHOSE TERRITORY THE FINDING // IS IN: // // FAIL — the finding contradicts THIS deliverable's OWN claims, acceptance, // docs or tests: the thing this item changed is internally // inconsistent, or its own documentation now describes behaviour it // does not have. The worker who built it has the full context and can // fix it now; a follow-up item does not, and often never arrives. The // DOCUMENTATION CASE is the sharpest test (a doc/comment/README/test // title the deliverable ADDED or RENDERED FALSE is a FAIL, never a // note), and a PARTIAL ROLLOUT of something the item INTRODUCED is a // FAIL (the deliverable is INCOMPLETE). // NOTE — the finding is about PRE-EXISTING code, another item's territory, or // a genuine improvement outside this item's scope: PASS is correct. A // serious NEARBY concern the reviewer judges out of scope is a NOTE // plus a FILED/filing-worthy call, never a FAIL. // // A FAIL must be SCOPED (the anti-loop guardrail): it names the defects AND // says plainly what is correct and must NOT be rebuilt; a FAIL that implies // "redo everything" is a contract violation. So the report must state which // findings drove the verdict, give EVERY finding a FAIL/NOTE disposition, and // on a FAIL state what must not be rebuilt — ALL required fields, enforced by // validateReviewReport exactly as the coverage statement is. import { ACCEPTANCE_EVIDENCE_START, ACCEPTANCE_EVIDENCE_END } from "./acceptance-evidence.ts"; import { matchVerdictLine, type Verdict } from "./verdict.ts"; /** The three coverage statuses, one spelling shared by the prompt and the * parser. A load-bearing claim must carry exactly one of these. */ export const COVERAGE_STATUSES = [ "VERIFIED INDEPENDENTLY", "ACCEPTED FROM WORKER'S EVIDENCE", "COULD NOT CHECK", ] as const; export type CoverageStatus = (typeof COVERAGE_STATUSES)[number]; export interface CoverageEntry { /** The load-bearing claim this entry is about. */ claim: string; status: CoverageStatus; } export interface CoverageScan { /** true when a coverage section heading was found at all. */ sawHeading: boolean; /** The well-formed `- : ` entries. */ entries: CoverageEntry[]; /** Bullet lines under the heading whose status was not one of the three. */ malformed: string[]; } /** A coverage-section heading: `## Coverage statement`, `**Coverage:**`, etc. */ const COVERAGE_HEADING_RE = /^\s*(?:#{1,6}\s+)?[*_`]*\s*coverage(?:\s+statement)?\b[^\w]*$/i; /** A well-formed coverage bullet: `- : `. The status match is * restricted to the three statuses (any case, optional trailing period). */ const COVERAGE_BULLET_RE = /^\s*[-*+]\s+(.+?)\s*:\s*(VERIFIED\s+INDEPENDENTLY|ACCEPTED\s+FROM\s+(?:THE\s+)?WORKER'?S\s+EVIDENCE|COULD\s+NOT\s+CHECK)\b.*$/i; function normalizeStatus(raw: string): CoverageStatus { const s = raw.toUpperCase().replace(/\s+/g, " ").trim(); if (s.startsWith("VERIFIED")) return "VERIFIED INDEPENDENTLY"; if (s.startsWith("ACCEPTED")) return "ACCEPTED FROM WORKER'S EVIDENCE"; return "COULD NOT CHECK"; } /** Scan a reviewer body for its coverage statement. Tolerant of heading shape * and case (the reviewer's own wording varies); the STATUS vocabulary is exact. */ export function scanCoverageStatement(text: string): CoverageScan { const lines = (text ?? "").split(/\r?\n/); let sawHeading = false; let inCoverage = false; const entries: CoverageEntry[] = []; const malformed: string[] = []; for (const line of lines) { if (COVERAGE_HEADING_RE.test(line)) { sawHeading = true; inCoverage = true; continue; } if (!inCoverage) continue; if (/^\s*(?:#{1,6}\s+|\*\*[^*].*?\*\*\s*:?\s*$)/.test(line)) { inCoverage = false; // a new heading ends the section continue; } const m = line.match(COVERAGE_BULLET_RE); if (m) { entries.push({ claim: m[1].trim(), status: normalizeStatus(m[2]) }); continue; } if (/^\s*[-*+]\s+\S/.test(line)) malformed.push(line.trim()); } return { sawHeading, entries, malformed }; } /** The coverage entries a reviewer reported (empty when none are well-formed). */ export function parseCoverageStatement(text: string): CoverageEntry[] { return scanCoverageStatement(text).entries; } // --------------------------------------------------------------------------- // The territory rule's required fields: verdict reason, finding dispositions, // and the scoped-FAIL guardrail (KEY: AUTOPILOT-114). // --------------------------------------------------------------------------- /** The two dispositions a finding can carry, one spelling each. A finding in * THIS deliverable's territory is a FAIL; anything else is a NOTE. */ export const FINDING_DISPOSITIONS = ["FAIL", "NOTE"] as const; export type FindingDisposition = (typeof FINDING_DISPOSITIONS)[number]; export interface FindingDispositionEntry { /** The finding this entry is about. */ finding: string; /** FAIL (in this deliverable's territory) or NOTE (out of it). */ disposition: FindingDisposition; /** Why — required for a NOTE (why it is NOT FAIL-worthy / out of territory). */ reason: string; } export interface FindingsScan { /** true when a findings section heading was found at all. */ sawHeading: boolean; /** The well-formed `- : FAIL|NOTE — ` entries. */ entries: FindingDispositionEntry[]; /** Bullet lines under the heading with no FAIL/NOTE disposition. */ undispositioned: string[]; /** NOTE entries carrying no justification. */ notesWithoutReason: FindingDispositionEntry[]; /** Non-bullet lines (the explicit `None` when there are no findings). */ prose: string[]; } /** A findings section heading: `## Findings`, `**Findings:**`, etc. */ const FINDINGS_HEADING_RE = /^\s*(?:#{1,6}\s+)?[*_`]*\s*findings?\b[^\w]*$/i; /** A verdict-reason heading: `## Verdict reason`, `**Verdict reason:**`, etc. */ const VERDICT_REASON_HEADING_RE = /^\s*(?:#{1,6}\s+)?[*_`]*\s*verdict\s+reason\b[^\w]*$/i; /** The scoped-FAIL guardrail heading: `## Do not rebuild`, `**Do not rebuild:**`. */ const DO_NOT_REBUILD_HEADING_RE = /^\s*(?:#{1,6}\s+)?[*_`]*\s*do\s+not\s+rebuild\b[^\w]*$/i; /** A well-formed disposition bullet: `- : FAIL|NOTE — `. The * finding text may itself contain colons; the disposition is the first * `: FAIL`/`: NOTE` token and the reason is everything after it. */ const DISPOSITION_BULLET_RE = /^\s*[-*+]\s+(.+?)\s*:\s*(FAIL|NOTE)\b\s*(?:[—–:-]+\s*)?(.*)$/i; /** Any section heading — markdown `#`..`######` + text, or a bold-only line. * Bounds a required section's body (the next heading ends it). */ const SECTION_HEADING_RE = /^\s*(?:#{1,6}\s+\S|\*\*[^*].*?\*\*\s*:?\s*$)/; /** The non-empty lines under the FIRST heading matching `headingRe`, bounded by * the next section heading (or the end of the report). null when no such * heading appears anywhere. */ function sectionBody(text: string, headingRe: RegExp): string[] | null { const lines = (text ?? "").split(/\r?\n/); let body: string[] | null = null; for (const line of lines) { if (headingRe.test(line)) { if (body) break; // a SECOND match is the next section — the first ended body = []; continue; } if (body === null) continue; if (SECTION_HEADING_RE.test(line)) break; const trimmed = line.trim(); if (trimmed) body.push(trimmed); } return body; } /** Scan a reviewer body for its findings + dispositions (AUTOPILOT-114). * Tolerant of heading shape; the FAIL/NOTE vocabulary is exact. */ export function scanFindings(text: string): FindingsScan { const body = sectionBody(text, FINDINGS_HEADING_RE); if (body === null) { return { sawHeading: false, entries: [], undispositioned: [], notesWithoutReason: [], prose: [] }; } const entries: FindingDispositionEntry[] = []; const undispositioned: string[] = []; const prose: string[] = []; for (const line of body) { if (/^\s*[-*+]\s+\S/.test(line)) { const m = line.match(DISPOSITION_BULLET_RE); if (!m) { undispositioned.push(line); continue; } entries.push({ finding: m[1].trim(), disposition: m[2].toUpperCase() === "FAIL" ? "FAIL" : "NOTE", reason: (m[3] ?? "").trim(), }); } else { prose.push(line); } } return { sawHeading: true, entries, undispositioned, notesWithoutReason: entries.filter((e) => e.disposition === "NOTE" && !e.reason), prose, }; } /** The report's own verdict value (the FIRST line matching the grammar), used * to make the scoped-FAIL guardrail conditional. null when there is none. */ function reportVerdict(text: string): Verdict | null { for (const line of (text ?? "").split(/\r?\n/)) { const v = matchVerdictLine(line); if (v) return v; } return null; } /** * Validate a reviewer report against the contract. Returns the deficiencies — * EMPTY means the report carries every required field. The load-bearing * requirements are non-omit-ability: the AUTOPILOT-108 coverage statement, the * AUTOPILOT-114 verdict reason, a FAIL/NOTE disposition (with a justification) * for every finding, and — on a FAIL — the "do not rebuild" statement. A * report that omits any of them FAILS here (recorded as a review-contract gap, * never silently accepted). */ export function validateReviewReport(text: string): string[] { const source = text ?? ""; const deficiencies: string[] = []; // (0) The AUTOPILOT-108 coverage statement — checked FIRST so a report that // omits everything is named by its oldest requirement first. const { sawHeading, entries, malformed } = scanCoverageStatement(source); if (!sawHeading) { deficiencies.push( "missing the required COVERAGE STATEMENT — report EVERY load-bearing claim as " + "`- : VERIFIED INDEPENDENTLY`, `- : ACCEPTED FROM WORKER'S EVIDENCE`, or `- : COULD NOT CHECK` " + "(a claim with no acceptance artifact can only be COULD NOT CHECK)", ); } else if (!entries.length) { deficiencies.push( "the COVERAGE STATEMENT carries no well-formed entries — one `- : ` bullet per load-bearing claim", ); } for (const bad of malformed) { deficiencies.push( `coverage entry not understood (status must be one of ${COVERAGE_STATUSES.join(" | ")}): "${bad}"`, ); } // (1) The VERDICT REASON (AUTOPILOT-114): which findings drove the verdict, // and — for each note — why it is not FAIL-worthy. const reason = sectionBody(source, VERDICT_REASON_HEADING_RE); if (reason === null) { deficiencies.push( "missing the required VERDICT REASON — state which finding(s) drove the verdict (and, for a PASS, why each finding below is a NOTE and not a FAIL)", ); } else if (!reason.length) { deficiencies.push("the VERDICT REASON section is empty — name the finding(s) that drove the verdict"); } // (2) EVERY finding dispositioned FAIL or NOTE; every NOTE justified (why it // is out of this deliverable's territory, i.e. not FAIL-worthy). const findings = scanFindings(source); if (!findings.sawHeading) { deficiencies.push( "missing the required FINDINGS section — list EVERY finding as `- : FAIL — ` or `- : NOTE — `, or write `None` when there are no findings", ); } else { for (const bad of findings.undispositioned) { deficiencies.push( `finding not dispositioned as ${FINDING_DISPOSITIONS.join(" or ")} — say explicitly which kind each finding is: "${bad}"`, ); } for (const note of findings.notesWithoutReason) { deficiencies.push( `NOTE finding carries no justification — say why it is NOT FAIL-worthy (out of this deliverable's territory): "${note.finding}"`, ); } const statedNone = findings.prose.some((line) => /\bnone\b|\bno findings\b/i.test(line)); if (!findings.entries.length && !findings.undispositioned.length && !statedNone) { deficiencies.push( "the FINDINGS section lists no findings — give each finding a FAIL/NOTE disposition, or write `None` when there are none", ); } } // (3) The scoped-FAIL guardrail (AUTOPILOT-114): a FAIL must say what is // correct and must NOT be rebuilt, so the redo stays cheap and precise. if (reportVerdict(source) === "FAIL") { const doNotRebuild = sectionBody(source, DO_NOT_REBUILD_HEADING_RE); if (doNotRebuild === null) { deficiencies.push( 'a FAIL must carry the required DO NOT REBUILD section — name what is already correct and must NOT be rebuilt or re-run (a FAIL that implies "redo everything" is a contract violation)', ); } else if (!doNotRebuild.length) { deficiencies.push("the DO NOT REBUILD section is empty — name what is correct and must NOT be rebuilt"); } } return deficiencies; } /** The evidence-and-coverage contract block appended to BOTH review-task * builders (reviewTask in framework/auto-dispatch.ts and queueReview's default * task), so the reviewer is told the same thing regardless of lane. */ export const REVIEW_CONTRACT_TEXT = [ "## Evidence & coverage (REQUIRED — KEY: AUTOPILOT-108)", "Do NOT re-run the worker's acceptance suite. In a deps-less worktree, under a read-only review scope, or behind a permission layer that refuses the runner you cannot run it — and re-running the SAME suite adds cost without independence. The worker's own evidence is the test artifact; your job is to make its limits VISIBLE:", " (i) AUDIT the worker's captured acceptance artifact (given below, between " + `\`${ACCEPTANCE_EVIDENCE_START}\` and \`${ACCEPTANCE_EVIDENCE_END}\`): is it from the sha you are reviewing, does the stated ENVIRONMENT explain the numbers, and is each load-bearing claim independently derivable from the diff/artifact?`, " (ii) RE-DERIVE cheap static facts by a DIFFERENT method where that is possible — deleted ranges, symbol liveness, spec-pointer survival, typecheck error-site arithmetic, a byte-comparison against the base — and cite what you found.", "Then write a `## Coverage statement` section listing EVERY load-bearing claim as exactly one of:", " - : VERIFIED INDEPENDENTLY", " - : ACCEPTED FROM WORKER'S EVIDENCE", " - : COULD NOT CHECK", 'The coverage statement is REQUIRED — a report that omits it FAILS validation and is recorded as a review-contract gap. "The worker already ran the tests" is NOT a sufficient basis for a verdict unless the artifact above has been audited; a claim with no artifact can only be COULD NOT CHECK.', "", "## Verdict, findings and the TERRITORY RULE (REQUIRED — KEY: AUTOPILOT-114)", "Decide the verdict by WHOSE TERRITORY THE FINDING IS IN — not by how severe it feels, and never by pushing a fix into a follow-up item that may never be dispatched:", " - FAIL when the finding contradicts THIS deliverable's OWN claims, acceptance, docs or tests — the thing this item changed is internally inconsistent, so it is INCOMPLETE. The worker who built it has the full context and can fix it NOW; a follow-up item does not, and often never arrives.", " - NOTE when the finding is about PRE-EXISTING code, another item's territory, or a genuine improvement outside this item's scope — PASS is correct, and `- : NOTE — ` says so explicitly.", "The DOCUMENTATION CASE is the SHARPEST test: a doc, comment, README or TEST TITLE that the deliverable ADDED, or RENDERED FALSE (it now describes behaviour the deliverable does not have), is a FAIL — never a note. A PARTIAL ROLLOUT of something the item INTRODUCED is a FAIL: if the item adds a state/member/flag and some of the code that must understand it does not, the deliverable is INCOMPLETE.", 'A FAIL must be SCOPED (the anti-loop guardrail): name the defects AND say plainly what is correct and must NOT be rebuilt or re-run. A FAIL that implies "redo everything" is a CONTRACT VIOLATION — the redo is cheap and precise only when the fixed parts are named. Nitpicks, style and genuinely pre-existing defects are NOTES; if the deliverable is correct but a NEARBY concern is serious, that is a NOTE plus a SAY-SO THAT IT IS FILING-WORTHY, not a FAIL.', "Every report therefore carries these REQUIRED fields (validateReviewReport fails a report that omits them, exactly as the coverage statement is enforced):", " - `## Verdict reason` — which finding(s) drove the verdict, and, for each NOTE, why it is not FAIL-worthy (i.e. why it is out of this deliverable's territory).", " - `## Findings` — EVERY finding as exactly one of:", " - : FAIL — ", " - : NOTE — ", " or `None` when there are no findings.", " - `## Do not rebuild` — REQUIRED on a FAIL: what is already correct and must NOT be rebuilt or re-run.", ].join("\n");