Fail forward when goal judge is inconclusive

README.md

@@ -134,15 +134,18 @@ Latency target came from the SLO review; keep the existing client API.
 else is the agent editing the file. It reads the goal's `evidence:` block from `.pi/goals.md`, then:
 
 1. If the goal has a `verify:` command, it runs. A non-zero exit rejects right away, no model call.
-2. Otherwise a read-only `pi` subprocess (a fresh `--no-session` context, so it never sees the working
+2. Then a read-only `pi` subprocess (a fresh `--no-session` context, so it never sees the working
    agent's transcript) inspects the `evidence:` against the repo, the `discriminator`, and the
    `subtle failure mode`. It re-derives from the cited artifacts rather than trusting the claim, so
    list real artifacts, not assertions.
-3. On accept, the goal flips to `[x]` and a `## Log` line is written. On reject, it stays open and the
-   agent is told what is missing. Either way the judge's reasoning comes back in the result.
+3. On accept, the goal flips to `[x]` and a `## Log` line is written. On judge reject, it stays open
+   and the agent is told what is missing. If the judge subprocess times out or its transport/model
+   fails after any `verify:` command has passed, the goal still flips to `[x]` with a `judge
+   inconclusive` log line and any partial judge output in the result. Either way the judge's
+   reasoning comes back in the result.
 
-The judge defaults to your current model (a fresh context, same weights). Point it at another with
-`/goals judge <provider/model>` for an independent cross-family check.
+The judge defaults to Pi's default model unless `/goals judge <provider/model>` is set. Point it at
+another model for an independent cross-family check.
 
 ## Prompts
 

docs/spec/2026-06-29_complete-goal-fail-forward.md

@@ -0,0 +1,63 @@
+# CompleteGoal fail-forward on judge failure
+
+## Goal
+Make `CompleteGoal` stop rejecting verified goals just because the read-only judge subprocess times out. Keep the judge useful when it works, and make failures explicit in the log/result.
+
+## Scope
+In: `CompleteGoal` sign-off behavior, judge transport, tests, docs.
+Out: broader autonomous loop work, plan-mode UX, model auto-selection.
+
+## Requirements
+- R1: If `verify:` fails, the goal is rejected immediately. Done means: existing `verify_failed` behavior remains. VERIFY: unit test for pure sign-off record still passes.
+- R2: If `verify:` passes and the judge accepts, mark the goal done as before. Done means: log records normal judge accept. VERIFY: unit test for accepted sign-off still passes.
+- R3: If any `verify:` command passes but the judge times out or subprocess/model transport fails, mark the goal done with an explicit inconclusive-judge log. Goals without `verify:` use the same fail-forward rule once evidence exists. VERIFY: a unit test records accepted status and a log line containing `judge inconclusive`.
+- R4: Judge transport should parse `pi --mode json` message events instead of raw `-p` terminal output. Done means: code captures final assistant text and provider stop errors distinctly. VERIFY: `npm run typecheck` and tests pass.
+
+## Tasks
+- [x] T1 (R3): Add an accepted-with-warning sign-off outcome.
+  - verify: `npm test`
+  - success: test shows status `[x]` plus `judge inconclusive` in `## Log`
+  - likely_fail: timeout still records `reject`
+  - sneaky_fail: accepted status lands but log hides judge failure
+  - UAT: [test/plan-file.test.ts](/home/wassname/.pi/agent/git/github.com/wassname/pi-plan/test/plan-file.test.ts)
+- [x] T2 (R4): Switch judge subprocess to JSON-mode parsing.
+  - verify: `npm run typecheck`
+  - success: no TypeScript errors, judge code has no ANSI-terminal parsing dependency
+  - likely_fail: compile errors around streamed event shape
+  - sneaky_fail: model error produces empty output and gets parsed as reject instead of transport failure
+  - UAT: [src/index.ts](/home/wassname/.pi/agent/git/github.com/wassname/pi-plan/src/index.ts)
+- [x] T3 (docs): Update README sign-off semantics.
+  - verify: `rg "inconclusive|timeout|judge accept" README.md src test`
+  - success: docs name fail-forward behavior
+  - likely_fail: README still says all rejects keep goal open
+  - sneaky_fail: docs imply subagent evidence was accepted when it timed out
+  - UAT: [README.md](/home/wassname/.pi/agent/git/github.com/wassname/pi-plan/README.md)
+
+## Context
+Observed result from downstream use:
+
+```json
+{
+  "goal": "Make persona validation fail-fast and evidence-correct",
+  "outcome": "rejected",
+  "durationMs": 120003,
+  "verifyCommand": "`uv run python -m compileall -q scripts/validate_persona_axes_openrouter.py`",
+  "reasoning": "VERDICT: reject\nmissing: judge timed out after 120s",
+  "isError": true
+}
+```
+
+Interpretation: latest surfaced output proves the internal judge timed out. It does not prove the verify command passed, though earlier logs indicated that pattern.
+
+## Log
+- 2026-06-29  current `runJudge` uses raw `pi -p --no-session` output plus ANSI stripping; oracle uses `--mode json` and parses message events, which is likely more reliable.
+- 2026-06-29  unset `/goals judge` spawns the judge without `--model`, so Pi resolves its configured default model; do not describe this as the current session model.
+- 2026-06-29  timeout/transport failure now maps to `accepted_inconclusive`, preserving partial output in reasoning when available.
+- 2026-06-29  fresh-eyes review found loose `/accept/i` verdict parsing and caller-abort fail-forward risk; fixed exact verdict parsing and made caller abort reject.
+
+## TODO
+- Consider making `CompleteGoal` expose `verifyExitCode: 0` and `judgeOutcome` separately in details.
+
+## Errors
+| Task | Error | Resolution |
+|------|-------|------------|

src/index.ts

@@ -298,14 +298,18 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 			if (res.content !== content) writePlan(ctx, res.content);
 			updateWidget(ctx);
 			const detail = reasoning ? `\n\n--- sign-off judge ---\n${reasoning}` : "";
-			const outcomeLabel = outcome.kind === "accepted" ? "accepted" : outcome.kind === "verify_failed" ? "verify_failed" : "rejected";
+			const outcomeLabel =
+				outcome.kind === "accepted" ? "accepted" :
+				outcome.kind === "accepted_inconclusive" ? "accepted_inconclusive" :
+				outcome.kind === "verify_failed" ? "verify_failed" :
+				"rejected";
 			const details: SignOffDetails = {
 				goal: goal.subject,
 				outcome: outcomeLabel,
 				durationMs,
 				verifyCommand: goal.verify ?? undefined,
 				verifyExitCode: outcome.kind === "verify_failed" ? outcome.exitCode : undefined,
-				judgeModel: state.judgeModel ?? undefined,
+				judgeModel: state.judgeModel ?? "pi default model",
 				reasoning,
 				isError: res.isError,
 			};
@@ -325,8 +329,13 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 			const body = result.content[0]?.type === "text" ? result.content[0].text : "(no output)";
 			if (!details || details.outcome === "running") return new Text(body, 0, 0);
 
-			const icon = details.outcome === "accepted" ? theme.fg("success", "✔") : theme.fg("error", "✗");
-			const outcomeText = details.outcome === "accepted" ? "accepted" : details.outcome === "verify_failed" ? `verify failed (exit ${details.verifyExitCode})` : "rejected";
+			const accepted = details.outcome === "accepted" || details.outcome === "accepted_inconclusive";
+			const icon = accepted ? theme.fg("success", "✔") : theme.fg("error", "✗");
+			const outcomeText =
+				details.outcome === "accepted" ? "accepted" :
+				details.outcome === "accepted_inconclusive" ? "accepted (judge inconclusive)" :
+				details.outcome === "verify_failed" ? `verify failed (exit ${details.verifyExitCode})` :
+				"rejected";
 			const header = `${icon} ${theme.fg("toolTitle", theme.bold("goal signoff "))}${theme.fg("accent", outcomeText)}`;
 			const duration = details.durationMs < 1000 ? `${details.durationMs}ms` : `${(details.durationMs / 1000).toFixed(1)}s`;
 			const sub = [details.judgeModel, duration].filter(Boolean).join(" · ");
@@ -440,7 +449,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 /** Structured details returned by CompleteGoal so renderCall/renderResult can show metadata. */
 interface SignOffDetails {
 	goal: string;
-	outcome: "accepted" | "rejected" | "verify_failed" | "running";
+	outcome: "accepted" | "accepted_inconclusive" | "rejected" | "verify_failed" | "running";
 	phase?: string; // "verifying" | "spawning" | "judging" — while running
 	durationMs: number;
 	verifyCommand?: string;
@@ -493,7 +502,12 @@ async function decideSignOff(
 		}
 	}
 	const verdict = await runJudge(goal, evidence, paths, verifyResult, judgeModel, cwd, signal, onUpdate);
-	const outcome: SignOff = verdict.accept ? { kind: "accepted" } : { kind: "rejected", missing: verdict.missing };
+	const outcome: SignOff =
+		verdict.kind === "accepted"
+			? { kind: "accepted" }
+			: verdict.kind === "inconclusive"
+				? { kind: "accepted_inconclusive", reason: verdict.reason }
+				: { kind: "rejected", missing: verdict.missing };
 	return { outcome, reasoning: verdict.reasoning, durationMs: verdict.durationMs };
 }
 
@@ -513,6 +527,23 @@ function getPiInvocation(args: string[]): { command: string; args: string[] } {
 	return { command: "pi", args };
 }
 
+function extractTextFromContent(content: unknown): string {
+	if (!Array.isArray(content)) return "";
+	const parts: string[] = [];
+	for (const part of content) {
+		if (part && typeof part === "object" && (part as { type?: string }).type === "text") {
+			const text = (part as { text?: string }).text;
+			if (typeof text === "string" && text.trim()) parts.push(text);
+		}
+	}
+	return parts.join("\n\n").trim();
+}
+
+type JudgeResult =
+	| { kind: "accepted"; reasoning: string; durationMs: number }
+	| { kind: "rejected"; missing: string; reasoning: string; durationMs: number }
+	| { kind: "inconclusive"; reason: string; reasoning: string; durationMs: number };
+
 /** Stage 2: a read-only pi subprocess inspects the evidence against the repo and returns a verdict. */
 async function runJudge(
 	goal: Goal,
@@ -523,7 +554,7 @@ async function runJudge(
 	cwd: string,
 	signal: AbortSignal | undefined,
 	onUpdate?: (partial: { content: Array<{ type: "text"; text: string }>; details: SignOffDetails }) => void,
-): Promise<{ accept: boolean; missing: string; reasoning: string; durationMs: number }> {
+): Promise<JudgeResult> {
 	const startedAt = Date.now();
 	const emit = (phase: string, text: string) => {
 		onUpdate?.({
@@ -540,7 +571,7 @@ async function runJudge(
 		evidence,
 		paths,
 	});
-	const args = ["-p", "--no-session", "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
+	const args = ["--mode", "json", "-p", "--no-session", "--tools", JUDGE_TOOLS.join(","), "--exclude-tools", JUDGE_BLOCKED_TOOLS.join(","), "--append-system-prompt", evidenceJudgeSystem];
 	if (judgeModel) args.push("--model", judgeModel);
 	args.push(task);
 
@@ -550,45 +581,123 @@ async function runJudge(
 	// the working dir), leaving a stale directory. The judge should run in a temp dir or inside the
 	// existing repo checkout so it doesn't pollute the user's workspace.
 	const JUDGE_TIMEOUT_MS = 120_000;
-	const output = await new Promise<string>((resolve) => {
+	const judge = await new Promise<{ output: string; error?: string; aborted?: boolean }>((resolve) => {
 		let settled = false;
+		let stdoutBuffer = "";
+		let finalOutput = "";
+		let currentText = "";
+		let stderr = "";
+		let lastStopReason: string | undefined;
+		let lastErrorMessage: string | undefined;
+		const partialOutput = (): string => [finalOutput, currentText, stderr.trim()].filter(Boolean).join("\n\n").trim();
+		const proc = spawn(inv.command, inv.args, { cwd, shell: false, stdio: ["ignore", "pipe", "pipe"], signal });
+		if (!proc.stdout || !proc.stderr) throw new Error("judge subprocess stdio must be piped");
 		const timer = setTimeout(() => {
 			if (!settled) {
 				settled = true;
 				proc.kill();
-				resolve(`VERDICT: reject\nmissing: judge timed out after ${JUDGE_TIMEOUT_MS / 1000}s`);
+				resolve({ output: partialOutput(), error: `judge timed out after ${JUDGE_TIMEOUT_MS / 1000}s` });
 			}
 		}, JUDGE_TIMEOUT_MS);
-		const proc = spawn(inv.command, inv.args, { cwd, shell: false, stdio: ["ignore", "pipe", "pipe"], signal });
-		let out = "";
-		proc.stdout.on("data", (d) => (out += d));
-		proc.stderr.on("data", (d) => (out += d));
-		proc.on("close", () => {
+
+		const processLine = (line: string) => {
+			if (!line.trim()) return;
+			let event: any;
+			try {
+				event = JSON.parse(line);
+			} catch {
+				return;
+			}
+
+			if (event.type === "message_start" && event.message?.role === "assistant") {
+				currentText = "";
+				return;
+			}
+			if (event.type === "message_update" && event.assistantMessageEvent?.type === "text_delta") {
+				currentText += event.assistantMessageEvent.delta ?? "";
+				return;
+			}
+			if (event.type === "message_end" && event.message?.role === "assistant") {
+				const text = extractTextFromContent(event.message.content) || currentText;
+				if (text) finalOutput = text;
+				currentText = "";
+				lastStopReason = typeof event.message.stopReason === "string" ? event.message.stopReason : undefined;
+				lastErrorMessage = typeof event.message.errorMessage === "string" ? event.message.errorMessage : undefined;
+			}
+		};
+
+		proc.stdout.on("data", (data) => {
+			stdoutBuffer += data.toString();
+			const lines = stdoutBuffer.split("\n");
+			stdoutBuffer = lines.pop() || "";
+			for (const line of lines) processLine(line);
+		});
+		proc.stderr.on("data", (data) => {
+			stderr += data.toString();
+		});
+		proc.on("close", (code) => {
 			if (!settled) {
 				settled = true;
 				clearTimeout(timer);
-				resolve(out);
+				if (stdoutBuffer.trim()) processLine(stdoutBuffer);
+				if (lastStopReason === "error" || lastStopReason === "aborted") {
+					resolve({ output: finalOutput, error: lastErrorMessage || `judge model ${lastStopReason}`, aborted: signal?.aborted });
+					return;
+				}
+				if ((code ?? 0) !== 0) {
+					resolve({ output: finalOutput, error: stderr.trim() || finalOutput || `judge subprocess exited ${code ?? 1}` });
+					return;
+				}
+				resolve({ output: finalOutput });
 			}
 		});
 		proc.on("error", (e) => {
 			if (!settled) {
 				settled = true;
 				clearTimeout(timer);
-				resolve(`VERDICT: reject\nmissing: judge subprocess failed: ${e.message}`);
+				resolve({ output: "", error: `judge subprocess failed: ${e.message}`, aborted: signal?.aborted });
 			}
 		});
 	});
 
-	// The subprocess emits ANSI/CSI control codes in -p mode; strip them so they don't leak into `missing`.
-	const clean = output.replace(/\u001b\[[0-9;?]*[ -/]*[@-~]/g, "");
+	if (judge.error) {
+		const partial = judge.output.trim();
+		if (judge.aborted) {
+			return {
+				kind: "rejected",
+				missing: judge.error,
+				reasoning: `VERDICT: reject\nmissing: ${judge.error}`,
+				durationMs: Date.now() - startedAt,
+			};
+		}
+		return {
+			kind: "inconclusive",
+			reason: judge.error,
+			reasoning: partial
+				? `VERDICT: inconclusive\nreason: ${judge.error}\n\npartial judge output:\n${partial}`
+				: `VERDICT: inconclusive\nreason: ${judge.error}`,
+			durationMs: Date.now() - startedAt,
+		};
+	}
 
+	const clean = judge.output.trim();
 	const verdictLine = clean.split("\n").find((l) => /^\s*VERDICT\s*:/i.test(l)) ?? "";
-	const accept = /accept/i.test(verdictLine);
+	const verdictMatch = /^\s*VERDICT\s*:\s*(accept|reject)\s*$/i.exec(verdictLine);
+	if (!verdictMatch) {
+		return {
+			kind: "inconclusive",
+			reason: clean ? "judge returned no exact VERDICT line" : "judge finished without returning any text",
+			reasoning: clean || "VERDICT: inconclusive\nreason: judge finished without returning any text",
+			durationMs: Date.now() - startedAt,
+		};
+	}
 	const missingMatch = clean.match(/missing\s*:\s*([\s\S]*)$/i);
-	const missing = accept ? "" : (missingMatch?.[1].trim() || clean.trim().slice(-500) || "judge gave no reason");
 	// The judge's own words (inspection + verdict), so CompleteGoal can show them. The verdict is at the
 	// end, so keep the tail when it's long.
 	const trimmed = clean.trim();
 	const reasoning = trimmed.length > 1800 ? `...\n${trimmed.slice(-1800)}` : trimmed;
-	return { accept, missing, reasoning, durationMs: Date.now() - startedAt };
+	if (verdictMatch[1].toLowerCase() === "accept") return { kind: "accepted", reasoning, durationMs: Date.now() - startedAt };
+
+	const missing = missingMatch?.[1].trim() || clean.slice(-500) || "judge gave no reason";
+	return { kind: "rejected", missing, reasoning, durationMs: Date.now() - startedAt };
 }

src/plan-file.ts

@@ -235,7 +235,8 @@ export function setGoalStatus(text: string, subject: string, status: GoalStatus)
 export type SignOff =
 	| { kind: "verify_failed"; exitCode: number; outputTail: string }
 	| { kind: "rejected"; missing: string }
-	| { kind: "accepted" };
+	| { kind: "accepted" }
+	| { kind: "accepted_inconclusive"; reason: string };
 
 /** Apply a sign-off outcome to goals.md text: accept flips the goal checkbox to [x] + logs; reject only logs. Pure. */
 export function recordSignOff(
@@ -257,6 +258,15 @@ export function recordSignOff(
 		return { content, message: `Sign-off rejected. Missing:\n${outcome.missing}`, isError: true };
 	}
 	const flipped = setGoalStatus(text, subject, "done");
+	if (outcome.kind === "accepted_inconclusive") {
+		const oneLine = outcome.reason.replace(/\s+/g, " ").trim().slice(0, 200);
+		const content = appendLog(flipped, `${when} signed off "${subject}" (judge inconclusive: ${oneLine})`);
+		return {
+			content,
+			message: `Signed off "${subject}". Marked done in goals.md.\nJudge inconclusive: ${outcome.reason}`,
+			isError: false,
+		};
+	}
 	const content = appendLog(flipped, `${when} signed off "${subject}" (judge accept)`);
 	return { content, message: `Signed off "${subject}". Marked done in goals.md.`, isError: false };
 }

src/prompts.ts

@@ -211,8 +211,10 @@ export const completeGoalDescription =
 	"was avoided; ruling out the failure modes is necessary but not sufficient. Then call this with the " +
 	"goal's desc (the text after 'goal:'). Runs the goal's verify command (if any) then a read-only " +
 	"subagent that inspects that evidence against the repo and the discriminator. On accept, the goal is " +
-	"marked done and logged; on reject, it stays open and you get what is missing. The subagent's " +
-	"reasoning is returned either way.";
+	"marked done and logged; on judge reject, it stays open and you get what is missing. If the " +
+	"subagent times out or its transport/model fails after any verify command has passed, the goal is " +
+	"marked done with a judge-inconclusive log line. The subagent's reasoning or partial output is " +
+	"returned either way.";
 
 export const completeGoalParamDescription = "The goal's desc: the exact text after 'goal:' in its line.";
 

test/plan-file.test.ts

@@ -148,6 +148,18 @@ describe("recordSignOff (CompleteGoal's pure record logic)", () => {
 		expect(doc.log.at(-1)).toBe(`- ${WHEN} signed off "Implement cache layer" (judge accept)`);
 	});
 
+	it("accepted_inconclusive still marks done and logs the judge failure", () => {
+		const r = recordSignOff(SAMPLE, "Implement cache layer", WHEN, {
+			kind: "accepted_inconclusive",
+			reason: "judge timed out after 120s",
+		});
+		expect(r.isError).toBe(false);
+		const doc = parse(r.content);
+		expect(findGoal(doc, "Implement cache layer")?.status).toBe("done");
+		expect(doc.log.at(-1)).toBe(`- ${WHEN} signed off "Implement cache layer" (judge inconclusive: judge timed out after 120s)`);
+		expect(r.message).toContain("Judge inconclusive: judge timed out after 120s");
+	});
+
 	it("verify_failed only logs a reject line, status stays active", () => {
 		const r = recordSignOff(SAMPLE, "Implement cache layer", WHEN, { kind: "verify_failed", exitCode: 1, outputTail: "boom" });
 		expect(r.isError).toBe(true);