diff --git a/docs/changelogs/preview.md b/docs/changelogs/preview.md index 5bb8d5b575..95feee1e2a 100644 --- a/docs/changelogs/preview.md +++ b/docs/changelogs/preview.md @@ -1,6 +1,6 @@ -# Preview release: v0.37.0-preview.1 +# Preview release: v0.37.0-preview.2 -Released: April 02, 2026 +Released: April 07, 2026 Our preview release includes the latest, new, and experimental features. This release may not be as stable as our [latest weekly release](latest.md). @@ -33,6 +33,10 @@ npm install -g @google/gemini-cli@preview ## What's Changed +- fix(patch): cherry-pick cb7f7d6 to release/v0.37.0-preview.1-pr-24342 to patch + version v0.37.0-preview.1 and create version 0.37.0-preview.2 by + @gemini-cli-robot in + [#24842](https://github.com/google-gemini/gemini-cli/pull/24842) - fix(patch): cherry-pick 64c928f to release/v0.37.0-preview.0-pr-23257 to patch version v0.37.0-preview.0 and create version 0.37.0-preview.1 by @gemini-cli-robot in @@ -419,4 +423,4 @@ npm install -g @google/gemini-cli@preview [#23275](https://github.com/google-gemini/gemini-cli/pull/23275) **Full Changelog**: -https://github.com/google-gemini/gemini-cli/compare/v0.36.0-preview.8...v0.37.0-preview.1 +https://github.com/google-gemini/gemini-cli/compare/v0.36.0-preview.8...v0.37.0-preview.2 diff --git a/docs/cli/settings.md b/docs/cli/settings.md index 4a6b9a77b7..dbb3651a4f 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -75,7 +75,7 @@ they appear in the UI. | Show User Identity | `ui.showUserIdentity` | Show the signed-in user's identity (e.g. email) in the UI. | `true` | | Use Alternate Screen Buffer | `ui.useAlternateBuffer` | Use an alternate screen buffer for the UI, preserving shell history. | `false` | | Render Process | `ui.renderProcess` | Enable Ink render process for the UI. | `true` | -| Terminal Buffer | `ui.terminalBuffer` | Use the new terminal buffer architecture for rendering. | `true` | +| Terminal Buffer | `ui.terminalBuffer` | Use the new terminal buffer architecture for rendering. | `false` | | Use Background Color | `ui.useBackgroundColor` | Whether to use background colors in the UI. | `true` | | Incremental Rendering | `ui.incrementalRendering` | Enable incremental rendering for the UI. This option will reduce flickering but may cause rendering artifacts. Only supported when useAlternateBuffer is enabled. | `true` | | Show Spinner | `ui.showSpinner` | Show the spinner during operations. | `true` | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 1955507c62..1fdbc755f0 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -346,7 +346,7 @@ their corresponding top-level category object in your `settings.json` file. - **`ui.terminalBuffer`** (boolean): - **Description:** Use the new terminal buffer architecture for rendering. - - **Default:** `true` + - **Default:** `false` - **Requires restart:** Yes - **`ui.useBackgroundColor`** (boolean): diff --git a/docs/reference/keyboard-shortcuts.md b/docs/reference/keyboard-shortcuts.md index 68b3d884fe..4ef61ac003 100644 --- a/docs/reference/keyboard-shortcuts.md +++ b/docs/reference/keyboard-shortcuts.md @@ -86,13 +86,14 @@ available combinations. #### Text Input -| Command | Action | Keys | -| -------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | -| `input.submit` | Submit the current prompt. | `Enter` | -| `input.queueMessage` | Queue the current prompt to be processed after the current task finishes. | `Tab` | -| `input.newline` | Insert a newline without submitting. | `Ctrl+Enter`
`Cmd/Win+Enter`
`Alt+Enter`
`Shift+Enter`
`Ctrl+J` | -| `input.openExternalEditor` | Open the current prompt or the plan in an external editor. | `Ctrl+X` | -| `input.paste` | Paste from the clipboard. | `Ctrl+V`
`Cmd/Win+V`
`Alt+V` | +| Command | Action | Keys | +| ------------------------------------ | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | +| `input.submit` | Submit the current prompt. | `Enter` | +| `input.queueMessage` | Queue the current prompt to be processed after the current task finishes. | `Tab` | +| `input.newline` | Insert a newline without submitting. | `Ctrl+Enter`
`Cmd/Win+Enter`
`Alt+Enter`
`Shift+Enter`
`Ctrl+J` | +| `input.openExternalEditor` | Open the current prompt or the plan in an external editor. | `Ctrl+G` | +| `input.deprecatedOpenExternalEditor` | Deprecated command to open external editor. | `Ctrl+X` | +| `input.paste` | Paste from the clipboard. | `Ctrl+V`
`Cmd/Win+V`
`Alt+V` | #### App Controls @@ -100,7 +101,7 @@ available combinations. | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | | `app.showErrorDetails` | Toggle detailed error information. | `F12` | | `app.showFullTodos` | Toggle the full TODO list. | `Ctrl+T` | -| `app.showIdeContextDetail` | Show IDE context details. | `Ctrl+G` | +| `app.showIdeContextDetail` | Show IDE context details. | `F4` | | `app.toggleMarkdown` | Toggle Markdown rendering. | `Alt+M` | | `app.toggleCopyMode` | Toggle copy mode when in alternate buffer mode. | `F9` | | `app.toggleMouseMode` | Toggle mouse mode (scrolling and clicking). | `Ctrl+S` | diff --git a/docs/tools/mcp-server.md b/docs/tools/mcp-server.md index 9fc84d54c0..3baeb746df 100644 --- a/docs/tools/mcp-server.md +++ b/docs/tools/mcp-server.md @@ -290,7 +290,7 @@ When connecting to an OAuth-enabled server: > OAuth authentication requires that your local machine can: > > - Open a web browser for authentication -> - Receive redirects on `http://localhost:7777/oauth/callback` +> - Receive redirects on `http://localhost:/oauth/callback` (or a specific port if configured via `redirectUri`) This feature will not work in: @@ -323,8 +323,8 @@ Use the `/mcp auth` command to manage OAuth authentication: if omitted) - **`tokenUrl`** (string): OAuth token endpoint (auto-discovered if omitted) - **`scopes`** (string[]): Required OAuth scopes -- **`redirectUri`** (string): Custom redirect URI (defaults to - `http://localhost:7777/oauth/callback`) +- **`redirectUri`** (string): Custom redirect URI (defaults to an OS-assigned + random port, e.g., `http://localhost:/oauth/callback`) - **`tokenParamName`** (string): Query parameter name for tokens in SSE URLs - **`audiences`** (string[]): Audiences the token is valid for diff --git a/integration-tests/browser-agent.concurrent.responses b/integration-tests/browser-agent.concurrent.responses new file mode 100644 index 0000000000..f64397e02d --- /dev/null +++ b/integration-tests/browser-agent.concurrent.responses @@ -0,0 +1,8 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I'll launch two browser agents concurrently to check both repositories."},{"functionCall":{"name":"browser_agent","args":{"task":"Navigate to https://example.com and get the page title"}}},{"functionCall":{"name":"browser_agent","args":{"task":"Navigate to https://example.com and get the page title"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"navigate_page","args":{"url":"https://example.com"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":20,"totalTokenCount":120}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"navigate_page","args":{"url":"https://example.com"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":20,"totalTokenCount":120}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":150,"candidatesTokenCount":15,"totalTokenCount":165}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"result":{"success":true,"summary":"Page title is Example Domain."}}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":30,"totalTokenCount":230}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"result":{"success":true,"summary":"Page title is Example Domain."}}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":30,"totalTokenCount":230}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Both browser agents completed successfully. Agent 1 and Agent 2 both navigated to their respective pages and confirmed the page titles."}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":300,"candidatesTokenCount":40,"totalTokenCount":340}}]} diff --git a/integration-tests/browser-agent.test.ts b/integration-tests/browser-agent.test.ts index 09e20bcb26..325fdc1db5 100644 --- a/integration-tests/browser-agent.test.ts +++ b/integration-tests/browser-agent.test.ts @@ -307,4 +307,48 @@ describe.skipIf(!chromeAvailable)('browser-agent', () => { await run.expectText('successfully written', 15000); }); + + it('should handle concurrent browser agents with isolated session mode', async () => { + rig.setup('browser-concurrent', { + fakeResponsesPath: join(__dirname, 'browser-agent.concurrent.responses'), + settings: { + agents: { + overrides: { + browser_agent: { + enabled: true, + }, + }, + browser: { + headless: true, + // Isolated mode supports concurrent browser agents. + // Persistent/existing modes reject concurrent calls to prevent + // Chrome profile lock conflicts. + sessionMode: 'isolated', + }, + }, + }, + }); + + const result = await rig.run({ + args: 'Launch two browser agents concurrently to check example.com', + }); + + assertModelHasOutput(result); + + const toolLogs = rig.readToolLogs(); + const browserCalls = toolLogs.filter( + (t) => t.toolRequest.name === 'browser_agent', + ); + + // Both browser_agent invocations should have been called + expect(browserCalls.length).toBe(2); + + // Both should complete successfully (no errors) + for (const call of browserCalls) { + expect( + call.toolRequest.success, + `browser_agent call failed: ${JSON.stringify(call.toolRequest)}`, + ).toBe(true); + } + }); }); diff --git a/packages/cli/src/config/policy-engine.integration.test.ts b/packages/cli/src/config/policy-engine.integration.test.ts index edc06bfbf0..b7b9be1193 100644 --- a/packages/cli/src/config/policy-engine.integration.test.ts +++ b/packages/cli/src/config/policy-engine.integration.test.ts @@ -520,8 +520,8 @@ describe('Policy Engine Integration Tests', () => { const readOnlyToolRule = rules.find( (r) => r.toolName === 'glob' && !r.subagent, ); - // Priority 70 in default tier β†’ 1.07 (Overriding Plan Mode Deny) - expect(readOnlyToolRule?.priority).toBeCloseTo(1.07, 5); + // Priority 50 in default tier β†’ 1.05 (Overriding Plan Mode Deny) + expect(readOnlyToolRule?.priority).toBeCloseTo(1.05, 5); // Verify the engine applies these priorities correctly expect( @@ -677,8 +677,8 @@ describe('Policy Engine Integration Tests', () => { expect(server1Rule?.priority).toBe(4.1); // Allowed servers (user tier) const globRule = rules.find((r) => r.toolName === 'glob' && !r.subagent); - // Priority 70 in default tier β†’ 1.07 - expect(globRule?.priority).toBeCloseTo(1.07, 5); // Auto-accept read-only + // Priority 50 in default tier β†’ 1.05 + expect(globRule?.priority).toBeCloseTo(1.05, 5); // Auto-accept read-only // The PolicyEngine will sort these by priority when it's created const engine = new PolicyEngine(config); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 730bd4b939..c041aaa8c3 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -757,7 +757,7 @@ const SETTINGS_SCHEMA = { label: 'Terminal Buffer', category: 'UI', requiresRestart: true, - default: true, + default: false, description: 'Use the new terminal buffer architecture for rendering.', showInDialog: true, }, diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index fd19ffa79c..611850bd4a 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -379,15 +379,30 @@ describe('initializeOutputListenersAndFlush', () => { describe('getNodeMemoryArgs', () => { let osTotalMemSpy: MockInstance; let v8GetHeapStatisticsSpy: MockInstance; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let originalConfig: any; beforeEach(() => { osTotalMemSpy = vi.spyOn(os, 'totalmem'); v8GetHeapStatisticsSpy = vi.spyOn(v8, 'getHeapStatistics'); delete process.env['GEMINI_CLI_NO_RELAUNCH']; + + originalConfig = process.config; + Object.defineProperty(process, 'config', { + value: { + ...originalConfig, + variables: { ...originalConfig?.variables, v8_enable_sandbox: 1 }, + }, + configurable: true, + }); }); afterEach(() => { vi.restoreAllMocks(); + Object.defineProperty(process, 'config', { + value: originalConfig, + configurable: true, + }); }); it('should return empty array if GEMINI_CLI_NO_RELAUNCH is set', () => { @@ -400,8 +415,10 @@ describe('getNodeMemoryArgs', () => { v8GetHeapStatisticsSpy.mockReturnValue({ heap_size_limit: 8 * 1024 * 1024 * 1024, // 8GB }); - // Target is 50% of 16GB = 8GB. Current is 8GB. No relaunch needed. - expect(getNodeMemoryArgs(false)).toEqual([]); + // Target is 50% of 16GB = 8GB. Current is 8GB. Relaunch needed for EPT size only. + expect(getNodeMemoryArgs(false)).toEqual([ + '--max-external-pointer-table-size=268435456', + ]); }); it('should return memory args if current heap limit is insufficient', () => { @@ -409,8 +426,11 @@ describe('getNodeMemoryArgs', () => { v8GetHeapStatisticsSpy.mockReturnValue({ heap_size_limit: 4 * 1024 * 1024 * 1024, // 4GB }); - // Target is 50% of 16GB = 8GB. Current is 4GB. Relaunch needed. - expect(getNodeMemoryArgs(false)).toEqual(['--max-old-space-size=8192']); + // Target is 50% of 16GB = 8GB. Current is 4GB. Relaunch needed for both. + expect(getNodeMemoryArgs(false)).toEqual([ + '--max-external-pointer-table-size=268435456', + '--max-old-space-size=8192', + ]); }); it('should log debug info when isDebugMode is true', () => { diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index fa22f59267..f77fc11d61 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -111,6 +111,8 @@ export function validateDnsResolutionOrder( return defaultValue; } +const DEFAULT_EPT_SIZE = (256 * 1024 * 1024).toString(); + export function getNodeMemoryArgs(isDebugMode: boolean): string[] { const totalMemoryMB = os.totalmem() / (1024 * 1024); const heapStats = v8.getHeapStatistics(); @@ -130,16 +132,35 @@ export function getNodeMemoryArgs(isDebugMode: boolean): string[] { return []; } + const args: string[] = []; + + // Automatically expand the V8 External Pointer Table to 256MB to prevent + // out-of-memory crashes during high native-handle concurrency. + // Note: Only supported in specific Node.js versions compiled with V8 Sandbox enabled. + const eptFlag = `--max-external-pointer-table-size=${DEFAULT_EPT_SIZE}`; + const isV8SandboxEnabled = + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-type-assertion + (process.config?.variables as any)?.v8_enable_sandbox === 1; + + if ( + isV8SandboxEnabled && + !process.execArgv.some((arg) => + arg.startsWith('--max-external-pointer-table-size'), + ) + ) { + args.push(eptFlag); + } + if (targetMaxOldSpaceSizeInMB > currentMaxOldSpaceSizeMb) { if (isDebugMode) { debugLogger.debug( `Need to relaunch with more memory: ${targetMaxOldSpaceSizeInMB.toFixed(2)} MB`, ); } - return [`--max-old-space-size=${targetMaxOldSpaceSizeInMB}`]; + args.push(`--max-old-space-size=${targetMaxOldSpaceSizeInMB}`); } - return []; + return args; } export function setupUnhandledRejectionHandler() { diff --git a/packages/cli/src/interactiveCli.tsx b/packages/cli/src/interactiveCli.tsx index 418f58b193..965bc27693 100644 --- a/packages/cli/src/interactiveCli.tsx +++ b/packages/cli/src/interactiveCli.tsx @@ -156,8 +156,9 @@ export async function startInteractiveUI( useAlternateBuffer || config.getUseTerminalBuffer(), patchConsole: false, alternateBuffer: useAlternateBuffer, - renderProcess: config.getUseRenderProcess(), terminalBuffer: config.getUseTerminalBuffer(), + renderProcess: + config.getUseRenderProcess() && config.getUseTerminalBuffer(), incrementalRendering: settings.merged.ui.incrementalRendering !== false && useAlternateBuffer && diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 855707de9e..5d0c3d1016 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -71,6 +71,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { Scheduler: class { schedule = mockSchedulerSchedule; cancelAll = vi.fn(); + dispose = vi.fn(); }, isTelemetrySdkInitialized: vi.fn().mockReturnValue(true), ChatRecordingService: MockChatRecordingService, diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 26daaf66a1..dc5255edee 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -187,6 +187,7 @@ export async function runNonInteractive( }; let errorToHandle: unknown | undefined; + let scheduler: Scheduler | undefined; try { consolePatcher.patch(); @@ -215,7 +216,7 @@ export async function runNonInteractive( }); const geminiClient = config.getGeminiClient(); - const scheduler = new Scheduler({ + scheduler = new Scheduler({ context: config, messageBus: config.getMessageBus(), getPreferredEditor: () => undefined, @@ -528,6 +529,7 @@ export async function runNonInteractive( // Cleanup stdin cancellation before other cleanup cleanupStdinCancellation(); + scheduler?.dispose(); consolePatcher.cleanup(); coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); } diff --git a/packages/cli/src/nonInteractiveCliAgentSession.test.ts b/packages/cli/src/nonInteractiveCliAgentSession.test.ts index 617f80aca6..923109643c 100644 --- a/packages/cli/src/nonInteractiveCliAgentSession.test.ts +++ b/packages/cli/src/nonInteractiveCliAgentSession.test.ts @@ -71,6 +71,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { Scheduler: class { schedule = mockSchedulerSchedule; cancelAll = vi.fn(); + dispose = vi.fn(); }, isTelemetrySdkInitialized: vi.fn().mockReturnValue(true), ChatRecordingService: MockChatRecordingService, diff --git a/packages/cli/src/nonInteractiveCliAgentSession.ts b/packages/cli/src/nonInteractiveCliAgentSession.ts index fe5fbceba2..7f36ce6cf5 100644 --- a/packages/cli/src/nonInteractiveCliAgentSession.ts +++ b/packages/cli/src/nonInteractiveCliAgentSession.ts @@ -184,6 +184,7 @@ export async function runNonInteractive({ }; let errorToHandle: unknown | undefined; + let scheduler: Scheduler | undefined; let abortSession = () => {}; try { consolePatcher.patch(); @@ -215,7 +216,7 @@ export async function runNonInteractive({ }); const geminiClient = config.getGeminiClient(); - const scheduler = new Scheduler({ + scheduler = new Scheduler({ context: config, messageBus: config.getMessageBus(), getPreferredEditor: () => undefined, @@ -612,6 +613,7 @@ export async function runNonInteractive({ cleanupStdinCancellation(); abortController.signal.removeEventListener('abort', abortSession); + scheduler?.dispose(); consolePatcher.cleanup(); coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); } diff --git a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap index 94b1f9b1a4..611f2e0908 100644 --- a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap +++ b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap @@ -55,6 +55,12 @@ Footer Gemini CLI v1.2.3 + +Tips for getting started: +1. Create GEMINI.md files to customize your interactions +2. /help for more information +3. Ask coding questions, edit code or run commands +4. Be specific for the best results Composer " `; diff --git a/packages/cli/src/ui/components/ContextSummaryDisplay.tsx b/packages/cli/src/ui/components/ContextSummaryDisplay.tsx index 696793bc06..171e29e905 100644 --- a/packages/cli/src/ui/components/ContextSummaryDisplay.tsx +++ b/packages/cli/src/ui/components/ContextSummaryDisplay.tsx @@ -8,6 +8,8 @@ import type React from 'react'; import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; import { type IdeContext, type MCPServerConfig } from '@google/gemini-cli-core'; +import { Command } from '../key/keyMatchers.js'; +import { formatCommand } from '../key/keybindingUtils.js'; interface ContextSummaryDisplayProps { geminiMdFileCount: number; @@ -49,7 +51,7 @@ export const ContextSummaryDisplay: React.FC = ({ } return `${openFileCount} open file${ openFileCount > 1 ? 's' : '' - } (ctrl+g to view)`; + } (${formatCommand(Command.SHOW_IDE_CONTEXT_DETAIL)} to view)`; })(); const geminiMdText = (() => { diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx index 18f2f02224..6925c749d7 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx @@ -587,7 +587,7 @@ Implement a comprehensive authentication system with multiple providers. expect(onFeedback).not.toHaveBeenCalled(); }); - it('automatically submits feedback when Ctrl+X is used to edit the plan', async () => { + it('automatically submits feedback when Ctrl+G is used to edit the plan', async () => { const { stdin, lastFrame } = await act(async () => renderDialog({ useAlternateBuffer }), ); @@ -600,9 +600,9 @@ Implement a comprehensive authentication system with multiple providers. expect(lastFrame()).toContain('Add user authentication'); }); - // Press Ctrl+X + // Press Ctrl+G await act(async () => { - writeKey(stdin, '\x18'); // Ctrl+X + writeKey(stdin, '\x07'); // Ctrl+G }); await waitFor(() => { diff --git a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx index b2c28abaeb..11adf8e82b 100644 --- a/packages/cli/src/ui/components/ExitPlanModeDialog.tsx +++ b/packages/cli/src/ui/components/ExitPlanModeDialog.tsx @@ -25,6 +25,11 @@ import { useKeypress } from '../hooks/useKeypress.js'; import { Command } from '../key/keyMatchers.js'; import { formatCommand } from '../key/keybindingUtils.js'; import { useKeyMatchers } from '../hooks/useKeyMatchers.js'; +import { + appEvents, + AppEvent, + TransientMessageType, +} from '../../utils/events.js'; export interface ExitPlanModeDialogProps { planPath: string; @@ -173,6 +178,14 @@ export const ExitPlanModeDialog: React.FC = ({ void handleOpenEditor(); return true; } + if (keyMatchers[Command.DEPRECATED_OPEN_EXTERNAL_EDITOR](key)) { + const cmdKey = formatCommand(Command.OPEN_EXTERNAL_EDITOR); + appEvents.emit(AppEvent.TransientMessage, { + message: `Use ${cmdKey} to open the external editor.`, + type: TransientMessageType.Hint, + }); + return true; + } return false; }, { isActive: true, priority: true }, diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 4d40809837..7a241691e8 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -69,6 +69,7 @@ import { AppEvent, TransientMessageType, } from '../../utils/events.js'; +import '../../test-utils/customMatchers.js'; vi.mock('../hooks/useShellHistory.js'); vi.mock('../hooks/useCommandCompletion.js'); @@ -254,7 +255,7 @@ describe('InputPrompt', () => { setText: vi.fn( (newText: string, cursorPosition?: 'start' | 'end' | number) => { mockBuffer.text = newText; - mockBuffer.lines = [newText]; + mockBuffer.lines = newText.split('\n'); let col = 0; if (typeof cursorPosition === 'number') { col = cursorPosition; @@ -264,11 +265,18 @@ describe('InputPrompt', () => { col = newText.length; } mockBuffer.cursor = [0, col]; - mockBuffer.allVisualLines = [newText]; - mockBuffer.viewportVisualLines = [newText]; - mockBuffer.allVisualLines = [newText]; - mockBuffer.visualToLogicalMap = [[0, 0]]; + mockBuffer.allVisualLines = newText.split('\n'); + mockBuffer.viewportVisualLines = newText.split('\n'); + mockBuffer.visualToLogicalMap = newText + .split('\n') + .map((_, i) => [i, 0] as [number, number]); mockBuffer.visualCursor = [0, col]; + mockBuffer.visualScrollRow = 0; + mockBuffer.viewportHeight = 10; + mockBuffer.visualToTransformedMap = newText + .split('\n') + .map((_, i) => i); + mockBuffer.transformationsByLine = newText.split('\n').map(() => []); }, ), replaceRangeByOffset: vi.fn(), @@ -276,6 +284,7 @@ describe('InputPrompt', () => { allVisualLines: [''], visualCursor: [0, 0], visualScrollRow: 0, + viewportHeight: 10, handleInput: vi.fn((key: Key) => { if (defaultKeyMatchers[Command.CLEAR_INPUT](key)) { if (mockBuffer.text.length > 0) { @@ -409,6 +418,7 @@ describe('InputPrompt', () => { getTargetDir: () => path.join('test', 'project', 'src'), getVimMode: () => false, getUseBackgroundColor: () => true, + getUseTerminalBuffer: () => false, getTerminalBackground: () => undefined, getWorkspaceContext: () => ({ getDirectories: () => ['/test/project/src'], @@ -3779,11 +3789,7 @@ describe('InputPrompt', () => { ); it('should unfocus embedded shell on click', async () => { - props.buffer.text = 'hello'; - props.buffer.lines = ['hello']; - props.buffer.allVisualLines = ['hello']; - props.buffer.viewportVisualLines = ['hello']; - props.buffer.visualToLogicalMap = [[0, 0]]; + props.buffer.setText('hello'); props.isEmbeddedShellFocused = true; const { stdin, stdout, unmount } = await renderWithProviders( @@ -4291,11 +4297,7 @@ describe('InputPrompt', () => { describe('IME Cursor Support', () => { it('should report correct cursor position for simple ASCII text', async () => { const text = 'hello'; - mockBuffer.text = text; - mockBuffer.lines = [text]; - mockBuffer.allVisualLines = [text]; - mockBuffer.viewportVisualLines = [text]; - mockBuffer.visualToLogicalMap = [[0, 0]]; + mockBuffer.setText(text); mockBuffer.visualCursor = [0, 3]; // Cursor after 'hel' mockBuffer.visualScrollRow = 0; @@ -4322,11 +4324,7 @@ describe('InputPrompt', () => { it('should report correct cursor position for text with double-width characters', async () => { const text = 'πŸ‘hello'; - mockBuffer.text = text; - mockBuffer.lines = [text]; - mockBuffer.allVisualLines = [text]; - mockBuffer.viewportVisualLines = [text]; - mockBuffer.visualToLogicalMap = [[0, 0]]; + mockBuffer.setText(text); mockBuffer.visualCursor = [0, 2]; // Cursor after 'πŸ‘h' (Note: 'πŸ‘' is one code point but width 2) mockBuffer.visualScrollRow = 0; @@ -4352,11 +4350,7 @@ describe('InputPrompt', () => { it('should report correct cursor position for a line full of "πŸ˜€" emojis', async () => { const text = 'πŸ˜€πŸ˜€πŸ˜€'; - mockBuffer.text = text; - mockBuffer.lines = [text]; - mockBuffer.allVisualLines = [text]; - mockBuffer.viewportVisualLines = [text]; - mockBuffer.visualToLogicalMap = [[0, 0]]; + mockBuffer.setText(text); mockBuffer.visualCursor = [0, 2]; // Cursor after 2 emojis (each 1 code point, width 2) mockBuffer.visualScrollRow = 0; @@ -4501,12 +4495,12 @@ describe('InputPrompt', () => { mockBuffer.lines = [logicalLine]; mockBuffer.allVisualLines = [visualLine]; mockBuffer.viewportVisualLines = [visualLine]; - mockBuffer.allVisualLines = [visualLine]; mockBuffer.visualToLogicalMap = [[0, 0]]; mockBuffer.visualToTransformedMap = [0]; mockBuffer.transformationsByLine = [transformations]; mockBuffer.cursor = [0, cursorCol]; - mockBuffer.visualCursor = [0, 0]; + mockBuffer.visualCursor = [0, cursorCol]; + mockBuffer.visualScrollRow = 0; }; it('should snapshot collapsed image path', async () => { @@ -5071,8 +5065,8 @@ describe('InputPrompt', () => { input: '\x12', }, { - name: 'Ctrl+X hotkey is pressed', - input: '\x18', + name: 'Ctrl+G hotkey is pressed', + input: '\x07', }, { name: 'F12 hotkey is pressed', diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index c8d7efa1b4..b36de8ebb0 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -5,7 +5,14 @@ */ import type React from 'react'; -import { useCallback, useEffect, useState, useRef, useMemo } from 'react'; +import { + useCallback, + useEffect, + useState, + useRef, + useMemo, + Fragment, +} from 'react'; import clipboardy from 'clipboardy'; import { Box, Text, useStdout, type DOMElement } from 'ink'; import { SuggestionsDisplay, MAX_WIDTH } from './SuggestionsDisplay.js'; @@ -1265,6 +1272,15 @@ export const InputPrompt: React.FC = ({ return true; } + if (keyMatchers[Command.DEPRECATED_OPEN_EXTERNAL_EDITOR](key)) { + const cmdKey = formatCommand(Command.OPEN_EXTERNAL_EDITOR); + appEvents.emit(AppEvent.TransientMessage, { + message: `Use ${cmdKey} to open the external editor.`, + type: TransientMessageType.Hint, + }); + return true; + } + // Ctrl+V for clipboard paste if (keyMatchers[Command.PASTE_CLIPBOARD](key)) { // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -1820,24 +1836,45 @@ export const InputPrompt: React.FC = ({ height={Math.min(buffer.viewportHeight, scrollableData.length)} width="100%" > - 1} - keyExtractor={(item) => - item.type === 'visualLine' - ? `line-${item.absoluteVisualIdx}` - : `ghost-${item.index}` - } - width="100%" - backgroundColor={listBackgroundColor} - containerHeight={Math.min( - buffer.viewportHeight, - scrollableData.length, - )} - /> + {isAlternateBuffer ? ( + 1} + fixedItemHeight={true} + keyExtractor={(item) => + item.type === 'visualLine' + ? `line-${item.absoluteVisualIdx}` + : `ghost-${item.index}` + } + width={inputWidth} + backgroundColor={listBackgroundColor} + containerHeight={Math.min( + buffer.viewportHeight, + scrollableData.length, + )} + /> + ) : ( + scrollableData + .slice( + buffer.visualScrollRow, + buffer.visualScrollRow + buffer.viewportHeight, + ) + .map((item, index) => { + const actualIndex = buffer.visualScrollRow + index; + const key = + item.type === 'visualLine' + ? `line-${item.absoluteVisualIdx}` + : `ghost-${item.index}`; + return ( + + {renderItem({ item, index: actualIndex })} + + ); + }) + )} )} diff --git a/packages/cli/src/ui/components/StatusRow.tsx b/packages/cli/src/ui/components/StatusRow.tsx index 24b5a97d4e..f162481ce5 100644 --- a/packages/cli/src/ui/components/StatusRow.tsx +++ b/packages/cli/src/ui/components/StatusRow.tsx @@ -331,7 +331,7 @@ export const StatusRow: React.FC = ({ ) : isInteractiveShellWaiting ? ( - ! Shell awaiting input (Tab to focus) + {INTERACTIVE_SHELL_WAITING_PHRASE} ) : ( diff --git a/packages/cli/src/ui/components/__snapshots__/ContextSummaryDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ContextSummaryDisplay.test.tsx.snap index 876524bdb8..7330b89e4d 100644 --- a/packages/cli/src/ui/components/__snapshots__/ContextSummaryDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ContextSummaryDisplay.test.tsx.snap @@ -1,16 +1,16 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[` > should not render empty parts 1`] = ` -" 1 open file (ctrl+g to view) +" 1 open file (F4 to view) " `; exports[` > should render on a single line on a wide screen 1`] = ` -" 1 open file (ctrl+g to view) Β· 1 GEMINI.md file Β· 1 MCP server Β· 1 skill +" 1 open file (F4 to view) Β· 1 GEMINI.md file Β· 1 MCP server Β· 1 skill " `; exports[` > should render on multiple lines on a narrow screen 1`] = ` -" 1 open file (ctrl+g to view) Β· 1 GEMINI.md file Β· 1 MCP server Β· 1 skill +" 1 open file (F4 to view) Β· 1 GEMINI.md file Β· 1 MCP server Β· 1 skill " `; diff --git a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap index 073c106ceb..71acb9388c 100644 --- a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap @@ -23,7 +23,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -50,7 +50,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -82,7 +82,7 @@ Implementation Steps Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -109,7 +109,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -136,7 +136,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -163,7 +163,7 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -216,7 +216,7 @@ Testing Strategy Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; @@ -243,6 +243,6 @@ Files to Modify Approves plan but requires confirmation for each tool 3. Type your feedback... -Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel +Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel " `; diff --git a/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap index 7d6fdeb42c..d237b30f99 100644 --- a/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap @@ -112,48 +112,7 @@ exports[` > gemini items (alternateBuffer=false) > should exports[` > gemini items (alternateBuffer=false) > should render a truncated gemini item 1`] = ` "✦ Example code block: - 1 Line 1 - 2 Line 2 - 3 Line 3 - 4 Line 4 - 5 Line 5 - 6 Line 6 - 7 Line 7 - 8 Line 8 - 9 Line 9 - 10 Line 10 - 11 Line 11 - 12 Line 12 - 13 Line 13 - 14 Line 14 - 15 Line 15 - 16 Line 16 - 17 Line 17 - 18 Line 18 - 19 Line 19 - 20 Line 20 - 21 Line 21 - 22 Line 22 - 23 Line 23 - 24 Line 24 - 25 Line 25 - 26 Line 26 - 27 Line 27 - 28 Line 28 - 29 Line 29 - 30 Line 30 - 31 Line 31 - 32 Line 32 - 33 Line 33 - 34 Line 34 - 35 Line 35 - 36 Line 36 - 37 Line 37 - 38 Line 38 - 39 Line 39 - 40 Line 40 - 41 Line 41 - 42 Line 42 + ... 42 hidden (Ctrl+O) ... 43 Line 43 44 Line 44 45 Line 45 @@ -167,48 +126,7 @@ exports[` > gemini items (alternateBuffer=false) > should exports[` > gemini items (alternateBuffer=false) > should render a truncated gemini_content item 1`] = ` " Example code block: - 1 Line 1 - 2 Line 2 - 3 Line 3 - 4 Line 4 - 5 Line 5 - 6 Line 6 - 7 Line 7 - 8 Line 8 - 9 Line 9 - 10 Line 10 - 11 Line 11 - 12 Line 12 - 13 Line 13 - 14 Line 14 - 15 Line 15 - 16 Line 16 - 17 Line 17 - 18 Line 18 - 19 Line 19 - 20 Line 20 - 21 Line 21 - 22 Line 22 - 23 Line 23 - 24 Line 24 - 25 Line 25 - 26 Line 26 - 27 Line 27 - 28 Line 28 - 29 Line 29 - 30 Line 30 - 31 Line 31 - 32 Line 32 - 33 Line 33 - 34 Line 34 - 35 Line 35 - 36 Line 36 - 37 Line 37 - 38 Line 38 - 39 Line 39 - 40 Line 40 - 41 Line 41 - 42 Line 42 + ... 42 hidden (Ctrl+O) ... 43 Line 43 44 Line 44 45 Line 45 diff --git a/packages/cli/src/ui/components/__snapshots__/InputPrompt.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/InputPrompt.test.tsx.snap index caa270d8c4..ab6fe9b928 100644 --- a/packages/cli/src/ui/components/__snapshots__/InputPrompt.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/InputPrompt.test.tsx.snap @@ -93,7 +93,7 @@ exports[`InputPrompt > Highlighting and Cursor Display > single-line scenarios > exports[`InputPrompt > History Navigation and Completion Suppression > should not render suggestions during history navigation 1`] = ` "β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€ > second message - +β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„ " `; @@ -120,30 +120,30 @@ exports[`InputPrompt > command search (Ctrl+R when not in shell) > expands and c exports[`InputPrompt > command search (Ctrl+R when not in shell) > renders match window and expanded view (snapshots) > command-search-render-collapsed-match 1`] = ` "β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€ (r:) commit - - +β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„ + git commit -m "feat: add search" in src/app " `; exports[`InputPrompt > command search (Ctrl+R when not in shell) > renders match window and expanded view (snapshots) > command-search-render-expanded-match 1`] = ` "β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€ (r:) commit - - +β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„ + git commit -m "feat: add search" in src/app " `; exports[`InputPrompt > image path transformation snapshots > should snapshot collapsed image path 1`] = ` "β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€ > [Image ...reenshot2x.png] - +β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„ " `; exports[`InputPrompt > image path transformation snapshots > should snapshot expanded image path when cursor is on it 1`] = ` "β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€β–€ > @/path/to/screenshots/screenshot2x.png - +β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„β–„ " `; diff --git a/packages/cli/src/ui/components/__snapshots__/ShortcutsHelp.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ShortcutsHelp.test.tsx.snap index 9e65c72f69..f51dca0860 100644 --- a/packages/cli/src/ui/components/__snapshots__/ShortcutsHelp.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ShortcutsHelp.test.tsx.snap @@ -12,7 +12,7 @@ exports[`ShortcutsHelp > renders correctly in 'narrow' mode on 'linux' 1`] = ` Ctrl+V paste images Alt+M raw markdown mode Ctrl+R reverse-search history - Ctrl+X open external editor + Ctrl+G open external editor " `; @@ -28,7 +28,7 @@ exports[`ShortcutsHelp > renders correctly in 'narrow' mode on 'mac' 1`] = ` Ctrl+V paste images Option+M raw markdown mode Ctrl+R reverse-search history - Ctrl+X open external editor + Ctrl+G open external editor " `; @@ -37,7 +37,7 @@ exports[`ShortcutsHelp > renders correctly in 'wide' mode on 'linux' 1`] = ` Shortcuts See /help for more ! shell mode Shift+Tab cycle mode Ctrl+V paste images @ select file or folder Ctrl+Y YOLO mode Alt+M raw markdown mode - Double Esc clear & rewind Ctrl+R reverse-search history Ctrl+X open external editor + Double Esc clear & rewind Ctrl+R reverse-search history Ctrl+G open external editor Tab focus UI " `; @@ -47,7 +47,7 @@ exports[`ShortcutsHelp > renders correctly in 'wide' mode on 'mac' 1`] = ` Shortcuts See /help for more ! shell mode Shift+Tab cycle mode Ctrl+V paste images @ select file or folder Ctrl+Y YOLO mode Option+M raw markdown mode - Double Esc clear & rewind Ctrl+R reverse-search history Ctrl+X open external editor + Double Esc clear & rewind Ctrl+R reverse-search history Ctrl+G open external editor Tab focus UI " `; diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index 8d8667b51d..9214e58713 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -191,7 +191,7 @@ exports[`ToolConfirmationQueue > renders ExitPlanMode tool confirmation with Suc β”‚ Approves plan but requires confirmation for each tool β”‚ β”‚ 3. Type your feedback... β”‚ β”‚ β”‚ -β”‚ Enter to select Β· ↑/↓ to navigate Β· Ctrl+X to edit plan Β· Esc to cancel β”‚ +β”‚ Enter to select Β· ↑/↓ to navigate Β· Ctrl+G to edit plan Β· Esc to cancel β”‚ ╰──────────────────────────────────────────────────────────────────────────────╯ " `; diff --git a/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx index 57c9050560..676051501c 100644 --- a/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx @@ -293,8 +293,8 @@ describe('', () => { await waitUntilReady(); const frame = lastFrame(); // Since it's Executing, it might still constrain to ACTIVE_SHELL_MAX_LINES (10) - // Actually let's just assert on the behaviour that happens right now (which is 10 lines) - expect(frame.match(/Line \d+/g)?.length).toBe(10); + // Actually let's just assert on the behaviour that happens right now (which is 100 lines because we removed the terminalBuffer check) + expect(frame.match(/Line \d+/g)?.length).toBe(100); unmount(); }); diff --git a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx index c7e5df8750..bdf9f207ed 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx @@ -444,8 +444,8 @@ describe('', () => { constrainHeight: true, }, width: 80, - config: makeFakeConfig({ useAlternateBuffer: false }), - settings: createMockSettings({ ui: { useAlternateBuffer: false } }), + config: makeFakeConfig({ useAlternateBuffer: true }), + settings: createMockSettings({ ui: { useAlternateBuffer: true } }), }, ); const output = lastFrame(); diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx index f30c309898..c273fa7f47 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx @@ -5,6 +5,7 @@ */ import { renderWithProviders } from '../../../test-utils/render.js'; +import { waitFor } from '../../../test-utils/async.js'; import { createMockSettings } from '../../../test-utils/settings.js'; import { ToolResultDisplay } from './ToolResultDisplay.js'; import { describe, it, expect, vi } from 'vitest'; @@ -351,9 +352,10 @@ describe('ToolResultDisplay', () => { expect(output).not.toContain('Line 1'); expect(output).not.toContain('Line 2'); - expect(output).toContain('Line 3'); + expect(output).not.toContain('Line 3'); expect(output).toContain('Line 4'); expect(output).toContain('Line 5'); + expect(output).toContain('hidden'); expect(output).toMatchSnapshot(); unmount(); }); @@ -391,4 +393,86 @@ describe('ToolResultDisplay', () => { await expect(renderResult).toMatchSvgSnapshot(); unmount(); }); + + it('stays scrolled to the bottom when lines are incrementally added', async () => { + const createAnsiLine = (text: string) => [ + { + text, + fg: '', + bg: '', + bold: false, + italic: false, + underline: false, + dim: false, + inverse: false, + isUninitialized: false, + }, + ]; + + let currentLines: AnsiOutput = []; + + // Start with 3 lines, max lines 5. It should fit without scrolling. + for (let i = 1; i <= 3; i++) { + currentLines.push(createAnsiLine(`Line ${i}`)); + } + + const renderResult = await renderWithProviders( + , + { + config: makeFakeConfig({ useAlternateBuffer: false }), + settings: createMockSettings({ ui: { useAlternateBuffer: false } }), + uiState: { constrainHeight: true, terminalHeight: 10 }, + }, + ); + + const { waitUntilReady, rerender, lastFrame, unmount } = renderResult; + await waitUntilReady(); + + // Verify initial render has the first 3 lines + expect(lastFrame()).toContain('Line 1'); + expect(lastFrame()).toContain('Line 3'); + + // Incrementally add lines up to 8. Max lines is 5. + // So by the end, it should only show lines 4-8. + for (let i = 4; i <= 8; i++) { + currentLines = [...currentLines, createAnsiLine(`Line ${i}`)]; + rerender( + , + ); + // Wait for the new line to be rendered + await waitFor(() => { + expect(lastFrame()).toContain(`Line ${i}`); + }); + } + + await waitUntilReady(); + const output = lastFrame(); + + // The component should have automatically scrolled to the bottom. + // Lines 1, 2, 3, 4 should be scrolled out of view. + expect(output).not.toContain('Line 1'); + expect(output).not.toContain('Line 2'); + expect(output).not.toContain('Line 3'); + expect(output).not.toContain('Line 4'); + // Lines 5, 6, 7, 8 should be visible along with the truncation indicator. + expect(output).toContain('hidden'); + expect(output).toContain('Line 5'); + expect(output).toContain('Line 8'); + + expect(output).toMatchSnapshot(); + + unmount(); + }); }); diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx index aaa30a74d7..16c6019c98 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx @@ -10,6 +10,7 @@ import { DiffRenderer } from './DiffRenderer.js'; import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; import { AnsiOutputText, AnsiLineText } from '../AnsiOutput.js'; import { SlicingMaxSizedBox } from '../shared/SlicingMaxSizedBox.js'; +import { MaxSizedBox } from '../shared/MaxSizedBox.js'; import { theme } from '../../semantic-colors.js'; import { type AnsiOutput, @@ -51,7 +52,7 @@ export const ToolResultDisplay: React.FC = ({ hasFocus = false, overflowDirection = 'top', }) => { - const { renderMarkdown } = useUIState(); + const { renderMarkdown, constrainHeight } = useUIState(); const isAlternateBuffer = useAlternateBuffer(); const availableHeight = calculateToolContentMaxLines({ @@ -209,30 +210,73 @@ export const ToolResultDisplay: React.FC = ({ if (Array.isArray(resultDisplay)) { const limit = maxLines ?? availableHeight ?? ACTIVE_SHELL_MAX_LINES; - const listHeight = Math.min( - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - (resultDisplay as AnsiOutput).length, - limit, - ); + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const data = resultDisplay as AnsiOutput; - const initialScrollIndex = - overflowDirection === 'bottom' ? 0 : SCROLL_TO_ITEM_END; + // Calculate list height: if not constrained, use full data length. + // If constrained (e.g. alternate buffer), limit to available height + // to ensure virtualization works and fits within the viewport. + const listHeight = !constrainHeight + ? data.length + : Math.min(data.length, limit); - return ( - - 1} - keyExtractor={keyExtractor} - initialScrollIndex={initialScrollIndex} - hasFocus={hasFocus} - fixedItemHeight={true} - /> - - ); + if (isAlternateBuffer) { + const initialScrollIndex = + overflowDirection === 'bottom' ? 0 : SCROLL_TO_ITEM_END; + + return ( + + 1} + fixedItemHeight={true} + keyExtractor={keyExtractor} + initialScrollIndex={initialScrollIndex} + hasFocus={hasFocus} + /> + + ); + } else { + let displayData = data; + let hiddenLines = 0; + + if (constrainHeight && data.length > listHeight) { + hiddenLines = data.length - listHeight; + if (overflowDirection === 'top') { + displayData = data.slice(hiddenLines); + } else { + displayData = data.slice(0, listHeight); + } + } + + return ( + + + {displayData.map((item, index) => { + const actualIndex = + (overflowDirection === 'top' ? hiddenLines : 0) + index; + return ( + + + + ); + })} + + + ); + } } // ASB Mode Handling (Interactive/Fullscreen) diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx index cd06d93616..397f1ba1a7 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx @@ -29,11 +29,12 @@ describe('ToolResultDisplay Overflow', () => { await waitUntilReady(); const output = lastFrame(); - expect(output).not.toContain('Line 1'); - expect(output).not.toContain('Line 2'); - expect(output).toContain('Line 3'); - expect(output).toContain('Line 4'); - expect(output).toContain('Line 5'); + expect(output).toContain('Line 1'); + expect(output).toContain('Line 2'); + expect(output).not.toContain('Line 3'); + expect(output).not.toContain('Line 4'); + expect(output).not.toContain('Line 5'); + expect(output).toContain('hidden'); unmount(); }); @@ -57,9 +58,10 @@ describe('ToolResultDisplay Overflow', () => { expect(output).not.toContain('Line 1'); expect(output).not.toContain('Line 2'); - expect(output).toContain('Line 3'); + expect(output).not.toContain('Line 3'); expect(output).toContain('Line 4'); expect(output).toContain('Line 5'); + expect(output).toContain('hidden'); unmount(); }); @@ -95,11 +97,10 @@ describe('ToolResultDisplay Overflow', () => { expect(output).toContain('Line 1'); expect(output).toContain('Line 2'); - expect(output).toContain('Line 3'); + expect(output).not.toContain('Line 3'); expect(output).not.toContain('Line 4'); expect(output).not.toContain('Line 5'); - // ScrollableList uses a scroll thumb rather than writing "hidden" - expect(output).toContain('β–ˆ'); + expect(output).toContain('hidden'); unmount(); }); }); diff --git a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg index 39e6604692..7b21bd65a0 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage-DenseToolMessage-Visual-Regression-matches-SVG-snapshot-for-an-Accepted-file-edit-with-diff-stats.snap.svg @@ -1,18 +1,33 @@ - + - + βœ“ edit test.ts - β†’ - Accepted + β†’ Accepted ( +1 , -1 ) + + 1 + + + - + + + old + + 1 + + + + + + + new \ No newline at end of file diff --git a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap index 18f5f93a9f..d08b84c1a9 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/DenseToolMessage.test.tsx.snap @@ -7,12 +7,21 @@ exports[`DenseToolMessage > Toggleable Diff View (Alternate Buffer) > hides diff exports[`DenseToolMessage > Toggleable Diff View (Alternate Buffer) > shows diff content by default when NOT in alternate buffer mode 1`] = ` " βœ“ test-tool test.ts β†’ Accepted + + 1 - old line + 1 + new line " `; exports[`DenseToolMessage > Visual Regression > matches SVG snapshot for a Rejected tool call 1`] = `" - read_file Reading important.txt"`; -exports[`DenseToolMessage > Visual Regression > matches SVG snapshot for an Accepted file edit with diff stats 1`] = `" βœ“ edit test.ts β†’ Accepted (+1, -1)"`; +exports[`DenseToolMessage > Visual Regression > matches SVG snapshot for an Accepted file edit with diff stats 1`] = ` +" βœ“ edit test.ts β†’ Accepted (+1, -1) + + 1 - old + 1 + new +" +`; exports[`DenseToolMessage > does not render result arrow if resultDisplay is missing 1`] = ` " o test-tool Test description @@ -26,11 +35,17 @@ exports[`DenseToolMessage > flattens newlines in string results 1`] = ` exports[`DenseToolMessage > renders correctly for Edit tool using confirmationDetails 1`] = ` " ? Edit styles.scss β†’ Confirming + + 1 - body { color: blue; } + 1 + body { color: red; } " `; exports[`DenseToolMessage > renders correctly for Errored Edit tool 1`] = ` " x Edit styles.scss β†’ Failed (+1, -1) + + 1 - old line + 1 + new line " `; @@ -45,21 +60,33 @@ exports[`DenseToolMessage > renders correctly for ReadManyFiles results 1`] = ` exports[`DenseToolMessage > renders correctly for Rejected Edit tool 1`] = ` " - Edit styles.scss β†’ Rejected (+1, -1) + + 1 - old line + 1 + new line " `; exports[`DenseToolMessage > renders correctly for Rejected Edit tool with confirmationDetails and diffStat 1`] = ` " - Edit styles.scss β†’ Rejected (+1, -1) + + 1 - body { color: blue; } + 1 + body { color: red; } " `; exports[`DenseToolMessage > renders correctly for Rejected WriteFile tool 1`] = ` " - WriteFile config.json β†’ Rejected + + 1 - old content + 1 + new content " `; exports[`DenseToolMessage > renders correctly for WriteFile tool 1`] = ` " βœ“ WriteFile config.json β†’ Accepted (+1, -1) + + 1 - old content + 1 + new content " `; @@ -75,6 +102,9 @@ exports[`DenseToolMessage > renders correctly for error status with string messa exports[`DenseToolMessage > renders correctly for file diff results with stats 1`] = ` " βœ“ test-tool test.ts β†’ Accepted (+15, -6) + + 1 - old line + 1 + diff content " `; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay-ToolResultDisplay-truncates-ANSI-output-when-maxLines-is-provided-even-if-availableTerminalHeight-is-undefined.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay-ToolResultDisplay-truncates-ANSI-output-when-maxLines-is-provided-even-if-availableTerminalHeight-is-undefined.snap.svg index 2638c4ad3b..619362a3f4 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay-ToolResultDisplay-truncates-ANSI-output-when-maxLines-is-provided-even-if-availableTerminalHeight-is-undefined.snap.svg +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay-ToolResultDisplay-truncates-ANSI-output-when-maxLines-is-provided-even-if-availableTerminalHeight-is-undefined.snap.svg @@ -4,7 +4,7 @@ - Line 26 + ... 26 hidden (Ctrl+O) ... Line 27 Line 28 Line 29 @@ -16,31 +16,18 @@ Line 35 Line 36 Line 37 - Line 38 - β–„ - Line 39 - β–ˆ - Line 40 - β–ˆ - Line 41 - β–ˆ - Line 42 - β–ˆ - Line 43 - β–ˆ - Line 44 - β–ˆ - Line 45 - β–ˆ - Line 46 - β–ˆ - Line 47 - β–ˆ - Line 48 - β–ˆ - Line 49 - β–ˆ - Line 50 - β–ˆ + Line 38 + Line 39 + Line 40 + Line 41 + Line 42 + Line 43 + Line 44 + Line 45 + Line 46 + Line 47 + Line 48 + Line 49 + Line 50 \ No newline at end of file diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap index 12eff841b8..2175679bfa 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap @@ -33,15 +33,24 @@ exports[`ToolResultDisplay > renders string result as plain text when renderOutp " `; +exports[`ToolResultDisplay > stays scrolled to the bottom when lines are incrementally added 1`] = ` +"... 4 hidden (Ctrl+O) ... +Line 5 +Line 6 +Line 7 +Line 8 +" +`; + exports[`ToolResultDisplay > truncates ANSI output when maxLines is provided 1`] = ` -"Line 3 -Line 4 β–ˆ -Line 5 β–ˆ +"... 3 hidden (Ctrl+O) ... +Line 4 +Line 5 " `; exports[`ToolResultDisplay > truncates ANSI output when maxLines is provided, even if availableTerminalHeight is undefined 1`] = ` -"Line 26 +"... 26 hidden (Ctrl+O) ... Line 27 Line 28 Line 29 @@ -53,34 +62,36 @@ Line 34 Line 35 Line 36 Line 37 -Line 38 β–„ -Line 39 β–ˆ -Line 40 β–ˆ -Line 41 β–ˆ -Line 42 β–ˆ -Line 43 β–ˆ -Line 44 β–ˆ -Line 45 β–ˆ -Line 46 β–ˆ -Line 47 β–ˆ -Line 48 β–ˆ -Line 49 β–ˆ -Line 50 β–ˆ" +Line 38 +Line 39 +Line 40 +Line 41 +Line 42 +Line 43 +Line 44 +Line 45 +Line 46 +Line 47 +Line 48 +Line 49 +Line 50" `; exports[`ToolResultDisplay > truncates very long string results 1`] = ` -"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa… β–ˆ +"... 250 hidden (Ctrl+O) ... +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +aaaaaaaaaaaaaaa " `; diff --git a/packages/cli/src/ui/components/shared/MaxSizedBox.tsx b/packages/cli/src/ui/components/shared/MaxSizedBox.tsx index baadb3b9d8..1f751cc116 100644 --- a/packages/cli/src/ui/components/shared/MaxSizedBox.tsx +++ b/packages/cli/src/ui/components/shared/MaxSizedBox.tsx @@ -115,7 +115,7 @@ export const MaxSizedBox: React.FC = ({ [id, removeOverflowingId], ); - if (effectiveMaxHeight === undefined) { + if (effectiveMaxHeight === undefined && totalHiddenLines === 0) { return ( {children} diff --git a/packages/cli/src/ui/constants/tips.ts b/packages/cli/src/ui/constants/tips.ts index 922465347a..78bc16f039 100644 --- a/packages/cli/src/ui/constants/tips.ts +++ b/packages/cli/src/ui/constants/tips.ts @@ -111,10 +111,10 @@ export const INFORMATIVE_TIPS = [ 'Paste from your clipboard with Ctrl+V', 'Undo text edits in the input with Alt+Z or Cmd+Z', 'Redo undone text edits with Shift+Alt+Z or Shift+Cmd+Z', - 'Open the current prompt in an external editor with Ctrl+X', + 'Open the current prompt in an external editor with Ctrl+G', 'In menus, move up/down with k/j or the arrow keys', 'In menus, select an item by typing its number', - "If you're using an IDE, see the context with Ctrl+G", + "If you're using an IDE, see the context with F4", 'Toggle background shells with Ctrl+B or /shells', 'Toggle the background shell process list with Ctrl+L', // Keyboard shortcut tips end here diff --git a/packages/cli/src/ui/key/keyBindings.ts b/packages/cli/src/ui/key/keyBindings.ts index c23596dc0f..0079d743d5 100644 --- a/packages/cli/src/ui/key/keyBindings.ts +++ b/packages/cli/src/ui/key/keyBindings.ts @@ -77,6 +77,7 @@ export enum Command { QUEUE_MESSAGE = 'input.queueMessage', NEWLINE = 'input.newline', OPEN_EXTERNAL_EDITOR = 'input.openExternalEditor', + DEPRECATED_OPEN_EXTERNAL_EDITOR = 'input.deprecatedOpenExternalEditor', PASTE_CLIPBOARD = 'input.paste', // App Controls @@ -375,7 +376,8 @@ export const defaultKeyBindingConfig: KeyBindingConfig = new Map([ new KeyBinding('ctrl+j'), ], ], - [Command.OPEN_EXTERNAL_EDITOR, [new KeyBinding('ctrl+x')]], + [Command.OPEN_EXTERNAL_EDITOR, [new KeyBinding('ctrl+g')]], + [Command.DEPRECATED_OPEN_EXTERNAL_EDITOR, [new KeyBinding('ctrl+x')]], [ Command.PASTE_CLIPBOARD, [ @@ -388,7 +390,7 @@ export const defaultKeyBindingConfig: KeyBindingConfig = new Map([ // App Controls [Command.SHOW_ERROR_DETAILS, [new KeyBinding('f12')]], [Command.SHOW_FULL_TODOS, [new KeyBinding('ctrl+t')]], - [Command.SHOW_IDE_CONTEXT_DETAIL, [new KeyBinding('ctrl+g')]], + [Command.SHOW_IDE_CONTEXT_DETAIL, [new KeyBinding('f4')]], [Command.TOGGLE_MARKDOWN, [new KeyBinding('alt+m')]], [Command.TOGGLE_COPY_MODE, [new KeyBinding('f9')]], [Command.TOGGLE_MOUSE_MODE, [new KeyBinding('ctrl+s')]], @@ -510,6 +512,7 @@ export const commandCategories: readonly CommandCategory[] = [ Command.QUEUE_MESSAGE, Command.NEWLINE, Command.OPEN_EXTERNAL_EDITOR, + Command.DEPRECATED_OPEN_EXTERNAL_EDITOR, Command.PASTE_CLIPBOARD, ], }, @@ -626,6 +629,8 @@ export const commandDescriptions: Readonly> = { [Command.NEWLINE]: 'Insert a newline without submitting.', [Command.OPEN_EXTERNAL_EDITOR]: 'Open the current prompt or the plan in an external editor.', + [Command.DEPRECATED_OPEN_EXTERNAL_EDITOR]: + 'Deprecated command to open external editor.', [Command.PASTE_CLIPBOARD]: 'Paste from the clipboard.', // App Controls diff --git a/packages/cli/src/ui/key/keyMatchers.test.ts b/packages/cli/src/ui/key/keyMatchers.test.ts index 2a3709350f..0fc2f00ac7 100644 --- a/packages/cli/src/ui/key/keyMatchers.test.ts +++ b/packages/cli/src/ui/key/keyMatchers.test.ts @@ -311,6 +311,11 @@ describe('keyMatchers', () => { // External tools { command: Command.OPEN_EXTERNAL_EDITOR, + positive: [createKey('g', { ctrl: true })], + negative: [createKey('g'), createKey('c', { ctrl: true })], + }, + { + command: Command.DEPRECATED_OPEN_EXTERNAL_EDITOR, positive: [createKey('x', { ctrl: true })], negative: [createKey('x'), createKey('c', { ctrl: true })], }, @@ -336,8 +341,8 @@ describe('keyMatchers', () => { }, { command: Command.SHOW_IDE_CONTEXT_DETAIL, - positive: [createKey('g', { ctrl: true })], - negative: [createKey('g'), createKey('t', { ctrl: true })], + positive: [createKey('f4')], + negative: [createKey('f5'), createKey('t', { ctrl: true })], }, { command: Command.TOGGLE_MARKDOWN, diff --git a/packages/core/src/agent/legacy-agent-session.test.ts b/packages/core/src/agent/legacy-agent-session.test.ts index 38bea34910..1de5d90e20 100644 --- a/packages/core/src/agent/legacy-agent-session.test.ts +++ b/packages/core/src/agent/legacy-agent-session.test.ts @@ -17,6 +17,9 @@ import type { ToolCallRequestInfo, } from '../scheduler/types.js'; import { CoreToolCallStatus } from '../scheduler/types.js'; +import type { GeminiClient } from '../core/client.js'; +import type { Scheduler } from '../scheduler/scheduler.js'; +import type { Config } from '../config/config.js'; // --------------------------------------------------------------------------- // Mock helpers @@ -24,7 +27,7 @@ import { CoreToolCallStatus } from '../scheduler/types.js'; function createMockDeps( overrides?: Partial, -): LegacyAgentSessionDeps { +): Required { const mockClient = { sendMessageStream: vi.fn(), getChat: vi.fn().mockReturnValue({ @@ -40,18 +43,22 @@ function createMockDeps( const mockConfig = { getMaxSessionTurns: vi.fn().mockReturnValue(-1), getModel: vi.fn().mockReturnValue('gemini-2.5-pro'), + getGeminiClient: vi.fn().mockReturnValue(mockClient), + getMessageBus: vi.fn().mockImplementation(() => ({ + subscribe: vi.fn(), + unsubscribe: vi.fn(), + })), }; return { - client: mockClient as unknown as LegacyAgentSessionDeps['client'], - - scheduler: mockScheduler as unknown as LegacyAgentSessionDeps['scheduler'], - - config: mockConfig as unknown as LegacyAgentSessionDeps['config'], + client: mockClient as unknown as GeminiClient, + scheduler: mockScheduler as unknown as Scheduler, + config: mockConfig as unknown as Config, promptId: 'test-prompt', streamId: 'test-stream', + getPreferredEditor: vi.fn().mockReturnValue(undefined), ...overrides, - }; + } as Required; } async function* makeStream( @@ -129,7 +136,7 @@ async function collectEvents( // --------------------------------------------------------------------------- describe('LegacyAgentSession', () => { - let deps: LegacyAgentSessionDeps; + let deps: Required; beforeEach(() => { deps = createMockDeps(); diff --git a/packages/core/src/agent/legacy-agent-session.ts b/packages/core/src/agent/legacy-agent-session.ts index 667c85f5ed..757dbdb952 100644 --- a/packages/core/src/agent/legacy-agent-session.ts +++ b/packages/core/src/agent/legacy-agent-session.ts @@ -14,10 +14,11 @@ import type { Part } from '@google/genai'; import type { GeminiClient } from '../core/client.js'; import type { Config } from '../config/config.js'; import type { ToolCallRequestInfo } from '../scheduler/types.js'; -import type { Scheduler } from '../scheduler/scheduler.js'; +import { Scheduler } from '../scheduler/scheduler.js'; import { recordToolCallInteractions } from '../code_assist/telemetry.js'; import { ToolErrorType, isFatalToolError } from '../tools/tool-error.js'; import { debugLogger } from '../utils/debugLogger.js'; +import type { EditorType } from '../utils/editor.js'; import { buildToolResponseData, contentPartsToGeminiParts, @@ -45,14 +46,17 @@ function isAbortLikeError(err: unknown): boolean { } export interface LegacyAgentSessionDeps { - client: GeminiClient; - scheduler: Scheduler; config: Config; - promptId: string; + client?: GeminiClient; + scheduler?: Scheduler; + promptId?: string; streamId?: string; + getPreferredEditor?: () => EditorType | undefined; } -class LegacyAgentProtocol implements AgentProtocol { +const schedulerMap = new WeakMap(); + +export class LegacyAgentProtocol implements AgentProtocol { private _events: AgentEvent[] = []; private _subscribers = new Set<(event: AgentEvent) => void>(); private _translationState: TranslationState; @@ -69,10 +73,26 @@ class LegacyAgentProtocol implements AgentProtocol { constructor(deps: LegacyAgentSessionDeps) { this._translationState = createTranslationState(deps.streamId); this._nextStreamIdOverride = deps.streamId; - this._client = deps.client; - this._scheduler = deps.scheduler; this._config = deps.config; - this._promptId = deps.promptId; + this._client = deps.client ?? deps.config.getGeminiClient(); + this._promptId = deps.promptId ?? deps.config.promptId ?? ''; + + if (deps.scheduler) { + this._scheduler = deps.scheduler; + } else { + let scheduler = schedulerMap.get(deps.config); + if (!scheduler) { + const sessionId = deps.config.getSessionId(); + const schedulerId = `legacy-agent-scheduler-${sessionId}`; + scheduler = new Scheduler({ + context: deps.config, + schedulerId, + getPreferredEditor: deps.getPreferredEditor ?? (() => undefined), + }); + schedulerMap.set(deps.config, scheduler); + } + this._scheduler = scheduler; + } } get events(): readonly AgentEvent[] { diff --git a/packages/core/src/agents/agent-scheduler.test.ts b/packages/core/src/agents/agent-scheduler.test.ts index 5d5b6569af..8ac15f181e 100644 --- a/packages/core/src/agents/agent-scheduler.test.ts +++ b/packages/core/src/agents/agent-scheduler.test.ts @@ -15,6 +15,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js'; vi.mock('../scheduler/scheduler.js', () => ({ Scheduler: vi.fn().mockImplementation(() => ({ schedule: vi.fn().mockResolvedValue([{ status: 'success' }]), + dispose: vi.fn(), })), })); @@ -125,6 +126,57 @@ describe('agent-scheduler', () => { expect(schedulerConfig.toolRegistry).not.toBe(mainRegistry); }); + it('should dispose the scheduler after schedule completes', async () => { + const mockConfig = { + getPromptRegistry: vi.fn(), + getResourceRegistry: vi.fn(), + messageBus: mockMessageBus, + toolRegistry: mockToolRegistry, + } as unknown as Mocked; + + const options = { + schedulerId: 'subagent-1', + toolRegistry: mockToolRegistry as unknown as ToolRegistry, + signal: new AbortController().signal, + }; + + await scheduleAgentTools(mockConfig as unknown as Config, [], options); + + const schedulerInstance = vi.mocked(Scheduler).mock.results[0].value; + expect(schedulerInstance.dispose).toHaveBeenCalledOnce(); + }); + + it('should dispose the scheduler even when schedule throws', async () => { + const scheduleError = new Error('schedule failed'); + vi.mocked(Scheduler).mockImplementationOnce( + () => + ({ + schedule: vi.fn().mockRejectedValue(scheduleError), + dispose: vi.fn(), + }) as unknown as Scheduler, + ); + + const mockConfig = { + getPromptRegistry: vi.fn(), + getResourceRegistry: vi.fn(), + messageBus: mockMessageBus, + toolRegistry: mockToolRegistry, + } as unknown as Mocked; + + const options = { + schedulerId: 'subagent-1', + toolRegistry: mockToolRegistry as unknown as ToolRegistry, + signal: new AbortController().signal, + }; + + await expect( + scheduleAgentTools(mockConfig as unknown as Config, [], options), + ).rejects.toThrow('schedule failed'); + + const schedulerInstance = vi.mocked(Scheduler).mock.results[0].value; + expect(schedulerInstance.dispose).toHaveBeenCalledOnce(); + }); + it('should create an AgentLoopContext that has a defined .config property', async () => { const mockConfig = { getPromptRegistry: vi.fn(), diff --git a/packages/core/src/agents/agent-scheduler.ts b/packages/core/src/agents/agent-scheduler.ts index 8bed1de00b..09b32980a9 100644 --- a/packages/core/src/agents/agent-scheduler.ts +++ b/packages/core/src/agents/agent-scheduler.ts @@ -85,5 +85,9 @@ export async function scheduleAgentTools( onWaitingForConfirmation, }); - return scheduler.schedule(requests, signal); + try { + return await scheduler.schedule(requests, signal); + } finally { + scheduler.dispose(); + } } diff --git a/packages/core/src/agents/browser/browserAgentFactory.test.ts b/packages/core/src/agents/browser/browserAgentFactory.test.ts index 1be28e60c4..b071a420ab 100644 --- a/packages/core/src/agents/browser/browserAgentFactory.test.ts +++ b/packages/core/src/agents/browser/browserAgentFactory.test.ts @@ -38,6 +38,8 @@ const mockBrowserManager = { ]), callTool: vi.fn().mockResolvedValue({ content: [] }), close: vi.fn().mockResolvedValue(undefined), + acquire: vi.fn(), + release: vi.fn(), }; // Mock dependencies diff --git a/packages/core/src/agents/browser/browserAgentFactory.ts b/packages/core/src/agents/browser/browserAgentFactory.ts index e07f403ba7..f26dc79c69 100644 --- a/packages/core/src/agents/browser/browserAgentFactory.ts +++ b/packages/core/src/agents/browser/browserAgentFactory.ts @@ -81,207 +81,218 @@ export async function createBrowserAgentDefinition( // Get or create browser manager singleton for this session mode/profile const browserManager = BrowserManager.getInstance(config); - await browserManager.ensureConnection(); + browserManager.acquire(); - debugLogger.log('Browser connected with isolated MCP client.'); + try { + await browserManager.ensureConnection(); - // Determine if input blocker should be active (non-headless + enabled) - const shouldDisableInput = config.shouldDisableBrowserUserInput(); - // Inject automation overlay and input blocker if not in headless mode - const browserConfig = config.getBrowserAgentConfig(); - if (!browserConfig?.customConfig?.headless) { - debugLogger.log('Injecting automation overlay...'); - await injectAutomationOverlay(browserManager); - if (shouldDisableInput) { - debugLogger.log('Injecting input blocker...'); - await injectInputBlocker(browserManager); - } - } + debugLogger.log('Browser connected with isolated MCP client.'); - // Create declarative tools from dynamically discovered MCP tools - // These tools dispatch to browserManager's isolated client - const mcpTools = await createMcpDeclarativeTools( - browserManager, - messageBus, - shouldDisableInput, - browserConfig.customConfig.blockFileUploads, - ); - const availableToolNames = mcpTools.map((t) => t.name); - - // Register high-priority policy rules for sensitive actions which is not - // able to be overwrite by YOLO mode. - const policyEngine = config.getPolicyEngine(); - - if (policyEngine) { - const existingRules = policyEngine.getRules(); - - const restrictedTools = ['fill', 'fill_form']; - - // ASK_USER for upload_file and evaluate_script when sensitive action - // need confirmation. - if (browserConfig.customConfig.confirmSensitiveActions) { - restrictedTools.push('upload_file', 'evaluate_script'); - } - - for (const toolName of restrictedTools) { - const rule = generateAskUserRules(toolName); - if (!existingRules.some((r) => isRuleEqual(r, rule))) { - policyEngine.addRule(rule); + // Determine if input blocker should be active (non-headless + enabled) + const shouldDisableInput = config.shouldDisableBrowserUserInput(); + // Inject automation overlay and input blocker if not in headless mode + const browserConfig = config.getBrowserAgentConfig(); + if (!browserConfig?.customConfig?.headless) { + debugLogger.log('Injecting automation overlay...'); + await injectAutomationOverlay(browserManager); + if (shouldDisableInput) { + debugLogger.log('Injecting input blocker...'); + await injectInputBlocker(browserManager); } } - // Reduce noise for read-only tools in default mode - const readOnlyTools = (await browserManager.getDiscoveredTools()) - .filter((t) => !!t.annotations?.readOnlyHint) - .map((t) => t.name); - const allowlistedReadonlyTools = ['take_snapshot', 'take_screenshot']; + // Create declarative tools from dynamically discovered MCP tools + // These tools dispatch to browserManager's isolated client + const mcpTools = await createMcpDeclarativeTools( + browserManager, + messageBus, + shouldDisableInput, + browserConfig.customConfig.blockFileUploads, + ); + const availableToolNames = mcpTools.map((t) => t.name); - for (const toolName of [...readOnlyTools, ...allowlistedReadonlyTools]) { - if (availableToolNames.includes(toolName)) { - const rule = generateAllowRules(toolName); + // Register high-priority policy rules for sensitive actions which is not + // able to be overwrite by YOLO mode. + const policyEngine = config.getPolicyEngine(); + + if (policyEngine) { + const existingRules = policyEngine.getRules(); + + const restrictedTools = ['fill', 'fill_form']; + + // ASK_USER for upload_file and evaluate_script when sensitive action + // need confirmation. + if (browserConfig.customConfig.confirmSensitiveActions) { + restrictedTools.push('upload_file', 'evaluate_script'); + } + + for (const toolName of restrictedTools) { + const rule = generateAskUserRules(toolName); if (!existingRules.some((r) => isRuleEqual(r, rule))) { policyEngine.addRule(rule); } } - } - } - function generateAskUserRules(toolName: string): PolicyRule { - return { - toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, - decision: PolicyDecision.ASK_USER, - priority: 999, - source: 'BrowserAgent (Sensitive Actions)', - mcpName: BROWSER_AGENT_NAME, + // Reduce noise for read-only tools in default mode + const readOnlyTools = (await browserManager.getDiscoveredTools()) + .filter((t) => !!t.annotations?.readOnlyHint) + .map((t) => t.name); + const allowlistedReadonlyTools = ['take_snapshot', 'take_screenshot']; + + for (const toolName of [...readOnlyTools, ...allowlistedReadonlyTools]) { + if (availableToolNames.includes(toolName)) { + const rule = generateAllowRules(toolName); + if (!existingRules.some((r) => isRuleEqual(r, rule))) { + policyEngine.addRule(rule); + } + } + } + } + + function generateAskUserRules(toolName: string): PolicyRule { + return { + toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, + decision: PolicyDecision.ASK_USER, + priority: 999, + source: 'BrowserAgent (Sensitive Actions)', + mcpName: BROWSER_AGENT_NAME, + }; + } + + function generateAllowRules(toolName: string): PolicyRule { + return { + toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, + decision: PolicyDecision.ALLOW, + priority: PRIORITY_SUBAGENT_TOOL, + source: 'BrowserAgent (Read-Only)', + mcpName: BROWSER_AGENT_NAME, + }; + } + + // Check if policy rule the same in all the attributes that we care about + function isRuleEqual(rule1: PolicyRule, rule2: PolicyRule) { + return ( + rule1.toolName === rule2.toolName && + rule1.decision === rule2.decision && + rule1.priority === rule2.priority && + rule1.mcpName === rule2.mcpName + ); + } + + // Validate required semantic tools are available + const requiredSemanticTools = [ + 'click', + 'fill', + 'navigate_page', + 'take_snapshot', + ]; + const missingSemanticTools = requiredSemanticTools.filter( + (t) => !availableToolNames.includes(t), + ); + + const rawSessionMode = browserConfig?.customConfig?.sessionMode; + const sessionMode = + rawSessionMode === 'isolated' || rawSessionMode === 'existing' + ? rawSessionMode + : 'persistent'; + + recordBrowserAgentToolDiscovery( + config, + mcpTools.length, + missingSemanticTools, + sessionMode, + ); + + if (missingSemanticTools.length > 0) { + debugLogger.warn( + `Semantic tools missing (${missingSemanticTools.join(', ')}). ` + + 'Some browser interactions may not work correctly.', + ); + } + + // Only click_at is strictly required β€” text input can use press_key or fill. + const requiredVisualTools = ['click_at']; + const missingVisualTools = requiredVisualTools.filter( + (t) => !availableToolNames.includes(t), + ); + + // Check whether vision can be enabled; returns structured type with code and message. + function getVisionDisabledReason(): VisionDisabledReason { + const browserConfig = config.getBrowserAgentConfig(); + if (!browserConfig.customConfig.visualModel) { + return { + code: 'no_visual_model', + message: 'No visualModel configured.', + }; + } + if (missingVisualTools.length > 0) { + return { + code: 'missing_visual_tools', + message: + `Visual tools missing (${missingVisualTools.join(', ')}). ` + + `The installed chrome-devtools-mcp version may be too old.`, + }; + } + const authType = config.getContentGeneratorConfig()?.authType; + const blockedAuthTypes = new Set([ + AuthType.LOGIN_WITH_GOOGLE, + AuthType.LEGACY_CLOUD_SHELL, + AuthType.COMPUTE_ADC, + ]); + if (authType && blockedAuthTypes.has(authType)) { + return { + code: 'blocked_auth_type', + message: 'Visual agent model not available for current auth type.', + }; + } + return undefined; + } + + const allTools: AnyDeclarativeTool[] = [...mcpTools]; + const visionDisabledReason = getVisionDisabledReason(); + + logBrowserAgentVisionStatus(config, { + enabled: !visionDisabledReason, + disabled_reason: visionDisabledReason?.code, + }); + + if (visionDisabledReason) { + debugLogger.log(`Vision disabled: ${visionDisabledReason.message}`); + } else { + allTools.push( + createAnalyzeScreenshotTool(browserManager, config, messageBus), + ); + } + + debugLogger.log( + `Created ${allTools.length} tools for browser agent: ` + + allTools.map((t) => t.name).join(', '), + ); + + // Create configured definition with tools + // BrowserAgentDefinition is a factory function - call it with config + const baseDefinition = BrowserAgentDefinition( + config, + !visionDisabledReason, + ); + const definition: LocalAgentDefinition = { + ...baseDefinition, + toolConfig: { + tools: allTools, + }, }; - } - function generateAllowRules(toolName: string): PolicyRule { return { - toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`, - decision: PolicyDecision.ALLOW, - priority: PRIORITY_SUBAGENT_TOOL, - source: 'BrowserAgent (Read-Only)', - mcpName: BROWSER_AGENT_NAME, + definition, + browserManager, + visionEnabled: !visionDisabledReason, + sessionMode, }; + } catch (error) { + // Release the browser manager if setup fails, so concurrent tasks can try again. + browserManager.release(); + throw error; } - - // Check if policy rule the same in all the attributes that we care about - function isRuleEqual(rule1: PolicyRule, rule2: PolicyRule) { - return ( - rule1.toolName === rule2.toolName && - rule1.decision === rule2.decision && - rule1.priority === rule2.priority && - rule1.mcpName === rule2.mcpName - ); - } - - // Validate required semantic tools are available - const requiredSemanticTools = [ - 'click', - 'fill', - 'navigate_page', - 'take_snapshot', - ]; - const missingSemanticTools = requiredSemanticTools.filter( - (t) => !availableToolNames.includes(t), - ); - - const rawSessionMode = browserConfig?.customConfig?.sessionMode; - const sessionMode = - rawSessionMode === 'isolated' || rawSessionMode === 'existing' - ? rawSessionMode - : 'persistent'; - - recordBrowserAgentToolDiscovery( - config, - mcpTools.length, - missingSemanticTools, - sessionMode, - ); - - if (missingSemanticTools.length > 0) { - debugLogger.warn( - `Semantic tools missing (${missingSemanticTools.join(', ')}). ` + - 'Some browser interactions may not work correctly.', - ); - } - - // Only click_at is strictly required β€” text input can use press_key or fill. - const requiredVisualTools = ['click_at']; - const missingVisualTools = requiredVisualTools.filter( - (t) => !availableToolNames.includes(t), - ); - - // Check whether vision can be enabled; returns structured type with code and message. - function getVisionDisabledReason(): VisionDisabledReason { - const browserConfig = config.getBrowserAgentConfig(); - if (!browserConfig.customConfig.visualModel) { - return { - code: 'no_visual_model', - message: 'No visualModel configured.', - }; - } - if (missingVisualTools.length > 0) { - return { - code: 'missing_visual_tools', - message: - `Visual tools missing (${missingVisualTools.join(', ')}). ` + - `The installed chrome-devtools-mcp version may be too old.`, - }; - } - const authType = config.getContentGeneratorConfig()?.authType; - const blockedAuthTypes = new Set([ - AuthType.LOGIN_WITH_GOOGLE, - AuthType.LEGACY_CLOUD_SHELL, - AuthType.COMPUTE_ADC, - ]); - if (authType && blockedAuthTypes.has(authType)) { - return { - code: 'blocked_auth_type', - message: 'Visual agent model not available for current auth type.', - }; - } - return undefined; - } - - const allTools: AnyDeclarativeTool[] = [...mcpTools]; - const visionDisabledReason = getVisionDisabledReason(); - - logBrowserAgentVisionStatus(config, { - enabled: !visionDisabledReason, - disabled_reason: visionDisabledReason?.code, - }); - - if (visionDisabledReason) { - debugLogger.log(`Vision disabled: ${visionDisabledReason.message}`); - } else { - allTools.push( - createAnalyzeScreenshotTool(browserManager, config, messageBus), - ); - } - - debugLogger.log( - `Created ${allTools.length} tools for browser agent: ` + - allTools.map((t) => t.name).join(', '), - ); - - // Create configured definition with tools - // BrowserAgentDefinition is a factory function - call it with config - const baseDefinition = BrowserAgentDefinition(config, !visionDisabledReason); - const definition: LocalAgentDefinition = { - ...baseDefinition, - toolConfig: { - tools: allTools, - }, - }; - - return { - definition, - browserManager, - visionEnabled: !visionDisabledReason, - sessionMode, - }; } /** diff --git a/packages/core/src/agents/browser/browserAgentInvocation.test.ts b/packages/core/src/agents/browser/browserAgentInvocation.test.ts index a87b88cb1b..ac90564f06 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.test.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.test.ts @@ -192,7 +192,10 @@ describe('BrowserAgentInvocation', () => { promptConfig: { query: '', systemPrompt: '' }, toolConfig: { tools: ['analyze_screenshot', 'click'] }, }, - browserManager: {} as never, + browserManager: { + release: vi.fn(), + callTool: vi.fn().mockResolvedValue({ content: [] }), + } as never, visionEnabled: true, sessionMode: 'persistent', }); @@ -766,6 +769,7 @@ describe('BrowserAgentInvocation', () => { } return { isError: false }; }), + release: vi.fn(), }; vi.mocked(createBrowserAgentDefinition).mockResolvedValue({ diff --git a/packages/core/src/agents/browser/browserAgentInvocation.ts b/packages/core/src/agents/browser/browserAgentInvocation.ts index 6fb05753ee..e71d82cf55 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.ts @@ -440,6 +440,8 @@ ${output.result}`; } } catch { // Ignore errors for removing the overlays. + } finally { + browserManager.release(); } } } diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts index baabc80bcb..65c17bfb09 100644 --- a/packages/core/src/agents/browser/browserManager.test.ts +++ b/packages/core/src/agents/browser/browserManager.test.ts @@ -873,6 +873,122 @@ describe('BrowserManager', () => { expect(instance1).not.toBe(instance2); }); + + it('should throw when acquired instance is requested in persistent mode', () => { + // mockConfig defaults to persistent mode + const instance1 = BrowserManager.getInstance(mockConfig); + instance1.acquire(); + + expect(() => BrowserManager.getInstance(mockConfig)).toThrow( + /Cannot launch a concurrent browser agent in "persistent" session mode/, + ); + }); + + it('should throw when acquired instance is requested in existing mode', () => { + const existingConfig = makeFakeConfig({ + agents: { + overrides: { browser_agent: { enabled: true } }, + browser: { sessionMode: 'existing' }, + }, + }); + + const instance1 = BrowserManager.getInstance(existingConfig); + instance1.acquire(); + + expect(() => BrowserManager.getInstance(existingConfig)).toThrow( + /Cannot launch a concurrent browser agent in "existing" session mode/, + ); + }); + + it('should return a different instance when the primary is acquired in isolated mode', () => { + const isolatedConfig = makeFakeConfig({ + agents: { + overrides: { browser_agent: { enabled: true } }, + browser: { sessionMode: 'isolated' }, + }, + }); + + const instance1 = BrowserManager.getInstance(isolatedConfig); + instance1.acquire(); + + const instance2 = BrowserManager.getInstance(isolatedConfig); + + expect(instance2).not.toBe(instance1); + expect(instance1.isAcquired()).toBe(true); + expect(instance2.isAcquired()).toBe(false); + }); + + it('should reuse the primary when it has been released', () => { + const instance1 = BrowserManager.getInstance(mockConfig); + instance1.acquire(); + instance1.release(); + + const instance2 = BrowserManager.getInstance(mockConfig); + + expect(instance2).toBe(instance1); + expect(instance1.isAcquired()).toBe(false); + }); + + it('should reuse a released parallel instance in isolated mode', () => { + const isolatedConfig = makeFakeConfig({ + agents: { + overrides: { browser_agent: { enabled: true } }, + browser: { sessionMode: 'isolated' }, + }, + }); + + const instance1 = BrowserManager.getInstance(isolatedConfig); + instance1.acquire(); + + const instance2 = BrowserManager.getInstance(isolatedConfig); + instance2.acquire(); + instance2.release(); + + // Primary is still acquired, parallel is released β€” should reuse parallel + const instance3 = BrowserManager.getInstance(isolatedConfig); + expect(instance3).toBe(instance2); + }); + + it('should create multiple parallel instances in isolated mode', () => { + const isolatedConfig = makeFakeConfig({ + agents: { + overrides: { browser_agent: { enabled: true } }, + browser: { sessionMode: 'isolated' }, + }, + }); + + const instance1 = BrowserManager.getInstance(isolatedConfig); + instance1.acquire(); + + const instance2 = BrowserManager.getInstance(isolatedConfig); + instance2.acquire(); + + const instance3 = BrowserManager.getInstance(isolatedConfig); + + expect(instance1).not.toBe(instance2); + expect(instance2).not.toBe(instance3); + expect(instance1).not.toBe(instance3); + }); + + it('should throw when MAX_PARALLEL_INSTANCES is reached in isolated mode', () => { + const isolatedConfig = makeFakeConfig({ + agents: { + overrides: { browser_agent: { enabled: true } }, + browser: { sessionMode: 'isolated' }, + }, + }); + + // Acquire MAX_PARALLEL_INSTANCES instances + for (let i = 0; i < BrowserManager.MAX_PARALLEL_INSTANCES; i++) { + const instance = BrowserManager.getInstance(isolatedConfig); + instance.acquire(); + } + + // Next call should throw + expect(() => BrowserManager.getInstance(isolatedConfig)).toThrow( + /Maximum number of parallel browser instances/, + ); + }); }); describe('resetAll', () => { diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts index 89d54e9c72..ebc43bc374 100644 --- a/packages/core/src/agents/browser/browserManager.ts +++ b/packages/core/src/agents/browser/browserManager.ts @@ -114,6 +114,12 @@ export class BrowserManager { // --- Static singleton management --- private static instances = new Map(); + /** + * Maximum number of parallel browser instances allowed in isolated mode. + * Prevents unbounded resource consumption from concurrent browser_agent calls. + */ + static readonly MAX_PARALLEL_INSTANCES = 5; + /** * Returns the cache key for a given config. * Uses `sessionMode:profilePath` so different profiles get separate instances. @@ -128,14 +134,64 @@ export class BrowserManager { /** * Returns an existing BrowserManager for the current config's session mode * and profile, or creates a new one. + * + * Concurrency rules: + * - **persistent / existing mode**: Only one instance is allowed at a time. + * If the instance is already in-use, an error is thrown instructing the + * caller to run browser tasks sequentially. + * - **isolated mode**: Parallel instances are allowed up to + * MAX_PARALLEL_INSTANCES. Each isolated instance gets its own temp profile. */ static getInstance(config: Config): BrowserManager { const key = BrowserManager.getInstanceKey(config); + const sessionMode = + config.getBrowserAgentConfig().customConfig.sessionMode ?? 'persistent'; let instance = BrowserManager.instances.get(key); if (!instance) { instance = new BrowserManager(config); BrowserManager.instances.set(key, instance); debugLogger.log(`Created new BrowserManager singleton (key: ${key})`); + } else if (instance.inUse) { + // Persistent and existing modes share a browser profile directory. + // Chrome prevents multiple instances from using the same profile, so + // concurrent usage would cause "profile locked" errors. + if (sessionMode === 'persistent' || sessionMode === 'existing') { + throw new Error( + `Cannot launch a concurrent browser agent in "${sessionMode}" session mode. ` + + `The browser instance is already in use by another task. ` + + `Please run browser tasks sequentially, or switch to "isolated" session mode for concurrent browser usage.`, + ); + } + + // Isolated mode: allow parallel instances up to the limit. + let inUseCount = 1; // primary is already in-use + let suffix = 1; + let parallelKey = `${key}:${suffix}`; + let parallel = BrowserManager.instances.get(parallelKey); + while (parallel?.inUse) { + inUseCount++; + if (inUseCount >= BrowserManager.MAX_PARALLEL_INSTANCES) { + throw new Error( + `Maximum number of parallel browser instances (${BrowserManager.MAX_PARALLEL_INSTANCES}) reached. ` + + `Please wait for an existing browser task to complete before starting a new one.`, + ); + } + suffix++; + parallelKey = `${key}:${suffix}`; + parallel = BrowserManager.instances.get(parallelKey); + } + if (!parallel) { + parallel = new BrowserManager(config); + BrowserManager.instances.set(parallelKey, parallel); + debugLogger.log( + `Created parallel BrowserManager (key: ${parallelKey})`, + ); + } else { + debugLogger.log( + `Reusing released parallel BrowserManager (key: ${parallelKey})`, + ); + } + instance = parallel; } else { debugLogger.log( `Reusing existing BrowserManager singleton (key: ${key})`, @@ -180,6 +236,36 @@ export class BrowserManager { private isClosing = false; private connectionPromise: Promise | undefined; + /** + * Whether this instance is currently acquired by an active invocation. + * Used by getInstance() to avoid handing the same browser to concurrent + * browser_agent calls. + */ + private inUse = false; + + /** + * Marks this instance as in-use. Call this when starting a browser agent + * invocation so concurrent calls get a separate instance. + */ + acquire(): void { + this.inUse = true; + } + + /** + * Marks this instance as available for reuse. Call this in the finally + * block of a browser agent invocation. + */ + release(): void { + this.inUse = false; + } + + /** + * Returns whether this instance is currently acquired by an active invocation. + */ + isAcquired(): boolean { + return this.inUse; + } + /** State for action rate limiting */ private actionCounter = 0; private readonly maxActionsPerTask: number; diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 55517a20d5..22ac42e6ed 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -1075,7 +1075,7 @@ describe('AgentRegistry', () => { expect.objectContaining({ toolName: 'PolicyTestAgent', decision: PolicyDecision.ALLOW, - priority: 1.05, + priority: 1.03, }), ); }); @@ -1102,7 +1102,7 @@ describe('AgentRegistry', () => { expect.objectContaining({ toolName: 'RemotePolicyAgent', decision: PolicyDecision.ASK_USER, - priority: 1.05, + priority: 1.03, }), ); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index d4c7c498a5..0edd4af7b0 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1224,7 +1224,7 @@ export class Config implements McpContext, AgentLoopContext { this.useRipgrep = params.useRipgrep ?? true; this.useBackgroundColor = params.useBackgroundColor ?? true; this.useAlternateBuffer = params.useAlternateBuffer ?? false; - this.useTerminalBuffer = params.useTerminalBuffer ?? true; + this.useTerminalBuffer = params.useTerminalBuffer ?? false; this.useRenderProcess = params.useRenderProcess ?? true; this.enableInteractiveShell = params.enableInteractiveShell ?? false; diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 9147a66a9d..359054add3 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -398,9 +398,10 @@ export async function createPolicyEngineConfig( // TOML policy priorities (before transformation): // 10: Write tools default to ASK_USER (becomes 1.010 in default tier) // 15: Auto-edit tool override (becomes 1.015 in default tier) + // 30: Unknown subagents (blocked by Plan Mode's 40) + // 40: Plan mode catch-all DENY override (becomes 1.040 in default tier) // 50: Read-only tools (becomes 1.050 in default tier) - // 60: Plan mode catch-all DENY override (becomes 1.060 in default tier) - // 70: Plan mode explicit ALLOW override (becomes 1.070 in default tier) + // 70: Mode transition overrides (becomes 1.070 in default tier) // 999: YOLO mode allow-all (becomes 1.999 in default tier) // MCP servers that are explicitly excluded in settings.mcp.excluded diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 80b59ba2d5..eaf1f9471b 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -23,8 +23,10 @@ # # TOML policy priorities (before transformation): # 10: Write tools default to ASK_USER (becomes 1.010 in default tier) -# 60: Plan mode catch-all DENY override (becomes 1.060 in default tier) -# 70: Plan mode explicit ALLOW override (becomes 1.070 in default tier) +# 30: Unknown subagents (blocked by Plan Mode's 40) +# 40: Plan mode catch-all DENY override (becomes 1.040 in default tier) +# 50: Read-only tools / Plan mode explicit ALLOW (becomes 1.050 in default tier) +# 70: Mode transition overrides (into/out of Plan Mode) # 999: YOLO mode allow-all (becomes 1.999 in default tier) # Mode Transitions (into/out of Plan Mode) @@ -59,6 +61,7 @@ interactive = true toolName = "exit_plan_mode" decision = "allow" priority = 70 +modes = ["plan"] interactive = false [[rule]] @@ -73,18 +76,23 @@ denyMessage = "You are not currently in Plan Mode. Use enter_plan_mode first to [[rule]] toolName = "*" decision = "deny" -priority = 60 +priority = 40 modes = ["plan"] denyMessage = "You are in Plan Mode with access to read-only tools. Execution of scripts (including those from skills) is blocked." # Explicitly Allow Read-Only Tools in Plan mode. +[[rule]] +toolName = ["activate_skill"] +decision = "allow" +priority = 50 +modes = ["plan"] [[rule]] toolName = "*" mcpName = "*" toolAnnotations = { readOnlyHint = true } decision = "ask_user" -priority = 70 +priority = 50 modes = ["plan"] interactive = true @@ -93,45 +101,21 @@ toolName = "*" mcpName = "*" toolAnnotations = { readOnlyHint = true } decision = "deny" -priority = 70 +priority = 50 modes = ["plan"] interactive = false -[[rule]] -toolName = [ - "glob", - "grep_search", - "list_directory", - "read_file", - "google_web_search", - "activate_skill", - "codebase_investigator", - "cli_help", - "get_internal_docs", - "complete_task" -] -decision = "allow" -priority = 70 -modes = ["plan"] - -# Topic grouping tool is innocuous and used for UI organization. -[[rule]] -toolName = "update_topic" -decision = "allow" -priority = 70 -modes = ["plan"] - [[rule]] toolName = ["ask_user", "save_memory", "web_fetch"] decision = "ask_user" -priority = 70 +priority = 50 modes = ["plan"] interactive = true [[rule]] toolName = ["ask_user", "save_memory", "web_fetch"] decision = "deny" -priority = 70 +priority = 50 modes = ["plan"] interactive = false diff --git a/packages/core/src/policy/policies/read-only.toml b/packages/core/src/policy/policies/read-only.toml index c56984b522..0a8b465fe8 100644 --- a/packages/core/src/policy/policies/read-only.toml +++ b/packages/core/src/policy/policies/read-only.toml @@ -28,43 +28,26 @@ # 999: YOLO mode allow-all (becomes 1.999 in default tier) [[rule]] -toolName = "glob" +toolName = [ + "glob", + "grep_search", + "list_directory", + "read_file", + "google_web_search", + "codebase_investigator", + "cli_help", + "get_internal_docs", + # Tracker tools for task management (safe as they only modify internal state) + "tracker_create_task", + "tracker_update_task", + "tracker_get_task", + "tracker_list_tasks", + "tracker_add_dependency", + "tracker_visualize", + # Topic grouping tool is innocuous and used for UI organization. + "update_topic", + # Core agent lifecycle tool + "complete_task" +] decision = "allow" priority = 50 - -[[rule]] -toolName = "grep_search" -decision = "allow" -priority = 50 - -[[rule]] -toolName = "list_directory" -decision = "allow" -priority = 50 - -[[rule]] -toolName = "read_file" -decision = "allow" -priority = 50 - -[[rule]] -toolName = "google_web_search" -decision = "allow" -priority = 50 - -[[rule]] -toolName = ["codebase_investigator", "cli_help", "get_internal_docs"] -decision = "allow" -priority = 50 - -# Topic grouping tool is innocuous and used for UI organization. -[[rule]] -toolName = "update_topic" -decision = "allow" -priority = 50 - -# Core agent lifecycle tool -[[rule]] -toolName = "complete_task" -decision = "allow" -priority = 50 \ No newline at end of file diff --git a/packages/core/src/policy/policies/tracker.toml b/packages/core/src/policy/policies/tracker.toml deleted file mode 100644 index e17c4fc387..0000000000 --- a/packages/core/src/policy/policies/tracker.toml +++ /dev/null @@ -1,34 +0,0 @@ -# Priority system for policy rules: -# - Higher priority numbers win over lower priority numbers -# - When multiple rules match, the highest priority rule is applied -# - Rules are evaluated in order of priority (highest first) -# -# Priority bands (tiers): -# - Default policies (TOML): 1 + priority/1000 (e.g., priority 100 β†’ 1.100) -# - Extension policies (TOML): 2 + priority/1000 (e.g., priority 100 β†’ 2.100) -# - Workspace policies (TOML): 3 + priority/1000 (e.g., priority 100 β†’ 3.100) -# - User policies (TOML): 4 + priority/1000 (e.g., priority 100 β†’ 4.100) -# - Admin policies (TOML): 5 + priority/1000 (e.g., priority 100 β†’ 5.100) -# -# Settings-based and dynamic rules (all in user tier 4.x): -# 4.95: Tools that the user has selected as "Always Allow" in the interactive UI -# 4.9: MCP servers excluded list (security: persistent server blocks) -# 4.4: Command line flag --exclude-tools (explicit temporary blocks) -# 4.3: Command line flag --allowed-tools (explicit temporary allows) -# 4.2: MCP servers with trust=true (persistent trusted servers) -# 4.1: MCP servers allowed list (persistent general server allows) - -# Allow tracker tools to execute without asking the user. -# These tools are only registered when the tracker feature is enabled, -# so this rule is a no-op when the feature is disabled. -[[rule]] -toolName = [ - "tracker_create_task", - "tracker_update_task", - "tracker_get_task", - "tracker_list_tasks", - "tracker_add_dependency", - "tracker_visualize" -] -decision = "allow" -priority = 50 diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 0299000f73..1d27107ee2 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -1715,13 +1715,13 @@ describe('PolicyEngine', () => { describe('Plan Mode vs Subagent Priority (Regression)', () => { it('should DENY subagents in Plan Mode despite dynamic allow rules', async () => { - // Plan Mode Deny (1.06) > Subagent Allow (1.05) + // Plan Mode Deny (1.04) > Subagent Allow (1.03) const fixedRules: PolicyRule[] = [ { toolName: '*', decision: PolicyDecision.DENY, - priority: 1.06, + priority: 1.04, modes: [ApprovalMode.PLAN], }, { diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 6835e200b4..9c1e424c60 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -890,8 +890,8 @@ priority = 100 readOnlyHint: true, }); expect(annotationRule!.decision).toBe(PolicyDecision.ASK_USER); - // Priority 70 in tier 1 => 1.070 - expect(annotationRule!.priority).toBe(1.07); + // Priority 50 in tier 1 => 1.050 + expect(annotationRule!.priority).toBe(1.05); // Verify deny rule was loaded correctly const denyRule = result.rules.find( @@ -904,8 +904,8 @@ priority = 100 denyRule, 'Should have loaded the catch-all deny rule', ).toBeDefined(); - // Priority 60 in tier 1 => 1.060 - expect(denyRule!.priority).toBe(1.06); + // Priority 40 in tier 1 => 1.040 + expect(denyRule!.priority).toBe(1.04); // 2. Initialize Policy Engine in Plan Mode const engine = new PolicyEngine({ @@ -974,12 +974,23 @@ priority = 100 it('should override default subagent rules when in Plan Mode for unknown subagents', async () => { const planTomlPath = path.resolve(__dirname, 'policies', 'plan.toml'); - const fileContent = await fs.readFile(planTomlPath, 'utf-8'); + const readOnlyTomlPath = path.resolve( + __dirname, + 'policies', + 'read-only.toml', + ); + const planContent = await fs.readFile(planTomlPath, 'utf-8'); + const readOnlyContent = await fs.readFile(readOnlyTomlPath, 'utf-8'); + const tempPolicyDir = await fs.mkdtemp( path.join(os.tmpdir(), 'plan-policy-test-'), ); try { - await fs.writeFile(path.join(tempPolicyDir, 'plan.toml'), fileContent); + await fs.writeFile(path.join(tempPolicyDir, 'plan.toml'), planContent); + await fs.writeFile( + path.join(tempPolicyDir, 'read-only.toml'), + readOnlyContent, + ); const getPolicyTier = () => 1; // Default tier // 1. Load the actual Plan Mode policies @@ -1004,6 +1015,7 @@ priority = 100 // 4. Verify Behavior: // The Plan Mode "Catch-All Deny" (from plan.toml) should override the Subagent Allow + // Plan Mode Deny (1.04) > Subagent Allow (1.03) const checkResult = await engine.check( { name: 'unknown_subagent' }, undefined, @@ -1015,7 +1027,7 @@ priority = 100 ).toBe(PolicyDecision.DENY); // 5. Verify Explicit Allows still work - // e.g. 'read_file' should be allowed because its priority in plan.toml (70) is higher than the deny (60) + // e.g. 'read_file' should be allowed because its priority in read-only.toml (50) is higher than the deny (40) const readResult = await engine.check({ name: 'read_file' }, undefined); expect( readResult.decision, @@ -1023,6 +1035,7 @@ priority = 100 ).toBe(PolicyDecision.ALLOW); // 6. Verify Built-in Research Subagents are ALLOWED + // codebase_investigator is priority 50 in read-only.toml const codebaseResult = await engine.check( { name: 'codebase_investigator' }, undefined, diff --git a/packages/core/src/policy/types.ts b/packages/core/src/policy/types.ts index 622cde0abd..b843129c99 100644 --- a/packages/core/src/policy/types.ts +++ b/packages/core/src/policy/types.ts @@ -354,9 +354,11 @@ export interface CheckResult { /** * Priority for subagent tools (registered dynamically). - * Effective priority matching Tier 1 (Default) read-only tools. + * Effective priority matching Tier 1 (Default) at priority 30. + * This ensures they are blocked by Plan Mode (priority 40) while + * remaining above directive write tools (priority 10). */ -export const PRIORITY_SUBAGENT_TOOL = 1.05; +export const PRIORITY_SUBAGENT_TOOL = 1.03; /** * The fractional priority of "Always allow" rules (e.g., 950/1000). diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 3609848cc1..6910dc2971 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -804,6 +804,32 @@ describe('ShellTool', () => { }); }); + describe('invocation getDescription', () => { + it('should return the description if it is present and not empty whitespace', () => { + const invocation = shellTool.build({ + command: 'echo hello', + description: 'prints hello', + }); + expect(invocation.getDescription()).toBe('prints hello'); + }); + + it('should return the raw command if description is an empty string', () => { + const invocation = shellTool.build({ + command: 'echo hello', + description: '', + }); + expect(invocation.getDescription()).toBe('echo hello'); + }); + + it('should return the raw command if description is just whitespace', () => { + const invocation = shellTool.build({ + command: 'echo hello', + description: ' ', + }); + expect(invocation.getDescription()).toBe('echo hello'); + }); + }); + describe('llmContent output format', () => { const mockAbortSignal = new AbortController().signal; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 75ddf78402..9688280f59 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -136,7 +136,9 @@ export class ShellToolInvocation extends BaseToolInvocation< } getDescription(): string { - return this.params.description || ''; + return this.params.description?.trim() + ? this.params.description + : this.params.command; } private simplifyPaths(paths: Set): string[] { diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 5179263596..bb5c9a9d54 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -465,8 +465,8 @@ "terminalBuffer": { "title": "Terminal Buffer", "description": "Use the new terminal buffer architecture for rendering.", - "markdownDescription": "Use the new terminal buffer architecture for rendering.\n\n- Category: `UI`\n- Requires restart: `yes`\n- Default: `true`", - "default": true, + "markdownDescription": "Use the new terminal buffer architecture for rendering.\n\n- Category: `UI`\n- Requires restart: `yes`\n- Default: `false`", + "default": false, "type": "boolean" }, "useBackgroundColor": {