diff --git a/slop/reviews/full-profile-supervisor/review.md b/slop/reviews/full-profile-supervisor/review.md index 26d526c..9fa644c 100644 --- a/slop/reviews/full-profile-supervisor/review.md +++ b/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. diff --git a/slop/reviews/lifecycle-recovery/cancellation-before.txt b/slop/reviews/lifecycle-recovery/cancellation-before.txt new file mode 100644 index 0000000..f1c03ec --- /dev/null +++ b/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) + diff --git a/slop/reviews/lifecycle-recovery/checkpoint-validation.txt b/slop/reviews/lifecycle-recovery/checkpoint-validation.txt new file mode 100644 index 0000000..5de9051 --- /dev/null +++ b/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 + diff --git a/src/index.ts b/src/index.ts index 12236c3..42fd399 100644 --- a/src/index.ts +++ b/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(); diff --git a/test/goals-flow.test.ts b/test/goals-flow.test.ts index 73880f8..482c0bf 100644 --- a/test/goals-flow.test.ts +++ b/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"]);