Integrate orch batch 20260320T111421 (#112)

* feat(TP-036): complete Step 0 — Preflight

* feat(TP-036): complete Step 1 — implement review skip logic for low-risk steps

* feat(TP-036): complete Step 1 — skip reviews for low-risk steps (Step 0 and final step)

* feat(TP-036): complete Step 2 — Testing & Verification

* hydrate: add R004 revision items to Step 3

* checkpoint: TP-036 task artifacts (.DONE, STATUS.md)

* checkpoint: wave 1 task artifacts (.DONE, STATUS.md, REVIEW_VERDICT.json)

Author: HenryLach

docs/explanation/execution-model.md

@@ -59,6 +59,12 @@ For each step:
 - `1`: plan review before implementation
 - `2+`: plan review + code review
 
+**Low-risk step exception:** Step 0 (Preflight) and the final step
+(Documentation & Delivery) always skip both plan and code reviews, regardless
+of the configured review level. These steps perform file reading and `.DONE`
+creation respectively — cross-model review adds overhead without catching
+meaningful issues. Middle steps are unaffected by this exception.
+
 ---
 
 ## Worker iteration loop

docs/explanation/review-loop.md

@@ -50,6 +50,10 @@ Task `Review Level` controls review rigor:
 
 Current runner behavior applies plan review at `>=1` and code review at `>=2`.
 
+**Exception:** Step 0 (Preflight) and the final step (Documentation & Delivery)
+always skip both plan and code reviews, regardless of review level. These
+low-risk steps don't benefit from cross-model review.
+
 ---
 
 ## Loop mechanics in task-runner

extensions/task-runner.ts

@@ -1406,6 +1406,20 @@ export const _readExitSummary = readExitSummary;
 export const _buildExitDiagnostic = buildExitDiagnostic;
 export type { BuildExitDiagnosticInput };
 
+/**
+ * Determine whether a step is "low-risk" and should skip reviews.
+ * Low-risk steps: Step 0 (Preflight) and the final step (Delivery/Docs).
+ *
+ * @param stepNumber  The 0-based step number being evaluated
+ * @param totalSteps  Total number of steps in the task
+ * @returns true if the step should skip plan and code reviews
+ */
+export function isLowRiskStep(stepNumber: number, totalSteps: number): boolean {
+	if (totalSteps <= 0) return false;
+	const lastStepIndex = totalSteps - 1;
+	return stepNumber === 0 || stepNumber === lastStepIndex;
+}
+
 // ── TMUX Agent Spawner ───────────────────────────────────────────────
 
 /**
@@ -2081,11 +2095,20 @@ export default function (pi: ExtensionAPI) {
 		logExecution(statusPath, `Step ${step.number} started`, step.name);
 		updateWidgets();
 
+		// Skip reviews for low-risk steps (Step 0 / Preflight and final step / Delivery)
+		const _isLowRiskStep = isLowRiskStep(step.number, task.steps.length);
+
 		// Plan review (level ≥ 1)
 		if (task.reviewLevel >= 1) {
-			const verdict = await doReview("plan", step, ctx, stepBaselineCommit);
-			if (verdict === "RETHINK") {
-				ctx.ui.notify(`Reviewer: RETHINK on Step ${step.number} plan. Proceeding with caution.`, "warning");
+			if (_isLowRiskStep) {
+				const label = step.number === 0 ? "Preflight" : "final step";
+				logExecution(statusPath, `Skip plan review`, `Step ${step.number} (${label}) — low-risk`);
+				ctx.ui.notify(`⏭️ Skipping plan review for Step ${step.number} (${label})`, "info");
+			} else {
+				const verdict = await doReview("plan", step, ctx, stepBaselineCommit);
+				if (verdict === "RETHINK") {
+					ctx.ui.notify(`Reviewer: RETHINK on Step ${step.number} plan. Proceeding with caution.`, "warning");
+				}
 			}
 		}
 
@@ -2135,10 +2158,16 @@ export default function (pi: ExtensionAPI) {
 
 		// Code review (level ≥ 2)
 		if (task.reviewLevel >= 2 && state.phase === "running") {
-			const verdict = await doReview("code", step, ctx, stepBaselineCommit);
-			if (verdict === "REVISE") {
-				ctx.ui.notify(`Reviewer: REVISE on Step ${step.number}. Running worker to fix...`, "warning");
-				await runWorker(step, ctx); // One more pass to address issues
+			if (_isLowRiskStep) {
+				const label = step.number === 0 ? "Preflight" : "final step";
+				logExecution(statusPath, `Skip code review`, `Step ${step.number} (${label}) — low-risk`);
+				ctx.ui.notify(`⏭️ Skipping code review for Step ${step.number} (${label})`, "info");
+			} else {
+				const verdict = await doReview("code", step, ctx, stepBaselineCommit);
+				if (verdict === "REVISE") {
+					ctx.ui.notify(`Reviewer: REVISE on Step ${step.number}. Running worker to fix...`, "warning");
+					await runWorker(step, ctx); // One more pass to address issues
+				}
 			}
 		}
 

extensions/tests/task-runner-review-skip.test.ts

@@ -0,0 +1,248 @@
+/**
+ * Task-Runner Review Skip Tests — TP-036
+ *
+ * Tests for the low-risk step detection logic that skips plan and code
+ * reviews for Step 0 (Preflight) and the final step (Delivery/Docs).
+ *
+ * Test categories:
+ *   1.x — isLowRiskStep pure function (boundary detection)
+ *   2.x — Review level interactions (level 0, 1, 2 with low-risk steps)
+ *   3.x — Edge cases (single-step task, two-step task, large step counts)
+ *
+ * Run: npx vitest run tests/task-runner-review-skip.test.ts
+ */
+
+import { describe, it, expect } from "vitest";
+import { isLowRiskStep } from "../task-runner.ts";
+
+// ══════════════════════════════════════════════════════════════════════
+// 1.x — isLowRiskStep pure function
+// ══════════════════════════════════════════════════════════════════════
+
+describe("1.x: isLowRiskStep boundary detection", () => {
+	// ── Standard 4-step task (Steps 0, 1, 2, 3) ─────────────────────
+
+	it("1.1: Step 0 in a 4-step task → low-risk (Preflight)", () => {
+		expect(isLowRiskStep(0, 4)).toBe(true);
+	});
+
+	it("1.2: Step 3 (last) in a 4-step task → low-risk (final step)", () => {
+		expect(isLowRiskStep(3, 4)).toBe(true);
+	});
+
+	it("1.3: Step 1 in a 4-step task → NOT low-risk (middle step)", () => {
+		expect(isLowRiskStep(1, 4)).toBe(false);
+	});
+
+	it("1.4: Step 2 in a 4-step task → NOT low-risk (middle step)", () => {
+		expect(isLowRiskStep(2, 4)).toBe(false);
+	});
+
+	// ── Review level 2 interactions ──────────────────────────────────
+	// The review decision logic uses isLowRiskStep to gate BOTH plan
+	// and code reviews. Verify that:
+	//   - review level ≥ 1: plan review gated by isLowRiskStep
+	//   - review level ≥ 2: code review gated by isLowRiskStep
+	// Since the gating is: if (reviewLevel >= N) { if (isLowRiskStep) skip; else review; }
+	// the pure function test verifies the correct inputs produce the right skip decision.
+
+	it("1.5: Step 0 at review level 2 — both plan and code reviews should skip", () => {
+		// isLowRiskStep returns true → both plan (level≥1) and code (level≥2) skip
+		const reviewLevel = 2;
+		const skip = isLowRiskStep(0, 4);
+		expect(skip).toBe(true);
+		// Plan review: reviewLevel >= 1 && skip → skipped ✓
+		// Code review: reviewLevel >= 2 && skip → skipped ✓
+		expect(reviewLevel >= 1 && skip).toBe(true);
+		expect(reviewLevel >= 2 && skip).toBe(true);
+	});
+
+	it("1.6: Final step at review level 2 — both plan and code reviews should skip", () => {
+		const reviewLevel = 2;
+		const skip = isLowRiskStep(3, 4);
+		expect(skip).toBe(true);
+		expect(reviewLevel >= 1 && skip).toBe(true);
+		expect(reviewLevel >= 2 && skip).toBe(true);
+	});
+
+	it("1.7: Middle step at review level 2 — reviews should NOT skip", () => {
+		const reviewLevel = 2;
+		const skip = isLowRiskStep(1, 4);
+		expect(skip).toBe(false);
+		// Plan review: reviewLevel >= 1 && !skip → review runs ✓
+		// Code review: reviewLevel >= 2 && !skip → review runs ✓
+		expect(reviewLevel >= 1 && !skip).toBe(true);
+		expect(reviewLevel >= 2 && !skip).toBe(true);
+	});
+
+	// ── Review level 0 (all reviews disabled) ────────────────────────
+
+	it("1.8: Review level 0 — no reviews regardless of step position", () => {
+		const reviewLevel = 0;
+		// Even if isLowRiskStep returns true, level 0 means no reviews at all
+		// The outer gate: if (reviewLevel >= 1) never fires
+		expect(reviewLevel >= 1).toBe(false);
+		expect(reviewLevel >= 2).toBe(false);
+		// isLowRiskStep is never consulted, but let's verify it still works
+		expect(isLowRiskStep(0, 4)).toBe(true);
+		expect(isLowRiskStep(1, 4)).toBe(false);
+		expect(isLowRiskStep(3, 4)).toBe(true);
+	});
+
+	// ── Review level 1 (plan only) ──────────────────────────────────
+
+	it("1.9: Review level 1, Step 0 — plan review skipped, no code review", () => {
+		const reviewLevel = 1;
+		const skip = isLowRiskStep(0, 4);
+		expect(reviewLevel >= 1 && skip).toBe(true); // plan review gate → skip
+		expect(reviewLevel >= 2).toBe(false); // code review gate never fires
+	});
+
+	it("1.10: Review level 1, middle step — plan review runs, no code review", () => {
+		const reviewLevel = 1;
+		const skip = isLowRiskStep(2, 4);
+		expect(reviewLevel >= 1 && !skip).toBe(true); // plan review runs
+		expect(reviewLevel >= 2).toBe(false); // code review gate never fires
+	});
+});
+
+// ══════════════════════════════════════════════════════════════════════
+// 2.x — Edge cases
+// ══════════════════════════════════════════════════════════════════════
+
+describe("2.x: Edge cases", () => {
+	it("2.1: Single-step task — Step 0 is both first AND last → low-risk", () => {
+		// totalSteps=1, lastStepIndex=0, stepNumber=0 matches both conditions
+		expect(isLowRiskStep(0, 1)).toBe(true);
+	});
+
+	it("2.2: Two-step task — Step 0 is low-risk", () => {
+		expect(isLowRiskStep(0, 2)).toBe(true);
+	});
+
+	it("2.3: Two-step task — Step 1 (last) is low-risk", () => {
+		expect(isLowRiskStep(1, 2)).toBe(true);
+	});
+
+	it("2.4: Two-step task — both steps are low-risk (no middle steps)", () => {
+		// In a 2-step task, Step 0 is first and Step 1 is last
+		// No middle steps exist — all reviews would be skipped
+		expect(isLowRiskStep(0, 2)).toBe(true);
+		expect(isLowRiskStep(1, 2)).toBe(true);
+	});
+
+	it("2.5: Three-step task — only Step 1 is NOT low-risk", () => {
+		expect(isLowRiskStep(0, 3)).toBe(true);  // first
+		expect(isLowRiskStep(1, 3)).toBe(false); // middle
+		expect(isLowRiskStep(2, 3)).toBe(true);  // last
+	});
+
+	it("2.6: Large task (10 steps) — only first and last are low-risk", () => {
+		expect(isLowRiskStep(0, 10)).toBe(true);   // first
+		expect(isLowRiskStep(9, 10)).toBe(true);   // last
+		// All middle steps
+		for (let i = 1; i < 9; i++) {
+			expect(isLowRiskStep(i, 10)).toBe(false);
+		}
+	});
+
+	it("2.7: Zero totalSteps → false (defensive)", () => {
+		expect(isLowRiskStep(0, 0)).toBe(false);
+	});
+
+	it("2.8: Negative totalSteps → false (defensive)", () => {
+		expect(isLowRiskStep(0, -1)).toBe(false);
+	});
+});
+
+// ══════════════════════════════════════════════════════════════════════
+// 3.x — Integration: review gating decision matrix
+// ══════════════════════════════════════════════════════════════════════
+
+describe("3.x: Review gating decision matrix", () => {
+	// This simulates the full decision logic from task-runner.ts:
+	//   Plan review: if (reviewLevel >= 1) { if (isLowRisk) skip; else review; }
+	//   Code review: if (reviewLevel >= 2) { if (isLowRisk) skip; else review; }
+
+	type ReviewDecision = "skip" | "review" | "no-gate";
+
+	function planReviewDecision(reviewLevel: number, stepNumber: number, totalSteps: number): ReviewDecision {
+		if (reviewLevel < 1) return "no-gate";
+		return isLowRiskStep(stepNumber, totalSteps) ? "skip" : "review";
+	}
+
+	function codeReviewDecision(reviewLevel: number, stepNumber: number, totalSteps: number): ReviewDecision {
+		if (reviewLevel < 2) return "no-gate";
+		return isLowRiskStep(stepNumber, totalSteps) ? "skip" : "review";
+	}
+
+	// ── Level 0: no reviews at all ──────────────────────────────────
+
+	it("3.1: Level 0, Step 0 — no plan review, no code review", () => {
+		expect(planReviewDecision(0, 0, 4)).toBe("no-gate");
+		expect(codeReviewDecision(0, 0, 4)).toBe("no-gate");
+	});
+
+	it("3.2: Level 0, middle step — no plan review, no code review", () => {
+		expect(planReviewDecision(0, 2, 4)).toBe("no-gate");
+		expect(codeReviewDecision(0, 2, 4)).toBe("no-gate");
+	});
+
+	it("3.3: Level 0, final step — no plan review, no code review", () => {
+		expect(planReviewDecision(0, 3, 4)).toBe("no-gate");
+		expect(codeReviewDecision(0, 3, 4)).toBe("no-gate");
+	});
+
+	// ── Level 1: plan reviews only ──────────────────────────────────
+
+	it("3.4: Level 1, Step 0 — plan review SKIPPED, no code review gate", () => {
+		expect(planReviewDecision(1, 0, 4)).toBe("skip");
+		expect(codeReviewDecision(1, 0, 4)).toBe("no-gate");
+	});
+
+	it("3.5: Level 1, middle step — plan review RUNS, no code review gate", () => {
+		expect(planReviewDecision(1, 1, 4)).toBe("review");
+		expect(codeReviewDecision(1, 1, 4)).toBe("no-gate");
+	});
+
+	it("3.6: Level 1, final step — plan review SKIPPED, no code review gate", () => {
+		expect(planReviewDecision(1, 3, 4)).toBe("skip");
+		expect(codeReviewDecision(1, 3, 4)).toBe("no-gate");
+	});
+
+	// ── Level 2: plan + code reviews ────────────────────────────────
+
+	it("3.7: Level 2, Step 0 — plan review SKIPPED, code review SKIPPED", () => {
+		expect(planReviewDecision(2, 0, 4)).toBe("skip");
+		expect(codeReviewDecision(2, 0, 4)).toBe("skip");
+	});
+
+	it("3.8: Level 2, middle step — plan review RUNS, code review RUNS", () => {
+		expect(planReviewDecision(2, 1, 4)).toBe("review");
+		expect(codeReviewDecision(2, 1, 4)).toBe("review");
+	});
+
+	it("3.9: Level 2, final step — plan review SKIPPED, code review SKIPPED", () => {
+		expect(planReviewDecision(2, 3, 4)).toBe("skip");
+		expect(codeReviewDecision(2, 3, 4)).toBe("skip");
+	});
+
+	// ── Level 2, single-step task ───────────────────────────────────
+
+	it("3.10: Level 2, single step (Step 0 is also final) — all reviews SKIPPED", () => {
+		expect(planReviewDecision(2, 0, 1)).toBe("skip");
+		expect(codeReviewDecision(2, 0, 1)).toBe("skip");
+	});
+
+	// ── Level 3+ (future-proofing) ──────────────────────────────────
+
+	it("3.11: Level 3, Step 0 — both reviews still skip (higher levels don't change skip logic)", () => {
+		expect(planReviewDecision(3, 0, 4)).toBe("skip");
+		expect(codeReviewDecision(3, 0, 4)).toBe("skip");
+	});
+
+	it("3.12: Level 3, middle step — both reviews run", () => {
+		expect(planReviewDecision(3, 1, 4)).toBe("review");
+		expect(codeReviewDecision(3, 1, 4)).toBe("review");
+	});
+});

taskplane-tasks/TP-036-skip-reviews-low-risk-steps/.DONE

@@ -0,0 +1 @@
+completed: 2026-03-20

taskplane-tasks/TP-036-skip-reviews-low-risk-steps/.reviews/R001-plan-step0.md

@@ -0,0 +1,15 @@
+## Plan Review: Step 0: Preflight
+
+### Verdict: APPROVE
+
+### Summary
+The Step 0 plan is correctly scoped to the stated preflight outcomes: find the existing review-gating decision points and confirm where current-step and total-step metadata are available. That is sufficient preparation for Step 1 without over-constraining implementation details. I do not see any blocking gaps that would risk incorrect behavior later.
+
+### Issues Found
+1. **[Severity: minor]** — No blocking issues found.
+
+### Missing Items
+- None.
+
+### Suggestions
+- During preflight, explicitly note both gating locations in `extensions/task-runner.ts` (`plan` and `code` review checks in `executeStep`) plus the source of total steps (`task.steps.length`) so Step 1 can apply the skip condition consistently.

taskplane-tasks/TP-036-skip-reviews-low-risk-steps/.reviews/R002-plan-step1.md

@@ -0,0 +1,17 @@
+## Plan Review: Step 1: Implement Review Skip Logic
+
+### Verdict: APPROVE
+
+### Summary
+The Step 1 plan is correctly aligned with the task outcomes: it adds boundary-step skip logic for both plan and code reviews, defines how to detect the final step, and preserves existing behavior for middle steps. It also includes explicit logging requirements so operators can see why reviews were not run. I do not see any blocking gaps that would prevent the step from achieving its stated goal.
+
+### Issues Found
+1. **[Severity: minor]** — No blocking issues found.
+
+### Missing Items
+- None.
+
+### Suggestions
+- In `extensions/task-runner.ts` (current review gates around `executeStep`), compute a single boolean for boundary-step skipping and reuse it for both the plan-review gate and code-review gate to prevent drift.
+- For final-step detection, prefer using position in `task.steps` (or last parsed step identity) rather than assuming contiguous numeric step labels.
+- Keep skip log messages distinct for Step 0 vs final step so future debugging clearly shows which rule triggered.

taskplane-tasks/TP-036-skip-reviews-low-risk-steps/.reviews/R003-plan-step2.md

@@ -0,0 +1,17 @@
+## Plan Review: Step 2: Testing & Verification
+
+### Verdict: APPROVE
+
+### Summary
+The Step 2 plan covers all required behavioral outcomes from `PROMPT.md`: boundary-step review skipping (Step 0 and final step), unchanged middle-step behavior, unchanged review-level-0 behavior, and the single-step edge case. It also includes full-suite verification, which is appropriate for guarding regressions in `task-runner.ts` behavior. I do not see any blocking gaps that would prevent this step from validating the change correctly.
+
+### Issues Found
+1. **[Severity: minor]** — No blocking issues found.
+
+### Missing Items
+- None.
+
+### Suggestions
+- In the new test file (`extensions/tests/task-runner-review-skip.test.ts`), assert both review types explicitly for each scenario (plan + code where applicable), so a partial skip regression is caught.
+- For “no review spawned” assertions, prefer checking durable artifacts/state transitions (e.g., review request files or counters) rather than only log text.
+- Keep scenario names mapped 1:1 to the Step 2 checklist items to make STATUS.md updates and future triage straightforward.