From 1aa798dd18326efcfbe8ca856bfc958f51938d07 Mon Sep 17 00:00:00 2001 From: JAYADITYA <96861162+JayadityaGit@users.noreply.github.com> Date: Wed, 8 Apr 2026 05:06:44 +0530 Subject: [PATCH 01/13] refactor(cli): remove duplication in interactive shell awaiting input hint (#24801) --- packages/cli/src/ui/components/StatusRow.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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} ) : ( From 16768c08f2fa1f0fe2dca32021f81d9f6c8e3316 Mon Sep 17 00:00:00 2001 From: Michael Bleigh Date: Tue, 7 Apr 2026 16:45:22 -0700 Subject: [PATCH 02/13] refactor(core): make LegacyAgentSession dependencies optional (#24287) Co-authored-by: Adam Weidman Co-authored-by: Adam Weidman --- .../src/agent/legacy-agent-session.test.ts | 23 +++++++----- .../core/src/agent/legacy-agent-session.ts | 36 ++++++++++++++----- 2 files changed, 43 insertions(+), 16 deletions(-) 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[] { From 9fd92c0eeacae8c7e612f9cb795e7a37373bc452 Mon Sep 17 00:00:00 2001 From: gemini-cli-robot Date: Tue, 7 Apr 2026 17:13:06 -0700 Subject: [PATCH 03/13] Changelog for v0.37.0-preview.2 (#24848) Co-authored-by: gemini-cli-robot <224641728+gemini-cli-robot@users.noreply.github.com> --- docs/changelogs/preview.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 From 28efab483fc921ee889eacc723a84806928d956d Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 7 Apr 2026 18:52:33 -0700 Subject: [PATCH 04/13] fix(cli): always show shell command description or actual command (#24774) --- packages/core/src/tools/shell.test.ts | 26 ++++++++++++++++++++++++++ packages/core/src/tools/shell.ts | 4 +++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 245b7f0eee..9551fd9638 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -803,6 +803,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 7ca475808a..3ea29474c6 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[] { From 47c5d25d93a767e96f2fd4f1e2d9092b2c024cc9 Mon Sep 17 00:00:00 2001 From: Dev Randalpura Date: Tue, 7 Apr 2026 23:03:36 -0400 Subject: [PATCH 05/13] Added flag for ept size and increased default size (#24859) --- packages/cli/src/gemini.test.tsx | 28 ++++++++++++++++++++++++---- packages/cli/src/gemini.tsx | 25 +++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 6 deletions(-) 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() { From b9f1d832c80b644eec2e997e85a6105b9d0c0b5d Mon Sep 17 00:00:00 2001 From: Anjaligarhwal Date: Wed, 8 Apr 2026 08:35:53 +0530 Subject: [PATCH 06/13] fix(core): dispose Scheduler to prevent McpProgress listener leak (#24870) --- packages/cli/src/nonInteractiveCli.test.ts | 1 + packages/cli/src/nonInteractiveCli.ts | 4 +- .../src/nonInteractiveCliAgentSession.test.ts | 1 + .../cli/src/nonInteractiveCliAgentSession.ts | 4 +- .../core/src/agents/agent-scheduler.test.ts | 52 +++++++++++++++++++ packages/core/src/agents/agent-scheduler.ts | 6 ++- 6 files changed, 65 insertions(+), 3 deletions(-) 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/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(); + } } From 7e1938c1bc9d00156ee0650e0a7fdcb3e167308f Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 7 Apr 2026 22:47:54 -0700 Subject: [PATCH 07/13] fix(cli): switch default back to terminalBuffer=false and fix regressions introduced for that mode (#24873) --- docs/cli/settings.md | 2 +- docs/reference/configuration.md | 2 +- packages/cli/src/config/settingsSchema.ts | 2 +- packages/cli/src/interactiveCli.tsx | 3 +- .../src/ui/__snapshots__/App.test.tsx.snap | 6 ++ .../src/ui/components/InputPrompt.test.tsx | 48 +++++----- .../cli/src/ui/components/InputPrompt.tsx | 66 ++++++++++---- .../HistoryItemDisplay.test.tsx.snap | 86 +----------------- .../__snapshots__/InputPrompt.test.tsx.snap | 14 +-- .../messages/ShellToolMessage.test.tsx | 4 +- .../components/messages/ToolMessage.test.tsx | 4 +- .../messages/ToolResultDisplay.test.tsx | 86 +++++++++++++++++- .../components/messages/ToolResultDisplay.tsx | 90 ++++++++++++++----- .../ToolResultDisplayOverflow.test.tsx | 19 ++-- ...ccepted-file-edit-with-diff-stats.snap.svg | 23 ++++- .../DenseToolMessage.test.tsx.snap | 32 ++++++- ...ilableTerminalHeight-is-undefined.snap.svg | 41 +++------ .../ToolResultDisplay.test.tsx.snap | 71 ++++++++------- .../src/ui/components/shared/MaxSizedBox.tsx | 2 +- packages/core/src/config/config.ts | 2 +- schemas/settings.schema.json | 4 +- 21 files changed, 363 insertions(+), 244 deletions(-) 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/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/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/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/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 4d40809837..3fdaa479cc 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 () => { diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index c8d7efa1b4..7e59ab4d14 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'; @@ -1820,24 +1827,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/__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/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/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/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": { From cbacdc67d0622c2b4ae632aaa261e61d124ae1a0 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 7 Apr 2026 23:22:45 -0700 Subject: [PATCH 08/13] feat(cli): switch to ctrl+g from ctrl-x (#24861) --- docs/reference/keyboard-shortcuts.md | 17 +++++++++-------- .../src/ui/components/ContextSummaryDisplay.tsx | 4 +++- .../ui/components/ExitPlanModeDialog.test.tsx | 6 +++--- .../src/ui/components/ExitPlanModeDialog.tsx | 13 +++++++++++++ .../cli/src/ui/components/InputPrompt.test.tsx | 4 ++-- packages/cli/src/ui/components/InputPrompt.tsx | 9 +++++++++ .../ContextSummaryDisplay.test.tsx.snap | 6 +++--- .../ExitPlanModeDialog.test.tsx.snap | 16 ++++++++-------- .../__snapshots__/ShortcutsHelp.test.tsx.snap | 8 ++++---- .../ToolConfirmationQueue.test.tsx.snap | 2 +- packages/cli/src/ui/constants/tips.ts | 4 ++-- packages/cli/src/ui/key/keyBindings.ts | 9 +++++++-- packages/cli/src/ui/key/keyMatchers.test.ts | 9 +++++++-- 13 files changed, 71 insertions(+), 36 deletions(-) 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/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 3fdaa479cc..7a241691e8 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -5065,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 7e59ab4d14..b36de8ebb0 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -1272,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 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__/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/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, From 651ad63ed6daf4decf9071d5aa0bc9a4e715434d Mon Sep 17 00:00:00 2001 From: Gaurav Ghosh Date: Fri, 20 Mar 2026 13:39:10 -0700 Subject: [PATCH 09/13] feat: Introduce an AI-driven interactive shell mode with new `read-shell` and `write-to-shell` tools and a configurable mode setting. --- packages/cli/src/config/config.ts | 1 + packages/cli/src/config/settingsSchema.ts | 20 ++ packages/cli/src/ui/hooks/shellReducer.ts | 18 +- .../src/ui/hooks/useBackgroundShellManager.ts | 101 ++++++++ .../cli/src/ui/hooks/useExecutionLifecycle.ts | 5 + packages/cli/src/ui/hooks/useGeminiStream.ts | 3 + packages/core/src/config/config.ts | 27 +- packages/core/src/prompts/promptProvider.ts | 1 + packages/core/src/prompts/snippets.ts | 16 +- .../src/services/shellExecutionService.ts | 41 ++++ .../tools/definitions/base-declarations.ts | 12 + .../core/src/tools/definitions/coreTools.ts | 11 + .../dynamic-declaration-helpers.ts | 30 +++ .../model-family-sets/default-legacy.ts | 2 + .../definitions/model-family-sets/gemini-3.ts | 2 + packages/core/src/tools/definitions/types.ts | 1 + packages/core/src/tools/read-shell.ts | 148 +++++++++++ packages/core/src/tools/shell.test.ts | 6 +- packages/core/src/tools/shell.ts | 167 +++++++------ .../core/src/tools/shellOutputFormatter.ts | 128 ++++++++++ packages/core/src/tools/tool-names.ts | 19 ++ packages/core/src/tools/write-to-shell.ts | 230 ++++++++++++++++++ 22 files changed, 906 insertions(+), 83 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useBackgroundShellManager.ts create mode 100644 packages/core/src/tools/read-shell.ts create mode 100644 packages/core/src/tools/shellOutputFormatter.ts create mode 100644 packages/core/src/tools/write-to-shell.ts diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 4e7e1db6f2..499b57b522 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1009,6 +1009,7 @@ export async function loadCliConfig( enableInteractiveShell: settings.tools?.shell?.enableInteractiveShell, shellBackgroundCompletionBehavior: settings.tools?.shell ?.backgroundCompletionBehavior as string | undefined, + interactiveShellMode: settings.tools?.shell?.interactiveShellMode, shellToolInactivityTimeout: settings.tools?.shell?.inactivityTimeout, enableShellOutputEfficiency: settings.tools?.shell?.enableShellOutputEfficiency ?? true, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index c041aaa8c3..e654391566 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1512,6 +1512,26 @@ const SETTINGS_SCHEMA = { { label: 'Notify', value: 'notify' }, ], }, + interactiveShellMode: { + type: 'enum', + label: 'Interactive Shell Mode', + category: 'Tools', + requiresRestart: true, + default: undefined as 'human' | 'ai' | 'off' | undefined, + description: oneLine` + Controls who can interact with backgrounded shell processes. + "human": user can Tab-focus and type into shells (default). + "ai": model gets write_to_shell/read_shell tools for TUI interaction. + "off": no interactive shell. + When set, overrides enableInteractiveShell. + `, + showInDialog: true, + options: [ + { value: 'human', label: 'Human (Tab to focus)' }, + { value: 'ai', label: 'AI (model-driven tools)' }, + { value: 'off', label: 'Off' }, + ], + }, pager: { type: 'string', label: 'Pager', diff --git a/packages/cli/src/ui/hooks/shellReducer.ts b/packages/cli/src/ui/hooks/shellReducer.ts index 0e9307259d..ea467fc327 100644 --- a/packages/cli/src/ui/hooks/shellReducer.ts +++ b/packages/cli/src/ui/hooks/shellReducer.ts @@ -92,7 +92,23 @@ export function shellReducer( nextTasks.delete(action.pid); } nextTasks.set(action.pid, updatedTask); - return { ...state, backgroundTasks: nextTasks }; + + // Auto-hide panel when all tasks have exited + let nextVisible = state.isBackgroundTaskVisible; + if (action.update.status === 'exited') { + const hasRunning = Array.from(nextTasks.values()).some( + (s) => s.status === 'running', + ); + if (!hasRunning) { + nextVisible = false; + } + } + + return { + ...state, + backgroundTasks: nextTasks, + isBackgroundTaskVisible: nextVisible, + }; } case 'APPEND_TASK_OUTPUT': { const task = state.backgroundTasks.get(action.pid); diff --git a/packages/cli/src/ui/hooks/useBackgroundShellManager.ts b/packages/cli/src/ui/hooks/useBackgroundShellManager.ts new file mode 100644 index 0000000000..eb43ae1cfb --- /dev/null +++ b/packages/cli/src/ui/hooks/useBackgroundShellManager.ts @@ -0,0 +1,101 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect, useMemo, useRef } from 'react'; +import { type BackgroundTask } from './shellReducer.js'; + +export interface BackgroundShellManagerProps { + backgroundTasks: Map; + backgroundTaskCount: number; + isBackgroundTaskVisible: boolean; + activePtyId: number | null | undefined; + embeddedShellFocused: boolean; + setEmbeddedShellFocused: (focused: boolean) => void; + terminalHeight: number; +} + +export function useBackgroundShellManager({ + backgroundTasks, + backgroundTaskCount, + isBackgroundTaskVisible, + activePtyId, + embeddedShellFocused, + setEmbeddedShellFocused, + terminalHeight, +}: BackgroundShellManagerProps) { + const [isBackgroundShellListOpen, setIsBackgroundShellListOpen] = + useState(false); + const [activeBackgroundShellPid, setActiveBackgroundShellPid] = useState< + number | null + >(null); + + const prevShellCountRef = useRef(backgroundTaskCount); + + useEffect(() => { + if (backgroundTasks.size === 0) { + if (activeBackgroundShellPid !== null) { + setActiveBackgroundShellPid(null); + } + if (isBackgroundShellListOpen) { + setIsBackgroundShellListOpen(false); + } + } else if ( + activeBackgroundShellPid === null || + !backgroundTasks.has(activeBackgroundShellPid) + ) { + // If active shell is closed or none selected, select the first one + setActiveBackgroundShellPid(backgroundTasks.keys().next().value ?? null); + } else if (backgroundTaskCount > prevShellCountRef.current) { + // A new shell was added β€” auto-switch to the newest one (last in the map) + const pids = Array.from(backgroundTasks.keys()); + const newestPid = pids[pids.length - 1]; + if (newestPid !== undefined && newestPid !== activeBackgroundShellPid) { + setActiveBackgroundShellPid(newestPid); + } + } + prevShellCountRef.current = backgroundTaskCount; + }, [ + backgroundTasks, + activeBackgroundShellPid, + backgroundTaskCount, + isBackgroundShellListOpen, + ]); + + useEffect(() => { + if (embeddedShellFocused) { + const hasActiveForegroundShell = !!activePtyId; + const hasVisibleBackgroundShell = + isBackgroundTaskVisible && backgroundTasks.size > 0; + + if (!hasActiveForegroundShell && !hasVisibleBackgroundShell) { + setEmbeddedShellFocused(false); + } + } + }, [ + isBackgroundTaskVisible, + backgroundTasks, + embeddedShellFocused, + backgroundTaskCount, + activePtyId, + setEmbeddedShellFocused, + ]); + + const backgroundShellHeight = useMemo( + () => + isBackgroundTaskVisible && backgroundTasks.size > 0 + ? Math.max(Math.floor(terminalHeight * 0.3), 5) + : 0, + [isBackgroundTaskVisible, backgroundTasks.size, terminalHeight], + ); + + return { + isBackgroundShellListOpen, + setIsBackgroundShellListOpen, + activeBackgroundShellPid, + setActiveBackgroundShellPid, + backgroundShellHeight, + }; +} diff --git a/packages/cli/src/ui/hooks/useExecutionLifecycle.ts b/packages/cli/src/ui/hooks/useExecutionLifecycle.ts index 2e80bf8f95..02e9e88cf5 100644 --- a/packages/cli/src/ui/hooks/useExecutionLifecycle.ts +++ b/packages/cli/src/ui/hooks/useExecutionLifecycle.ts @@ -661,6 +661,10 @@ export const useExecutionLifecycle = ( (s: BackgroundTask) => s.status === 'running', ).length; + const showBackgroundShell = useCallback(() => { + dispatch({ type: 'SET_VISIBILITY', visible: true }); + }, [dispatch]); + return { handleShellCommand, activeShellPtyId: state.activeShellPtyId, @@ -668,6 +672,7 @@ export const useExecutionLifecycle = ( backgroundTaskCount, isBackgroundTaskVisible: state.isBackgroundTaskVisible, toggleBackgroundTasks, + showBackgroundShell, backgroundCurrentExecution, registerBackgroundTask, dismissBackgroundTask, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index a2621c4546..c4a9c58d5e 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -390,6 +390,7 @@ export const useGeminiStream = ( backgroundTaskCount, isBackgroundTaskVisible, toggleBackgroundTasks, + showBackgroundShell, backgroundCurrentExecution, registerBackgroundTask, dismissBackgroundTask, @@ -1917,6 +1918,7 @@ export const useGeminiStream = ( backgroundedTool.command, backgroundedTool.initialOutput, ); + showBackgroundShell(); } } @@ -2056,6 +2058,7 @@ export const useGeminiStream = ( modelSwitchedFromQuotaError, addItem, registerBackgroundTask, + showBackgroundShell, consumeUserHint, isLowErrorVerbosity, maybeAddSuppressedToolErrorNote, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 0edd4af7b0..c82cc315b7 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -36,6 +36,8 @@ import { GlobTool } from '../tools/glob.js'; import { ActivateSkillTool } from '../tools/activate-skill.js'; import { EditTool } from '../tools/edit.js'; import { ShellTool } from '../tools/shell.js'; +import { WriteToShellTool } from '../tools/write-to-shell.js'; +import { ReadShellTool } from '../tools/read-shell.js'; import { WriteFileTool } from '../tools/write-file.js'; import { WebFetchTool } from '../tools/web-fetch.js'; import { MemoryTool, setGeminiMdFilename } from '../tools/memoryTool.js'; @@ -656,6 +658,7 @@ export interface ConfigParameters { useRipgrep?: boolean; enableInteractiveShell?: boolean; shellBackgroundCompletionBehavior?: string; + interactiveShellMode?: 'human' | 'ai' | 'off'; skipNextSpeakerCheck?: boolean; shellExecutionConfig?: ShellExecutionConfig; extensionManagement?: boolean; @@ -868,6 +871,7 @@ export class Config implements McpContext, AgentLoopContext { | 'inject' | 'notify' | 'silent'; + private readonly interactiveShellMode: 'human' | 'ai' | 'off'; private readonly skipNextSpeakerCheck: boolean; private readonly useBackgroundColor: boolean; private readonly useAlternateBuffer: boolean; @@ -1235,6 +1239,14 @@ export class Config implements McpContext, AgentLoopContext { this.shellBackgroundCompletionBehavior = 'silent'; } + // interactiveShellMode takes precedence over enableInteractiveShell. + // If not set, derive from enableInteractiveShell for backward compat. + if (params.interactiveShellMode) { + this.interactiveShellMode = params.interactiveShellMode; + } else { + this.interactiveShellMode = this.enableInteractiveShell ? 'human' : 'off'; + } + this.skipNextSpeakerCheck = params.skipNextSpeakerCheck ?? true; this.shellExecutionConfig = { terminalWidth: params.shellExecutionConfig?.terminalWidth ?? 80, @@ -3211,10 +3223,14 @@ export class Config implements McpContext, AgentLoopContext { return ( this.interactive && this.ptyInfo !== 'child_process' && - this.enableInteractiveShell + this.interactiveShellMode !== 'off' ); } + getInteractiveShellMode(): 'human' | 'ai' | 'off' { + return this.interactiveShellMode; + } + isSkillsSupportEnabled(): boolean { return this.skillsSupport; } @@ -3575,6 +3591,15 @@ export class Config implements McpContext, AgentLoopContext { new ReadBackgroundOutputTool(this, this.messageBus), ), ); + // Register AI-driven interactive shell tools when mode is 'ai' + if (this.getInteractiveShellMode() === 'ai') { + maybeRegister(WriteToShellTool, () => + registry.registerTool(new WriteToShellTool(this.messageBus)), + ); + maybeRegister(ReadShellTool, () => + registry.registerTool(new ReadShellTool(this.messageBus)), + ); + } if (!this.isMemoryManagerEnabled()) { maybeRegister(MemoryTool, () => registry.registerTool(new MemoryTool(this.messageBus, this.storage)), diff --git a/packages/core/src/prompts/promptProvider.ts b/packages/core/src/prompts/promptProvider.ts index 0036dae560..c4077afc95 100644 --- a/packages/core/src/prompts/promptProvider.ts +++ b/packages/core/src/prompts/promptProvider.ts @@ -200,6 +200,7 @@ export class PromptProvider { enableShellEfficiency: context.config.getEnableShellOutputEfficiency(), interactiveShellEnabled: context.config.isInteractiveShellEnabled(), + interactiveShellMode: context.config.getInteractiveShellMode(), topicUpdateNarration: context.config.isTopicUpdateNarrationEnabled(), memoryManagerEnabled: context.config.isMemoryManagerEnabled(), diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 59315e1ca6..b049ddf58e 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -18,6 +18,8 @@ import { MEMORY_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, + READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, WRITE_TODOS_TOOL_NAME, GREP_PARAM_TOTAL_MAX_MATCHES, @@ -81,6 +83,7 @@ export interface PrimaryWorkflowsOptions { export interface OperationalGuidelinesOptions { interactive: boolean; interactiveShellEnabled: boolean; + interactiveShellMode?: 'human' | 'ai' | 'off'; topicUpdateNarration: boolean; memoryManagerEnabled: boolean; } @@ -391,7 +394,7 @@ export function renderOperationalGuidelines( - **Command Execution:** Use the ${formatToolName(SHELL_TOOL_NAME)} tool for running shell commands, remembering the safety rule to explain modifying commands first.${toolUsageInteractive( options.interactive, options.interactiveShellEnabled, - )}${toolUsageRememberingFacts(options)} + )}${toolUsageRememberingFacts(options)}${toolUsageAiShell(options)} - **Confirmation Protocol:** If a tool call is declined or cancelled, respect the decision immediately. Do not re-attempt the action or "negotiate" for the same tool call unless the user explicitly directs you to. Offer an alternative technical path if possible. ## Interaction Details @@ -800,6 +803,17 @@ function toolUsageInteractive( - **Interactive Commands:** Always prefer non-interactive commands (e.g., using 'run once' or 'CI' flags for test runners to avoid persistent watch modes or 'git --no-pager') unless a persistent process is specifically required; however, some commands are only interactive and expect user input during their execution (e.g. ssh, vim).`; } +function toolUsageAiShell(options: OperationalGuidelinesOptions): string { + if (options.interactiveShellMode !== 'ai') return ''; + return ` +- **AI-Driven Interactive Shell:** Commands using \`wait_for_output_seconds\` auto-promote to background when they stall. Once promoted, use ${formatToolName(READ_SHELL_TOOL_NAME)} to see the terminal screen, then ${formatToolName(WRITE_TO_SHELL_TOOL_NAME)} to send text input and/or special keys (arrows, Enter, Ctrl-C, etc.). + - Set \`wait_for_output_seconds\` **low (2-5)** for commands that prompt for input (npx, installers, REPLs). Set **high (60+)** for long builds. Omit for instant commands. + - **Always read the screen before writing input.** The screen state tells you what the process is waiting for. + - When waiting for a command to finish (e.g. npm install), use ${formatToolName(READ_SHELL_TOOL_NAME)} with \`wait_seconds\` to delay before reading. Do NOT poll in a tight loop. + - **Clean up when done:** when your task is complete, kill background processes with ${formatToolName(WRITE_TO_SHELL_TOOL_NAME)} sending Ctrl-C, or note the PID for the user to clean up. + - You are the sole operator of promoted shells β€” the user cannot type into them.`; +} + function toolUsageRememberingFacts( options: OperationalGuidelinesOptions, ): string { diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index dfbb3a5033..95b3f2d17b 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -105,6 +105,7 @@ export interface ShellExecutionConfig { backgroundCompletionBehavior?: 'inject' | 'notify' | 'silent'; originalCommand?: string; sessionId?: string; + autoPromoteTimeoutMs?: number; } /** @@ -889,6 +890,21 @@ export class ShellExecutionService { sessionId: shellExecutionConfig.sessionId, }); + let autoPromoteTimer: NodeJS.Timeout | undefined; + const resetAutoPromoteTimer = () => { + if (shellExecutionConfig.autoPromoteTimeoutMs !== undefined) { + if (autoPromoteTimer) clearTimeout(autoPromoteTimer); + autoPromoteTimer = setTimeout(() => { + ShellExecutionService.background( + ptyPid, + shellExecutionConfig.sessionId, + ); + }, shellExecutionConfig.autoPromoteTimeoutMs); + } + }; + + resetAutoPromoteTimer(); + const result = ExecutionLifecycleService.attachExecution(ptyPid, { executionMethod: ptyInfo?.name ?? 'node-pty', writeInput: (input) => { @@ -1066,6 +1082,7 @@ export class ShellExecutionService { }); const handleOutput = (data: Buffer) => { + resetAutoPromoteTimer(); processingChain = processingChain.then( () => new Promise((resolveChunk) => { @@ -1135,6 +1152,7 @@ export class ShellExecutionService { ptyProcess.onExit( ({ exitCode, signal }: { exitCode: number; signal?: number }) => { + if (autoPromoteTimer) clearTimeout(autoPromoteTimer); exited = true; abortSignal.removeEventListener('abort', abortHandler); // Attempt to destroy the PTY to ensure FD is closed @@ -1220,6 +1238,7 @@ export class ShellExecutionService { ); const abortHandler = async () => { + if (autoPromoteTimer) clearTimeout(autoPromoteTimer); if (ptyProcess.pid && !exited) { await killProcessGroup({ pid: ptyPid, @@ -1398,6 +1417,28 @@ export class ShellExecutionService { return ExecutionLifecycleService.subscribe(pid, listener); } + /** + * Reads the current rendered screen state of a running process. + * Returns the full terminal buffer text for PTY processes, + * or the accumulated output for child processes. + * + * @param pid The process ID of the target process. + * @returns The screen text, or null if the process is not found. + */ + static readScreen(pid: number): string | null { + const activePty = this.activePtys.get(pid); + if (activePty) { + return getFullBufferText(activePty.headlessTerminal); + } + + const activeChild = this.activeChildProcesses.get(pid); + if (activeChild) { + return activeChild.state.output; + } + + return null; + } + /** * Resizes the pseudo-terminal (PTY) of a running process. * diff --git a/packages/core/src/tools/definitions/base-declarations.ts b/packages/core/src/tools/definitions/base-declarations.ts index 89a5aa1614..e1575966af 100644 --- a/packages/core/src/tools/definitions/base-declarations.ts +++ b/packages/core/src/tools/definitions/base-declarations.ts @@ -56,6 +56,18 @@ export const READ_FILE_PARAM_END_LINE = 'end_line'; export const SHELL_TOOL_NAME = 'run_shell_command'; export const SHELL_PARAM_COMMAND = 'command'; export const SHELL_PARAM_IS_BACKGROUND = 'is_background'; +export const SHELL_PARAM_WAIT_SECONDS = 'wait_for_output_seconds'; + +// -- write_to_shell -- +export const WRITE_TO_SHELL_TOOL_NAME = 'write_to_shell'; +export const WRITE_TO_SHELL_PARAM_PID = 'pid'; +export const WRITE_TO_SHELL_PARAM_INPUT = 'input'; +export const WRITE_TO_SHELL_PARAM_SPECIAL_KEYS = 'special_keys'; + +// -- read_shell -- +export const READ_SHELL_TOOL_NAME = 'read_shell'; +export const READ_SHELL_PARAM_PID = 'pid'; +export const READ_SHELL_PARAM_WAIT_SECONDS = 'wait_seconds'; // -- write_file -- export const WRITE_FILE_TOOL_NAME = 'write_file'; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index d1b81a6e99..a70ed1a33c 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -27,6 +27,8 @@ export { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, + READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -73,6 +75,12 @@ export { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, + SHELL_PARAM_WAIT_SECONDS, + WRITE_TO_SHELL_PARAM_PID, + WRITE_TO_SHELL_PARAM_INPUT, + WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, + READ_SHELL_PARAM_PID, + READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -249,18 +257,21 @@ export function getShellDefinition( enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean = false, + interactiveShellMode?: string, ): ToolDefinition { return { base: getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ), overrides: (modelId) => getToolSet(modelId).run_shell_command( enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ), }; } diff --git a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts index 29da313bf4..6f001c7459 100644 --- a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts +++ b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts @@ -22,6 +22,7 @@ import { PARAM_DIR_PATH, SHELL_PARAM_IS_BACKGROUND, EXIT_PLAN_PARAM_PLAN_FILENAME, + SHELL_PARAM_WAIT_SECONDS, SKILL_PARAM_NAME, PARAM_ADDITIONAL_PERMISSIONS, UPDATE_TOPIC_TOOL_NAME, @@ -36,7 +37,9 @@ import { export function getShellToolDescription( enableInteractiveShell: boolean, enableEfficiency: boolean, + interactiveShellMode?: string, ): string { + const isAiMode = interactiveShellMode === 'ai'; const efficiencyGuidelines = enableEfficiency ? ` @@ -56,6 +59,11 @@ export function getShellToolDescription( Background PIDs: Only included if background processes were started. Process Group PGID: Only included if available.`; + if (isAiMode) { + const autoPromoteInstructions = `Commands that do not complete within \`${SHELL_PARAM_WAIT_SECONDS}\` seconds are automatically promoted to background. Once promoted, use \`write_to_shell\` and \`read_shell\` to interact with the process. Do NOT use \`&\` to background commands.`; + return `This tool executes a given shell command as \`bash -c \`. ${autoPromoteInstructions} Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${efficiencyGuidelines}${returnedInfo}`; + } + if (os.platform() === 'win32') { const backgroundInstructions = enableInteractiveShell ? `To run a command in the background, set the \`${SHELL_PARAM_IS_BACKGROUND}\` parameter to true. Do NOT use PowerShell background constructs.` @@ -86,12 +94,33 @@ export function getShellDeclaration( enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean = false, + interactiveShellMode?: string, ): FunctionDeclaration { + const isAiMode = interactiveShellMode === 'ai'; + + // In AI mode, use wait_for_output_seconds instead of is_background + const backgroundParam = isAiMode + ? { + [SHELL_PARAM_WAIT_SECONDS]: { + type: 'number' as const, + description: + 'Max seconds to wait for command to complete before auto-promoting to background (default: 5). Set low (2-5) for commands likely to prompt for input (npx, installers, REPLs). Set high (60-300) for long builds or installs. Once promoted, use write_to_shell/read_shell to interact.', + }, + } + : { + [SHELL_PARAM_IS_BACKGROUND]: { + type: 'boolean' as const, + description: + 'Set to true if this command should be run in the background (e.g. for long-running servers or watchers). The command will be started, allowed to run for a brief moment to check for immediate errors, and then moved to the background.', + }, + }; + return { name: SHELL_TOOL_NAME, description: getShellToolDescription( enableInteractiveShell, enableEfficiency, + interactiveShellMode, ), parametersJsonSchema: { type: 'object', @@ -120,6 +149,7 @@ export function getShellDeclaration( description: 'Optional. Delay in milliseconds to wait after starting the process in the background. Useful to allow the process to start and generate initial output before returning.', }, + ...backgroundParam, ...(enableToolSandboxing ? { [PARAM_ADDITIONAL_PERMISSIONS]: { diff --git a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts index 60a52fc6ad..5441c39d09 100644 --- a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts +++ b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts @@ -337,11 +337,13 @@ export const DEFAULT_LEGACY_SET: CoreToolSet = { enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ) => getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ), replace: { diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index a86a20378e..f29f9e6814 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -344,11 +344,13 @@ export const GEMINI_3_SET: CoreToolSet = { enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ) => getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, + interactiveShellMode, ), replace: { diff --git a/packages/core/src/tools/definitions/types.ts b/packages/core/src/tools/definitions/types.ts index 42c0cc7028..d4f532f513 100644 --- a/packages/core/src/tools/definitions/types.ts +++ b/packages/core/src/tools/definitions/types.ts @@ -38,6 +38,7 @@ export interface CoreToolSet { enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean, + interactiveShellMode?: string, ) => FunctionDeclaration; replace: FunctionDeclaration; google_web_search: FunctionDeclaration; diff --git a/packages/core/src/tools/read-shell.ts b/packages/core/src/tools/read-shell.ts new file mode 100644 index 0000000000..4e74cbbfa5 --- /dev/null +++ b/packages/core/src/tools/read-shell.ts @@ -0,0 +1,148 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + BaseDeclarativeTool, + BaseToolInvocation, + Kind, + type ToolInvocation, + type ToolResult, +} from './tools.js'; +import { ShellExecutionService } from '../services/shellExecutionService.js'; +import { + READ_SHELL_TOOL_NAME, + READ_SHELL_PARAM_PID, + READ_SHELL_PARAM_WAIT_SECONDS, +} from './tool-names.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; + +export interface ReadShellParams { + pid: number; + wait_seconds?: number; +} + +export class ReadShellToolInvocation extends BaseToolInvocation< + ReadShellParams, + ToolResult +> { + constructor( + params: ReadShellParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ) { + super(params, messageBus, _toolName, _toolDisplayName); + } + + getDescription(): string { + const waitPart = + this.params.wait_seconds !== undefined + ? ` (after ${this.params.wait_seconds}s)` + : ''; + return `read shell screen PID ${this.params.pid}${waitPart}`; + } + + async execute(signal: AbortSignal): Promise { + const { pid, wait_seconds } = this.params; + + // Wait before reading if requested + if (wait_seconds !== undefined && wait_seconds > 0) { + const waitMs = Math.min(wait_seconds, 30) * 1000; // Cap at 30s + await new Promise((resolve) => { + const timer = setTimeout(resolve, waitMs); + const onAbort = () => { + clearTimeout(timer); + resolve(); + }; + signal.addEventListener('abort', onAbort, { once: true }); + }); + } + + // Validate the PID is active + if (!ShellExecutionService.isPtyActive(pid)) { + return { + llmContent: `Error: No active process found with PID ${pid}. The process may have exited.`, + returnDisplay: `No active process with PID ${pid}.`, + }; + } + + const screen = ShellExecutionService.readScreen(pid); + if (screen === null) { + return { + llmContent: `Error: Could not read screen for PID ${pid}. The process may have exited.`, + returnDisplay: `Could not read screen for PID ${pid}.`, + }; + } + + return { + llmContent: screen, + returnDisplay: `Screen read from PID ${pid} (${screen.split('\n').length} lines).`, + }; + } +} + +export class ReadShellTool extends BaseDeclarativeTool< + ReadShellParams, + ToolResult +> { + static readonly Name = READ_SHELL_TOOL_NAME; + + constructor(messageBus: MessageBus) { + super( + ReadShellTool.Name, + 'ReadShell', + 'Reads the current screen state of a running background shell process. Returns the rendered terminal screen as text, preserving the visual layout. Use after write_to_shell to see updated output, or to check progress of a running command.', + Kind.Read, + { + type: 'object', + properties: { + [READ_SHELL_PARAM_PID]: { + type: 'number', + description: + 'The PID of the background process to read from. Obtained from a previous run_shell_command call that was auto-promoted to background or started with is_background=true.', + }, + [READ_SHELL_PARAM_WAIT_SECONDS]: { + type: 'number', + description: + 'Seconds to wait before reading the screen. Use this to let the process run for a while before checking output (e.g. wait for a build to finish). Max 30 seconds.', + }, + }, + required: [READ_SHELL_PARAM_PID], + }, + messageBus, + false, // output is not markdown + ); + } + + protected override validateToolParamValues( + params: ReadShellParams, + ): string | null { + if (!params.pid || params.pid <= 0) { + return 'PID must be a positive number.'; + } + if ( + params.wait_seconds !== undefined && + (params.wait_seconds < 0 || params.wait_seconds > 30) + ) { + return 'wait_seconds must be between 0 and 30.'; + } + return null; + } + + protected createInvocation( + params: ReadShellParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ): ToolInvocation { + return new ReadShellToolInvocation( + params, + messageBus, + _toolName, + _toolDisplayName, + ); + } +} diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 9551fd9638..8ed78ba464 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -149,6 +149,8 @@ describe('ShellTool', () => { getShellBackgroundCompletionBehavior: vi.fn().mockReturnValue('silent'), getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), getSandboxEnabled: vi.fn().mockReturnValue(false), + getInteractiveShellMode: vi.fn().mockReturnValue('off'), + getSessionId: vi.fn().mockReturnValue('test-session-id'), sanitizationConfig: {}, get sandboxManager() { return mockSandboxManager; @@ -422,7 +424,7 @@ describe('ShellTool', () => { expect(mockShellBackground).toHaveBeenCalledWith( 12345, - 'default', + 'test-session-id', 'sleep 10', ); @@ -666,7 +668,7 @@ describe('ShellTool', () => { expect(mockShellBackground).toHaveBeenCalledWith( 12345, - 'default', + 'test-session-id', 'sleep 10', ); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 3ea29474c6..0407cb99bf 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -33,6 +33,7 @@ import { import { getErrorMessage } from '../utils/errors.js'; import { summarizeToolOutput } from '../utils/summarizer.js'; +import { formatShellOutput } from './shellOutputFormatter.js'; import { ShellExecutionService, type ShellOutputEvent, @@ -71,6 +72,7 @@ export interface ShellToolParams { is_background?: boolean; delay_ms?: number; [PARAM_ADDITIONAL_PERMISSIONS]?: SandboxPermissions; + wait_for_output_seconds?: number; } export class ShellToolInvocation extends BaseToolInvocation< @@ -78,6 +80,7 @@ export class ShellToolInvocation extends BaseToolInvocation< ToolResult > { private proactivePermissionsConfirmed?: SandboxPermissions; + private _autoPromoteTimer?: NodeJS.Timeout; constructor( private readonly context: AgentLoopContext, @@ -223,7 +226,12 @@ export class ShellToolInvocation extends BaseToolInvocation< } override getExplanation(): string { - return this.getContextualDetails().trim(); + let explanation = this.getContextualDetails().trim(); + const isAiMode = this.context.config.getInteractiveShellMode() === 'ai'; + if (this.params.wait_for_output_seconds !== undefined || isAiMode) { + explanation += ` [auto-background after ${this.params.wait_for_output_seconds ?? 5}s]`; + } + return explanation; } override getPolicyUpdateOptions( @@ -497,6 +505,21 @@ export class ShellToolInvocation extends BaseToolInvocation< }, timeoutMs); }; + let currentPid: number | undefined; + const isAiMode = this.context.config.getInteractiveShellMode() === 'ai'; + const shouldAutoPromote = + this.params.wait_for_output_seconds !== undefined || isAiMode; + const waitMs = (this.params.wait_for_output_seconds ?? 5) * 1000; + + const resetAutoPromoteTimer = () => { + if (shouldAutoPromote && currentPid) { + if (this._autoPromoteTimer) clearTimeout(this._autoPromoteTimer); + this._autoPromoteTimer = setTimeout(() => { + ShellExecutionService.background(currentPid!); + }, waitMs); + } + }; + signal.addEventListener('abort', onAbort, { once: true }); timeoutController.signal.addEventListener('abort', onAbort, { once: true, @@ -511,6 +534,7 @@ export class ShellToolInvocation extends BaseToolInvocation< cwd, (event: ShellOutputEvent) => { resetTimeout(); // Reset timeout on any event + resetAutoPromoteTimer(); // Reset auto-promote on any event if (!updateOutput) { return; } @@ -582,6 +606,7 @@ export class ShellToolInvocation extends BaseToolInvocation< backgroundCompletionBehavior: this.context.config.getShellBackgroundCompletionBehavior(), originalCommand: strippedCommand, + autoPromoteTimeoutMs: shouldAutoPromote ? waitMs : undefined, }, ); @@ -618,6 +643,11 @@ export class ShellToolInvocation extends BaseToolInvocation< }; } } + + // In AI mode with wait_for_output_seconds, set up auto-promotion timer. + // When the timer fires, promote to background instead of cancelling. + currentPid = pid; + resetAutoPromoteTimer(); } const result = await resultPromise; @@ -658,97 +688,75 @@ export class ShellToolInvocation extends BaseToolInvocation< } } - let data: BackgroundExecutionData | undefined; - - let llmContent = ''; let timeoutMessage = ''; if (result.aborted) { if (timeoutController.signal.aborted) { timeoutMessage = `Command was automatically cancelled because it exceeded the timeout of ${( timeoutMs / 60000 ).toFixed(1)} minutes without output.`; - llmContent = timeoutMessage; - } else { - llmContent = - 'Command was cancelled by user before it could complete.'; } - if (result.output.trim()) { - llmContent += ` Below is the output before it was cancelled:\n${result.output}`; - } else { - llmContent += ' There was no output before it was cancelled.'; - } - } else if (this.params.is_background || result.backgrounded) { - llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; - data = { - pid: result.pid, - command: this.params.command, - initialOutput: result.output, - }; - } else { - // Create a formatted error string for display, replacing the wrapper command - // with the user-facing command. - const llmContentParts = [`Output: ${result.output || '(empty)'}`]; - - if (result.error) { - const finalError = result.error.message.replaceAll( - commandToExecute, - this.params.command, - ); - llmContentParts.push(`Error: ${finalError}`); - } - - if (result.exitCode !== null && result.exitCode !== 0) { - llmContentParts.push(`Exit Code: ${result.exitCode}`); - data = { - exitCode: result.exitCode, - isError: true, - }; - } - - if (result.signal) { - llmContentParts.push(`Signal: ${result.signal}`); - } - if (backgroundPIDs.length) { - llmContentParts.push(`Background PIDs: ${backgroundPIDs.join(', ')}`); - } - if (result.pid) { - llmContentParts.push(`Process Group PGID: ${result.pid}`); - } - - llmContent = llmContentParts.join('\n'); } - let returnDisplay: string | AnsiOutput = ''; - if (this.context.config.getDebugMode()) { - returnDisplay = llmContent; - } else { - if (this.params.is_background || result.backgrounded) { - returnDisplay = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; - } else if (result.aborted) { - const cancelMsg = timeoutMessage || 'Command cancelled by user.'; - if (result.output.trim()) { - returnDisplay = `${cancelMsg}\n\nOutput before cancellation:\n${result.output}`; + const formatterOutput = formatShellOutput({ + params: this.params, + result, + debugMode: this.context.config.getDebugMode(), + backgroundPIDs, + isAiMode, + timeoutMessage, + }); + + let data: BackgroundExecutionData | undefined; + data = formatterOutput.data as BackgroundExecutionData | undefined; + let returnDisplay: string | AnsiOutput = formatterOutput.returnDisplay; + let llmContent = formatterOutput.llmContent; + + if (!this.context.config.getDebugMode()) { + if ( + !this.params.is_background && + !result.backgrounded && + !result.aborted + ) { + if (result.output.trim() || result.ansiOutput) { + returnDisplay = + result.ansiOutput && result.ansiOutput.length > 0 + ? result.ansiOutput + : result.output; } else { - returnDisplay = cancelMsg; + if (result.signal) { + returnDisplay = `Command terminated by signal: ${result.signal}`; + } else if (result.error) { + returnDisplay = `Command failed: ${getErrorMessage(result.error)}`; + } else if (result.exitCode !== null && result.exitCode !== 0) { + returnDisplay = `Command exited with code: ${result.exitCode}`; + } } - } else if (result.output.trim() || result.ansiOutput) { - returnDisplay = - result.ansiOutput && result.ansiOutput.length > 0 - ? result.ansiOutput - : result.output; - } else { - if (result.signal) { - returnDisplay = `Command terminated by signal: ${result.signal}`; - } else if (result.error) { - returnDisplay = `Command failed: ${getErrorMessage(result.error)}`; - } else if (result.exitCode !== null && result.exitCode !== 0) { - returnDisplay = `Command exited with code: ${result.exitCode}`; - } - // If output is empty and command succeeded (code 0, no error/signal/abort), - // returnDisplay will remain empty, which is fine. } } + // Replace wrapper command with actual command in error messages + if (result.error && !result.aborted) { + llmContent = llmContent.replaceAll( + commandToExecute, + this.params.command, + ); + } + + // Update data with specific things needed by ShellTool + if (this.params.is_background || result.backgrounded) { + data = { + ...data, + initialOutput: result.output, + pid: result.pid!, + command: this.params.command, + }; + } else if (result.exitCode !== null && result.exitCode !== 0) { + data = { + exitCode: result.exitCode, + isError: true, + } as BackgroundExecutionData; + } + // Heuristic Sandbox Denial Detection if ( !!result.error || @@ -929,6 +937,8 @@ export class ShellToolInvocation extends BaseToolInvocation< }; } finally { if (timeoutTimer) clearTimeout(timeoutTimer); + const autoTimer = this._autoPromoteTimer; + if (autoTimer) clearTimeout(autoTimer); signal.removeEventListener('abort', onAbort); timeoutController.signal.removeEventListener('abort', onAbort); try { @@ -1007,6 +1017,7 @@ export class ShellTool extends BaseDeclarativeTool< this.context.config.getEnableInteractiveShell(), this.context.config.getEnableShellOutputEfficiency(), this.context.config.getSandboxEnabled(), + this.context.config.getInteractiveShellMode(), ); return resolveToolDeclaration(definition, modelId); } diff --git a/packages/core/src/tools/shellOutputFormatter.ts b/packages/core/src/tools/shellOutputFormatter.ts new file mode 100644 index 0000000000..04d16fb42e --- /dev/null +++ b/packages/core/src/tools/shellOutputFormatter.ts @@ -0,0 +1,128 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { type ShellExecutionResult } from '../services/shellExecutionService.js'; +import { type ShellToolParams } from './shell.js'; + +export interface FormatShellOutputOptions { + params: ShellToolParams; + result: ShellExecutionResult; + debugMode: boolean; + timeoutMessage?: string; + backgroundPIDs: number[]; + summarizedOutput?: string; + isAiMode: boolean; +} + +export interface FormattedShellOutput { + llmContent: string; + returnDisplay: string; + data: Record; +} + +export function formatShellOutput( + options: FormatShellOutputOptions, +): FormattedShellOutput { + const { + params, + result, + debugMode, + timeoutMessage, + backgroundPIDs, + summarizedOutput, + } = options; + + let llmContent = ''; + let data: Record = {}; + + if (result.aborted) { + llmContent = timeoutMessage || 'Command cancelled by user.'; + if (result.output.trim()) { + llmContent += ` Below is the output before it was cancelled:\n${result.output}`; + } else { + llmContent += ' There was no output before it was cancelled.'; + } + } else if (params.is_background || result.backgrounded) { + const isAutoPromoted = result.backgrounded && !params.is_background; + if (isAutoPromoted) { + llmContent = `Command auto-promoted to background (PID: ${result.pid}). The process is still running. To check its screen state, call the read_shell tool with pid ${result.pid}. To send input or keystrokes, call the write_to_shell tool with pid ${result.pid}. If the process does not exit on its own when done, kill it with write_to_shell using special_keys=["Ctrl-C"].`; + } else { + llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; + } + data = { + pid: result.pid, + command: params.command, + directory: params.dir_path, + backgrounded: true, + }; + } else { + const llmContentParts: string[] = []; + + let content = summarizedOutput ?? result.output.trim(); + if (!content) { + content = '(empty)'; + } + + llmContentParts.push(`Output: ${content}`); + + if (result.error) { + llmContentParts.push(`Error: ${result.error.message}`); + } + + if (result.exitCode !== null && result.exitCode !== 0) { + llmContentParts.push(`Exit Code: ${result.exitCode}`); + } + if (result.signal !== null) { + llmContentParts.push(`Signal: ${result.signal}`); + } + if (backgroundPIDs.length) { + llmContentParts.push(`Background PIDs: ${backgroundPIDs.join(', ')}`); + } + if (result.pid) { + llmContentParts.push(`Process Group PGID: ${result.pid}`); + } + + llmContent = llmContentParts.join('\n'); + } + + let returnDisplay = ''; + if (debugMode) { + returnDisplay = llmContent; + } else { + if (params.is_background || result.backgrounded) { + const isAutoPromotedDisplay = + result.backgrounded && !params.is_background; + if (isAutoPromotedDisplay) { + returnDisplay = `Command auto-promoted to background (PID: ${result.pid}).`; + } else { + returnDisplay = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; + } + } else if (result.aborted) { + const cancelMsg = timeoutMessage || 'Command cancelled by user.'; + if (result.output.trim()) { + returnDisplay = `${cancelMsg}\n\nOutput before cancellation:\n${result.output}`; + } else { + returnDisplay = cancelMsg; + } + } else if (result.error) { + returnDisplay = `Command failed: ${result.error.message}`; + } else if (result.exitCode !== 0 && result.exitCode !== null) { + returnDisplay = `Command exited with code ${result.exitCode}`; + if (result.output.trim()) { + returnDisplay += `\n\n${result.output}`; + } + } else if (summarizedOutput) { + returnDisplay = `Command succeeded. Output summarized:\n${summarizedOutput}`; + } else { + returnDisplay = `Command succeeded.`; + if (result.output.trim()) { + returnDisplay += `\n\n${result.output}`; + } + } + } + + return { llmContent, returnDisplay, data }; +} diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 224f2ab0d5..47cc906c27 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -10,6 +10,8 @@ import { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, + READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -52,6 +54,12 @@ import { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, + SHELL_PARAM_WAIT_SECONDS, + WRITE_TO_SHELL_PARAM_PID, + WRITE_TO_SHELL_PARAM_INPUT, + WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, + READ_SHELL_PARAM_PID, + READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -90,6 +98,8 @@ export { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, + READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -136,6 +146,12 @@ export { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, + SHELL_PARAM_WAIT_SECONDS, + WRITE_TO_SHELL_PARAM_PID, + WRITE_TO_SHELL_PARAM_INPUT, + WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, + READ_SHELL_PARAM_PID, + READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -179,6 +195,7 @@ export const TOOLS_REQUIRING_NARROWING = new Set([ WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, ]); export const TRACKER_CREATE_TASK_TOOL_NAME = 'tracker_create_task'; @@ -251,6 +268,8 @@ export const ALL_BUILTIN_TOOL_NAMES = [ WEB_FETCH_TOOL_NAME, EDIT_TOOL_NAME, SHELL_TOOL_NAME, + WRITE_TO_SHELL_TOOL_NAME, + READ_SHELL_TOOL_NAME, GREP_TOOL_NAME, READ_MANY_FILES_TOOL_NAME, READ_FILE_TOOL_NAME, diff --git a/packages/core/src/tools/write-to-shell.ts b/packages/core/src/tools/write-to-shell.ts new file mode 100644 index 0000000000..652cb31bf5 --- /dev/null +++ b/packages/core/src/tools/write-to-shell.ts @@ -0,0 +1,230 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + type ToolConfirmationOutcome, + BaseDeclarativeTool, + BaseToolInvocation, + Kind, + type ToolInvocation, + type ToolResult, + type ToolCallConfirmationDetails, + type ToolExecuteConfirmationDetails, +} from './tools.js'; +import { ShellExecutionService } from '../services/shellExecutionService.js'; +import { + WRITE_TO_SHELL_TOOL_NAME, + WRITE_TO_SHELL_PARAM_PID, + WRITE_TO_SHELL_PARAM_INPUT, + WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, +} from './tool-names.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; + +/** + * Mapping of named special keys to their ANSI escape sequences. + */ +const SPECIAL_KEY_MAP: Record = { + Enter: '\r', + Tab: '\t', + Up: '\x1b[A', + Down: '\x1b[B', + Left: '\x1b[D', + Right: '\x1b[C', + Escape: '\x1b', + Backspace: '\x7f', + 'Ctrl-C': '\x03', + 'Ctrl-D': '\x04', + 'Ctrl-Z': '\x1a', + Space: ' ', + Delete: '\x1b[3~', + Home: '\x1b[H', + End: '\x1b[F', +}; + +const VALID_SPECIAL_KEYS = Object.keys(SPECIAL_KEY_MAP); + +/** Delay in ms to wait after writing input for the process to react. */ +const POST_INPUT_DELAY_MS = 150; + +export interface WriteToShellParams { + pid: number; + input?: string; + special_keys?: string[]; +} + +export class WriteToShellToolInvocation extends BaseToolInvocation< + WriteToShellParams, + ToolResult +> { + constructor( + params: WriteToShellParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ) { + super(params, messageBus, _toolName, _toolDisplayName); + } + + getDescription(): string { + const parts: string[] = [`write to shell PID ${this.params.pid}`]; + if (this.params.input) { + const display = + this.params.input.length > 50 + ? `${this.params.input.substring(0, 50)}...` + : this.params.input; + parts.push(`input: "${display}"`); + } + if (this.params.special_keys?.length) { + parts.push(`keys: [${this.params.special_keys.join(', ')}]`); + } + return parts.join(' '); + } + + protected override async getConfirmationDetails( + _abortSignal: AbortSignal, + ): Promise { + const confirmationDetails: ToolExecuteConfirmationDetails = { + type: 'exec', + title: 'Confirm Shell Input', + command: this.getDescription(), + rootCommand: 'write_to_shell', + rootCommands: ['write_to_shell'], + onConfirm: async (_outcome: ToolConfirmationOutcome) => { + // Policy updates handled centrally + }, + }; + return confirmationDetails; + } + + async execute(_signal: AbortSignal): Promise { + const { pid, input, special_keys } = this.params; + + // Validate the PID is active + if (!ShellExecutionService.isPtyActive(pid)) { + return { + llmContent: `Error: No active process found with PID ${pid}. The process may have exited.`, + returnDisplay: `No active process with PID ${pid}.`, + }; + } + + // Validate special keys + if (special_keys?.length) { + const invalidKeys = special_keys.filter( + (k) => !VALID_SPECIAL_KEYS.includes(k), + ); + if (invalidKeys.length > 0) { + return { + llmContent: `Error: Invalid special keys: ${invalidKeys.join(', ')}. Valid keys are: ${VALID_SPECIAL_KEYS.join(', ')}`, + returnDisplay: `Invalid special keys: ${invalidKeys.join(', ')}`, + }; + } + } + + // Send text input + if (input) { + ShellExecutionService.writeToPty(pid, input); + } + + // Send special keys + if (special_keys?.length) { + for (const key of special_keys) { + const sequence = SPECIAL_KEY_MAP[key]; + if (sequence) { + ShellExecutionService.writeToPty(pid, sequence); + } + } + } + + // Wait briefly for the process to react + await new Promise((resolve) => setTimeout(resolve, POST_INPUT_DELAY_MS)); + + // Read the screen after writing + const screen = ShellExecutionService.readScreen(pid); + if (screen === null) { + return { + llmContent: `Input sent, but the process (PID ${pid}) has exited.`, + returnDisplay: `Process exited after input.`, + }; + } + + return { + llmContent: `Input sent to PID ${pid}. Current screen:\n${screen}`, + returnDisplay: `Input sent to PID ${pid}.`, + }; + } +} + +export class WriteToShellTool extends BaseDeclarativeTool< + WriteToShellParams, + ToolResult +> { + static readonly Name = WRITE_TO_SHELL_TOOL_NAME; + + constructor(messageBus: MessageBus) { + super( + WriteToShellTool.Name, + 'WriteToShell', + 'Sends input to a running background shell process. Use this to interact with TUI applications, REPLs, and interactive commands. After writing, the current screen state is returned. Works with processes that were auto-promoted to background via wait_for_output_seconds or started with is_background=true.', + Kind.Execute, + { + type: 'object', + properties: { + [WRITE_TO_SHELL_PARAM_PID]: { + type: 'number', + description: + 'The PID of the background process to write to. Obtained from a previous run_shell_command call that was auto-promoted to background or started with is_background=true.', + }, + [WRITE_TO_SHELL_PARAM_INPUT]: { + type: 'string', + description: + '(OPTIONAL) Text to send to the process. This is literal text typed into the terminal.', + }, + [WRITE_TO_SHELL_PARAM_SPECIAL_KEYS]: { + type: 'array', + items: { + type: 'string', + enum: VALID_SPECIAL_KEYS, + }, + description: + '(OPTIONAL) Named special keys to send after the input text. Each key is sent in sequence. Examples: ["Enter"], ["Tab"], ["Up", "Enter"], ["Ctrl-C"].', + }, + }, + required: [WRITE_TO_SHELL_PARAM_PID], + }, + messageBus, + false, // output is not markdown + ); + } + + protected override validateToolParamValues( + params: WriteToShellParams, + ): string | null { + if (!params.pid || params.pid <= 0) { + return 'PID must be a positive number.'; + } + if ( + !params.input && + (!params.special_keys || !params.special_keys.length) + ) { + return 'At least one of input or special_keys must be provided.'; + } + return null; + } + + protected createInvocation( + params: WriteToShellParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ): ToolInvocation { + return new WriteToShellToolInvocation( + params, + messageBus, + _toolName, + _toolDisplayName, + ); + } +} From e7f8d9cf1ac64f18d196a9b60f8dc6cd4049ed37 Mon Sep 17 00:00:00 2001 From: Gaurav Ghosh Date: Wed, 8 Apr 2026 07:31:17 -0700 Subject: [PATCH 10/13] Revert "feat: Introduce an AI-driven interactive shell mode with new" This reverts commit 651ad63ed6daf4decf9071d5aa0bc9a4e715434d. --- packages/cli/src/config/config.ts | 1 - packages/cli/src/config/settingsSchema.ts | 20 -- packages/cli/src/ui/hooks/shellReducer.ts | 18 +- .../src/ui/hooks/useBackgroundShellManager.ts | 101 -------- .../cli/src/ui/hooks/useExecutionLifecycle.ts | 5 - packages/cli/src/ui/hooks/useGeminiStream.ts | 3 - packages/core/src/config/config.ts | 27 +- packages/core/src/prompts/promptProvider.ts | 1 - packages/core/src/prompts/snippets.ts | 16 +- .../src/services/shellExecutionService.ts | 41 ---- .../tools/definitions/base-declarations.ts | 12 - .../core/src/tools/definitions/coreTools.ts | 11 - .../dynamic-declaration-helpers.ts | 30 --- .../model-family-sets/default-legacy.ts | 2 - .../definitions/model-family-sets/gemini-3.ts | 2 - packages/core/src/tools/definitions/types.ts | 1 - packages/core/src/tools/read-shell.ts | 148 ----------- packages/core/src/tools/shell.test.ts | 6 +- packages/core/src/tools/shell.ts | 169 ++++++------- .../core/src/tools/shellOutputFormatter.ts | 128 ---------- packages/core/src/tools/tool-names.ts | 19 -- packages/core/src/tools/write-to-shell.ts | 230 ------------------ 22 files changed, 84 insertions(+), 907 deletions(-) delete mode 100644 packages/cli/src/ui/hooks/useBackgroundShellManager.ts delete mode 100644 packages/core/src/tools/read-shell.ts delete mode 100644 packages/core/src/tools/shellOutputFormatter.ts delete mode 100644 packages/core/src/tools/write-to-shell.ts diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 499b57b522..4e7e1db6f2 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1009,7 +1009,6 @@ export async function loadCliConfig( enableInteractiveShell: settings.tools?.shell?.enableInteractiveShell, shellBackgroundCompletionBehavior: settings.tools?.shell ?.backgroundCompletionBehavior as string | undefined, - interactiveShellMode: settings.tools?.shell?.interactiveShellMode, shellToolInactivityTimeout: settings.tools?.shell?.inactivityTimeout, enableShellOutputEfficiency: settings.tools?.shell?.enableShellOutputEfficiency ?? true, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index e654391566..c041aaa8c3 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1512,26 +1512,6 @@ const SETTINGS_SCHEMA = { { label: 'Notify', value: 'notify' }, ], }, - interactiveShellMode: { - type: 'enum', - label: 'Interactive Shell Mode', - category: 'Tools', - requiresRestart: true, - default: undefined as 'human' | 'ai' | 'off' | undefined, - description: oneLine` - Controls who can interact with backgrounded shell processes. - "human": user can Tab-focus and type into shells (default). - "ai": model gets write_to_shell/read_shell tools for TUI interaction. - "off": no interactive shell. - When set, overrides enableInteractiveShell. - `, - showInDialog: true, - options: [ - { value: 'human', label: 'Human (Tab to focus)' }, - { value: 'ai', label: 'AI (model-driven tools)' }, - { value: 'off', label: 'Off' }, - ], - }, pager: { type: 'string', label: 'Pager', diff --git a/packages/cli/src/ui/hooks/shellReducer.ts b/packages/cli/src/ui/hooks/shellReducer.ts index ea467fc327..0e9307259d 100644 --- a/packages/cli/src/ui/hooks/shellReducer.ts +++ b/packages/cli/src/ui/hooks/shellReducer.ts @@ -92,23 +92,7 @@ export function shellReducer( nextTasks.delete(action.pid); } nextTasks.set(action.pid, updatedTask); - - // Auto-hide panel when all tasks have exited - let nextVisible = state.isBackgroundTaskVisible; - if (action.update.status === 'exited') { - const hasRunning = Array.from(nextTasks.values()).some( - (s) => s.status === 'running', - ); - if (!hasRunning) { - nextVisible = false; - } - } - - return { - ...state, - backgroundTasks: nextTasks, - isBackgroundTaskVisible: nextVisible, - }; + return { ...state, backgroundTasks: nextTasks }; } case 'APPEND_TASK_OUTPUT': { const task = state.backgroundTasks.get(action.pid); diff --git a/packages/cli/src/ui/hooks/useBackgroundShellManager.ts b/packages/cli/src/ui/hooks/useBackgroundShellManager.ts deleted file mode 100644 index eb43ae1cfb..0000000000 --- a/packages/cli/src/ui/hooks/useBackgroundShellManager.ts +++ /dev/null @@ -1,101 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { useState, useEffect, useMemo, useRef } from 'react'; -import { type BackgroundTask } from './shellReducer.js'; - -export interface BackgroundShellManagerProps { - backgroundTasks: Map; - backgroundTaskCount: number; - isBackgroundTaskVisible: boolean; - activePtyId: number | null | undefined; - embeddedShellFocused: boolean; - setEmbeddedShellFocused: (focused: boolean) => void; - terminalHeight: number; -} - -export function useBackgroundShellManager({ - backgroundTasks, - backgroundTaskCount, - isBackgroundTaskVisible, - activePtyId, - embeddedShellFocused, - setEmbeddedShellFocused, - terminalHeight, -}: BackgroundShellManagerProps) { - const [isBackgroundShellListOpen, setIsBackgroundShellListOpen] = - useState(false); - const [activeBackgroundShellPid, setActiveBackgroundShellPid] = useState< - number | null - >(null); - - const prevShellCountRef = useRef(backgroundTaskCount); - - useEffect(() => { - if (backgroundTasks.size === 0) { - if (activeBackgroundShellPid !== null) { - setActiveBackgroundShellPid(null); - } - if (isBackgroundShellListOpen) { - setIsBackgroundShellListOpen(false); - } - } else if ( - activeBackgroundShellPid === null || - !backgroundTasks.has(activeBackgroundShellPid) - ) { - // If active shell is closed or none selected, select the first one - setActiveBackgroundShellPid(backgroundTasks.keys().next().value ?? null); - } else if (backgroundTaskCount > prevShellCountRef.current) { - // A new shell was added β€” auto-switch to the newest one (last in the map) - const pids = Array.from(backgroundTasks.keys()); - const newestPid = pids[pids.length - 1]; - if (newestPid !== undefined && newestPid !== activeBackgroundShellPid) { - setActiveBackgroundShellPid(newestPid); - } - } - prevShellCountRef.current = backgroundTaskCount; - }, [ - backgroundTasks, - activeBackgroundShellPid, - backgroundTaskCount, - isBackgroundShellListOpen, - ]); - - useEffect(() => { - if (embeddedShellFocused) { - const hasActiveForegroundShell = !!activePtyId; - const hasVisibleBackgroundShell = - isBackgroundTaskVisible && backgroundTasks.size > 0; - - if (!hasActiveForegroundShell && !hasVisibleBackgroundShell) { - setEmbeddedShellFocused(false); - } - } - }, [ - isBackgroundTaskVisible, - backgroundTasks, - embeddedShellFocused, - backgroundTaskCount, - activePtyId, - setEmbeddedShellFocused, - ]); - - const backgroundShellHeight = useMemo( - () => - isBackgroundTaskVisible && backgroundTasks.size > 0 - ? Math.max(Math.floor(terminalHeight * 0.3), 5) - : 0, - [isBackgroundTaskVisible, backgroundTasks.size, terminalHeight], - ); - - return { - isBackgroundShellListOpen, - setIsBackgroundShellListOpen, - activeBackgroundShellPid, - setActiveBackgroundShellPid, - backgroundShellHeight, - }; -} diff --git a/packages/cli/src/ui/hooks/useExecutionLifecycle.ts b/packages/cli/src/ui/hooks/useExecutionLifecycle.ts index 02e9e88cf5..2e80bf8f95 100644 --- a/packages/cli/src/ui/hooks/useExecutionLifecycle.ts +++ b/packages/cli/src/ui/hooks/useExecutionLifecycle.ts @@ -661,10 +661,6 @@ export const useExecutionLifecycle = ( (s: BackgroundTask) => s.status === 'running', ).length; - const showBackgroundShell = useCallback(() => { - dispatch({ type: 'SET_VISIBILITY', visible: true }); - }, [dispatch]); - return { handleShellCommand, activeShellPtyId: state.activeShellPtyId, @@ -672,7 +668,6 @@ export const useExecutionLifecycle = ( backgroundTaskCount, isBackgroundTaskVisible: state.isBackgroundTaskVisible, toggleBackgroundTasks, - showBackgroundShell, backgroundCurrentExecution, registerBackgroundTask, dismissBackgroundTask, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index c4a9c58d5e..a2621c4546 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -390,7 +390,6 @@ export const useGeminiStream = ( backgroundTaskCount, isBackgroundTaskVisible, toggleBackgroundTasks, - showBackgroundShell, backgroundCurrentExecution, registerBackgroundTask, dismissBackgroundTask, @@ -1918,7 +1917,6 @@ export const useGeminiStream = ( backgroundedTool.command, backgroundedTool.initialOutput, ); - showBackgroundShell(); } } @@ -2058,7 +2056,6 @@ export const useGeminiStream = ( modelSwitchedFromQuotaError, addItem, registerBackgroundTask, - showBackgroundShell, consumeUserHint, isLowErrorVerbosity, maybeAddSuppressedToolErrorNote, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index c82cc315b7..0edd4af7b0 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -36,8 +36,6 @@ import { GlobTool } from '../tools/glob.js'; import { ActivateSkillTool } from '../tools/activate-skill.js'; import { EditTool } from '../tools/edit.js'; import { ShellTool } from '../tools/shell.js'; -import { WriteToShellTool } from '../tools/write-to-shell.js'; -import { ReadShellTool } from '../tools/read-shell.js'; import { WriteFileTool } from '../tools/write-file.js'; import { WebFetchTool } from '../tools/web-fetch.js'; import { MemoryTool, setGeminiMdFilename } from '../tools/memoryTool.js'; @@ -658,7 +656,6 @@ export interface ConfigParameters { useRipgrep?: boolean; enableInteractiveShell?: boolean; shellBackgroundCompletionBehavior?: string; - interactiveShellMode?: 'human' | 'ai' | 'off'; skipNextSpeakerCheck?: boolean; shellExecutionConfig?: ShellExecutionConfig; extensionManagement?: boolean; @@ -871,7 +868,6 @@ export class Config implements McpContext, AgentLoopContext { | 'inject' | 'notify' | 'silent'; - private readonly interactiveShellMode: 'human' | 'ai' | 'off'; private readonly skipNextSpeakerCheck: boolean; private readonly useBackgroundColor: boolean; private readonly useAlternateBuffer: boolean; @@ -1239,14 +1235,6 @@ export class Config implements McpContext, AgentLoopContext { this.shellBackgroundCompletionBehavior = 'silent'; } - // interactiveShellMode takes precedence over enableInteractiveShell. - // If not set, derive from enableInteractiveShell for backward compat. - if (params.interactiveShellMode) { - this.interactiveShellMode = params.interactiveShellMode; - } else { - this.interactiveShellMode = this.enableInteractiveShell ? 'human' : 'off'; - } - this.skipNextSpeakerCheck = params.skipNextSpeakerCheck ?? true; this.shellExecutionConfig = { terminalWidth: params.shellExecutionConfig?.terminalWidth ?? 80, @@ -3223,14 +3211,10 @@ export class Config implements McpContext, AgentLoopContext { return ( this.interactive && this.ptyInfo !== 'child_process' && - this.interactiveShellMode !== 'off' + this.enableInteractiveShell ); } - getInteractiveShellMode(): 'human' | 'ai' | 'off' { - return this.interactiveShellMode; - } - isSkillsSupportEnabled(): boolean { return this.skillsSupport; } @@ -3591,15 +3575,6 @@ export class Config implements McpContext, AgentLoopContext { new ReadBackgroundOutputTool(this, this.messageBus), ), ); - // Register AI-driven interactive shell tools when mode is 'ai' - if (this.getInteractiveShellMode() === 'ai') { - maybeRegister(WriteToShellTool, () => - registry.registerTool(new WriteToShellTool(this.messageBus)), - ); - maybeRegister(ReadShellTool, () => - registry.registerTool(new ReadShellTool(this.messageBus)), - ); - } if (!this.isMemoryManagerEnabled()) { maybeRegister(MemoryTool, () => registry.registerTool(new MemoryTool(this.messageBus, this.storage)), diff --git a/packages/core/src/prompts/promptProvider.ts b/packages/core/src/prompts/promptProvider.ts index c4077afc95..0036dae560 100644 --- a/packages/core/src/prompts/promptProvider.ts +++ b/packages/core/src/prompts/promptProvider.ts @@ -200,7 +200,6 @@ export class PromptProvider { enableShellEfficiency: context.config.getEnableShellOutputEfficiency(), interactiveShellEnabled: context.config.isInteractiveShellEnabled(), - interactiveShellMode: context.config.getInteractiveShellMode(), topicUpdateNarration: context.config.isTopicUpdateNarrationEnabled(), memoryManagerEnabled: context.config.isMemoryManagerEnabled(), diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index b049ddf58e..59315e1ca6 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -18,8 +18,6 @@ import { MEMORY_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, - READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, WRITE_TODOS_TOOL_NAME, GREP_PARAM_TOTAL_MAX_MATCHES, @@ -83,7 +81,6 @@ export interface PrimaryWorkflowsOptions { export interface OperationalGuidelinesOptions { interactive: boolean; interactiveShellEnabled: boolean; - interactiveShellMode?: 'human' | 'ai' | 'off'; topicUpdateNarration: boolean; memoryManagerEnabled: boolean; } @@ -394,7 +391,7 @@ export function renderOperationalGuidelines( - **Command Execution:** Use the ${formatToolName(SHELL_TOOL_NAME)} tool for running shell commands, remembering the safety rule to explain modifying commands first.${toolUsageInteractive( options.interactive, options.interactiveShellEnabled, - )}${toolUsageRememberingFacts(options)}${toolUsageAiShell(options)} + )}${toolUsageRememberingFacts(options)} - **Confirmation Protocol:** If a tool call is declined or cancelled, respect the decision immediately. Do not re-attempt the action or "negotiate" for the same tool call unless the user explicitly directs you to. Offer an alternative technical path if possible. ## Interaction Details @@ -803,17 +800,6 @@ function toolUsageInteractive( - **Interactive Commands:** Always prefer non-interactive commands (e.g., using 'run once' or 'CI' flags for test runners to avoid persistent watch modes or 'git --no-pager') unless a persistent process is specifically required; however, some commands are only interactive and expect user input during their execution (e.g. ssh, vim).`; } -function toolUsageAiShell(options: OperationalGuidelinesOptions): string { - if (options.interactiveShellMode !== 'ai') return ''; - return ` -- **AI-Driven Interactive Shell:** Commands using \`wait_for_output_seconds\` auto-promote to background when they stall. Once promoted, use ${formatToolName(READ_SHELL_TOOL_NAME)} to see the terminal screen, then ${formatToolName(WRITE_TO_SHELL_TOOL_NAME)} to send text input and/or special keys (arrows, Enter, Ctrl-C, etc.). - - Set \`wait_for_output_seconds\` **low (2-5)** for commands that prompt for input (npx, installers, REPLs). Set **high (60+)** for long builds. Omit for instant commands. - - **Always read the screen before writing input.** The screen state tells you what the process is waiting for. - - When waiting for a command to finish (e.g. npm install), use ${formatToolName(READ_SHELL_TOOL_NAME)} with \`wait_seconds\` to delay before reading. Do NOT poll in a tight loop. - - **Clean up when done:** when your task is complete, kill background processes with ${formatToolName(WRITE_TO_SHELL_TOOL_NAME)} sending Ctrl-C, or note the PID for the user to clean up. - - You are the sole operator of promoted shells β€” the user cannot type into them.`; -} - function toolUsageRememberingFacts( options: OperationalGuidelinesOptions, ): string { diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 95b3f2d17b..dfbb3a5033 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -105,7 +105,6 @@ export interface ShellExecutionConfig { backgroundCompletionBehavior?: 'inject' | 'notify' | 'silent'; originalCommand?: string; sessionId?: string; - autoPromoteTimeoutMs?: number; } /** @@ -890,21 +889,6 @@ export class ShellExecutionService { sessionId: shellExecutionConfig.sessionId, }); - let autoPromoteTimer: NodeJS.Timeout | undefined; - const resetAutoPromoteTimer = () => { - if (shellExecutionConfig.autoPromoteTimeoutMs !== undefined) { - if (autoPromoteTimer) clearTimeout(autoPromoteTimer); - autoPromoteTimer = setTimeout(() => { - ShellExecutionService.background( - ptyPid, - shellExecutionConfig.sessionId, - ); - }, shellExecutionConfig.autoPromoteTimeoutMs); - } - }; - - resetAutoPromoteTimer(); - const result = ExecutionLifecycleService.attachExecution(ptyPid, { executionMethod: ptyInfo?.name ?? 'node-pty', writeInput: (input) => { @@ -1082,7 +1066,6 @@ export class ShellExecutionService { }); const handleOutput = (data: Buffer) => { - resetAutoPromoteTimer(); processingChain = processingChain.then( () => new Promise((resolveChunk) => { @@ -1152,7 +1135,6 @@ export class ShellExecutionService { ptyProcess.onExit( ({ exitCode, signal }: { exitCode: number; signal?: number }) => { - if (autoPromoteTimer) clearTimeout(autoPromoteTimer); exited = true; abortSignal.removeEventListener('abort', abortHandler); // Attempt to destroy the PTY to ensure FD is closed @@ -1238,7 +1220,6 @@ export class ShellExecutionService { ); const abortHandler = async () => { - if (autoPromoteTimer) clearTimeout(autoPromoteTimer); if (ptyProcess.pid && !exited) { await killProcessGroup({ pid: ptyPid, @@ -1417,28 +1398,6 @@ export class ShellExecutionService { return ExecutionLifecycleService.subscribe(pid, listener); } - /** - * Reads the current rendered screen state of a running process. - * Returns the full terminal buffer text for PTY processes, - * or the accumulated output for child processes. - * - * @param pid The process ID of the target process. - * @returns The screen text, or null if the process is not found. - */ - static readScreen(pid: number): string | null { - const activePty = this.activePtys.get(pid); - if (activePty) { - return getFullBufferText(activePty.headlessTerminal); - } - - const activeChild = this.activeChildProcesses.get(pid); - if (activeChild) { - return activeChild.state.output; - } - - return null; - } - /** * Resizes the pseudo-terminal (PTY) of a running process. * diff --git a/packages/core/src/tools/definitions/base-declarations.ts b/packages/core/src/tools/definitions/base-declarations.ts index e1575966af..89a5aa1614 100644 --- a/packages/core/src/tools/definitions/base-declarations.ts +++ b/packages/core/src/tools/definitions/base-declarations.ts @@ -56,18 +56,6 @@ export const READ_FILE_PARAM_END_LINE = 'end_line'; export const SHELL_TOOL_NAME = 'run_shell_command'; export const SHELL_PARAM_COMMAND = 'command'; export const SHELL_PARAM_IS_BACKGROUND = 'is_background'; -export const SHELL_PARAM_WAIT_SECONDS = 'wait_for_output_seconds'; - -// -- write_to_shell -- -export const WRITE_TO_SHELL_TOOL_NAME = 'write_to_shell'; -export const WRITE_TO_SHELL_PARAM_PID = 'pid'; -export const WRITE_TO_SHELL_PARAM_INPUT = 'input'; -export const WRITE_TO_SHELL_PARAM_SPECIAL_KEYS = 'special_keys'; - -// -- read_shell -- -export const READ_SHELL_TOOL_NAME = 'read_shell'; -export const READ_SHELL_PARAM_PID = 'pid'; -export const READ_SHELL_PARAM_WAIT_SECONDS = 'wait_seconds'; // -- write_file -- export const WRITE_FILE_TOOL_NAME = 'write_file'; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index a70ed1a33c..d1b81a6e99 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -27,8 +27,6 @@ export { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, - READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -75,12 +73,6 @@ export { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, - SHELL_PARAM_WAIT_SECONDS, - WRITE_TO_SHELL_PARAM_PID, - WRITE_TO_SHELL_PARAM_INPUT, - WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, - READ_SHELL_PARAM_PID, - READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -257,21 +249,18 @@ export function getShellDefinition( enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean = false, - interactiveShellMode?: string, ): ToolDefinition { return { base: getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ), overrides: (modelId) => getToolSet(modelId).run_shell_command( enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ), }; } diff --git a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts index 6f001c7459..29da313bf4 100644 --- a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts +++ b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts @@ -22,7 +22,6 @@ import { PARAM_DIR_PATH, SHELL_PARAM_IS_BACKGROUND, EXIT_PLAN_PARAM_PLAN_FILENAME, - SHELL_PARAM_WAIT_SECONDS, SKILL_PARAM_NAME, PARAM_ADDITIONAL_PERMISSIONS, UPDATE_TOPIC_TOOL_NAME, @@ -37,9 +36,7 @@ import { export function getShellToolDescription( enableInteractiveShell: boolean, enableEfficiency: boolean, - interactiveShellMode?: string, ): string { - const isAiMode = interactiveShellMode === 'ai'; const efficiencyGuidelines = enableEfficiency ? ` @@ -59,11 +56,6 @@ export function getShellToolDescription( Background PIDs: Only included if background processes were started. Process Group PGID: Only included if available.`; - if (isAiMode) { - const autoPromoteInstructions = `Commands that do not complete within \`${SHELL_PARAM_WAIT_SECONDS}\` seconds are automatically promoted to background. Once promoted, use \`write_to_shell\` and \`read_shell\` to interact with the process. Do NOT use \`&\` to background commands.`; - return `This tool executes a given shell command as \`bash -c \`. ${autoPromoteInstructions} Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${efficiencyGuidelines}${returnedInfo}`; - } - if (os.platform() === 'win32') { const backgroundInstructions = enableInteractiveShell ? `To run a command in the background, set the \`${SHELL_PARAM_IS_BACKGROUND}\` parameter to true. Do NOT use PowerShell background constructs.` @@ -94,33 +86,12 @@ export function getShellDeclaration( enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean = false, - interactiveShellMode?: string, ): FunctionDeclaration { - const isAiMode = interactiveShellMode === 'ai'; - - // In AI mode, use wait_for_output_seconds instead of is_background - const backgroundParam = isAiMode - ? { - [SHELL_PARAM_WAIT_SECONDS]: { - type: 'number' as const, - description: - 'Max seconds to wait for command to complete before auto-promoting to background (default: 5). Set low (2-5) for commands likely to prompt for input (npx, installers, REPLs). Set high (60-300) for long builds or installs. Once promoted, use write_to_shell/read_shell to interact.', - }, - } - : { - [SHELL_PARAM_IS_BACKGROUND]: { - type: 'boolean' as const, - description: - 'Set to true if this command should be run in the background (e.g. for long-running servers or watchers). The command will be started, allowed to run for a brief moment to check for immediate errors, and then moved to the background.', - }, - }; - return { name: SHELL_TOOL_NAME, description: getShellToolDescription( enableInteractiveShell, enableEfficiency, - interactiveShellMode, ), parametersJsonSchema: { type: 'object', @@ -149,7 +120,6 @@ export function getShellDeclaration( description: 'Optional. Delay in milliseconds to wait after starting the process in the background. Useful to allow the process to start and generate initial output before returning.', }, - ...backgroundParam, ...(enableToolSandboxing ? { [PARAM_ADDITIONAL_PERMISSIONS]: { diff --git a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts index 5441c39d09..60a52fc6ad 100644 --- a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts +++ b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts @@ -337,13 +337,11 @@ export const DEFAULT_LEGACY_SET: CoreToolSet = { enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ) => getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ), replace: { diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index f29f9e6814..a86a20378e 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -344,13 +344,11 @@ export const GEMINI_3_SET: CoreToolSet = { enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ) => getShellDeclaration( enableInteractiveShell, enableEfficiency, enableToolSandboxing, - interactiveShellMode, ), replace: { diff --git a/packages/core/src/tools/definitions/types.ts b/packages/core/src/tools/definitions/types.ts index d4f532f513..42c0cc7028 100644 --- a/packages/core/src/tools/definitions/types.ts +++ b/packages/core/src/tools/definitions/types.ts @@ -38,7 +38,6 @@ export interface CoreToolSet { enableInteractiveShell: boolean, enableEfficiency: boolean, enableToolSandboxing: boolean, - interactiveShellMode?: string, ) => FunctionDeclaration; replace: FunctionDeclaration; google_web_search: FunctionDeclaration; diff --git a/packages/core/src/tools/read-shell.ts b/packages/core/src/tools/read-shell.ts deleted file mode 100644 index 4e74cbbfa5..0000000000 --- a/packages/core/src/tools/read-shell.ts +++ /dev/null @@ -1,148 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - BaseDeclarativeTool, - BaseToolInvocation, - Kind, - type ToolInvocation, - type ToolResult, -} from './tools.js'; -import { ShellExecutionService } from '../services/shellExecutionService.js'; -import { - READ_SHELL_TOOL_NAME, - READ_SHELL_PARAM_PID, - READ_SHELL_PARAM_WAIT_SECONDS, -} from './tool-names.js'; -import type { MessageBus } from '../confirmation-bus/message-bus.js'; - -export interface ReadShellParams { - pid: number; - wait_seconds?: number; -} - -export class ReadShellToolInvocation extends BaseToolInvocation< - ReadShellParams, - ToolResult -> { - constructor( - params: ReadShellParams, - messageBus: MessageBus, - _toolName?: string, - _toolDisplayName?: string, - ) { - super(params, messageBus, _toolName, _toolDisplayName); - } - - getDescription(): string { - const waitPart = - this.params.wait_seconds !== undefined - ? ` (after ${this.params.wait_seconds}s)` - : ''; - return `read shell screen PID ${this.params.pid}${waitPart}`; - } - - async execute(signal: AbortSignal): Promise { - const { pid, wait_seconds } = this.params; - - // Wait before reading if requested - if (wait_seconds !== undefined && wait_seconds > 0) { - const waitMs = Math.min(wait_seconds, 30) * 1000; // Cap at 30s - await new Promise((resolve) => { - const timer = setTimeout(resolve, waitMs); - const onAbort = () => { - clearTimeout(timer); - resolve(); - }; - signal.addEventListener('abort', onAbort, { once: true }); - }); - } - - // Validate the PID is active - if (!ShellExecutionService.isPtyActive(pid)) { - return { - llmContent: `Error: No active process found with PID ${pid}. The process may have exited.`, - returnDisplay: `No active process with PID ${pid}.`, - }; - } - - const screen = ShellExecutionService.readScreen(pid); - if (screen === null) { - return { - llmContent: `Error: Could not read screen for PID ${pid}. The process may have exited.`, - returnDisplay: `Could not read screen for PID ${pid}.`, - }; - } - - return { - llmContent: screen, - returnDisplay: `Screen read from PID ${pid} (${screen.split('\n').length} lines).`, - }; - } -} - -export class ReadShellTool extends BaseDeclarativeTool< - ReadShellParams, - ToolResult -> { - static readonly Name = READ_SHELL_TOOL_NAME; - - constructor(messageBus: MessageBus) { - super( - ReadShellTool.Name, - 'ReadShell', - 'Reads the current screen state of a running background shell process. Returns the rendered terminal screen as text, preserving the visual layout. Use after write_to_shell to see updated output, or to check progress of a running command.', - Kind.Read, - { - type: 'object', - properties: { - [READ_SHELL_PARAM_PID]: { - type: 'number', - description: - 'The PID of the background process to read from. Obtained from a previous run_shell_command call that was auto-promoted to background or started with is_background=true.', - }, - [READ_SHELL_PARAM_WAIT_SECONDS]: { - type: 'number', - description: - 'Seconds to wait before reading the screen. Use this to let the process run for a while before checking output (e.g. wait for a build to finish). Max 30 seconds.', - }, - }, - required: [READ_SHELL_PARAM_PID], - }, - messageBus, - false, // output is not markdown - ); - } - - protected override validateToolParamValues( - params: ReadShellParams, - ): string | null { - if (!params.pid || params.pid <= 0) { - return 'PID must be a positive number.'; - } - if ( - params.wait_seconds !== undefined && - (params.wait_seconds < 0 || params.wait_seconds > 30) - ) { - return 'wait_seconds must be between 0 and 30.'; - } - return null; - } - - protected createInvocation( - params: ReadShellParams, - messageBus: MessageBus, - _toolName?: string, - _toolDisplayName?: string, - ): ToolInvocation { - return new ReadShellToolInvocation( - params, - messageBus, - _toolName, - _toolDisplayName, - ); - } -} diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 8ed78ba464..9551fd9638 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -149,8 +149,6 @@ describe('ShellTool', () => { getShellBackgroundCompletionBehavior: vi.fn().mockReturnValue('silent'), getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), getSandboxEnabled: vi.fn().mockReturnValue(false), - getInteractiveShellMode: vi.fn().mockReturnValue('off'), - getSessionId: vi.fn().mockReturnValue('test-session-id'), sanitizationConfig: {}, get sandboxManager() { return mockSandboxManager; @@ -424,7 +422,7 @@ describe('ShellTool', () => { expect(mockShellBackground).toHaveBeenCalledWith( 12345, - 'test-session-id', + 'default', 'sleep 10', ); @@ -668,7 +666,7 @@ describe('ShellTool', () => { expect(mockShellBackground).toHaveBeenCalledWith( 12345, - 'test-session-id', + 'default', 'sleep 10', ); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 0407cb99bf..3ea29474c6 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -33,7 +33,6 @@ import { import { getErrorMessage } from '../utils/errors.js'; import { summarizeToolOutput } from '../utils/summarizer.js'; -import { formatShellOutput } from './shellOutputFormatter.js'; import { ShellExecutionService, type ShellOutputEvent, @@ -72,7 +71,6 @@ export interface ShellToolParams { is_background?: boolean; delay_ms?: number; [PARAM_ADDITIONAL_PERMISSIONS]?: SandboxPermissions; - wait_for_output_seconds?: number; } export class ShellToolInvocation extends BaseToolInvocation< @@ -80,7 +78,6 @@ export class ShellToolInvocation extends BaseToolInvocation< ToolResult > { private proactivePermissionsConfirmed?: SandboxPermissions; - private _autoPromoteTimer?: NodeJS.Timeout; constructor( private readonly context: AgentLoopContext, @@ -226,12 +223,7 @@ export class ShellToolInvocation extends BaseToolInvocation< } override getExplanation(): string { - let explanation = this.getContextualDetails().trim(); - const isAiMode = this.context.config.getInteractiveShellMode() === 'ai'; - if (this.params.wait_for_output_seconds !== undefined || isAiMode) { - explanation += ` [auto-background after ${this.params.wait_for_output_seconds ?? 5}s]`; - } - return explanation; + return this.getContextualDetails().trim(); } override getPolicyUpdateOptions( @@ -505,21 +497,6 @@ export class ShellToolInvocation extends BaseToolInvocation< }, timeoutMs); }; - let currentPid: number | undefined; - const isAiMode = this.context.config.getInteractiveShellMode() === 'ai'; - const shouldAutoPromote = - this.params.wait_for_output_seconds !== undefined || isAiMode; - const waitMs = (this.params.wait_for_output_seconds ?? 5) * 1000; - - const resetAutoPromoteTimer = () => { - if (shouldAutoPromote && currentPid) { - if (this._autoPromoteTimer) clearTimeout(this._autoPromoteTimer); - this._autoPromoteTimer = setTimeout(() => { - ShellExecutionService.background(currentPid!); - }, waitMs); - } - }; - signal.addEventListener('abort', onAbort, { once: true }); timeoutController.signal.addEventListener('abort', onAbort, { once: true, @@ -534,7 +511,6 @@ export class ShellToolInvocation extends BaseToolInvocation< cwd, (event: ShellOutputEvent) => { resetTimeout(); // Reset timeout on any event - resetAutoPromoteTimer(); // Reset auto-promote on any event if (!updateOutput) { return; } @@ -606,7 +582,6 @@ export class ShellToolInvocation extends BaseToolInvocation< backgroundCompletionBehavior: this.context.config.getShellBackgroundCompletionBehavior(), originalCommand: strippedCommand, - autoPromoteTimeoutMs: shouldAutoPromote ? waitMs : undefined, }, ); @@ -643,11 +618,6 @@ export class ShellToolInvocation extends BaseToolInvocation< }; } } - - // In AI mode with wait_for_output_seconds, set up auto-promotion timer. - // When the timer fires, promote to background instead of cancelling. - currentPid = pid; - resetAutoPromoteTimer(); } const result = await resultPromise; @@ -688,73 +658,95 @@ export class ShellToolInvocation extends BaseToolInvocation< } } + let data: BackgroundExecutionData | undefined; + + let llmContent = ''; let timeoutMessage = ''; if (result.aborted) { if (timeoutController.signal.aborted) { timeoutMessage = `Command was automatically cancelled because it exceeded the timeout of ${( timeoutMs / 60000 ).toFixed(1)} minutes without output.`; + llmContent = timeoutMessage; + } else { + llmContent = + 'Command was cancelled by user before it could complete.'; } - } - - const formatterOutput = formatShellOutput({ - params: this.params, - result, - debugMode: this.context.config.getDebugMode(), - backgroundPIDs, - isAiMode, - timeoutMessage, - }); - - let data: BackgroundExecutionData | undefined; - data = formatterOutput.data as BackgroundExecutionData | undefined; - let returnDisplay: string | AnsiOutput = formatterOutput.returnDisplay; - let llmContent = formatterOutput.llmContent; - - if (!this.context.config.getDebugMode()) { - if ( - !this.params.is_background && - !result.backgrounded && - !result.aborted - ) { - if (result.output.trim() || result.ansiOutput) { - returnDisplay = - result.ansiOutput && result.ansiOutput.length > 0 - ? result.ansiOutput - : result.output; - } else { - if (result.signal) { - returnDisplay = `Command terminated by signal: ${result.signal}`; - } else if (result.error) { - returnDisplay = `Command failed: ${getErrorMessage(result.error)}`; - } else if (result.exitCode !== null && result.exitCode !== 0) { - returnDisplay = `Command exited with code: ${result.exitCode}`; - } - } + if (result.output.trim()) { + llmContent += ` Below is the output before it was cancelled:\n${result.output}`; + } else { + llmContent += ' There was no output before it was cancelled.'; } - } - - // Replace wrapper command with actual command in error messages - if (result.error && !result.aborted) { - llmContent = llmContent.replaceAll( - commandToExecute, - this.params.command, - ); - } - - // Update data with specific things needed by ShellTool - if (this.params.is_background || result.backgrounded) { + } else if (this.params.is_background || result.backgrounded) { + llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; data = { - ...data, - initialOutput: result.output, - pid: result.pid!, + pid: result.pid, command: this.params.command, + initialOutput: result.output, }; - } else if (result.exitCode !== null && result.exitCode !== 0) { - data = { - exitCode: result.exitCode, - isError: true, - } as BackgroundExecutionData; + } else { + // Create a formatted error string for display, replacing the wrapper command + // with the user-facing command. + const llmContentParts = [`Output: ${result.output || '(empty)'}`]; + + if (result.error) { + const finalError = result.error.message.replaceAll( + commandToExecute, + this.params.command, + ); + llmContentParts.push(`Error: ${finalError}`); + } + + if (result.exitCode !== null && result.exitCode !== 0) { + llmContentParts.push(`Exit Code: ${result.exitCode}`); + data = { + exitCode: result.exitCode, + isError: true, + }; + } + + if (result.signal) { + llmContentParts.push(`Signal: ${result.signal}`); + } + if (backgroundPIDs.length) { + llmContentParts.push(`Background PIDs: ${backgroundPIDs.join(', ')}`); + } + if (result.pid) { + llmContentParts.push(`Process Group PGID: ${result.pid}`); + } + + llmContent = llmContentParts.join('\n'); + } + + let returnDisplay: string | AnsiOutput = ''; + if (this.context.config.getDebugMode()) { + returnDisplay = llmContent; + } else { + if (this.params.is_background || result.backgrounded) { + returnDisplay = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; + } else if (result.aborted) { + const cancelMsg = timeoutMessage || 'Command cancelled by user.'; + if (result.output.trim()) { + returnDisplay = `${cancelMsg}\n\nOutput before cancellation:\n${result.output}`; + } else { + returnDisplay = cancelMsg; + } + } else if (result.output.trim() || result.ansiOutput) { + returnDisplay = + result.ansiOutput && result.ansiOutput.length > 0 + ? result.ansiOutput + : result.output; + } else { + if (result.signal) { + returnDisplay = `Command terminated by signal: ${result.signal}`; + } else if (result.error) { + returnDisplay = `Command failed: ${getErrorMessage(result.error)}`; + } else if (result.exitCode !== null && result.exitCode !== 0) { + returnDisplay = `Command exited with code: ${result.exitCode}`; + } + // If output is empty and command succeeded (code 0, no error/signal/abort), + // returnDisplay will remain empty, which is fine. + } } // Heuristic Sandbox Denial Detection @@ -937,8 +929,6 @@ export class ShellToolInvocation extends BaseToolInvocation< }; } finally { if (timeoutTimer) clearTimeout(timeoutTimer); - const autoTimer = this._autoPromoteTimer; - if (autoTimer) clearTimeout(autoTimer); signal.removeEventListener('abort', onAbort); timeoutController.signal.removeEventListener('abort', onAbort); try { @@ -1017,7 +1007,6 @@ export class ShellTool extends BaseDeclarativeTool< this.context.config.getEnableInteractiveShell(), this.context.config.getEnableShellOutputEfficiency(), this.context.config.getSandboxEnabled(), - this.context.config.getInteractiveShellMode(), ); return resolveToolDeclaration(definition, modelId); } diff --git a/packages/core/src/tools/shellOutputFormatter.ts b/packages/core/src/tools/shellOutputFormatter.ts deleted file mode 100644 index 04d16fb42e..0000000000 --- a/packages/core/src/tools/shellOutputFormatter.ts +++ /dev/null @@ -1,128 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { type ShellExecutionResult } from '../services/shellExecutionService.js'; -import { type ShellToolParams } from './shell.js'; - -export interface FormatShellOutputOptions { - params: ShellToolParams; - result: ShellExecutionResult; - debugMode: boolean; - timeoutMessage?: string; - backgroundPIDs: number[]; - summarizedOutput?: string; - isAiMode: boolean; -} - -export interface FormattedShellOutput { - llmContent: string; - returnDisplay: string; - data: Record; -} - -export function formatShellOutput( - options: FormatShellOutputOptions, -): FormattedShellOutput { - const { - params, - result, - debugMode, - timeoutMessage, - backgroundPIDs, - summarizedOutput, - } = options; - - let llmContent = ''; - let data: Record = {}; - - if (result.aborted) { - llmContent = timeoutMessage || 'Command cancelled by user.'; - if (result.output.trim()) { - llmContent += ` Below is the output before it was cancelled:\n${result.output}`; - } else { - llmContent += ' There was no output before it was cancelled.'; - } - } else if (params.is_background || result.backgrounded) { - const isAutoPromoted = result.backgrounded && !params.is_background; - if (isAutoPromoted) { - llmContent = `Command auto-promoted to background (PID: ${result.pid}). The process is still running. To check its screen state, call the read_shell tool with pid ${result.pid}. To send input or keystrokes, call the write_to_shell tool with pid ${result.pid}. If the process does not exit on its own when done, kill it with write_to_shell using special_keys=["Ctrl-C"].`; - } else { - llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; - } - data = { - pid: result.pid, - command: params.command, - directory: params.dir_path, - backgrounded: true, - }; - } else { - const llmContentParts: string[] = []; - - let content = summarizedOutput ?? result.output.trim(); - if (!content) { - content = '(empty)'; - } - - llmContentParts.push(`Output: ${content}`); - - if (result.error) { - llmContentParts.push(`Error: ${result.error.message}`); - } - - if (result.exitCode !== null && result.exitCode !== 0) { - llmContentParts.push(`Exit Code: ${result.exitCode}`); - } - if (result.signal !== null) { - llmContentParts.push(`Signal: ${result.signal}`); - } - if (backgroundPIDs.length) { - llmContentParts.push(`Background PIDs: ${backgroundPIDs.join(', ')}`); - } - if (result.pid) { - llmContentParts.push(`Process Group PGID: ${result.pid}`); - } - - llmContent = llmContentParts.join('\n'); - } - - let returnDisplay = ''; - if (debugMode) { - returnDisplay = llmContent; - } else { - if (params.is_background || result.backgrounded) { - const isAutoPromotedDisplay = - result.backgrounded && !params.is_background; - if (isAutoPromotedDisplay) { - returnDisplay = `Command auto-promoted to background (PID: ${result.pid}).`; - } else { - returnDisplay = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; - } - } else if (result.aborted) { - const cancelMsg = timeoutMessage || 'Command cancelled by user.'; - if (result.output.trim()) { - returnDisplay = `${cancelMsg}\n\nOutput before cancellation:\n${result.output}`; - } else { - returnDisplay = cancelMsg; - } - } else if (result.error) { - returnDisplay = `Command failed: ${result.error.message}`; - } else if (result.exitCode !== 0 && result.exitCode !== null) { - returnDisplay = `Command exited with code ${result.exitCode}`; - if (result.output.trim()) { - returnDisplay += `\n\n${result.output}`; - } - } else if (summarizedOutput) { - returnDisplay = `Command succeeded. Output summarized:\n${summarizedOutput}`; - } else { - returnDisplay = `Command succeeded.`; - if (result.output.trim()) { - returnDisplay += `\n\n${result.output}`; - } - } - } - - return { llmContent, returnDisplay, data }; -} diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 47cc906c27..224f2ab0d5 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -10,8 +10,6 @@ import { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, - READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -54,12 +52,6 @@ import { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, - SHELL_PARAM_WAIT_SECONDS, - WRITE_TO_SHELL_PARAM_PID, - WRITE_TO_SHELL_PARAM_INPUT, - WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, - READ_SHELL_PARAM_PID, - READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -98,8 +90,6 @@ export { LS_TOOL_NAME, READ_FILE_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, - READ_SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, WEB_SEARCH_TOOL_NAME, @@ -146,12 +136,6 @@ export { LS_PARAM_IGNORE, SHELL_PARAM_COMMAND, SHELL_PARAM_IS_BACKGROUND, - SHELL_PARAM_WAIT_SECONDS, - WRITE_TO_SHELL_PARAM_PID, - WRITE_TO_SHELL_PARAM_INPUT, - WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, - READ_SHELL_PARAM_PID, - READ_SHELL_PARAM_WAIT_SECONDS, WEB_SEARCH_PARAM_QUERY, WEB_FETCH_PARAM_PROMPT, READ_MANY_PARAM_INCLUDE, @@ -195,7 +179,6 @@ export const TOOLS_REQUIRING_NARROWING = new Set([ WRITE_FILE_TOOL_NAME, EDIT_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, ]); export const TRACKER_CREATE_TASK_TOOL_NAME = 'tracker_create_task'; @@ -268,8 +251,6 @@ export const ALL_BUILTIN_TOOL_NAMES = [ WEB_FETCH_TOOL_NAME, EDIT_TOOL_NAME, SHELL_TOOL_NAME, - WRITE_TO_SHELL_TOOL_NAME, - READ_SHELL_TOOL_NAME, GREP_TOOL_NAME, READ_MANY_FILES_TOOL_NAME, READ_FILE_TOOL_NAME, diff --git a/packages/core/src/tools/write-to-shell.ts b/packages/core/src/tools/write-to-shell.ts deleted file mode 100644 index 652cb31bf5..0000000000 --- a/packages/core/src/tools/write-to-shell.ts +++ /dev/null @@ -1,230 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - type ToolConfirmationOutcome, - BaseDeclarativeTool, - BaseToolInvocation, - Kind, - type ToolInvocation, - type ToolResult, - type ToolCallConfirmationDetails, - type ToolExecuteConfirmationDetails, -} from './tools.js'; -import { ShellExecutionService } from '../services/shellExecutionService.js'; -import { - WRITE_TO_SHELL_TOOL_NAME, - WRITE_TO_SHELL_PARAM_PID, - WRITE_TO_SHELL_PARAM_INPUT, - WRITE_TO_SHELL_PARAM_SPECIAL_KEYS, -} from './tool-names.js'; -import type { MessageBus } from '../confirmation-bus/message-bus.js'; - -/** - * Mapping of named special keys to their ANSI escape sequences. - */ -const SPECIAL_KEY_MAP: Record = { - Enter: '\r', - Tab: '\t', - Up: '\x1b[A', - Down: '\x1b[B', - Left: '\x1b[D', - Right: '\x1b[C', - Escape: '\x1b', - Backspace: '\x7f', - 'Ctrl-C': '\x03', - 'Ctrl-D': '\x04', - 'Ctrl-Z': '\x1a', - Space: ' ', - Delete: '\x1b[3~', - Home: '\x1b[H', - End: '\x1b[F', -}; - -const VALID_SPECIAL_KEYS = Object.keys(SPECIAL_KEY_MAP); - -/** Delay in ms to wait after writing input for the process to react. */ -const POST_INPUT_DELAY_MS = 150; - -export interface WriteToShellParams { - pid: number; - input?: string; - special_keys?: string[]; -} - -export class WriteToShellToolInvocation extends BaseToolInvocation< - WriteToShellParams, - ToolResult -> { - constructor( - params: WriteToShellParams, - messageBus: MessageBus, - _toolName?: string, - _toolDisplayName?: string, - ) { - super(params, messageBus, _toolName, _toolDisplayName); - } - - getDescription(): string { - const parts: string[] = [`write to shell PID ${this.params.pid}`]; - if (this.params.input) { - const display = - this.params.input.length > 50 - ? `${this.params.input.substring(0, 50)}...` - : this.params.input; - parts.push(`input: "${display}"`); - } - if (this.params.special_keys?.length) { - parts.push(`keys: [${this.params.special_keys.join(', ')}]`); - } - return parts.join(' '); - } - - protected override async getConfirmationDetails( - _abortSignal: AbortSignal, - ): Promise { - const confirmationDetails: ToolExecuteConfirmationDetails = { - type: 'exec', - title: 'Confirm Shell Input', - command: this.getDescription(), - rootCommand: 'write_to_shell', - rootCommands: ['write_to_shell'], - onConfirm: async (_outcome: ToolConfirmationOutcome) => { - // Policy updates handled centrally - }, - }; - return confirmationDetails; - } - - async execute(_signal: AbortSignal): Promise { - const { pid, input, special_keys } = this.params; - - // Validate the PID is active - if (!ShellExecutionService.isPtyActive(pid)) { - return { - llmContent: `Error: No active process found with PID ${pid}. The process may have exited.`, - returnDisplay: `No active process with PID ${pid}.`, - }; - } - - // Validate special keys - if (special_keys?.length) { - const invalidKeys = special_keys.filter( - (k) => !VALID_SPECIAL_KEYS.includes(k), - ); - if (invalidKeys.length > 0) { - return { - llmContent: `Error: Invalid special keys: ${invalidKeys.join(', ')}. Valid keys are: ${VALID_SPECIAL_KEYS.join(', ')}`, - returnDisplay: `Invalid special keys: ${invalidKeys.join(', ')}`, - }; - } - } - - // Send text input - if (input) { - ShellExecutionService.writeToPty(pid, input); - } - - // Send special keys - if (special_keys?.length) { - for (const key of special_keys) { - const sequence = SPECIAL_KEY_MAP[key]; - if (sequence) { - ShellExecutionService.writeToPty(pid, sequence); - } - } - } - - // Wait briefly for the process to react - await new Promise((resolve) => setTimeout(resolve, POST_INPUT_DELAY_MS)); - - // Read the screen after writing - const screen = ShellExecutionService.readScreen(pid); - if (screen === null) { - return { - llmContent: `Input sent, but the process (PID ${pid}) has exited.`, - returnDisplay: `Process exited after input.`, - }; - } - - return { - llmContent: `Input sent to PID ${pid}. Current screen:\n${screen}`, - returnDisplay: `Input sent to PID ${pid}.`, - }; - } -} - -export class WriteToShellTool extends BaseDeclarativeTool< - WriteToShellParams, - ToolResult -> { - static readonly Name = WRITE_TO_SHELL_TOOL_NAME; - - constructor(messageBus: MessageBus) { - super( - WriteToShellTool.Name, - 'WriteToShell', - 'Sends input to a running background shell process. Use this to interact with TUI applications, REPLs, and interactive commands. After writing, the current screen state is returned. Works with processes that were auto-promoted to background via wait_for_output_seconds or started with is_background=true.', - Kind.Execute, - { - type: 'object', - properties: { - [WRITE_TO_SHELL_PARAM_PID]: { - type: 'number', - description: - 'The PID of the background process to write to. Obtained from a previous run_shell_command call that was auto-promoted to background or started with is_background=true.', - }, - [WRITE_TO_SHELL_PARAM_INPUT]: { - type: 'string', - description: - '(OPTIONAL) Text to send to the process. This is literal text typed into the terminal.', - }, - [WRITE_TO_SHELL_PARAM_SPECIAL_KEYS]: { - type: 'array', - items: { - type: 'string', - enum: VALID_SPECIAL_KEYS, - }, - description: - '(OPTIONAL) Named special keys to send after the input text. Each key is sent in sequence. Examples: ["Enter"], ["Tab"], ["Up", "Enter"], ["Ctrl-C"].', - }, - }, - required: [WRITE_TO_SHELL_PARAM_PID], - }, - messageBus, - false, // output is not markdown - ); - } - - protected override validateToolParamValues( - params: WriteToShellParams, - ): string | null { - if (!params.pid || params.pid <= 0) { - return 'PID must be a positive number.'; - } - if ( - !params.input && - (!params.special_keys || !params.special_keys.length) - ) { - return 'At least one of input or special_keys must be provided.'; - } - return null; - } - - protected createInvocation( - params: WriteToShellParams, - messageBus: MessageBus, - _toolName?: string, - _toolDisplayName?: string, - ): ToolInvocation { - return new WriteToShellToolInvocation( - params, - messageBus, - _toolName, - _toolDisplayName, - ); - } -} From 1b3e7d674f48f08aa4e1bfa3bb525d90fc2f0b66 Mon Sep 17 00:00:00 2001 From: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Date: Wed, 8 Apr 2026 07:06:30 -0700 Subject: [PATCH 11/13] docs: update MCP server OAuth redirect port documentation (#24844) --- docs/tools/mcp-server.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 From e77b22e638869f741d0c8d2760abcfeebf94ae35 Mon Sep 17 00:00:00 2001 From: Gaurav <39389231+gsquared94@users.noreply.github.com> Date: Wed, 8 Apr 2026 22:31:10 +0800 Subject: [PATCH 12/13] fix: isolate concurrent browser agent instances (#24794) --- .../browser-agent.concurrent.responses | 8 + integration-tests/browser-agent.test.ts | 44 ++ .../browser/browserAgentFactory.test.ts | 2 + .../src/agents/browser/browserAgentFactory.ts | 379 +++++++++--------- .../browser/browserAgentInvocation.test.ts | 6 +- .../agents/browser/browserAgentInvocation.ts | 2 + .../src/agents/browser/browserManager.test.ts | 116 ++++++ .../core/src/agents/browser/browserManager.ts | 86 ++++ 8 files changed, 458 insertions(+), 185 deletions(-) create mode 100644 integration-tests/browser-agent.concurrent.responses 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/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; From 34b4f1c6e4f2468cd35caac8bde87011f2691063 Mon Sep 17 00:00:00 2001 From: ruomeng Date: Wed, 8 Apr 2026 11:58:29 -0400 Subject: [PATCH 13/13] refactor(plan): simplify policy priorities and consolidate read-only rules (#24849) --- .../config/policy-engine.integration.test.ts | 8 +-- packages/core/src/agents/registry.test.ts | 4 +- packages/core/src/policy/config.ts | 5 +- packages/core/src/policy/policies/plan.toml | 46 +++++---------- .../core/src/policy/policies/read-only.toml | 59 +++++++------------ .../core/src/policy/policies/tracker.toml | 34 ----------- .../core/src/policy/policy-engine.test.ts | 4 +- packages/core/src/policy/toml-loader.test.ts | 27 ++++++--- packages/core/src/policy/types.ts | 6 +- 9 files changed, 71 insertions(+), 122 deletions(-) delete mode 100644 packages/core/src/policy/policies/tracker.toml 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/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/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).