From 3116eb2793ee0a3dfb4befd052c97ce2b403b663 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 16:58:03 -0400 Subject: [PATCH 1/7] docs: add Ctrl+Alt+P shortcut for Plan mode and enable opt-in mutation tool selection --- ...mode-shortcut-and-mutation-tool-toggles.md | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md diff --git a/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md b/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md new file mode 100644 index 0000000..c2b8450 --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md @@ -0,0 +1,201 @@ +# Plan-Mode Shortcut and Mutation-Tool Toggles 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:** Add a conflict-free `Ctrl+Alt+P` Plan-mode shortcut and let users independently opt `edit` and `write` into Plan mode through the existing persisted tool selector. + +**Architecture:** Reuse `selectedToolNames` as the single source of truth. A canonical mutation-tool tuple and helpers in `src/core/tools.ts` are consumed by selector policy, prompt generation, and runtime authorization. No new command, state field, dependency, or configuration format is introduced. Save-plan sessions retain their exact-content, write-only authorization and take precedence over ordinary mutation-tool selection. + +**Tech Stack:** TypeScript, Pi Extension API, `@earendil-works/pi-tui`, Vitest, Biome, pnpm. + +--- + +## File Map + +- `src/core/tools.ts`: canonical mutation-tool names, selector policy, active-tool composition, and existing persistence. +- `src/core/prompt.ts`: Plan-mode prompt rules parameterized by selected mutation tools. +- `src/tui/tool-selector.ts`: warning labels and toggle behavior driven by the shared policy. +- `src/index.ts`: `Ctrl+Alt+P`, selected-tool runtime authorization, dynamic prompt injection, and neutral notifications. +- `tests/helpers.ts`: shortcut-aware Pi test double. +- `tests/core/tools.test.ts`, `tests/core/prompt.test.ts`, `tests/tui/tool-selector.test.ts`, `tests/index.test.ts`: unit and integration regressions. +- `README.md`, `CHANGELOG.md`: current user-facing behavior; do not modify released changelog history. + +### Task 1: Add failing unit coverage for the shared mutation policy + +**Files:** `tests/core/tools.test.ts`, `tests/core/prompt.test.ts`, `tests/tui/tool-selector.test.ts` + +- [ ] **Step 1: Specify the canonical mutation helpers in tests.** Add expectations for the planned exports `isPlanMutationToolName` and `selectedPlanMutationToolNames`: + +```ts +expect(isPlanMutationToolName("edit")).toBe(true); +expect(isPlanMutationToolName("write")).toBe(true); +expect(isPlanMutationToolName("bash")).toBe(false); +expect(selectedPlanMutationToolNames([])).toEqual([]); +expect(selectedPlanMutationToolNames(["write", "custom", "edit"])).toEqual([ + "edit", + "write", +]); +``` + +- [ ] **Step 2: Replace blocked-policy expectations.** In `tests/core/tools.test.ts`, assert that both built-in `edit` and `write` return exactly `{ alwaysOn: false, toggleable: true, label: "user risk: built-in mutation" }`. Keep safe built-ins always-on and extension tools user-risk optional. Assert `planModeToolNames(["edit", "write"])` returns `read`, `bash`, `grep`, `find`, `ls`, `edit`, `write` in that order. + +- [ ] **Step 3: Add exact prompt-state tests.** Add tests for all four selection states. The expected dynamic lines are: + +```ts +expect(buildPlanModePrompt([])).toContain( + "Do not edit files, write files, or execute the plan.", +); +expect(buildPlanModePrompt(["edit"])).toContain( + "Enabled mutation tools: edit.", +); +expect(buildPlanModePrompt(["write"])).toContain( + "Enabled mutation tools: write.", +); +expect(buildPlanModePrompt(["edit", "write"])).toContain( + "Enabled mutation tools: edit, write.", +); +for (const selected of [[], ["edit"], ["write"], ["edit", "write"]]) { + expect(buildPlanModePrompt(selected)).toContain( + "Unselected mutation tools remain blocked. Do not execute the plan.", + ); +} +``` + +Also assert that the no-selection prompt contains the exit reminder for all changes, while a selected-tool prompt only requires exit before implementing the proposed plan. Assert all variants contain `Skills requiring unavailable tools or mutating bash commands will be blocked.` + +- [ ] **Step 4: Add selector interaction tests.** Update `tests/tui/tool-selector.test.ts` so built-in `edit` and `write` render unchecked with the shared warning label. Add a selector containing `edit`, `write`, and an extension tool; navigate and press Space on each row independently; verify Enter returns exactly the selected names. Keep safe built-ins non-toggleable. + +- [ ] **Step 5: Run the focused tests and verify red.** Run: + +```bash +mise exec node@24.15.0 -- pnpm exec vitest run \ + tests/core/tools.test.ts tests/core/prompt.test.ts tests/tui/tool-selector.test.ts +``` + +Expected: the new assertions fail against the current blocked-tool and static-prompt implementation. + +### Task 2: Implement the shared policy and coherent prompt + +**Files:** `src/core/tools.ts`, `src/tui/tool-selector.ts`, `src/core/prompt.ts` + +- [ ] **Step 1: Add the canonical mutation tuple and helpers.** In `src/core/tools.ts`, add: + +```ts +export const PLAN_MUTATION_TOOL_NAMES = ["edit", "write"] as const; +const PLAN_MUTATION_TOOL_NAME_SET = new Set(PLAN_MUTATION_TOOL_NAMES); + +export function isPlanMutationToolName(name: string): boolean { + return PLAN_MUTATION_TOOL_NAME_SET.has(name); +} + +export function selectedPlanMutationToolNames( + selected: readonly string[] = [], +): string[] { + return PLAN_MUTATION_TOOL_NAMES.filter((name) => selected.includes(name)); +} +``` + +Replace `BLOCKED_TOOL_NAMES` with this shared set. `getToolPolicy` must return the user-risk mutation policy for both names. Do not add either mutation tool to `SAFE_PLAN_TOOL_NAMES`; `planModeToolNames` must include them only when selected. Existing JSON persistence should then store and restore `edit: true` and `write: true` without schema changes. + +- [ ] **Step 2: Update selector copy without changing interaction mechanics.** Change the selector subtitle to `Optional tools run at user risk.`. Keep Space gated by `getToolPolicy(tool).toggleable` and keep safe-tool filtering on Enter; selected `edit`/`write` names must be returned as optional selections. + +- [ ] **Step 3: Make `buildPlanModePrompt` selection-aware.** Change the function signature to `buildPlanModePrompt(selectedToolNames: readonly string[] = [])` and derive enabled names with `selectedPlanMutationToolNames(selectedToolNames)`. Replace the static mutation and exit-rule text with these exact branches: + +```ts +const mutationRules = + enabled.length === 0 + ? "- Do not edit files, write files, or execute the plan." + : `- Enabled mutation tools: ${enabled.join(", ")}. Use them only for file changes the user explicitly requests.\n- Unselected mutation tools remain blocked. Do not execute the plan.`; + +const changeRule = + enabled.length === 0 + ? '- If the user asks you to make changes or implement something, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.' + : '- If the user asks you to implement the proposed plan, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; +``` + +Keep the existing phases, proposed-plan template, Bash restriction, and mode-transition rules. Replace the old skill sentence with `Skills requiring unavailable tools or mutating bash commands will be blocked.` so it is true in every selection state. + +- [ ] **Step 4: Rerun focused unit tests.** The three test files from Task 1 must pass. + +### Task 3: Add `Ctrl+Alt+P` and enforce permissions at runtime + +**Files:** `tests/helpers.ts`, `tests/index.test.ts`, `src/index.ts` + +- [ ] **Step 1: Extend the Pi test double.** Add a registered-shortcut type whose handler accepts `ExtensionContext`, add `shortcuts: Map` to `MockPi`, and implement `registerShortcut` by storing each registration under its string key. + +- [ ] **Step 2: Add failing shortcut tests.** In `tests/index.test.ts`, assert `mock.shortcuts.has("ctrl+alt+p")`, invoke its handler twice while idle, and verify status changes from `plan active` to cleared. Add a busy test that presses once, verifies the queued notification, settles the agent, and verifies the mode changes; press twice while busy and verify the second target cancels the first. + +- [ ] **Step 3: Add failing authorization tests.** Cover all four selection states in `tests/index.test.ts`: default `edit` and `write` calls block; edit-only permits edit but blocks write; write-only permits write but blocks edit; both permit both. Assert active tools match the same selections and `before_agent_start` includes the same prompt rules. + +- [ ] **Step 4: Add persistence and Save-plan regression tests.** Extend the existing temporary-agent-dir JSON tests to write/read both booleans. Add a session-start test proving persisted `edit`/`write` override session-only selections. Add a Save-plan test proving `edit` is blocked even if persisted, ordinary write authorization is not used during Save plan, and after `agent_settled` the selected ordinary tools are restored. + +- [ ] **Step 5: Run `tests/index.test.ts` and verify red.** Run: + +```bash +mise exec node@24.15.0 -- pnpm exec vitest run tests/index.test.ts +``` + +Expected: the new shortcut and permission cases fail before production changes. + +- [ ] **Step 6: Register the shortcut with Pi’s supported helper.** Import `Key` from `@earendil-works/pi-tui` and register after the existing command registrations: + +```ts +pi.registerShortcut(Key.ctrlAlt("p"), { + description: "Toggle Plan mode", + handler: async (ctx) => { + clearPendingMenu(); + const current = pendingModeTransition?.enabled ?? state.enabled; + const enabled = !current; + if (requestModeTransition({ enabled }, ctx)) { + ctx.ui.notify( + enabled ? "Plan mode enabled." : "Plan mode disabled.", + "info", + ); + } + }, +}); +``` + +Pi 0.83’s extension runner confirms this key is not reserved; Pi’s bundled Plan-mode example uses the same shortcut. Do not register Shift-Tab because Pi reserves it for `app.thinking.cycle`. + +- [ ] **Step 7: Replace unconditional mutation blocking.** In `src/index.ts`, handle Save plan first: route `write` to `savePlanSession.authorizeToolCall`, block `edit`, and do not allow ordinary mutation authorization during that session. Outside Save plan, for any `isPlanMutationToolName(event.toolName)`, return `undefined` only when `state.selectedToolNames?.includes(event.toolName)` is true; otherwise return the existing Plan-mode block result. Leave Bash validation unchanged. + +- [ ] **Step 8: Pass selection into the normal prompt and update notifications.** Call `buildPlanModePrompt(state.selectedToolNames ?? [])` from normal `before_agent_start`. Replace entry messages that say write tools are disabled with `Plan mode enabled.` and change the selector count message from `extension tool(s)` to `optional tool(s)`. + +- [ ] **Step 9: Rerun integration tests.** Run `mise exec node@24.15.0 -- pnpm exec vitest run tests/index.test.ts`; all existing and new cases must pass. + +### Task 4: Update current documentation only + +**Files:** `README.md`, `CHANGELOG.md` + +- [ ] **Step 1: Update README current behavior.** Change the description from unqualified read-only planning to read-only by default. Document `Ctrl+Alt+P` under Plan-mode usage and state that Shift-Tab remains Pi’s thinking-level shortcut. Update Configure Optional Tools and Safety Boundaries to say `edit` and `write` are independently disabled by default, persist after opt-in, and are limited to explicit user-requested changes. Keep Save plan’s exact one-write exception separate. + +- [ ] **Step 2: Update only `CHANGELOG.md`’s `[Unreleased]` section.** Add entries for the shortcut and independently persisted mutation-tool opt-ins. Do not edit released sections that describe historical behavior. + +### Task 5: Verify and review the implementation + +**Files:** all files modified above + +- [ ] **Step 1: Run the full project checks under the declared runtime.** Run: + +```bash +mise exec node@24.15.0 -- pnpm check +``` + +Expected: Biome lint, TypeScript, and all tests pass. The current baseline is 9 test files and 233 tests passing under this runtime. + +- [ ] **Step 2: Run the package dry run.** Run `mise exec node@24.15.0 -- pnpm run pack:dry-run`; confirm the package contains updated source, README, and changelog with no dependency or lockfile changes. + +- [ ] **Step 3: Smoke-test the local extension in isolation.** From the repository root run `mise exec node@24.15.0 -- pi --no-extensions -e ./src/index.ts --no-session`. Verify `Ctrl+Alt+P` toggles Plan mode, Shift-Tab still changes thinking level, `/plan:tools` independently selects `edit` and `write`, and unselected mutation calls are blocked. Use a temporary `PI_CODING_AGENT_DIR` when checking persisted JSON. + +- [ ] **Step 4: Review the final diff.** Run `git diff --check` and `git status --short`. Confirm only planned source, test, README, changelog, and plan files changed; no screenshot asset is modified. + +## Self-review + +- The four permission states have explicit policy, active-tool, prompt, runtime, persistence, and test coverage. +- The canonical mutation tuple prevents policy/prompt/runtime drift. +- Prompt rules no longer contradict explicit opt-in behavior or skill availability. +- Save-plan precedence remains exact-content and write-only, regardless of persisted mutation selections. +- Shortcut behavior matches Pi 0.83’s extension API and reserved-key policy; Shift-Tab remains untouched. +- Released changelog history and the screenshot are intentionally out of scope. +- No placeholders, new dependencies, migrations, or unrelated refactors remain. From b151b51c883ce9d55ccddb014d149a10ed18b48f Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 17:29:18 -0400 Subject: [PATCH 2/7] policy: canonical PLAN_MUTATION_TOOL_NAMES + helpers + selector subtitle + sorted output + tests --- src/core/tools.ts | 21 +++++++++++-- src/tui/tool-selector.ts | 10 +++--- tests/core/tools.test.ts | 54 ++++++++++++++++++++++++++++++--- tests/tui/tool-selector.test.ts | 31 +++++++++++++++---- 4 files changed, 98 insertions(+), 18 deletions(-) diff --git a/src/core/tools.ts b/src/core/tools.ts index 211b638..81f3ba6 100644 --- a/src/core/tools.ts +++ b/src/core/tools.ts @@ -8,10 +8,21 @@ import { const SAFE_PLAN_TOOL_NAMES = ["read", "bash", "grep", "find", "ls"]; const SAFE_PLAN_TOOL_NAME_SET = new Set(SAFE_PLAN_TOOL_NAMES); -const BLOCKED_TOOL_NAMES = new Set(["edit", "write"]); +export const PLAN_MUTATION_TOOL_NAMES = ["edit", "write"] as const; +const PLAN_MUTATION_TOOL_NAME_SET = new Set(PLAN_MUTATION_TOOL_NAMES); const NORMAL_MODE_TOOL_NAMES = ["read", "bash", "edit", "write"]; const CONFIG_FILENAME = "extensions/plan-tools.json"; +export function isPlanMutationToolName(name: string): boolean { + return PLAN_MUTATION_TOOL_NAME_SET.has(name); +} + +export function selectedPlanMutationToolNames( + selected: readonly string[] = [], +): string[] { + return PLAN_MUTATION_TOOL_NAMES.filter((name) => selected.includes(name)); +} + export type PlanToolInfo = Pick & { sourceInfo: Pick; }; @@ -35,8 +46,12 @@ export function getToolPolicy(tool: PlanToolInfo): { label: tool.name === "bash" ? "built-in limited" : "built-in", }; } - if (BLOCKED_TOOL_NAMES.has(tool.name)) { - return { alwaysOn: false, toggleable: false, label: "built-in blocked" }; + if (PLAN_MUTATION_TOOL_NAME_SET.has(tool.name)) { + return { + alwaysOn: false, + toggleable: true, + label: "user risk: built-in mutation", + }; } return { alwaysOn: false, toggleable: true, label: "built-in" }; } diff --git a/src/tui/tool-selector.ts b/src/tui/tool-selector.ts index 0c76b26..01e7ffd 100644 --- a/src/tui/tool-selector.ts +++ b/src/tui/tool-selector.ts @@ -10,7 +10,7 @@ const HELP_BASE = "Toggle: Space • Navigate: ↑/↓ • Page: ←/→ • Save: Enter • Cancel: Esc"; const HELP_SEARCHING = "Toggle: Space • Navigate: ↑/↓ • Cursor: ←/→ • Save: Enter • Cancel: Esc"; -const SUBTITLE = "Non-built-in tools run at user risk."; +const SUBTITLE = "Optional tools run at user risk."; const SEARCH_PLACEHOLDER = "Type to search"; interface ToolSelectorTheme { @@ -55,7 +55,6 @@ function clampCursor(state: ToolSelectorState, index: number): number { } function policyColor(label: string): string { - if (label === "built-in blocked") return "error"; if (label === "built-in limited" || label.startsWith("user risk")) return "warning"; return "dim"; } @@ -157,9 +156,10 @@ export function createToolSelectorComponent(options: { return; } if (matchesKey(data, Key.enter)) { - options.done( - [...state.selectedNames].filter((name) => !SAFE_PLAN_TOOL_NAMES.has(name)), - ); + const sortedNames = [...state.selectedNames] + .filter((name) => !SAFE_PLAN_TOOL_NAMES.has(name)) + .sort((a, b) => a.localeCompare(b)); + options.done(sortedNames); return; } if (matchesKey(data, Key.up)) { diff --git a/tests/core/tools.test.ts b/tests/core/tools.test.ts index a1af66d..6376357 100644 --- a/tests/core/tools.test.ts +++ b/tests/core/tools.test.ts @@ -5,6 +5,7 @@ import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { getToolPolicy, + isPlanMutationToolName, normalModeToolNames, type PlanToolInfo, planModeToolNames, @@ -12,6 +13,7 @@ import { safeGetActiveTools, safeGetAllTools, savePlanToolNames, + selectedPlanMutationToolNames, writeSelectedToolNames, } from "../../src/core/tools.ts"; @@ -19,6 +21,38 @@ function tool(name: string, source = "builtin"): PlanToolInfo { return { name, sourceInfo: { source } }; } +describe("isPlanMutationToolName", () => { + it("matches the canonical built-in mutation tools", () => { + expect(isPlanMutationToolName("edit")).toBe(true); + expect(isPlanMutationToolName("write")).toBe(true); + }); + + it("rejects non-mutation tool names", () => { + expect(isPlanMutationToolName("bash")).toBe(false); + expect(isPlanMutationToolName("read")).toBe(false); + expect(isPlanMutationToolName("custom")).toBe(false); + expect(isPlanMutationToolName("")).toBe(false); + }); +}); + +describe("selectedPlanMutationToolNames", () => { + it("returns an empty array for an empty or missing selection", () => { + expect(selectedPlanMutationToolNames([])).toEqual([]); + expect(selectedPlanMutationToolNames()).toEqual([]); + }); + + it("returns only the selected mutation tools in canonical order", () => { + expect(selectedPlanMutationToolNames(["write", "custom", "edit"])).toEqual([ + "edit", + "write", + ]); + }); + + it("ignores non-mutation tools in the selection", () => { + expect(selectedPlanMutationToolNames(["bash", "read", "custom"])).toEqual([]); + }); +}); + describe("getToolPolicy", () => { it("returns the fixed policy for every built-in row", () => { for (const name of ["read", "grep", "find", "ls"]) { @@ -35,13 +69,13 @@ describe("getToolPolicy", () => { }); expect(getToolPolicy(tool("edit"))).toEqual({ alwaysOn: false, - toggleable: false, - label: "built-in blocked", + toggleable: true, + label: "user risk: built-in mutation", }); expect(getToolPolicy(tool("write"))).toEqual({ alwaysOn: false, - toggleable: false, - label: "built-in blocked", + toggleable: true, + label: "user risk: built-in mutation", }); expect(getToolPolicy(tool("custom-built-in"))).toEqual({ alwaysOn: false, @@ -75,6 +109,18 @@ describe("tool names", () => { ]); }); + it("includes edit and write when explicitly selected", () => { + expect(planModeToolNames(["edit", "write"])).toEqual([ + "read", + "bash", + "grep", + "find", + "ls", + "edit", + "write", + ]); + }); + it("adds write only while saving", () => { expect(savePlanToolNames(true)).toEqual(["read", "bash", "grep", "find", "ls", "write"]); expect(savePlanToolNames(false)).toEqual(["read", "bash", "grep", "find", "ls"]); diff --git a/tests/tui/tool-selector.test.ts b/tests/tui/tool-selector.test.ts index 45c1411..e276c5c 100644 --- a/tests/tui/tool-selector.test.ts +++ b/tests/tui/tool-selector.test.ts @@ -63,13 +63,14 @@ describe("createToolSelectorComponent", () => { builtinTool("bash"), extensionTool("alpha"), builtinTool("edit"), + builtinTool("write"), ], ["alpha"], ); const lines = component.render(80); expect(lines[0]).toContain("Configure Plan-mode tools"); - expect(lines).toContain("Non-built-in tools run at user risk."); + expect(lines).toContain("Optional tools run at user risk."); expect(lines.at(-1)).toContain("Toggle"); expect(lines.at(-1)).toContain("Page"); expect(lines.findIndex((line) => line.includes("edit"))).toBeLessThan( @@ -80,7 +81,8 @@ describe("createToolSelectorComponent", () => { ); expect(row(lines, "read")).toContain("[•]"); expect(row(lines, "edit")).toContain("[ ]"); - expect(row(lines, "edit")).toContain("built-in blocked"); + expect(row(lines, "edit")).toContain("user risk: built-in mutation"); + expect(row(lines, "write")).toContain("user risk: built-in mutation"); expect(row(lines, "bash")).toContain("built-in limited"); expect(row(lines, "alpha")).toContain("[•]"); }); @@ -101,21 +103,38 @@ describe("createToolSelectorComponent", () => { it("does not toggle always-on or blocked built-ins", () => { const { component, done } = createSelector([ - builtinTool("edit"), builtinTool("read"), + builtinTool("bash"), extensionTool("custom"), ]); - component.handleInput?.(key.space); + component.handleInput?.(key.space); // safe component.handleInput?.(key.down); - component.handleInput?.(key.space); + component.handleInput?.(key.space); // safe component.handleInput?.(key.down); - component.handleInput?.(key.space); + component.handleInput?.(key.space); // custom (toggleable) component.handleInput?.(key.enter); expect(done).toHaveBeenCalledWith(["custom"]); }); + it("toggles edit and write independently and returns the selected names", () => { + const { component, done } = createSelector([ + builtinTool("edit"), + builtinTool("write"), + extensionTool("custom"), + ]); + + component.handleInput?.(key.space); // edit on + component.handleInput?.(key.down); + component.handleInput?.(key.space); // write on + component.handleInput?.(key.down); + component.handleInput?.(key.space); // custom on + component.handleInput?.(key.enter); + + expect(done).toHaveBeenCalledWith(["custom", "edit", "write"]); + }); + it("omits safe plan tool names from restored selections on save", () => { const { component, done } = createSelector([extensionTool("read")], ["read"]); From 71e2acdd474ef5c61e2981c6a2b2652b959b6539 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 17:29:21 -0400 Subject: [PATCH 3/7] prompt: selection-aware buildPlanModePrompt + tests --- src/core/prompt.ts | 61 +++++++++++++++++-------- tests/core/prompt.test.ts | 96 +++++++++++++++++++++++++++++++-------- 2 files changed, 119 insertions(+), 38 deletions(-) diff --git a/src/core/prompt.ts b/src/core/prompt.ts index df25380..7414bcc 100644 --- a/src/core/prompt.ts +++ b/src/core/prompt.ts @@ -1,22 +1,6 @@ -const PLAN_MODE_PROMPT = `[PLAN MODE ACTIVE] -# Plan Mode (Conversational) - -You are in Plan Mode. Produce a decision-complete implementation plan -before any code mutation happens. - -## Mode rules +import { selectedPlanMutationToolNames } from "./tools.ts"; -- Stay in Plan Mode until the user explicitly exits or chooses to implement. -- Do not edit files, write files, or execute the plan. -- If the user asks you to make changes or implement something, remind them - to exit Plan Mode first by running /plan and choosing "Implement this plan", - or by running /plan exit. -- Bash is restricted to read-only commands. -- Skills and tools listed in the system prompt are available if they operate - through currently enabled Plan Mode tools. Skills that require edit, write, - or mutating bash commands will be blocked. - -## Phase 1 -- Explore +const PHASES_AND_TEMPLATE = `## Phase 1 -- Explore - Use read-only tools to inspect files, search code, check configuration. - Resolve discoverable facts before asking the user. @@ -42,6 +26,43 @@ before any code mutation happens. - The plan must be decision-complete: no open questions for the implementer. - Do not ask "should I proceed?" -- the Plan Mode menu handles next steps.`; -export function buildPlanModePrompt(): string { - return PLAN_MODE_PROMPT; +const STATIC_RULES = `- Stay in Plan Mode until the user explicitly exits or chooses to implement. +- Bash is restricted to read-only commands. +- Skills requiring unavailable tools or mutating bash commands will be blocked.`; + +const NO_SELECTION_MUTATION_RULE = + "- Do not edit files, write files, or execute the plan.\n- Unselected mutation tools remain blocked. Do not execute the plan."; + +const NO_SELECTION_CHANGE_RULE = + '- If the user asks you to make changes or implement something, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; + +function buildSelectedMutationRules(enabled: string[]): string { + return [ + `- Enabled mutation tools: ${enabled.join(", ")}. Use them only for file changes the user explicitly requests.`, + "- Unselected mutation tools remain blocked. Do not execute the plan.", + ].join("\n"); +} + +const SELECTED_CHANGE_RULE = + '- If the user asks you to implement the proposed plan, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; + +export function buildPlanModePrompt(selectedToolNames: readonly string[] = []): string { + const enabled = selectedPlanMutationToolNames(selectedToolNames); + const mutationRule = + enabled.length === 0 ? NO_SELECTION_MUTATION_RULE : buildSelectedMutationRules(enabled); + const changeRule = enabled.length === 0 ? NO_SELECTION_CHANGE_RULE : SELECTED_CHANGE_RULE; + + return `[PLAN MODE ACTIVE] +# Plan Mode (Conversational) + +You are in Plan Mode. Produce a decision-complete implementation plan +before any code mutation happens. + +## Mode rules + +${STATIC_RULES} +${mutationRule} +${changeRule} + +${PHASES_AND_TEMPLATE}`; } diff --git a/tests/core/prompt.test.ts b/tests/core/prompt.test.ts index 1d8b2b2..0a7f8cd 100644 --- a/tests/core/prompt.test.ts +++ b/tests/core/prompt.test.ts @@ -6,26 +6,25 @@ describe("buildPlanModePrompt", () => { expect(buildPlanModePrompt()).toContain("[PLAN MODE ACTIVE]"); }); - it("contains mode rules", () => { - const prompt = buildPlanModePrompt(); - expect(prompt).toContain("Do not edit files"); - expect(prompt).toContain("/plan exit"); - expect(prompt).toContain("Bash is restricted to read-only commands"); - }); - - it("contains skill awareness line", () => { - expect(buildPlanModePrompt()).toContain("Skills and tools listed in the system prompt"); - }); - - it("contains the three planning phases", () => { - const prompt = buildPlanModePrompt(); + it.each([ + ["no selection", []], + ["edit only", ["edit"]], + ["write only", ["write"]], + ["edit and write", ["edit", "write"]], + ] as const)("contains the three planning phases for %s", (_label, selected) => { + const prompt = buildPlanModePrompt(selected); expect(prompt).toContain("Phase 1 -- Explore"); expect(prompt).toContain("Phase 2 -- Clarify"); expect(prompt).toContain("Phase 3 -- Plan"); }); - it("contains the proposed_plan template block", () => { - const prompt = buildPlanModePrompt(); + it.each([ + ["no selection", []], + ["edit only", ["edit"]], + ["write only", ["write"]], + ["edit and write", ["edit", "write"]], + ] as const)("contains the proposed_plan template for %s", (_label, selected) => { + const prompt = buildPlanModePrompt(selected); expect(prompt).toContain(""); expect(prompt).toContain(""); expect(prompt).toContain("## Summary"); @@ -34,8 +33,69 @@ describe("buildPlanModePrompt", () => { expect(prompt).toContain("## Assumptions"); }); - it("tells the agent not to ask should I proceed", () => { - expect(buildPlanModePrompt()).toContain("Do not ask"); - expect(buildPlanModePrompt()).toContain("menu handles next steps"); + it.each([ + ["no selection", []], + ["edit only", ["edit"]], + ["write only", ["write"]], + ["edit and write", ["edit", "write"]], + ] as const)("contains the phase-3 'do not ask' guidance for %s", (_label, selected) => { + const prompt = buildPlanModePrompt(selected); + expect(prompt).toContain("Do not ask"); + expect(prompt).toContain("menu handles next steps"); + }); + + it("keeps the strict no-mutation rule with no selection", () => { + expect(buildPlanModePrompt([])).toContain( + "Do not edit files, write files, or execute the plan.", + ); + }); + + it("names the enabled mutation tools when selected", () => { + expect(buildPlanModePrompt(["edit"])).toContain( + "Enabled mutation tools: edit.", + ); + expect(buildPlanModePrompt(["write"])).toContain( + "Enabled mutation tools: write.", + ); + expect(buildPlanModePrompt(["edit", "write"])).toContain( + "Enabled mutation tools: edit, write.", + ); + }); + + for (const selected of [[], ["edit"], ["write"], ["edit", "write"]] as const) { + it(`reaffirms unselected mutation tools remain blocked for ${JSON.stringify(selected)}`, () => { + expect(buildPlanModePrompt(selected)).toContain( + "Unselected mutation tools remain blocked. Do not execute the plan.", + ); + }); + } + + it("keeps bash restrictions regardless of selection", () => { + for (const selected of [[], ["edit"], ["write"], ["edit", "write"]] as const) { + expect(buildPlanModePrompt(selected)).toContain("Bash is restricted to read-only commands."); + } + }); + + it("warns about skills blocked by unavailable or mutating tools in every state", () => { + for (const selected of [[], ["edit"], ["write"], ["edit", "write"]] as const) { + expect(buildPlanModePrompt(selected)).toContain( + "Skills requiring unavailable tools or mutating bash commands will be blocked.", + ); + } + }); + + it("reminds to exit for any change request with no selection", () => { + expect(buildPlanModePrompt([])).toContain( + 'If the user asks you to make changes or implement something, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.', + ); + }); + + it("narrows the exit reminder to the proposed plan when tools are selected", () => { + expect(buildPlanModePrompt(["edit"])).toContain( + 'If the user asks you to implement the proposed plan, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.', + ); + expect(buildPlanModePrompt(["edit"])).not.toContain( + "If the user asks you to make changes or implement something", + ); }); }); From 1ebf82a8845f145380e584a3afee8fc6c633bc55 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 17:29:25 -0400 Subject: [PATCH 4/7] runtime: Ctrl+Alt+P shortcut + selection-aware tool_call auth + helpers + integration tests --- src/index.ts | 53 ++++-- tests/helpers.ts | 11 ++ tests/index.test.ts | 435 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 481 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 9039297..1fdb337 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,6 +3,7 @@ import type { ExtensionCommandContext, ExtensionContext, } from "@earendil-works/pi-coding-agent"; +import { Key } from "@earendil-works/pi-tui"; import { captureProposedPlan, filterLegacyProposedPlanMessages, @@ -12,6 +13,7 @@ import { createSavePlanSession } from "./core/save-plan.ts"; import { isSafeCommand } from "./core/safety.ts"; import { createInitialState, enterPlanMode, exitPlanMode, restoreState } from "./core/state.ts"; import { + isPlanMutationToolName, normalModeToolNames, planModeToolNames, readSelectedToolNames, @@ -191,7 +193,7 @@ export default function createExtension(pi: ExtensionAPI): void { const msg = count === 0 ? "Plan-mode tools reset to defaults." - : `Plan-mode tools updated: ${count} extension tool(s) enabled.`; + : `Plan-mode tools updated: ${count} optional tool(s) enabled.`; ctx.ui.notify(msg, "info"); } @@ -252,7 +254,7 @@ export default function createExtension(pi: ExtensionAPI): void { if (command) { if (!state.enabled) { if (requestModeTransition({ enabled: true, prompt: command }, ctx)) { - ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + ctx.ui.notify("Plan mode enabled.", "info"); } return; } @@ -262,7 +264,7 @@ export default function createExtension(pi: ExtensionAPI): void { if (!state.enabled || pendingModeTransition?.enabled === false) { if (requestModeTransition({ enabled: true }, ctx)) { - ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + ctx.ui.notify("Plan mode enabled.", "info"); } return; } @@ -292,32 +294,47 @@ export default function createExtension(pi: ExtensionAPI): void { } if (!state.enabled) { doEnter(ctx); - ctx.ui.notify("Plan mode enabled. Write tools disabled.", "info"); + ctx.ui.notify("Plan mode enabled.", "info"); } await runToolSelector(ctx); }, }); + pi.registerShortcut(Key.ctrlAlt("p"), { + description: "Toggle Plan mode", + handler: async (ctx) => { + clearPendingMenu(); + const current = pendingModeTransition?.enabled ?? state.enabled; + const enabled = !current; + if (requestModeTransition({ enabled }, ctx)) { + ctx.ui.notify(enabled ? "Plan mode enabled." : "Plan mode disabled.", "info"); + } + }, + }); + pi.on("tool_call", async (event, _ctx) => { if (!state.enabled) return; - if (event.toolName === "write") { - if (savePlanSession === undefined) { + if (savePlanSession !== undefined) { + if (event.toolName === "write") { + const result = savePlanSession.authorizeToolCall(event); + pi.setActiveTools(savePlanSession.toolNames()); + return result; + } + if (event.toolName === "edit") { return { block: true, - reason: "Plan mode blocks 'write'. Exit plan mode first with /plan:exit.", + reason: "Save plan does not allow 'edit'. Use only the approved write call.", + }; + } + } else if (isPlanMutationToolName(event.toolName)) { + const selected = state.selectedToolNames ?? []; + if (!selected.includes(event.toolName)) { + return { + block: true, + reason: `Plan mode blocks '${event.toolName}'. Enable it with /plan:tools, or exit plan mode first with /plan:exit.`, }; } - const result = savePlanSession.authorizeToolCall(event); - pi.setActiveTools(savePlanSession.toolNames()); - return result; - } - - if (event.toolName === "edit") { - return { - block: true, - reason: `Plan mode blocks '${event.toolName}'. Exit plan mode first with /plan:exit.`, - }; } if (event.toolName === "bash") { @@ -354,7 +371,7 @@ export default function createExtension(pi: ExtensionAPI): void { pi.setActiveTools(planModeToolNames(state.selectedToolNames)); return { - systemPrompt: `${event.systemPrompt}\n\n${buildPlanModePrompt()}`, + systemPrompt: `${event.systemPrompt}\n\n${buildPlanModePrompt(state.selectedToolNames)}`, }; }); diff --git a/tests/helpers.ts b/tests/helpers.ts index 64ffa4f..4fa6000 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -18,6 +18,11 @@ interface RegisteredCommand { getArgumentCompletions?: (prefix: string) => unknown; } +interface RegisteredShortcut { + description?: string; + handler: (ctx: ExtensionContext) => Promise | void; +} + type EventHandler = (event: unknown, ctx: ExtensionContext) => Promise | unknown; export interface ToolInfoLike { @@ -31,6 +36,7 @@ export interface MockPi { flags: Map; flagValues: Map; commands: Map; + shortcuts: Map; events: Map; activeTools: string[]; allTools: ToolInfoLike[]; @@ -59,6 +65,7 @@ export function createMockPi(options?: { const flags = new Map(); const flagValues = new Map(); const commands = new Map(); + const shortcuts = new Map(); const events = new Map(); let activeTools = options?.activeTools ?? ["read", "bash", "edit", "write"]; const allTools: ToolInfoLike[] = options?.allTools ?? []; @@ -77,6 +84,9 @@ export function createMockPi(options?: { registerCommand(name: string, opts: RegisteredCommand) { commands.set(name, opts); }, + registerShortcut(shortcut: string, opts: RegisteredShortcut) { + shortcuts.set(shortcut, opts); + }, on(event: string, handler: EventHandler) { const handlers = events.get(event) ?? []; handlers.push(handler); @@ -108,6 +118,7 @@ export function createMockPi(options?: { flags, flagValues, commands, + shortcuts, events, activeTools, allTools, diff --git a/tests/index.test.ts b/tests/index.test.ts index 7bb9e88..e800b49 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -1881,3 +1881,438 @@ describe("plan save lifecycle", () => { expect(ctx.statuses.get("pi-plan")).toBeUndefined(); }); }); + +describe("Ctrl+Alt+P shortcut", () => { + it("registers the conflict-free Plan-mode shortcut", () => { + const mock = createMockPi(); + createExtension(mock.pi); + expect(mock.shortcuts.has("ctrl+alt+p")).toBe(true); + }); + + it("toggles plan mode on and off while idle", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + + const handler = mock.shortcuts.get("ctrl+alt+p")!.handler; + await handler(ctx.ctx); + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + expect( + ctx.notifications.some((n) => n.type === "info" && n.message.includes("Plan mode enabled")), + ).toBe(true); + + await handler(ctx.ctx); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect( + ctx.notifications.some((n) => n.type === "info" && n.message.includes("Plan mode disabled")), + ).toBe(true); + }); + + it("queues a busy toggle and applies it once the agent settles", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + const handler = mock.shortcuts.get("ctrl+alt+p")!.handler; + await handler(ctx.ctx); + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect( + ctx.notifications.some((n) => n.message.includes("queued")), + ).toBe(true); + + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(ctx.statuses.get("pi-plan")).toBe("plan active"); + }); + + it("lets the latest busy toggle cancel a queued toggle", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ isIdle: false }); + + const handler = mock.shortcuts.get("ctrl+alt+p")!.handler; + await handler(ctx.ctx); // queued enable + await handler(ctx.ctx); // queued disable (cancels first) + ctx.setIdle(true); + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(ctx.statuses.get("pi-plan")).toBeUndefined(); + expect(mock.activeTools).toContain("write"); + }); + + it("does not register Shift-Tab as a Plan-mode shortcut", () => { + const mock = createMockPi(); + createExtension(mock.pi); + expect(mock.shortcuts.has("shift+tab")).toBe(false); + }); +}); + +describe("mutation-tool permission states", () => { + function expectBlock(result: unknown) { + expect(result).toEqual(expect.objectContaining({ block: true })); + } + + function expectAllowed(result: unknown) { + expect(result).toBeUndefined(); + } + + it("blocks both edit and write by default", async () => { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expectBlock( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "edit-default", toolName: "edit", input: {} }, + ctx, + ), + ); + expectBlock( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "write-default", toolName: "write", input: {} }, + ctx, + ), + ); + }); + + it("permits edit but blocks write when only edit is selected", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true }), + ); + + try { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(mock.activeTools).toContain("edit"); + expect(mock.activeTools).not.toContain("write"); + + expectAllowed( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "edit-allowed", toolName: "edit", input: {} }, + ctx, + ), + ); + expectBlock( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "write-still-blocked", toolName: "write", input: {} }, + ctx, + ), + ); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + } + }); + + it("permits write but blocks edit when only write is selected", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, write: true }), + ); + + try { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(mock.activeTools).toContain("write"); + expect(mock.activeTools).not.toContain("edit"); + + expectAllowed( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "write-allowed", toolName: "write", input: {} }, + ctx, + ), + ); + expectBlock( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "edit-still-blocked", toolName: "edit", input: {} }, + ctx, + ), + ); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + } + }); + + it("permits both edit and write when both are selected", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true, write: true }), + ); + + try { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(mock.activeTools).toContain("edit"); + expect(mock.activeTools).toContain("write"); + + expectAllowed( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "edit-both", toolName: "edit", input: {} }, + ctx, + ), + ); + expectAllowed( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "write-both", toolName: "write", input: {} }, + ctx, + ), + ); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + } + }); + + it("injects selection-aware prompt rules in before_agent_start", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true, write: true }), + ); + + try { + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + const result = await mock.fireEvent( + "before_agent_start", + { type: "before_agent_start", systemPrompt: "base" }, + ctx, + ); + + const { systemPrompt } = result as { systemPrompt: string }; + expect(systemPrompt).toContain("Enabled mutation tools: edit, write."); + expect(systemPrompt).toContain("Unselected mutation tools remain blocked."); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + } + }); +}); + +describe("persisted mutation-tool selections", () => { + const tempDirs: string[] = []; + afterEach(() => { + for (const path of tempDirs.splice(0)) rmSync(path, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + }); + + it("round-trips edit and write through the persisted JSON file", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-persist-")); + tempDirs.push(tempDir); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + + const mock = createMockPi({ + allTools: [ + { name: "read", description: "Read", sourceInfo: { source: "builtin" } }, + { name: "bash", description: "Bash", sourceInfo: { source: "builtin" } }, + { name: "edit", description: "Edit", sourceInfo: { source: "builtin" } }, + { name: "write", description: "Write", sourceInfo: { source: "builtin" } }, + { name: "my-tool", description: "My tool", sourceInfo: { source: "my-ext" } }, + ], + }); + createExtension(mock.pi); + + const ctx = createMockContext({ customResult: ["edit", "write", "my-tool"] }); + await mock.commands.get("plan")!.handler("", ctx.ctx); + await mock.commands.get("plan:tools")!.handler("", ctx.ctx); + await new Promise((resolve) => setTimeout(resolve, 50)); + + const written = JSON.parse( + readFileSync(join(tempDir, "extensions", "plan-tools.json"), "utf-8"), + ); + expect(written).toEqual({ + read: true, + bash: true, + edit: true, + write: true, + "my-tool": true, + }); + }); + + it("restores persisted edit and write selections on session_start", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-persist-")); + tempDirs.push(tempDir); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true, write: true }), + ); + + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + expect(mock.activeTools).toContain("edit"); + expect(mock.activeTools).toContain("write"); + + const persisted = mock.entries.filter((entry) => entry.customType === "plan-mode-state"); + expect(persisted.at(-1)?.data).toMatchObject({ + selectedToolNames: expect.arrayContaining(["edit", "write"]), + }); + }); + + it("overrides session-only selections when persisted JSON is present", async () => { + const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-persist-")); + tempDirs.push(tempDir); + process.env.PI_CODING_AGENT_DIR = tempDir; + mkdirSync(join(tempDir, "extensions"), { recursive: true }); + writeFileSync( + join(tempDir, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true }), + ); + + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + entries: [ + { + type: "custom", + customType: "plan-mode-state", + data: { enabled: true, selectedToolNames: ["session-only"] }, + id: "1", + parentId: null, + timestamp: new Date().toISOString(), + }, + ], + }); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + + expect(mock.activeTools).toContain("edit"); + expect(mock.activeTools).not.toContain("session-only"); + }); +}); + +describe("Save-plan regression with selected mutation tools", () => { + const tempDirs: string[] = []; + afterEach(() => { + for (const path of tempDirs.splice(0)) rmSync(path, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + }); + + it("still blocks edit during a save turn even when persisted", async () => { + const workspace = mkdtempSync(join(tmpdir(), "pi-plan-save-")); + tempDirs.push(workspace); + mkdirSync(join(workspace, "docs"), { recursive: true }); + process.env.PI_CODING_AGENT_DIR = workspace; + mkdirSync(join(workspace, "extensions"), { recursive: true }); + writeFileSync( + join(workspace, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true, write: true }), + ); + + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + cwd: workspace, + selectResponses: [PLAN_MENU_LABELS.save], + }); + + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + expect(mock.activeTools).toContain("edit"); + + await mock.fireEvent( + "agent_end", + { + type: "agent_end", + messages: [assistantText("\n# Save Me\n")], + }, + ctx, + ); + await mock.commands.get("plan")!.handler("", ctx.ctx); + + // Save turn narrows active tools to safe built-ins + write + expect(mock.activeTools).toEqual( + expect.arrayContaining(["read", "bash", "grep", "find", "ls", "write"]), + ); + expect(mock.activeTools).not.toContain("edit"); + + // edit is still blocked by the save-session policy + expect( + await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: "edit-during-save", toolName: "edit", input: {} }, + ctx, + ), + ).toEqual(expect.objectContaining({ block: true })); + }); + + it("restores the user's selected ordinary tools after a save turn settles", async () => { + const workspace = mkdtempSync(join(tmpdir(), "pi-plan-save-")); + tempDirs.push(workspace); + mkdirSync(join(workspace, "docs"), { recursive: true }); + process.env.PI_CODING_AGENT_DIR = workspace; + mkdirSync(join(workspace, "extensions"), { recursive: true }); + writeFileSync( + join(workspace, "extensions", "plan-tools.json"), + JSON.stringify({ read: true, bash: true, edit: true, write: true }), + ); + + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext({ + cwd: workspace, + selectResponses: [PLAN_MENU_LABELS.save], + }); + + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + 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); + + await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); + + expect(mock.activeTools).toContain("edit"); + expect(mock.activeTools).toContain("write"); + }); +}); From 85ebaaf74a7e3d2fe6a66ad8d48fcdd31041be82 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 17:30:08 -0400 Subject: [PATCH 5/7] docs: update README + CHANGELOG --- CHANGELOG.md | 6 ++++++ README.md | 20 +++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59ca6da..ffcba73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,17 @@ All notable changes to `@pi-vault/pi-plan` are documented in this file. ## [Unreleased] +### Added + +- Register a conflict-free `Ctrl+Alt+P` shortcut to toggle Plan mode (idle or queued when busy). +- Let users independently opt `edit` and `write` into Plan mode through the existing persisted optional tool selector. + ### 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. +- Plan-mode prompt rules and runtime authorization are now driven by the user's selected optional tools so the prompt no longer contradicts explicit opt-in behavior. ## [0.4.0] - 2026-07-26 diff --git a/README.md b/README.md index c7bb58f..99e2400 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ ## Description -`@pi-vault/pi-plan` adds a read-only planning workflow to Pi. Use it to inspect the repo, clarify intent, and return a decision-complete implementation plan before any write-capable work starts. +`@pi-vault/pi-plan` adds a read-only planning workflow to Pi. Use it to inspect the repo, clarify intent, and return a decision-complete implementation plan before any write-capable work starts. Plan mode is read-only by default; built-in `edit` and `write` are independently disabled until you opt them in. ## Screenshots @@ -51,6 +51,14 @@ Start Pi directly in Plan mode: pi --plan ``` +Toggle Plan mode with the keyboard: + +```text +Ctrl+Alt+P +``` + +`Ctrl+Alt+P` works while the agent is idle and queues a pending toggle when Pi is busy. `Shift+Tab` remains Pi's reserved thinking-level shortcut. + ### Work through a plan 1. Let the agent inspect the repo and ask clarifying questions. @@ -90,20 +98,22 @@ If Pi is busy, mode-changing commands and actions from the `/plan` menu wait unt ## Configure Optional Tools -Safe built-in planning tools are available by default. Use `/plan:tools` to enable additional optional extension tools during planning. +Safe built-in planning tools are available by default. Use `/plan:tools` to enable additional optional tools during planning. -Your selections persist across Pi sessions. Built-in `edit` and `write` remain blocked in Plan mode even when extra tools are enabled. Non-built-in tools may still expose broader capabilities through their own interfaces, so enable them deliberately. +Built-in `edit` and `write` are disabled by default. Use `/plan:tools` to opt `edit`, `write`, or both into the active Plan-mode tool set independently. Your selections persist across Pi sessions. When a mutation tool is selected, Plan mode limits it to file changes the user explicitly requests and keeps the unselected mutation tools blocked. Non-built-in tools may still expose broader capabilities through their own interfaces, so enable them deliberately. ## Safety Boundaries Plan mode keeps the default workflow read-only: -- built-in `edit` and `write` are blocked +- built-in `edit` and `write` are blocked by default - `bash` is limited to allowlisted read-only commands - mutating shell commands are blocked with a Plan-mode error - safe built-in planning tools remain available: `read`, `bash`, `grep`, `find`, and `ls` -The only exception is **Save plan**. During a save turn, Pi narrows active tools back to the safe built-ins and temporarily adds built-in `write` for the exact captured plan only. After a successful save, `write` is disabled again. +When you opt `edit` and/or `write` into the Plan-mode tool set through `/plan:tools`, the selected mutation tools are authorized only for file changes the user explicitly requests during the current Plan-mode session. The unselected mutation tools stay blocked. Both selections persist across sessions and can be reverted through `/plan:tools`. + +The only exception is **Save plan**. During a save turn, Pi narrows active tools back to the safe built-ins and temporarily adds built-in `write` for the exact captured plan only, regardless of any persisted `write` selection. After a successful save, `write` is disabled again and the persisted ordinary-mode selections are restored. Save-path preflight rejects: From 7efe4447b93c1c8a50c2095cf1430679cb5d9072 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 18:37:08 -0400 Subject: [PATCH 6/7] refactor: simplify plan-mode mutation handling --- src/core/prompt.ts | 37 +++---- src/core/tools.ts | 11 +- src/index.ts | 25 +++-- tests/core/prompt.test.ts | 27 ++--- tests/core/tools.test.ts | 19 ---- tests/index.test.ts | 221 ++++++++++---------------------------- 6 files changed, 95 insertions(+), 245 deletions(-) diff --git a/src/core/prompt.ts b/src/core/prompt.ts index 7414bcc..af67235 100644 --- a/src/core/prompt.ts +++ b/src/core/prompt.ts @@ -1,4 +1,4 @@ -import { selectedPlanMutationToolNames } from "./tools.ts"; +import { PLAN_MUTATION_TOOL_NAMES } from "./tools.ts"; const PHASES_AND_TEMPLATE = `## Phase 1 -- Explore @@ -26,31 +26,16 @@ const PHASES_AND_TEMPLATE = `## Phase 1 -- Explore - The plan must be decision-complete: no open questions for the implementer. - Do not ask "should I proceed?" -- the Plan Mode menu handles next steps.`; -const STATIC_RULES = `- Stay in Plan Mode until the user explicitly exits or chooses to implement. -- Bash is restricted to read-only commands. -- Skills requiring unavailable tools or mutating bash commands will be blocked.`; - -const NO_SELECTION_MUTATION_RULE = - "- Do not edit files, write files, or execute the plan.\n- Unselected mutation tools remain blocked. Do not execute the plan."; - -const NO_SELECTION_CHANGE_RULE = - '- If the user asks you to make changes or implement something, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; - -function buildSelectedMutationRules(enabled: string[]): string { - return [ - `- Enabled mutation tools: ${enabled.join(", ")}. Use them only for file changes the user explicitly requests.`, - "- Unselected mutation tools remain blocked. Do not execute the plan.", - ].join("\n"); -} - -const SELECTED_CHANGE_RULE = - '- If the user asks you to implement the proposed plan, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; - export function buildPlanModePrompt(selectedToolNames: readonly string[] = []): string { - const enabled = selectedPlanMutationToolNames(selectedToolNames); + const enabled = PLAN_MUTATION_TOOL_NAMES.filter((name) => selectedToolNames.includes(name)); const mutationRule = - enabled.length === 0 ? NO_SELECTION_MUTATION_RULE : buildSelectedMutationRules(enabled); - const changeRule = enabled.length === 0 ? NO_SELECTION_CHANGE_RULE : SELECTED_CHANGE_RULE; + enabled.length === 0 + ? "- Do not edit files, write files, or execute the plan.\n- Unselected mutation tools remain blocked. Do not execute the plan." + : `- Enabled mutation tools: ${enabled.join(", ")}. Use them only for file changes the user explicitly requests.\n- Unselected mutation tools remain blocked. Do not execute the plan.`; + const changeRule = + enabled.length === 0 + ? '- If the user asks you to make changes or implement something, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.' + : '- If the user asks you to implement the proposed plan, remind them to exit Plan Mode first by running /plan and choosing "Implement this plan", or by running /plan exit.'; return `[PLAN MODE ACTIVE] # Plan Mode (Conversational) @@ -60,7 +45,9 @@ before any code mutation happens. ## Mode rules -${STATIC_RULES} +- Stay in Plan Mode until the user explicitly exits or chooses to implement. +- Bash is restricted to read-only commands. +- Skills requiring unavailable tools or mutating bash commands will be blocked. ${mutationRule} ${changeRule} diff --git a/src/core/tools.ts b/src/core/tools.ts index 81f3ba6..ea0a262 100644 --- a/src/core/tools.ts +++ b/src/core/tools.ts @@ -9,18 +9,11 @@ import { const SAFE_PLAN_TOOL_NAMES = ["read", "bash", "grep", "find", "ls"]; const SAFE_PLAN_TOOL_NAME_SET = new Set(SAFE_PLAN_TOOL_NAMES); export const PLAN_MUTATION_TOOL_NAMES = ["edit", "write"] as const; -const PLAN_MUTATION_TOOL_NAME_SET = new Set(PLAN_MUTATION_TOOL_NAMES); const NORMAL_MODE_TOOL_NAMES = ["read", "bash", "edit", "write"]; const CONFIG_FILENAME = "extensions/plan-tools.json"; export function isPlanMutationToolName(name: string): boolean { - return PLAN_MUTATION_TOOL_NAME_SET.has(name); -} - -export function selectedPlanMutationToolNames( - selected: readonly string[] = [], -): string[] { - return PLAN_MUTATION_TOOL_NAMES.filter((name) => selected.includes(name)); + return PLAN_MUTATION_TOOL_NAMES.some((mutationToolName) => mutationToolName === name); } export type PlanToolInfo = Pick & { @@ -46,7 +39,7 @@ export function getToolPolicy(tool: PlanToolInfo): { label: tool.name === "bash" ? "built-in limited" : "built-in", }; } - if (PLAN_MUTATION_TOOL_NAME_SET.has(tool.name)) { + if (isPlanMutationToolName(tool.name)) { return { alwaysOn: false, toggleable: true, diff --git a/src/index.ts b/src/index.ts index 1fdb337..4670950 100644 --- a/src/index.ts +++ b/src/index.ts @@ -34,6 +34,7 @@ interface PendingModeTransition { enabled: boolean; prompt?: string; consumePlan?: boolean; + applyOnSettled?: boolean; } export default function createExtension(pi: ExtensionAPI): void { @@ -44,7 +45,7 @@ export default function createExtension(pi: ExtensionAPI): void { let pendingModeTransition: PendingModeTransition | undefined; pi.registerFlag("plan", { - description: "Start in plan mode (read-only exploration)", + description: "Start in plan mode (read-only by default)", type: "boolean", default: false, }); @@ -145,17 +146,19 @@ export default function createExtension(pi: ExtensionAPI): void { } const waitForIdle = (ctx as Partial).waitForIdle; - if (typeof waitForIdle !== "function") { + if (typeof waitForIdle !== "function" && !transition.applyOnSettled) { 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); - }); + if (typeof waitForIdle === "function") { + 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; } @@ -306,7 +309,7 @@ export default function createExtension(pi: ExtensionAPI): void { clearPendingMenu(); const current = pendingModeTransition?.enabled ?? state.enabled; const enabled = !current; - if (requestModeTransition({ enabled }, ctx)) { + if (requestModeTransition({ enabled, applyOnSettled: true }, ctx)) { ctx.ui.notify(enabled ? "Plan mode enabled." : "Plan mode disabled.", "info"); } }, @@ -404,6 +407,12 @@ export default function createExtension(pi: ExtensionAPI): void { ctx.ui.notify("Save plan failed or was not completed.", "warning"); } } + + const transition = pendingModeTransition?.applyOnSettled ? pendingModeTransition : undefined; + if (transition !== undefined) { + pendingModeTransition = undefined; + applyModeTransition(transition, ctx); + } }); pi.on("context", async (event) => { diff --git a/tests/core/prompt.test.ts b/tests/core/prompt.test.ts index 0a7f8cd..be57185 100644 --- a/tests/core/prompt.test.ts +++ b/tests/core/prompt.test.ts @@ -6,25 +6,15 @@ describe("buildPlanModePrompt", () => { expect(buildPlanModePrompt()).toContain("[PLAN MODE ACTIVE]"); }); - it.each([ - ["no selection", []], - ["edit only", ["edit"]], - ["write only", ["write"]], - ["edit and write", ["edit", "write"]], - ] as const)("contains the three planning phases for %s", (_label, selected) => { - const prompt = buildPlanModePrompt(selected); + it("contains the three planning phases", () => { + const prompt = buildPlanModePrompt(); expect(prompt).toContain("Phase 1 -- Explore"); expect(prompt).toContain("Phase 2 -- Clarify"); expect(prompt).toContain("Phase 3 -- Plan"); }); - it.each([ - ["no selection", []], - ["edit only", ["edit"]], - ["write only", ["write"]], - ["edit and write", ["edit", "write"]], - ] as const)("contains the proposed_plan template for %s", (_label, selected) => { - const prompt = buildPlanModePrompt(selected); + it("contains the proposed_plan template", () => { + const prompt = buildPlanModePrompt(); expect(prompt).toContain(""); expect(prompt).toContain(""); expect(prompt).toContain("## Summary"); @@ -33,13 +23,8 @@ describe("buildPlanModePrompt", () => { expect(prompt).toContain("## Assumptions"); }); - it.each([ - ["no selection", []], - ["edit only", ["edit"]], - ["write only", ["write"]], - ["edit and write", ["edit", "write"]], - ] as const)("contains the phase-3 'do not ask' guidance for %s", (_label, selected) => { - const prompt = buildPlanModePrompt(selected); + it("contains the phase-3 'do not ask' guidance", () => { + const prompt = buildPlanModePrompt(); expect(prompt).toContain("Do not ask"); expect(prompt).toContain("menu handles next steps"); }); diff --git a/tests/core/tools.test.ts b/tests/core/tools.test.ts index 6376357..e3bb4ad 100644 --- a/tests/core/tools.test.ts +++ b/tests/core/tools.test.ts @@ -13,7 +13,6 @@ import { safeGetActiveTools, safeGetAllTools, savePlanToolNames, - selectedPlanMutationToolNames, writeSelectedToolNames, } from "../../src/core/tools.ts"; @@ -35,24 +34,6 @@ describe("isPlanMutationToolName", () => { }); }); -describe("selectedPlanMutationToolNames", () => { - it("returns an empty array for an empty or missing selection", () => { - expect(selectedPlanMutationToolNames([])).toEqual([]); - expect(selectedPlanMutationToolNames()).toEqual([]); - }); - - it("returns only the selected mutation tools in canonical order", () => { - expect(selectedPlanMutationToolNames(["write", "custom", "edit"])).toEqual([ - "edit", - "write", - ]); - }); - - it("ignores non-mutation tools in the selection", () => { - expect(selectedPlanMutationToolNames(["bash", "read", "custom"])).toEqual([]); - }); -}); - describe("getToolPolicy", () => { it("returns the fixed policy for every built-in row", () => { for (const name of ["read", "grep", "find", "ls"]) { diff --git a/tests/index.test.ts b/tests/index.test.ts index e800b49..7aa1085 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -8,7 +8,11 @@ import { } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import type { AgentEndEvent } from "@earendil-works/pi-coding-agent"; +import type { + AgentEndEvent, + ExtensionCommandContext, + ExtensionContext, +} from "@earendil-works/pi-coding-agent"; import { afterEach, describe, expect, it, vi } from "vitest"; import createExtension from "../src/index.ts"; import { PLAN_MENU_LABELS } from "../src/tui/menus.ts"; @@ -22,12 +26,20 @@ function assistantText( return { role: "assistant", content: [{ type: "text", text }] }; } +function shortcutContext(ctx: ExtensionCommandContext): ExtensionContext { + const { waitForIdle: _waitForIdle, ...baseContext } = ctx; + return baseContext; +} + describe("createExtension", () => { it("registers the plan flag", () => { const mock = createMockPi(); createExtension(mock.pi); expect(mock.flags.has("plan")).toBe(true); expect(mock.flags.get("plan")?.type).toBe("boolean"); + expect(mock.flags.get("plan")?.description).toBe( + "Start in plan mode (read-only by default)", + ); }); it("registers plan, plan:exit, and plan:tools commands", () => { @@ -1914,7 +1926,7 @@ describe("Ctrl+Alt+P shortcut", () => { const ctx = createMockContext({ isIdle: false }); const handler = mock.shortcuts.get("ctrl+alt+p")!.handler; - await handler(ctx.ctx); + await handler(shortcutContext(ctx.ctx)); expect(ctx.statuses.get("pi-plan")).toBeUndefined(); expect( ctx.notifications.some((n) => n.message.includes("queued")), @@ -1932,8 +1944,9 @@ describe("Ctrl+Alt+P shortcut", () => { const ctx = createMockContext({ isIdle: false }); const handler = mock.shortcuts.get("ctrl+alt+p")!.handler; - await handler(ctx.ctx); // queued enable - await handler(ctx.ctx); // queued disable (cancels first) + const shortcutCtx = shortcutContext(ctx.ctx); + await handler(shortcutCtx); // queued enable + await handler(shortcutCtx); // queued disable (cancels first) ctx.setIdle(true); await mock.fireEvent("agent_settled", { type: "agent_settled" }, ctx); @@ -1949,182 +1962,64 @@ describe("Ctrl+Alt+P shortcut", () => { }); describe("mutation-tool permission states", () => { - function expectBlock(result: unknown) { - expect(result).toEqual(expect.objectContaining({ block: true })); - } - - function expectAllowed(result: unknown) { - expect(result).toBeUndefined(); - } - - it("blocks both edit and write by default", async () => { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - expectBlock( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "edit-default", toolName: "edit", input: {} }, - ctx, - ), - ); - expectBlock( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "write-default", toolName: "write", input: {} }, - ctx, - ), - ); - }); - - it("permits edit but blocks write when only edit is selected", async () => { - const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); - process.env.PI_CODING_AGENT_DIR = tempDir; - mkdirSync(join(tempDir, "extensions"), { recursive: true }); - writeFileSync( - join(tempDir, "extensions", "plan-tools.json"), - JSON.stringify({ read: true, bash: true, edit: true }), - ); - - try { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - expect(mock.activeTools).toContain("edit"); - expect(mock.activeTools).not.toContain("write"); - - expectAllowed( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "edit-allowed", toolName: "edit", input: {} }, - ctx, - ), - ); - expectBlock( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "write-still-blocked", toolName: "write", input: {} }, - ctx, - ), - ); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - delete process.env.PI_CODING_AGENT_DIR; - } - }); - - it("permits write but blocks edit when only write is selected", async () => { - const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); - process.env.PI_CODING_AGENT_DIR = tempDir; - mkdirSync(join(tempDir, "extensions"), { recursive: true }); - writeFileSync( - join(tempDir, "extensions", "plan-tools.json"), - JSON.stringify({ read: true, bash: true, write: true }), - ); - - try { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - expect(mock.activeTools).toContain("write"); - expect(mock.activeTools).not.toContain("edit"); - - expectAllowed( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "write-allowed", toolName: "write", input: {} }, - ctx, - ), - ); - expectBlock( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "edit-still-blocked", toolName: "edit", input: {} }, - ctx, - ), - ); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - delete process.env.PI_CODING_AGENT_DIR; - } + const tempDirs: string[] = []; + afterEach(() => { + for (const path of tempDirs.splice(0)) rmSync(path, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; }); - it("permits both edit and write when both are selected", async () => { + async function startWith(selected: readonly ("edit" | "write")[]) { const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); + tempDirs.push(tempDir); process.env.PI_CODING_AGENT_DIR = tempDir; mkdirSync(join(tempDir, "extensions"), { recursive: true }); writeFileSync( join(tempDir, "extensions", "plan-tools.json"), - JSON.stringify({ read: true, bash: true, edit: true, write: true }), + JSON.stringify(Object.fromEntries(["read", "bash", ...selected].map((name) => [name, true]))), ); - try { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - expect(mock.activeTools).toContain("edit"); - expect(mock.activeTools).toContain("write"); + const mock = createMockPi(); + createExtension(mock.pi); + const ctx = createMockContext(); + await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); + await mock.commands.get("plan")!.handler("", ctx.ctx); + return { mock, ctx }; + } - expectAllowed( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "edit-both", toolName: "edit", input: {} }, - ctx, - ), - ); - expectAllowed( - await mock.fireEvent( - "tool_call", - { type: "tool_call", toolCallId: "write-both", toolName: "write", input: {} }, - ctx, - ), + it.each([ + ["neither", [], false, false], + ["edit only", ["edit"], true, false], + ["write only", ["write"], false, true], + ["both", ["edit", "write"], true, true], + ] as const)("enforces the %s selection", async (_label, selected, editAllowed, writeAllowed) => { + const { mock, ctx } = await startWith(selected); + + for (const [toolName, allowed] of [ + ["edit", editAllowed], + ["write", writeAllowed], + ] as const) { + expect(mock.activeTools.includes(toolName)).toBe(allowed); + const result = await mock.fireEvent( + "tool_call", + { type: "tool_call", toolCallId: `${toolName}-call`, toolName, input: {} }, + ctx, ); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - delete process.env.PI_CODING_AGENT_DIR; + if (allowed) expect(result).toBeUndefined(); + else expect(result).toEqual(expect.objectContaining({ block: true })); } }); it("injects selection-aware prompt rules in before_agent_start", async () => { - const tempDir = mkdtempSync(join(tmpdir(), "pi-plan-ctrl-")); - process.env.PI_CODING_AGENT_DIR = tempDir; - mkdirSync(join(tempDir, "extensions"), { recursive: true }); - writeFileSync( - join(tempDir, "extensions", "plan-tools.json"), - JSON.stringify({ read: true, bash: true, edit: true, write: true }), + const { mock, ctx } = await startWith(["edit", "write"]); + const result = await mock.fireEvent( + "before_agent_start", + { type: "before_agent_start", systemPrompt: "base" }, + ctx, ); - try { - const mock = createMockPi(); - createExtension(mock.pi); - const ctx = createMockContext(); - await mock.fireEvent("session_start", { type: "session_start", reason: "resume" }, ctx); - await mock.commands.get("plan")!.handler("", ctx.ctx); - - const result = await mock.fireEvent( - "before_agent_start", - { type: "before_agent_start", systemPrompt: "base" }, - ctx, - ); - - const { systemPrompt } = result as { systemPrompt: string }; - expect(systemPrompt).toContain("Enabled mutation tools: edit, write."); - expect(systemPrompt).toContain("Unselected mutation tools remain blocked."); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - delete process.env.PI_CODING_AGENT_DIR; - } + const { systemPrompt } = result as { systemPrompt: string }; + expect(systemPrompt).toContain("Enabled mutation tools: edit, write."); + expect(systemPrompt).toContain("Unselected mutation tools remain blocked."); }); }); From 9acd585893889a98b77ddf8950a5e41c9ee62c18 Mon Sep 17 00:00:00 2001 From: Lanh Hoang Date: Sun, 2 Aug 2026 18:37:31 -0400 Subject: [PATCH 7/7] docs: align plan-mode behavior guidance --- README.md | 6 ++-- ...mode-shortcut-and-mutation-tool-toggles.md | 28 ++++++------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 99e2400..1c491db 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ ## Description -`@pi-vault/pi-plan` adds a read-only planning workflow to Pi. Use it to inspect the repo, clarify intent, and return a decision-complete implementation plan before any write-capable work starts. Plan mode is read-only by default; built-in `edit` and `write` are independently disabled until you opt them in. +`@pi-vault/pi-plan` adds a planning workflow to Pi that is read-only by default. Use it to inspect the repo, clarify intent, and return a decision-complete implementation plan before implementation of the proposed plan starts. Built-in `edit` and `write` are independently disabled until you opt them in. ## Screenshots @@ -81,7 +81,7 @@ When you choose it: - Pi asks the agent to choose a new lowercase `.md` filename in the workspace root, typically dated like `YYYY-MM-DD-.md`. - For that save turn only, Pi temporarily allows built-in `write` for the exact captured plan. - If the write fails, the agent can retry during the same save turn. -- After a successful save, `write` is disabled again and Plan mode remains active. +- After a successful save, the save-only authorization ends, the user's selected Plan-mode tools are restored, and Plan mode remains active. - Save-path validation rejects existing targets, broken symlinks, path traversal, and any path that resolves outside the current workspace. ## Command Reference @@ -113,7 +113,7 @@ Plan mode keeps the default workflow read-only: When you opt `edit` and/or `write` into the Plan-mode tool set through `/plan:tools`, the selected mutation tools are authorized only for file changes the user explicitly requests during the current Plan-mode session. The unselected mutation tools stay blocked. Both selections persist across sessions and can be reverted through `/plan:tools`. -The only exception is **Save plan**. During a save turn, Pi narrows active tools back to the safe built-ins and temporarily adds built-in `write` for the exact captured plan only, regardless of any persisted `write` selection. After a successful save, `write` is disabled again and the persisted ordinary-mode selections are restored. +**Save plan** uses separate authorization. During a save turn, Pi narrows active tools back to the safe built-ins and temporarily adds built-in `write` for the exact captured plan only, regardless of any persisted `write` selection. After a successful save, the save-only authorization ends and the persisted ordinary Plan-mode selections are restored. Save-path preflight rejects: diff --git a/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md b/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md index c2b8450..9c066da 100644 --- a/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md +++ b/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md @@ -4,7 +4,7 @@ **Goal:** Add a conflict-free `Ctrl+Alt+P` Plan-mode shortcut and let users independently opt `edit` and `write` into Plan mode through the existing persisted tool selector. -**Architecture:** Reuse `selectedToolNames` as the single source of truth. A canonical mutation-tool tuple and helpers in `src/core/tools.ts` are consumed by selector policy, prompt generation, and runtime authorization. No new command, state field, dependency, or configuration format is introduced. Save-plan sessions retain their exact-content, write-only authorization and take precedence over ordinary mutation-tool selection. +**Architecture:** Reuse `selectedToolNames` as the single source of truth. A canonical mutation-tool tuple and name guard in `src/core/tools.ts` are consumed by selector policy, prompt generation, and runtime authorization. No new command, state field, dependency, or configuration format is introduced. Save-plan sessions retain their exact-content, write-only authorization and take precedence over ordinary mutation-tool selection. **Tech Stack:** TypeScript, Pi Extension API, `@earendil-works/pi-tui`, Vitest, Biome, pnpm. @@ -24,17 +24,12 @@ **Files:** `tests/core/tools.test.ts`, `tests/core/prompt.test.ts`, `tests/tui/tool-selector.test.ts` -- [ ] **Step 1: Specify the canonical mutation helpers in tests.** Add expectations for the planned exports `isPlanMutationToolName` and `selectedPlanMutationToolNames`: +- [ ] **Step 1: Specify the canonical mutation guard in tests.** Add expectations for the planned export `isPlanMutationToolName`: ```ts expect(isPlanMutationToolName("edit")).toBe(true); expect(isPlanMutationToolName("write")).toBe(true); expect(isPlanMutationToolName("bash")).toBe(false); -expect(selectedPlanMutationToolNames([])).toEqual([]); -expect(selectedPlanMutationToolNames(["write", "custom", "edit"])).toEqual([ - "edit", - "write", -]); ``` - [ ] **Step 2: Replace blocked-policy expectations.** In `tests/core/tools.test.ts`, assert that both built-in `edit` and `write` return exactly `{ alwaysOn: false, toggleable: true, label: "user risk: built-in mutation" }`. Keep safe built-ins always-on and extension tools user-risk optional. Assert `planModeToolNames(["edit", "write"])` returns `read`, `bash`, `grep`, `find`, `ls`, `edit`, `write` in that order. @@ -82,24 +77,17 @@ Expected: the new assertions fail against the current blocked-tool and static-pr ```ts export const PLAN_MUTATION_TOOL_NAMES = ["edit", "write"] as const; -const PLAN_MUTATION_TOOL_NAME_SET = new Set(PLAN_MUTATION_TOOL_NAMES); export function isPlanMutationToolName(name: string): boolean { - return PLAN_MUTATION_TOOL_NAME_SET.has(name); -} - -export function selectedPlanMutationToolNames( - selected: readonly string[] = [], -): string[] { - return PLAN_MUTATION_TOOL_NAMES.filter((name) => selected.includes(name)); + return PLAN_MUTATION_TOOL_NAMES.some((mutationToolName) => mutationToolName === name); } ``` -Replace `BLOCKED_TOOL_NAMES` with this shared set. `getToolPolicy` must return the user-risk mutation policy for both names. Do not add either mutation tool to `SAFE_PLAN_TOOL_NAMES`; `planModeToolNames` must include them only when selected. Existing JSON persistence should then store and restore `edit: true` and `write: true` without schema changes. +Replace `BLOCKED_TOOL_NAMES` with this tuple and use the guard from `getToolPolicy`. `getToolPolicy` must return the user-risk mutation policy for both names. Do not add either mutation tool to `SAFE_PLAN_TOOL_NAMES`; `planModeToolNames` must include them only when selected. Existing JSON persistence should then store and restore `edit: true` and `write: true` without schema changes. - [ ] **Step 2: Update selector copy without changing interaction mechanics.** Change the selector subtitle to `Optional tools run at user risk.`. Keep Space gated by `getToolPolicy(tool).toggleable` and keep safe-tool filtering on Enter; selected `edit`/`write` names must be returned as optional selections. -- [ ] **Step 3: Make `buildPlanModePrompt` selection-aware.** Change the function signature to `buildPlanModePrompt(selectedToolNames: readonly string[] = [])` and derive enabled names with `selectedPlanMutationToolNames(selectedToolNames)`. Replace the static mutation and exit-rule text with these exact branches: +- [ ] **Step 3: Make `buildPlanModePrompt` selection-aware.** Change the function signature to `buildPlanModePrompt(selectedToolNames: readonly string[] = [])` and derive enabled names by filtering `PLAN_MUTATION_TOOL_NAMES`. Replace the static mutation and exit-rule text with these exact branches: ```ts const mutationRules = @@ -123,7 +111,7 @@ Keep the existing phases, proposed-plan template, Bash restriction, and mode-tra - [ ] **Step 1: Extend the Pi test double.** Add a registered-shortcut type whose handler accepts `ExtensionContext`, add `shortcuts: Map` to `MockPi`, and implement `registerShortcut` by storing each registration under its string key. -- [ ] **Step 2: Add failing shortcut tests.** In `tests/index.test.ts`, assert `mock.shortcuts.has("ctrl+alt+p")`, invoke its handler twice while idle, and verify status changes from `plan active` to cleared. Add a busy test that presses once, verifies the queued notification, settles the agent, and verifies the mode changes; press twice while busy and verify the second target cancels the first. +- [ ] **Step 2: Add failing shortcut tests.** In `tests/index.test.ts`, assert `mock.shortcuts.has("ctrl+alt+p")`, invoke its handler twice while idle, and verify status changes from `plan active` to cleared. Add a busy test using a production-shaped `ExtensionContext` without command-only `waitForIdle()`: press once, verify the queued notification, settle the agent, and verify the mode changes; press twice while busy and verify the second target cancels the first. - [ ] **Step 3: Add failing authorization tests.** Cover all four selection states in `tests/index.test.ts`: default `edit` and `write` calls block; edit-only permits edit but blocks write; write-only permits write but blocks edit; both permit both. Assert active tools match the same selections and `before_agent_start` includes the same prompt rules. @@ -146,7 +134,7 @@ pi.registerShortcut(Key.ctrlAlt("p"), { clearPendingMenu(); const current = pendingModeTransition?.enabled ?? state.enabled; const enabled = !current; - if (requestModeTransition({ enabled }, ctx)) { + if (requestModeTransition({ enabled, applyOnSettled: true }, ctx)) { ctx.ui.notify( enabled ? "Plan mode enabled." : "Plan mode disabled.", "info", @@ -156,7 +144,7 @@ pi.registerShortcut(Key.ctrlAlt("p"), { }); ``` -Pi 0.83’s extension runner confirms this key is not reserved; Pi’s bundled Plan-mode example uses the same shortcut. Do not register Shift-Tab because Pi reserves it for `app.thinking.cycle`. +Pi 0.83’s extension runner confirms this key is not reserved; Pi’s bundled Plan-mode example uses the same shortcut. Do not register Shift-Tab because Pi reserves it for `app.thinking.cycle`. The transition marker enables a shortcut-only fallback: command contexts continue using `waitForIdle()`, while busy shortcut contexts store the latest transition for `agent_settled` because Pi does not expose `waitForIdle()` on `ExtensionContext`. Keep other non-command contexts on the existing warning path so prompt-bearing transitions never run before all settlement handlers finish. - [ ] **Step 7: Replace unconditional mutation blocking.** In `src/index.ts`, handle Save plan first: route `write` to `savePlanSession.authorizeToolCall`, block `edit`, and do not allow ordinary mutation authorization during that session. Outside Save plan, for any `isPlanMutationToolName(event.toolName)`, return `undefined` only when `state.selectedToolNames?.includes(event.toolName)` is true; otherwise return the existing Plan-mode block result. Leave Bash validation unchanged.