diff --git a/README.md b/README.md index 56ed406..1a94278 100644 --- a/README.md +++ b/README.md @@ -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 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 diff --git a/src/index.ts b/src/index.ts index c3ccc70..ab439bd 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,8 +4,8 @@ import { createHash, randomUUID } from "node:crypto"; import { type FSWatcher, mkdirSync, readdirSync, readFileSync, watch, writeFileSync } from "node:fs"; import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; -import { type ExtensionAPI, type ExtensionContext, SessionManager, withFileMutationQueue } from "@earendil-works/pi-coding-agent"; -import { Text, truncateToWidth } from "@earendil-works/pi-tui"; +import { type ExtensionAPI, type ExtensionContext, getMarkdownTheme, keyHint, SessionManager, withFileMutationQueue } from "@earendil-works/pi-coding-agent"; +import { Markdown, truncateToWidth } from "@earendil-works/pi-tui"; import { INTERCOM_EXTENSION_REGISTER_EVENT, type IntercomExtensionChannel, type IntercomExtensionRegistration } from "pi-intercom/extension-api.js"; import { openProjectPane } from "pi-subagents/project-panes"; import { Type } from "typebox"; @@ -14,7 +14,6 @@ import { FOLD_LINE, foldPlan, GOAL_LINE, planRequirements as requirements } from import { planViews } from "./plan-view.js"; import { attachGoalPlanDescription, - attachNotice, childPlanAttached, childPlanRole, completeGoalDescription, @@ -285,7 +284,7 @@ export default function mainSupervisor(pi: ExtensionAPI) { } function restore(ctx: ExtensionContext) { cancelCheckInRemoval(); agentRunActive = false; - notices.restore(ctx); + notices.restore(ctx, [REVIEW]); generation++; state = initial(); for (const entry of ctx.sessionManager.getBranch()) { @@ -316,12 +315,13 @@ export default function mainSupervisor(pi: ExtensionAPI) { pi.sendMessage({ customType: "pi-goals-supervision", content, display: !collapse }, { deliverAs: "nextTurn" }); } } - async function confirmOwnership(ctx: ExtensionContext, target: string, text: string, solo = true): Promise { + async function confirmOwnership(ctx: ExtensionContext, target: string, text: string): Promise { + 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; } @@ -361,12 +361,12 @@ export default function mainSupervisor(pi: ExtensionAPI) { paneId: process.env.HERDR_PANE_ID ?? "", model: ctx.model ? ctx.model.provider + "/" + ctx.model.id : undefined }; } const records = (ctx: ExtensionContext, type: string): T[] => ctx.sessionManager.getBranch().flatMap(entry => entry.type === "custom" && entry.customType === type ? [entry.data as T] : []); - const pendingReports = (ctx: ExtensionContext) => records(ctx, REPORT).filter(report => !records(ctx, REVIEW).some(review => reviewedReportId(review) === report.id)); + const pendingReports = (ctx: ExtensionContext) => records(ctx, REPORT).filter(report => + !records(ctx, REVIEW).some(review => reviewedReportId(review) === report.id)); const reportLabel = (report: Report) => { const revision = report.id.slice(report.id.lastIndexOf(":") + 1); - const task = report.task ? `${report.task.trim().replace(/\s+/g, " ").slice(0, 100)} — ` : ""; - const summary = report.text.trim().replace(/\s+/g, " ").slice(0, 160) || "No worker summary"; - return `revision ${revision}: ${task}${summary} (reportId ${report.id})`; + const plainTask = report.task?.replace(/\[([^\]]+)\]\([^)]+\)/g, "$1").replace(/[*_`~<>]/g, "").trim().replace(/\s+/g, " ").slice(0, 100); + return `revision ${revision}${plainTask ? ` — ${plainTask}` : ""} (reportId ${report.id})`; }; function recordReport(ctx: ExtensionContext, report: Report, wake = true) { if (records(ctx, REPORT).some(saved => saved.id === report.id)) return; @@ -377,12 +377,22 @@ export default function mainSupervisor(pi: ExtensionAPI) { function remindReports(ctx: ExtensionContext) { if (state.child || state.mode !== "supervising") return; const reports = pendingReports(ctx); - const fingerprint = digest(JSON.stringify(reports.map(report => report.id))); - if (!reports.length || records(ctx, REVIEW_REMINDER).at(-1) === fingerprint) return; - pi.appendEntry(REVIEW_REMINDER, fingerprint); + const reportIds = reports.map(report => report.id); + const branch = ctx.sessionManager.getBranch(); + const sinceReminder = branch.slice(branch.map(entry => entry.type === "custom" ? entry.customType : "").lastIndexOf(REVIEW_REMINDER) + 1); + if (!sinceReminder.some(entry => entry.type === "custom" && entry.customType === REPORT && reportIds.includes((entry.data as Report).id))) return; + pi.appendEntry(REVIEW_REMINDER); send(pendingReportReviews(reports.map(reportLabel))); } - pi.registerEntryRenderer(REVIEW, entry => new Text((entry.data as ReportReview).content, 0, 0)); + pi.registerEntryRenderer(REVIEW, (entry, { expanded }, theme) => { + const review = entry.data as ReportReview; + if (expanded) return new Markdown(review.content, 0, 0, getMarkdownTheme()); + const revision = reviewedReportId(review)?.split(":").at(-1) ?? "unknown"; + return { + render: (width) => [truncateToWidth(theme.fg("muted", `[pi-goals] Worker review: ${review.verdict} · revision ${revision} · ${keyHint("app.tools.expand", "expand")}`), width)], + invalidate() {}, + }; + }); function registerChannel(ctx: ExtensionContext) { liveContext = ctx; const registration: IntercomExtensionRegistration = { @@ -408,7 +418,10 @@ export default function mainSupervisor(pi: ExtensionAPI) { if (!prior && review.verdict === "changes_requested" && (state.mode === "paused" || data.plan !== state.plan || data.requestId !== state.parent.requestId || !review.continuation.trim())) return; if (!prior) { pi.appendEntry(REVIEW, review); - if (review.verdict === "changes_requested") pi.sendUserMessage(review.content, { deliverAs: "followUp" }); + if (review.verdict === "changes_requested") { + notices.hide(review.content); + pi.sendUserMessage(review.content, { deliverAs: "followUp" }); + } } channel?.publish({ ...data, review, type: "review_saved", to: state.parent.intercomId }, { audience: "capable" }); return; @@ -432,8 +445,10 @@ export default function mainSupervisor(pi: ExtensionAPI) { sendAttachment(state.plan, event.fromSessionId, nativeMessages.attached(data.sessionFile)); } if (data.type === "stopped" && event.fromSessionId === worker.intercomId && typeof data.entryId === "string" && data.entryId && typeof data.text === "string") { + const id = `${event.fromSessionId}:${data.entryId}`; + if (records(ctx, REPORT).some(report => report.id === id)) return; if (data.identity) { worker.identity = data.identity; worker.sessionFile = data.identity.sessionFile; save(); } - const report: Report = { id: `${event.fromSessionId}:${data.entryId}`, plan: state.plan, session: event.fromSessionId, sessionFile: worker.sessionFile!, requestId: data.requestId, task: worker.task, text: data.text }; + const report: Report = { id, plan: state.plan, session: event.fromSessionId, sessionFile: worker.sessionFile!, requestId: data.requestId, task: worker.task, text: data.text }; recordReport(ctx, report); } }, @@ -645,7 +660,7 @@ export default function mainSupervisor(pi: ExtensionAPI) { `Last observed worker model: ${state.worker?.identity?.model ?? "unconfirmed"}; verify current choice before claiming configuration.`, `Pending worker revision reviews: ${pendingReports(ctx).map(reportLabel).join("; ") || "none"}`, `Recorded worker session: ${state.worker?.sessionFile ?? "not recorded"}`, - `Worker Intercom: ${state.worker?.intercomId ?? "unconfirmed"}; native pane: ${state.worker?.paneId ?? "unconfirmed"}`, + `Worker Intercom: ${state.worker?.intercomId ?? "unconfirmed"}; native pane: ${state.worker?.identity?.paneId || state.worker?.paneId || "unconfirmed"}`, notedPlanValue("worker session") ? `Worker session noted in plan: ${notedPlanValue("worker session")}` : "", `Check-in: session-scoped pi-scheduler task ${JSON.stringify(`goals-${ctx.sessionManager.getSessionId()}`)} (default 1h; /schedules all shows current recurrence; manage_scheduled_task updates it)`, "Inspect the exact Intercom session and native pane; a binding or idle status is not completion.", @@ -684,14 +699,14 @@ export default function mainSupervisor(pi: ExtensionAPI) { let text: string; try { text = readFileSync(target, "utf8"); } catch { ctx.ui.notify(`Cannot read plan at ${target}.`, "error"); return; } if (!goals(text).length || goals(text).some(g => !g.subject)) { ctx.ui.notify(`${target} has no '- [ ] goal:' lines with valid subjects; attach a judgeable plan.`, "warning"); 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; } + if (!solo && state.plan === target && state.mode !== "chat") { + notice = true; fullPlanContextDue = true; refresh(ctx); + ctx.ui.notify(nativeMessages.samePlanRestored, "info"); return; + } const noted = /^-\s*worker session:\s*(\S+)/im.exec(foldPlan(text))?.[1]; - if (!(await confirmOwnership(ctx, target, text, solo))) 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) }; - generation++; notice = true; fullPlanContextDue = true; save(); refresh(ctx); watchPlan(ctx); - if (solo) enterSolo(ctx); - else send(attachNotice(target, false, noted)); + if (!(await confirmOwnership(ctx, target, text))) return; + if (noted && !state.worker) state.worker = { sessionFile: resolve(ctx.cwd, noted) }; + enterSolo(ctx); return; } if (command === "exit") { diff --git a/src/notice-display.ts b/src/notice-display.ts index d57abfd..9d18135 100644 --- a/src/notice-display.ts +++ b/src/notice-display.ts @@ -10,7 +10,7 @@ export function noticeDisplay(pi: ExtensionAPI) { context.messageType === "user" && mirrored.has(markdown) ? "" : markdown); pi.registerEntryRenderer(NOTICE, (entry, { expanded }, theme) => { const { content } = entry.data as { content: string }; - const label = content.includes("\nPlan changed.") ? "Plan changed · review requested" + const label = content.includes("\nPlan changed") ? "Plan changed · review requested" : content.includes("## Worker revision reviews") ? "Worker revisions · review requested" : content.includes("## Worker revision report") ? "Worker revision report" : "Goal instructions"; @@ -21,14 +21,19 @@ export function noticeDisplay(pi: ExtensionAPI) { }; }); return { + hide(content: string) { + mirrored.add(content); + }, mirror(content: string) { mirrored.add(content); pi.appendEntry(NOTICE, { content }); }, - restore(ctx: ExtensionContext) { + restore(ctx: ExtensionContext, hiddenTypes: string[] = []) { mirrored.clear(); for (const entry of ctx.sessionManager.getBranch()) { - if (entry.type === "custom" && entry.customType === NOTICE) mirrored.add((entry.data as { content: string }).content); + if (entry.type !== "custom" || entry.customType !== NOTICE && !hiddenTypes.includes(entry.customType)) continue; + const content = (entry.data as { content?: unknown }).content; + if (typeof content === "string") mirrored.add(content); } }, }; diff --git a/src/prompts.ts b/src/prompts.ts index 92c55de..fdf3966 100644 --- a/src/prompts.ts +++ b/src/prompts.ts @@ -192,7 +192,7 @@ export function workerAttachment(plan: string, session: string, text: string): s } // Pi/OpenAI: supervisor-authored report reviews, separate from goal completion. export const reportReviewDescription = "Review one owned worker revision after inspecting its actual artifacts. reportId is the exact session:revision token shown in /goals status, not report prose. Quote the assigned goal/task and evidence from files; git:: reads an immutable tracked revision. An optional saved-session entryId selects decoded message text. State observations and unmet requirements; use accepted, changes_requested or blocked. Changes requested need a concrete continuation. Text quotes are checked, not their relevance or quality. Non-text evidence needs a nonempty capture and specific observation. Delivery stays pending until the worker saves the visible review. Acceptance never completes a goal or wakes/closes the worker."; -export const reportReviewContent = (report: string, sessionFile: string, sources: string[], observation: string, unmet: string, verdict: string, continuation: string) => `Worker review: ${verdict}\nReport: ${report}\nSaved session: ${sessionFile}\n\nAssigned goal/task:\n${sources[0]}\n\nEvidence:\n${sources.slice(1).join("\n\n")}\n\nInspected: ${observation}\nUnmet: ${unmet}\nContinuation: ${continuation || "none"}\nThis is a report review, not CompleteGoal.\n— Pi supervisor`; +export const reportReviewContent = (report: string, sessionFile: string, sources: string[], observation: string, unmet: string, verdict: string, continuation: string) => `## Worker review: ${verdict}\n\n- Report: \`${report}\`\n- Saved session: \`${sessionFile}\`\n\n### Assigned goal/task\n\n${sources[0]}\n\n### Evidence\n\n${sources.slice(1).join("\n\n")}\n\n### Review\n\n- Inspected: ${observation}\n- Unmet: ${unmet}\n- Continuation: ${continuation || "none"}\n\nThis is a report review, not CompleteGoal.\n\n— Pi supervisor`; export const pendingReportReviews = (reports: string[]) => `## Worker revision reviews\n\nInspect each saved stop report and actual artifacts, then use review_subagent with its reportId. Independent authorized work may continue; attachment receipts and ordinary Intercom messages are not review obligations.\n\n${reports.map(report => `- ${report}`).join("\n")}`; export function workerReview(plan: string, session: string, text: string): string { return `[pi-goals: worker review]\n## Worker revision report\n\n- Plan: \`${plan}\`\n- Intercom session: \`${session}\`\n\n### Report\n\n${text}\n\nThis is a report, not completion approval. Inspect actual artifacts and saved messages; if correction is needed, send it to the same session. Preserve its visible review conversation. Respect pauses; do not reply merely to acknowledge.`; @@ -253,8 +253,8 @@ export function completionResult(goal: string, sessionId: string, remaining: boo // Pause/resume and solo recovery. Stored stop confirmation is invalidated on every worker launch. export const pausedRole = "Goal work is paused. Do not launch, resume or authorize work. Incoming reports are observations, not permission. Help inspect or stop existing workers if requested."; -export function pauseExitNotice(worker: { intercomId?: string; sessionFile?: string; paneId?: string } | undefined, exited: boolean): string { - return `Goals ${exited ? "exited to ordinary chat" : "paused locally"}; plan and evidence retained. ${worker ? `Locate the recorded native pane ${worker.paneId ?? "unknown"}, Intercom session ${worker.intercomId ?? "unknown"}, saved session ${worker.sessionFile ?? "unknown"}. Send an explicit pause there; inspect and confirm actual stop without closing the review conversation.` : "No worker recorded: inspect Intercom and native panes; absence is not proof of stop."} Remote stop is NOT yet confirmed. Resume only after explicit authorization.`; +export function pauseExitNotice(worker: { intercomId?: string; sessionFile?: string; paneId?: string; identity?: { paneId?: string } } | undefined, exited: boolean): string { + return `Goals ${exited ? "exited to ordinary chat" : "paused locally"}; plan and evidence retained. ${worker ? `Locate the recorded native pane ${worker.identity?.paneId || worker.paneId || "unknown"}, Intercom session ${worker.intercomId ?? "unknown"}, saved session ${worker.sessionFile ?? "unknown"}. Send an explicit pause there; inspect and confirm actual stop without closing the review conversation.` : "No worker recorded: inspect Intercom and native panes; absence is not proof of stop."} Remote stop is NOT yet confirmed. Resume only after explicit authorization.`; } export function resumeNotice(workerName: string, planPath: string, worker: { sessionFile?: string; intercomId?: string } | undefined): string { return `User authorized continuation of ${planPath}. Inspect worker state before any launch. ${worker ? `Use the existing session ${worker.sessionFile ?? "unknown"} and exact Intercom UUID ${worker.intercomId ?? "unknown"}; if live, inspect/message it. Do not open a replacement. If stopped, inspect saved history and partial work, preserve drafts and use stock pane controls only after confirming safe stop. Continue only remaining work in a newly authorized context; never replay the completed assignment. Preserve later human model choices.` : `Use OpenGoalWorker for '${workerName}' only after confirming no prior writer exists.`} Continue only unfinished goals; retain saved progress and scheduler edits.`; @@ -263,11 +263,12 @@ 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.", modelDescription: "User preference for agent-led configuration and verification, not a launch override.", diff --git a/test/goals.test.ts b/test/goals.test.ts index e2134da..2e7911b 100644 --- a/test/goals.test.ts +++ b/test/goals.test.ts @@ -4,8 +4,8 @@ import { access, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, join, relative } from "node:path"; import { fileURLToPath } from "node:url"; -import { createEditTool, type ExtensionAPI, SessionManager, withFileMutationQueue } from "@earendil-works/pi-coding-agent"; -import { visibleWidth } from "@earendil-works/pi-tui"; +import { createEditTool, type ExtensionAPI, initTheme, SessionManager, withFileMutationQueue } from "@earendil-works/pi-coding-agent"; +import { Markdown, 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"; @@ -69,8 +69,8 @@ function fixture(child = false) { await delay(25); }; const start = (_id: string) => hooks.get("tool_call")({ toolName: "OpenGoalWorker" }, ctx); - const launch = async (details: { id: string; sessionFile: string }) => { - await tools.get("OpenGoalWorker").execute("open", { task: "Implement first output" }, undefined, undefined, ctx); + const launch = async (details: { id: string; sessionFile: string; task?: string }) => { + await tools.get("OpenGoalWorker").execute("open", { task: details.task ?? "Implement first output" }, undefined, undefined, ctx); const state = entries.at(-1).data; registration.onEvent({ type: "message", fromSessionId: details.id, payload: { type: "attached", to: state.worker.parentId, requestId: state.worker.requestId, plan: state.plan, sessionFile: details.sessionFile } }); }; @@ -333,15 +333,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"); @@ -576,21 +575,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`); @@ -622,9 +622,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 () => { @@ -648,7 +649,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"); @@ -656,21 +657,29 @@ 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 binding = f.entries.at(-1).data.worker; + f.ctx.ui.select.mockResolvedValueOnce("Worker confirmed stopped"); await f.command(`attach ${f.path} solo`); + expect(f.entries.at(-1).data.worker).toEqual(binding); 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); }); @@ -923,7 +932,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"); @@ -931,7 +940,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 () => { @@ -1088,7 +1100,7 @@ it("opens no-focus, records explicit attachment only, and wakes review only for f.channel.listSessions.mockRejectedValueOnce(new Error("Intercom is not connected")); const waiting = await f.tools.get("OpenGoalWorker").execute("open", { task: "first" }, undefined, undefined, f.ctx); expect(waiting.content[0].text).toContain("still connecting"); expect(openProjectPane).not.toHaveBeenCalled(); - await f.launch({ id: "worker-id", sessionFile: "/tmp/native-worker.jsonl" }); + await f.launch({ id: "worker-id", sessionFile: "/tmp/native-worker.jsonl", task: "Inspect [cached interruption audit](slop/audits/20260916_job1551_a2_cached_interruption_audit.md) before rerun" }); expect(openProjectPane).toHaveBeenCalledWith(expect.objectContaining({ cwd: f.ctx.cwd, focus: false })); expect(vi.mocked(openProjectPane).mock.calls[0][0].message).toContain("WAIT for an explicit assignment"); const worker = f.entries.at(-1).data.worker; @@ -1107,6 +1119,8 @@ it("opens no-focus, records explicit attachment only, and wakes review only for expect(f.messages.at(-2)?.message.content).toContain("Blocked: input missing"); expect(f.ctx.sessionManager.getBranch().some((entry: any) => entry.customType === "pi-goals-notice" && entry.data.content.includes("## Worker revision report"))).toBe(true); expect(f.messages.at(-1)?.message.content).toContain("## Worker revision reviews"); + expect(f.messages.at(-1)?.message.content).toContain("cached interruption audit"); + expect(f.messages.at(-1)?.message.content).not.toContain("]("); const afterFirstRevision = f.messages.length; for (const text of ["Done: output.txt", "Error: execution failed"]) f.event({ type: "message", fromSessionId: "worker-id", payload: { ...notice, text } }); expect(f.messages).toHaveLength(afterFirstRevision); @@ -1243,12 +1257,26 @@ it("reviews a saved worker revision through inspection, silent delivery, retry a expect(parent.ctx.ui.notify.mock.lastCall?.[0]).toContain("Pending worker revision reviews: none"); expect(worker.messages).toHaveLength(workerTurns); expect(readFileSync(sm.getSessionFile()!, "utf8")).toContain("Worker review: accepted"); + initTheme("dark"); + const reviewEntry = sm.getBranch().find(entry => entry.type === "custom" && entry.customType === "pi-goals-report-review")!; + const reviewRenderer = vi.mocked(worker.pi.registerEntryRenderer).mock.calls.find(([type]) => type === "pi-goals-report-review")![1]; + const collapsedReview = reviewRenderer(reviewEntry, { expanded: false }, worker.ctx.ui.theme); + expect(collapsedReview.render(100).join("\n")).toContain("Worker review: accepted"); + expect(collapsedReview.render(100).join("\n")).not.toContain("Inspected actual output"); + expect(reviewRenderer(reviewEntry, { expanded: true }, worker.ctx.ui.theme)).toBeInstanceOf(Markdown); await review(); worker.hooks.get("session_shutdown")(); parent.event({ type: "session_left", sessionId: workerId }); await parent.command("status"); expect(parent.ctx.ui.notify.mock.lastCall?.[0]).toContain("Pending worker revision reviews: none"); worker.hooks.get("session_start")({}, worker.ctx); + const staleStop = worker.channel.publish.mock.lastCall?.[0]; + vi.stubEnv("HERDR_PANE_ID", "restored-pane"); + await worker.tools.get("AttachGoalPlan").execute("restore", { path: parent.path }, undefined, undefined, worker.ctx); + parent.event({ type: "message", fromSessionId: workerId, payload: staleStop }); + await parent.command("status"); + expect(parent.ctx.ui.notify.mock.lastCall?.[0]).toContain("native pane: restored-pane"); await parent.command("stop"); + expect(parent.messages.at(-1)?.message.content).toContain("native pane restored-pane"); form.reportId = report("Revision failed", "error"); const paused = parent.messages.length; await parent.hooks.get("agent_settled")({}, parent.ctx); @@ -1294,21 +1322,32 @@ it("reviews a saved worker revision through inspection, silent delivery, retry a parent.hooks.get("session_start")({}, parent.ctx); // Reconcile the missed stop from real saved worker history. await parent.command("status"); expect(parent.ctx.ui.notify.mock.lastCall?.[0]).toContain(missed); + await parent.hooks.get("agent_settled")({}, parent.ctx); form.reportId = missed; await review(); // Old-plan blocked review remains deliverable after retargeting. await parent.command("status"); expect(parent.ctx.ui.notify.mock.lastCall?.[0]).not.toContain(missed); expect(parent.ctx.ui.notify.mock.lastCall?.[0]).toContain(nextReport); + const afterReview = parent.messages.length; + await parent.hooks.get("agent_settled")({}, parent.ctx); + parent.hooks.get("session_start")({}, parent.ctx); + await parent.hooks.get("agent_settled")({}, parent.ctx); + expect(parent.messages).toHaveLength(afterReview); // Shrinking/restoring the same backlog does not wake again. const ordinary = (id: string, text: string, links = {}, sender = workerId) => { const details = { from: { id: sender }, message: { id, timestamp: Date.now(), content: { text }, ...links } }; parent.ctx.sessionManager.getBranch().push({ type: "custom_message", id, customType: "intercom_message", content: text, details }); parent.hooks.get("message_end")({ message: { role: "custom", customType: "intercom_message", content: text, details } }, parent.ctx); }; + const beforeOrdinary = parent.messages.length; ordinary("ordinary-a", "Partial output needs review"); ordinary("ordinary-b", "Corrected output needs review", { supersedes: "ordinary-a" }); ordinary("retry-b", "Corrected output needs review", { retryOf: "ordinary-b" }); ordinary("retry-again", "Corrected output needs review", { retryOf: "retry-b" }); ordinary("ack-only", "OK"); ordinary("foreign", "Unowned report", {}, "foreign-peer"); - parent.hooks.get("session_start")({}, parent.ctx); await parent.command("status"); + expect(parent.messages).toHaveLength(beforeOrdinary); // No second body beside Intercom's saved/displayed original. + parent.hooks.get("session_start")({}, parent.ctx); + await parent.hooks.get("agent_settled")({}, parent.ctx); + expect(parent.messages).toHaveLength(beforeOrdinary); // Ordinary Intercom messages remain visible but are not review obligations. + await parent.command("status"); const pending = parent.ctx.ui.notify.mock.lastCall?.[0]; for (const nonReviewable of ["ordinary-a", "ordinary-b", "retry-b", "retry-again", "ack-only", "foreign-peer"]) expect(pending).not.toContain(nonReviewable); expect(pending).toContain(nextReport); diff --git a/test/notice-display.test.ts b/test/notice-display.test.ts index 74500d1..fa8f2e9 100644 --- a/test/notice-display.test.ts +++ b/test/notice-display.test.ts @@ -35,9 +35,14 @@ it("collapses mirrored prompts only in the UI, expands the exact text, and resto const reviewCollapsed = render({ type: "custom", customType: "pi-goals-notice", data: { content: review } }, { expanded: false }, theme); expect(reviewCollapsed.render(80).join("\n")).toContain("Worker revisions · review requested"); - display.restore({ sessionManager: { getBranch: () => [] } } as unknown as ExtensionContext); + const deliveredReview = "## Worker review: changes_requested\n\nCorrect output.txt."; + display.hide(deliveredReview); + expect(transform(deliveredReview, { messageType: "user" })).toBe(""); + display.restore({ sessionManager: { getBranch: () => [] } } as unknown as ExtensionContext, ["pi-goals-report-review"]); expect(transform(content, { messageType: "user" })).toBe(content); - display.restore({ sessionManager: { getBranch: () => [entry] } } as unknown as ExtensionContext); + const reviewEntry = { type: "custom", customType: "pi-goals-report-review", data: { content: deliveredReview } }; + display.restore({ sessionManager: { getBranch: () => [entry, reviewEntry] } } as unknown as ExtensionContext, ["pi-goals-report-review"]); expect(transform(content, { messageType: "user" })).toBe(""); + expect(transform(deliveredReview, { messageType: "user" })).toBe(""); expect(entry.data.content).toBe(content); }); diff --git a/test/package.test.ts b/test/package.test.ts index fcf242f..5d51b2a 100644 --- a/test/package.test.ts +++ b/test/package.test.ts @@ -6,14 +6,11 @@ import { expect, it } from "vitest"; it("declares current entry and bundled extension resources that exist after install", () => { const manifest = JSON.parse(readFileSync("package.json", "utf8")); - expect(manifest.pi.extensions[0]).toBe("./src/index.ts"); for (const path of manifest.pi.extensions) expect(existsSync(resolve(path)), path).toBe(true); for (const name of ["pi-subagents", "pi-intercom", "@jl1990/pi-scheduler"]) { expect(manifest.dependencies[name]).toBeTruthy(); expect(manifest.bundleDependencies).toContain(name); } - expect(manifest.dependencies["pi-subagents"]).toBe("0.66.0"); - expect(manifest.bundledDependencies).toBeUndefined(); expect(JSON.parse(readFileSync("package-lock.json", "utf8")).packages[""].bundleDependencies).toEqual(manifest.bundleDependencies); }); diff --git a/test/rpc-review.test.ts b/test/rpc-review.test.ts index ee370b6..2ac1ba3 100644 --- a/test/rpc-review.test.ts +++ b/test/rpc-review.test.ts @@ -185,6 +185,12 @@ describe("RPC review flow", () => { expect(client.messages.some(event => event.type === "message_end" && (event.message as any)?.role === "user" && (event.message as any)?.content[0]?.text === content)).toBe(true); expect(supervisor.messages.filter(message => message.role === "user" && messageText(message.content) === content)).toHaveLength(1); } + const restoreStart = client.messages.length; + client.send({ type: "prompt", id: "restore-same-plan", message: `/goals attach ${planPath}` }); + await client.waitFor(message => message.type === "response" && message.id === "restore-same-plan", restoreStart); + expect(client.messages.slice(restoreStart).filter(isSelect)).toEqual([]); + expect(client.messages.slice(restoreStart).some(message => message.type === "extension_ui_request" && message.method === "notify" && JSON.stringify(message).includes("mode and worker binding unchanged"))).toBe(true); + expect(requests).toHaveLength(beforeReady + 1); const beforeNotice = requests.length, noticeStart = client.messages.length; client.send({ type: "prompt", id: "attachment-notice", message: "/fixture-attachment-notice" }); const attachment = await client.waitFor(message => message.type === "message_end" && (message.message as any)?.customType === "pi-goals-supervision", noticeStart);