Separate provisional planning from intentional acceptance

This commit is contained in:
wassname2
2026-09-21 11:46:24 +08:00
parent 54268feb26
commit adc50dc322
6 changed files with 70 additions and 79 deletions
+21 -51
View File
@@ -8,7 +8,6 @@ import { visibleWidth } from "@earendil-works/pi-tui";
import { openProjectPane } from "pi-subagents/project-panes";
import { afterEach, expect, it, vi } from "vitest";
import goalsExtension from "../src/index.js";
import { goalCheckInWake, planDrafting, reportGoalEventDescription, supervisor } from "../src/prompts.js";
import { buildWorkerView } from "../src/worker-view.js";
vi.mock("pi-subagents/project-panes", () => ({ openProjectPane: vi.fn(async () => ({ ok: true, data: { bindingPath: "/project/.pi/subagents/project-pane.json", disposition: "opened", binding: { paneId: "native-pane", projectRoot: "/project", command: "pi" } } })) }));
@@ -78,39 +77,6 @@ function fixture(child = false) {
return { ctx, pi, hooks, tools, commands, messages, command, get path() { return path; }, plan, draft, shutdown, changed, atomicWrite, get entries() { return entries.filter(entry => entry.customType === "pi-goals-main-supervisor-v1"); }, start, launch, channel, event: (event: any) => registration.onEvent(event) };
}
it("puts exploration, protected decisions and grilling before the plan draft", () => {
const explore = planDrafting.indexOf("1. Explore first");
const protectedDecisions = planDrafting.indexOf("2. Infer which decisions the human reserves");
const grill = planDrafting.indexOf("3. Then use the grilling skill");
const draft = planDrafting.indexOf("5. When every goal");
expect(explore).toBeGreaterThan(0);
expect(explore).toBeLessThan(protectedDecisions);
expect(protectedDecisions).toBeLessThan(grill);
expect(grill).toBeLessThan(draft);
expect(planDrafting).toContain("Read every user-supplied link and resource");
expect(planDrafting).toContain("publication approval or editorial voice");
});
it("leaves stop events informal until the supervisor chooses full review", () => {
expect(reportGoalEventDescription).toContain("never create formal review by themselves");
expect(reportGoalEventDescription).toContain("only the supervisor can choose full review");
const role = supervisor("worker", "/tmp/plan.md", "parent");
expect(role).toContain("Goal, Changed, Judgment, Next, Need from you");
expect(role).toContain("personally perform the high-level diagnosis, research interpretation, experimental design and consequential judgment");
expect(role).toContain("do not outsource the central reasoning");
expect(role).toContain("Do not invent pass/fail thresholds or turn a ranking metric");
expect(role).toContain("never stopped, retasked, closed or reviewed without explicit user authority");
expect(role).toContain("Humour is a reflective meta-learning mechanism");
expect(role).toContain("The human can inspect, talk to and change /model in the worker pane directly");
expect(role).toContain("pi-goals owns attachment/report correlation, not generic writer concurrency");
expect(role).toContain("Only your third choice creates review paperwork");
expect(role).toContain("Never wait on an inferred or nonexistent pane");
expect(role).toContain("unable to display a secret file does not make an already authorized credential-backed command impossible");
expect(role).toContain("python-dotenv or a shell-sourced .env");
expect(role).toContain("without reading, printing or sending secret values");
expect(goalCheckInWake).toContain("Use formal review only when you choose to allow it to stop");
});
it("shows incremental VCC Markdown without raw tool results or compaction dumps", async () => {
initTheme("dark");
const f = fixture(true), history = f.ctx.sessionManager.getBranch(), timestamp = new Date().toISOString();
@@ -348,26 +314,30 @@ it("discusses plan changes only during planning", async () => {
await f.command("discuss"); expect(f.messages).toHaveLength(before);
});
it("automatically proposes a changed settled draft once and preserves Discuss", async () => {
it("keeps provisional drafts and interview updates separate from intentional acceptance", async () => {
const f = fixture(); await f.draft();
f.ctx.ui.select.mockResolvedValueOnce("Discuss");
await f.hooks.get("agent_settled")({}, f.ctx);
expect(f.messages.some(m => m.message.customType === "goal-plan-proposal" && m.message.content === f.plan)).toBe(true);
expect(f.entries.at(-1).data.mode).toBe("planning");
const calls = f.ctx.ui.select.mock.calls.length;
await f.hooks.get("agent_settled")({}, f.ctx);
expect(f.ctx.ui.select).toHaveBeenCalledTimes(calls);
writeFileSync(f.path, f.plan.replace("first output", "revised output"));
f.ctx.ui.select.mockResolvedValueOnce("Ready");
await f.hooks.get("agent_settled")({}, f.ctx);
expect(f.entries.at(-1).data.mode).toBe("supervising");
});
it("does not propose an empty draft or a delegated worker's plan", async () => {
const f = fixture(); await f.command("new");
const review = async () => {
await f.tools.get("RequestPlanReview").execute("review", {}, undefined, undefined, f.ctx);
await f.hooks.get("agent_settled")({}, f.ctx);
};
for (const text of [f.plan, f.plan + "## Interview\nTODO: consequential choice unanswered.\n", f.plan.replace("first output", "revised output")]) {
writeFileSync(f.path, text);
await f.hooks.get("agent_settled")({}, f.ctx);
}
await f.tools.get("RequestPlanReview").execute("stale", {}, undefined, undefined, f.ctx);
writeFileSync(f.path, f.plan + "\n## Interview\nNew unresolved choice.\n");
await f.hooks.get("agent_settled")({}, f.ctx);
expect(f.ctx.ui.select).not.toHaveBeenCalled();
const child = fixture(true); await child.hooks.get("agent_settled")({}, child.ctx);
expect(f.messages.some(m => m.message.customType === "goal-plan-proposal")).toBe(false);
f.ctx.ui.select.mockResolvedValueOnce("Discuss"); await review();
expect(f.entries.at(-1).data.mode).toBe("planning");
expect(f.messages.some(m => m.message.customType === "goal-plan-proposal")).toBe(true);
await f.hooks.get("agent_settled")({}, f.ctx);
expect(f.ctx.ui.select).toHaveBeenCalledTimes(1);
f.ctx.ui.select.mockResolvedValueOnce("Ready"); await review();
expect(f.entries.at(-1).data.mode).toBe("supervising");
const child = fixture(true);
await child.tools.get("RequestPlanReview").execute("child", {}, undefined, undefined, child.ctx);
expect(child.ctx.ui.select).not.toHaveBeenCalled();
});
+12 -4
View File
@@ -1,6 +1,6 @@
import { type ChildProcessWithoutNullStreams, execFileSync, spawn } from "node:child_process";
import { once } from "node:events";
import { cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
import { createServer } from "node:http";
import { tmpdir } from "node:os";
import { basename, join, resolve } from "node:path";
@@ -106,7 +106,7 @@ it("plans and reviews the same worker across failure, delivery retry and reload"
function start(role: "parent" | "worker", sessionFile?: string) {
const child = spawn(resolve("node_modules/.bin/pi"), ["--mode", "rpc", "--no-extensions", "--model", "offline/test",
"-e", resolve("test/fixtures/offline-model.ts"), "-e", resolve("src/index.ts"),
"-e", resolve("node_modules/pi-intercom/index.ts"), "-e", resolve("node_modules/@jl1990/pi-scheduler/extensions/scheduler/index.ts"),
"-e", resolve("node_modules/pi-intercom/index.ts"), "-e", realpathSync(resolve("node_modules/@jl1990/pi-scheduler/extensions/scheduler/index.ts")),
...(role === "worker" ? ["-e", resolve("node_modules/pi-subagents/index.ts")] : []),
...(sessionFile ? ["--session", sessionFile] : [])], { cwd, env: {
...Object.fromEntries(Object.entries(process.env).filter(([name]) => !name.startsWith("PI_SUBAGENT_") && !name.startsWith("PI_GOALS_") && !name.startsWith("HERDR_"))),
@@ -139,6 +139,13 @@ it("plans and reviews the same worker across failure, delivery retry and reload"
let parent = start("parent"), worker: RpcClient | undefined;
try {
parent.send({ type: "prompt", id: "new", message: "/goals new deliver the greeting" });
await parent.waitFor(m => m.type === "agent_settled");
expect(parent.messages.some(isSelect)).toBe(false);
const interviewed = plan + "\n## Interview\nProvisional: output format depends on the user's answer.\n";
await run(parent, "parent", call("write", { path: planPath, content: interviewed }), { content: "Provisional draft saved. Which greeting format do you want?" });
expect(parent.messages.some(isSelect)).toBe(false);
expect(records((await state(parent)).sessionFile, "pi-goals-main-supervisor-v1").at(-1).mode).toBe("planning");
parent.send({ type: "prompt", id: "review-edit", message: "/goals review" });
const proposal = await parent.waitFor(isSelect);
parent.send({ type: "extension_ui_response", id: proposal.id, value: "Edit" });
const editor = await parent.waitFor(isEditor);
@@ -146,14 +153,15 @@ it("plans and reviews the same worker across failure, delivery retry and reload"
const editAt = parent.messages.length;
parent.send({ type: "extension_ui_response", id: editor.id, value: approved });
await parent.waitFor(m => m.type === "extension_ui_request" && m.method === "setWidget", editAt);
expect(readFileSync(planPath, "utf8")).toBe(approved); expect(requests.parent).toHaveLength(2);
expect(readFileSync(planPath, "utf8")).toBe(approved);
const discussion = parent.messages.length;
parent.send({ type: "prompt", id: "discuss", message: "/goals review" });
const discuss = await parent.waitFor(isSelect, discussion);
parent.send({ type: "extension_ui_response", id: discuss.id, value: "Discuss" });
await parent.waitFor(m => m.type === "response" && m.command === "prompt", discussion);
const discussionAt = parent.messages.length;
parent.send({ type: "prompt", id: "discussion", message: "Keep the edited requirement." });
replies.parent.push(call("RequestPlanReview", {}), { content: "Human decision recorded." });
parent.send({ type: "prompt", id: "discussion", message: "Keep the edited requirement. Present the settled draft for acceptance." });
const ready = await parent.waitFor(isSelect, discussionAt);
expect(systemText(requests.parent.at(-1)!)).toContain("Plan only in");
const startupState = await state(parent), checkInName = `goals-${startupState.sessionId}`;