From 467a305f266d30047d3c69b5fd680745e7580e39 Mon Sep 17 00:00:00 2001 From: Gal Zahavi <38544478+galz10@users.noreply.github.com> Date: Wed, 8 Oct 2025 13:28:19 -0700 Subject: [PATCH] chore(shell): Enable interactive shell by default (#10661) --- docs/get-started/configuration.md | 4 ++-- packages/cli/src/config/config.ts | 3 ++- packages/cli/src/config/settings.ts | 2 +- packages/cli/src/config/settingsSchema.ts | 2 +- .../services/prompt-processors/shellProcessor.test.ts | 2 +- .../src/services/prompt-processors/shellProcessor.ts | 2 +- .../cli/src/ui/components/messages/ToolMessage.tsx | 2 +- .../cli/src/ui/hooks/shellCommandProcessor.test.ts | 2 +- packages/cli/src/ui/hooks/shellCommandProcessor.ts | 4 ++-- packages/core/src/config/config.ts | 10 +++++----- packages/core/src/tools/shell.test.ts | 2 +- packages/core/src/tools/shell.ts | 2 +- 12 files changed, 19 insertions(+), 18 deletions(-) diff --git a/docs/get-started/configuration.md b/docs/get-started/configuration.md index 340a1bea28..484d2d4c65 100644 --- a/docs/get-started/configuration.md +++ b/docs/get-started/configuration.md @@ -206,8 +206,8 @@ Settings are organized into categories. All settings should be placed within the - **Default:** `undefined` - **`tools.shell.enableInteractiveShell`** (boolean): - - Use `node-pty` for an interactive shell experience. Fallback to `child_process` still applies. Defaults to `false`. + - **Description:** Enables interactive terminal for running shell commands. If an interactive session cannot be started, it will fall back to a standard shell. + - **Default:** `true` - **`tools.core`** (array of strings): - **Description:** This can be used to restrict the set of built-in tools [with an allowlist](../cli/enterprise.md#restricting-tool-access). See [Built-in Tools](../core/tools-api.md#built-in-tools) for a list of core tools. The match semantics are the same as `tools.allowed`. diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 5b3a4a6ae7..d0b884a5b4 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -737,7 +737,8 @@ export async function loadCliConfig( interactive, trustedFolder, useRipgrep: settings.tools?.useRipgrep, - shouldUseNodePtyShell: settings.tools?.shell?.enableInteractiveShell, + enableInteractiveShell: + settings.tools?.shell?.enableInteractiveShell ?? true, skipNextSpeakerCheck: settings.model?.skipNextSpeakerCheck, enablePromptCompletion: settings.general?.enablePromptCompletion ?? false, truncateToolOutputThreshold: settings.tools?.truncateToolOutputThreshold, diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 26b67c5556..e9047c699d 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -110,7 +110,7 @@ const MIGRATION_MAP: Record = { preferredEditor: 'general.preferredEditor', sandbox: 'tools.sandbox', selectedAuthType: 'security.auth.selectedType', - shouldUseNodePtyShell: 'tools.shell.enableInteractiveShell', + enableInteractiveShell: 'tools.shell.enableInteractiveShell', shellPager: 'tools.shell.pager', shellShowColor: 'tools.shell.showColor', skipNextSpeakerCheck: 'model.skipNextSpeakerCheck', diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 8765b9d0e9..70096ea958 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -713,7 +713,7 @@ const SETTINGS_SCHEMA = { label: 'Enable Interactive Shell', category: 'Tools', requiresRestart: true, - default: false, + default: true, description: 'Use node-pty for an interactive shell experience. Fallback to child_process still applies.', showInDialog: true, diff --git a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts index 393acb4725..e4021b54db 100644 --- a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts +++ b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts @@ -70,7 +70,7 @@ describe('ShellProcessor', () => { mockConfig = { getTargetDir: vi.fn().mockReturnValue('/test/dir'), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), - getShouldUseNodePtyShell: vi.fn().mockReturnValue(false), + getEnableInteractiveShell: vi.fn().mockReturnValue(false), getShellExecutionConfig: vi.fn().mockReturnValue({}), }; diff --git a/packages/cli/src/services/prompt-processors/shellProcessor.ts b/packages/cli/src/services/prompt-processors/shellProcessor.ts index 6e2b4adb72..350421c1c5 100644 --- a/packages/cli/src/services/prompt-processors/shellProcessor.ts +++ b/packages/cli/src/services/prompt-processors/shellProcessor.ts @@ -171,7 +171,7 @@ export class ShellProcessor implements IPromptProcessor { config.getTargetDir(), () => {}, new AbortController().signal, - config.getShouldUseNodePtyShell(), + config.getEnableInteractiveShell(), shellExecutionConfig, ); diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index 5f86d0d5e6..d563ee8cfd 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -93,7 +93,7 @@ export const ToolMessage: React.FC = ({ const isThisShellFocusable = (name === SHELL_COMMAND_NAME || name === 'Shell') && status === ToolCallStatus.Executing && - config?.getShouldUseNodePtyShell(); + config?.getEnableInteractiveShell(); const shouldShowFocusHint = isThisShellFocusable && (showFocusHint || userHasFocused); diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts index fa4f69e381..7295524572 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts @@ -65,7 +65,7 @@ describe('useShellCommandProcessor', () => { setShellInputFocusedMock = vi.fn(); mockConfig = { getTargetDir: () => '/test/dir', - getShouldUseNodePtyShell: () => false, + getEnableInteractiveShell: () => false, getShellExecutionConfig: () => ({ terminalHeight: 20, terminalWidth: 80, diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.ts index f8764e7998..417fa410d5 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.ts @@ -160,7 +160,7 @@ export const useShellCommandProcessor = ( if (isBinaryStream) break; // PTY provides the full screen state, so we just replace. // Child process provides chunks, so we append. - if (config.getShouldUseNodePtyShell()) { + if (config.getEnableInteractiveShell()) { cumulativeStdout = event.chunk; shouldUpdate = true; } else if ( @@ -221,7 +221,7 @@ export const useShellCommandProcessor = ( } }, abortSignal, - config.getShouldUseNodePtyShell(), + config.getEnableInteractiveShell(), shellExecutionConfig, ); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index bbc26a0e62..e918ba7e26 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -253,7 +253,7 @@ export interface ConfigParameters { interactive?: boolean; trustedFolder?: boolean; useRipgrep?: boolean; - shouldUseNodePtyShell?: boolean; + enableInteractiveShell?: boolean; skipNextSpeakerCheck?: boolean; shellExecutionConfig?: ShellExecutionConfig; extensionManagement?: boolean; @@ -342,7 +342,7 @@ export class Config { private readonly interactive: boolean; private readonly trustedFolder: boolean | undefined; private readonly useRipgrep: boolean; - private readonly shouldUseNodePtyShell: boolean; + private readonly enableInteractiveShell: boolean; private readonly skipNextSpeakerCheck: boolean; private shellExecutionConfig: ShellExecutionConfig; private readonly extensionManagement: boolean = true; @@ -432,7 +432,7 @@ export class Config { this.interactive = params.interactive ?? false; this.trustedFolder = params.trustedFolder; this.useRipgrep = params.useRipgrep ?? true; - this.shouldUseNodePtyShell = params.shouldUseNodePtyShell ?? false; + this.enableInteractiveShell = params.enableInteractiveShell ?? false; this.skipNextSpeakerCheck = params.skipNextSpeakerCheck ?? true; this.shellExecutionConfig = { terminalWidth: params.shellExecutionConfig?.terminalWidth ?? 80, @@ -942,8 +942,8 @@ export class Config { return this.useRipgrep; } - getShouldUseNodePtyShell(): boolean { - return this.shouldUseNodePtyShell; + getEnableInteractiveShell(): boolean { + return this.enableInteractiveShell; } getSkipNextSpeakerCheck(): boolean { diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index ac861bffb2..d5854df49f 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -60,7 +60,7 @@ describe('ShellTool', () => { .fn() .mockReturnValue(createMockWorkspaceContext('/test/dir')), getGeminiClient: vi.fn(), - getShouldUseNodePtyShell: vi.fn().mockReturnValue(false), + getEnableInteractiveShell: vi.fn().mockReturnValue(false), isInteractive: vi.fn().mockReturnValue(true), } as unknown as Config; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 5990208fa2..f9d017b626 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -251,7 +251,7 @@ export class ShellToolInvocation extends BaseToolInvocation< } }, signal, - this.config.getShouldUseNodePtyShell(), + this.config.getEnableInteractiveShell(), shellExecutionConfig ?? {}, );