mirror of
https://github.com/wassname/pi-goals.git
synced 2026-09-17 12:40:07 +08:00
Block unverified external plan adoption without granting authority
This commit is contained in:
@@ -157,7 +157,9 @@ The parent and worker keep separate native conversations. Worker attachment and
|
||||
|
||||
`OpenGoalWorker` supplies startup only to a newly created stock Pi context. An existing live binding receives no message, so opening it does not replace its conversation or editor draft. The new worker calls `AttachGoalPlan`, reports its exact Intercom identity/model/saved-session path, and waits for a direct parent assignment. Revisions use that same session. A model preference is an instruction for agent-led configuration and verification, not a CLI override; later human changes take precedence.
|
||||
|
||||
There is no custom fresh/recover operation. Inspect stopped workers' saved history and partial results, preserve drafts/queued input, and confirm the exact writer stopped before stock `project.close`/`project.open`. Stock close checks ownership and idle state, but cannot establish editor-draft safety. If uncertain, retain the pane and inspect it. Before a new goals-managed worker, archive obsolete worker identity notes into Log and clear/reattach the plan after confirming stop; do not discard history or replay completed work. Automatic ownership transfer is not provided. — Pi/OpenAI
|
||||
There is no custom fresh/recover operation. Inspect stopped workers' saved history and partial results, preserve drafts/queued input, and confirm the exact writer stopped before stock `project.close`/`project.open`. Stock close checks ownership and idle state, but cannot establish editor-draft safety. If uncertain, retain the pane and inspect it. Preserve history and completed work. Automatic ownership transfer remains unresolved. — Pi/OpenAI
|
||||
|
||||
`/goals attach` now rejects a plan that is not already current in this context, including `attach <path> solo` and reattachment after Clear. The public roster cannot establish its supervisor's ownership; a checkbox or missing roster row is not proof. The command leaves current authority unchanged and provides read-only inspection controls. Keep the original supervisor context when available rather than clearing it to reconnect. Same-current-plan refresh and its separate explicit stopped-writer confirmation for solo recovery remain available. This guard does not solve generic adoption or cross-parent transfer. — Pi/OpenAI
|
||||
|
||||
## Context delivery
|
||||
|
||||
|
||||
+7
-9
@@ -13,7 +13,6 @@ import { FOLD_LINE, foldPlan, GOAL_LINE, planRequirements as requirements } from
|
||||
import { planViews } from "./plan-view.js";
|
||||
import {
|
||||
attachGoalPlanDescription,
|
||||
attachNotice,
|
||||
childPlanAttached,
|
||||
childPlanRole,
|
||||
completeGoalDescription,
|
||||
@@ -311,12 +310,13 @@ export default function mainSupervisor(pi: ExtensionAPI) {
|
||||
pi.sendUserMessage(prompt, { deliverAs: "followUp" });
|
||||
} else pi.sendMessage({ customType: "pi-goals-supervision", content, display: true }, { deliverAs: "nextTurn" });
|
||||
}
|
||||
async function confirmOwnership(ctx: ExtensionContext, target: string, text: string, solo = true): Promise<boolean> {
|
||||
async function confirmOwnership(ctx: ExtensionContext, target: string, text: string): Promise<boolean> {
|
||||
if (target !== state.plan || state.mode === "chat") { ctx.ui.notify(nativeMessages.externalOwnershipUnknown(target, state.worker), "warning"); return false; }
|
||||
if (opening) { ctx.ui.notify("A worker launch/resume is still pending; inspect its result before takeover.", "warning"); return false; }
|
||||
const stamp = generation;
|
||||
const revision = workerRevision;
|
||||
const confirmation = solo ? "Worker confirmed stopped" : "Previous supervisor confirmed stopped";
|
||||
const choice = await ctx.ui.select(solo ? "Confirm all other writers for the current and target plans are stopped (inspect Intercom and their native panes). A missing handle is not proof. Take over in this session?" : "Confirm no other supervisor owns this plan. Preserve any existing worker session and reconnect rather than starting another writer.", [confirmation, "Cancel"]);
|
||||
const confirmation = "Worker confirmed stopped";
|
||||
const choice = await ctx.ui.select("Confirm all other writers for the current and target plans are stopped (inspect Intercom and their native panes). A missing handle is not proof. Take over in this session?", [confirmation, "Cancel"]);
|
||||
if (stamp !== generation || revision !== workerRevision) return false;
|
||||
if (choice !== confirmation) return false;
|
||||
if (readFileSync(target, "utf8") !== text) { ctx.ui.notify("Plan changed during takeover; confirm again.", "warning"); return false; }
|
||||
@@ -695,14 +695,12 @@ export default function mainSupervisor(pi: ExtensionAPI) {
|
||||
notice = true; fullPlanContextDue = true; refresh(ctx);
|
||||
ctx.ui.notify(nativeMessages.samePlanRestored, "info"); return;
|
||||
}
|
||||
if (!solo && ((state.worker && !state.workerStopped) || state.mode === "supervising")) { ctx.ui.notify("Exit and resolve the existing worker before replacing the plan. The current plan is preserved.", "warning"); return; }
|
||||
const noted = /^-\s*worker session:\s*(\S+)/im.exec(foldPlan(text))?.[1];
|
||||
if (!(await confirmOwnership(ctx, target, text, solo))) return;
|
||||
if (!(await confirmOwnership(ctx, target, text))) return;
|
||||
const worker = noted ? { sessionFile: resolve(ctx.cwd, noted) } : state.workerStopped ? state.worker : undefined;
|
||||
state = { mode: solo ? "solo" : "planning", plan: target, worker, workerStopped: solo || (!noted && state.workerStopped) };
|
||||
state = { mode: "solo", plan: target, worker, workerStopped: true };
|
||||
generation++; notice = true; fullPlanContextDue = true; save(); refresh(ctx); watchPlan(ctx);
|
||||
if (solo) enterSolo(ctx);
|
||||
else send(attachNotice(target, false, noted));
|
||||
enterSolo(ctx);
|
||||
return;
|
||||
}
|
||||
if (command === "exit") {
|
||||
|
||||
+4
-4
@@ -263,11 +263,11 @@ export const soloRole = "Solo mode: implement the approved plan directly; do not
|
||||
export function soloNotice(planPath: string): string {
|
||||
return `User authorized solo work on ${planPath} after confirming no other writer remains. ${soloRole}`;
|
||||
}
|
||||
export function attachNotice(planPath: string, solo: boolean, notedWorker: string | undefined): string {
|
||||
return `Attached to the existing plan ${planPath}; read it and its evidence without restarting completed work or re-deriving settled decisions. ${notedWorker ? `Recorded worker session: ${notedWorker}; inspect liveness before resume.` : ""} ${solo ? soloRole : "Present /goals review or /goals ready; no implementation before approval."}`;
|
||||
}
|
||||
|
||||
export const nativeMessages = {
|
||||
externalOwnershipUnknown: (path: string, worker?: { intercomId?: string; sessionFile?: string; paneId?: string; identity?: { paneId?: string } }) => {
|
||||
const pane = worker?.identity?.paneId || worker?.paneId;
|
||||
return `Cannot verify ownership of ${path}: the supported Intercom roster does not identify per-plan supervisors; a missing row is not exit proof. Original supervisor unknown. Current context and authority unchanged; no adoption or takeover authorized. Read-only inspection: read({path:${JSON.stringify(path)}}). ${worker ? `Current worker only (not proof of the target's owner): ${worker.intercomId ? `intercom action:list, locate exact ID ${worker.intercomId}. ` : ""}${worker.sessionFile ? `read({path:${JSON.stringify(worker.sessionFile)}}). ` : ""}${pane ? `herdr pane process-info --pane ${JSON.stringify(pane)}. ` : ""}` : ""}Use /goals status for current references. Return to the original supervisor's saved context only when independently identified; no target can be inferred here.`;
|
||||
},
|
||||
samePlanRestored: "Plan context refreshed; mode and worker binding unchanged. No new work authorized.",
|
||||
workerPause: (paused: boolean) => `Worker ${paused ? "paused" : "unpaused"} locally; no new task submitted and no approval authority granted.`,
|
||||
taskRequired: "Supply an explicit bounded proposed task for a new worker context.",
|
||||
|
||||
+31
-21
@@ -332,15 +332,14 @@ it("rejects an existing zero-byte evidence file", async () => {
|
||||
expect(result.content[0].text).toContain("Empty evidence"); expect(readFileSync(f.path, "utf8")).toBe(before);
|
||||
});
|
||||
|
||||
it("requires actual nonempty evidence, distinguishes manual ticks, and retains reviewed markers through Clear/reattach", async () => {
|
||||
it("requires actual nonempty evidence, distinguishes manual ticks, and retains reviewed markers through same-context restoration", async () => {
|
||||
const f = fixture(); await f.draft(); await f.command("ready");
|
||||
const complete = (goal: string, evidence: string[], signal?: AbortSignal) => f.tools.get("CompleteGoal").execute("t", { goal, evidence, observation: "Inspected exact saved bytes" }, signal, undefined, f.ctx);
|
||||
expect((await complete("first output", ["missing.log"])).content[0].text).toContain("Evidence unavailable");
|
||||
mkdirSync(join(f.ctx.cwd, "evidence")); writeFileSync(join(f.ctx.cwd, "evidence/pass.log"), "actual fixture bytes\n");
|
||||
expect((await complete("first output", ["evidence/pass.log"], AbortSignal.abort())).content[0].text).toContain("Cancelled");
|
||||
await complete("first output", ["evidence/pass.log"]);
|
||||
await f.command("clear");
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Previous supervisor confirmed stopped"); await f.command(`attach ${f.path}`); await f.command("ready");
|
||||
await f.command(`attach ${f.path}`);
|
||||
writeFileSync(f.path, readFileSync(f.path, "utf8").replace("[ ] goal: second", "[x] goal: second"));
|
||||
f.hooks.get("session_start")({}, f.ctx);
|
||||
expect(f.ctx.ui.setStatus).toHaveBeenLastCalledWith("goals", "👀 1/2 goals");
|
||||
@@ -575,21 +574,22 @@ it("requires confirmed worker stop before solo takeover and never lets two write
|
||||
expect(text).toContain("self-verification");
|
||||
});
|
||||
|
||||
it("attaches an existing plan without restarting completed work, and restores its noted worker session", async () => {
|
||||
it("leaves an unverified external plan and its noted worker untouched", async () => {
|
||||
const f = fixture();
|
||||
const existing = join(f.ctx.cwd, "existing.md");
|
||||
writeFileSync(existing, "# Plan\n- preferred worker model: deepseek flash\n- worker session: /tmp/attach-child.jsonl\n- [ ] goal: attached goal\n\n## Log\n- previous progress kept\n");
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Previous supervisor confirmed stopped");
|
||||
await f.command(`attach ${existing}`);
|
||||
expect(f.entries.at(-1).data.mode).toBe("planning");
|
||||
expect(f.entries.at(-1).data.plan).toBe(existing);
|
||||
expect(f.messages.at(-1).message.content).toContain("without restarting completed work");
|
||||
expect(f.messages.at(-1).message.content).toContain("/tmp/attach-child.jsonl");
|
||||
expect(f.entries).toEqual([]);
|
||||
expect(f.messages).toEqual([]);
|
||||
expect(f.ctx.ui.select).not.toHaveBeenCalled();
|
||||
expect(readFileSync(existing, "utf8")).toContain("previous progress kept");
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining("Original supervisor unknown"), "warning");
|
||||
});
|
||||
|
||||
it("attaches directly into solo mode and reports the recorded session in status", async () => {
|
||||
const f = fixture();
|
||||
const existing = join(f.ctx.cwd, "existing.md");
|
||||
it("retains same-current-plan solo recovery and reports the recorded session in status", async () => {
|
||||
const f = fixture(); await f.draft();
|
||||
const existing = f.path;
|
||||
writeFileSync(existing, "# Plan\n- worker session: /tmp/attach-child.jsonl\n- [ ] goal: attached goal\n\n## Log\n");
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Worker confirmed stopped");
|
||||
await f.command(`attach ${existing} solo`);
|
||||
@@ -621,9 +621,10 @@ it.each(["exit", "quit", "clear", "menu"])("%s exits planning with the draft pre
|
||||
expect(f.ctx.ui.setWidget).toHaveBeenLastCalledWith("goals", undefined);
|
||||
expect(readFileSync(f.path, "utf8")).toContain("first output");
|
||||
expect(f.messages.length).toBe(before); // notify only, no model turn started
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Previous supervisor confirmed stopped");
|
||||
await f.command(`attach ${f.path}`);
|
||||
expect(f.entries.at(-1).data.mode).toBe("planning");
|
||||
expect(f.entries.at(-1).data.mode).toBe("chat");
|
||||
expect(f.messages.length).toBe(before);
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining("Cannot verify ownership"), "warning");
|
||||
});
|
||||
|
||||
it("records the preferred worker model as a visible plan preference", async () => {
|
||||
@@ -647,7 +648,7 @@ it.each(["solo", "attach"])("%s takeover cannot bypass confirmation or survive a
|
||||
expect(f.entries.at(-1).data.workerStopped).not.toBe(true);
|
||||
});
|
||||
|
||||
it("attach solo requires stop confirmation for a noted worker even in a fresh session", async () => {
|
||||
it("external attach solo cannot turn a noted worker or stop checkbox into ownership proof", async () => {
|
||||
const f = fixture(); const path = join(f.ctx.cwd, "saved.md");
|
||||
writeFileSync(path, `# Plan\n- worker session: /tmp/known.jsonl\n${f.plan}`);
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Cancel");
|
||||
@@ -655,21 +656,27 @@ it("attach solo requires stop confirmation for a noted worker even in a fresh se
|
||||
expect(f.entries).toHaveLength(0);
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Worker confirmed stopped");
|
||||
await f.command(`attach ${path} solo`);
|
||||
expect(f.entries.at(-1).data).toMatchObject({ mode: "solo", workerStopped: true, worker: { sessionFile: "/tmp/known.jsonl" } });
|
||||
expect(f.entries).toHaveLength(0);
|
||||
expect(f.ctx.ui.select).not.toHaveBeenCalled();
|
||||
expect(f.messages).toEqual([]);
|
||||
expect(readFileSync(path, "utf8")).toContain("worker session: /tmp/known.jsonl");
|
||||
});
|
||||
|
||||
it("retains the stopped session reference across plan changes", async () => {
|
||||
it("retains current solo authority and stopped-session reference when external adoption is blocked", async () => {
|
||||
const f = fixture(); await f.draft(); await f.command("ready");
|
||||
await f.launch({ id: "child", sessionFile: "/tmp/prior.jsonl" });
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Worker confirmed stopped"); await f.command("solo");
|
||||
const other = join(f.ctx.cwd, "another.md"); writeFileSync(other, "- [ ] goal: next\n## Log\n");
|
||||
const before = f.entries.at(-1), messageCount = f.messages.length;
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Previous supervisor confirmed stopped");
|
||||
await f.command(`attach ${other}`);
|
||||
expect(f.entries.at(-1).data).toMatchObject({ mode: "planning", plan: other, workerStopped: true, worker: { sessionFile: "/tmp/prior.jsonl" } });
|
||||
await f.command("ready");
|
||||
expect(f.entries.at(-1).data).toMatchObject({ mode: "solo", plan: f.path, workerStopped: true, worker: { sessionFile: "/tmp/prior.jsonl" } });
|
||||
expect(f.entries.at(-1)).toBe(before); expect(f.messages).toHaveLength(messageCount);
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining('read({path:"/tmp/prior.jsonl"})'), "warning");
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining('herdr pane process-info --pane "native-pane"'), "warning");
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining("locate exact ID child"), "warning");
|
||||
const response = await f.tools.get("OpenGoalWorker").execute("open", { task: "next task" }, undefined, undefined, f.ctx);
|
||||
expect(response.content[0].text).toContain("already recorded");
|
||||
expect(response.content[0].text).toContain("solo");
|
||||
expect(f.entries.at(-1).data.workerStopped).toBe(true);
|
||||
});
|
||||
|
||||
@@ -922,7 +929,7 @@ it("changed plan or shutdown during takeover never grants solo permission", asyn
|
||||
expect(f.entries.at(-1).data.mode).toBe("planning");
|
||||
});
|
||||
|
||||
it("requires explicit supervisor ownership confirmation when attaching an existing plan", async () => {
|
||||
it("blocks unknown external ownership without offering an attestation or launching work", async () => {
|
||||
const f = fixture(); const path = join(f.ctx.cwd, "shared.md");
|
||||
writeFileSync(path, f.plan);
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Cancel");
|
||||
@@ -930,7 +937,10 @@ it("requires explicit supervisor ownership confirmation when attaching an existi
|
||||
expect(f.entries).toHaveLength(0);
|
||||
f.ctx.ui.select.mockResolvedValueOnce("Previous supervisor confirmed stopped");
|
||||
await f.command(`attach ${path}`);
|
||||
expect(f.entries.at(-1).data).toMatchObject({ mode: "planning", plan: path });
|
||||
expect(f.entries).toHaveLength(0);
|
||||
expect(f.ctx.ui.select).not.toHaveBeenCalled();
|
||||
expect(f.messages).toEqual([]);
|
||||
expect(f.ctx.ui.notify).toHaveBeenLastCalledWith(expect.stringContaining("supported Intercom roster does not identify per-plan supervisors"), "warning");
|
||||
});
|
||||
|
||||
it("does not approve cancelled goals or display current completion for an unavailable plan", async () => {
|
||||
|
||||
Reference in New Issue
Block a user