Cancel stale completion and recheck Ready plan content

slop/reviews/full-profile-supervisor/review.md

@@ -32,7 +32,7 @@ git diff --check
 
 ## Limits and remaining acceptance
 
-This verifies normal discovery using a deterministic local model and an isolated custom extension. It does not prove a real user's complete profile respects the role instruction. Arbitrary extensions retain their own hooks/side effects/tool policies; tools can still write if the model disregards its task. Parent-owned real full-profile Herdr acceptance and independent reviewer gate remain required. No panes were opened, reloaded or operated.
+This verifies normal discovery using a deterministic local model and an isolated custom extension. It does not prove a real user's complete profile respects the role instruction. Arbitrary extensions retain their own hooks/side effects/tool policies; tools can still write if the model disregards its task. The parent still needs to run real full-profile Herdr acceptance and obtain an independent review. No panes were opened, reloaded or operated.
 
 Issue #6 cancellation, Ready content drift, compaction delivery, fresh-shell role restoration and other lifecycle bugs are intentionally not fixed in this scoped task. Existing running supervisors retain their already-loaded profile until appropriately restarted/reloaded by their owner.
 

slop/reviews/lifecycle-recovery/cancellation-before.txt

@@ -0,0 +1,33 @@
+
+ RUN  v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
+
+ ❯ test/goals-flow.test.ts (21 tests | 1 failed | 20 skipped) 71ms
+     × accepts only an approval for the exact clean commit and goal block 70ms
+
+⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
+
+ FAIL  test/goals-flow.test.ts > /goals flow > accepts only an approval for the exact clean commit and goal block
+AssertionError: expected false to be true // Object.is equality
+
+- Expected
++ Received
+
+- true
++ false
+
+ ❯ test/goals-flow.test.ts:318:38
+    316|    const cancelled = flow.tools.get("CompleteGoal").execute("cancelled…
+    317|    controller.abort(); // Cancel while the background-state lookup yie…
+    318|    expect((await cancelled).isError).toBe(true);
+       |                                      ^
+    319|    expect(readFileSync(planPath, "utf8")).toBe(beforeCancel);
+    320|    expect((await flow.tools.get("CompleteGoal").execute("already-cance…
+
+⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
+
+
+ Test Files  1 failed (1)
+      Tests  1 failed | 20 skipped (21)
+   Start at  12:28:54
+   Duration  734ms (transform 273ms, setup 0ms, import 544ms, tests 71ms, environment 0ms)
+

slop/reviews/lifecycle-recovery/checkpoint-validation.txt

@@ -0,0 +1,26 @@
+
+> @wassname2/pi-goals@0.2.2 test
+> vitest run
+
+
+ RUN  v4.1.9 /home/code/.pi/agent/git/github.com/wassname/pi-goals
+
+
+ Test Files  19 passed (19)
+      Tests  110 passed (110)
+   Start at  12:30:42
+   Duration  3.60s (transform 4.23s, setup 0ms, import 8.38s, tests 8.31s, environment 2ms)
+
+
+> @wassname2/pi-goals@0.2.2 typecheck
+> tsc --noEmit
+
+
+> @wassname2/pi-goals@0.2.2 lint
+> biome check src/ test/
+
+Checked 37 files in 34ms. No fixes applied.
+
+> @wassname2/pi-goals@0.2.2 build
+> tsc
+

src/index.ts

@@ -598,11 +598,17 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 			const attempt = {};
 			readyAttempt = attempt;
 			const current = () => !intercom.ended && readyAttempt === attempt && state.planVersion === version;
+			const checkApprovedPlan = () => {
+				if (readPlan(ctx) !== plan) throw new Error("The plan changed after Ready was selected. Review the changed plan and select Ready again; the existing supervisor pane is retained.");
+			};
 			try {
+				checkApprovedPlan();
 				await startSupervisor(ctx, current);
 				if (!current()) return;
+				checkApprovedPlan();
 				await restoreModel("worker", ctx);
 				if (!current()) return;
+				checkApprovedPlan();
 				state = { ...state, phase: "working" };
 				resyncReason = "The plan was approved.";
 				persist();
@@ -610,12 +616,14 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 				startWorkerTimers(ctx);
 				await publishWorkerView(ctx, "ready");
 				if (!current()) return;
+				checkApprovedPlan();
 				updateWidget(ctx);
 				ctx.ui.notify(`Visible supervisor opened in Herdr pane ${state.supervisorPaneId}.`, "info");
 				pi.sendUserMessage("The plan is approved. Begin implementation as the worker.");
 			} catch (error) {
 				if (!current()) return;
 				intercom.markNotReady();
+				stopWorkerTimers();
 				ctx.ui.notify(`Goal supervisor could not start: ${error instanceof Error ? error.message : String(error)} Use /goals reconnect to retry, or /goals restart to replace the tracked pane.`, "warning");
 				state = { ...state, phase: "planning" };
 				persist();
@@ -673,11 +681,15 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 		parameters: Type.Object({
 			goal: Type.String({ description: completeGoalParamDescription }),
 		}),
-		async execute(_id, params, _signal, _onUpdate, ctx) {
+		async execute(_id, params, signal, _onUpdate, ctx) {
+			if (signal?.aborted) return result("Goal sign-off cancelled; no completion recorded.", true);
+			const binding = state.approvalId;
+			const version = state.planVersion;
 			if (state.phase !== "working") return result("Planning is not approved. Choose Ready before signing off a goal.", true);
 			if (pauseReason()) return result(`Goal sign-off blocked: ${pauseReason()}`, true);
 			if (!state.approvalId) return result("Goal sign-off blocked: no current supervisor review.", true);
 			const background = await backgroundState(pi);
+			if (signal?.aborted || intercom.ended || state.approvalId !== binding || state.planVersion !== version || state.phase !== "working") return result("Goal sign-off cancelled or superseded; no completion recorded.", true);
 			if (intercom.ended || !background.quiet || pauseReason()) return result(`Goal sign-off blocked: ${pauseReason() ?? background.description}`, true);
 			const plan = readPlan(ctx);
 			if (!plan.trim()) return result(`No plan file at ${planRel(ctx)}. Run /goals to draft one.`, true);
@@ -704,6 +716,7 @@ export default function piGoalsExtension(pi: ExtensionAPI): void {
 			})) return result("Goal sign-off blocked: no matching supervisor approval checkpoint. Request a fresh supervisor review.", true);
 			const ticked = tickGoal(plan, params.goal);
 			if (!ticked) return result(`No unique exact goal line matched "${params.goal}" in ${planRel(ctx)}.`, true);
+			if (signal?.aborted) return result("Goal sign-off cancelled; no completion recorded.", true);
 			writePlan(ctx, appendLog(ticked, `${stamp()} mechanically signed off "${params.goal}" after matching supervisor approval`));
 			state = { ...state, signedOffGoals: [...state.signedOffGoals.filter(goal => goal !== goalKey(params.goal)), goalKey(params.goal)] };
 			persist();

test/goals-flow.test.ts

@@ -311,6 +311,14 @@ describe("/goals flow", () => {
 				supervisor: { sessionId: "supervisor", runId: null }, timestamp: new Date().toISOString(),
 			});
 			writeFileSync(planPath, `${plan}- Appended manual log after approval.\n1. [ ] goal: make the file\n2. [ ] goal: historical only\n`);
+			const controller = new AbortController();
+			const beforeCancel = readFileSync(planPath, "utf8");
+			const cancelled = flow.tools.get("CompleteGoal").execute("cancelled", { goal }, controller.signal, undefined, flow.ctx);
+			controller.abort(); // Cancel while the background-state lookup yields.
+			expect((await cancelled).isError).toBe(true);
+			expect(readFileSync(planPath, "utf8")).toBe(beforeCancel);
+			expect((await flow.tools.get("CompleteGoal").execute("already-cancelled", { goal }, controller.signal, undefined, flow.ctx)).isError).toBe(true);
+			expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working", signedOffGoals: [] });
 			const signed = await flow.tools.get("CompleteGoal").execute("id", { goal }, undefined, undefined, flow.ctx);
 			expect(signed.isError).toBe(false);
 			expect(readFileSync(planPath, "utf8")).toContain("1. [x] goal: make the file");
@@ -393,6 +401,24 @@ it("shows a missing resumed supervisor, pauses writes, and automatically unpause
 	} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
 });
 
+it.each(["launch", "model"])("rejects plan content changes during Ready %s without replacing its pane", async (stage) => {
+	const flow = setup(["Ready", "Ready"]);
+	try {
+		await flow.commands.get("goals").handler("make the file", flow.ctx);
+		const path = approvedPlan(flow.cwd);
+		const mutate = () => writeFileSync(path, readFileSync(path, "utf8").replace("make the file", "make a different report"));
+		if (stage === "launch") openSupervisorPane.mockImplementationOnce(async () => { mutate(); return "pane-2"; });
+		else flow.pi.setModel.mockImplementationOnce(async () => { mutate(); return true; });
+		await flow.hooks.get("agent_settled")({}, flow.ctx);
+		expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "planning", supervisorPaneId: "pane-2" });
+		expect(flow.messages.some(message => message.content.includes("Begin implementation"))).toBe(false);
+		expect(flow.notifications.join("\n")).toContain("plan changed after Ready");
+		await flow.hooks.get("agent_settled")({}, flow.ctx);
+		expect(openSupervisorPane).toHaveBeenCalledTimes(1);
+		expect(flow.entries.at(-1)?.data).toMatchObject({ phase: "working" });
+	} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
+});
+
 it("times out stale Ready retries in five seconds, without replacing the pane automatically", async () => {
 	vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "setInterval", "clearInterval"] });
 	const flow = setup(["Ready", "Ready"]);