feat(pi): submit plans from any mode

apps/pi-extension/index.ts

@@ -65,7 +65,6 @@ import {
 	isPlanWritePathAllowed,
 	PLAN_SUBMIT_TOOL,
 	type Phase,
-	stripPlanningOnlyTools,
 } from "./tool-scope.ts";
 import { isRemoteSession } from "./server/network.ts";
 
@@ -366,7 +365,7 @@ export default function plannotator(pi: ExtensionAPI): void {
 		}
 
 		if (phase === "planning" || phase === "executing") {
-			const baseTools = stripPlanningOnlyTools(savedState?.activeTools ?? pi.getActiveTools());
+			const baseTools = savedState?.activeTools ?? pi.getActiveTools();
 			const toolSet = new Set(baseTools);
 			for (const tool of profile?.activeTools ?? []) toolSet.add(tool);
 			if (phase === "planning") {
@@ -785,8 +784,8 @@ export default function plannotator(pi: ExtensionAPI): void {
 		name: PLAN_SUBMIT_TOOL,
 		label: "Submit Plan",
 		description:
-			"Submit your Plannotator plan for user review. " +
-			"Call this only while Plannotator planning mode is active, after writing your plan as a markdown file anywhere inside the working directory. " +
+			"Submit your Plannotator plan for user review from any mode. " +
+			"Call this after writing your plan as a markdown file anywhere inside the working directory. " +
 			"Pass the path to the plan file (e.g. PLAN.md or plans/auth.md). " +
 			"The user will review the plan in a visual browser UI and can approve, deny with feedback, or annotate it. " +
 			"If denied, edit the same file in place, then call this again with the same path.",
@@ -798,19 +797,7 @@ export default function plannotator(pi: ExtensionAPI): void {
 		}) as any,
 
 		async execute(_toolCallId, params, _signal, _onUpdate, ctx) {
-			// Guard: must be in planning phase
-			if (phase !== "planning") {
-				return {
-					content: [
-						{
-							type: "text",
-							text: "Error: Not in plan mode. Use /plannotator to enter planning mode first.",
-						},
-					],
-					details: { approved: false },
-				};
-			}
-
+			const submissionPhase = phase;
 			const inputPath = (params as { filePath?: string })?.filePath?.trim();
 			if (!inputPath) {
 				return {
@@ -889,16 +876,34 @@ export default function plannotator(pi: ExtensionAPI): void {
 				};
 			}
 
-			lastSubmittedPath = inputPath;
-			checklistItems = parseChecklist(planContent);
+			const submittedChecklist = parseChecklist(planContent);
+			if (submissionPhase === "planning") {
+				lastSubmittedPath = inputPath;
+				checklistItems = submittedChecklist;
+			}
 
-			// Non-interactive or no HTML: auto-approve
-			if (!ctx.hasUI || !hasPlanBrowserHtml()) {
+			const beginExecution = async (): Promise<void> => {
+				lastSubmittedPath = inputPath;
+				checklistItems = submittedChecklist;
 				phase = "executing";
 				await applyPhaseConfig(ctx, { restoreSavedState: true });
 				pi.appendEntry("plannotator-execute", { lastSubmittedPath });
 				persistState();
 				justApprovedPlan = true;
+			};
+			const shouldBeginExecution = (): boolean =>
+				submissionPhase === "planning" && phase === "planning";
+
+			// Non-interactive or no HTML: auto-approve
+			if (!ctx.hasUI || !hasPlanBrowserHtml()) {
+				if (!shouldBeginExecution()) {
+					return {
+						content: [{ type: "text", text: "Plan approved. Continue in the current mode." }],
+						details: { approved: true },
+					};
+				}
+
+				await beginExecution();
 				const { getPlanAutoApprovedPrompt } = await loadPlannotatorPrompts();
 				return {
 					content: [
@@ -925,12 +930,22 @@ export default function plannotator(pi: ExtensionAPI): void {
 			}
 
 			if (result.approved) {
-				phase = "executing";
-				await applyPhaseConfig(ctx, { restoreSavedState: true });
-				pi.appendEntry("plannotator-execute", { lastSubmittedPath });
-				persistState();
-				justApprovedPlan = true;
+				if (!shouldBeginExecution()) {
+					const feedback = result.feedback?.trim();
+					return {
+						content: [
+							{
+								type: "text",
+								text: feedback
+									? `Plan approved with notes:\n\n${feedback}`
+									: "Plan approved. Continue in the current mode.",
+							},
+						],
+						details: { approved: true, ...(feedback ? { feedback } : {}) },
+					};
+				}
 
+				await beginExecution();
 				const doneMsg =
 					checklistItems.length > 0
 						? `After completing each step, include [DONE:n] in your response where n is the step number.`
@@ -971,7 +986,7 @@ export default function plannotator(pi: ExtensionAPI): void {
 			}
 
 			// Denied
-			persistState();
+			if (submissionPhase === "planning" && phase === "planning") persistState();
 			const feedbackText = result.feedback || "Plan rejected. Please revise.";
 			const { buildPlanFileRule, getPlanDeniedPrompt, getPlanToolName } = await loadPlannotatorPrompts();
 			return {
@@ -1315,11 +1330,6 @@ Execute each step in order. After completing a step, include [DONE:n] in your re
 			if (savedState) {
 				await restoreSavedState(ctx);
 				savedState = null;
-			} else {
-				// Strip planning-only tools on fresh sessions where savedState is null.
-				// Without this, plannotator_submit_plan stays in the active tool set
-				// even though plan mode hasn't been activated. See #387.
-				pi.setActiveTools(stripPlanningOnlyTools(pi.getActiveTools()));
 			}
 		} else if (phase === "planning" || phase === "executing") {
 			await applyPhaseConfig(ctx, { restoreSavedState: true });

apps/pi-extension/submit-plan-runtime.test.ts

@@ -0,0 +1,134 @@
+import { afterEach, describe, expect, test } from "bun:test";
+import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import type { ToolDefinition } from "@earendil-works/pi-coding-agent";
+import plannotator from "./index.ts";
+
+const tempDirectories: string[] = [];
+
+afterEach(() => {
+	for (const directory of tempDirectories.splice(0)) {
+		rmSync(directory, { recursive: true, force: true });
+	}
+});
+
+function createContext(cwd: string) {
+	return {
+		cwd,
+		hasUI: false,
+		isIdle: () => true,
+		model: undefined,
+		modelRegistry: { find: () => undefined },
+		sessionManager: {
+			getEntries: () => [],
+			getSessionFile: () => undefined,
+			getSessionId: () => "test-session",
+			getSessionName: () => undefined,
+		},
+		ui: {
+			notify: () => undefined,
+			setStatus: () => undefined,
+			setWidget: () => undefined,
+			theme: {
+				bold: (text: string) => text,
+				fg: (_color: string, text: string) => text,
+				strikethrough: (text: string) => text,
+			},
+		},
+	};
+}
+
+function createRuntime() {
+	type Context = ReturnType<typeof createContext>;
+	type Handler = (event: unknown, context: Context) => unknown;
+	const commands = new Map<string, { handler: (args: string, context: Context) => unknown }>();
+	const handlers = new Map<string, Handler[]>();
+	const entries: Array<{ type: string; data: unknown }> = [];
+	const tools = new Map<string, ToolDefinition>();
+	let activeTools = ["inspect", "plannotator_submit_plan"];
+
+	const pi = {
+		appendEntry: (type: string, data: unknown) => entries.push({ type, data }),
+		events: { on: () => () => undefined },
+		getActiveTools: () => [...activeTools],
+		getFlag: () => false,
+		getThinkingLevel: () => "medium",
+		on: (event: string, handler: Handler) => {
+			handlers.set(event, [...(handlers.get(event) ?? []), handler]);
+		},
+		registerCommand: (name: string, command: { handler: (args: string, context: Context) => unknown }) => {
+			commands.set(name, command);
+		},
+		registerFlag: () => undefined,
+		registerShortcut: () => undefined,
+		registerTool: (tool: ToolDefinition) => tools.set(tool.name, tool),
+		sendMessage: () => undefined,
+		sendUserMessage: () => undefined,
+		setActiveTools: (nextTools: string[]) => {
+			activeTools = [...nextTools];
+		},
+		setModel: async () => true,
+		setThinkingLevel: () => undefined,
+	};
+
+	plannotator(pi as never);
+
+	return {
+		commands,
+		entries,
+		getActiveTools: () => activeTools,
+		run: async (event: string, context: Context) => {
+			for (const handler of handlers.get(event) ?? []) await handler({}, context);
+		},
+		tools,
+	};
+}
+
+describe("plannotator_submit_plan availability", () => {
+	test("reviews from idle without starting execution, then keeps planning approval behavior", async () => {
+		const cwd = mkdtempSync(join(tmpdir(), "plannotator-submit-any-mode-"));
+		tempDirectories.push(cwd);
+		writeFileSync(join(cwd, "PLAN.md"), "# Plan\n\n- [ ] First step\n", "utf8");
+		writeFileSync(join(cwd, "SECOND.md"), "# Second plan\n", "utf8");
+		const runtime = createRuntime();
+		const context = createContext(cwd);
+		await runtime.run("session_start", context);
+		expect(runtime.getActiveTools()).toContain("plannotator_submit_plan");
+
+		const submitPlan = runtime.tools.get("plannotator_submit_plan");
+		expect(submitPlan).toBeDefined();
+		const idleResult = await submitPlan!.execute(
+			"idle-review",
+			{ filePath: "PLAN.md" },
+			undefined,
+			undefined,
+			context as never,
+		);
+		expect(idleResult.details).toMatchObject({ approved: true });
+		expect(runtime.entries.some((entry) => entry.type === "plannotator-execute")).toBe(false);
+
+		await runtime.commands.get("plannotator")?.handler("", context);
+		const planningResult = await submitPlan!.execute(
+			"planning-review",
+			{ filePath: "PLAN.md" },
+			undefined,
+			undefined,
+			context as never,
+		);
+		expect(planningResult.details).toMatchObject({ approved: true });
+		expect(runtime.entries.filter((entry) => entry.type === "plannotator-execute")).toHaveLength(1);
+
+		const executingResult = await submitPlan!.execute(
+			"executing-review",
+			{ filePath: "SECOND.md" },
+			undefined,
+			undefined,
+			context as never,
+		);
+		expect(executingResult.details).toMatchObject({ approved: true });
+		expect(runtime.entries.filter((entry) => entry.type === "plannotator-execute")).toHaveLength(1);
+		const executionEntry = runtime.entries.find((entry) => entry.type === "plannotator-execute");
+		expect(executionEntry?.data).toEqual({ lastSubmittedPath: "PLAN.md" });
+	});
+});

apps/pi-extension/tool-scope.test.ts

@@ -3,7 +3,6 @@ import {
 	getToolsForPhase,
 	isPlanWritePathAllowed,
 	PLAN_SUBMIT_TOOL,
-	stripPlanningOnlyTools,
 } from "./tool-scope.ts";
 
 describe("pi plan tool scoping", () => {
@@ -20,29 +19,11 @@ describe("pi plan tool scoping", () => {
 		]);
 	});
 
-	test("idle and executing phases strip the planning-only submit tool", () => {
-		const leakedTools = ["read", "bash", "grep", PLAN_SUBMIT_TOOL, "write"];
+	test("idle and executing phases preserve the submit tool", () => {
+		const activeTools = ["read", "bash", "grep", PLAN_SUBMIT_TOOL, "write"];
 
-		expect(getToolsForPhase(leakedTools, "idle")).toEqual([
-			"read",
-			"bash",
-			"grep",
-			"write",
-		]);
-		expect(getToolsForPhase(leakedTools, "executing")).toEqual([
-			"read",
-			"bash",
-			"grep",
-			"write",
-		]);
-	});
-
-	test("stripping planning-only tools preserves unrelated tools", () => {
-		expect(stripPlanningOnlyTools([PLAN_SUBMIT_TOOL, "todo", "question", "read"])).toEqual([
-			"todo",
-			"question",
-			"read",
-		]);
+		expect(getToolsForPhase(activeTools, "idle")).toEqual(activeTools);
+		expect(getToolsForPhase(activeTools, "executing")).toEqual(activeTools);
 	});
 });
 

apps/pi-extension/tool-scope.ts

@@ -5,24 +5,15 @@ export type Phase = "idle" | "planning" | "executing";
 export const PLAN_SUBMIT_TOOL = "plannotator_submit_plan";
 export const PLANNING_DISCOVERY_TOOLS = ["grep", "find", "ls"] as const;
 
-const PLANNING_ONLY_TOOLS = new Set<string>([PLAN_SUBMIT_TOOL]);
 const ALLOWED_PLAN_EXTENSIONS = new Set<string>([".md", ".mdx"]);
 
-export function stripPlanningOnlyTools(tools: readonly string[]): string[] {
-	return tools.filter((tool) => !PLANNING_ONLY_TOOLS.has(tool));
-}
-
 export function getToolsForPhase(
 	baseTools: readonly string[],
 	phase: Phase,
 ): string[] {
-	const tools = stripPlanningOnlyTools(baseTools);
-	if (phase !== "planning") {
-		return [...new Set(tools)];
-	}
-
+	if (phase !== "planning") return [...new Set(baseTools)];
 	return [
-		...new Set([...tools, ...PLANNING_DISCOVERY_TOOLS, PLAN_SUBMIT_TOOL]),
+		...new Set([...baseTools, ...PLANNING_DISCOVERY_TOOLS, PLAN_SUBMIT_TOOL]),
 	];
 }