From 4e7a7e2656e69e7cf265aea058699f61165286e0 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Wed, 24 Sep 2025 14:26:17 +0800 Subject: [PATCH] feat: Implement Plan Mode for Safe Code Planning (#658) --- docs/cli/commands.md | 12 + docs/cli/configuration.md | 23 +- docs/keyboard-shortcuts.md | 20 +- .../cli/src/config/config.integration.test.ts | 29 +- packages/cli/src/config/config.test.ts | 96 +++- packages/cli/src/config/config.ts | 69 ++- packages/cli/src/config/settingsSchema.ts | 10 + .../src/services/BuiltinCommandLoader.test.ts | 12 + .../cli/src/services/BuiltinCommandLoader.ts | 10 +- .../cli/src/test-utils/mockCommandContext.ts | 5 +- .../ui/commands/approvalModeCommand.test.ts | 495 ++++++++++++++++++ .../src/ui/commands/approvalModeCommand.ts | 434 +++++++++++++++ .../src/ui/components/AutoAcceptIndicator.tsx | 11 +- packages/cli/src/ui/components/Help.tsx | 8 +- .../src/ui/components/PlanSummaryDisplay.tsx | 41 ++ .../messages/ToolConfirmationMessage.test.tsx | 25 + .../messages/ToolConfirmationMessage.tsx | 28 + .../ui/components/messages/ToolMessage.tsx | 34 ++ .../subagents/manage/AgentsManagerDialog.tsx | 2 +- .../ui/hooks/useAutoAcceptIndicator.test.ts | 208 +++----- .../src/ui/hooks/useAutoAcceptIndicator.ts | 52 +- .../src/ui/hooks/useVisionAutoSwitch.test.ts | 21 + .../cli/src/zed-integration/zedIntegration.ts | 19 + packages/core/src/config/config.test.ts | 14 + packages/core/src/config/config.ts | 15 +- packages/core/src/core/client.test.ts | 55 +- packages/core/src/core/client.ts | 47 +- .../core/src/core/coreToolScheduler.test.ts | 295 ++++++++++- packages/core/src/core/coreToolScheduler.ts | 23 +- .../core/openaiContentGenerator/converter.ts | 22 +- packages/core/src/core/prompts.test.ts | 57 +- packages/core/src/core/prompts.ts | 50 ++ packages/core/src/tools/exitPlanMode.test.ts | 292 +++++++++++ packages/core/src/tools/exitPlanMode.ts | 191 +++++++ packages/core/src/tools/shell.test.ts | 13 + packages/core/src/tools/shell.ts | 6 + packages/core/src/tools/tool-names.ts | 1 + packages/core/src/tools/tools.ts | 17 +- packages/core/src/tools/web-search.ts | 24 + packages/core/src/utils/shell-utils.test.ts | 17 + packages/core/src/utils/shell-utils.ts | 17 + .../src/utils/shellReadOnlyChecker.test.ts | 56 ++ .../core/src/utils/shellReadOnlyChecker.ts | 300 +++++++++++ 43 files changed, 2895 insertions(+), 281 deletions(-) create mode 100644 packages/cli/src/ui/commands/approvalModeCommand.test.ts create mode 100644 packages/cli/src/ui/commands/approvalModeCommand.ts create mode 100644 packages/cli/src/ui/components/PlanSummaryDisplay.tsx create mode 100644 packages/core/src/tools/exitPlanMode.test.ts create mode 100644 packages/core/src/tools/exitPlanMode.ts create mode 100644 packages/core/src/utils/shellReadOnlyChecker.test.ts create mode 100644 packages/core/src/utils/shellReadOnlyChecker.ts diff --git a/docs/cli/commands.md b/docs/cli/commands.md index 09c1bce2..4c66d6d2 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -124,6 +124,18 @@ Slash commands provide meta-level control over the CLI itself. - **`/auth`** - **Description:** Open a dialog that lets you change the authentication method. +- **`/approval-mode`** + - **Description:** Change the approval mode for tool usage. + - **Usage:** `/approval-mode [mode] [--session|--project|--user]` + - **Available Modes:** + - **`plan`**: Analyze only; do not modify files or execute commands + - **`default`**: Require approval for file edits or shell commands + - **`auto-edit`**: Automatically approve file edits + - **`yolo`**: Automatically approve all tools + - **Examples:** + - `/approval-mode plan --project` (persist plan mode for this project) + - `/approval-mode yolo --user` (persist YOLO mode for this user across projects) + - **`/about`** - **Description:** Show version info. Please share this information when filing issues. diff --git a/docs/cli/configuration.md b/docs/cli/configuration.md index aad246ed..0a16eb2e 100644 --- a/docs/cli/configuration.md +++ b/docs/cli/configuration.md @@ -362,6 +362,18 @@ If you are experiencing performance issues with file searching (e.g., with `@` c "skipLoopDetection": true ``` +- **`approvalMode`** (string): + - **Description:** Sets the default approval mode for tool usage. Accepted values are: + - `plan`: Analyze only, do not modify files or execute commands. + - `default`: Require approval before file edits or shell commands run. + - `auto-edit`: Automatically approve file edits. + - `yolo`: Automatically approve all tool calls. + - **Default:** `"default"` + - **Example:** + ```json + "approvalMode": "plan" + ``` + ### Example `settings.json`: ```json @@ -486,12 +498,13 @@ Arguments passed directly when running the CLI can override other configurations - **`--yolo`**: - Enables YOLO mode, which automatically approves all tool calls. - **`--approval-mode `**: - - Sets the approval mode for tool calls. Available modes: - - `default`: Prompt for approval on each tool call (default behavior) - - `auto_edit`: Automatically approve edit tools (edit, write_file) while prompting for others - - `yolo`: Automatically approve all tool calls (equivalent to `--yolo`) + - Sets the approval mode for tool calls. Supported modes: + - `plan`: Analyze only—do not modify files or execute commands. + - `default`: Require approval for file edits or shell commands (default behavior). + - `auto-edit`: Automatically approve edit tools (edit, write_file) while prompting for others. + - `yolo`: Automatically approve all tool calls (equivalent to `--yolo`). - Cannot be used together with `--yolo`. Use `--approval-mode=yolo` instead of `--yolo` for the new unified approach. - - Example: `qwen --approval-mode auto_edit` + - Example: `qwen --approval-mode auto-edit` - **`--allowed-tools `**: - A comma-separated list of tool names that will bypass the confirmation dialog. - Example: `qwen --allowed-tools "ShellTool(git status)"` diff --git a/docs/keyboard-shortcuts.md b/docs/keyboard-shortcuts.md index 35ca538e..b029c4a0 100644 --- a/docs/keyboard-shortcuts.md +++ b/docs/keyboard-shortcuts.md @@ -4,16 +4,16 @@ This document lists the available keyboard shortcuts in Qwen Code. ## General -| Shortcut | Description | -| -------- | --------------------------------------------------------------------------------------------------------------------- | -| `Esc` | Close dialogs and suggestions. | -| `Ctrl+C` | Cancel the ongoing request and clear the input. Press twice to exit the application. | -| `Ctrl+D` | Exit the application if the input is empty. Press twice to confirm. | -| `Ctrl+L` | Clear the screen. | -| `Ctrl+O` | Toggle the display of the debug console. | -| `Ctrl+S` | Allows long responses to print fully, disabling truncation. Use your terminal's scrollback to view the entire output. | -| `Ctrl+T` | Toggle the display of tool descriptions. | -| `Ctrl+Y` | Toggle auto-approval (YOLO mode) for all tool calls. | +| Shortcut | Description | +| ----------- | --------------------------------------------------------------------------------------------------------------------- | +| `Esc` | Close dialogs and suggestions. | +| `Ctrl+C` | Cancel the ongoing request and clear the input. Press twice to exit the application. | +| `Ctrl+D` | Exit the application if the input is empty. Press twice to confirm. | +| `Ctrl+L` | Clear the screen. | +| `Ctrl+O` | Toggle the display of the debug console. | +| `Ctrl+S` | Allows long responses to print fully, disabling truncation. Use your terminal's scrollback to view the entire output. | +| `Ctrl+T` | Toggle the display of tool descriptions. | +| `Shift+Tab` | Cycle approval modes (`plan` → `default` → `auto-edit` → `yolo`). | ## Input Prompt diff --git a/packages/cli/src/config/config.integration.test.ts b/packages/cli/src/config/config.integration.test.ts index 6916996a..ff503df8 100644 --- a/packages/cli/src/config/config.integration.test.ts +++ b/packages/cli/src/config/config.integration.test.ts @@ -269,7 +269,7 @@ describe('Configuration Integration Tests', () => { parseArguments = parseArgs; }); - it('should parse --approval-mode=auto_edit correctly through the full argument parsing flow', async () => { + it('should parse --approval-mode=auto-edit correctly through the full argument parsing flow', async () => { const originalArgv = process.argv; try { @@ -277,7 +277,7 @@ describe('Configuration Integration Tests', () => { 'node', 'script.js', '--approval-mode', - 'auto_edit', + 'auto-edit', '-p', 'test', ]; @@ -285,7 +285,30 @@ describe('Configuration Integration Tests', () => { const argv = await parseArguments({} as Settings); // Verify that the argument was parsed correctly - expect(argv.approvalMode).toBe('auto_edit'); + expect(argv.approvalMode).toBe('auto-edit'); + expect(argv.prompt).toBe('test'); + expect(argv.yolo).toBe(false); + } finally { + process.argv = originalArgv; + } + }); + + it('should parse --approval-mode=plan correctly through the full argument parsing flow', async () => { + const originalArgv = process.argv; + + try { + process.argv = [ + 'node', + 'script.js', + '--approval-mode', + 'plan', + '-p', + 'test', + ]; + + const argv = await parseArguments({} as Settings); + + expect(argv.approvalMode).toBe('plan'); expect(argv.prompt).toBe('test'); expect(argv.yolo).toBe(false); } finally { diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 8acbe717..5abf181e 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -262,9 +262,9 @@ describe('parseArguments', () => { }); it('should allow --approval-mode without --yolo', async () => { - process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit']; + process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit']; const argv = await parseArguments({} as Settings); - expect(argv.approvalMode).toBe('auto_edit'); + expect(argv.approvalMode).toBe('auto-edit'); expect(argv.yolo).toBe(false); }); @@ -1087,6 +1087,32 @@ describe('Approval mode tool exclusion logic', () => { expect(excludedTools).toContain(WriteFileTool.Name); }); + it('should exclude all interactive tools in non-interactive mode with plan approval mode', async () => { + process.argv = [ + 'node', + 'script.js', + '--approval-mode', + 'plan', + '-p', + 'test', + ]; + const argv = await parseArguments({} as Settings); + const settings: Settings = {}; + const extensions: Extension[] = []; + + const config = await loadCliConfig( + settings, + extensions, + 'test-session', + argv, + ); + + const excludedTools = config.getExcludeTools(); + expect(excludedTools).toContain(ShellTool.Name); + expect(excludedTools).toContain(EditTool.Name); + expect(excludedTools).toContain(WriteFileTool.Name); + }); + it('should exclude all interactive tools in non-interactive mode with explicit default approval mode', async () => { process.argv = [ 'node', @@ -1113,12 +1139,12 @@ describe('Approval mode tool exclusion logic', () => { expect(excludedTools).toContain(WriteFileTool.Name); }); - it('should exclude only shell tools in non-interactive mode with auto_edit approval mode', async () => { + it('should exclude only shell tools in non-interactive mode with auto-edit approval mode', async () => { process.argv = [ 'node', 'script.js', '--approval-mode', - 'auto_edit', + 'auto-edit', '-p', 'test', ]; @@ -1189,8 +1215,9 @@ describe('Approval mode tool exclusion logic', () => { const testCases = [ { args: ['node', 'script.js'] }, // default + { args: ['node', 'script.js', '--approval-mode', 'plan'] }, { args: ['node', 'script.js', '--approval-mode', 'default'] }, - { args: ['node', 'script.js', '--approval-mode', 'auto_edit'] }, + { args: ['node', 'script.js', '--approval-mode', 'auto-edit'] }, { args: ['node', 'script.js', '--approval-mode', 'yolo'] }, { args: ['node', 'script.js', '--yolo'] }, ]; @@ -1215,12 +1242,12 @@ describe('Approval mode tool exclusion logic', () => { } }); - it('should merge approval mode exclusions with settings exclusions in auto_edit mode', async () => { + it('should merge approval mode exclusions with settings exclusions in auto-edit mode', async () => { process.argv = [ 'node', 'script.js', '--approval-mode', - 'auto_edit', + 'auto-edit', '-p', 'test', ]; @@ -1238,8 +1265,8 @@ describe('Approval mode tool exclusion logic', () => { const excludedTools = config.getExcludeTools(); expect(excludedTools).toContain('custom_tool'); // From settings expect(excludedTools).toContain(ShellTool.Name); // From approval mode - expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto_edit - expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto_edit + expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto-edit + expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto-edit }); it('should throw an error for invalid approval mode values in loadCliConfig', async () => { @@ -1262,7 +1289,7 @@ describe('Approval mode tool exclusion logic', () => { invalidArgv as CliArgs, ), ).rejects.toThrow( - 'Invalid approval mode: invalid_mode. Valid values are: yolo, auto_edit, default', + 'Invalid approval mode: invalid_mode. Valid values are: plan, default, auto-edit, yolo', ); }); }); @@ -1929,6 +1956,13 @@ describe('loadCliConfig approval mode', () => { expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT); }); + it('should set PLAN approval mode when --approval-mode=plan', async () => { + process.argv = ['node', 'script.js', '--approval-mode', 'plan']; + const argv = await parseArguments({} as Settings); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN); + }); + it('should set YOLO approval mode when --yolo flag is used', async () => { process.argv = ['node', 'script.js', '--yolo']; const argv = await parseArguments({} as Settings); @@ -1950,8 +1984,8 @@ describe('loadCliConfig approval mode', () => { expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT); }); - it('should set AUTO_EDIT approval mode when --approval-mode=auto_edit', async () => { - process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit']; + it('should set AUTO_EDIT approval mode when --approval-mode=auto-edit', async () => { + process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit']; const argv = await parseArguments({} as Settings); const config = await loadCliConfig({}, [], 'test-session', argv); expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.AUTO_EDIT); @@ -1964,6 +1998,33 @@ describe('loadCliConfig approval mode', () => { expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO); }); + it('should use approval mode from settings when CLI flags are not provided', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments({} as Settings); + const settings: Settings = { approvalMode: 'plan' }; + const config = await loadCliConfig(settings, [], 'test-session', argv); + expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN); + }); + + it('should normalize approval mode values from settings', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments({} as Settings); + const settings: Settings = { approvalMode: 'AutoEdit' }; + const config = await loadCliConfig(settings, [], 'test-session', argv); + expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.AUTO_EDIT); + }); + + it('should throw when approval mode in settings is invalid', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments({} as Settings); + const settings: Settings = { approvalMode: 'invalid_mode' }; + await expect( + loadCliConfig(settings, [], 'test-session', argv), + ).rejects.toThrow( + 'Invalid approval mode: invalid_mode. Valid values are: plan, default, auto-edit, yolo', + ); + }); + it('should prioritize --approval-mode over --yolo when both would be valid (but validation prevents this)', async () => { // Note: This test documents the intended behavior, but in practice the validation // prevents both flags from being used together @@ -1995,8 +2056,8 @@ describe('loadCliConfig approval mode', () => { expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT); }); - it('should override --approval-mode=auto_edit to DEFAULT', async () => { - process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit']; + it('should override --approval-mode=auto-edit to DEFAULT', async () => { + process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit']; const argv = await parseArguments({} as Settings); const config = await loadCliConfig({}, [], 'test-session', argv); expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT); @@ -2015,6 +2076,13 @@ describe('loadCliConfig approval mode', () => { const config = await loadCliConfig({}, [], 'test-session', argv); expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT); }); + + it('should allow PLAN approval mode in untrusted folders', async () => { + process.argv = ['node', 'script.js', '--approval-mode', 'plan']; + const argv = await parseArguments({} as Settings); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN); + }); }); }); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index e1ee021f..da06e35c 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -52,6 +52,39 @@ const logger = { error: (...args: any[]) => console.error('[ERROR]', ...args), }; +const VALID_APPROVAL_MODE_VALUES = [ + 'plan', + 'default', + 'auto-edit', + 'yolo', +] as const; + +function formatApprovalModeError(value: string): Error { + return new Error( + `Invalid approval mode: ${value}. Valid values are: ${VALID_APPROVAL_MODE_VALUES.join( + ', ', + )}`, + ); +} + +function parseApprovalModeValue(value: string): ApprovalMode { + const normalized = value.trim().toLowerCase(); + switch (normalized) { + case 'plan': + return ApprovalMode.PLAN; + case 'default': + return ApprovalMode.DEFAULT; + case 'yolo': + return ApprovalMode.YOLO; + case 'auto_edit': + case 'autoedit': + case 'auto-edit': + return ApprovalMode.AUTO_EDIT; + default: + throw formatApprovalModeError(value); + } +} + export interface CliArgs { model: string | undefined; sandbox: boolean | string | undefined; @@ -147,9 +180,9 @@ export async function parseArguments(settings: Settings): Promise { }) .option('approval-mode', { type: 'string', - choices: ['default', 'auto_edit', 'yolo'], + choices: ['plan', 'default', 'auto-edit', 'yolo'], description: - 'Set the approval mode: default (prompt for approval), auto_edit (auto-approve edit tools), yolo (auto-approve all tools)', + 'Set the approval mode: plan (plan only), default (prompt for approval), auto-edit (auto-approve edit tools), yolo (auto-approve all tools)', }) .option('telemetry', { type: 'boolean', @@ -438,30 +471,21 @@ export async function loadCliConfig( // Determine approval mode with backward compatibility let approvalMode: ApprovalMode; if (argv.approvalMode) { - // New --approval-mode flag takes precedence - switch (argv.approvalMode) { - case 'yolo': - approvalMode = ApprovalMode.YOLO; - break; - case 'auto_edit': - approvalMode = ApprovalMode.AUTO_EDIT; - break; - case 'default': - approvalMode = ApprovalMode.DEFAULT; - break; - default: - throw new Error( - `Invalid approval mode: ${argv.approvalMode}. Valid values are: yolo, auto_edit, default`, - ); - } + approvalMode = parseApprovalModeValue(argv.approvalMode); + } else if (argv.yolo) { + approvalMode = ApprovalMode.YOLO; + } else if (settings.approvalMode) { + approvalMode = parseApprovalModeValue(settings.approvalMode); } else { - // Fallback to legacy --yolo flag behavior - approvalMode = - argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT; + approvalMode = ApprovalMode.DEFAULT; } // Force approval mode to default if the folder is not trusted. - if (!trustedFolder && approvalMode !== ApprovalMode.DEFAULT) { + if ( + !trustedFolder && + approvalMode !== ApprovalMode.DEFAULT && + approvalMode !== ApprovalMode.PLAN + ) { logger.warn( `Approval mode overridden to "default" because the current folder is not trusted.`, ); @@ -474,6 +498,7 @@ export async function loadCliConfig( const extraExcludes: string[] = []; if (!interactive && !argv.experimentalAcp) { switch (approvalMode) { + case ApprovalMode.PLAN: case ApprovalMode.DEFAULT: // In default non-interactive mode, all tools that require approval are excluded. extraExcludes.push(ShellTool.Name, EditTool.Name, WriteFileTool.Name); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 815b5c58..9fb29a99 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -892,6 +892,16 @@ export const SETTINGS_SCHEMA = { description: 'Disable all loop detection checks (streaming and LLM).', showInDialog: true, }, + approvalMode: { + type: 'string', + label: 'Default Approval Mode', + category: 'General', + requiresRestart: false, + default: 'default', + description: + 'Default approval mode for tool usage. Valid values: plan, default, auto-edit, yolo.', + showInDialog: true, + }, enableWelcomeBack: { type: 'boolean', label: 'Enable Welcome Back', diff --git a/packages/cli/src/services/BuiltinCommandLoader.test.ts b/packages/cli/src/services/BuiltinCommandLoader.test.ts index 38deb425..36136e13 100644 --- a/packages/cli/src/services/BuiltinCommandLoader.test.ts +++ b/packages/cli/src/services/BuiltinCommandLoader.test.ts @@ -15,6 +15,14 @@ vi.mock('../ui/commands/aboutCommand.js', async () => { }; }); +vi.mock('../ui/commands/approvalModeCommand.js', () => ({ + approvalModeCommand: { + name: 'approval-mode', + description: 'Approval mode command', + kind: 'built-in', + }, +})); + vi.mock('../ui/commands/ideCommand.js', () => ({ ideCommand: vi.fn() })); vi.mock('../ui/commands/restoreCommand.js', () => ({ restoreCommand: vi.fn(), @@ -128,6 +136,10 @@ describe('BuiltinCommandLoader', () => { expect(aboutCmd).toBeDefined(); expect(aboutCmd?.kind).toBe(CommandKind.BUILT_IN); + const approvalModeCmd = commands.find((c) => c.name === 'approval-mode'); + expect(approvalModeCmd).toBeDefined(); + expect(approvalModeCmd?.kind).toBe(CommandKind.BUILT_IN); + const ideCmd = commands.find((c) => c.name === 'ide'); expect(ideCmd).toBeDefined(); diff --git a/packages/cli/src/services/BuiltinCommandLoader.ts b/packages/cli/src/services/BuiltinCommandLoader.ts index 12c3cfc9..2d162333 100644 --- a/packages/cli/src/services/BuiltinCommandLoader.ts +++ b/packages/cli/src/services/BuiltinCommandLoader.ts @@ -8,6 +8,8 @@ import type { ICommandLoader } from './types.js'; import type { SlashCommand } from '../ui/commands/types.js'; import type { Config } from '@qwen-code/qwen-code-core'; import { aboutCommand } from '../ui/commands/aboutCommand.js'; +import { agentsCommand } from '../ui/commands/agentsCommand.js'; +import { approvalModeCommand } from '../ui/commands/approvalModeCommand.js'; import { authCommand } from '../ui/commands/authCommand.js'; import { bugCommand } from '../ui/commands/bugCommand.js'; import { chatCommand } from '../ui/commands/chatCommand.js'; @@ -24,19 +26,18 @@ import { ideCommand } from '../ui/commands/ideCommand.js'; import { initCommand } from '../ui/commands/initCommand.js'; import { mcpCommand } from '../ui/commands/mcpCommand.js'; import { memoryCommand } from '../ui/commands/memoryCommand.js'; +import { modelCommand } from '../ui/commands/modelCommand.js'; import { privacyCommand } from '../ui/commands/privacyCommand.js'; import { quitCommand, quitConfirmCommand } from '../ui/commands/quitCommand.js'; import { restoreCommand } from '../ui/commands/restoreCommand.js'; +import { settingsCommand } from '../ui/commands/settingsCommand.js'; import { statsCommand } from '../ui/commands/statsCommand.js'; import { summaryCommand } from '../ui/commands/summaryCommand.js'; +import { terminalSetupCommand } from '../ui/commands/terminalSetupCommand.js'; import { themeCommand } from '../ui/commands/themeCommand.js'; import { toolsCommand } from '../ui/commands/toolsCommand.js'; -import { settingsCommand } from '../ui/commands/settingsCommand.js'; import { vimCommand } from '../ui/commands/vimCommand.js'; import { setupGithubCommand } from '../ui/commands/setupGithubCommand.js'; -import { terminalSetupCommand } from '../ui/commands/terminalSetupCommand.js'; -import { modelCommand } from '../ui/commands/modelCommand.js'; -import { agentsCommand } from '../ui/commands/agentsCommand.js'; /** * Loads the core, hard-coded slash commands that are an integral part @@ -56,6 +57,7 @@ export class BuiltinCommandLoader implements ICommandLoader { const allDefinitions: Array = [ aboutCommand, agentsCommand, + approvalModeCommand, authCommand, bugCommand, chatCommand, diff --git a/packages/cli/src/test-utils/mockCommandContext.ts b/packages/cli/src/test-utils/mockCommandContext.ts index 377fc6a3..170620b5 100644 --- a/packages/cli/src/test-utils/mockCommandContext.ts +++ b/packages/cli/src/test-utils/mockCommandContext.ts @@ -35,7 +35,10 @@ export const createMockCommandContext = ( }, services: { config: null, - settings: { merged: {} } as LoadedSettings, + settings: { + merged: {}, + setValue: vi.fn(), + } as unknown as LoadedSettings, git: undefined as GitService | undefined, logger: { log: vi.fn(), diff --git a/packages/cli/src/ui/commands/approvalModeCommand.test.ts b/packages/cli/src/ui/commands/approvalModeCommand.test.ts new file mode 100644 index 00000000..c52c84fc --- /dev/null +++ b/packages/cli/src/ui/commands/approvalModeCommand.test.ts @@ -0,0 +1,495 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { approvalModeCommand } from './approvalModeCommand.js'; +import { + type CommandContext, + CommandKind, + type MessageActionReturn, +} from './types.js'; +import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; +import { ApprovalMode } from '@qwen-code/qwen-code-core'; +import { SettingScope, type LoadedSettings } from '../../config/settings.js'; + +describe('approvalModeCommand', () => { + let mockContext: CommandContext; + let setApprovalModeMock: ReturnType; + let setSettingsValueMock: ReturnType; + const originalEnv = { ...process.env }; + const userSettingsPath = '/mock/user/settings.json'; + const projectSettingsPath = '/mock/project/settings.json'; + const userSettingsFile = { path: userSettingsPath, settings: {} }; + const projectSettingsFile = { path: projectSettingsPath, settings: {} }; + + const getModeSubCommand = (mode: ApprovalMode) => + approvalModeCommand.subCommands?.find((cmd) => cmd.name === mode); + + const getScopeSubCommand = ( + mode: ApprovalMode, + scope: '--session' | '--user' | '--project', + ) => getModeSubCommand(mode)?.subCommands?.find((cmd) => cmd.name === scope); + + beforeEach(() => { + setApprovalModeMock = vi.fn(); + setSettingsValueMock = vi.fn(); + + mockContext = createMockCommandContext({ + services: { + config: { + getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), + setApprovalMode: setApprovalModeMock, + }, + settings: { + merged: {}, + setValue: setSettingsValueMock, + forScope: vi + .fn() + .mockImplementation((scope: SettingScope) => + scope === SettingScope.User + ? userSettingsFile + : scope === SettingScope.Workspace + ? projectSettingsFile + : { path: '', settings: {} }, + ), + } as unknown as LoadedSettings, + }, + } as unknown as CommandContext); + }); + + afterEach(() => { + process.env = { ...originalEnv }; + vi.clearAllMocks(); + }); + + it('should have the correct command properties', () => { + expect(approvalModeCommand.name).toBe('approval-mode'); + expect(approvalModeCommand.kind).toBe(CommandKind.BUILT_IN); + expect(approvalModeCommand.description).toBe( + 'View or change the approval mode for tool usage', + ); + }); + + it('should show current mode, options, and usage when no arguments provided', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + '', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('info'); + const expectedMessage = [ + 'Current approval mode: default', + '', + 'Available approval modes:', + ' - plan: Plan mode - Analyze only, do not modify files or execute commands', + ' - default: Default mode - Require approval for file edits or shell commands', + ' - auto-edit: Auto-edit mode - Automatically approve file edits', + ' - yolo: YOLO mode - Automatically approve all tools', + '', + 'Usage: /approval-mode [--session|--user|--project]', + ].join('\n'); + expect(result.content).toBe(expectedMessage); + }); + + it('should display error when config is not available', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const nullConfigContext = createMockCommandContext({ + services: { + config: null, + }, + } as unknown as CommandContext); + + const result = (await approvalModeCommand.action( + nullConfigContext, + '', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toBe('Configuration not available.'); + }); + + it('should change approval mode when valid mode is provided', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + 'plan', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + expect(result.type).toBe('message'); + expect(result.messageType).toBe('info'); + expect(result.content).toBe('Approval mode changed to: plan'); + }); + + it('should accept canonical auto-edit mode value', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + 'auto-edit', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + expect(result.type).toBe('message'); + expect(result.messageType).toBe('info'); + expect(result.content).toBe('Approval mode changed to: auto-edit'); + }); + + it('should accept auto-edit alias for compatibility', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + 'auto-edit', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + expect(result.content).toBe('Approval mode changed to: auto-edit'); + }); + + it('should display error when invalid mode is provided', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + 'invalid', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toContain('Invalid approval mode: invalid'); + expect(result.content).toContain('Available approval modes:'); + expect(result.content).toContain( + 'Usage: /approval-mode [--session|--user|--project]', + ); + }); + + it('should display error when setApprovalMode throws an error', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const errorMessage = 'Failed to set approval mode'; + mockContext.services.config!.setApprovalMode = vi + .fn() + .mockImplementation(() => { + throw new Error(errorMessage); + }); + + const result = (await approvalModeCommand.action( + mockContext, + 'plan', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toBe( + `Failed to change approval mode: ${errorMessage}`, + ); + }); + + it('should allow selecting auto-edit with user scope via nested subcommands', async () => { + if (!approvalModeCommand.subCommands) { + throw new Error('approvalModeCommand must have subCommands.'); + } + + const userSubCommand = getScopeSubCommand(ApprovalMode.AUTO_EDIT, '--user'); + if (!userSubCommand?.action) { + throw new Error('--user scope subcommand must have an action.'); + } + + const result = (await userSubCommand.action( + mockContext, + '', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.User, + 'approvalMode', + 'auto-edit', + ); + expect(result.content).toBe( + `Approval mode changed to: auto-edit (saved to user settings at ${userSettingsPath})`, + ); + }); + + it('should allow selecting plan with project scope via nested subcommands', async () => { + if (!approvalModeCommand.subCommands) { + throw new Error('approvalModeCommand must have subCommands.'); + } + + const projectSubCommand = getScopeSubCommand( + ApprovalMode.PLAN, + '--project', + ); + if (!projectSubCommand?.action) { + throw new Error('--project scope subcommand must have an action.'); + } + + const result = (await projectSubCommand.action( + mockContext, + '', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.Workspace, + 'approvalMode', + 'plan', + ); + expect(result.content).toBe( + `Approval mode changed to: plan (saved to project settings at ${projectSettingsPath})`, + ); + }); + + it('should allow selecting plan with session scope via nested subcommands', async () => { + if (!approvalModeCommand.subCommands) { + throw new Error('approvalModeCommand must have subCommands.'); + } + + const sessionSubCommand = getScopeSubCommand( + ApprovalMode.PLAN, + '--session', + ); + if (!sessionSubCommand?.action) { + throw new Error('--session scope subcommand must have an action.'); + } + + const result = (await sessionSubCommand.action( + mockContext, + '', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + expect(result.content).toBe('Approval mode changed to: plan'); + }); + + it('should allow providing a scope argument after selecting a mode subcommand', async () => { + if (!approvalModeCommand.subCommands) { + throw new Error('approvalModeCommand must have subCommands.'); + } + + const planSubCommand = getModeSubCommand(ApprovalMode.PLAN); + if (!planSubCommand?.action) { + throw new Error('plan subcommand must have an action.'); + } + + const result = (await planSubCommand.action( + mockContext, + '--user', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.User, + 'approvalMode', + 'plan', + ); + expect(result.content).toBe( + `Approval mode changed to: plan (saved to user settings at ${userSettingsPath})`, + ); + }); + + it('should support --user plan pattern (scope first)', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + '--user plan', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.User, + 'approvalMode', + 'plan', + ); + expect(result.content).toBe( + `Approval mode changed to: plan (saved to user settings at ${userSettingsPath})`, + ); + }); + + it('should support plan --user pattern (mode first)', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + 'plan --user', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.PLAN); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.User, + 'approvalMode', + 'plan', + ); + expect(result.content).toBe( + `Approval mode changed to: plan (saved to user settings at ${userSettingsPath})`, + ); + }); + + it('should support --project auto-edit pattern', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + '--project auto-edit', + )) as MessageActionReturn; + + expect(setApprovalModeMock).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT); + expect(setSettingsValueMock).toHaveBeenCalledWith( + SettingScope.Workspace, + 'approvalMode', + 'auto-edit', + ); + expect(result.content).toBe( + `Approval mode changed to: auto-edit (saved to project settings at ${projectSettingsPath})`, + ); + }); + + it('should display error when only scope flag is provided', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + '--user', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toContain('Missing approval mode'); + expect(setApprovalModeMock).not.toHaveBeenCalled(); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + }); + + it('should display error when multiple scope flags are provided', async () => { + if (!approvalModeCommand.action) { + throw new Error('approvalModeCommand must have an action.'); + } + + const result = (await approvalModeCommand.action( + mockContext, + '--user --project plan', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toContain('Multiple scope flags provided'); + expect(setApprovalModeMock).not.toHaveBeenCalled(); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + }); + + it('should surface a helpful error when scope subcommands receive extra arguments', async () => { + if (!approvalModeCommand.subCommands) { + throw new Error('approvalModeCommand must have subCommands.'); + } + + const userSubCommand = getScopeSubCommand(ApprovalMode.DEFAULT, '--user'); + if (!userSubCommand?.action) { + throw new Error('--user scope subcommand must have an action.'); + } + + const result = (await userSubCommand.action( + mockContext, + 'extra', + )) as MessageActionReturn; + + expect(result.type).toBe('message'); + expect(result.messageType).toBe('error'); + expect(result.content).toBe( + 'Scope subcommands do not accept additional arguments.', + ); + expect(setApprovalModeMock).not.toHaveBeenCalled(); + expect(setSettingsValueMock).not.toHaveBeenCalled(); + }); + + it('should provide completion for approval modes', async () => { + if (!approvalModeCommand.completion) { + throw new Error('approvalModeCommand must have a completion function.'); + } + + // Test partial mode completion + const result = await approvalModeCommand.completion(mockContext, 'p'); + expect(result).toEqual(['plan']); + + const result2 = await approvalModeCommand.completion(mockContext, 'a'); + expect(result2).toEqual(['auto-edit']); + + // Test empty completion - should suggest available modes first + const result3 = await approvalModeCommand.completion(mockContext, ''); + expect(result3).toEqual(['plan', 'default', 'auto-edit', 'yolo']); + + const result4 = await approvalModeCommand.completion(mockContext, 'AUTO'); + expect(result4).toEqual(['auto-edit']); + + // Test mode first pattern: 'plan ' should suggest scope flags + const result5 = await approvalModeCommand.completion(mockContext, 'plan '); + expect(result5).toEqual(['--session', '--project', '--user']); + + const result6 = await approvalModeCommand.completion( + mockContext, + 'plan --u', + ); + expect(result6).toEqual(['--user']); + + // Test scope first pattern: '--user ' should suggest modes + const result7 = await approvalModeCommand.completion( + mockContext, + '--user ', + ); + expect(result7).toEqual(['plan', 'default', 'auto-edit', 'yolo']); + + const result8 = await approvalModeCommand.completion( + mockContext, + '--user p', + ); + expect(result8).toEqual(['plan']); + + // Test completed patterns should return empty + const result9 = await approvalModeCommand.completion( + mockContext, + 'plan --user ', + ); + expect(result9).toEqual([]); + + const result10 = await approvalModeCommand.completion( + mockContext, + '--user plan ', + ); + expect(result10).toEqual([]); + }); +}); diff --git a/packages/cli/src/ui/commands/approvalModeCommand.ts b/packages/cli/src/ui/commands/approvalModeCommand.ts new file mode 100644 index 00000000..6cef96c2 --- /dev/null +++ b/packages/cli/src/ui/commands/approvalModeCommand.ts @@ -0,0 +1,434 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { + SlashCommand, + CommandContext, + MessageActionReturn, +} from './types.js'; +import { CommandKind } from './types.js'; +import { ApprovalMode, APPROVAL_MODES } from '@qwen-code/qwen-code-core'; +import { SettingScope } from '../../config/settings.js'; + +const USAGE_MESSAGE = + 'Usage: /approval-mode [--session|--user|--project]'; + +const normalizeInputMode = (value: string): string => + value.trim().toLowerCase(); + +const tokenizeArgs = (args: string): string[] => { + const matches = args.match(/(?:"[^"]*"|'[^']*'|[^\s"']+)/g); + if (!matches) { + return []; + } + + return matches.map((token) => { + if ( + (token.startsWith('"') && token.endsWith('"')) || + (token.startsWith("'") && token.endsWith("'")) + ) { + return token.slice(1, -1); + } + return token; + }); +}; + +const parseApprovalMode = (value: string | null): ApprovalMode | null => { + if (!value) { + return null; + } + + const normalized = normalizeInputMode(value).replace(/_/g, '-'); + const matchIndex = APPROVAL_MODES.findIndex( + (candidate) => candidate === normalized, + ); + + return matchIndex === -1 ? null : APPROVAL_MODES[matchIndex]; +}; + +const formatModeDescription = (mode: ApprovalMode): string => { + switch (mode) { + case ApprovalMode.PLAN: + return 'Plan mode - Analyze only, do not modify files or execute commands'; + case ApprovalMode.DEFAULT: + return 'Default mode - Require approval for file edits or shell commands'; + case ApprovalMode.AUTO_EDIT: + return 'Auto-edit mode - Automatically approve file edits'; + case ApprovalMode.YOLO: + return 'YOLO mode - Automatically approve all tools'; + default: + return `${mode} mode`; + } +}; + +const parseApprovalArgs = ( + args: string, +): { + mode: string | null; + scope: 'session' | 'user' | 'project'; + error?: string; +} => { + const trimmedArgs = args.trim(); + if (!trimmedArgs) { + return { mode: null, scope: 'session' }; + } + + const tokens = tokenizeArgs(trimmedArgs); + let mode: string | null = null; + let scope: 'session' | 'user' | 'project' = 'session'; + let scopeFlag: string | null = null; + + // Find scope flag and mode + for (const token of tokens) { + if (token === '--session' || token === '--user' || token === '--project') { + if (scopeFlag) { + return { + mode: null, + scope: 'session', + error: 'Multiple scope flags provided', + }; + } + scopeFlag = token; + scope = token.substring(2) as 'session' | 'user' | 'project'; + } else if (!mode) { + mode = token; + } else { + return { + mode: null, + scope: 'session', + error: 'Invalid arguments provided', + }; + } + } + + if (!mode) { + return { mode: null, scope: 'session', error: 'Missing approval mode' }; + } + + return { mode, scope }; +}; + +const setApprovalModeWithScope = async ( + context: CommandContext, + mode: ApprovalMode, + scope: 'session' | 'user' | 'project', +): Promise => { + const { services } = context; + const { config } = services; + + if (!config) { + return { + type: 'message', + messageType: 'error', + content: 'Configuration not available.', + }; + } + + try { + // Always set the mode in the current session + config.setApprovalMode(mode); + + // If scope is not session, also persist to settings + if (scope !== 'session') { + const { settings } = context.services; + if (!settings || typeof settings.setValue !== 'function') { + return { + type: 'message', + messageType: 'error', + content: + 'Settings service is not available; unable to persist the approval mode.', + }; + } + + const settingScope = + scope === 'user' ? SettingScope.User : SettingScope.Workspace; + const scopeLabel = scope === 'user' ? 'user' : 'project'; + let settingsPath: string | undefined; + + try { + if (typeof settings.forScope === 'function') { + settingsPath = settings.forScope(settingScope)?.path; + } + } catch (_error) { + settingsPath = undefined; + } + + try { + settings.setValue(settingScope, 'approvalMode', mode); + } catch (error) { + return { + type: 'message', + messageType: 'error', + content: `Failed to save approval mode: ${(error as Error).message}`, + }; + } + + const locationSuffix = settingsPath ? ` at ${settingsPath}` : ''; + + const scopeSuffix = ` (saved to ${scopeLabel} settings${locationSuffix})`; + + return { + type: 'message', + messageType: 'info', + content: `Approval mode changed to: ${mode}${scopeSuffix}`, + }; + } + + return { + type: 'message', + messageType: 'info', + content: `Approval mode changed to: ${mode}`, + }; + } catch (error) { + return { + type: 'message', + messageType: 'error', + content: `Failed to change approval mode: ${(error as Error).message}`, + }; + } +}; + +export const approvalModeCommand: SlashCommand = { + name: 'approval-mode', + description: 'View or change the approval mode for tool usage', + kind: CommandKind.BUILT_IN, + action: async ( + context: CommandContext, + args: string, + ): Promise => { + const { config } = context.services; + if (!config) { + return { + type: 'message', + messageType: 'error', + content: 'Configuration not available.', + }; + } + + // If no arguments provided, show current mode and available options + if (!args || args.trim() === '') { + const currentMode = + typeof config.getApprovalMode === 'function' + ? config.getApprovalMode() + : null; + + const messageLines: string[] = []; + + if (currentMode) { + messageLines.push(`Current approval mode: ${currentMode}`); + messageLines.push(''); + } + + messageLines.push('Available approval modes:'); + for (const mode of APPROVAL_MODES) { + messageLines.push(` - ${mode}: ${formatModeDescription(mode)}`); + } + messageLines.push(''); + messageLines.push(USAGE_MESSAGE); + + return { + type: 'message', + messageType: 'info', + content: messageLines.join('\n'), + }; + } + + // Parse arguments flexibly + const parsed = parseApprovalArgs(args); + + if (parsed.error) { + return { + type: 'message', + messageType: 'error', + content: `${parsed.error}. ${USAGE_MESSAGE}`, + }; + } + + if (!parsed.mode) { + return { + type: 'message', + messageType: 'info', + content: USAGE_MESSAGE, + }; + } + + const requestedMode = parseApprovalMode(parsed.mode); + + if (!requestedMode) { + let message = `Invalid approval mode: ${parsed.mode}\n\n`; + message += 'Available approval modes:\n'; + for (const mode of APPROVAL_MODES) { + message += ` - ${mode}: ${formatModeDescription(mode)}\n`; + } + message += `\n${USAGE_MESSAGE}`; + return { + type: 'message', + messageType: 'error', + content: message, + }; + } + + return setApprovalModeWithScope(context, requestedMode, parsed.scope); + }, + subCommands: APPROVAL_MODES.map((mode) => ({ + name: mode, + description: formatModeDescription(mode), + kind: CommandKind.BUILT_IN, + subCommands: [ + { + name: '--session', + description: 'Apply to current session only (temporary)', + kind: CommandKind.BUILT_IN, + action: async ( + context: CommandContext, + args: string, + ): Promise => { + if (args.trim().length > 0) { + return { + type: 'message', + messageType: 'error', + content: 'Scope subcommands do not accept additional arguments.', + }; + } + return setApprovalModeWithScope(context, mode, 'session'); + }, + }, + { + name: '--project', + description: 'Persist for this project/workspace', + kind: CommandKind.BUILT_IN, + action: async ( + context: CommandContext, + args: string, + ): Promise => { + if (args.trim().length > 0) { + return { + type: 'message', + messageType: 'error', + content: 'Scope subcommands do not accept additional arguments.', + }; + } + return setApprovalModeWithScope(context, mode, 'project'); + }, + }, + { + name: '--user', + description: 'Persist for this user on this machine', + kind: CommandKind.BUILT_IN, + action: async ( + context: CommandContext, + args: string, + ): Promise => { + if (args.trim().length > 0) { + return { + type: 'message', + messageType: 'error', + content: 'Scope subcommands do not accept additional arguments.', + }; + } + return setApprovalModeWithScope(context, mode, 'user'); + }, + }, + ], + action: async ( + context: CommandContext, + args: string, + ): Promise => { + if (args.trim().length > 0) { + // Allow users who type `/approval-mode plan --user` via the subcommand path + const parsed = parseApprovalArgs(`${mode} ${args}`); + if (parsed.error) { + return { + type: 'message', + messageType: 'error', + content: `${parsed.error}. ${USAGE_MESSAGE}`, + }; + } + + const normalizedMode = parseApprovalMode(parsed.mode); + if (!normalizedMode) { + return { + type: 'message', + messageType: 'error', + content: `Invalid approval mode: ${parsed.mode}. ${USAGE_MESSAGE}`, + }; + } + + return setApprovalModeWithScope(context, normalizedMode, parsed.scope); + } + + return setApprovalModeWithScope(context, mode, 'session'); + }, + })), + completion: async (_context: CommandContext, partialArg: string) => { + const tokens = tokenizeArgs(partialArg); + const hasTrailingSpace = /\s$/.test(partialArg); + const currentSegment = hasTrailingSpace + ? '' + : tokens.length > 0 + ? tokens[tokens.length - 1] + : ''; + + const normalizedCurrent = normalizeInputMode(currentSegment).replace( + /_/g, + '-', + ); + + const scopeValues = ['--session', '--project', '--user']; + + const normalizeToken = (token: string) => + normalizeInputMode(token).replace(/_/g, '-'); + + const normalizedTokens = tokens.map(normalizeToken); + + if (tokens.length === 0) { + if (currentSegment.startsWith('-')) { + return scopeValues.filter((scope) => scope.startsWith(currentSegment)); + } + return APPROVAL_MODES; + } + + if (tokens.length === 1 && !hasTrailingSpace) { + const originalToken = tokens[0]; + if (originalToken.startsWith('-')) { + return scopeValues.filter((scope) => + scope.startsWith(normalizedCurrent), + ); + } + return APPROVAL_MODES.filter((mode) => + mode.startsWith(normalizedCurrent), + ); + } + + if (tokens.length === 1 && hasTrailingSpace) { + const normalizedFirst = normalizedTokens[0]; + if (scopeValues.includes(tokens[0])) { + return APPROVAL_MODES; + } + if (APPROVAL_MODES.includes(normalizedFirst as ApprovalMode)) { + return scopeValues; + } + return APPROVAL_MODES; + } + + if (tokens.length === 2 && !hasTrailingSpace) { + const normalizedFirst = normalizedTokens[0]; + if (scopeValues.includes(tokens[0])) { + return APPROVAL_MODES.filter((mode) => + mode.startsWith(normalizedCurrent), + ); + } + if (APPROVAL_MODES.includes(normalizedFirst as ApprovalMode)) { + return scopeValues.filter((scope) => + scope.startsWith(normalizedCurrent), + ); + } + return []; + } + + return []; + }, +}; diff --git a/packages/cli/src/ui/components/AutoAcceptIndicator.tsx b/packages/cli/src/ui/components/AutoAcceptIndicator.tsx index b655e17b..0b425ecc 100644 --- a/packages/cli/src/ui/components/AutoAcceptIndicator.tsx +++ b/packages/cli/src/ui/components/AutoAcceptIndicator.tsx @@ -21,15 +21,20 @@ export const AutoAcceptIndicator: React.FC = ({ let subText = ''; switch (approvalMode) { + case ApprovalMode.PLAN: + textColor = Colors.AccentBlue; + textContent = 'plan mode'; + subText = ' (shift + tab to cycle)'; + break; case ApprovalMode.AUTO_EDIT: textColor = Colors.AccentGreen; - textContent = 'accepting edits'; - subText = ' (shift + tab to toggle)'; + textContent = 'auto-accept edits'; + subText = ' (shift + tab to cycle)'; break; case ApprovalMode.YOLO: textColor = Colors.AccentRed; textContent = 'YOLO mode'; - subText = ' (ctrl + y to toggle)'; + subText = ' (shift + tab to cycle)'; break; case ApprovalMode.DEFAULT: default: diff --git a/packages/cli/src/ui/components/Help.tsx b/packages/cli/src/ui/components/Help.tsx index 93607ed6..edc8324a 100644 --- a/packages/cli/src/ui/components/Help.tsx +++ b/packages/cli/src/ui/components/Help.tsx @@ -133,12 +133,6 @@ export const Help: React.FC = ({ commands }) => ( {' '} - Open input in external editor - - - Ctrl+Y - {' '} - - Toggle YOLO mode - Enter @@ -155,7 +149,7 @@ export const Help: React.FC = ({ commands }) => ( Shift+Tab {' '} - - Toggle auto-accepting edits + - Cycle approval modes diff --git a/packages/cli/src/ui/components/PlanSummaryDisplay.tsx b/packages/cli/src/ui/components/PlanSummaryDisplay.tsx new file mode 100644 index 00000000..27b3b625 --- /dev/null +++ b/packages/cli/src/ui/components/PlanSummaryDisplay.tsx @@ -0,0 +1,41 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { Box, Text } from 'ink'; +import { MarkdownDisplay } from '../utils/MarkdownDisplay.js'; +import { Colors } from '../colors.js'; +import type { PlanResultDisplay } from '@qwen-code/qwen-code-core'; + +interface PlanSummaryDisplayProps { + data: PlanResultDisplay; + availableHeight?: number; + childWidth: number; +} + +export const PlanSummaryDisplay: React.FC = ({ + data, + availableHeight, + childWidth, +}) => { + const { message, plan } = data; + + return ( + + + + {message} + + + + + ); +}; diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index f3367744..7eabcdcb 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -5,6 +5,7 @@ */ import { describe, it, expect, vi } from 'vitest'; +import { EOL } from 'node:os'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import type { ToolCallConfirmationDetails, @@ -66,6 +67,30 @@ describe('ToolConfirmationMessage', () => { ); }); + it('should render plan confirmation with markdown plan content', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'plan', + title: 'Would you like to proceed?', + plan: '# Implementation Plan\n- Step one\n- Step two'.replace(/\n/g, EOL), + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame()).toContain('Yes, and auto-accept edits'); + expect(lastFrame()).toContain('Yes, and manually approve edits'); + expect(lastFrame()).toContain('No, keep planning'); + expect(lastFrame()).toContain('Implementation Plan'); + expect(lastFrame()).toContain('Step one'); + }); + describe('with folder trust', () => { const editConfirmationDetails: ToolCallConfirmationDetails = { type: 'edit', diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index b3afc6b8..523012dd 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -9,6 +9,7 @@ import { Box, Text } from 'ink'; import { DiffRenderer } from './DiffRenderer.js'; import { Colors } from '../../colors.js'; import { RenderInline } from '../../utils/InlineMarkdownRenderer.js'; +import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; import type { ToolCallConfirmationDetails, ToolExecuteConfirmationDetails, @@ -235,6 +236,33 @@ export const ToolConfirmationMessage: React.FC< ); + } else if (confirmationDetails.type === 'plan') { + const planProps = confirmationDetails; + + question = planProps.title; + options.push({ + label: 'Yes, and auto-accept edits', + value: ToolConfirmationOutcome.ProceedAlways, + }); + options.push({ + label: 'Yes, and manually approve edits', + value: ToolConfirmationOutcome.ProceedOnce, + }); + options.push({ + label: 'No, keep planning (esc)', + value: ToolConfirmationOutcome.Cancel, + }); + + bodyContent = ( + + + + ); } else if (confirmationDetails.type === 'info') { const infoProps = confirmationDetails; const displayUrls = diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index f2acea2a..2397d98f 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -18,9 +18,11 @@ import { TOOL_STATUS } from '../../constants.js'; import type { TodoResultDisplay, TaskResultDisplay, + PlanResultDisplay, Config, } from '@qwen-code/qwen-code-core'; import { AgentExecutionDisplay } from '../subagents/index.js'; +import { PlanSummaryDisplay } from '../PlanSummaryDisplay.js'; const STATIC_HEIGHT = 1; const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc. @@ -35,6 +37,7 @@ export type TextEmphasis = 'high' | 'medium' | 'low'; type DisplayRendererResult = | { type: 'none' } | { type: 'todo'; data: TodoResultDisplay } + | { type: 'plan'; data: PlanResultDisplay } | { type: 'string'; data: string } | { type: 'diff'; data: { fileDiff: string; fileName: string } } | { type: 'task'; data: TaskResultDisplay }; @@ -63,6 +66,18 @@ const useResultDisplayRenderer = ( }; } + if ( + typeof resultDisplay === 'object' && + resultDisplay !== null && + 'type' in resultDisplay && + resultDisplay.type === 'plan_summary' + ) { + return { + type: 'plan', + data: resultDisplay as PlanResultDisplay, + }; + } + // Check for SubagentExecutionResultDisplay (for non-task tools) if ( typeof resultDisplay === 'object' && @@ -102,6 +117,18 @@ const TodoResultRenderer: React.FC<{ data: TodoResultDisplay }> = ({ data, }) => ; +const PlanResultRenderer: React.FC<{ + data: PlanResultDisplay; + availableHeight?: number; + childWidth: number; +}> = ({ data, availableHeight, childWidth }) => ( + +); + /** * Component to render subagent execution results */ @@ -229,6 +256,13 @@ export const ToolMessage: React.FC = ({ {displayRenderer.type === 'todo' && ( )} + {displayRenderer.type === 'plan' && ( + + )} {displayRenderer.type === 'task' && ( { expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(1); }); - it('should toggle the indicator and update config when Shift+Tab or Ctrl+Y is pressed', () => { + it('should initialize with ApprovalMode.PLAN if config.getApprovalMode returns ApprovalMode.PLAN', () => { + mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.PLAN); + const { result } = renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + addItem: vi.fn(), + }), + ); + expect(result.current).toBe(ApprovalMode.PLAN); + expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(1); + }); + + it('should cycle approval modes when Shift+Tab is pressed', () => { mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT); const { result } = renderHook(() => useAutoAcceptIndicator({ @@ -180,23 +192,10 @@ describe('useAutoAcceptIndicator', () => { expect(result.current).toBe(ApprovalMode.AUTO_EDIT); act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); - }); - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.YOLO, - ); - expect(result.current).toBe(ApprovalMode.YOLO); - - act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); - }); - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.DEFAULT, - ); - expect(result.current).toBe(ApprovalMode.DEFAULT); - - act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); + capturedUseKeypressHandler({ + name: 'tab', + shift: true, + } as Key); }); expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( ApprovalMode.YOLO, @@ -210,9 +209,9 @@ describe('useAutoAcceptIndicator', () => { } as Key); }); expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.AUTO_EDIT, + ApprovalMode.PLAN, ); - expect(result.current).toBe(ApprovalMode.AUTO_EDIT); + expect(result.current).toBe(ApprovalMode.PLAN); act(() => { capturedUseKeypressHandler({ @@ -314,118 +313,10 @@ describe('useAutoAcceptIndicator', () => { mockConfigInstance.isTrustedFolder.mockReturnValue(false); }); - it('should not enable YOLO mode when Ctrl+Y is pressed', () => { - mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT); - mockConfigInstance.setApprovalMode.mockImplementation(() => { - throw new Error( - 'Cannot enable privileged approval modes in an untrusted folder.', - ); - }); - const mockAddItem = vi.fn(); - const { result } = renderHook(() => - useAutoAcceptIndicator({ - config: mockConfigInstance as unknown as ActualConfigType, - addItem: mockAddItem, - }), - ); - - expect(result.current).toBe(ApprovalMode.DEFAULT); - - act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); - }); - - // We expect setApprovalMode to be called, and the error to be caught. - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.YOLO, - ); - expect(mockAddItem).toHaveBeenCalled(); - // Verify the underlying config value was not changed - expect(mockConfigInstance.getApprovalMode()).toBe(ApprovalMode.DEFAULT); - }); - - it('should not enable AUTO_EDIT mode when Shift+Tab is pressed', () => { - mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT); - mockConfigInstance.setApprovalMode.mockImplementation(() => { - throw new Error( - 'Cannot enable privileged approval modes in an untrusted folder.', - ); - }); - const mockAddItem = vi.fn(); - const { result } = renderHook(() => - useAutoAcceptIndicator({ - config: mockConfigInstance as unknown as ActualConfigType, - addItem: mockAddItem, - }), - ); - - expect(result.current).toBe(ApprovalMode.DEFAULT); - - act(() => { - capturedUseKeypressHandler({ - name: 'tab', - shift: true, - } as Key); - }); - - // We expect setApprovalMode to be called, and the error to be caught. - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.AUTO_EDIT, - ); - expect(mockAddItem).toHaveBeenCalled(); - // Verify the underlying config value was not changed - expect(mockConfigInstance.getApprovalMode()).toBe(ApprovalMode.DEFAULT); - }); - - it('should disable YOLO mode when Ctrl+Y is pressed', () => { - mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO); - const mockAddItem = vi.fn(); - renderHook(() => - useAutoAcceptIndicator({ - config: mockConfigInstance as unknown as ActualConfigType, - addItem: mockAddItem, - }), - ); - - act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); - }); - - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.DEFAULT, - ); - expect(mockConfigInstance.getApprovalMode()).toBe(ApprovalMode.DEFAULT); - }); - - it('should disable AUTO_EDIT mode when Shift+Tab is pressed', () => { - mockConfigInstance.getApprovalMode.mockReturnValue( - ApprovalMode.AUTO_EDIT, - ); - const mockAddItem = vi.fn(); - renderHook(() => - useAutoAcceptIndicator({ - config: mockConfigInstance as unknown as ActualConfigType, - addItem: mockAddItem, - }), - ); - - act(() => { - capturedUseKeypressHandler({ - name: 'tab', - shift: true, - } as Key); - }); - - expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( - ApprovalMode.DEFAULT, - ); - expect(mockConfigInstance.getApprovalMode()).toBe(ApprovalMode.DEFAULT); - }); - - it('should show a warning when trying to enable privileged modes', () => { - // Mock the error thrown by setApprovalMode + it('should show a warning when cycling from DEFAULT to AUTO_EDIT', () => { const errorMessage = 'Cannot enable privileged approval modes in an untrusted folder.'; + mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT); mockConfigInstance.setApprovalMode.mockImplementation(() => { throw new Error(errorMessage); }); @@ -438,11 +329,13 @@ describe('useAutoAcceptIndicator', () => { }), ); - // Try to enable YOLO mode act(() => { - capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key); + capturedUseKeypressHandler({ name: 'tab', shift: true } as Key); }); + expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.AUTO_EDIT, + ); expect(mockAddItem).toHaveBeenCalledWith( { type: MessageType.INFO, @@ -450,15 +343,33 @@ describe('useAutoAcceptIndicator', () => { }, expect.any(Number), ); + }); - // Try to enable AUTO_EDIT mode - act(() => { - capturedUseKeypressHandler({ - name: 'tab', - shift: true, - } as Key); + it('should show a warning when cycling from AUTO_EDIT to YOLO', () => { + const errorMessage = + 'Cannot enable privileged approval modes in an untrusted folder.'; + mockConfigInstance.getApprovalMode.mockReturnValue( + ApprovalMode.AUTO_EDIT, + ); + mockConfigInstance.setApprovalMode.mockImplementation(() => { + throw new Error(errorMessage); }); + const mockAddItem = vi.fn(); + renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + addItem: mockAddItem, + }), + ); + + act(() => { + capturedUseKeypressHandler({ name: 'tab', shift: true } as Key); + }); + + expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.YOLO, + ); expect(mockAddItem).toHaveBeenCalledWith( { type: MessageType.INFO, @@ -466,8 +377,27 @@ describe('useAutoAcceptIndicator', () => { }, expect.any(Number), ); + }); - expect(mockAddItem).toHaveBeenCalledTimes(2); + it('should cycle from YOLO to PLAN when Shift+Tab is pressed', () => { + mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO); + const mockAddItem = vi.fn(); + renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + addItem: mockAddItem, + }), + ); + + act(() => { + capturedUseKeypressHandler({ name: 'tab', shift: true } as Key); + }); + + expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.PLAN, + ); + expect(mockConfigInstance.getApprovalMode()).toBe(ApprovalMode.PLAN); + expect(mockAddItem).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts index 0c534338..34cb4b05 100644 --- a/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts +++ b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ApprovalMode, type Config } from '@qwen-code/qwen-code-core'; +import { + type ApprovalMode, + APPROVAL_MODES, + type Config, +} from '@qwen-code/qwen-code-core'; import { useEffect, useState } from 'react'; import { useKeypress } from './useKeypress.js'; import type { HistoryItemWithoutId } from '../types.js'; @@ -29,34 +33,28 @@ export function useAutoAcceptIndicator({ useKeypress( (key) => { - let nextApprovalMode: ApprovalMode | undefined; - - if (key.ctrl && key.name === 'y') { - nextApprovalMode = - config.getApprovalMode() === ApprovalMode.YOLO - ? ApprovalMode.DEFAULT - : ApprovalMode.YOLO; - } else if (key.shift && key.name === 'tab') { - nextApprovalMode = - config.getApprovalMode() === ApprovalMode.AUTO_EDIT - ? ApprovalMode.DEFAULT - : ApprovalMode.AUTO_EDIT; + if (!(key.shift && key.name === 'tab')) { + return; } - if (nextApprovalMode) { - try { - config.setApprovalMode(nextApprovalMode); - // Update local state immediately for responsiveness - setShowAutoAcceptIndicator(nextApprovalMode); - } catch (e) { - addItem( - { - type: MessageType.INFO, - text: (e as Error).message, - }, - Date.now(), - ); - } + const currentMode = config.getApprovalMode(); + const currentIndex = APPROVAL_MODES.indexOf(currentMode); + const nextIndex = + currentIndex === -1 ? 0 : (currentIndex + 1) % APPROVAL_MODES.length; + const nextApprovalMode = APPROVAL_MODES[nextIndex]; + + try { + config.setApprovalMode(nextApprovalMode); + // Update local state immediately for responsiveness + setShowAutoAcceptIndicator(nextApprovalMode); + } catch (e) { + addItem( + { + type: MessageType.INFO, + text: (e as Error).message, + }, + Date.now(), + ); } }, { isActive: true }, diff --git a/packages/cli/src/ui/hooks/useVisionAutoSwitch.test.ts b/packages/cli/src/ui/hooks/useVisionAutoSwitch.test.ts index c04a2404..782986ce 100644 --- a/packages/cli/src/ui/hooks/useVisionAutoSwitch.test.ts +++ b/packages/cli/src/ui/hooks/useVisionAutoSwitch.test.ts @@ -9,6 +9,27 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import type { Part, PartListUnion } from '@google/genai'; import { AuthType, type Config, ApprovalMode } from '@qwen-code/qwen-code-core'; + +// Mock the image format functions from core package +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + isSupportedImageMimeType: vi.fn((mimeType: string) => + [ + 'image/png', + 'image/jpeg', + 'image/jpg', + 'image/gif', + 'image/webp', + ].includes(mimeType), + ), + getUnsupportedImageFormatWarning: vi.fn( + () => + 'Only the following image formats are supported: BMP, JPEG, JPG, PNG, TIFF, WEBP, HEIC. Other formats may not work as expected.', + ), + }; +}); import { shouldOfferVisionSwitch, processVisionSwitchOutcome, diff --git a/packages/cli/src/zed-integration/zedIntegration.ts b/packages/cli/src/zed-integration/zedIntegration.ts index 7fdadc5d..8e39d3f0 100644 --- a/packages/cli/src/zed-integration/zedIntegration.ts +++ b/packages/cli/src/zed-integration/zedIntegration.ts @@ -883,6 +883,16 @@ function toToolCallContent(toolResult: ToolResult): acp.ToolCallContent | null { type: 'content', content: { type: 'text', text: todoText }, }; + } else if ( + 'type' in toolResult.returnDisplay && + toolResult.returnDisplay.type === 'plan_summary' + ) { + const planDisplay = toolResult.returnDisplay; + const planText = `${planDisplay.message}\n\n${planDisplay.plan}`; + return { + type: 'content', + content: { type: 'text', text: planText }, + }; } else if ('fileDiff' in toolResult.returnDisplay) { // Handle FileDiff return { @@ -954,6 +964,15 @@ function toPermissionOptions( }, ...basicPermissionOptions, ]; + case 'plan': + return [ + { + optionId: ToolConfirmationOutcome.ProceedAlways, + name: `Always Allow Plans`, + kind: 'allow_always', + }, + ...basicPermissionOptions, + ]; default: { const unreachable: never = confirmation; throw new Error(`Unexpected: ${unreachable}`); diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 5d83ce20..e47a2f74 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -710,6 +710,18 @@ describe('setApprovalMode with folder trust', () => { expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); }); + it('should NOT throw an error when setting PLAN mode in an untrusted folder', () => { + const config = new Config({ + sessionId: 'test', + targetDir: '.', + debugMode: false, + model: 'test-model', + cwd: '.', + trustedFolder: false, // Untrusted + }); + expect(() => config.setApprovalMode(ApprovalMode.PLAN)).not.toThrow(); + }); + it('should NOT throw an error when setting any mode in a trusted folder', () => { const config = new Config({ sessionId: 'test', @@ -722,6 +734,7 @@ describe('setApprovalMode with folder trust', () => { expect(() => config.setApprovalMode(ApprovalMode.YOLO)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.AUTO_EDIT)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); + expect(() => config.setApprovalMode(ApprovalMode.PLAN)).not.toThrow(); }); it('should NOT throw an error when setting any mode if trustedFolder is undefined', () => { @@ -736,6 +749,7 @@ describe('setApprovalMode with folder trust', () => { expect(() => config.setApprovalMode(ApprovalMode.YOLO)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.AUTO_EDIT)).not.toThrow(); expect(() => config.setApprovalMode(ApprovalMode.DEFAULT)).not.toThrow(); + expect(() => config.setApprovalMode(ApprovalMode.PLAN)).not.toThrow(); }); describe('Model Switch Logging', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9ff19919..5065eb17 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -33,6 +33,7 @@ import { import { logCliConfiguration, logIdeConnection } from '../telemetry/loggers.js'; import { IdeConnectionEvent, IdeConnectionType } from '../telemetry/types.js'; import { EditTool } from '../tools/edit.js'; +import { ExitPlanModeTool } from '../tools/exitPlanMode.js'; import { GlobTool } from '../tools/glob.js'; import { GrepTool } from '../tools/grep.js'; import { LSTool } from '../tools/ls.js'; @@ -62,11 +63,14 @@ import { Logger, type ModelSwitchEvent } from '../core/logger.js'; export type { AnyToolInvocation, MCPOAuthConfig }; export enum ApprovalMode { + PLAN = 'plan', DEFAULT = 'default', - AUTO_EDIT = 'autoEdit', + AUTO_EDIT = 'auto-edit', YOLO = 'yolo', } +export const APPROVAL_MODES = Object.values(ApprovalMode); + export interface AccessibilitySettings { disableLoadingPhrases?: boolean; screenReader?: boolean; @@ -700,7 +704,11 @@ export class Config { } setApprovalMode(mode: ApprovalMode): void { - if (this.isTrustedFolder() === false && mode !== ApprovalMode.DEFAULT) { + if ( + this.isTrustedFolder() === false && + mode !== ApprovalMode.DEFAULT && + mode !== ApprovalMode.PLAN + ) { throw new Error( 'Cannot enable privileged approval modes in an untrusted folder.', ); @@ -1043,11 +1051,12 @@ export class Config { registerCoreTool(GlobTool, this); registerCoreTool(EditTool, this); registerCoreTool(WriteFileTool, this); - registerCoreTool(WebFetchTool, this); registerCoreTool(ReadManyFilesTool, this); registerCoreTool(ShellTool, this); registerCoreTool(MemoryTool); registerCoreTool(TodoWriteTool, this); + registerCoreTool(ExitPlanModeTool, this); + registerCoreTool(WebFetchTool, this); // Conditionally register web search tool only if Tavily API key is set if (this.getTavilyApiKey()) { registerCoreTool(WebSearchTool, this); diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index bb37c77e..3501ab7b 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -23,6 +23,7 @@ import type { } from '@google/genai'; import { GoogleGenAI } from '@google/genai'; import { findIndexAfterFraction, GeminiClient } from './client.js'; +import { getPlanModeSystemReminder } from './prompts.js'; import { AuthType, type ContentGenerator, @@ -50,6 +51,10 @@ const mockGenerateContentFn = vi.fn(); const mockEmbedContentFn = vi.fn(); const mockTurnRunFn = vi.fn(); +let ApprovalModeEnum: typeof import('../config/config.js').ApprovalMode; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +let mockConfigObject: any; + vi.mock('@google/genai'); vi.mock('./turn', async (importOriginal) => { const actual = await importOriginal(); @@ -178,6 +183,12 @@ describe('Gemini Client (client.ts)', () => { beforeEach(async () => { vi.resetAllMocks(); + ApprovalModeEnum = ( + await vi.importActual( + '../config/config.js', + ) + ).ApprovalMode; + // Disable 429 simulation for tests setSimulate429(false); @@ -229,7 +240,7 @@ describe('Gemini Client (client.ts)', () => { const mockSubagentManager = { listSubagents: vi.fn().mockResolvedValue([]), }; - const mockConfigObject = { + mockConfigObject = { getContentGeneratorConfig: vi .fn() .mockReturnValue(contentGeneratorConfig), @@ -252,6 +263,7 @@ describe('Gemini Client (client.ts)', () => { getNoBrowser: vi.fn().mockReturnValue(false), getSystemPromptMappings: vi.fn().mockReturnValue(undefined), getUsageStatisticsEnabled: vi.fn().mockReturnValue(true), + getApprovalMode: vi.fn().mockReturnValue(ApprovalModeEnum.DEFAULT), getIdeModeFeature: vi.fn().mockReturnValue(false), getIdeMode: vi.fn().mockReturnValue(true), getDebugMode: vi.fn().mockReturnValue(false), @@ -948,6 +960,42 @@ describe('Gemini Client (client.ts)', () => { }); describe('sendMessageStream', () => { + it('injects a plan mode reminder before user queries when approval mode is PLAN', async () => { + const mockStream = (async function* () {})(); + mockTurnRunFn.mockReturnValue(mockStream); + + mockConfigObject.getApprovalMode.mockReturnValue(ApprovalModeEnum.PLAN); + + const mockChat: Partial = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + }; + client['chat'] = mockChat as GeminiChat; + + const mockGenerator: Partial = { + countTokens: vi.fn().mockResolvedValue({ totalTokens: 0 }), + generateContent: mockGenerateContentFn, + }; + client['contentGenerator'] = mockGenerator as ContentGenerator; + + const stream = client.sendMessageStream( + 'Plan mode test', + new AbortController().signal, + 'prompt-plan-1', + ); + + await fromAsync(stream); + + expect(mockTurnRunFn).toHaveBeenCalledWith( + [getPlanModeSystemReminder(), 'Plan mode test'], + expect.any(Object), + ); + + mockConfigObject.getApprovalMode.mockReturnValue( + ApprovalModeEnum.DEFAULT, + ); + }); + it('emits a compression event when the context was automatically compressed', async () => { // Arrange const mockStream = (async function* () { @@ -1176,10 +1224,7 @@ ${JSON.stringify( // Assert expect(ideContext.getIdeContext).toHaveBeenCalled(); - expect(mockTurnRunFn).toHaveBeenCalledWith( - initialRequest, - expect.any(Object), - ); + expect(mockTurnRunFn).toHaveBeenCalledWith(['Hi'], expect.any(Object)); }); it('should add context if ideMode is enabled and there is one active file', async () => { diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 8b965001..05be91b6 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -17,6 +17,7 @@ import type { import { ProxyAgent, setGlobalDispatcher } from 'undici'; import type { UserTierId } from '../code_assist/types.js'; import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../config/config.js'; import { DEFAULT_GEMINI_FLASH_MODEL } from '../config/models.js'; import type { File, IdeContext } from '../ide/ideContext.js'; import { ideContext } from '../ide/ideContext.js'; @@ -40,6 +41,7 @@ import { getFunctionCalls } from '../utils/generateContentResponseUtilities.js'; import { isFunctionResponse } from '../utils/messageInspectors.js'; import { checkNextSpeaker } from '../utils/nextSpeakerChecker.js'; import { retryWithBackoff } from '../utils/retry.js'; +import { flatMapTextParts } from '../utils/partUtils.js'; import type { ContentGenerator, ContentGeneratorConfig, @@ -50,6 +52,8 @@ import { getCompressionPrompt, getCoreSystemPrompt, getCustomSystemPrompt, + getPlanModeSystemReminder, + getSubagentSystemReminder, } from './prompts.js'; import { tokenLimit } from './tokenLimits.js'; import type { ChatCompressionInfo, ServerGeminiStreamEvent } from './turn.js'; @@ -598,24 +602,6 @@ export class GeminiClient { this.forceFullIdeContext = false; } - if (isNewPrompt) { - const taskTool = this.config.getToolRegistry().getTool(TaskTool.Name); - const subagents = ( - await this.config.getSubagentManager().listSubagents() - ).filter((subagent) => subagent.level !== 'builtin'); - - if (taskTool && subagents.length > 0) { - this.getChat().addHistory({ - role: 'user', - parts: [ - { - text: `You have powerful specialized agents at your disposal, available agent types are: ${subagents.map((subagent) => subagent.name).join(', ')}. PROACTIVELY use the ${TaskTool.Name} tool to delegate user's task to appropriate agent when user's task matches agent capabilities. Ignore this message if user's task is not relevant to any agent. This message is for internal use only. Do not mention this to user in your response.`, - }, - ], - }); - } - } - const turn = new Turn(this.getChat(), prompt_id); if (!this.config.getSkipLoopDetection()) { @@ -626,7 +612,30 @@ export class GeminiClient { } } - const resultStream = turn.run(request, signal); + // append system reminders to the request + let requestToSent = await flatMapTextParts(request, async (text) => [text]); + if (isNewPrompt) { + const systemReminders = []; + + // add subagent system reminder if there are subagents + const hasTaskTool = this.config.getToolRegistry().getTool(TaskTool.Name); + const subagents = (await this.config.getSubagentManager().listSubagents()) + .filter((subagent) => subagent.level !== 'builtin') + .map((subagent) => subagent.name); + + if (hasTaskTool && subagents.length > 0) { + systemReminders.push(getSubagentSystemReminder(subagents)); + } + + // add plan mode system reminder if approval mode is plan + if (this.config.getApprovalMode() === ApprovalMode.PLAN) { + systemReminders.push(getPlanModeSystemReminder()); + } + + requestToSent = [...systemReminders, ...requestToSent]; + } + + const resultStream = turn.run(requestToSent, signal); for await (const event of resultStream) { if (!this.config.getSkipLoopDetection()) { if (this.loopDetector.addAndCheck(event)) { diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 10d4f181..7834bcb6 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -10,11 +10,13 @@ import { describe, expect, it, vi } from 'vitest'; import type { Config, ToolCallConfirmationDetails, + ToolCallRequestInfo, ToolConfirmationPayload, ToolInvocation, ToolResult, ToolResultDisplay, ToolRegistry, + SuccessfulToolCall, } from '../index.js'; import { ApprovalMode, @@ -24,11 +26,16 @@ import { ToolConfirmationOutcome, } from '../index.js'; import { MockModifiableTool, MockTool } from '../test-utils/tools.js'; -import type { ToolCall, WaitingToolCall } from './coreToolScheduler.js'; +import type { + ToolCall, + WaitingToolCall, + ErroredToolCall, +} from './coreToolScheduler.js'; import { CoreToolScheduler, convertToFunctionResponse, } from './coreToolScheduler.js'; +import { getPlanModeSystemReminder } from './prompts.js'; class TestApprovalTool extends BaseDeclarativeTool<{ id: string }, ToolResult> { static readonly Name = 'testApprovalTool'; @@ -101,6 +108,49 @@ class TestApprovalInvocation extends BaseToolInvocation< } } +class SimpleToolInvocation extends BaseToolInvocation< + Record, + ToolResult +> { + constructor( + params: Record, + private readonly executeImpl: () => Promise | ToolResult, + ) { + super(params); + } + + getDescription(): string { + return 'simple tool invocation'; + } + + async execute(): Promise { + return await Promise.resolve(this.executeImpl()); + } +} + +class SimpleTool extends BaseDeclarativeTool< + Record, + ToolResult +> { + constructor( + name: string, + kind: Kind, + private readonly executeImpl: () => Promise | ToolResult, + ) { + super(name, name, 'Simple test tool', kind, { + type: 'object', + properties: {}, + additionalProperties: true, + }); + } + + protected createInvocation( + params: Record, + ): ToolInvocation, ToolResult> { + return new SimpleToolInvocation(params, this.executeImpl); + } +} + async function waitForStatus( onToolCallsUpdate: Mock, status: 'awaiting_approval' | 'executing' | 'success' | 'error' | 'cancelled', @@ -197,6 +247,249 @@ describe('CoreToolScheduler', () => { expect(completedCalls[0].status).toBe('cancelled'); }); + describe('plan mode enforcement', () => { + it('returns plan reminder and skips execution for edit tools', async () => { + const executeSpy = vi.fn().mockResolvedValue({ + llmContent: 'should not execute', + returnDisplay: 'should not execute', + }); + // Use MockTool with shouldConfirm=true to simulate a tool that requires confirmation + const tool = new MockTool('write_file'); + tool.shouldConfirm = true; + tool.executeFn = executeSpy; + + const mockToolRegistry = { + getTool: vi.fn().mockReturnValue(tool), + getAllToolNames: vi.fn().mockReturnValue([tool.name]), + } as unknown as ToolRegistry; + + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + + const mockConfig = { + getSessionId: () => 'plan-session', + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.PLAN), + getAllowedTools: () => [], + getContentGeneratorConfig: () => ({ + model: 'test-model', + authType: 'oauth-personal', + }), + getToolRegistry: () => mockToolRegistry, + } as unknown as Config; + + const scheduler = new CoreToolScheduler({ + config: mockConfig, + onAllToolCallsComplete, + onToolCallsUpdate, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const request: ToolCallRequestInfo = { + callId: 'plan-1', + name: 'write_file', + args: {}, + isClientInitiated: false, + prompt_id: 'prompt-plan', + }; + + await scheduler.schedule([request], new AbortController().signal); + + const errorCall = (await waitForStatus( + onToolCallsUpdate, + 'error', + )) as ErroredToolCall; + + expect(executeSpy).not.toHaveBeenCalled(); + expect( + errorCall.response.responseParts[0]?.functionResponse?.response?.[ + 'output' + ], + ).toBe(getPlanModeSystemReminder()); + expect(errorCall.response.resultDisplay).toContain('Plan mode'); + }); + + it('allows read tools to execute in plan mode', async () => { + const executeSpy = vi.fn().mockResolvedValue({ + llmContent: 'read ok', + returnDisplay: 'read ok', + }); + const tool = new SimpleTool('read_file', Kind.Read, executeSpy); + + const mockToolRegistry = { + getTool: vi.fn().mockReturnValue(tool), + getAllToolNames: vi.fn().mockReturnValue([tool.name]), + } as unknown as ToolRegistry; + + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + + const mockConfig = { + getSessionId: () => 'plan-session', + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.PLAN), + getAllowedTools: () => [], + getContentGeneratorConfig: () => ({ + model: 'test-model', + authType: 'oauth-personal', + }), + getToolRegistry: () => mockToolRegistry, + } as unknown as Config; + + const scheduler = new CoreToolScheduler({ + config: mockConfig, + onAllToolCallsComplete, + onToolCallsUpdate, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const request: ToolCallRequestInfo = { + callId: 'plan-2', + name: tool.name, + args: {}, + isClientInitiated: false, + prompt_id: 'prompt-plan', + }; + + await scheduler.schedule([request], new AbortController().signal); + + const successCall = (await waitForStatus( + onToolCallsUpdate, + 'success', + )) as SuccessfulToolCall; + + expect(executeSpy).toHaveBeenCalledTimes(1); + expect( + successCall.response.responseParts[0]?.functionResponse?.response?.[ + 'output' + ], + ).toBe('read ok'); + }); + + it('enforces shell command restrictions in plan mode', async () => { + const executeSpyAllowed = vi.fn().mockResolvedValue({ + llmContent: 'shell ok', + returnDisplay: 'shell ok', + }); + const allowedTool = new SimpleTool( + 'run_shell_command', + Kind.Execute, + executeSpyAllowed, + ); + + const allowedToolRegistry = { + getTool: vi.fn().mockReturnValue(allowedTool), + getAllToolNames: vi.fn().mockReturnValue([allowedTool.name]), + } as unknown as ToolRegistry; + + const allowedConfig = { + getSessionId: () => 'plan-session', + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.PLAN), + getAllowedTools: () => [], + getContentGeneratorConfig: () => ({ + model: 'test-model', + authType: 'oauth-personal', + }), + getToolRegistry: () => allowedToolRegistry, + } as unknown as Config; + + const allowedUpdates = vi.fn(); + const allowedScheduler = new CoreToolScheduler({ + config: allowedConfig, + onAllToolCallsComplete: vi.fn(), + onToolCallsUpdate: allowedUpdates, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const allowedRequest: ToolCallRequestInfo = { + callId: 'plan-shell-allowed', + name: allowedTool.name, + args: { command: 'ls -la' }, + isClientInitiated: false, + prompt_id: 'prompt-plan', + }; + + await allowedScheduler.schedule( + [allowedRequest], + new AbortController().signal, + ); + + await waitForStatus(allowedUpdates, 'success'); + expect(executeSpyAllowed).toHaveBeenCalledTimes(1); + + const executeSpyBlocked = vi.fn().mockResolvedValue({ + llmContent: 'blocked', + returnDisplay: 'blocked', + }); + // Use MockTool with shouldConfirm=true to simulate a shell tool that requires confirmation + const blockedTool = new MockTool('run_shell_command'); + blockedTool.shouldConfirm = true; + blockedTool.executeFn = executeSpyBlocked; + + const blockedToolRegistry = { + getTool: vi.fn().mockReturnValue(blockedTool), + getAllToolNames: vi.fn().mockReturnValue([blockedTool.name]), + } as unknown as ToolRegistry; + + const blockedConfig = { + getSessionId: () => 'plan-session', + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.PLAN), + getAllowedTools: () => [], + getContentGeneratorConfig: () => ({ + model: 'test-model', + authType: 'oauth-personal', + }), + getToolRegistry: () => blockedToolRegistry, + } as unknown as Config; + + const blockedUpdates = vi.fn(); + const blockedScheduler = new CoreToolScheduler({ + config: blockedConfig, + onAllToolCallsComplete: vi.fn(), + onToolCallsUpdate: blockedUpdates, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const blockedRequest: ToolCallRequestInfo = { + callId: 'plan-shell-blocked', + name: 'run_shell_command', + args: { command: 'rm -rf tmp' }, + isClientInitiated: false, + prompt_id: 'prompt-plan', + }; + + await blockedScheduler.schedule( + [blockedRequest], + new AbortController().signal, + ); + + const blockedCall = (await waitForStatus( + blockedUpdates, + 'error', + )) as ErroredToolCall; + expect(executeSpyBlocked).not.toHaveBeenCalled(); + expect( + blockedCall.response.responseParts[0]?.functionResponse?.response?.[ + 'output' + ], + ).toBe(getPlanModeSystemReminder()); + const observedStatuses = blockedUpdates.mock.calls + .flatMap((call) => call[0] as ToolCall[]) + .map((tc) => tc.status); + expect(observedStatuses).not.toContain('awaiting_approval'); + }); + }); + describe('getToolSuggestion', () => { it('should suggest the top N closest tool names for a typo', () => { // Create mocked tool registry diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 6278a009..60e2b04f 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -34,6 +34,7 @@ import { import * as Diff from 'diff'; import { doesToolInvocationMatch } from '../utils/tool-utils.js'; import levenshtein from 'fast-levenshtein'; +import { getPlanModeSystemReminder } from './prompts.js'; export type ValidatingToolCall = { status: 'validating'; @@ -674,7 +675,27 @@ export class CoreToolScheduler { } const allowedTools = this.config.getAllowedTools() || []; - if ( + const isPlanMode = + this.config.getApprovalMode() === ApprovalMode.PLAN; + const isExitPlanModeTool = reqInfo.name === 'exit_plan_mode'; + + if (isPlanMode && !isExitPlanModeTool) { + if (confirmationDetails) { + this.setStatusInternal(reqInfo.callId, 'error', { + callId: reqInfo.callId, + responseParts: convertToFunctionResponse( + reqInfo.name, + reqInfo.callId, + getPlanModeSystemReminder(), + ), + resultDisplay: 'Plan mode blocked a non-read-only tool call.', + error: undefined, + errorType: undefined, + }); + } else { + this.setStatusInternal(reqInfo.callId, 'scheduled'); + } + } else if ( this.config.getApprovalMode() === ApprovalMode.YOLO || doesToolInvocationMatch(toolCall.tool, invocation, allowedTools) ) { diff --git a/packages/core/src/core/openaiContentGenerator/converter.ts b/packages/core/src/core/openaiContentGenerator/converter.ts index e273282f..8157fada 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.ts @@ -376,28 +376,22 @@ export class OpenAIContentConverter { parsedParts: Pick, ): OpenAI.Chat.ChatCompletionMessageParam | null { const { textParts, mediaParts } = parsedParts; - const combinedText = textParts.join(''); + const content = textParts.map((text) => ({ type: 'text' as const, text })); // If no media parts, return simple text message if (mediaParts.length === 0) { - return combinedText ? { role, content: combinedText } : null; + return content.length > 0 ? { role, content } : null; } // For assistant messages with media, convert to text only // since OpenAI assistant messages don't support media content arrays if (role === 'assistant') { - return combinedText - ? { role: 'assistant' as const, content: combinedText } + return content.length > 0 + ? { role: 'assistant' as const, content } : null; } - // Create multimodal content array for user messages - const contentArray: OpenAI.Chat.ChatCompletionContentPart[] = []; - - // Add text content - if (combinedText) { - contentArray.push({ type: 'text', text: combinedText }); - } + const contentArray: OpenAI.Chat.ChatCompletionContentPart[] = [...content]; // Add media content for (const mediaPart of mediaParts) { @@ -405,14 +399,14 @@ export class OpenAIContentConverter { if (mediaPart.fileUri) { // For file URIs, use the URI directly contentArray.push({ - type: 'image_url', + type: 'image_url' as const, image_url: { url: mediaPart.fileUri }, }); } else if (mediaPart.data) { // For inline data, create data URL const dataUrl = `data:${mediaPart.mimeType};base64,${mediaPart.data}`; contentArray.push({ - type: 'image_url', + type: 'image_url' as const, image_url: { url: dataUrl }, }); } @@ -421,7 +415,7 @@ export class OpenAIContentConverter { const format = this.getAudioFormat(mediaPart.mimeType); if (format) { contentArray.push({ - type: 'input_audio', + type: 'input_audio' as const, input_audio: { data: mediaPart.data, format: format as 'wav' | 'mp3', diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index a5bb5157..af60aa77 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -5,7 +5,12 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { getCoreSystemPrompt, getCustomSystemPrompt } from './prompts.js'; +import { + getCoreSystemPrompt, + getCustomSystemPrompt, + getSubagentSystemReminder, + getPlanModeSystemReminder, +} from './prompts.js'; import { isGitRepository } from '../utils/gitUtils.js'; import fs from 'node:fs'; import os from 'node:os'; @@ -519,3 +524,53 @@ describe('getCustomSystemPrompt', () => { expect(result).toContain('---'); }); }); + +describe('getSubagentSystemReminder', () => { + it('should format single agent type correctly', () => { + const result = getSubagentSystemReminder(['python']); + + expect(result).toMatch(/^.*<\/system-reminder>$/); + expect(result).toContain('available agent types are: python'); + expect(result).toContain('PROACTIVELY use the'); + }); + + it('should join multiple agent types with commas', () => { + const result = getSubagentSystemReminder(['python', 'web', 'analysis']); + + expect(result).toContain( + 'available agent types are: python, web, analysis', + ); + }); + + it('should handle empty array', () => { + const result = getSubagentSystemReminder([]); + + expect(result).toContain('available agent types are: '); + expect(result).toContain(''); + }); +}); + +describe('getPlanModeSystemReminder', () => { + it('should return plan mode system reminder with proper structure', () => { + const result = getPlanModeSystemReminder(); + + expect(result).toMatch(/^[\s\S]*<\/system-reminder>$/); + expect(result).toContain('Plan mode is active'); + expect(result).toContain('MUST NOT make any edits'); + }); + + it('should include workflow instructions', () => { + const result = getPlanModeSystemReminder(); + + expect(result).toContain("1. Answer the user's query comprehensively"); + expect(result).toContain("2. When you're done researching"); + expect(result).toContain('exit_plan_mode tool'); + }); + + it('should be deterministic', () => { + const result1 = getPlanModeSystemReminder(); + const result2 = getPlanModeSystemReminder(); + + expect(result1).toBe(result2); + }); +}); diff --git a/packages/core/src/core/prompts.ts b/packages/core/src/core/prompts.ts index f08cbf75..33032ede 100644 --- a/packages/core/src/core/prompts.ts +++ b/packages/core/src/core/prompts.ts @@ -832,3 +832,53 @@ function getToolCallExamples(model?: string): string { return generalToolCallExamples; } + +/** + * Generates a system reminder message about available subagents for the AI assistant. + * + * This function creates an internal system message that informs the AI about specialized + * agents it can delegate tasks to. The reminder encourages proactive use of the TASK tool + * when user requests match agent capabilities. + * + * @param agentTypes - Array of available agent type names (e.g., ['python', 'web', 'analysis']) + * @returns A formatted system reminder string wrapped in XML tags for internal AI processing + * + * @example + * ```typescript + * const reminder = getSubagentSystemReminder(['python', 'web']); + * // Returns: "You have powerful specialized agents..." + * ``` + */ +export function getSubagentSystemReminder(agentTypes: string[]): string { + return `You have powerful specialized agents at your disposal, available agent types are: ${agentTypes.join(', ')}. PROACTIVELY use the ${ToolNames.TASK} tool to delegate user's task to appropriate agent when user's task matches agent capabilities. Ignore this message if user's task is not relevant to any agent. This message is for internal use only. Do not mention this to user in your response.`; +} + +/** + * Generates a system reminder message for plan mode operation. + * + * This function creates an internal system message that enforces plan mode constraints, + * preventing the AI from making any modifications to the system until the user confirms + * the proposed plan. It overrides other instructions to ensure read-only behavior. + * + * @returns A formatted system reminder string that enforces plan mode restrictions + * + * @example + * ```typescript + * const reminder = getPlanModeSystemReminder(); + * // Returns: "Plan mode is active..." + * ``` + * + * @remarks + * Plan mode ensures the AI will: + * - Only perform read-only operations (research, analysis) + * - Present a comprehensive plan via ExitPlanMode tool + * - Wait for user confirmation before making any changes + * - Override any other instructions that would modify system state + */ +export function getPlanModeSystemReminder(): string { + return ` +Plan mode is active. The user indicated that they do not want you to execute yet -- you MUST NOT make any edits, run any non-readonly tools (including changing configs or making commits), or otherwise make any changes to the system. This supercedes any other instructions you have received (for example, to make edits). Instead, you should: +1. Answer the user's query comprehensively +2. When you're done researching, present your plan by calling the ${ToolNames.EXIT_PLAN_MODE} tool, which will prompt the user to confirm the plan. Do NOT make any file changes or run any tools that modify the system state in any way until the user has confirmed the plan. +`; +} diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts new file mode 100644 index 00000000..59248b5b --- /dev/null +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -0,0 +1,292 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ExitPlanModeTool, type ExitPlanModeParams } from './exitPlanMode.js'; +import { ApprovalMode, type Config } from '../config/config.js'; +import { ToolConfirmationOutcome } from './tools.js'; + +describe('ExitPlanModeTool', () => { + let tool: ExitPlanModeTool; + let mockConfig: Config; + let approvalMode: ApprovalMode; + + beforeEach(() => { + approvalMode = ApprovalMode.PLAN; + mockConfig = { + getApprovalMode: vi.fn(() => approvalMode), + setApprovalMode: vi.fn((mode: ApprovalMode) => { + approvalMode = mode; + }), + } as unknown as Config; + + tool = new ExitPlanModeTool(mockConfig); + }); + + describe('constructor and metadata', () => { + it('should have correct tool name', () => { + expect(tool.name).toBe('exit_plan_mode'); + expect(ExitPlanModeTool.Name).toBe('exit_plan_mode'); + }); + + it('should have correct display name', () => { + expect(tool.displayName).toBe('ExitPlanMode'); + }); + + it('should have correct kind', () => { + expect(tool.kind).toBe('think'); + }); + + it('should have correct schema', () => { + expect(tool.schema).toEqual({ + name: 'exit_plan_mode', + description: expect.stringContaining( + 'Use this tool when you are in plan mode', + ), + parametersJsonSchema: { + type: 'object', + properties: { + plan: { + type: 'string', + description: expect.stringContaining('The plan you came up with'), + }, + }, + required: ['plan'], + additionalProperties: false, + $schema: 'http://json-schema.org/draft-07/schema#', + }, + }); + }); + }); + + describe('validateToolParams', () => { + it('should accept valid parameters', () => { + const params: ExitPlanModeParams = { + plan: 'This is a comprehensive plan for the implementation.', + }; + + const result = tool.validateToolParams(params); + expect(result).toBeNull(); + }); + + it('should reject missing plan parameter', () => { + const params = {} as ExitPlanModeParams; + + const result = tool.validateToolParams(params); + expect(result).toBe('Parameter "plan" must be a non-empty string.'); + }); + + it('should reject empty plan parameter', () => { + const params: ExitPlanModeParams = { + plan: '', + }; + + const result = tool.validateToolParams(params); + expect(result).toBe('Parameter "plan" must be a non-empty string.'); + }); + + it('should reject whitespace-only plan parameter', () => { + const params: ExitPlanModeParams = { + plan: ' \n\t ', + }; + + const result = tool.validateToolParams(params); + expect(result).toBe('Parameter "plan" must be a non-empty string.'); + }); + + it('should reject non-string plan parameter', () => { + const params = { + plan: 123, + } as unknown as ExitPlanModeParams; + + const result = tool.validateToolParams(params); + expect(result).toBe('Parameter "plan" must be a non-empty string.'); + }); + }); + + describe('tool execution', () => { + it('should execute successfully through tool interface after approval', async () => { + const params: ExitPlanModeParams = { + plan: 'This is my implementation plan:\n1. Step 1\n2. Step 2\n3. Step 3', + }; + const signal = new AbortController().signal; + + // Use the tool's public build method + const invocation = tool.build(params); + expect(invocation).toBeDefined(); + expect(invocation.params).toEqual(params); + + const confirmation = await invocation.shouldConfirmExecute(signal); + expect(confirmation).toMatchObject({ + type: 'plan', + title: 'Would you like to proceed?', + plan: params.plan, + }); + + if (confirmation) { + await confirmation.onConfirm(ToolConfirmationOutcome.ProceedOnce); + } + + const result = await invocation.execute(signal); + const expectedLlmMessage = + 'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.'; + + expect(result).toEqual({ + llmContent: expectedLlmMessage, + returnDisplay: { + type: 'plan_summary', + message: 'User approved the plan.', + plan: params.plan, + }, + }); + + expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.DEFAULT, + ); + expect(approvalMode).toBe(ApprovalMode.DEFAULT); + }); + + it('should request confirmation with plan details', async () => { + const params: ExitPlanModeParams = { + plan: 'Simple plan', + }; + const signal = new AbortController().signal; + + const invocation = tool.build(params); + const confirmation = await invocation.shouldConfirmExecute(signal); + + if (confirmation) { + expect(confirmation.type).toBe('plan'); + if (confirmation.type === 'plan') { + expect(confirmation.plan).toBe(params.plan); + } + + await confirmation.onConfirm(ToolConfirmationOutcome.ProceedAlways); + } + + expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.AUTO_EDIT, + ); + expect(approvalMode).toBe(ApprovalMode.AUTO_EDIT); + }); + + it('should remain in plan mode when confirmation is rejected', async () => { + const params: ExitPlanModeParams = { + plan: 'Remain in planning', + }; + const signal = new AbortController().signal; + + const invocation = tool.build(params); + const confirmation = await invocation.shouldConfirmExecute(signal); + + if (confirmation) { + await confirmation.onConfirm(ToolConfirmationOutcome.Cancel); + } + + const result = await invocation.execute(signal); + + expect(result).toEqual({ + llmContent: JSON.stringify({ + success: false, + plan: params.plan, + error: 'Plan execution was not approved. Remaining in plan mode.', + }), + returnDisplay: + 'Plan execution was not approved. Remaining in plan mode.', + }); + + expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( + ApprovalMode.PLAN, + ); + expect(approvalMode).toBe(ApprovalMode.PLAN); + }); + + it('should have correct description', () => { + const params: ExitPlanModeParams = { + plan: 'Test plan', + }; + + const invocation = tool.build(params); + expect(invocation.getDescription()).toBe( + 'Present implementation plan for user approval', + ); + }); + + it('should handle execution errors gracefully', async () => { + const params: ExitPlanModeParams = { + plan: 'Test plan', + }; + + const invocation = tool.build(params); + const confirmation = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + if (confirmation) { + // Don't approve the plan so we go through the rejection path + await confirmation.onConfirm(ToolConfirmationOutcome.Cancel); + } + + // Create a spy to simulate an error during the execution + const consoleSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + // Mock JSON.stringify to throw an error in the rejection path + const originalStringify = JSON.stringify; + vi.spyOn(JSON, 'stringify').mockImplementationOnce(() => { + throw new Error('JSON stringify error'); + }); + + const result = await invocation.execute(new AbortController().signal); + + expect(result).toEqual({ + llmContent: JSON.stringify({ + success: false, + error: 'Failed to present plan. Detail: JSON stringify error', + }), + returnDisplay: 'Error presenting plan: JSON stringify error', + }); + + expect(consoleSpy).toHaveBeenCalledWith( + '[ExitPlanModeTool] Error executing exit_plan_mode: JSON stringify error', + ); + + // Restore original JSON.stringify + JSON.stringify = originalStringify; + consoleSpy.mockRestore(); + }); + + it('should return empty tool locations', () => { + const params: ExitPlanModeParams = { + plan: 'Test plan', + }; + + const invocation = tool.build(params); + expect(invocation.toolLocations()).toEqual([]); + }); + }); + + describe('tool description', () => { + it('should contain usage guidelines', () => { + expect(tool.description).toContain( + 'Only use this tool when the task requires planning', + ); + expect(tool.description).toContain( + 'Do not use the exit plan mode tool because you are not planning', + ); + expect(tool.description).toContain( + 'Use the exit plan mode tool after you have finished planning', + ); + }); + + it('should contain examples', () => { + expect(tool.description).toContain( + 'Search for and understand the implementation of vim mode', + ); + expect(tool.description).toContain('Help me implement yank mode for vim'); + }); + }); +}); diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts new file mode 100644 index 00000000..4143d12f --- /dev/null +++ b/packages/core/src/tools/exitPlanMode.ts @@ -0,0 +1,191 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ToolPlanConfirmationDetails, ToolResult } from './tools.js'; +import { + BaseDeclarativeTool, + BaseToolInvocation, + Kind, + ToolConfirmationOutcome, +} from './tools.js'; +import type { FunctionDeclaration } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../config/config.js'; + +export interface ExitPlanModeParams { + plan: string; +} + +const exitPlanModeToolDescription = `Use this tool when you are in plan mode and have finished presenting your plan and are ready to code. This will prompt the user to exit plan mode. +IMPORTANT: Only use this tool when the task requires planning the implementation steps of a task that requires writing code. For research tasks where you're gathering information, searching files, reading files or in general trying to understand the codebase - do NOT use this tool. + +Eg. +1. Initial task: "Search for and understand the implementation of vim mode in the codebase" - Do not use the exit plan mode tool because you are not planning the implementation steps of a task. +2. Initial task: "Help me implement yank mode for vim" - Use the exit plan mode tool after you have finished planning the implementation steps of the task. +`; + +const exitPlanModeToolSchemaData: FunctionDeclaration = { + name: 'exit_plan_mode', + description: exitPlanModeToolDescription, + parametersJsonSchema: { + type: 'object', + properties: { + plan: { + type: 'string', + description: + 'The plan you came up with, that you want to run by the user for approval. Supports markdown. The plan should be pretty concise.', + }, + }, + required: ['plan'], + additionalProperties: false, + $schema: 'http://json-schema.org/draft-07/schema#', + }, +}; + +class ExitPlanModeToolInvocation extends BaseToolInvocation< + ExitPlanModeParams, + ToolResult +> { + private wasApproved = false; + + constructor( + private readonly config: Config, + params: ExitPlanModeParams, + ) { + super(params); + } + + getDescription(): string { + return 'Present implementation plan for user approval'; + } + + override async shouldConfirmExecute( + _abortSignal: AbortSignal, + ): Promise { + const details: ToolPlanConfirmationDetails = { + type: 'plan', + title: 'Would you like to proceed?', + plan: this.params.plan, + onConfirm: async (outcome: ToolConfirmationOutcome) => { + switch (outcome) { + case ToolConfirmationOutcome.ProceedAlways: + this.wasApproved = true; + this.setApprovalModeSafely(ApprovalMode.AUTO_EDIT); + break; + case ToolConfirmationOutcome.ProceedOnce: + this.wasApproved = true; + this.setApprovalModeSafely(ApprovalMode.DEFAULT); + break; + case ToolConfirmationOutcome.Cancel: + this.wasApproved = false; + this.setApprovalModeSafely(ApprovalMode.PLAN); + break; + default: + // Treat any other outcome as manual approval to preserve conservative behaviour. + this.wasApproved = true; + this.setApprovalModeSafely(ApprovalMode.DEFAULT); + break; + } + }, + }; + + return details; + } + + private setApprovalModeSafely(mode: ApprovalMode): void { + try { + this.config.setApprovalMode(mode); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + console.error( + `[ExitPlanModeTool] Failed to set approval mode to "${mode}": ${errorMessage}`, + ); + } + } + + async execute(_signal: AbortSignal): Promise { + const { plan } = this.params; + + try { + if (!this.wasApproved) { + const rejectionMessage = + 'Plan execution was not approved. Remaining in plan mode.'; + return { + llmContent: JSON.stringify({ + success: false, + plan, + error: rejectionMessage, + }), + returnDisplay: rejectionMessage, + }; + } + + const llmMessage = + 'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.'; + const displayMessage = 'User approved the plan.'; + + return { + llmContent: llmMessage, + returnDisplay: { + type: 'plan_summary', + message: displayMessage, + plan, + }, + }; + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + console.error( + `[ExitPlanModeTool] Error executing exit_plan_mode: ${errorMessage}`, + ); + return { + llmContent: JSON.stringify({ + success: false, + error: `Failed to present plan. Detail: ${errorMessage}`, + }), + returnDisplay: `Error presenting plan: ${errorMessage}`, + }; + } + } +} + +export class ExitPlanModeTool extends BaseDeclarativeTool< + ExitPlanModeParams, + ToolResult +> { + static readonly Name: string = exitPlanModeToolSchemaData.name!; + + constructor(private readonly config: Config) { + super( + ExitPlanModeTool.Name, + 'ExitPlanMode', + exitPlanModeToolDescription, + Kind.Think, + exitPlanModeToolSchemaData.parametersJsonSchema as Record< + string, + unknown + >, + ); + } + + override validateToolParams(params: ExitPlanModeParams): string | null { + // Validate plan parameter + if ( + !params.plan || + typeof params.plan !== 'string' || + params.plan.trim() === '' + ) { + return 'Parameter "plan" must be a non-empty string.'; + } + + return null; + } + + protected createInvocation(params: ExitPlanModeParams) { + return new ExitPlanModeToolInvocation(this.config, params); + } +} diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index f39ba0d2..0a5065ef 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -777,6 +777,19 @@ describe('ShellTool', () => { }); describe('shouldConfirmExecute', () => { + it('should not request confirmation for read-only commands', async () => { + const invocation = shellTool.build({ + command: 'ls -la', + is_background: false, + }); + + const confirmation = await invocation.shouldConfirmExecute( + new AbortController().signal, + ); + + expect(confirmation).toBe(false); + }); + it('should request confirmation for a new command and whitelist it on "Always"', async () => { const params = { command: 'npm install', is_background: false }; const invocation = shellTool.build(params); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index a195b113..f37c6c74 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -32,6 +32,7 @@ import { formatMemoryUsage } from '../utils/formatters.js'; import { getCommandRoots, isCommandAllowed, + isCommandNeedsPermission, stripShellWrapper, } from '../utils/shell-utils.js'; @@ -87,6 +88,11 @@ class ShellToolInvocation extends BaseToolInvocation< return false; // already approved and whitelisted } + const permissionCheck = isCommandNeedsPermission(command); + if (!permissionCheck.requiresPermission) { + return false; + } + const confirmationDetails: ToolExecuteConfirmationDetails = { type: 'exec', title: 'Confirm Shell Command', diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 443c44b5..71359f10 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -20,4 +20,5 @@ export const ToolNames = { TODO_WRITE: 'todo_write', MEMORY: 'save_memory', TASK: 'task', + EXIT_PLAN_MODE: 'exit_plan_mode', } as const; diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 5a5608a8..342e97ec 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -464,6 +464,7 @@ export type ToolResultDisplay = | string | FileDiff | TodoResultDisplay + | PlanResultDisplay | TaskResultDisplay; export interface FileDiff { @@ -490,6 +491,12 @@ export interface TodoResultDisplay { }>; } +export interface PlanResultDisplay { + type: 'plan_summary'; + message: string; + plan: string; +} + export interface ToolEditConfirmationDetails { type: 'edit'; title: string; @@ -541,7 +548,15 @@ export type ToolCallConfirmationDetails = | ToolEditConfirmationDetails | ToolExecuteConfirmationDetails | ToolMcpConfirmationDetails - | ToolInfoConfirmationDetails; + | ToolInfoConfirmationDetails + | ToolPlanConfirmationDetails; + +export interface ToolPlanConfirmationDetails { + type: 'plan'; + title: string; + plan: string; + onConfirm: (outcome: ToolConfirmationOutcome) => Promise; +} export enum ToolConfirmationOutcome { ProceedOnce = 'proceed_once', diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index 0be98082..27d9dc6e 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -10,9 +10,13 @@ import { Kind, type ToolInvocation, type ToolResult, + type ToolCallConfirmationDetails, + type ToolInfoConfirmationDetails, + ToolConfirmationOutcome, } from './tools.js'; import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../config/config.js'; import { getErrorMessage } from '../utils/errors.js'; interface TavilyResultItem { @@ -61,6 +65,26 @@ class WebSearchToolInvocation extends BaseToolInvocation< return `Searching the web for: "${this.params.query}"`; } + override async shouldConfirmExecute( + _abortSignal: AbortSignal, + ): Promise { + if (this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT) { + return false; + } + + const confirmationDetails: ToolInfoConfirmationDetails = { + type: 'info', + title: 'Confirm Web Search', + prompt: `Search the web for: "${this.params.query}"`, + onConfirm: async (outcome: ToolConfirmationOutcome) => { + if (outcome === ToolConfirmationOutcome.ProceedAlways) { + this.config.setApprovalMode(ApprovalMode.AUTO_EDIT); + } + }, + }; + return confirmationDetails; + } + async execute(signal: AbortSignal): Promise { const apiKey = this.config.getTavilyApiKey() || process.env['TAVILY_API_KEY']; diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 3c18fff4..e87c90bc 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -11,6 +11,7 @@ import { getCommandRoots, getShellConfiguration, isCommandAllowed, + isCommandNeedsPermission, stripShellWrapper, } from './shell-utils.js'; import type { Config } from '../config/config.js'; @@ -27,8 +28,10 @@ vi.mock('os', () => ({ })); const mockQuote = vi.hoisted(() => vi.fn()); +const mockParse = vi.hoisted(() => vi.fn()); vi.mock('shell-quote', () => ({ quote: mockQuote, + parse: mockParse, })); let config: Config; @@ -38,6 +41,7 @@ beforeEach(() => { mockQuote.mockImplementation((args: string[]) => args.map((arg) => `'${arg}'`).join(' '), ); + mockParse.mockImplementation((cmd: string) => cmd.split(' ')); config = { getCoreTools: () => [], getExcludeTools: () => [], @@ -436,3 +440,16 @@ describe('getShellConfiguration', () => { }); }); }); + +describe('isCommandNeedPermission', () => { + it('returns false for read-only commands', () => { + const result = isCommandNeedsPermission('ls'); + expect(result.requiresPermission).toBe(false); + }); + + it('returns true for mutating commands with reason', () => { + const result = isCommandNeedsPermission('rm -rf temp'); + expect(result.requiresPermission).toBe(true); + expect(result.reason).toContain('requires permission to execute'); + }); +}); diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index cedebe6c..e0e62ece 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -9,6 +9,7 @@ import type { Config } from '../config/config.js'; import os from 'node:os'; import { quote } from 'shell-quote'; import { doesToolInvocationMatch } from './tool-utils.js'; +import { isShellCommandReadOnly } from './shellReadOnlyChecker.js'; const SHELL_TOOL_NAMES = ['run_shell_command', 'ShellTool']; @@ -469,3 +470,19 @@ export function isCommandAllowed( } return { allowed: false, reason: blockReason }; } + +export function isCommandNeedsPermission(command: string): { + requiresPermission: boolean; + reason?: string; +} { + const isAllowed = isShellCommandReadOnly(command); + + if (isAllowed) { + return { requiresPermission: false }; + } + + return { + requiresPermission: true, + reason: 'Command requires permission to execute.', + }; +} diff --git a/packages/core/src/utils/shellReadOnlyChecker.test.ts b/packages/core/src/utils/shellReadOnlyChecker.test.ts new file mode 100644 index 00000000..c2bc5e41 --- /dev/null +++ b/packages/core/src/utils/shellReadOnlyChecker.test.ts @@ -0,0 +1,56 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { isShellCommandReadOnly } from './shellReadOnlyChecker.js'; + +describe('evaluateShellCommandReadOnly', () => { + it('allows simple read-only command', () => { + const result = isShellCommandReadOnly('ls -la'); + expect(result).toBe(true); + }); + + it('rejects mutating commands like rm', () => { + const result = isShellCommandReadOnly('rm -rf temp'); + expect(result).toBe(false); + }); + + it('rejects redirection output', () => { + const result = isShellCommandReadOnly('ls > out.txt'); + expect(result).toBe(false); + }); + + it('rejects command substitution', () => { + const result = isShellCommandReadOnly('echo $(touch file)'); + expect(result).toBe(false); + }); + + it('allows git status but rejects git commit', () => { + expect(isShellCommandReadOnly('git status')).toBe(true); + const commitResult = isShellCommandReadOnly('git commit -am "msg"'); + expect(commitResult).toBe(false); + }); + + it('rejects find with exec', () => { + const result = isShellCommandReadOnly('find . -exec rm {} \\;'); + expect(result).toBe(false); + }); + + it('rejects sed in-place', () => { + const result = isShellCommandReadOnly("sed -i 's/foo/bar/' file"); + expect(result).toBe(false); + }); + + it('rejects empty command', () => { + const result = isShellCommandReadOnly(' '); + expect(result).toBe(false); + }); + + it('respects environment prefix followed by allowed command', () => { + const result = isShellCommandReadOnly('FOO=bar ls'); + expect(result).toBe(true); + }); +}); diff --git a/packages/core/src/utils/shellReadOnlyChecker.ts b/packages/core/src/utils/shellReadOnlyChecker.ts new file mode 100644 index 00000000..4f29ff66 --- /dev/null +++ b/packages/core/src/utils/shellReadOnlyChecker.ts @@ -0,0 +1,300 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { parse } from 'shell-quote'; +import { + detectCommandSubstitution, + splitCommands, + stripShellWrapper, +} from './shell-utils.js'; + +const READ_ONLY_ROOT_COMMANDS = new Set([ + 'awk', + 'basename', + 'cat', + 'cd', + 'column', + 'cut', + 'df', + 'dirname', + 'du', + 'echo', + 'env', + 'find', + 'git', + 'grep', + 'head', + 'less', + 'ls', + 'more', + 'printenv', + 'printf', + 'ps', + 'pwd', + 'rg', + 'ripgrep', + 'sed', + 'sort', + 'stat', + 'tail', + 'tree', + 'uniq', + 'wc', + 'which', + 'where', + 'whoami', +]); + +const BLOCKED_FIND_FLAGS = new Set([ + '-delete', + '-exec', + '-execdir', + '-ok', + '-okdir', +]); + +const BLOCKED_FIND_PREFIXES = ['-fprint', '-fprintf']; + +const READ_ONLY_GIT_SUBCOMMANDS = new Set([ + 'blame', + 'branch', + 'cat-file', + 'diff', + 'grep', + 'log', + 'ls-files', + 'remote', + 'rev-parse', + 'show', + 'status', + 'describe', +]); + +const BLOCKED_GIT_REMOTE_ACTIONS = new Set([ + 'add', + 'remove', + 'rename', + 'set-url', + 'prune', + 'update', +]); + +const BLOCKED_GIT_BRANCH_FLAGS = new Set([ + '-d', + '-D', + '--delete', + '--move', + '-m', +]); + +const BLOCKED_SED_PREFIXES = ['-i']; + +const ENV_ASSIGNMENT_REGEX = /^[A-Za-z_][A-Za-z0-9_]*=/; + +function containsWriteRedirection(command: string): boolean { + let inSingleQuotes = false; + let inDoubleQuotes = false; + let escapeNext = false; + + for (const char of command) { + if (escapeNext) { + escapeNext = false; + continue; + } + + if (char === '\\' && !inSingleQuotes) { + escapeNext = true; + continue; + } + + if (char === "'" && !inDoubleQuotes) { + inSingleQuotes = !inSingleQuotes; + continue; + } + + if (char === '"' && !inSingleQuotes) { + inDoubleQuotes = !inDoubleQuotes; + continue; + } + + if (!inSingleQuotes && !inDoubleQuotes && char === '>') { + return true; + } + } + + return false; +} + +function normalizeTokens(segment: string): string[] { + const parsed = parse(segment); + const tokens: string[] = []; + for (const token of parsed) { + if (typeof token === 'string') { + tokens.push(token); + } + } + return tokens; +} + +function skipEnvironmentAssignments(tokens: string[]): { + root?: string; + args: string[]; +} { + let index = 0; + while (index < tokens.length && ENV_ASSIGNMENT_REGEX.test(tokens[index]!)) { + index++; + } + + if (index >= tokens.length) { + return { args: [] }; + } + + return { + root: tokens[index], + args: tokens.slice(index + 1), + }; +} + +function evaluateFindCommand(tokens: string[]): boolean { + const [, ...rest] = tokens; + for (const token of rest) { + const lower = token.toLowerCase(); + if (BLOCKED_FIND_FLAGS.has(lower)) { + return false; + } + if (BLOCKED_FIND_PREFIXES.some((prefix) => lower.startsWith(prefix))) { + return false; + } + } + return true; +} + +function evaluateSedCommand(tokens: string[]): boolean { + const [, ...rest] = tokens; + for (const token of rest) { + if ( + BLOCKED_SED_PREFIXES.some((prefix) => token.startsWith(prefix)) || + token === '--in-place' + ) { + return false; + } + } + return true; +} + +function evaluateGitRemoteArgs(args: string[]): boolean { + for (const arg of args) { + if (BLOCKED_GIT_REMOTE_ACTIONS.has(arg.toLowerCase())) { + return false; + } + } + return true; +} + +function evaluateGitBranchArgs(args: string[]): boolean { + for (const arg of args) { + if (BLOCKED_GIT_BRANCH_FLAGS.has(arg)) { + return false; + } + } + return true; +} + +function evaluateGitCommand(tokens: string[]): boolean { + let index = 1; + while (index < tokens.length && tokens[index]!.startsWith('-')) { + const flag = tokens[index]!.toLowerCase(); + if (flag === '--version' || flag === '--help') { + return true; + } + index++; + } + + if (index >= tokens.length) { + return true; + } + + const subcommand = tokens[index]!.toLowerCase(); + if (!READ_ONLY_GIT_SUBCOMMANDS.has(subcommand)) { + return false; + } + + const args = tokens.slice(index + 1); + + if (subcommand === 'remote') { + return evaluateGitRemoteArgs(args); + } + + if (subcommand === 'branch') { + return evaluateGitBranchArgs(args); + } + + return true; +} + +function evaluateShellSegment(segment: string): boolean { + if (!segment.trim()) { + return true; + } + + const stripped = stripShellWrapper(segment); + if (!stripped) { + return true; + } + + if (detectCommandSubstitution(stripped)) { + return false; + } + + if (containsWriteRedirection(stripped)) { + return false; + } + + const tokens = normalizeTokens(stripped); + if (tokens.length === 0) { + return true; + } + + const { root, args } = skipEnvironmentAssignments(tokens); + if (!root) { + return true; + } + + const normalizedRoot = root.toLowerCase(); + if (!READ_ONLY_ROOT_COMMANDS.has(normalizedRoot)) { + return false; + } + + if (normalizedRoot === 'find') { + return evaluateFindCommand([normalizedRoot, ...args]); + } + + if (normalizedRoot === 'sed') { + return evaluateSedCommand([normalizedRoot, ...args]); + } + + if (normalizedRoot === 'git') { + return evaluateGitCommand([normalizedRoot, ...args]); + } + + return true; +} + +export function isShellCommandReadOnly(command: string): boolean { + if (typeof command !== 'string' || !command.trim()) { + return false; + } + + const segments = splitCommands(command); + for (const segment of segments) { + const isAllowed = evaluateShellSegment(segment); + if (!isAllowed) { + return false; + } + } + + return true; +}