From 016ab802350c110bd94b8718411f292fc730756b Mon Sep 17 00:00:00 2001
From: Henry Lach <henry@lach.ca>
Date: Wed, 16 Sep 2026 00:32:38 -0400
Subject: [PATCH 1/3] feat(review): worker self-check gate before review_step;
 reviewer round semantics (#657)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review cycles dominated wall time (penster TP-1921: ~52 min authoring vs
~117 min review; every gate to the 2-round cap; round-2 findings were the
worker's own Step 0 invariants).

- review_step (code/test) refuses unless STATUS.md carries a
  '### Self-check (Step N)' section AFTER the step's last checked box with
  at least one data row (selfCheckRefusal). Opt-out:
  taskRunner.worker.requireSelfCheck=false → TASKPLANE_REQUIRE_SELF_CHECK=0.
- Every review request states its round (reviewRoundForGate: 1 + prior
  files for the gate) and, from round 2, the prior review path. Round 1 is
  exhaustive; round ≥ 2 verifies the fold and raises NEW blocking findings
  only at the top severity label (taskRunner.reviewer.round2NewFindings
  'p0-only' default | 'any' → TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS).
  Code-review requests also point the reviewer at the self-check table.
- Templates: task-worker.md gains the self-check step (3) in the Order of
  Operations; task-reviewer.md gains 'Round semantics'; reviewer-extension
  promptGuidelines carry the round line.
- Config: schema/types/loader/env for both keys; task-runner config doc.

Tests: review-self-check.test.ts (10): refusal matrix (missing / empty /
stale-before-last-checkbox / step-scoped), round counting per gate, round
lines both modes, config→env, wiring, templates. 4132 pass.
---
 CHANGELOG.md                                  |   4 +
 .../configuration/task-runner.yaml.md         |   2 +
 extensions/reviewer-extension.ts              |   1 +
 .../taskplane/agent-bridge-extension.ts       | 141 +++++++++++++++
 extensions/taskplane/config-loader.ts         |   6 +
 extensions/taskplane/config-schema.ts         |  16 ++
 extensions/taskplane/engine.ts                |   2 +
 extensions/taskplane/execution.ts             |   9 +
 extensions/taskplane/types.ts                 |   4 +
 extensions/tests/review-self-check.test.ts    | 161 ++++++++++++++++++
 .../worker-step-completion-protocol.test.ts   |   8 +-
 templates/agents/task-reviewer.md             |  21 +++
 templates/agents/task-worker.md               |  28 ++-
 13 files changed, 394 insertions(+), 9 deletions(-)
 create mode 100644 extensions/tests/review-self-check.test.ts

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c5231e9..e16e644f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### New
+
+- **Worker self-check before review; reviewer round semantics** (#657). Review cycles, not authoring, dominated task wall time (penster TP-1921: ~52 min authoring vs ~117 min in review cycles; every gate went to the 2-round cap, and the round-2 findings were invariants the worker had written in its own Step 0 design). Two base-template changes: (1) `review_step` (code/test) now **refuses** unless STATUS.md carries a `### Self-check (Step N)` table placed after the step's checkboxes — one row per outcome / Step 0 design decision / Completion Criterion (and prior-review finding on round ≥ 2) with `file:line` evidence; opt out with `taskRunner.worker.requireSelfCheck: false`. (2) Every review request states its **round** and, from round 2, the prior review path: round 1 is exhaustive, round ≥ 2 verifies the fold and raises new blocking findings only at the top severity label (`taskRunner.reviewer.round2NewFindings`: `"p0-only"` default | `"any"`), everything else → Suggestions + APPROVE. Worker/reviewer templates and the reviewer tool guidance carry the contract.
+
 ### Fixed
 
 - **A released hold left the task badged `held` and let its lane successor show `running` with the wrong telemetry** (#651). Three writers: (1) the strict hold-store write that persists a release (or an open) now projects the bound task's record in the same write — open → `held`, released → `running` with the stale hold-timeout reason cleared — instead of waiting for the next unrelated task-transition persist; (2) resume reconciliation decides "session alive" per **task** from the registry manifests' task ids, so a not-yet-started successor sharing the lane's session name stays `pending` (legacy registries without task ids keep the old session semantics); (3) the dashboard attaches a lane's live worker telemetry to the task named by the lane snapshot — including a row still badged `held` — never to a sibling that merely has `status: running`.
diff --git a/docs/reference/configuration/task-runner.yaml.md b/docs/reference/configuration/task-runner.yaml.md
index 355f2b0f..e4f97f66 100644
--- a/docs/reference/configuration/task-runner.yaml.md
+++ b/docs/reference/configuration/task-runner.yaml.md
@@ -88,6 +88,7 @@ If a task path matches a configured task area, that area's override applies.
 | `worker.spawn_mode` | `"subprocess"` \| `"tmux"` | commented in template | Optional spawn mode override for task-runner. |
 | `worker.exit_intercept_timeout_sec` | number | `60` (15..1800) | How long the lane waits for a supervisor reply when it intercepts a worker's premature exit before letting the session close. Raise it when the supervisor is often inside long tool calls (a blocking `--wait` cannot answer in 60 s). In `.pi/taskplane-config.json` (the preferred project config) the key is camelCase: `taskRunner.worker.exitInterceptTimeoutSec`. |
 | `worker.hold_timeout_minutes` | number | `240` (5..10080) | How long a lane may stay **held** (worker escalated, waiting for a typed ruling, no worker process running) before the batch parks with pause cause `hold-timeout`. Acknowledgements never extend it; the hold stays open when it expires — rule, then `orch_resume(force=true)`. JSON key: `taskRunner.worker.holdTimeoutMinutes`. |
+| `worker.require_self_check` | boolean | `true` | `review_step` (code/test reviews) refuses unless STATUS.md has a `### Self-check (Step N)` table for the step, placed after its checkboxes: one row per outcome / Step 0 design decision / Completion Criterion (and, on round ≥ 2, per prior finding) with `file:line` evidence. Most round-2 findings in the field were invariants the worker had already written down. Set `false` to opt out. JSON key: `taskRunner.worker.requireSelfCheck`. |
 
 Notes:
 - `spawn_mode` defaults to `subprocess` when not set.
@@ -101,6 +102,7 @@ Notes:
 | `reviewer.model` | string | `""` | Reviewer model (empty = inherit session model). |
 | `reviewer.tools` | string | `"read,write,bash,grep,find,ls"` | Tool allowlist for reviewer agent. |
 | `reviewer.thinking` | string | `"off"` | Thinking mode for reviewer. Empty string (or explicit `"inherit"`) = inherit active session thinking. |
+| `reviewer.round2_new_findings` | `"p0-only"` \| `"any"` | `"p0-only"` | Round semantics for a gate that already has a REVISE. Round 1 is exhaustive; round ≥ 2 verifies the fold, and a *new* finding is blocking only at the top severity label (`p0-only`) or at any severity (`any`) — everything else goes to Suggestions with an APPROVE verdict. The review request states the round and the prior review path. JSON key: `taskRunner.reviewer.round2NewFindings`. |
 
 ### `context`
 
diff --git a/extensions/reviewer-extension.ts b/extensions/reviewer-extension.ts
index 404233cc..d3265140 100644
--- a/extensions/reviewer-extension.ts
+++ b/extensions/reviewer-extension.ts
@@ -55,6 +55,7 @@ export default function reviewerExtension(pi: ExtensionAPI) {
 			"After writing your review to the specified output file, call wait_for_review() again.",
 			"When it returns 'SHUTDOWN', exit cleanly — the task is complete.",
 			"Reference your previous reviews when relevant (e.g., 'I flagged X in Step 1 — checking if addressed').",
+			"Round semantics (#657): each review request states its round. Round 1 is EXHAUSTIVE — list every finding now. Round ≥ 2 VERIFIES THE FOLD against the prior review; raise a new finding as blocking only at the top severity label unless the request says otherwise — everything else is a Suggestion and the verdict is APPROVE.",
 		],
 		parameters: Type.Object({}),
 		async execute() {
diff --git a/extensions/taskplane/agent-bridge-extension.ts b/extensions/taskplane/agent-bridge-extension.ts
index cf3be030..74e68264 100644
--- a/extensions/taskplane/agent-bridge-extension.ts
+++ b/extensions/taskplane/agent-bridge-extension.ts
@@ -216,6 +216,104 @@ export function hasOutstandingNonApproveReview(
 	}
 }
 
+/**
+ * #657: the worker's pre-review self-check. A `### Self-check (Step N)` section
+ * (heading level 3–4, case-insensitive) must exist in STATUS.md, contain at
+ * least one table data row or list item, and appear AFTER the step's last
+ * checked checkbox in the file — i.e. it was written once the step's work was
+ * done, not pasted in early. Returns null when satisfied, else the refusal.
+ */
+export function selfCheckRefusal(statusContent: string, stepNum: number): string | null {
+	const text = statusContent.replace(/\r\n/g, "\n");
+	const headingRe = new RegExp(
+		"^#{3,4}\\s+Self-check\\s*\\(\\s*Step\\s+" + String(stepNum) + "\\s*\\)\\s*$",
+		"im",
+	);
+	const m = headingRe.exec(text);
+	if (!m) {
+		return `no "### Self-check (Step ${stepNum})" section in STATUS.md`;
+	}
+	const sectionStart = m.index + m[0].length;
+	const rel = text.slice(sectionStart).search(/^#{1,4}\s/m);
+	const body = text.slice(sectionStart, rel === -1 ? undefined : sectionStart + rel);
+	const rows = body
+		.split("\n")
+		.filter((l) => /^\s*(\|.*\||[-*]\s+\S)/.test(l) && !/^\s*\|?\s*:?-{3,}/.test(l));
+	const dataRows = rows.filter((l) => !/^\s*\|\s*(item|check|criterion|finding|what)\b/i.test(l));
+	if (dataRows.length === 0) {
+		return `"### Self-check (Step ${stepNum})" section has no rows — list each outcome / design decision / completion criterion with file:line evidence`;
+	}
+	// Freshness: the section must come after the step's last checked box.
+	const stepHeadRe = new RegExp("^###\\s+Step\\s+" + String(stepNum) + "\\b", "m");
+	const sh = stepHeadRe.exec(text);
+	if (sh) {
+		const stepStart = sh.index;
+		const stepEndRel = text.slice(stepStart + 1).search(/^###\s+Step\s+\d+/m);
+		const stepBody = text.slice(
+			stepStart,
+			stepEndRel === -1 ? undefined : stepStart + 1 + stepEndRel,
+		);
+		let lastChecked = -1;
+		const cb = /^\s*-\s*\[x\]/gim;
+		let cm: RegExpExecArray | null = cb.exec(stepBody);
+		while (cm !== null) {
+			lastChecked = stepStart + cm.index;
+			cm = cb.exec(stepBody);
+		}
+		if (lastChecked !== -1 && m.index < lastChecked) {
+			return `"### Self-check (Step ${stepNum})" appears before the step's last checked box — re-run the self-check after finishing the step's work`;
+		}
+	}
+	return null;
+}
+
+/** #657: round number for this gate = 1 + number of existing review files for it. */
+export function reviewRoundForGate(
+	reviewsDir: string,
+	reviewType: string,
+	stepNum: number,
+): { round: number; priorReviewPath: string | null } {
+	try {
+		if (!existsSync(reviewsDir)) return { round: 1, priorReviewPath: null };
+		const suffix = `-${reviewType.toLowerCase()}-step${stepNum}.md`;
+		const files = readdirSync(reviewsDir)
+			.filter((f) => /^R\d{3}-/.test(f) && f.toLowerCase().endsWith(suffix))
+			.sort();
+		return {
+			round: files.length + 1,
+			priorReviewPath: files.length > 0 ? join(reviewsDir, files[files.length - 1]) : null,
+		};
+	} catch {
+		return { round: 1, priorReviewPath: null };
+	}
+}
+
+/** #657: lines appended to the reviewer request describing the round's scope. */
+export function buildReviewRoundLines(
+	round: number,
+	priorReviewPath: string | null,
+	round2Mode: "p0-only" | "any",
+): string[] {
+	if (round === 1) {
+		return [
+			"## Review round: 1 of 2 (exhaustive)",
+			"This is the FIRST review of this gate. List EVERY finding now, with severity. A REVISE that " +
+				"withholds findings for a later round is a defect: the gate has a hard 2-round cap and round 2 " +
+				"is verify-only.",
+		];
+	}
+	return [
+		`## Review round: ${round} (verify the fold)`,
+		`Prior review for this gate: \`${priorReviewPath ?? "(unknown)"}\``,
+		"Scope: verify that each finding in the prior review is addressed (cite file:line). " +
+			(round2Mode === "any"
+				? "New findings may be raised at any severity."
+				: "Raise a NEW finding as blocking ONLY at the top severity label (P0 / critical). " +
+					'Every other new observation goes in "### Suggestions" and does NOT change the verdict.'),
+		"If all prior findings are addressed and no top-severity finding remains, the verdict is APPROVE.",
+	];
+}
+
 export function isStepMarkedComplete(statusPath: string, stepNum: number): boolean {
 	let content: string;
 	try {
@@ -899,6 +997,44 @@ export default function (pi: ExtensionAPI) {
 				/* default 0 */
 			}
 
+			// ── #657: self-check gate (code/test reviews) ─────────────────
+			// The worker must have diffed its draft against its own design, the
+			// step's outcomes and the Completion Criteria before asking for a
+			// review. Opt-out: taskRunner.worker.requireSelfCheck = false.
+			if (reviewType !== "plan" && process.env.TASKPLANE_REQUIRE_SELF_CHECK !== "0") {
+				let statusForCheck = "";
+				try {
+					statusForCheck = readFileSync(statusPath, "utf-8");
+				} catch {
+					/* fall through — refusal below names the missing section */
+				}
+				const refusal = selfCheckRefusal(statusForCheck, stepNum);
+				if (refusal) {
+					return {
+						content: [
+							{
+								type: "text" as const,
+								text:
+									`⛔ review_step refused — self-check missing: ${refusal}.\n\n` +
+									`Before requesting a code review, add a "### Self-check (Step ${stepNum})" section to STATUS.md ` +
+									`(directly under the step, AFTER its checkboxes) with one row per: (a) step checkbox/outcome, ` +
+									`(b) each Step 0 design decision this step implements, (c) each PROMPT Completion Criterion it ` +
+									`touches, and — on round ≥ 2 — (d) each finding from the prior review file. Each row carries ` +
+									`file:line evidence and OK / "fixed in <hash>". Fix anything not OK FIRST, then call review_step ` +
+									`again. Most round-2 findings in the field were invariants the worker had already written down.`,
+							},
+						],
+						details: undefined,
+					};
+				}
+			}
+
+			// #657: round semantics for the reviewer prompt
+			const roundInfo = reviewRoundForGate(reviewsDir, reviewType, stepNum);
+			const round2Mode: "p0-only" | "any" =
+				process.env.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS === "any" ? "any" : "p0-only";
+			const roundLines = buildReviewRoundLines(roundInfo.round, roundInfo.priorReviewPath, round2Mode);
+
 			reviewCounter++;
 			const num = String(reviewCounter).padStart(3, "0");
 			const outputPath = join(reviewsDir, `R${num}-${reviewType}-step${stepNum}.md`);
@@ -945,6 +1081,8 @@ export default function (pi: ExtensionAPI) {
 					`2. Read STATUS.md for progress so far`,
 					`3. Evaluate the plan for this step`,
 					``,
+					...roundLines,
+					``,
 					`## Output`,
 					`Write your review to: \`${outputPath}\``,
 				].join("\n");
@@ -965,6 +1103,9 @@ export default function (pi: ExtensionAPI) {
 					`1. Run \`${diffNamesCmd}\` to see changed files`,
 					`2. Run \`${diffCmd}\` for the full diff`,
 					`3. Read changed files for context`,
+					`4. Read the worker's "### Self-check (Step ${stepNum})" table in STATUS.md — verify its evidence rather than re-deriving it`,
+					``,
+					...roundLines,
 					``,
 					`## Output`,
 					`Write your review to: \`${outputPath}\``,
diff --git a/extensions/taskplane/config-loader.ts b/extensions/taskplane/config-loader.ts
index ff343e3e..6f9dc224 100644
--- a/extensions/taskplane/config-loader.ts
+++ b/extensions/taskplane/config-loader.ts
@@ -1274,6 +1274,9 @@ export function toTaskRunnerConfig(config: TaskplaneConfig): import("./types.ts"
 			...(typeof config.taskRunner.worker.holdTimeoutMinutes === "number"
 				? { holdTimeoutMinutes: config.taskRunner.worker.holdTimeoutMinutes }
 				: {}),
+			...(typeof config.taskRunner.worker.requireSelfCheck === "boolean"
+				? { requireSelfCheck: config.taskRunner.worker.requireSelfCheck }
+				: {}),
 		},
 		model_fallback: config.taskRunner.modelFallback ?? "inherit",
 		reviewer: {
@@ -1281,6 +1284,9 @@ export function toTaskRunnerConfig(config: TaskplaneConfig): import("./types.ts"
 			thinking: config.taskRunner.reviewer.thinking,
 			tools: config.taskRunner.reviewer.tools,
 			excludeExtensions: [...(config.taskRunner.reviewer.excludeExtensions ?? [])],
+			...(config.taskRunner.reviewer.round2NewFindings
+				? { round2NewFindings: config.taskRunner.reviewer.round2NewFindings }
+				: {}),
 			...(config.taskRunner.reviewer.severityLabels
 				? { severityLabels: [...config.taskRunner.reviewer.severityLabels] }
 				: {}),
diff --git a/extensions/taskplane/config-schema.ts b/extensions/taskplane/config-schema.ts
index 95ed55f2..96aee177 100644
--- a/extensions/taskplane/config-schema.ts
+++ b/extensions/taskplane/config-schema.ts
@@ -132,6 +132,15 @@ export interface WorkerConfig {
 	 * bounded to 5..10080 (7 days).
 	 */
 	holdTimeoutMinutes?: number;
+	/**
+	 * #657: `review_step` refuses when STATUS.md has no `### Self-check (Step N)`
+	 * section for the step (or the section is older than the step's last
+	 * checkbox flip). The worker must diff its draft against its own design,
+	 * the step's outcomes and the Completion Criteria BEFORE asking for a
+	 * review — most round-2 findings in the field were invariants the worker
+	 * had written down itself. Default true.
+	 */
+	requireSelfCheck?: boolean;
 }
 
 /** Reviewer agent configuration */
@@ -170,6 +179,13 @@ export interface ReviewerConfig {
 	severityLabels: string[];
 	/** Revision-spiral detection tuning. */
 	spiral: ReviewSpiralConfig;
+	/**
+	 * #657: round semantics for a gate that already has a REVISE. Round 1 is
+	 * exhaustive; round 2 verifies the fold. New findings in round ≥ 2 are
+	 * blocking only at the top severity label ("p0-only", default) or always
+	 * ("any"). Everything else goes to Suggestions with an APPROVE verdict.
+	 */
+	round2NewFindings?: "p0-only" | "any";
 }
 
 /** Context/resource limits for task execution */
diff --git a/extensions/taskplane/engine.ts b/extensions/taskplane/engine.ts
index 5c26f6c4..e95914af 100644
--- a/extensions/taskplane/engine.ts
+++ b/extensions/taskplane/engine.ts
@@ -2447,6 +2447,7 @@ async function attemptStaleWorktreeRecovery(
 			excludeExtensions: runnerConfig?.reviewer?.excludeExtensions ?? [],
 			severityLabels: runnerConfig?.reviewer?.severityLabels,
 			spiral: runnerConfig?.reviewer?.spiral,
+			round2NewFindings: runnerConfig?.reviewer?.round2NewFindings,
 		},
 		runnerConfig?.worker
 			? {
@@ -3241,6 +3242,7 @@ export async function executeOrchBatch(
 				excludeExtensions: runnerConfig?.reviewer?.excludeExtensions ?? [],
 				severityLabels: runnerConfig?.reviewer?.severityLabels,
 				spiral: runnerConfig?.reviewer?.spiral,
+				round2NewFindings: runnerConfig?.reviewer?.round2NewFindings,
 			},
 			runnerConfig?.worker
 				? {
diff --git a/extensions/taskplane/execution.ts b/extensions/taskplane/execution.ts
index 1065456b..ea754a5b 100644
--- a/extensions/taskplane/execution.ts
+++ b/extensions/taskplane/execution.ts
@@ -1991,6 +1991,7 @@ export async function executeWave(
 		excludeExtensions?: string[];
 		severityLabels?: string[];
 		spiral?: import("./config-schema.ts").ReviewSpiralConfig;
+		round2NewFindings?: "p0-only" | "any";
 	},
 	workerConfig?: {
 		model?: string;
@@ -2880,6 +2881,7 @@ export function buildReviewerEnv(
 		excludeExtensions?: string[];
 		severityLabels?: string[];
 		spiral?: import("./config-schema.ts").ReviewSpiralConfig;
+		round2NewFindings?: "p0-only" | "any";
 	} | null,
 ): Record<string, string> {
 	const env: Record<string, string> = {};
@@ -2898,6 +2900,9 @@ export function buildReviewerEnv(
 		analysis.severityLabels = reviewerConfig.severityLabels;
 	}
 	if (reviewerConfig?.spiral) analysis.spiral = reviewerConfig.spiral;
+	// #657: round semantics for the reviewer prompt
+	env.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS =
+		reviewerConfig?.round2NewFindings === "any" ? "any" : "p0-only";
 	if (Object.keys(analysis).length > 0) {
 		env.TASKPLANE_REVIEW_ANALYSIS = JSON.stringify(analysis);
 	}
@@ -2920,6 +2925,7 @@ export function buildWorkerEnv(
 		excludeExtensions?: string[];
 		exitInterceptTimeoutSec?: number;
 		holdTimeoutMinutes?: number;
+		requireSelfCheck?: boolean;
 	} | null,
 ): Record<string, string> {
 	const env: Record<string, string> = {};
@@ -2942,6 +2948,8 @@ export function buildWorkerEnv(
 			normalizeHoldTimeoutMinutes(workerConfig.holdTimeoutMinutes),
 		);
 	}
+	// #657: opt-out only; the default (unset) is "required".
+	if (workerConfig?.requireSelfCheck === false) env.TASKPLANE_REQUIRE_SELF_CHECK = "0";
 
 	return env;
 }
@@ -3089,6 +3097,7 @@ export async function executeLaneV2(
 				const parsed = JSON.parse(extraEnvVars.TASKPLANE_REVIEW_ANALYSIS) as {
 					severityLabels?: string[];
 					spiral?: import("./config-schema.ts").ReviewSpiralConfig;
+					round2NewFindings?: "p0-only" | "any";
 				};
 				if (Array.isArray(parsed.severityLabels)) reviewSeverityLabels = parsed.severityLabels;
 				if (parsed.spiral && typeof parsed.spiral === "object") reviewSpiral = parsed.spiral;
diff --git a/extensions/taskplane/types.ts b/extensions/taskplane/types.ts
index abf8f01f..1769a521 100644
--- a/extensions/taskplane/types.ts
+++ b/extensions/taskplane/types.ts
@@ -371,6 +371,8 @@ export interface TaskRunnerConfig {
 		excludeExtensions?: string[];
 		/** Ordered severity vocabulary for review finding-count analysis (review-boundary notifications). */
 		severityLabels?: string[];
+		/** #657: blocking scope for NEW findings in round ≥ 2. Default "p0-only". */
+		round2NewFindings?: "p0-only" | "any";
 		/** Revision-spiral detection tuning. */
 		spiral?: import("./config-schema.ts").ReviewSpiralConfig;
 	};
@@ -392,6 +394,8 @@ export interface TaskRunnerConfig {
 		exitInterceptTimeoutSec?: number;
 		/** Minutes a hold may stay open before the batch parks (`hold-timeout`). Default 240; 5..10080. @since #627 */
 		holdTimeoutMinutes?: number;
+		/** #657: review_step requires a fresh `### Self-check (Step N)` section. Default true. */
+		requireSelfCheck?: boolean;
 	};
 	/** Worker agent extension exclusion list. @since TP-180 */
 	workerExcludeExtensions?: string[];
diff --git a/extensions/tests/review-self-check.test.ts b/extensions/tests/review-self-check.test.ts
new file mode 100644
index 00000000..48ec118f
--- /dev/null
+++ b/extensions/tests/review-self-check.test.ts
@@ -0,0 +1,161 @@
+/**
+ * #657 — worker self-check gate before review_step; reviewer round semantics.
+ */
+
+import { afterEach, beforeEach, describe, it } from "node:test";
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+import { expect } from "./expect.ts";
+import {
+	buildReviewRoundLines,
+	reviewRoundForGate,
+	selfCheckRefusal,
+} from "../taskplane/agent-bridge-extension.ts";
+import { buildReviewerEnv, buildWorkerEnv } from "../taskplane/execution.ts";
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const readSrc = (rel: string) => readFileSync(join(HERE, "..", rel), "utf-8");
+
+const STATUS_NO_CHECK = `# TP-1 — Status
+
+### Step 0: Design
+**Status:** ✅ Complete
+- [x] Design table written
+
+### Step 1: Implement
+**Status:** 🟨 In Progress
+- [x] Do the thing
+- [x] Test the thing
+
+### Step 2: Docs
+**Status:** ⬜ Not Started
+- [ ] Update docs
+`;
+
+const SELF_CHECK = `
+### Self-check (Step 1)
+
+| Item | Evidence | Result |
+|------|----------|--------|
+| Do the thing | src/thing.ts:12-40 | OK |
+| Design: fail-closed on missing id | src/thing.ts:22 | fixed in abc1234 |
+| Completion criterion: tests pass | tests/thing.test.ts | OK |
+`;
+
+describe("#657 — selfCheckRefusal", () => {
+	it("refuses when the section is missing, names the step", () => {
+		const r = selfCheckRefusal(STATUS_NO_CHECK, 1);
+		expect(r).toContain('no "### Self-check (Step 1)" section');
+	});
+
+	it("accepts a section with data rows placed after the step's checkboxes; heading level 4 and case-insensitive spelling also accepted", () => {
+		const s = STATUS_NO_CHECK.replace("### Step 2: Docs", `${SELF_CHECK}\n### Step 2: Docs`);
+		expect(selfCheckRefusal(s, 1)).toBe(null);
+		const h4 = s.replace("### Self-check (Step 1)", "#### self-CHECK (step 1)");
+		expect(selfCheckRefusal(h4, 1)).toBe(null);
+	});
+
+	it("refuses an empty section (header + separator only)", () => {
+		const empty = STATUS_NO_CHECK.replace(
+			"### Step 2: Docs",
+			"### Self-check (Step 1)\n\n| Item | Evidence | Result |\n|------|----------|--------|\n\n### Step 2: Docs",
+		);
+		expect(selfCheckRefusal(empty, 1)).toContain("has no rows");
+	});
+
+	it("refuses a stale section that sits BEFORE the step's last checked box (pasted in early)", () => {
+		const early = STATUS_NO_CHECK.replace(
+			"- [x] Do the thing\n- [x] Test the thing",
+			`- [x] Do the thing\n${SELF_CHECK}\n- [x] Test the thing`,
+		);
+		expect(selfCheckRefusal(early, 1)).toContain("appears before the step's last checked box");
+	});
+
+	it("is step-scoped: a self-check for step 1 does not satisfy step 2", () => {
+		const s = STATUS_NO_CHECK.replace("### Step 2: Docs", `${SELF_CHECK}\n### Step 2: Docs`);
+		expect(selfCheckRefusal(s, 2)).toContain("Step 2");
+	});
+});
+
+describe("#657 — round semantics", () => {
+	let dir: string;
+	beforeEach(() => {
+		dir = mkdtempSync(join(tmpdir(), "tp657-"));
+	});
+	afterEach(() => {
+		rmSync(dir, { recursive: true, force: true });
+	});
+
+	it("reviewRoundForGate counts prior files for THIS gate only and returns the latest as prior", () => {
+		const reviews = join(dir, ".reviews");
+		mkdirSync(reviews);
+		expect(reviewRoundForGate(reviews, "code", 3)).toEqual({ round: 1, priorReviewPath: null });
+		writeFileSync(join(reviews, "R001-plan-step3.md"), "Verdict: APPROVE");
+		writeFileSync(join(reviews, "R002-code-step3.md"), "Verdict: REVISE");
+		writeFileSync(join(reviews, "R003-code-step4.md"), "Verdict: REVISE");
+		const r = reviewRoundForGate(reviews, "code", 3);
+		expect(r.round).toBe(2);
+		expect(r.priorReviewPath).toBe(join(reviews, "R002-code-step3.md"));
+		expect(reviewRoundForGate(reviews, "plan", 3).round).toBe(2);
+		expect(reviewRoundForGate(join(dir, "missing"), "code", 1).round).toBe(1);
+	});
+
+	it("round-1 lines demand exhaustiveness; round-2 lines scope to the fold, name the prior file, and restrict new findings to P0 by default", () => {
+		const r1 = buildReviewRoundLines(1, null, "p0-only").join("\n");
+		expect(r1).toContain("Review round: 1 of 2 (exhaustive)");
+		expect(r1).toContain("List EVERY finding now");
+		const r2 = buildReviewRoundLines(2, "/x/.reviews/R002-code-step3.md", "p0-only").join("\n");
+		expect(r2).toContain("Review round: 2 (verify the fold)");
+		expect(r2).toContain("R002-code-step3.md");
+		expect(r2).toContain("ONLY at the top severity label");
+		expect(r2).toContain("the verdict is APPROVE");
+		const any = buildReviewRoundLines(2, "/x/p.md", "any").join("\n");
+		expect(any).toContain("any severity");
+		expect(any).not.toContain("ONLY at the top severity");
+	});
+
+	it("config → env: requireSelfCheck opt-out and round2NewFindings mode", () => {
+		expect(buildWorkerEnv({ requireSelfCheck: false }).TASKPLANE_REQUIRE_SELF_CHECK).toBe("0");
+		expect(buildWorkerEnv({}).TASKPLANE_REQUIRE_SELF_CHECK).toBe(undefined); // default = required
+		expect(buildReviewerEnv({}).TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS).toBe("p0-only");
+		expect(buildReviewerEnv({ round2NewFindings: "any" }).TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS).toBe(
+			"any",
+		);
+		expect(readSrc("taskplane/config-loader.ts")).toContain(
+			"requireSelfCheck: config.taskRunner.worker.requireSelfCheck",
+		);
+		expect(readSrc("taskplane/config-loader.ts")).toContain(
+			"round2NewFindings: config.taskRunner.reviewer.round2NewFindings",
+		);
+	});
+
+	it("review_step wiring: gate runs for code/test reviews before the counter increments, honours the opt-out, and both request prompts carry the round lines", () => {
+		const src = readSrc("taskplane/agent-bridge-extension.ts");
+		const gate = src.indexOf(
+			'reviewType !== "plan" && process.env.TASKPLANE_REQUIRE_SELF_CHECK !== "0"',
+		);
+		const counter = src.indexOf("reviewCounter++;");
+		expect(gate).toBeGreaterThan(-1);
+		expect(gate).toBeLessThan(counter);
+		expect(src).toContain("review_step refused — self-check missing");
+		expect((src.match(/\.\.\.roundLines,/g) ?? []).length).toBe(2);
+		expect(src).toContain(
+			"4. Read the worker's \"### Self-check (Step $" + '{stepNum})" table in STATUS.md',
+		);
+	});
+
+	it("templates carry the contract: worker self-check step in the review path; reviewer round rules; reviewer-extension guideline", () => {
+		const worker = readSrc("../templates/agents/task-worker.md");
+		expect(worker).toContain("3. **Self-check (MANDATORY — `review_step` refuses without it).**");
+		expect(worker).toContain("### Self-check (Step N)");
+		expect(worker).toContain('4. **Call** `review_step(step=N, type="code", baseline=<sha>)`.');
+		const reviewer = readSrc("../templates/agents/task-reviewer.md");
+		expect(reviewer).toContain("### Round semantics (the gate has a hard 2-round cap)");
+		expect(reviewer).toContain("**Round 1 — exhaustive.**");
+		expect(reviewer).toContain("**Round ≥ 2 — verify the fold.**");
+		expect(readSrc("reviewer-extension.ts")).toContain("Round semantics (#657)");
+	});
+});
diff --git a/extensions/tests/worker-step-completion-protocol.test.ts b/extensions/tests/worker-step-completion-protocol.test.ts
index e7c1b2b6..d27e91e5 100644
--- a/extensions/tests/worker-step-completion-protocol.test.ts
+++ b/extensions/tests/worker-step-completion-protocol.test.ts
@@ -43,9 +43,11 @@ describe("1.x — task-worker.md prompt: TP-186 sections", () => {
 		// handle REVISE, mark Complete on APPROVE, move on.
 		expect(WORKER_PROMPT).toContain("1. **Implement**");
 		expect(WORKER_PROMPT).toContain("2. **Commit**");
-		expect(WORKER_PROMPT).toContain('3. **Call** `review_step(step=N, type="code"');
-		expect(WORKER_PROMPT).toContain("5. If the verdict is **APPROVE**");
-		expect(WORKER_PROMPT).toContain("6. **Move to step N+1.**");
+		// #657: self-check is step 3; review_step moves to 4, APPROVE to 6, next step to 7.
+		expect(WORKER_PROMPT).toContain("3. **Self-check (MANDATORY");
+		expect(WORKER_PROMPT).toContain('4. **Call** `review_step(step=N, type="code"');
+		expect(WORKER_PROMPT).toContain("6. If the verdict is **APPROVE**");
+		expect(WORKER_PROMPT).toContain("7. **Move to step N+1.**");
 	});
 
 	it("1.2 — contains the Recovery Recipe with the keyword 'revert'", () => {
diff --git a/templates/agents/task-reviewer.md b/templates/agents/task-reviewer.md
index 8bd5358d..90cb78ff 100644
--- a/templates/agents/task-reviewer.md
+++ b/templates/agents/task-reviewer.md
@@ -143,6 +143,25 @@ high-signal for catching regressions the behavioural diff review would miss.
 - A critical edge case is unhandled and would cause runtime failure
 - Backward compatibility is broken without migration
 
+### Round semantics (the gate has a hard 2-round cap)
+
+Every review request tells you its **round** and, from round 2, the path of the
+prior review for this gate. The cap means a round-2 REVISE can only be closed
+by a supervisor ruling — so the two rounds have different jobs:
+
+- **Round 1 — exhaustive.** List EVERY finding now, each with a severity
+  label. Withholding a finding for a later round is a defect in your review,
+  not diligence.
+- **Round ≥ 2 — verify the fold.** Your scope is the prior review's findings:
+  for each, confirm it is addressed (cite `file:line`) or state precisely what
+  is still missing. Read the worker's `### Self-check (Step N)` table and
+  verify its evidence rather than re-deriving the review from scratch. A NEW
+  finding is blocking only at the **top severity label** (P0 / critical) unless
+  the request says new findings may be raised at any severity. Every other new
+  observation goes under `### Suggestions` and does not change the verdict.
+  If all prior findings are addressed and no top-severity finding remains, the
+  verdict is **APPROVE**.
+
 ### Do NOT issue REVISE for
 
 - Missing checkboxes for work that's already covered by a broader item
@@ -151,6 +170,8 @@ high-signal for catching regressions the behavioural diff review would miss.
 - "Re-run tests and record the result" — test runs are the worker's concern
 - "Check If Affected" docs that turn out to need no changes
 - Suggestions that improve quality but aren't required for correctness
+- In round ≥ 2: any new finding below the top severity label — that is a
+  Suggestion (see Round semantics)
 
 ## Plan Review Format
 
diff --git a/templates/agents/task-worker.md b/templates/agents/task-worker.md
index ae17a5b5..48e3195c 100644
--- a/templates/agents/task-worker.md
+++ b/templates/agents/task-worker.md
@@ -341,13 +341,29 @@ STATUS.md before the code review for that step has returned APPROVE.**
    step's `**Status:**` heading set to `🟨 In Progress`.
 2. **Commit** the implementation:
    `git add -A && git commit -m "feat(TASK-ID): step N implementation"`
-3. **Call** `review_step(step=N, type="code", baseline=<sha>)`.
-4. If the verdict is **REVISE**: read the review file in `.reviews/`, apply
-   the fixes, commit them, and call `review_step` again. Repeat until APPROVE
-   (max 2 code review cycles per step).
-5. If the verdict is **APPROVE**: NOW update the step's `**Status:**` heading
+3. **Self-check (MANDATORY — `review_step` refuses without it).** Before you
+   ask for a review, diff your work against what YOU already wrote down. Add a
+   `### Self-check (Step N)` section to STATUS.md directly under the step,
+   AFTER its checkboxes, as a table with one row per:
+   - (a) each checkbox / outcome of the step,
+   - (b) each Step 0 design decision this step implements (from your STATUS
+     design table — ordering invariants, fail-closed rules, interfaces),
+   - (c) each PROMPT `## Completion Criteria` item the step touches,
+   - (d) on round ≥ 2: each finding in the prior review file for this gate.
+   Each row: `| item | evidence file:line | OK / fixed in <hash> |`. Anything
+   not OK you fix FIRST, commit, then complete the table. This is not paperwork:
+   in the field, most round-2 findings were invariants the worker had itself
+   written in Step 0 and never re-read. Commit the self-check.
+4. **Call** `review_step(step=N, type="code", baseline=<sha>)`.
+5. If the verdict is **REVISE**: read the review file in `.reviews/`, apply
+   the fixes, commit them, **refresh the self-check** (add the (d) rows for the
+   review's findings, each with `fixed in <hash>`), and call `review_step`
+   again. Round 2 is verify-only for the reviewer: it checks your fold against
+   its round-1 findings and raises new blocking findings only at the top
+   severity. Max 2 code review cycles per step.
+6. If the verdict is **APPROVE**: NOW update the step's `**Status:**` heading
    to `✅ Complete` in STATUS.md and commit the status update.
-6. **Move to step N+1.**
+7. **Move to step N+1.**
 
 The key invariant: **`Status: ✅ Complete` is the worker's commitment that the
 reviewer has signed off on the step.** It is not an in-progress marker. Setting

From f08ffeb73e1664b4b89378a4d12d8ab311d42c36 Mon Sep 17 00:00:00 2001
From: Henry Lach <henry@lach.ca>
Date: Wed, 16 Sep 2026 00:42:47 -0400
Subject: [PATCH 2/3] =?UTF-8?q?fix(review):=20#657=20Sage=20round=201=20?=
 =?UTF-8?q?=E2=80=94=20defects=20always=20block=20in=20round=20=E2=89=A52;?=
 =?UTF-8?q?=20round=20counts=20only=20prior=20REVISE/RETHINK;=20settings?=
 =?UTF-8?q?=20reach=20the=20worker?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. Round ≥ 2 no longer downgrades genuine correctness defects: a regression
   introduced by the fold, a bug or an unmet requirement is BLOCKING at any
   severity label; only non-defect observations (hardening, style, extra
   tests) are subject to the p0-only/any scope. The unconditional '→ APPROVE'
   line is gone ('never manufacture an APPROVE'). Templates + reviewer tool
   guidance updated to match.
2. reviewRoundForGate counts only prior files whose verdict parses as
   REVISE/RETHINK — an APPROVE, an incomplete file or an unparseable verdict
   is not evidence of a completed exhaustive review and does not narrow scope.
3. Settings now reach the worker on every path: engine maps
   requireSelfCheck (+ exitInterceptTimeoutSec/holdTimeoutMinutes, which had
   the same gap) into the executeWave worker config at both sites; execution
   consumes TASKPLANE_REQUIRE_SELF_CHECK / TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS
   into LaneRunnerConfig; lane-runner forwards them into the worker env.
4. Refusal text starts with 'REFUSED:' so the disposition parser classifies
   it (not a review verdict); self-check data-row detection is structural
   (header = row above a |---| separator), not keyword-based.
Tests: round counting matrix, defect-blocks lines, end-to-end env chain;
review-step-guard-runtime fixture carries a self-check. 4134 pass.
---
 CHANGELOG.md                                  |  2 +-
 extensions/reviewer-extension.ts              |  2 +-
 .../taskplane/agent-bridge-extension.ts       | 61 +++++++++----
 extensions/taskplane/engine.ts                |  6 ++
 extensions/taskplane/execution.ts             |  7 ++
 extensions/taskplane/lane-runner.ts           |  7 ++
 extensions/tests/review-self-check.test.ts    | 85 +++++++++++++++++--
 .../tests/review-step-guard-runtime.test.ts   |  7 ++
 templates/agents/task-reviewer.md             | 23 +++--
 9 files changed, 164 insertions(+), 36 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e16e644f..c078fe77 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### New
 
-- **Worker self-check before review; reviewer round semantics** (#657). Review cycles, not authoring, dominated task wall time (penster TP-1921: ~52 min authoring vs ~117 min in review cycles; every gate went to the 2-round cap, and the round-2 findings were invariants the worker had written in its own Step 0 design). Two base-template changes: (1) `review_step` (code/test) now **refuses** unless STATUS.md carries a `### Self-check (Step N)` table placed after the step's checkboxes — one row per outcome / Step 0 design decision / Completion Criterion (and prior-review finding on round ≥ 2) with `file:line` evidence; opt out with `taskRunner.worker.requireSelfCheck: false`. (2) Every review request states its **round** and, from round 2, the prior review path: round 1 is exhaustive, round ≥ 2 verifies the fold and raises new blocking findings only at the top severity label (`taskRunner.reviewer.round2NewFindings`: `"p0-only"` default | `"any"`), everything else → Suggestions + APPROVE. Worker/reviewer templates and the reviewer tool guidance carry the contract.
+- **Worker self-check before review; reviewer round semantics** (#657). Review cycles, not authoring, dominated task wall time (penster TP-1921: ~52 min authoring vs ~117 min in review cycles; every gate went to the 2-round cap, and the round-2 findings were invariants the worker had written in its own Step 0 design). Two base-template changes: (1) `review_step` (code/test) now **refuses** unless STATUS.md carries a `### Self-check (Step N)` table placed after the step's checkboxes — one row per outcome / Step 0 design decision / Completion Criterion (and prior-review finding on round ≥ 2) with `file:line` evidence; opt out with `taskRunner.worker.requireSelfCheck: false`. (2) Every review request states its **round** and, from round 2, the prior review path: round 1 is exhaustive, round ≥ 2 verifies the fold; a new finding that is a genuine defect (regression from the fold, bug, unmet requirement) still blocks at any severity, while other new observations block only at the top severity label (`taskRunner.reviewer.round2NewFindings`: `"p0-only"` default | `"any"`) and otherwise go to Suggestions. A round only counts as "round 2" when a prior REVISE/RETHINK review of the gate exists. Worker/reviewer templates and the reviewer tool guidance carry the contract.
 
 ### Fixed
 
diff --git a/extensions/reviewer-extension.ts b/extensions/reviewer-extension.ts
index d3265140..f2157d0f 100644
--- a/extensions/reviewer-extension.ts
+++ b/extensions/reviewer-extension.ts
@@ -55,7 +55,7 @@ export default function reviewerExtension(pi: ExtensionAPI) {
 			"After writing your review to the specified output file, call wait_for_review() again.",
 			"When it returns 'SHUTDOWN', exit cleanly — the task is complete.",
 			"Reference your previous reviews when relevant (e.g., 'I flagged X in Step 1 — checking if addressed').",
-			"Round semantics (#657): each review request states its round. Round 1 is EXHAUSTIVE — list every finding now. Round ≥ 2 VERIFIES THE FOLD against the prior review; raise a new finding as blocking only at the top severity label unless the request says otherwise — everything else is a Suggestion and the verdict is APPROVE.",
+			"Round semantics (#657): each review request states its round. Round 1 is EXHAUSTIVE — list every finding now. Round ≥ 2 VERIFIES THE FOLD against the prior review. A new finding that is a genuine defect (regression from the fold, bug, unmet requirement) blocks at ANY severity; other new observations block only at the top severity label unless the request says otherwise and otherwise go to Suggestions. Never manufacture an APPROVE — an exhausted round budget is the worker's escalation, not your approval.",
 		],
 		parameters: Type.Object({}),
 		async execute() {
diff --git a/extensions/taskplane/agent-bridge-extension.ts b/extensions/taskplane/agent-bridge-extension.ts
index 74e68264..738310a6 100644
--- a/extensions/taskplane/agent-bridge-extension.ts
+++ b/extensions/taskplane/agent-bridge-extension.ts
@@ -32,7 +32,7 @@ import {
 	renameSync,
 	unlinkSync,
 } from "fs";
-import { join, dirname } from "path";
+import { join } from "path";
 import { spawn as nodeSpawn } from "child_process";
 import { resolvePiCliPath, resolveTaskplaneAgentTemplate } from "./path-resolver.ts";
 import { loadPiSettingsPackages, filterExcludedExtensions } from "./settings-loader.ts";
@@ -236,10 +236,20 @@ export function selfCheckRefusal(statusContent: string, stepNum: number): string
 	const sectionStart = m.index + m[0].length;
 	const rel = text.slice(sectionStart).search(/^#{1,4}\s/m);
 	const body = text.slice(sectionStart, rel === -1 ? undefined : sectionStart + rel);
-	const rows = body
-		.split("\n")
-		.filter((l) => /^\s*(\|.*\||[-*]\s+\S)/.test(l) && !/^\s*\|?\s*:?-{3,}/.test(l));
-	const dataRows = rows.filter((l) => !/^\s*\|\s*(item|check|criterion|finding|what)\b/i.test(l));
+	// Data rows: table rows that are neither a separator (|---|) nor the header
+	// row directly above a separator; plus list items. Structural, not keyword-based.
+	const bodyLines = body.split("\n");
+	const isSep = (l: string) => /^\s*\|?\s*:?-{3,}/.test(l);
+	const isTableRow = (l: string) => /^\s*\|.*\|\s*$/.test(l);
+	const dataRows = bodyLines.filter((l, idx) => {
+		if (isSep(l)) return false;
+		if (isTableRow(l)) {
+			let k = idx + 1;
+			while (k < bodyLines.length && bodyLines[k].trim() === "") k++;
+			return !(k < bodyLines.length && isSep(bodyLines[k])); // header row sits above the separator
+		}
+		return /^\s*[-*]\s+\S/.test(l);
+	});
 	if (dataRows.length === 0) {
 		return `"### Self-check (Step ${stepNum})" section has no rows — list each outcome / design decision / completion criterion with file:line evidence`;
 	}
@@ -267,7 +277,12 @@ export function selfCheckRefusal(statusContent: string, stepNum: number): string
 	return null;
 }
 
-/** #657: round number for this gate = 1 + number of existing review files for it. */
+/**
+ * #657: round number for this gate = 1 + the number of prior reviews of this
+ * gate that actually returned REVISE/RETHINK. An APPROVE, an incomplete file
+ * or an unparseable verdict is not evidence of a completed exhaustive review
+ * of the current work, so it does not narrow the next review's scope (Sage).
+ */
 export function reviewRoundForGate(
 	reviewsDir: string,
 	reviewType: string,
@@ -276,12 +291,20 @@ export function reviewRoundForGate(
 	try {
 		if (!existsSync(reviewsDir)) return { round: 1, priorReviewPath: null };
 		const suffix = `-${reviewType.toLowerCase()}-step${stepNum}.md`;
-		const files = readdirSync(reviewsDir)
+		const revised = readdirSync(reviewsDir)
 			.filter((f) => /^R\d{3}-/.test(f) && f.toLowerCase().endsWith(suffix))
-			.sort();
+			.sort()
+			.filter((f) => {
+				try {
+					const v = parseReviewVerdict(readFileSync(join(reviewsDir, f), "utf-8"));
+					return v === "REVISE" || v === "RETHINK";
+				} catch {
+					return false;
+				}
+			});
 		return {
-			round: files.length + 1,
-			priorReviewPath: files.length > 0 ? join(reviewsDir, files[files.length - 1]) : null,
+			round: revised.length + 1,
+			priorReviewPath: revised.length > 0 ? join(reviewsDir, revised[revised.length - 1]) : null,
 		};
 	} catch {
 		return { round: 1, priorReviewPath: null };
@@ -305,12 +328,16 @@ export function buildReviewRoundLines(
 	return [
 		`## Review round: ${round} (verify the fold)`,
 		`Prior review for this gate: \`${priorReviewPath ?? "(unknown)"}\``,
-		"Scope: verify that each finding in the prior review is addressed (cite file:line). " +
-			(round2Mode === "any"
-				? "New findings may be raised at any severity."
-				: "Raise a NEW finding as blocking ONLY at the top severity label (P0 / critical). " +
-					'Every other new observation goes in "### Suggestions" and does NOT change the verdict.'),
-		"If all prior findings are addressed and no top-severity finding remains, the verdict is APPROVE.",
+		"Scope: verify that each finding in the prior review is addressed (cite file:line).",
+		"A NEW finding is BLOCKING (REVISE) at any severity label when it is a genuine correctness defect: " +
+			"a regression introduced by the fold, a bug, or a stated requirement that is not met. Never downgrade " +
+			"a defect to a Suggestion because of the round.",
+		round2Mode === "any"
+			? "Other new observations (hardening, style, extra tests, robustness beyond the stated requirements) may also be raised as findings."
+			: "Other new observations (hardening, style, extra tests, robustness beyond the stated requirements) are " +
+				'blocking ONLY at the top severity label (P0 / critical); below that they go in "### Suggestions" and do not change the verdict.',
+		"Verdict: REVISE if any prior finding is unaddressed or any blocking finding remains; otherwise APPROVE. " +
+			"If the round budget is exhausted the worker escalates — do not manufacture an APPROVE.",
 	];
 }
 
@@ -1015,7 +1042,7 @@ export default function (pi: ExtensionAPI) {
 							{
 								type: "text" as const,
 								text:
-									`⛔ review_step refused — self-check missing: ${refusal}.\n\n` +
+									`REFUSED: self-check missing — ${refusal}.\n\n` +
 									`Before requesting a code review, add a "### Self-check (Step ${stepNum})" section to STATUS.md ` +
 									`(directly under the step, AFTER its checkboxes) with one row per: (a) step checkbox/outcome, ` +
 									`(b) each Step 0 design decision this step implements, (c) each PROMPT Completion Criterion it ` +
diff --git a/extensions/taskplane/engine.ts b/extensions/taskplane/engine.ts
index e95914af..ab756b26 100644
--- a/extensions/taskplane/engine.ts
+++ b/extensions/taskplane/engine.ts
@@ -2455,6 +2455,9 @@ async function attemptStaleWorktreeRecovery(
 					thinking: runnerConfig.worker.thinking || "",
 					tools: runnerConfig.worker.tools || "",
 					excludeExtensions: runnerConfig.worker.excludeExtensions ?? [],
+					exitInterceptTimeoutSec: runnerConfig.worker.exitInterceptTimeoutSec,
+					holdTimeoutMinutes: runnerConfig.worker.holdTimeoutMinutes,
+					requireSelfCheck: runnerConfig.worker.requireSelfCheck,
 				}
 			: undefined,
 		runnerConfig?.workerExcludeExtensions ?? [],
@@ -3250,6 +3253,9 @@ export async function executeOrchBatch(
 						thinking: runnerConfig.worker.thinking || "",
 						tools: runnerConfig.worker.tools || "",
 						excludeExtensions: runnerConfig.worker.excludeExtensions ?? [],
+						exitInterceptTimeoutSec: runnerConfig.worker.exitInterceptTimeoutSec,
+						holdTimeoutMinutes: runnerConfig.worker.holdTimeoutMinutes,
+						requireSelfCheck: runnerConfig.worker.requireSelfCheck,
 					}
 				: undefined,
 			runnerConfig?.workerExcludeExtensions ?? [],
diff --git a/extensions/taskplane/execution.ts b/extensions/taskplane/execution.ts
index ea754a5b..f7834f8a 100644
--- a/extensions/taskplane/execution.ts
+++ b/extensions/taskplane/execution.ts
@@ -1998,6 +1998,9 @@ export async function executeWave(
 		thinking?: string;
 		tools?: string;
 		excludeExtensions?: string[];
+		exitInterceptTimeoutSec?: number;
+		holdTimeoutMinutes?: number;
+		requireSelfCheck?: boolean;
 	} | null,
 	workerExcludeExtensions?: string[],
 	onLaneTerminated?: import("./types.ts").LaneTerminatedCallback,
@@ -3132,6 +3135,10 @@ export async function executeLaneV2(
 			workerSystemPrompt,
 			workerSegmentPrompt,
 			reviewerModel: extraEnvVars?.TASKPLANE_REVIEWER_MODEL || "",
+			// #657
+			requireSelfCheck: extraEnvVars?.TASKPLANE_REQUIRE_SELF_CHECK !== "0",
+			reviewRound2NewFindings:
+				extraEnvVars?.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS === "any" ? "any" : "p0-only",
 			reviewerThinking: extraEnvVars?.TASKPLANE_REVIEWER_THINKING || "",
 			reviewerTools: extraEnvVars?.TASKPLANE_REVIEWER_TOOLS || "",
 			// TP-180: Extension exclusion lists from config
diff --git a/extensions/taskplane/lane-runner.ts b/extensions/taskplane/lane-runner.ts
index cb84a874..5b82c42c 100644
--- a/extensions/taskplane/lane-runner.ts
+++ b/extensions/taskplane/lane-runner.ts
@@ -513,6 +513,10 @@ export interface LaneRunnerConfig {
 	noProgressLimit: number;
 	/** Exit-intercept supervisor-reply window (seconds; default 60; 15..1800). */
 	exitInterceptTimeoutSec?: number;
+	/** #657: review_step requires a fresh self-check section (default true). */
+	requireSelfCheck?: boolean;
+	/** #657: blocking scope for NEW findings in review round ≥ 2. */
+	reviewRound2NewFindings?: "p0-only" | "any";
 	/**
 	 * #627: durable hold store supplied by the engine. When absent (legacy
 	 * callers, unit tests) a volatile in-memory store is used — holds then do
@@ -2005,6 +2009,9 @@ export async function executeTaskV2(
 				TASKPLANE_SUPERVISOR_AUTONOMY: config.supervisorAutonomy || "autonomous",
 				ORCH_BATCH_ID: config.batchId,
 				...(config.reviewerModel ? { TASKPLANE_REVIEWER_MODEL: config.reviewerModel } : {}),
+				// #657: self-check gate + reviewer round semantics (consumed by review_step)
+				...(config.requireSelfCheck === false ? { TASKPLANE_REQUIRE_SELF_CHECK: "0" } : {}),
+				TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS: config.reviewRound2NewFindings ?? "p0-only",
 				...(config.reviewerThinking ? { TASKPLANE_REVIEWER_THINKING: config.reviewerThinking } : {}),
 				...(config.reviewerTools ? { TASKPLANE_REVIEWER_TOOLS: config.reviewerTools } : {}),
 				// TP-180: Pass state root and reviewer exclusions for extension forwarding
diff --git a/extensions/tests/review-self-check.test.ts b/extensions/tests/review-self-check.test.ts
index 48ec118f..82d400c7 100644
--- a/extensions/tests/review-self-check.test.ts
+++ b/extensions/tests/review-self-check.test.ts
@@ -93,13 +93,19 @@ describe("#657 — round semantics", () => {
 		const reviews = join(dir, ".reviews");
 		mkdirSync(reviews);
 		expect(reviewRoundForGate(reviews, "code", 3)).toEqual({ round: 1, priorReviewPath: null });
-		writeFileSync(join(reviews, "R001-plan-step3.md"), "Verdict: APPROVE");
-		writeFileSync(join(reviews, "R002-code-step3.md"), "Verdict: REVISE");
-		writeFileSync(join(reviews, "R003-code-step4.md"), "Verdict: REVISE");
+		writeFileSync(join(reviews, "R001-plan-step3.md"), "### Verdict: APPROVE");
+		writeFileSync(join(reviews, "R002-code-step3.md"), "### Verdict: REVISE");
+		writeFileSync(join(reviews, "R003-code-step4.md"), "### Verdict: REVISE");
 		const r = reviewRoundForGate(reviews, "code", 3);
 		expect(r.round).toBe(2);
 		expect(r.priorReviewPath).toBe(join(reviews, "R002-code-step3.md"));
-		expect(reviewRoundForGate(reviews, "plan", 3).round).toBe(2);
+		// Sage: an APPROVE, an incomplete file or an unparseable verdict is NOT a
+		// completed exhaustive review — it must not narrow the next review's scope.
+		expect(reviewRoundForGate(reviews, "plan", 3).round).toBe(1);
+		writeFileSync(join(reviews, "R004-code-step5.md"), "reviewer crashed mid-write");
+		expect(reviewRoundForGate(reviews, "code", 5).round).toBe(1);
+		writeFileSync(join(reviews, "R005-code-step3.md"), "### Verdict: APPROVE");
+		expect(reviewRoundForGate(reviews, "code", 3).round).toBe(2); // APPROVE after REVISE: still counts only the REVISE
 		expect(reviewRoundForGate(join(dir, "missing"), "code", 1).round).toBe(1);
 	});
 
@@ -110,11 +116,18 @@ describe("#657 — round semantics", () => {
 		const r2 = buildReviewRoundLines(2, "/x/.reviews/R002-code-step3.md", "p0-only").join("\n");
 		expect(r2).toContain("Review round: 2 (verify the fold)");
 		expect(r2).toContain("R002-code-step3.md");
-		expect(r2).toContain("ONLY at the top severity label");
-		expect(r2).toContain("the verdict is APPROVE");
+		// Sage: a genuine defect (regression from the fold, bug, unmet requirement) blocks at ANY label
+		expect(r2).toContain(
+			"BLOCKING (REVISE) at any severity label when it is a genuine correctness defect",
+		);
+		expect(r2).toContain("Never downgrade a defect to a Suggestion");
+		expect(r2).toContain("blocking ONLY at the top severity label");
+		expect(r2).toContain("do not manufacture an APPROVE");
+		expect(r2).not.toContain("no top-severity finding remains, the verdict is APPROVE");
 		const any = buildReviewRoundLines(2, "/x/p.md", "any").join("\n");
-		expect(any).toContain("any severity");
-		expect(any).not.toContain("ONLY at the top severity");
+		expect(any).toContain("may also be raised as findings");
+		expect(any).not.toContain("blocking ONLY at the top severity");
+		expect(any).toContain("Never downgrade a defect");
 	});
 
 	it("config → env: requireSelfCheck opt-out and round2NewFindings mode", () => {
@@ -140,7 +153,11 @@ describe("#657 — round semantics", () => {
 		const counter = src.indexOf("reviewCounter++;");
 		expect(gate).toBeGreaterThan(-1);
 		expect(gate).toBeLessThan(counter);
-		expect(src).toContain("review_step refused — self-check missing");
+		expect(src).toContain("`REFUSED: self-check missing — ${refusal}.");
+		// the disposition parser keys on ^REFUSED — the refusal must not read as a review verdict
+		expect(readSrc("taskplane/agent-host.ts")).toContain(
+			'if (/^REFUSED\\b/.test(lead)) return "REFUSED";',
+		);
 		expect((src.match(/\.\.\.roundLines,/g) ?? []).length).toBe(2);
 		expect(src).toContain(
 			"4. Read the worker's \"### Self-check (Step $" + '{stepNum})" table in STATUS.md',
@@ -159,3 +176,53 @@ describe("#657 — round semantics", () => {
 		expect(readSrc("reviewer-extension.ts")).toContain("Round semantics (#657)");
 	});
 });
+
+describe("#657 — settings reach the worker process (Sage blocker 2)", () => {
+	it("engine maps requireSelfCheck (and the other worker fields) into the executeWave worker config; execution consumes both env vars into LaneRunnerConfig; lane-runner forwards them to the worker env", () => {
+		const eng = readSrc("taskplane/engine.ts").replace(/\s+/g, " ");
+		expect(
+			(eng.match(/requireSelfCheck: runnerConfig\.worker\.requireSelfCheck,/g) ?? []).length,
+		).toBe(2);
+		expect(
+			(eng.match(/holdTimeoutMinutes: runnerConfig\.worker\.holdTimeoutMinutes,/g) ?? []).length,
+		).toBe(2);
+		const exec = readSrc("taskplane/execution.ts").replace(/\s+/g, " ");
+		expect(exec).toContain('requireSelfCheck: extraEnvVars?.TASKPLANE_REQUIRE_SELF_CHECK !== "0",');
+		expect(exec).toContain(
+			'reviewRound2NewFindings: extraEnvVars?.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS === "any" ? "any" : "p0-only",',
+		);
+		const lr = readSrc("taskplane/lane-runner.ts").replace(/\s+/g, " ");
+		expect(lr).toContain(
+			'...(config.requireSelfCheck === false ? { TASKPLANE_REQUIRE_SELF_CHECK: "0" } : {}),',
+		);
+		expect(lr).toContain(
+			'TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS: config.reviewRound2NewFindings ?? "p0-only",',
+		);
+	});
+
+	it("end to end: config → buildWorkerEnv/buildReviewerEnv → lane config → worker env (opt-out and 'any' both survive)", () => {
+		const worker = buildWorkerEnv({ requireSelfCheck: false });
+		const reviewer = buildReviewerEnv({ round2NewFindings: "any" });
+		const extra = { ...worker, ...reviewer };
+		// mirror execution.ts's lane-config derivation
+		const laneCfg = {
+			requireSelfCheck: extra.TASKPLANE_REQUIRE_SELF_CHECK !== "0",
+			reviewRound2NewFindings:
+				extra.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS === "any" ? "any" : "p0-only",
+		};
+		expect(laneCfg).toEqual({ requireSelfCheck: false, reviewRound2NewFindings: "any" });
+		// mirror lane-runner's worker env derivation
+		const workerEnv = {
+			...(laneCfg.requireSelfCheck === false ? { TASKPLANE_REQUIRE_SELF_CHECK: "0" } : {}),
+			TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS: laneCfg.reviewRound2NewFindings,
+		};
+		expect(workerEnv).toEqual({
+			TASKPLANE_REQUIRE_SELF_CHECK: "0",
+			TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS: "any",
+		});
+		// defaults
+		const dflt = { ...buildWorkerEnv({}), ...buildReviewerEnv({}) };
+		expect(dflt.TASKPLANE_REQUIRE_SELF_CHECK).toBe(undefined);
+		expect(dflt.TASKPLANE_REVIEW_ROUND2_NEW_FINDINGS).toBe("p0-only");
+	});
+});
diff --git a/extensions/tests/review-step-guard-runtime.test.ts b/extensions/tests/review-step-guard-runtime.test.ts
index 5e32c9e4..d42e1ea9 100644
--- a/extensions/tests/review-step-guard-runtime.test.ts
+++ b/extensions/tests/review-step-guard-runtime.test.ts
@@ -110,6 +110,13 @@ function makeStatusContent(stepNum: number, stepStatus: "✅ Complete" | "🟨 I
 		"- [x] item one",
 		"- [x] item two",
 		"",
+		// #657: a code review request must carry the worker's self-check (placed after the checkboxes)
+		`### Self-check (Step ${stepNum})`,
+		"",
+		"| Item | Evidence | Result |",
+		"|------|----------|--------|",
+		"| item one | src/a.ts:1 | OK |",
+		"",
 		"### Step 99: Sentinel",
 		"**Status:** ⬜ Not Started",
 	].join("\n");
diff --git a/templates/agents/task-reviewer.md b/templates/agents/task-reviewer.md
index 90cb78ff..6402a944 100644
--- a/templates/agents/task-reviewer.md
+++ b/templates/agents/task-reviewer.md
@@ -155,12 +155,18 @@ by a supervisor ruling — so the two rounds have different jobs:
 - **Round ≥ 2 — verify the fold.** Your scope is the prior review's findings:
   for each, confirm it is addressed (cite `file:line`) or state precisely what
   is still missing. Read the worker's `### Self-check (Step N)` table and
-  verify its evidence rather than re-deriving the review from scratch. A NEW
-  finding is blocking only at the **top severity label** (P0 / critical) unless
-  the request says new findings may be raised at any severity. Every other new
-  observation goes under `### Suggestions` and does not change the verdict.
-  If all prior findings are addressed and no top-severity finding remains, the
-  verdict is **APPROVE**.
+  verify its evidence rather than re-deriving the review from scratch.
+  - A NEW finding that is a **genuine correctness defect** — a regression
+    introduced by the fold, a bug, a stated requirement not met — is blocking
+    at **any** severity label. Never downgrade a defect to a Suggestion because
+    of the round.
+  - Other new observations (hardening, style, extra tests, robustness beyond
+    the stated requirements) are blocking only at the **top severity label**
+    (P0 / critical) unless the request says otherwise; below that they go under
+    `### Suggestions` and do not change the verdict.
+  - Verdict: REVISE if any prior finding is unaddressed or any blocking finding
+    remains; otherwise **APPROVE**. If the round budget is exhausted the worker
+    escalates — never manufacture an APPROVE.
 
 ### Do NOT issue REVISE for
 
@@ -170,8 +176,9 @@ by a supervisor ruling — so the two rounds have different jobs:
 - "Re-run tests and record the result" — test runs are the worker's concern
 - "Check If Affected" docs that turn out to need no changes
 - Suggestions that improve quality but aren't required for correctness
-- In round ≥ 2: any new finding below the top severity label — that is a
-  Suggestion (see Round semantics)
+- In round ≥ 2: a new *non-defect* observation below the top severity label —
+  that is a Suggestion (see Round semantics). A regression or unmet
+  requirement is a defect and still blocks.
 
 ## Plan Review Format
 

From 0be6b1f43e98bcb8f9180fbebb9d927867f4c7ef Mon Sep 17 00:00:00 2001
From: Henry Lach <henry@lach.ca>
Date: Wed, 16 Sep 2026 00:46:00 -0400
Subject: [PATCH 3/3] fix(engine): model-fallback retry forwards non-model
 worker settings (#657 Sage round 2)

attemptModelFallbackRetry built its env without buildWorkerEnv, so a
model-access retry silently reset requireSelfCheck to true and dropped the
configured exit-intercept / hold timeouts. Forward every worker setting via
buildWorkerEnv with the configured model withheld (the retry must use the
session model, not the one that just failed). Test asserts the built env.
---
 extensions/taskplane/engine.ts                |  6 ++++
 .../tests/runtime-model-fallback.test.ts      | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/extensions/taskplane/engine.ts b/extensions/taskplane/engine.ts
index ab756b26..9e422bf0 100644
--- a/extensions/taskplane/engine.ts
+++ b/extensions/taskplane/engine.ts
@@ -2112,9 +2112,15 @@ async function attemptModelFallbackRetry(
 			// Pass TASKPLANE_MODEL_FALLBACK=1 as extra env var to signal
 			// the task-runner to use the session model instead of configured model.
 			// TP-089: Also include ORCH_BATCH_ID so mailbox steering works for retries.
+			// #657 (Sage): forward every NON-model worker setting (self-check opt-out,
+			// intercept window, hold timeout, tools, thinking) exactly as the normal
+			// wave does — only the configured model is withheld so the retry uses the
+			// session model instead of the one that just failed.
+			const { model: _unavailableModel, ...workerWithoutModel } = runnerConfig?.worker ?? {};
 			const modelFallbackEnv = {
 				TASKPLANE_MODEL_FALLBACK: "1",
 				ORCH_BATCH_ID: batchState.batchId,
+				...buildWorkerEnv(runnerConfig?.worker ? workerWithoutModel : undefined),
 				...buildReviewerEnv(runnerConfig?.reviewer),
 				...buildWorkerExcludeEnv(runnerConfig?.workerExcludeExtensions),
 			};
diff --git a/extensions/tests/runtime-model-fallback.test.ts b/extensions/tests/runtime-model-fallback.test.ts
index 9b800091..5a7c6676 100644
--- a/extensions/tests/runtime-model-fallback.test.ts
+++ b/extensions/tests/runtime-model-fallback.test.ts
@@ -331,6 +331,35 @@ describe("model fallback retry logic", () => {
 			expect(engineSource).toContain('"1"');
 		});
 
+		it("#657 (Sage): the fallback retry forwards every NON-model worker setting and withholds only the model", async () => {
+			const flat = engineSource.replace(/\s+/g, " ");
+			expect(flat).toContain(
+				"const { model: _unavailableModel, ...workerWithoutModel } = runnerConfig?.worker ?? {};",
+			);
+			expect(flat).toContain(
+				"...buildWorkerEnv(runnerConfig?.worker ? workerWithoutModel : undefined), ...buildReviewerEnv(runnerConfig?.reviewer),",
+			);
+			// behavioural: the exact env the retry builds, given a configured worker
+			const { buildWorkerEnv } = await import("../taskplane/execution.ts");
+			const worker = {
+				model: "anthropic/unavailable",
+				thinking: "high",
+				tools: "read,edit",
+				excludeExtensions: [],
+				exitInterceptTimeoutSec: 300,
+				holdTimeoutMinutes: 30,
+				requireSelfCheck: false,
+			};
+			const { model: _m, ...withoutModel } = worker;
+			const env = buildWorkerEnv(withoutModel);
+			expect(env.TASKPLANE_WORKER_MODEL).toBe(undefined); // the failed model is NOT retried
+			expect(env.TASKPLANE_REQUIRE_SELF_CHECK).toBe("0");
+			expect(env.TASKPLANE_EXIT_INTERCEPT_TIMEOUT_SEC).toBe("300");
+			expect(env.TASKPLANE_HOLD_TIMEOUT_MIN).toBe("30");
+			expect(env.TASKPLANE_WORKER_THINKING).toBe("high");
+			expect(env.TASKPLANE_WORKER_TOOLS).toBe("read,edit");
+		});
+
 		it("uses model_fallback scope key for budget tracking", () => {
 			expect(engineSource).toContain('tier0ScopeKey("model_fallback"');
 		});