From 489f9b8c358d102a7f8c0f1710dd300b5787dd48 Mon Sep 17 00:00:00 2001 From: wassname Date: Wed, 17 Jun 2026 18:09:03 +0800 Subject: [PATCH] Clean pi-plan references, add judge timeout, fix heading format - Rename spec doc to 2026-06-15_pi-goals.md, update title - Update review.md spec reference - Rename piPlanExtension -> piGoalsExtension in src/index.ts - Add 120s timeout to judge subprocess (was unbounded, caused hang) - Change planInjection heading from 'Goals (goals.md):' to '.pi/goals.md:' - Add FIXMEs for tool label, progress visibility, heading format --- docs/reviews/review.md | 2 +- ...6-15_pi-plan.md => 2026-06-15_pi-goals.md} | 2 +- src/index.ts | 37 +++++++++++++++++-- src/prompts.ts | 6 ++- 4 files changed, 40 insertions(+), 7 deletions(-) rename docs/spec/{2026-06-15_pi-plan.md => 2026-06-15_pi-goals.md} (99%) diff --git a/docs/reviews/review.md b/docs/reviews/review.md index e385d18..b0d1965 100644 --- a/docs/reviews/review.md +++ b/docs/reviews/review.md @@ -1,4 +1,4 @@ -Code review against spec `docs/spec/2026-06-15_pi-plan.md`. +Code review against spec `docs/spec/2026-06-15_pi-goals.md`. --- diff --git a/docs/spec/2026-06-15_pi-plan.md b/docs/spec/2026-06-15_pi-goals.md similarity index 99% rename from docs/spec/2026-06-15_pi-plan.md rename to docs/spec/2026-06-15_pi-goals.md index 87d8d13..bdde0c0 100644 --- a/docs/spec/2026-06-15_pi-plan.md +++ b/docs/spec/2026-06-15_pi-goals.md @@ -1,4 +1,4 @@ -# pi-plan — design spec +# pi-goals — design spec Working title. A pi extension: set up goals (with subtasks and evidence) through plan mode, work them autonomously, and sign a goal off only when a check passes. One markdown file holds everything. The form guides a process; it does not police one. Successor to `pi-lgtm`, deliberately smaller. diff --git a/src/index.ts b/src/index.ts index b60f38d..d782649 100644 --- a/src/index.ts +++ b/src/index.ts @@ -84,7 +84,7 @@ interface PlanState { judgeModel: string | null; } -export default function piPlanExtension(pi: ExtensionAPI): void { +export default function piGoalsExtension(pi: ExtensionAPI): void { let state: PlanState = { isPlanMode: false, objective: null, judgeModel: null }; // Reminder cadence: fire when an active goal exists but goals.md was not touched since last turn. let lastInjectedPlan = ""; @@ -270,6 +270,12 @@ export default function piPlanExtension(pi: ExtensionAPI): void { pi.registerTool({ name: "CompleteGoal", + // FIXME(label): "Complete goal" is ambiguous in the TUI — the user sees it running with no + // hint that it spawns a read-only subagent. Change to "Subagent goal signoff" or similar so the + // running tool reads as an action, not just a checkbox flip. + // FIXME(progress): while the judge runs (potentially 10-120s), the user only sees "Working". Surface + // judge progress — e.g. set the tool label to "Subagent goal signoff (judging...)" via onUpdate, + // or emit a status notification so the user knows what's happening and can tell a hang from slowness. label: "Complete goal", description: completeGoalDescription, parameters: Type.Object({ @@ -451,13 +457,38 @@ async function runJudge( args.push(task); const inv = getPiInvocation(args); + // FIXME(hang): no timeout — if the judge subprocess stalls, this promise never resolves and the + // user sees "Working" indefinitely with no way to know what's happening. Add a setTimeout that + // resolves with a reject verdict after, say, 120s, and surface progress (e.g. stderr lines) so + // the user can tell the judge is still alive. See also: no progress indication in TUI while judging. + const JUDGE_TIMEOUT_MS = 120_000; const output = await new Promise((resolve) => { + let settled = false; + const timer = setTimeout(() => { + if (!settled) { + settled = true; + proc.kill(); + resolve(`VERDICT: reject\nmissing: judge timed out after ${JUDGE_TIMEOUT_MS / 1000}s — the subagent did not return a verdict`); + } + }, JUDGE_TIMEOUT_MS); const proc = spawn(inv.command, inv.args, { cwd, shell: false, stdio: ["ignore", "pipe", "pipe"], signal }); let out = ""; proc.stdout.on("data", (d) => (out += d)); proc.stderr.on("data", (d) => (out += d)); - proc.on("close", () => resolve(out)); - proc.on("error", (e) => resolve(`VERDICT: reject\nmissing: judge subprocess failed: ${e.message}`)); + proc.on("close", () => { + if (!settled) { + settled = true; + clearTimeout(timer); + resolve(out); + } + }); + proc.on("error", (e) => { + if (!settled) { + settled = true; + clearTimeout(timer); + resolve(`VERDICT: reject\nmissing: judge subprocess failed: ${e.message}`); + } + }); }); // The subprocess emits ANSI/CSI control codes in -p mode; strip them so they don't leak into `missing`. diff --git a/src/prompts.ts b/src/prompts.ts index cd4e708..841bf2d 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -117,14 +117,16 @@ export function planInjection(p: { counts: { done: number; open: number }; }): string { if (!p.activeGoal) { - return `Goals (goals.md): ${p.title}\nNo active goal. ${p.counts.open} open, ${p.counts.done} done. Pick the next goal (set its checkbox to [/]) or run /goals.`; + // FIXME(heading): user wants the heading to show ".pi/goals.md: " so the filename is explicit + // even in the injection. Currently says "Goals (goals.md):" which is close but not the same. + return `.pi/goals.md: ${p.title}\nNo active goal. ${p.counts.open} open, ${p.counts.done} done. Pick the next goal (set its checkbox to [/]) or run /goals.`; } const subtasks = p.activeGoal.openSubtasks.length ? p.activeGoal.openSubtasks.map((s) => ` - [ ] ${s}`).join("\n") : " (no open subtasks)"; const disc = p.activeGoal.discriminator.length ? p.activeGoal.discriminator.join("; ") : "(none set)"; return `\ -Goals (goals.md): ${p.title} +.pi/goals.md: ${p.title} Active goal: ${p.activeGoal.subject} discriminator (the success test): ${disc} Open subtasks: