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..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. +`@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 @@ -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. @@ -73,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 @@ -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`. + +**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 new file mode 100644 index 0000000..9c066da --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-plan-mode-shortcut-and-mutation-tool-toggles.md @@ -0,0 +1,189 @@ +# 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 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. + +--- + +## 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 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); +``` + +- [ ] **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; + +export function isPlanMutationToolName(name: string): boolean { + return PLAN_MUTATION_TOOL_NAMES.some((mutationToolName) => mutationToolName === name); +} +``` + +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 by filtering `PLAN_MUTATION_TOOL_NAMES`. 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 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. + +- [ ] **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, applyOnSettled: true }, 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`. 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. + +- [ ] **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. diff --git a/src/core/prompt.ts b/src/core/prompt.ts index df25380..af67235 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 { PLAN_MUTATION_TOOL_NAMES } 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,30 @@ 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; +export function buildPlanModePrompt(selectedToolNames: readonly string[] = []): string { + const enabled = PLAN_MUTATION_TOOL_NAMES.filter((name) => selectedToolNames.includes(name)); + const mutationRule = + 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) + +You are in Plan Mode. Produce a decision-complete implementation plan +before any code mutation happens. + +## Mode 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} + +${PHASES_AND_TEMPLATE}`; } diff --git a/src/core/tools.ts b/src/core/tools.ts index 211b638..ea0a262 100644 --- a/src/core/tools.ts +++ b/src/core/tools.ts @@ -8,10 +8,14 @@ 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 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_NAMES.some((mutationToolName) => mutationToolName === name); +} + export type PlanToolInfo = Pick & { sourceInfo: Pick; }; @@ -35,8 +39,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 (isPlanMutationToolName(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/index.ts b/src/index.ts index 9039297..4670950 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, @@ -32,6 +34,7 @@ interface PendingModeTransition { enabled: boolean; prompt?: string; consumePlan?: boolean; + applyOnSettled?: boolean; } export default function createExtension(pi: ExtensionAPI): void { @@ -42,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, }); @@ -143,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; } @@ -191,7 +196,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 +257,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 +267,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 +297,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, applyOnSettled: true }, 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 +374,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)}`, }; }); @@ -387,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/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/prompt.test.ts b/tests/core/prompt.test.ts index 1d8b2b2..be57185 100644 --- a/tests/core/prompt.test.ts +++ b/tests/core/prompt.test.ts @@ -6,17 +6,6 @@ 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(); expect(prompt).toContain("Phase 1 -- Explore"); @@ -24,7 +13,7 @@ describe("buildPlanModePrompt", () => { expect(prompt).toContain("Phase 3 -- Plan"); }); - it("contains the proposed_plan template block", () => { + it("contains the proposed_plan template", () => { const prompt = buildPlanModePrompt(); expect(prompt).toContain(""); expect(prompt).toContain(""); @@ -34,8 +23,64 @@ 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("contains the phase-3 'do not ask' guidance", () => { + const prompt = buildPlanModePrompt(); + 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", + ); }); }); diff --git a/tests/core/tools.test.ts b/tests/core/tools.test.ts index a1af66d..e3bb4ad 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, @@ -19,6 +20,20 @@ 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("getToolPolicy", () => { it("returns the fixed policy for every built-in row", () => { for (const name of ["read", "grep", "find", "ls"]) { @@ -35,13 +50,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 +90,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/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..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", () => { @@ -1881,3 +1893,321 @@ 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(shortcutContext(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; + 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); + + 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", () => { + const tempDirs: string[] = []; + afterEach(() => { + for (const path of tempDirs.splice(0)) rmSync(path, { recursive: true, force: true }); + delete process.env.PI_CODING_AGENT_DIR; + }); + + 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(Object.fromEntries(["read", "bash", ...selected].map((name) => [name, 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); + return { mock, 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, + ); + 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 { mock, ctx } = await startWith(["edit", "write"]); + 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."); + }); +}); + +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"); + }); +}); 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"]);