diff --git a/CHANGELOG.md b/CHANGELOG.md index 371d40a..59ca6da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to `@pi-vault/pi-plan` are documented in this file. +## [Unreleased] + +### Changed + +- Preserve complete conversation context when switching between Plan mode and normal mode; old display-only proposed-plan messages remain filtered to avoid duplicate context. +- Defer mode changes requested during an active turn until Pi is idle, with the latest queued request taking precedence. +- Submit `Implement the plan.` in the retained conversation instead of copying the full cached plan into a new handoff prompt. + ## [0.4.0] - 2026-07-26 Pi 0.82.0 or newer is now required. diff --git a/README.md b/README.md index 46a5c17..c7bb58f 100644 --- a/README.md +++ b/README.md @@ -56,10 +56,10 @@ pi --plan 1. Let the agent inspect the repo and ask clarifying questions. 2. When the plan is ready, you receive exactly one `` block. 3. While the agent is idle, choose what to do next: - - **Implement this plan** - turn Plan mode off, restore full tool access, and send the full proposed plan back into the conversation as the implementation instruction. + - **Implement this plan** - turn Plan mode off, restore full tool access, and submit `Implement the plan.` in the same conversation. - **Save plan** - keep Plan mode active and write the exact current plan to one new lowercase `.md` file in the workspace root. The agent chooses the filename; you do not provide a path. - **Stay in Plan mode** - keep planning. - - **Exit Plan mode** - leave planning and preserve the latest plan for only the first normal-mode turn. + - **Exit Plan mode** - leave planning without removing prior user or assistant messages. The latest-plan menu cache remains available until the next turn starts. - **Show latest proposed plan** - review the current plan again. - **Configure tools** - change which optional tools are available during planning. @@ -83,9 +83,11 @@ When you choose it: | `/plan` | Turn on Plan mode, or open the Plan mode menu if it is already on. | | `/plan ` | Turn on Plan mode and send `` as the planning request. | | `/plan:tools` | Open the optional tool selector. If Plan mode is not active yet, Pi enables it first. | -| `/plan:exit` | Turn off Plan mode, restore the previous tool set, and preserve the latest plan for one normal-mode turn. | +| `/plan:exit` | Turn off Plan mode and restore the previous tool set without removing conversation history. | | `pi --plan` | Start Pi directly in Plan mode. | +If Pi is busy, mode-changing commands and actions from the `/plan` menu wait until the current turn settles. Only the latest queued switch is applied. An automatically opened plan-ready menu cannot queue a mode change until Pi is fully settled; retry with `/plan` if warned. Showing a plan and staying in Plan mode remain immediate; tool configuration requires Pi to be idle. + ## Configure Optional Tools Safe built-in planning tools are available by default. Use `/plan:tools` to enable additional optional extension tools during planning. diff --git a/docs/superpowers/plans/2026-08-02-preserve-plan-context.md b/docs/superpowers/plans/2026-08-02-preserve-plan-context.md new file mode 100644 index 0000000..565b515 --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-preserve-plan-context.md @@ -0,0 +1,908 @@ +# Preserve Plan Context Across Mode Switches Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let Plan mode and normal mode share one conversation while applying busy mode changes only after Pi becomes idle. + +**Architecture:** Mode is session state, not a context boundary. The context hook preserves every real user and assistant message while filtering only legacy v0.3 `customType: "proposed-plan"` duplicates. `src/index.ts` owns one non-persisted pending transition; idle requests apply immediately, busy requests replace that slot, and `agent_settled` applies the latest request after Save-plan cleanup. + +**Tech Stack:** TypeScript, Pi extension API, Vitest, Biome, pnpm. + +**Design:** `docs/superpowers/specs/2026-08-02-preserve-plan-context-design.md` + +--- + +## File Map + +- Modify `src/core/context.ts`: retain plan capture and replace assistant sanitization with a legacy-custom-message compatibility filter. +- Modify `src/core/state.ts`: preserve `latestPlan` when entering Plan mode. +- Modify `src/index.ts`: keep the context compatibility hook, clear menu cache at real turn start, queue busy transitions, and submit `Implement the plan.` after switching to normal mode. +- Modify `tests/helpers.ts`: let integration tests move a mock context from busy to idle. +- Modify `tests/core/context.test.ts` and `tests/core/state.test.ts`: cover the compatibility filter and cache-preserving entry. +- Modify `tests/index.test.ts`: cover context preservation, immediate and deferred transitions, latest-request-wins behavior, cache lifecycle, Save-plan ordering, and shutdown cleanup. +- Modify `README.md` and `CHANGELOG.md`: document continuous context and idle-deferred switching. +- Keep `src/shared/constants.ts` unchanged: `PROPOSED_PLAN_MESSAGE_TYPE` is still required for old-session compatibility. + +### Task 1: Preserve conversation messages and legacy-session compatibility + +**Files:** +- Modify: `tests/core/context.test.ts:1-150` +- Modify: `tests/core/state.test.ts:45-66` +- Modify: `tests/index.test.ts:927-1060` +- Modify: `src/core/context.ts:1-87` +- Modify: `src/core/state.ts:14-16` +- Modify: `src/index.ts:1-5,328-330` + +- [ ] **Step 1: Replace the sanitizer unit tests with compatibility-filter tests and update the entry-state expectation** + + In `tests/core/context.test.ts`, import `filterLegacyProposedPlanMessages` instead of `sanitizePlanModeContext` and replace the sanitizer describe block with: + + ```ts + describe("filterLegacyProposedPlanMessages", () => { + it("removes only legacy custom plan messages", () => { + const plan = assistantText("\n# Current\n"); + const messages = [ + user("hello"), + proposedPlanMessage("old duplicate"), + plan, + ]; + + expect(filterLegacyProposedPlanMessages(messages)).toEqual({ + messages: [user("hello"), plan], + }); + }); + + it("leaves assistant plan blocks and clean context unchanged", () => { + const messages = [ + user("\n# User text\n"), + assistantText("Before\n\n# Assistant plan\n\nAfter"), + ]; + + expect(filterLegacyProposedPlanMessages(messages)).toBeUndefined(); + }); + }); + ``` + + In `tests/core/state.test.ts`, replace the `enterPlanMode` cache test with: + + ```ts + it("preserves the latest plan and clears awaiting action", () => { + const state = { + ...createInitialState(), + latestPlan: "cached plan", + awaitingAction: true, + }; + + const next = enterPlanMode(state); + + expect(next.enabled).toBe(true); + expect(next.latestPlan).toBe("cached plan"); + expect(next.awaitingAction).toBe(false); + }); + ``` + +- [ ] **Step 2: Replace the index-level context tests with a both-modes regression** + + In `tests/index.test.ts`, replace the current `describe("context handler", ...)` block with: + + ```ts + describe("context handler", () => { + it.each([false, true])( + "preserves assistant plans and filters legacy duplicates when enabled=%s", + async (enabled) => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + if (enabled) await mock.commands.get("plan")!.handler("", ctx.ctx); + const assistantPlan = { + role: "assistant" as const, + content: [{ type: "text" as const, text: "\n# Plan\n" }], + }; + + const result = await mock.fireEvent( + "context", + { + type: "context", + messages: [ + { role: "user", content: "plan this" }, + { + role: "custom", + customType: "proposed-plan", + content: "legacy duplicate", + display: true, + timestamp: 0, + }, + assistantPlan, + ], + }, + ctx, + ); + + expect(result).toEqual({ + messages: [{ role: "user", content: "plan this" }, assistantPlan], + }); + }, + ); + + it("returns undefined for clean context", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + + expect( + await mock.fireEvent( + "context", + { type: "context", messages: [{ role: "user", content: "hello" }] }, + ctx, + ), + ).toBeUndefined(); + }); + }); + ``` + +- [ ] **Step 3: Run the focused tests and verify they fail for the old behavior** + + Run: + + ```bash + pnpm vitest run tests/core/context.test.ts tests/core/state.test.ts tests/index.test.ts + ``` + + Expected: FAIL because `filterLegacyProposedPlanMessages` is not exported, `enterPlanMode` clears `latestPlan`, and normal-mode assistant plans are stripped. + +- [ ] **Step 4: Replace assistant sanitization with the minimal legacy filter** + + In `src/core/context.ts`, remove `ALL_PLAN_BLOCK_PATTERN` and `sanitizeAssistantMessage`. Keep `PLAN_BLOCK_PATTERN`, `assistantText`, and `captureProposedPlan`, then add: + + ```ts + export function filterLegacyProposedPlanMessages( + messages: ContextEvent["messages"], + ): { messages: ContextEvent["messages"] } | undefined { + const filtered = messages.filter( + (message) => + message.role !== "custom" || message.customType !== PROPOSED_PLAN_MESSAGE_TYPE, + ); + + return filtered.length === messages.length ? undefined : { messages: filtered }; + } + ``` + + In `src/core/state.ts`, replace `enterPlanMode` with: + + ```ts + export function enterPlanMode(state: PlanModeState): PlanModeState { + return { ...state, enabled: true, awaitingAction: false }; + } + ``` + + In `src/index.ts`, import `filterLegacyProposedPlanMessages` and keep a mode-independent context hook: + + ```ts + import { + captureProposedPlan, + filterLegacyProposedPlanMessages, + } from "./core/context.ts"; + ``` + + ```ts + pi.on("context", async (event) => { + return filterLegacyProposedPlanMessages(event.messages); + }); + ``` + +- [ ] **Step 5: Run the focused tests and verify they pass** + + Run: + + ```bash + pnpm vitest run tests/core/context.test.ts tests/core/state.test.ts tests/index.test.ts + ``` + + Expected: PASS. + +- [ ] **Step 6: Commit the context and state behavior** + + ```bash + git add src/core/context.ts src/core/state.ts src/index.ts tests/core/context.test.ts tests/core/state.test.ts tests/index.test.ts + git commit -m "fix: preserve plan messages across mode switches" + ``` + +### Task 2: Queue busy mode transitions until Pi is idle + +**Files:** +- Modify: `tests/helpers.ts:29-51,131-207` +- Modify: `tests/index.test.ts:52-256,1137-1222,1652-1761` +- Modify: `src/index.ts:27-231,316-326,351-359` + +- [ ] **Step 1: Make test idleness mutable** + + In `tests/helpers.ts`, add `setIdle` to `MockContext`, store idleness in a local variable, and use it from `ctx.isIdle`: + + ```ts + export interface MockContext { + ctx: ExtensionCommandContext; + statuses: Map; + notifications: Array<{ message: string; type?: string }>; + widgets: Map; + selectCalls: Array<{ title: string; options: string[] }>; + inputCalls: Array<{ title: string; placeholder?: string }>; + customCalls: Array<{ result: unknown }>; + setIdle(value: boolean): void; + } + ``` + + Add this beside `sessionEntries`: + + ```ts + let idle = options?.isIdle ?? true; + ``` + + Replace the mock `isIdle` property and add the controller to the returned object: + + ```ts + isIdle: () => idle, + ``` + + ```ts + setIdle(value: boolean) { + idle = value; + }, + ``` + +- [ ] **Step 2: Add deferred-transition regressions** + + Add this block to `tests/index.test.ts` after the command tests: + + ```ts + describe("deferred mode transitions", () => { + it("queues plan entry and its prompt until agent_settled", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + const initialTools = [...mock.activeTools]; + ctx.setIdle(false); + + await mock.commands.get("plan")!.handler("draft a migration plan", ctx.ctx); + + expect(mock.activeTools).toEqual(initialTools); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.userMessages).toHaveLength(0); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).not.toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + expect(mock.userMessages).toEqual([ + { content: "draft a migration plan", options: undefined }, + ]); + }); + + it("queues plan exit without changing current tools or state", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + const planTools = [...mock.activeTools]; + ctx.setIdle(false); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + + expect(mock.activeTools).toEqual(planTools); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + }); + + it("sends an already-active Plan prompt as an immediate follow-up", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + + await mock.commands.get("plan")!.handler("refine the plan", ctx.ctx); + + expect(mock.userMessages).toEqual([ + { content: "refine the plan", options: { deliverAs: "followUp" } }, + ]); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + }); + + it("queues implementation and consumes the plan only when settled", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + entries: [ + { + type: "custom", + customType: "plan-mode-state", + data: { enabled: true, latestPlan: "# Queued Plan", awaitingAction: true }, + id: "queued-plan", + parentId: null, + timestamp: new Date().toISOString(), + }, + ], + selectResponses: [PLAN_MENU_LABELS.implement], + }); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + ctx.setIdle(false); + const entriesBefore = mock.entries.length; + + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(mock.activeTools).not.toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBe("plan ready"); + expect(mock.userMessages).toHaveLength(0); + expect(mock.entries).toHaveLength(entriesBefore); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toContain("write"); + expect(mock.userMessages).toEqual([{ content: "Implement the plan.", options: undefined }]); + expect(mock.entries.at(-1)?.data).toMatchObject({ + latestPlan: undefined, + awaitingAction: false, + }); + }); + + it("lets the latest request cancel a queued entry", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + const initialTools = [...mock.activeTools]; + + await mock.commands.get("plan")!.handler("obsolete prompt", ctx.ctx); + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toEqual(initialTools); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.userMessages).toHaveLength(0); + }); + + it("lets /plan cancel a queued exit without opening the menu", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(ctx.selectCalls).toHaveLength(0); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + expect(mock.activeTools).not.toContain("write"); + }); + + it("finishes Save-plan cleanup before applying a queued exit", async () => { + const mock = createMockPi(); + const setActiveTools = vi.spyOn(mock.pi, "setActiveTools"); + createExtension(mock.pi); + const ctx = createMockContext({ selectResponses: [PLAN_MENU_LABELS.save] }); + await mock.commands.get("plan")!.handler("", ctx.ctx); + await mock.fireEvent( + "agent_end", + { + type: "agent_end", + messages: [assistantText("\n# Save Me\n")], + }, + ctx, + ); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(setActiveTools.mock.calls.at(-2)?.[0]).not.toContain("write"); + expect(setActiveTools.mock.calls.at(-1)?.[0]).toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + }); + + it("drops a queued transition on session shutdown", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + await mock.commands.get("plan")!.handler("queued prompt", ctx.ctx); + await mock.fireEvent("session_shutdown", { type: "session_shutdown", reason: "quit" }, ctx); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.activeTools).toContain("write"); + expect(mock.userMessages).toHaveLength(0); + }); + + it("warns instead of opening tool configuration while busy", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + await mock.commands.get("plan:tools")!.handler("", ctx.ctx); + + expect(ctx.customCalls).toHaveLength(0); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(ctx.notifications).toContainEqual({ + message: "Plan-mode tool configuration is unavailable while Pi is busy.", + type: "warning", + }); + }); + }); + ``` + + Delete the old `"implement: queues the full plan as a follow-up when busy"` test; the deferred implementation case above replaces it. In the existing idle `"implement: exits plan mode and sends implementation message"` test, replace the full-plan assertion with: + + ```ts + expect(mock.userMessages).toEqual([{ content: "Implement the plan.", options: undefined }]); + ``` + + In `"warns and does nothing for stale Save plan menu action while busy"`, create an initially idle context and make it busy only after the plan is captured: + + ```ts + const ctx = createMockContext({ selectResponses: [PLAN_MENU_LABELS.save] }); + ``` + + ```ts + await mock.fireEvent( + "agent_end", + { + type: "agent_end", + messages: [assistantText("\n# Plan\n")], + }, + ctx, + ); + ctx.setIdle(false); + await handler("", ctx.ctx); + ``` + +- [ ] **Step 3: Run the integration tests and verify the new cases fail** + + Run: + + ```bash + pnpm vitest run tests/index.test.ts + ``` + + Expected: FAIL because busy commands currently mutate tools/state immediately and submit follow-up messages before `agent_settled`. + +- [ ] **Step 4: Add the single pending-transition slot and its apply/request helpers** + + In `src/index.ts`, add the local type above `createExtension`: + + ```ts + interface PendingModeTransition { + enabled: boolean; + prompt?: string; + consumePlan?: boolean; + } + ``` + + Add the slot beside the other in-memory lifecycle state: + + ```ts + let pendingModeTransition: PendingModeTransition | undefined; + ``` + + Add these functions after `sendPlanModeMessage`: + + ```ts + function applyModeTransition( + transition: PendingModeTransition, + ctx: ExtensionContext, + ): void { + if (transition.enabled !== state.enabled) { + if (transition.enabled) doEnter(ctx); + else doExit(ctx); + } + + if (transition.consumePlan) { + state = { ...state, latestPlan: undefined, awaitingAction: false }; + persist(); + updateUi(ctx); + } + + if (transition.prompt) sendPlanModeMessage(transition.prompt, ctx); + } + + function requestModeTransition( + transition: PendingModeTransition, + ctx: ExtensionContext, + ): boolean { + if (ctx.isIdle()) { + pendingModeTransition = undefined; + applyModeTransition(transition, ctx); + return true; + } + + pendingModeTransition = transition; + ctx.ui.notify("Plan mode change queued until Pi is idle.", "info"); + return false; + } + ``` + +- [ ] **Step 5: Route mode-changing commands and menu actions through the helper** + + Replace the `implement`, `exit`, and `tools` menu branches with: + + ```ts + case "implement": + if (!state.latestPlan) { + ctx.ui.notify("No latest plan is available to implement.", "warning"); + break; + } + if ( + requestModeTransition( + { enabled: false, prompt: "Implement the plan.", consumePlan: true }, + ctx, + ) + ) { + ctx.ui.notify("Implementing plan. Full access restored.", "info"); + } + break; + case "exit": + if (requestModeTransition({ enabled: false }, ctx)) { + ctx.ui.notify("Plan mode disabled.", "info"); + } + break; + case "tools": + await runToolSelector(ctx); + break; + ``` + + Start `runToolSelector` with: + + ```ts + if (!ctx.isIdle()) { + ctx.ui.notify("Plan-mode tool configuration is unavailable while Pi is busy.", "warning"); + return; + } + ``` + + Replace the `/plan` command's argument and disabled-mode branches with: + + ```ts + if (command) { + if (!state.enabled) { + if (requestModeTransition({ enabled: true, prompt: command }, ctx)) { + ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + } + return; + } + sendPlanModeMessage(command, ctx); + return; + } + + if (!state.enabled || pendingModeTransition?.enabled === false) { + if (requestModeTransition({ enabled: true }, ctx)) { + ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + } + return; + } + ``` + + Replace the `/plan:exit` handler body with: + + ```ts + clearPendingMenu(); + if (requestModeTransition({ enabled: false }, ctx)) { + ctx.ui.notify("Plan mode disabled.", "info"); + } + ``` + + In `/plan:tools`, guard before `doEnter`: + + ```ts + clearPendingMenu(); + if (!ctx.isIdle()) { + await runToolSelector(ctx); + return; + } + if (!state.enabled) { + doEnter(ctx); + ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + } + await runToolSelector(ctx); + ``` + +- [ ] **Step 6: Apply the latest request after Save cleanup and drop it at shutdown** + + Replace the `agent_settled` handler with: + + ```ts + pi.on("agent_settled", async (_event, ctx) => { + if (savePlanSession !== undefined) { + const didSave = savePlanSession.saved(); + savePlanSession = undefined; + if (state.enabled) { + pi.setActiveTools(planModeToolNames(state.selectedToolNames)); + } + if (!didSave) { + ctx.ui.notify("Save plan failed or was not completed.", "warning"); + } + } + + const transition = pendingModeTransition; + pendingModeTransition = undefined; + if (transition) applyModeTransition(transition, ctx); + }); + ``` + + Add this line near the start of `session_shutdown`, immediately after `clearPendingMenu()`: + + ```ts + pendingModeTransition = undefined; + ``` + +- [ ] **Step 7: Run the focused tests and verify they pass** + + Run: + + ```bash + pnpm vitest run tests/index.test.ts + ``` + + Expected: PASS, including deferred entry, exit, implementation, cancellation, Save ordering, tool configuration, and shutdown cases. + +- [ ] **Step 8: Commit deferred switching** + + ```bash + git add src/index.ts tests/helpers.ts tests/index.test.ts + git commit -m "fix: defer plan mode switches until idle" + ``` + +### Task 3: Clear the Plan-menu cache at the next real turn + +**Files:** +- Modify: `tests/index.test.ts:671-823,1137-1222,1693-1728` +- Modify: `src/index.ts:274-296` + +- [ ] **Step 1: Replace handoff/cache tests with shared turn-start behavior** + + Replace the restored-disabled-state handoff test with: + + ```ts + it("clears and persists restored cache on the next normal turn without a handoff", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + entries: [ + { + type: "custom", + customType: "plan-mode-state", + data: { enabled: false, latestPlan: "# Restored Plan", awaitingAction: true }, + id: "restored", + parentId: null, + timestamp: new Date().toISOString(), + }, + ], + }); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + + const result = await mock.fireEvent( + "before_agent_start", + { type: "before_agent_start", systemPrompt: "base prompt" }, + ctx, + ); + + expect(result).toBeUndefined(); + expect(mock.entries.at(-1)?.data).toMatchObject({ + latestPlan: undefined, + awaitingAction: false, + }); + }); + ``` + + Replace the re-entry discard test with: + + ```ts + it("keeps cached plan actions across exit and re-entry until the next turn", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + entries: [ + { + type: "custom", + customType: "plan-mode-state", + data: { enabled: true, latestPlan: "# Pending Plan", awaitingAction: true }, + id: "pending", + parentId: null, + timestamp: new Date().toISOString(), + }, + ], + selectResponses: [PLAN_MENU_LABELS.stay], + }); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(ctx.selectCalls[0]?.options).toContain(PLAN_MENU_LABELS.implement); + expect(ctx.selectCalls[0]?.options).toContain(PLAN_MENU_LABELS["show-plan"]); + + const result = await mock.fireEvent( + "before_agent_start", + { type: "before_agent_start", systemPrompt: "base prompt" }, + ctx, + ); + + expect((result as { systemPrompt: string }).systemPrompt).toContain("[PLAN MODE ACTIVE]"); + expect(mock.entries.at(-1)?.data).toMatchObject({ + latestPlan: undefined, + awaitingAction: false, + }); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + }); + ``` + + Update the Save-turn exit assertion to remove `[PLAN HANDOFF]`: + + ```ts + expect(result).toBeUndefined(); + ``` + +- [ ] **Step 2: Run the index tests and verify the lifecycle cases fail** + + Run: + + ```bash + pnpm vitest run tests/index.test.ts + ``` + + Expected: FAIL because normal mode still injects `[PLAN HANDOFF]` and enabled-mode cache clearing is not persisted. + +- [ ] **Step 3: Replace `before_agent_start` with one shared cache-clear path** + + Keep the Save-plan branch first, then replace the remaining handler body with: + + ```ts + if (state.latestPlan !== undefined || state.awaitingAction) { + state = { ...state, latestPlan: undefined, awaitingAction: false }; + persist(); + updateUi(ctx); + } + + if (!state.enabled) return; + + pi.setActiveTools(planModeToolNames(state.selectedToolNames)); + return { + systemPrompt: `${event.systemPrompt}\n\n${buildPlanModePrompt()}`, + }; + ``` + + This removes `[PLAN HANDOFF]` entirely. Do not change the preceding Save-plan branch; it must retain the cache until its specialized turn settles. + +- [ ] **Step 4: Run all behavior-focused tests** + + Run: + + ```bash + pnpm vitest run tests/core/context.test.ts tests/core/state.test.ts tests/index.test.ts tests/core/save-plan.test.ts tests/tui/menus.test.ts + ``` + + Expected: PASS. + +- [ ] **Step 5: Commit cache lifecycle behavior** + + ```bash + git add src/index.ts tests/index.test.ts + git commit -m "fix: clear plan menu cache at turn start" + ``` + +### Task 4: Document continuous context and idle-deferred switching + +**Files:** +- Modify: `README.md:54-87` +- Modify: `CHANGELOG.md:1-5` + +- [ ] **Step 1: Update the README workflow and command reference** + + Replace the Implement and Exit bullets under “Work through a plan” with: + + ```markdown + - **Implement this plan** - turn Plan mode off, restore full tool access, and submit `Implement the plan.` in the same conversation. + - **Exit Plan mode** - leave planning without removing prior user or assistant messages. The latest-plan menu cache remains available until the next turn starts. + ``` + + Replace the `/plan:exit` command row with: + + ```markdown + | `/plan:exit` | Turn off Plan mode and restore the previous tool set without removing conversation history. | + ``` + + Add this paragraph after the command table: + + ```markdown + If Pi is busy, mode-changing commands and menu actions wait until the current turn settles. Only the latest queued switch is applied. Showing a plan and staying in Plan mode remain immediate; tool configuration requires Pi to be idle. + ``` + +- [ ] **Step 2: Add an Unreleased changelog entry** + + Insert this above `0.4.0`: + + ```markdown + ## [Unreleased] + + ### Changed + + - Preserve complete conversation context when switching between Plan mode and normal mode; old display-only proposed-plan messages remain filtered to avoid duplicate context. + - Defer mode changes requested during an active turn until Pi is idle, with the latest queued request taking precedence. + - Submit `Implement the plan.` in the retained conversation instead of copying the full cached plan into a new handoff prompt. + ``` + +- [ ] **Step 3: Run documentation formatting checks** + + Run: + + ```bash + git diff --check -- README.md CHANGELOG.md + ``` + + Expected: exit code 0 with no whitespace diagnostics. + +- [ ] **Step 4: Commit the documentation** + + ```bash + git add README.md CHANGELOG.md + git commit -m "docs: explain persistent plan mode context" + ``` + +### Task 5: Verify the complete package + +**Files:** None. + +- [ ] **Step 1: Run the full quality suite** + + Run: + + ```bash + pnpm check + ``` + + Expected: Biome lint, TypeScript typecheck, and all Vitest tests pass. + +- [ ] **Step 2: Verify package contents without publishing** + + Run: + + ```bash + pnpm run pack:dry-run + ``` + + Expected: exit code 0; the tarball includes `src`, `docs/assets`, `LICENSE`, `CHANGELOG.md`, and `README.md`, with no test files or generated implementation artifacts. + +- [ ] **Step 3: Check scope and worktree cleanliness** + + Run: + + ```bash + git diff --check + git status --short + ``` + + Expected: `git diff --check` exits 0. `git status --short` shows no uncommitted implementation changes after the task commits. + +## Acceptance Criteria + +- User and assistant messages, including assistant `` blocks, survive Plan → normal → Plan switches. +- Legacy `customType: "proposed-plan"` messages are filtered in both modes so old sessions do not duplicate a plan under a user role. +- Entering or exiting while idle updates tools, state, UI, and persistence immediately. +- Entering, exiting, or implementing while busy changes nothing until `agent_settled`; then the latest queued request applies once. +- An opposite busy request can cancel a queued transition even when its target matches the current mode. +- Save-plan cleanup occurs before a pending transition, and session shutdown discards pending work. +- Exit and re-entry without a turn preserve `latestPlan`; the next real turn clears and persists the menu cache in either mode. +- Implement switches to normal mode, consumes `latestPlan`, and submits exactly `Implement the plan.` in the retained conversation. +- Tool configuration while busy warns without changing mode or opening a selector. +- No persisted state field, migration, dependency, new module, session branch, Shift-Tab shortcut, or fresh-context action is added. diff --git a/docs/superpowers/specs/2026-08-02-preserve-plan-context-design.md b/docs/superpowers/specs/2026-08-02-preserve-plan-context-design.md new file mode 100644 index 0000000..8d1c105 --- /dev/null +++ b/docs/superpowers/specs/2026-08-02-preserve-plan-context-design.md @@ -0,0 +1,81 @@ +# Preserve Plan Context Across Mode Switches Design + +## Summary + +Plan mode and normal mode will share one continuous Pi conversation. Switching modes changes only the extension's active tools, safety policy, UI, persisted enabled state, and per-turn Plan instructions. User and assistant messages, including `` blocks, remain available to later turns. + +Mode changes requested while Pi is busy will be deferred until `agent_settled`. Only the latest queued transition is applied. This prevents a running turn from combining one mode's system prompt with the other mode's tools. + +## Reference Behavior + +The design follows behavior confirmed in the referenced source trees: + +- Codex stores collaboration mode as thread/session configuration, updates it in place, and renders only the changed mode instructions into model context (`codex-rs/core/src/session/session.rs` and `codex-rs/core/src/context/world_state/collaboration_mode.rs`). +- Codex's same-context implementation action switches to Default mode and submits `Implement the plan.`; clearing context is a separate explicit action (`codex-rs/tui/src/chatwidget/plan_implementation.rs`). +- Codex rejects mode changes while a turn is running (`codex-rs/tui/src/chatwidget/input_flow.rs`). +- Pi executes extension commands during streaming, and an active-tool update can affect the next provider request in the same run (`packages/coding-agent/src/core/agent-session.ts` and `packages/coding-agent/test/suite/regressions/6162-extension-active-tools-next-turn.test.ts`). +- Pi marks the session idle before emitting `agent_settled`, making that event the safe transition boundary. + +## Context and Cache Behavior + +The context hook will stop removing or rewriting assistant `` blocks. It will retain one compatibility responsibility: remove legacy `customType: "proposed-plan"` messages in every mode. Pi converts custom messages to model-visible user messages regardless of their display purpose, so retaining these obsolete v0.3 duplicates would add the same plan twice under different roles. + +`latestPlan` remains persisted menu/save state, not a substitute for conversation context. Entering or exiting without another turn preserves it. At the start of the next real turn, a shared cache-clear path removes `latestPlan` and `awaitingAction`, persists the change, and refreshes the UI before applying the selected mode's prompt. Save-plan turns retain their captured plan until their specialized lifecycle finishes. + +## Deferred Transition Model + +The extension keeps one non-persisted pending record local to `src/index.ts`: + +```ts +interface PendingModeTransition { + enabled: boolean; + prompt?: string; + consumePlan?: boolean; +} +``` + +When Pi is idle, a requested transition applies immediately. When Pi is busy, it replaces the pending record and produces a concise notification. Replacing the whole record gives the latest request precedence and discards intermediate toggles and prompts. + +At `agent_settled`, the extension first completes any Save-plan cleanup, then takes and clears the pending record. It changes mode only if the target differs from current state, consumes the cached plan only for implementation, and finally sends the optional prompt. Since Pi is idle at this point, the prompt starts a clean turn through the normal `before_agent_start` flow. + +| Request | Idle behavior | Busy behavior | +| --- | --- | --- | +| `/plan` while normal | Enter Plan mode | Queue Plan mode | +| `/plan ` while normal | Enter, then send prompt | Queue Plan mode and prompt | +| `/plan ` while already planning | Send prompt | Send as same-mode follow-up | +| `/plan:exit` or Exit action | Exit Plan mode | Queue normal mode | +| Implement action | Exit, consume cache, send `Implement the plan.` | Queue those three actions | +| Show plan or Stay | Perform immediately | Perform immediately | +| Configure tools | Open selector | Warn that configuration requires idle Pi | + +An opposite request may replace a pending transition even when the current mode already matches the new target. This lets `/plan:exit` cancel a queued entry and `/plan` cancel a queued exit. Session shutdown discards the pending in-memory request; it must not leak into another session or require a state migration. + +## Components + +- `src/core/context.ts` will retain plan capture and a small legacy-custom-message filter; assistant plan sanitization is removed. +- `src/core/state.ts` will preserve `latestPlan` on entry while continuing to reset `awaitingAction`. +- `src/index.ts` will own pending transitions, shared turn-start cache clearing, deferred settlement, and same-context implementation submission. +- Existing Save-plan, safety, tool-selection, and menu modules keep their responsibilities. No new module or dependency is needed. + +## Error Handling + +- Mode-changing commands never call `setActiveTools` while Pi is busy. +- Tool configuration while busy reports a warning instead of queuing an interactive modal. +- A later transition safely replaces the earlier pending transition; there is no FIFO replay. +- Save-plan settlement runs before a deferred mode transition so temporary write access is removed before normal tools are restored or Plan tools are reapplied. +- Existing Pi runtime error reporting remains responsible for failures when an idle deferred prompt starts. + +## Test Strategy + +- Verify assistant plan blocks survive context transformation in both modes while legacy custom duplicates are removed in both modes. +- Verify exit and re-entry without a turn preserve `latestPlan`; the next normal or Plan turn clears and persists only the cache. +- Verify idle implementation switches immediately and submits exactly `Implement the plan.`. +- Verify busy entry, exit, and implementation leave current tools/state/messages unchanged until `agent_settled`, then apply atomically. +- Verify the latest busy transition replaces an earlier one, including cancellation by a target matching the current mode. +- Verify Save-plan cleanup precedes a pending transition and session shutdown drops pending work. +- Run `pnpm check` and `pnpm run pack:dry-run` as final verification. + +## Non-Goals + +- No Shift-Tab mode cycling, fresh-context implementation option, new persisted field, migration, dependency, or session branch. +- No changes to Plan-mode shell safety, Save-plan authorization, tool-selection persistence, or proposed-plan parsing format. diff --git a/src/core/context.ts b/src/core/context.ts index 922ccee..40b75c7 100644 --- a/src/core/context.ts +++ b/src/core/context.ts @@ -6,8 +6,6 @@ import { PROPOSED_PLAN_MESSAGE_TYPE } from "../shared/constants.ts"; const PLAN_BLOCK_PATTERN = /^[ \t]*[ \t]*\r?\n([\s\S]*?)^[ \t]*<\/proposed_plan>[ \t]*\r?$/im; -const ALL_PLAN_BLOCK_PATTERN = - /^[ \t]*[ \t]*\r?\n[\s\S]*?^[ \t]*<\/proposed_plan>[ \t]*\r?$/gim; type AssistantMessage = Extract; @@ -18,42 +16,6 @@ function assistantText(message: AssistantMessage): string { .join("\n"); } -function sanitizeAssistantMessage( - message: AssistantMessage, -): AssistantMessage { - const content = message.content; - const ranges = [...assistantText(message).matchAll(ALL_PLAN_BLOCK_PATTERN)].map( - (match) => [match.index, match.index + match[0].length] as const, - ); - if (ranges.length === 0) return message; - - let offset = 0; - const sanitizedContent = content.map((part) => { - if (part.type !== "text") return part; - const start = offset; - const end = start + part.text.length; - offset = end + 1; - - let sanitizedText = part.text; - for (let index = ranges.length - 1; index >= 0; index -= 1) { - const range = ranges[index]; - if (!range) continue; - const overlapStart = Math.max(start, range[0]); - const overlapEnd = Math.min(end, range[1]); - if (overlapStart >= overlapEnd) continue; - - const localStart = overlapStart - start; - const localEnd = overlapEnd - start; - sanitizedText = - sanitizedText.slice(0, localStart) + sanitizedText.slice(localEnd); - } - - return sanitizedText === part.text ? part : { ...part, text: sanitizedText }; - }); - - return { ...message, content: sanitizedContent }; -} - export function captureProposedPlan( messages: AgentEndEvent["messages"], ): string | undefined { @@ -67,21 +29,13 @@ export function captureProposedPlan( return undefined; } -export function sanitizePlanModeContext( +export function filterLegacyProposedPlanMessages( messages: ContextEvent["messages"], - enabled: boolean, ): { messages: ContextEvent["messages"] } | undefined { - if (enabled) return undefined; - - const sanitizedMessages = messages.flatMap((message) => { - if (message.role === "custom" && message.customType === PROPOSED_PLAN_MESSAGE_TYPE) { - return []; - } - if (message.role !== "assistant") return [message]; - return [sanitizeAssistantMessage(message)]; - }); + const filtered = messages.filter( + (message) => + message.role !== "custom" || message.customType !== PROPOSED_PLAN_MESSAGE_TYPE, + ); - return messages.every((message, index) => sanitizedMessages[index] === message) - ? undefined - : { messages: sanitizedMessages }; + return filtered.length === messages.length ? undefined : { messages: filtered }; } diff --git a/src/core/state.ts b/src/core/state.ts index 73580b9..be04f50 100644 --- a/src/core/state.ts +++ b/src/core/state.ts @@ -12,7 +12,7 @@ export function createInitialState(): PlanModeState { } export function enterPlanMode(state: PlanModeState): PlanModeState { - return { ...state, enabled: true, latestPlan: undefined, awaitingAction: false }; + return { ...state, enabled: true, awaitingAction: false }; } export function exitPlanMode(state: PlanModeState): PlanModeState { diff --git a/src/index.ts b/src/index.ts index b2e894a..9039297 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,11 @@ -import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; +import type { + ExtensionAPI, + ExtensionCommandContext, + ExtensionContext, +} from "@earendil-works/pi-coding-agent"; import { captureProposedPlan, - sanitizePlanModeContext, + filterLegacyProposedPlanMessages, } from "./core/context.ts"; import { buildPlanModePrompt } from "./core/prompt.ts"; import { createSavePlanSession } from "./core/save-plan.ts"; @@ -24,11 +28,18 @@ import type { PlanModeState } from "./shared/types.ts"; import { type PlanMenuAction, showPlanMenu, showPlanReadyMenu } from "./tui/menus.ts"; import { createToolSelectorComponent } from "./tui/tool-selector.ts"; +interface PendingModeTransition { + enabled: boolean; + prompt?: string; + consumePlan?: boolean; +} + export default function createExtension(pi: ExtensionAPI): void { let state: PlanModeState = createInitialState(); let previousTools: string[] | undefined; let pendingMenuTimer: ReturnType | undefined; let savePlanSession: ReturnType | undefined; + let pendingModeTransition: PendingModeTransition | undefined; pi.registerFlag("plan", { description: "Start in plan mode (read-only exploration)", @@ -103,7 +114,55 @@ export default function createExtension(pi: ExtensionAPI): void { pi.sendUserMessage(content, ctx.isIdle() ? undefined : { deliverAs: "followUp" }); } + function applyModeTransition( + transition: PendingModeTransition, + ctx: ExtensionContext, + ): void { + if (transition.enabled !== state.enabled) { + if (transition.enabled) doEnter(ctx); + else doExit(ctx); + } + + if (transition.consumePlan) { + state = { ...state, latestPlan: undefined, awaitingAction: false }; + persist(); + updateUi(ctx); + } + + if (transition.prompt) sendPlanModeMessage(transition.prompt, ctx); + } + + function requestModeTransition( + transition: PendingModeTransition, + ctx: ExtensionContext, + ): boolean { + if (ctx.isIdle()) { + pendingModeTransition = undefined; + applyModeTransition(transition, ctx); + return true; + } + + const waitForIdle = (ctx as Partial).waitForIdle; + if (typeof waitForIdle !== "function") { + ctx.ui.notify("Plan mode change is unavailable until Pi is idle.", "warning"); + return false; + } + + pendingModeTransition = transition; + void waitForIdle.call(ctx).then(() => { + if (pendingModeTransition !== transition) return; + pendingModeTransition = undefined; + applyModeTransition(transition, ctx); + }); + ctx.ui.notify("Plan mode change queued until Pi is idle.", "info"); + return false; + } + async function runToolSelector(ctx: ExtensionContext): Promise { + if (!ctx.isIdle()) { + ctx.ui.notify("Plan-mode tool configuration is unavailable while Pi is busy.", "warning"); + return; + } const allTools = safeGetAllTools(pi); const selections = await ctx.ui.custom((_tui, theme, _keybindings, done) => createToolSelectorComponent({ @@ -139,17 +198,18 @@ export default function createExtension(pi: ExtensionAPI): void { async function handleMenuAction(action: PlanMenuAction, ctx: ExtensionContext): Promise { switch (action) { case "implement": { - const plan = state.latestPlan; - doExit(ctx); - if (plan) { - state = { ...state, latestPlan: undefined, awaitingAction: false }; - persist(); - sendPlanModeMessage( - `Plan mode is now disabled. Full tool access is restored. Implement this proposed plan now:\n\n${plan}`, + if (!state.latestPlan) { + ctx.ui.notify("No latest plan is available to implement.", "warning"); + break; + } + if ( + requestModeTransition( + { enabled: false, prompt: "Implement the plan.", consumePlan: true }, ctx, - ); + ) + ) { + ctx.ui.notify("Implementing plan. Full access restored.", "info"); } - ctx.ui.notify("Implementing plan. Full access restored.", "info"); break; } case "save": @@ -166,8 +226,9 @@ export default function createExtension(pi: ExtensionAPI): void { sendPlanModeMessage(savePlanSession.userPrompt, ctx); break; case "exit": - doExit(ctx); - ctx.ui.notify("Plan mode disabled.", "info"); + if (requestModeTransition({ enabled: false }, ctx)) { + ctx.ui.notify("Plan mode disabled.", "info"); + } break; case "show-plan": if (state.latestPlan) { @@ -190,16 +251,19 @@ export default function createExtension(pi: ExtensionAPI): void { if (command) { if (!state.enabled) { - doEnter(ctx); - ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + if (requestModeTransition({ enabled: true, prompt: command }, ctx)) { + ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + } + return; } sendPlanModeMessage(command, ctx); return; } - if (!state.enabled) { - doEnter(ctx); - ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + if (!state.enabled || pendingModeTransition?.enabled === false) { + if (requestModeTransition({ enabled: true }, ctx)) { + ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + } return; } @@ -212,10 +276,9 @@ export default function createExtension(pi: ExtensionAPI): void { description: "Exit plan mode", handler: async (_args, ctx) => { clearPendingMenu(); - if (state.enabled) { - doExit(ctx); + if (requestModeTransition({ enabled: false }, ctx)) { + ctx.ui.notify("Plan mode disabled.", "info"); } - ctx.ui.notify("Plan mode disabled.", "info"); }, }); @@ -223,6 +286,10 @@ export default function createExtension(pi: ExtensionAPI): void { description: "Configure plan mode tools", handler: async (_args, ctx) => { clearPendingMenu(); + if (!ctx.isIdle()) { + await runToolSelector(ctx); + return; + } if (!state.enabled) { doEnter(ctx); ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); @@ -277,19 +344,15 @@ export default function createExtension(pi: ExtensionAPI): void { return savePlanSession.beforeAgentStart(event); } - if (!state.enabled) { - const plan = state.latestPlan; - if (!plan) return; + if (state.latestPlan !== undefined || state.awaitingAction) { state = { ...state, latestPlan: undefined, awaitingAction: false }; persist(); updateUi(ctx); - return { - systemPrompt: `${event.systemPrompt}\n\n[PLAN HANDOFF]\nThe latest proposed plan is available for this turn as context. Follow the current user request; do not implement the plan unless asked.\n\n${plan}`, - }; } + + if (!state.enabled) return; + pi.setActiveTools(planModeToolNames(state.selectedToolNames)); - state = { ...state, latestPlan: undefined, awaitingAction: false }; - updateUi(ctx); return { systemPrompt: `${event.systemPrompt}\n\n${buildPlanModePrompt()}`, }; @@ -314,19 +377,20 @@ export default function createExtension(pi: ExtensionAPI): void { }); pi.on("agent_settled", async (_event, ctx) => { - if (savePlanSession === undefined) return; - const didSave = savePlanSession.saved(); - savePlanSession = undefined; - if (state.enabled) { - pi.setActiveTools(planModeToolNames(state.selectedToolNames)); - } - if (!didSave) { - ctx.ui.notify("Save plan failed or was not completed.", "warning"); + if (savePlanSession !== undefined) { + const didSave = savePlanSession.saved(); + savePlanSession = undefined; + if (state.enabled) { + pi.setActiveTools(planModeToolNames(state.selectedToolNames)); + } + if (!didSave) { + ctx.ui.notify("Save plan failed or was not completed.", "warning"); + } } }); pi.on("context", async (event) => { - return sanitizePlanModeContext(event.messages, state.enabled); + return filterLegacyProposedPlanMessages(event.messages); }); pi.on("session_start", async (_event, ctx) => { @@ -350,6 +414,7 @@ export default function createExtension(pi: ExtensionAPI): void { pi.on("session_shutdown", async (_event, ctx) => { clearPendingMenu(); + pendingModeTransition = undefined; if (savePlanSession !== undefined && state.enabled) { pi.setActiveTools(planModeToolNames(state.selectedToolNames)); } diff --git a/tests/core/context.test.ts b/tests/core/context.test.ts index 5a9e967..380ee72 100644 --- a/tests/core/context.test.ts +++ b/tests/core/context.test.ts @@ -2,7 +2,7 @@ import type { AgentEndEvent, ContextEvent } from "@earendil-works/pi-coding-agen import { describe, expect, it } from "vitest"; import { captureProposedPlan, - sanitizePlanModeContext, + filterLegacyProposedPlanMessages, } from "../../src/core/context.ts"; type AgentMessage = ContextEvent["messages"][number]; @@ -90,61 +90,23 @@ describe("captureProposedPlan", () => { ).toBe("# \nLatest"); }); }); -describe("sanitizePlanModeContext", () => { - it("removes legacy proposed-plan messages and standalone blocks while disabled", () => { - const messages = [ - user("hello"), - proposedPlanMessage("old plan"), - assistantText( - "Intro mentions inline.\n\n# Old\n\nAfter.", - ), - ]; - - expect(sanitizePlanModeContext(messages, false)).toEqual({ - messages: [ - user("hello"), - assistantText("Intro mentions inline.\n\nAfter."), - ], - }); - }); - it("removes a standalone block split across text parts", () => { - const messages = [ - assistant([ - { type: "text", text: "Before.\n\n# " }, - { type: "thinking", thinking: "reasoning" }, - { type: "text", text: "Old\n\nAfter." }, - ]), - ]; +describe("filterLegacyProposedPlanMessages", () => { + it("removes only legacy custom plan messages", () => { + const plan = assistantText("\n# Current\n"); + const messages = [user("hello"), proposedPlanMessage("old duplicate"), plan]; - expect(sanitizePlanModeContext(messages, false)).toEqual({ - messages: [ - assistant([ - { type: "text", text: "Before.\n" }, - { type: "thinking", thinking: "reasoning" }, - { type: "text", text: "\nAfter." }, - ]), - ], + expect(filterLegacyProposedPlanMessages(messages)).toEqual({ + messages: [user("hello"), plan], }); }); - it("preserves user messages, non-text content, and malformed blocks", () => { + it("leaves assistant plan blocks and clean context unchanged", () => { const messages = [ - user("\n# User Plan\n"), - assistant([ - { type: "thinking", thinking: "reasoning" }, - { type: "text", text: "# Inline" }, - { type: "toolCall", id: "call-1", name: "read", arguments: {} }, - ]), + user("\n# User text\n"), + assistantText("Before\n\n# Assistant plan\n\nAfter"), ]; - expect(sanitizePlanModeContext(messages, false)).toBeUndefined(); - }); - - it("returns undefined without changes while enabled or when disabled context is clean", () => { - const messages = [assistantText("\n# Current\n")]; - - expect(sanitizePlanModeContext(messages, true)).toBeUndefined(); - expect(sanitizePlanModeContext([assistantText("No plan")], false)).toBeUndefined(); + expect(filterLegacyProposedPlanMessages(messages)).toBeUndefined(); }); }); diff --git a/tests/core/state.test.ts b/tests/core/state.test.ts index 7f98b73..32de338 100644 --- a/tests/core/state.test.ts +++ b/tests/core/state.test.ts @@ -43,15 +43,17 @@ describe("createInitialState", () => { }); describe("enterPlanMode", () => { - it("clears a pending plan and awaiting action", () => { + it("preserves the latest plan and clears awaiting action", () => { const state = { ...createInitialState(), - latestPlan: "stale plan", + latestPlan: "cached plan", awaitingAction: true, }; + const next = enterPlanMode(state); + expect(next.enabled).toBe(true); - expect(next.latestPlan).toBeUndefined(); + expect(next.latestPlan).toBe("cached plan"); expect(next.awaitingAction).toBe(false); }); diff --git a/tests/helpers.ts b/tests/helpers.ts index 4429b37..64ffa4f 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -48,6 +48,8 @@ export interface MockContext { selectCalls: Array<{ title: string; options: string[] }>; inputCalls: Array<{ title: string; placeholder?: string }>; customCalls: Array<{ result: unknown }>; + setIdle(value: boolean): void; + resolveIdleWait(): void; } export function createMockPi(options?: { @@ -121,6 +123,10 @@ export function createMockPi(options?: { return result; } } + if (name === "agent_settled") { + mockCtx.resolveIdleWait(); + await Promise.resolve(); + } return result; }, }; @@ -144,6 +150,8 @@ export function createMockContext(options?: { const customCalls: Array<{ result: unknown }> = []; const selectQueue = [...(options?.selectResponses ?? [])]; const sessionEntries: SessionEntry[] = options?.entries ?? []; + const idleWaiters: Array<() => void> = []; + let idle = options?.isIdle ?? true; const mockCtx: MockContext = { ctx: { @@ -190,7 +198,11 @@ export function createMockContext(options?: { }, }, hasUI: options?.hasUI ?? true, - isIdle: () => options?.isIdle ?? true, + isIdle: () => idle, + async waitForIdle() { + if (idle) return; + await new Promise((resolve) => idleWaiters.push(resolve)); + }, cwd: options?.cwd ?? process.cwd(), sessionManager: { getEntries: () => sessionEntries, @@ -202,6 +214,12 @@ export function createMockContext(options?: { selectCalls, inputCalls, customCalls, + setIdle(value: boolean) { + idle = value; + }, + resolveIdleWait() { + for (const resolve of idleWaiters.splice(0)) resolve(); + }, }; return mockCtx; diff --git a/tests/index.test.ts b/tests/index.test.ts index db3903b..7bb9e88 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -96,7 +96,7 @@ describe("/plan command", () => { it("warns and does nothing for stale Save plan menu action while busy", async () => { const mock = createMockPi(); createExtension(mock.pi); - const ctx = createMockContext({ isIdle: false, selectResponses: [PLAN_MENU_LABELS.save] }); + const ctx = createMockContext({ selectResponses: [PLAN_MENU_LABELS.save] }); const handler = mock.commands.get("plan")!.handler; await handler("", ctx.ctx); // on @@ -112,6 +112,7 @@ describe("/plan command", () => { ctx, ); + ctx.setIdle(false); await handler("", ctx.ctx); // stale save action await new Promise((resolve) => setTimeout(resolve, 0)); @@ -669,7 +670,7 @@ describe("session persistence", () => { }); describe("before_agent_start", () => { - it("injects a pending plan once after restoring disabled state", async () => { + it("clears and persists restored cache on the next normal turn without a handoff", async () => { const mock = createMockPi(); createExtension(mock.pi); const ctx = createMockContext({ @@ -687,30 +688,18 @@ describe("before_agent_start", () => { await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - const first = await mock.fireEvent( - "before_agent_start", - { type: "before_agent_start", systemPrompt: "base prompt" }, - ctx, - ); - expect((first as { systemPrompt: string }).systemPrompt).toContain("base prompt"); - expect((first as { systemPrompt: string }).systemPrompt).toContain("[PLAN HANDOFF]"); - expect((first as { systemPrompt: string }).systemPrompt).toContain( - "The latest proposed plan is available for this turn as context. Follow the current user request; do not implement the plan unless asked.", - ); - expect((first as { systemPrompt: string }).systemPrompt).toContain("# Restored Plan"); - - const second = await mock.fireEvent( + const result = await mock.fireEvent( "before_agent_start", { type: "before_agent_start", systemPrompt: "base prompt" }, ctx, ); - expect(second).toBeUndefined(); + expect(result).toBeUndefined(); const persisted = mock.entries.filter((entry) => entry.customType === "plan-mode-state"); expect(persisted.at(-1)?.data).toMatchObject({ latestPlan: undefined, awaitingAction: false }); }); - it("discards a pending plan when /plan re-enters plan mode", async () => { + it("keeps cached plan actions across exit and re-entry until the next turn", async () => { const mock = createMockPi(); createExtension(mock.pi); const ctx = createMockContext({ @@ -718,26 +707,33 @@ describe("before_agent_start", () => { { type: "custom", customType: "plan-mode-state", - data: { enabled: false, latestPlan: "# Pending Plan", awaitingAction: true }, + data: { enabled: true, latestPlan: "# Pending Plan", awaitingAction: true }, id: "1", parentId: null, timestamp: new Date().toISOString(), }, ], + selectResponses: [PLAN_MENU_LABELS.stay], }); - await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); await mock.commands.get("plan")!.handler("", ctx.ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(ctx.selectCalls[0]?.options).toContain(PLAN_MENU_LABELS.implement); + expect(ctx.selectCalls[0]?.options).toContain(PLAN_MENU_LABELS["show-plan"]); const result = await mock.fireEvent( "before_agent_start", { type: "before_agent_start", systemPrompt: "base prompt" }, ctx, ); - const { systemPrompt } = result as { systemPrompt: string }; - expect(systemPrompt).toContain("[PLAN MODE ACTIVE]"); - expect(systemPrompt).not.toContain("[PLAN HANDOFF]"); - expect(systemPrompt).not.toContain("# Pending Plan"); + + expect((result as { systemPrompt: string }).systemPrompt).toContain("[PLAN MODE ACTIVE]"); + const persisted = mock.entries.filter((entry) => entry.customType === "plan-mode-state"); + expect(persisted.at(-1)?.data).toMatchObject({ latestPlan: undefined, awaitingAction: false }); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); }); it("injects plan mode prompt when plan mode is enabled", async () => { @@ -925,137 +921,55 @@ describe("agent_end", () => { }); describe("context handler", () => { - it("filters proposed-plan messages when plan mode is off", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - - const result = await mock.fireEvent( - "context", - { - type: "context", - messages: [ - { role: "user", content: "hello" }, - { - role: "custom", - customType: "proposed-plan", - content: "old plan", - display: true, - timestamp: 0, - }, - { role: "assistant", content: [{ type: "text", text: "world" }] }, - ], - }, - ctx, - ); - - expect(result).toBeDefined(); - const { messages } = result as { messages: unknown[] }; - expect(messages).toHaveLength(2); - }); - - it("keeps proposed-plan messages when plan mode is on", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - const result = await mock.fireEvent( - "context", - { - type: "context", - messages: [ - { - role: "custom", - customType: "proposed-plan", - content: "current plan", - display: true, - timestamp: 0, - }, - { role: "assistant", content: [{ type: "text", text: "world" }] }, - ], - }, - ctx, - ); - - // No filtering needed — both messages stay - expect(result).toBeUndefined(); - }); - - it("returns undefined when nothing to filter", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - - const result = await mock.fireEvent( - "context", - { - type: "context", - messages: [ - { role: "user", content: "hello" }, - { role: "assistant", content: [{ type: "text", text: "world" }] }, - ], - }, - ctx, - ); - - expect(result).toBeUndefined(); - }); - - it("strips proposed_plan blocks from assistant messages when plan mode is off", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - - const result = await mock.fireEvent( - "context", - { - type: "context", - messages: [ - { - role: "assistant", - content: [ - { - type: "text", - text: "Here is the plan:\n\n# Old Plan\n\nEnd.", - }, - ], - }, - ], - }, - ctx, - ); + it.each([false, true])( + "preserves assistant plans and filters legacy duplicates when enabled=%s", + async (enabled) => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + if (enabled) await mock.commands.get("plan")!.handler("", ctx.ctx); + const assistantPlan = { + role: "assistant" as const, + content: [{ type: "text" as const, text: "\n# Plan\n" }], + }; + + const result = await mock.fireEvent( + "context", + { + type: "context", + messages: [ + { role: "user", content: "plan this" }, + { + role: "custom", + customType: "proposed-plan", + content: "legacy duplicate", + display: true, + timestamp: 0, + }, + assistantPlan, + ], + }, + ctx, + ); - expect(result).toBeDefined(); - const { messages } = result as { - messages: Array<{ content: Array<{ text?: string }> }>; - }; - expect(messages).toHaveLength(1); - expect(messages[0]?.content[0]?.text).toBe("Here is the plan:\n\nEnd."); - }); + expect(result).toEqual({ + messages: [{ role: "user", content: "plan this" }, assistantPlan], + }); + }, + ); - it("does not strip proposed_plan blocks when plan mode is on", async () => { + it("returns undefined for clean context", async () => { const mock = createMockPi(); createExtension(mock.pi); const ctx = createMockContext(); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - const result = await mock.fireEvent( - "context", - { - type: "context", - messages: [ - { - role: "assistant", - content: [{ type: "text", text: "\n# Current Plan\n" }], - }, - ], - }, - ctx, - ); - // No filtering — plan mode is on, no state entries to remove - expect(result).toBeUndefined(); + expect( + await mock.fireEvent( + "context", + { type: "context", messages: [{ role: "user", content: "hello" }] }, + ctx, + ), + ).toBeUndefined(); }); }); @@ -1162,37 +1076,42 @@ describe("plan menu actions", () => { expect(ctx.statuses.get("pi-plan")).toBeUndefined(); expect(mock.userMessages).toHaveLength(1); - expect(mock.userMessages[0].content).toBe( - "Plan mode is now disabled. Full tool access is restored. Implement this proposed plan now:\n\n# My Plan\n## Summary\nBuild the thing", - ); + expect(mock.userMessages[0].content).toBe("Implement the plan."); expect(ctx.inputCalls).toHaveLength(0); const persisted = mock.entries.filter((entry) => entry.customType === "plan-mode-state"); expect(persisted.at(-1)?.data).toMatchObject({ latestPlan: undefined, awaitingAction: false }); }); - it("implement: queues the full plan as a follow-up when busy", async () => { + it("queues the implementation and consumes the plan only when settled", async () => { const mock = createMockPi(); createExtension(mock.pi); + const plan = "# Queued Plan\n\n## Details\nKeep every line."; const ctx = createMockContext({ - isIdle: false, selectResponses: [PLAN_MENU_LABELS.implement], + entries: [ + { + type: "custom", + customType: "plan-mode-state", + data: { enabled: true, latestPlan: plan, awaitingAction: true }, + id: "queued-plan", + parentId: null, + timestamp: new Date().toISOString(), + }, + ], }); - const plan = "# Queued Plan\n\n## Details\nKeep every line."; + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - const handler = mock.commands.get("plan")!.handler; - await handler("", ctx.ctx); - await mock.fireEvent( - "agent_end", - { - type: "agent_end", - messages: [assistantText(`\n${plan}\n`)], - }, - ctx, - ); - await handler("", ctx.ctx); + ctx.setIdle(false); + await mock.commands.get("plan")!.handler("", ctx.ctx); - expect(mock.userMessages[0].content).toContain(plan); - expect(mock.userMessages[0].options).toEqual({ deliverAs: "followUp" }); + expect(mock.activeTools).not.toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBe("plan ready"); + expect(mock.userMessages).toHaveLength(0); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.userMessages).toEqual([{ content: "Implement the plan.", options: undefined }]); const persisted = mock.entries.filter((entry) => entry.customType === "plan-mode-state"); expect(persisted.at(-1)?.data).toMatchObject({ latestPlan: undefined, awaitingAction: false }); }); @@ -1262,6 +1181,209 @@ describe("plan menu actions", () => { }); +describe("deferred mode transitions", () => { + it("waits for every agent_settled handler before sending the queued prompt", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + let finishLaterHandler: (() => void) | undefined; + mock.pi.on( + "agent_settled", + () => + new Promise((resolve) => { + finishLaterHandler = resolve; + }), + ); + + await mock.commands.get("plan")!.handler("draft a migration plan", ctx.ctx); + ctx.setIdle(true); + const settlement = mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(mock.userMessages).toHaveLength(0); + + finishLaterHandler?.(); + await settlement; + await Promise.resolve(); + expect(mock.userMessages).toEqual([ + { content: "draft a migration plan", options: undefined }, + ]); + }); + + it("warns when a busy auto-menu action cannot wait for full settlement", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ selectResponses: [PLAN_MENU_LABELS.implement] }); + await mock.commands.get("plan")!.handler("", ctx.ctx); + delete (ctx.ctx as { waitForIdle?: unknown }).waitForIdle; + ctx.setIdle(false); + + await mock.fireEvent( + "agent_end", + { + type: "agent_end", + messages: [assistantText("\n# Plan\n")], + }, + ctx, + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(ctx.notifications).toContainEqual({ + message: "Plan mode change is unavailable until Pi is idle.", + type: "warning", + }); + expect(ctx.statuses.get("pi-plan")).toBe("plan ready"); + expect(mock.userMessages).toHaveLength(0); + }); + + it("queues plan entry and its prompt until agent_settled", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + const initialTools = [...mock.activeTools]; + ctx.setIdle(false); + + await mock.commands.get("plan")!.handler("draft a migration plan", ctx.ctx); + + expect(mock.activeTools).toEqual(initialTools); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.userMessages).toHaveLength(0); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).not.toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + expect(mock.userMessages).toEqual([ + { content: "draft a migration plan", options: undefined }, + ]); + }); + + it("queues plan exit without changing current tools or state", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + const planTools = [...mock.activeTools]; + ctx.setIdle(false); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + + expect(mock.activeTools).toEqual(planTools); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + }); + + it("sends an already-active Plan prompt as an immediate follow-up", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + + await mock.commands.get("plan")!.handler("refine the plan", ctx.ctx); + + expect(mock.userMessages).toEqual([ + { content: "refine the plan", options: { deliverAs: "followUp" } }, + ]); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + }); + + it("lets the latest request cancel a queued entry", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + const initialTools = [...mock.activeTools]; + + await mock.commands.get("plan")!.handler("obsolete prompt", ctx.ctx); + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toEqual(initialTools); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.userMessages).toHaveLength(0); + }); + + it("warns instead of opening tool configuration while busy", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + await mock.commands.get("plan:tools")!.handler("", ctx.ctx); + + expect(ctx.customCalls).toHaveLength(0); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(ctx.notifications).toContainEqual({ + message: "Plan-mode tool configuration is unavailable while Pi is busy.", + type: "warning", + }); + }); + + it("lets /plan cancel a queued exit without opening the menu", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(ctx.selectCalls).toHaveLength(0); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + expect(mock.activeTools).not.toContain("write"); + }); + + it("finishes Save-plan cleanup before applying a queued exit", async () => { + const mock = createMockPi(); + const setActiveTools = vi.spyOn(mock.pi, "setActiveTools"); + createExtension(mock.pi); + const ctx = createMockContext({ selectResponses: [PLAN_MENU_LABELS.save] }); + await mock.commands.get("plan")!.handler("", ctx.ctx); + await mock.fireEvent( + "agent_end", + { + type: "agent_end", + messages: [assistantText("\n# Save Me\n")], + }, + ctx, + ); + await mock.commands.get("plan")!.handler("", ctx.ctx); + ctx.setIdle(false); + await mock.commands.get("plan:exit")!.handler("", ctx.ctx); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(setActiveTools.mock.calls.at(-2)?.[0]).not.toContain("write"); + expect(setActiveTools.mock.calls.at(-1)?.[0]).toContain("write"); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + }); + + it("drops a queued transition on session shutdown", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + await mock.commands.get("plan")!.handler("queued prompt", ctx.ctx); + await mock.fireEvent("session_shutdown", { type: "session_shutdown", reason: "quit" }, ctx); + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.activeTools).toContain("write"); + expect(mock.userMessages).toHaveLength(0); + }); +}); + describe("/plan ", () => { it("enters plan mode and sends the prompt as a user message", async () => { const mock = createMockPi(); @@ -1327,7 +1449,7 @@ describe("agent_end auto-show menu", () => { expect(ctx.selectCalls).toHaveLength(1); expect(ctx.statuses.get("pi-plan")).toBeUndefined(); expect(mock.userMessages).toHaveLength(1); - expect(mock.userMessages[0].content as string).toContain("# Auto Plan"); + expect(mock.userMessages[0].content).toBe("Implement the plan."); }); it("cancels auto-menu when user manually invokes /plan first", async () => { @@ -1723,8 +1845,7 @@ describe("plan save lifecycle", () => { { type: "before_agent_start", systemPrompt: "base prompt" }, ctx, ); - expect((result as { systemPrompt: string }).systemPrompt).toContain("[PLAN HANDOFF]"); - expect((result as { systemPrompt: string }).systemPrompt).not.toContain("[PLAN SAVE TURN]"); + expect(result).toBeUndefined(); }); it("restores ordinary plan tools and clears UI on session shutdown during save turn", async () => {