wassname/pi-goals
1
0
Fork 0
mirror of https://github.com/wassname/pi-goals.git synced 2026-09-20 13:10:47 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix completed recovery and steer payload validation

Browse Source
This commit is contained in:
wassname
2026-09-09 22:54:48 +08:00
parent 9350d9f54b
commit f49be72a30
10 changed files with 139 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
slop/reviews/solo-recovery/review-round-3-disposition.md
+10
View File
@@ -0,0 +1,10 @@
# Round-3 review disposition
Disposition by the implementation worker against the independent review report at `/home/code/.pi/agent/sessions/--home-code-.pi-agent-git-github.com-wassname-pi-goals--/subagent-artifacts/outputs/52a907b5-4861-4ea5-88fb-90c4a32df3f0/review-round-3.md`, reviewing `9350d9f`.
Both findings are accepted.
1. **F1 — completed manual recovery:** agreed. The manual `/goals reconnect` and `/goals restart` failure catch bypassed the existing `planIsComplete` protection used by automatic recovery. It could demote a completed, signed-off pairing to solo and send the solo continuation follow-up. The catch now keeps a complete plan supervised, preserves recorded sign-offs, and reports the failed recovery without a solo transition. Regression covers both reconnect and restart timeouts.
2. **F2 — oversize steer retention:** agreed. `steer()` recorded a disconnected instruction before `publish()` applied the 16,000-byte wire limit; connected steers also recorded pending state before a publish failure. Steer payload validation now precedes all persistence and pending-state changes, using the same shared 16,000-byte limit as `publish()`. Regression checks both connected and disconnected calls leave entries and sent messages unchanged.
The changes preserve solo fallback for unfinished valid working plans. Full validation and its isolated evidence environment are recorded in `review-round-3-validation.txt` and `round-3-native/`.
slop/reviews/solo-recovery/review-round-3-validation.txt
+23
View File
@@ -0,0 +1,23 @@
Round-3 review fixes validation
Reviewed base: 9350d9f
Environment isolation: every command unsets `PI_SUBAGENT_CHILD` and `PI_GOALS_ROLE`; native RPC evidence is redirected to the fresh, committed `slop/reviews/solo-recovery/round-3-native/` directory. No inherited review-fixes-native evidence directory was used.
$ npx vitest run test/intercom.test.ts test/goals-flow.test.ts
2 files passed; 71 tests passed. The F1 matrix covers both manual reconnect and restart timeouts; F2 covers connected and disconnected oversize steers.
$ env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE PI_GOALS_EVIDENCE_DIR="$PWD/slop/reviews/solo-recovery/round-3-native" npm test
22 files passed; 164 tests passed in 9.66s. The `fatal: not a git repository` stderr is the expected negative preflight regression.
$ env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE PI_GOALS_EVIDENCE_DIR="$PWD/slop/reviews/solo-recovery/round-3-native" npm run typecheck
Passed (`tsc --noEmit`).
$ env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE PI_GOALS_EVIDENCE_DIR="$PWD/slop/reviews/solo-recovery/round-3-native" npm run lint
Passed (Biome checked 42 files; no fixes applied).
$ env -u PI_SUBAGENT_CHILD -u PI_GOALS_ROLE PI_GOALS_EVIDENCE_DIR="$PWD/slop/reviews/solo-recovery/round-3-native" npm run build
Passed (`tsc`).
$ git diff --check
Passed.
slop/reviews/solo-recovery/round-3-native/supervisor-events.jsonl
+19
View File
File diff suppressed because one or more lines are too long
slop/reviews/solo-recovery/round-3-native/supervisor-stderr.txt
View File
slop/reviews/solo-recovery/round-3-native/worker-events.jsonl
+32
View File
@@ -0,0 +1,32 @@
{"id":"planning","type":"response","command":"prompt","success":true}
{"type":"agent_start"}
{"type":"turn_start"}
{"type":"message_start","message":{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788965637965}}
{"type":"message_end","message":{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788965637965}}
{"type":"message_start","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"totalTokens":0,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"pending","timestamp":1788965637979}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_start","contentIndex":0}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_delta","contentIndex":0,"delta":"Test context retained. Actual outputs still need inspection."}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_end","contentIndex":0,"content":"Test context retained. Actual outputs still need inspection."}}
{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965637979,"rawStopReason":"stop"}}
{"type":"turn_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965637979,"rawStopReason":"stop"},"toolResults":[]}
{"type":"agent_end","messages":[{"role":"user","content":[{"type":"text","text":"Retain this planning context for the supervisor fork."}],"timestamp":1788965637965},{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965637979,"rawStopReason":"stop"}],"willRetry":false}
{"type":"agent_settled"}
{"id":"worker-state","type":"response","command":"get_state","success":true,"data":{"model":{"id":"test","name":"Offline test model","reasoning":false,"input":["text"],"contextWindow":16000,"maxTokens":1000,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0},"api":"openai-completions","provider":"offline","baseUrl":"http://127.0.0.1:33921"},"thinkingLevel":"off","isStreaming":false,"isCompacting":false,"steeringMode":"one-at-a-time","followUpMode":"one-at-a-time","sessionFile":"/tmp/goals-native-pair-LFF9w2/agent/sessions/--tmp-goals-native-pair-LFF9w2--/2026-09-09T14-53-57-375Z_01a086a9-4cfe-76b0-998e-3a106c78b226.jsonl","sessionId":"01a086a9-4cfe-76b0-998e-3a106c78b226","autoCompactionEnabled":true,"messageCount":2,"pendingMessageCount":0}}
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"out","message":{"binding":"native-pair-test","role":"worker","kind":"view","id":"55e3995b-2ae7-49ab-b38f-4406fd48a4be","text":"The worker stopped.\n\nInspect the results and judge whether the agreed goal is actually achieved. If unfinished, investigate why the worker stopped and use SteerWorker to send the next useful instruction and resume work. If a verified dependency prevents progress, establish what will resume it and how that will be observed. Do not treat stopping as completion. Consider ApproveGoal only after the results satisfy the goal.\n\nreview trigger: settled\nsource session: /tmp/goals-native-pair-LFF9w2/agent/sessions/--tmp-goals-native-pair-LFF9w2--/2026-09-09T14-53-57-375Z_01a086a9-4cfe-76b0-998e-3a106c78b226.jsonl\nworker model: offline/test\nlatest human direction:\nInspect actual outputs.\ntool calls with no result: none\ntracked background work: No tracked work in this fixture.\n\nnew worker overview (initial or reset view) (VCC algorithmic compression; local # refs index new messages; tool-result bodies omitted; inspect source for evidence):\n[Session Goal]\n- Retain this planning context for the supervisor fork.\n\n[user]\nRetain this planning context for the supervisor fork.\n\n[assistant]\nTest context retained. Actual outputs still need inspection. (#1)\n\nworker view id: 55e3995b-2ae7-49ab-b38f-4406fd48a4be","reason":"settled","backgroundQuiet":true}},"id":"4544a17d","parentId":"2378d250","timestamp":"2026-09-09T14:53:58.674Z"}}
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"ack","message":{"binding":"native-pair-test","role":"supervisor","kind":"received","id":"55e3995b-2ae7-49ab-b38f-4406fd48a4be"}},"id":"9a591425","parentId":"4544a17d","timestamp":"2026-09-09T14:53:58.680Z"}}
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"ack","message":{"binding":"native-pair-test","role":"supervisor","kind":"received","id":"55e3995b-2ae7-49ab-b38f-4406fd48a4be"}},"id":"70b9472b","parentId":"9a591425","timestamp":"2026-09-09T14:53:58.705Z"}}
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"queued","message":{"binding":"native-pair-test","role":"supervisor","kind":"steer","id":"f49295bc-d941-4684-b9f4-20ef94e0bbb1","text":"Read the real outputs before declaring completion."}},"id":"bd79ee34","parentId":"70b9472b","timestamp":"2026-09-09T14:53:58.717Z"}}
{"type":"agent_start"}
{"type":"turn_start"}
{"type":"entry_appended","entry":{"type":"custom","customType":"pi-goals-intercom","data":{"direction":"in","message":{"binding":"native-pair-test","role":"supervisor","kind":"steer","id":"f49295bc-d941-4684-b9f4-20ef94e0bbb1","text":"Read the real outputs before declaring completion."}},"id":"dd0f19bb","parentId":"bd79ee34","timestamp":"2026-09-09T14:53:58.718Z"}}
{"type":"message_start","message":{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788965638718}}
{"type":"message_end","message":{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788965638718}}
{"type":"message_start","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"totalTokens":0,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"pending","timestamp":1788965638719}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_start","contentIndex":0}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_delta","contentIndex":0,"delta":"Test context retained. Actual outputs still need inspection."}}
{"type":"message_update","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"assistantMessageEvent":{"type":"text_end","contentIndex":0,"content":"Test context retained. Actual outputs still need inspection."}}
{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965638719,"rawStopReason":"stop"}}
{"type":"turn_end","message":{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965638719,"rawStopReason":"stop"},"toolResults":[]}
{"type":"agent_end","messages":[{"role":"user","content":[{"type":"text","text":"[supervisor] Read the real outputs before declaring completion."}],"timestamp":1788965638718},{"role":"assistant","content":[{"type":"text","text":"Test context retained. Actual outputs still need inspection."}],"api":"openai-completions","provider":"offline","model":"test","usage":{"input":10,"output":10,"cacheRead":0,"cacheWrite":0,"reasoning":0,"totalTokens":20,"cost":{"input":0,"output":0,"cacheRead":0,"cacheWrite":0,"total":0}},"stopReason":"stop","timestamp":1788965638719,"rawStopReason":"stop"}],"willRetry":false}
{"type":"agent_settled"}
{"type":"extension_ui_request","id":"141c01fc-c3ed-441f-98b7-78220c0aa126","method":"notify","message":"Goal supervision peer disconnected; reconnecting the existing session.","notifyType":"warning"}
slop/reviews/solo-recovery/round-3-native/worker-stderr.txt
View File
src/index.ts
+9 -1
View File
@@ -575,7 +575,15 @@ export function registerWorker(pi: ExtensionAPI): void {
ctx.ui.notify(state.phase === "planning" ? "Planning model restored. Choose Ready when the plan is agreed." : "Goal supervision reconnected; the current plan is unchanged.", "info");
} catch (error) {
if (!current()) return;
if (error instanceof SupervisorFailure && state.phase === "working" && !modelError) { enterSolo(ctx, `Supervisor recovery failed: ${error.message}`); return; }
if (error instanceof SupervisorFailure && state.phase === "working" && !modelError) {
if (planIsComplete(ctx)) {
ctx.ui.notify(`Supervisor recovery failed: ${error.message} The completed plan remains supervised and its recorded sign-offs are unchanged.`, "warning");
updateWidget(ctx);
return;
}
enterSolo(ctx, `Supervisor recovery failed: ${error.message}`);
return;
}
ctx.ui.notify(`Goal recovery failed: ${String(error)} Use /goals reconnect to retry, or /goals restart to explicitly replace the tracked pane.`, "warning");
} finally { if (recoveryCommand === command) recoveryCommand = undefined; }
updateWidget(ctx);
src/intercom.ts
+8 -3
View File
@@ -6,6 +6,7 @@ export type Role = "worker" | "supervisor";
export interface View { id: string; text: string; reason: string; through?: string; backgroundQuiet: boolean }
interface Message { binding: string; role: Role; kind: "hello" | "view" | "steer" | "received"; id: string; text?: string; reason?: string; failure?: string; ready?: boolean; reply?: boolean; through?: string; backgroundQuiet?: boolean }
const STATE = "pi-goals-intercom";
const PAYLOAD_LIMIT_BYTES = 16_000;
export class GoalIntercom {
private channel?: IntercomExtensionChannel;
@@ -183,13 +184,14 @@ export class GoalIntercom {
steer(text: string): { id: string; queued: boolean } {
if (!this.bound) throw new Error("Worker pairing is not active; no instruction was retained.");
if (this.failure || this.peerFailure) throw new Error(this.failure ?? this.peerFailure);
const message: Message = { binding: this.binding, role: this.role, kind: "steer", id: randomUUID(), text };
this.validatePayload(message, "; no instruction was retained.");
if (!this.connected) {
const retained = [...this.pending.values()].find(message => message.text === text);
const retained = [...this.pending.values()].find(pending => pending.text === text);
if (retained) return { id: retained.id, queued: true };
for (const pending of this.pending.values()) this.record("superseded", pending);
this.pending.clear();
}
const message: Message = { binding: this.binding, role: this.role, kind: "steer", id: randomUUID(), text };
this.record("out", message);
this.pending.set(message.id, message);
if (this.connected) this.publish(message);
@@ -229,9 +231,12 @@ export class GoalIntercom {
}
private record(direction: string, message: Message): void { this.pi.appendEntry(STATE, { direction, message }); }
private validatePayload(message: Message, suffix = ""): void {
if (Buffer.byteLength(JSON.stringify(message)) > PAYLOAD_LIMIT_BYTES) throw new Error(`Intercom message exceeds the 16 KB payload limit${suffix}`);
}
private publish(message: Message): void {
if (this.stopped) throw new Error("Intercom session ended.");
if (Buffer.byteLength(JSON.stringify(message)) > 16_000) throw new Error("Supervisor message exceeds the Intercom payload limit.");
this.validatePayload(message);
if (!this.channel?.snapshot().supported) throw new Error("pi-intercom broker does not support extension channels.");
this.channel.publish(message, { audience: "capable" });
}
test/goals-flow.test.ts
+22
View File
@@ -780,6 +780,28 @@ it.each(["startup", "resume", "disconnect"])("allows the existing five-minute re
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
});
it.each(["reconnect", "restart"])("keeps a fully signed-off completed plan supervised when manual %s recovery times out", async action => {
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "setInterval", "clearInterval"] });
const flow = setup([]);
try {
const path = restoredPlan(flow);
writeFileSync(path, readFileSync(path, "utf8").replace("[ ] goal:", "[x] goal:"));
Object.assign(flow.entries.at(-1)!.data as any, { mode: "supervised", soloReason: null, signedOffGoals: ["make the file"] });
flow.transport.replyToHello(false);
await flow.hooks.get("session_start")({}, flow.ctx);
const recovering = flow.commands.get("goals").handler(action, flow.ctx);
await new Promise(resolve => setImmediate(resolve));
await vi.advanceTimersByTimeAsync(300_000);
await recovering;
const state = flow.entries.at(-1)?.data as any;
expect(state).toMatchObject({ phase: "working", mode: "supervised", signedOffGoals: ["make the file"] });
expect(state.approvalId).toBeTruthy();
expect(state.soloReason).toBeNull();
expect(flow.messages.some(message => message.content.includes("Continue useful implementation"))).toBe(false);
expect(flow.notifications.at(-1)).toContain("completed plan remains supervised");
} finally { rmSync(flow.cwd, { recursive: true, force: true }); }
});
it("falls back with the exact reported terminal peer failure without confusing it with worker model failure", async () => {
const flow = setup([]);
try {
test/intercom.test.ts
+16
View File
@@ -106,6 +106,22 @@ it("retries an unanswered active-binding hello twice after delayed channel regis
vi.useRealTimers();
});
it("rejects an oversize steer before retaining it whether connected or disconnected", async () => {
const runtime = setup("supervisor");
runtime.link.markReady();
await runtime.link.waitReady();
const oversized = "x".repeat(16_000);
for (const connected of [true, false]) {
if (!connected) runtime.fixture.connect(false);
const entriesBefore = runtime.entries.length;
const sentBefore = runtime.fixture.sent.length;
expect(() => runtime.link.steer(oversized)).toThrow("16 KB payload limit; no instruction was retained");
expect(runtime.entries).toHaveLength(entriesBefore);
expect(runtime.fixture.sent).toHaveLength(sentBefore);
}
await runtime.hooks.get("session_shutdown")();
});
it("retains only the newest disconnected steer across a supervisor reload", async () => {
const first = setup("supervisor");
first.link.markReady();