diff options
Diffstat (limited to 'packages/cli')
6 files changed, 61 insertions, 96 deletions
diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index b66afcc8..d3b73e06 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -539,6 +539,7 @@ export async function loadCliConfig( folderTrust, interactive, trustedFolder, + shouldUseNodePtyShell: settings.shouldUseNodePtyShell, }); } diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index ebea3ad5..8c1b5191 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -277,6 +277,17 @@ export const SETTINGS_SCHEMA = { showInDialog: true, }, + shouldUseNodePtyShell: { + type: 'boolean', + label: 'Use node-pty for Shell Execution', + category: 'Shell', + requiresRestart: true, + default: false, + description: + 'Use node-pty for shell command execution. Fallback to child_process still applies.', + showInDialog: true, + }, + selectedAuthType: { type: 'string', label: 'Selected Auth Type', diff --git a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts index 49d30799..3f51e858 100644 --- a/packages/cli/src/services/prompt-processors/shellProcessor.test.ts +++ b/packages/cli/src/services/prompt-processors/shellProcessor.test.ts @@ -46,8 +46,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { }); const SUCCESS_RESULT = { - stdout: 'default shell output', - stderr: '', + output: 'default shell output', exitCode: 0, error: null, aborted: false, @@ -64,6 +63,7 @@ describe('ShellProcessor', () => { mockConfig = { getTargetDir: vi.fn().mockReturnValue('/test/dir'), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), + getShouldUseNodePtyShell: vi.fn().mockReturnValue(false), }; context = createMockCommandContext({ @@ -120,7 +120,7 @@ describe('ShellProcessor', () => { disallowedCommands: [], }); mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'On branch main' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'On branch main' }), }); const result = await processor.process(prompt, context); @@ -135,6 +135,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe('The current status is: On branch main'); }); @@ -151,11 +152,11 @@ describe('ShellProcessor', () => { .mockReturnValueOnce({ result: Promise.resolve({ ...SUCCESS_RESULT, - stdout: 'On branch main', + output: 'On branch main', }), }) .mockReturnValueOnce({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: '/usr/home' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: '/usr/home' }), }); const result = await processor.process(prompt, context); @@ -188,7 +189,7 @@ describe('ShellProcessor', () => { // Override the approval mode for this test (mockConfig.getApprovalMode as Mock).mockReturnValue(ApprovalMode.YOLO); mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'deleted' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'deleted' }), }); const result = await processor.process(prompt, context); @@ -199,6 +200,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe('Do something dangerous: deleted'); }); @@ -324,10 +326,10 @@ describe('ShellProcessor', () => { mockShellExecute .mockReturnValueOnce({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'output1' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'output1' }), }) .mockReturnValueOnce({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'output2' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'output2' }), }); const result = await processor.process(prompt, context); @@ -361,7 +363,7 @@ describe('ShellProcessor', () => { disallowedCommands: [], }); mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'total 0' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'total 0' }), }); await processor.process(prompt, context); @@ -376,6 +378,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); }); @@ -398,7 +401,7 @@ describe('ShellProcessor', () => { const command = "awk '{print $1}' file.txt"; const prompt = `Output: !{${command}}`; mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'result' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'result' }), }); const result = await processor.process(prompt, context); @@ -413,6 +416,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe('Output: result'); }); @@ -422,7 +426,7 @@ describe('ShellProcessor', () => { const command = "echo '{{a},{b}}'"; const prompt = `!{${command}}`; mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: '{{a},{b}}' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: '{{a},{b}}' }), }); const result = await processor.process(prompt, context); @@ -431,6 +435,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe('{{a},{b}}'); }); @@ -455,45 +460,13 @@ describe('ShellProcessor', () => { }); describe('Error Reporting', () => { - it('should append stderr information if the command produces it', async () => { + it('should append exit code and command name on failure', async () => { const processor = new ShellProcessor('test-command'); const prompt = '!{cmd}'; mockShellExecute.mockReturnValue({ result: Promise.resolve({ ...SUCCESS_RESULT, - stdout: 'some output', - stderr: 'some error', - }), - }); - - const result = await processor.process(prompt, context); - - expect(result).toBe('some output\n--- STDERR ---\nsome error'); - }); - - it('should handle stderr-only output correctly', async () => { - const processor = new ShellProcessor('test-command'); - const prompt = '!{cmd}'; - mockShellExecute.mockReturnValue({ - result: Promise.resolve({ - ...SUCCESS_RESULT, - stdout: '', - stderr: 'error only', - }), - }); - - const result = await processor.process(prompt, context); - - expect(result).toBe('--- STDERR ---\nerror only'); - }); - - it('should append exit code and command name if the command fails', async () => { - const processor = new ShellProcessor('test-command'); - const prompt = 'Run a failing command: !{exit 1}'; - mockShellExecute.mockReturnValue({ - result: Promise.resolve({ - ...SUCCESS_RESULT, - stdout: 'some error output', + output: 'some error output', stderr: '', exitCode: 1, }), @@ -502,7 +475,7 @@ describe('ShellProcessor', () => { const result = await processor.process(prompt, context); expect(result).toBe( - "Run a failing command: some error output\n[Shell command 'exit 1' exited with code 1]", + "some error output\n[Shell command 'cmd' exited with code 1]", ); }); @@ -512,7 +485,7 @@ describe('ShellProcessor', () => { mockShellExecute.mockReturnValue({ result: Promise.resolve({ ...SUCCESS_RESULT, - stdout: 'output', + output: 'output', stderr: '', exitCode: null, signal: 'SIGTERM', @@ -526,25 +499,6 @@ describe('ShellProcessor', () => { ); }); - it('should append stderr and exit code information correctly', async () => { - const processor = new ShellProcessor('test-command'); - const prompt = '!{cmd}'; - mockShellExecute.mockReturnValue({ - result: Promise.resolve({ - ...SUCCESS_RESULT, - stdout: 'out', - stderr: 'err', - exitCode: 127, - }), - }); - - const result = await processor.process(prompt, context); - - expect(result).toBe( - "out\n--- STDERR ---\nerr\n[Shell command 'cmd' exited with code 127]", - ); - }); - it('should throw a detailed error if the shell fails to spawn', async () => { const processor = new ShellProcessor('test-command'); const prompt = '!{bad-command}'; @@ -572,7 +526,7 @@ describe('ShellProcessor', () => { mockShellExecute.mockReturnValue({ result: Promise.resolve({ ...SUCCESS_RESULT, - stdout: 'partial output', + output: 'partial output', stderr: '', exitCode: null, error: spawnError, @@ -609,7 +563,7 @@ describe('ShellProcessor', () => { const processor = new ShellProcessor('test-command'); const prompt = 'Outside: {{args}}. Inside: !{echo "hello"}'; mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'hello' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'hello' }), }); const result = await processor.process(prompt, context); @@ -621,7 +575,7 @@ describe('ShellProcessor', () => { const processor = new ShellProcessor('test-command'); const prompt = 'Command: !{grep {{args}} file.txt}'; mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'match found' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'match found' }), }); const result = await processor.process(prompt, context); @@ -634,6 +588,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe('Command: match found'); @@ -643,7 +598,7 @@ describe('ShellProcessor', () => { const processor = new ShellProcessor('test-command'); const prompt = 'User "({{args}})" requested search: !{search {{args}}}'; mockShellExecute.mockReturnValue({ - result: Promise.resolve({ ...SUCCESS_RESULT, stdout: 'results' }), + result: Promise.resolve({ ...SUCCESS_RESULT, output: 'results' }), }); const result = await processor.process(prompt, context); @@ -655,6 +610,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); expect(result).toBe(`User "(${rawArgs})" requested search: results`); @@ -718,6 +674,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); }); @@ -745,6 +702,7 @@ describe('ShellProcessor', () => { expect.any(String), expect.any(Function), expect.any(Object), + false, ); }); }); diff --git a/packages/cli/src/services/prompt-processors/shellProcessor.ts b/packages/cli/src/services/prompt-processors/shellProcessor.ts index 7acf2415..5242fa7a 100644 --- a/packages/cli/src/services/prompt-processors/shellProcessor.ts +++ b/packages/cli/src/services/prompt-processors/shellProcessor.ts @@ -137,11 +137,12 @@ export class ShellProcessor implements IPromptProcessor { // Execute the resolved command (which already has ESCAPED input). if (injection.resolvedCommand) { - const { result } = ShellExecutionService.execute( + const { result } = await ShellExecutionService.execute( injection.resolvedCommand, config.getTargetDir(), () => {}, new AbortController().signal, + config.getShouldUseNodePtyShell(), ); const executionResult = await result; @@ -154,15 +155,7 @@ export class ShellProcessor implements IPromptProcessor { } // Append the output, making stderr explicit for the model. - if (executionResult.stdout) { - processedPrompt += executionResult.stdout; - } - if (executionResult.stderr) { - if (executionResult.stdout) { - processedPrompt += '\n'; - } - processedPrompt += `--- STDERR ---\n${executionResult.stderr}`; - } + processedPrompt += executionResult.output; // Append a status message if the command did not succeed. if (executionResult.aborted) { diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts index d5270aba..9c13c8ec 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.test.ts @@ -65,7 +65,10 @@ describe('useShellCommandProcessor', () => { setPendingHistoryItemMock = vi.fn(); onExecMock = vi.fn(); onDebugMessageMock = vi.fn(); - mockConfig = { getTargetDir: () => '/test/dir' } as Config; + mockConfig = { + getTargetDir: () => '/test/dir', + getShouldUseNodePtyShell: () => false, + } as Config; mockGeminiClient = { addHistory: vi.fn() } as unknown as GeminiClient; vi.mocked(os.platform).mockReturnValue('linux'); @@ -104,13 +107,12 @@ describe('useShellCommandProcessor', () => { ): ShellExecutionResult => ({ rawOutput: Buffer.from(overrides.output || ''), output: 'Success', - stdout: 'Success', - stderr: '', exitCode: 0, signal: null, error: null, aborted: false, pid: 12345, + executionMethod: 'child_process', ...overrides, }); @@ -141,6 +143,7 @@ describe('useShellCommandProcessor', () => { '/test/dir', expect.any(Function), expect.any(Object), + false, ); expect(onExecMock).toHaveBeenCalledWith(expect.any(Promise)); }); @@ -223,7 +226,6 @@ describe('useShellCommandProcessor', () => { act(() => { mockShellOutputCallback({ type: 'data', - stream: 'stdout', chunk: 'hello', }); }); @@ -238,7 +240,6 @@ describe('useShellCommandProcessor', () => { act(() => { mockShellOutputCallback({ type: 'data', - stream: 'stdout', chunk: ' world', }); }); @@ -319,6 +320,7 @@ describe('useShellCommandProcessor', () => { '/test/dir', expect.any(Function), expect.any(Object), + false, ); }); diff --git a/packages/cli/src/ui/hooks/shellCommandProcessor.ts b/packages/cli/src/ui/hooks/shellCommandProcessor.ts index 08df0a74..23f2bb29 100644 --- a/packages/cli/src/ui/hooks/shellCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/shellCommandProcessor.ts @@ -101,10 +101,11 @@ export const useShellCommandProcessor = ( commandToExecute = `{ ${command} }; __code=$?; pwd > "${pwdFilePath}"; exit $__code`; } - const execPromise = new Promise<void>((resolve) => { + const executeCommand = async ( + resolve: (value: void | PromiseLike<void>) => void, + ) => { let lastUpdateTime = Date.now(); let cumulativeStdout = ''; - let cumulativeStderr = ''; let isBinaryStream = false; let binaryBytesReceived = 0; @@ -134,7 +135,7 @@ export const useShellCommandProcessor = ( onDebugMessage(`Executing in ${targetDir}: ${commandToExecute}`); try { - const { pid, result } = ShellExecutionService.execute( + const { pid, result } = await ShellExecutionService.execute( commandToExecute, targetDir, (event) => { @@ -142,11 +143,7 @@ export const useShellCommandProcessor = ( case 'data': // Do not process text data if we've already switched to binary mode. if (isBinaryStream) break; - if (event.stream === 'stdout') { - cumulativeStdout += event.chunk; - } else { - cumulativeStderr += event.chunk; - } + cumulativeStdout += event.chunk; break; case 'binary_detected': isBinaryStream = true; @@ -172,9 +169,7 @@ export const useShellCommandProcessor = ( '[Binary output detected. Halting stream...]'; } } else { - currentDisplayOutput = - cumulativeStdout + - (cumulativeStderr ? `\n${cumulativeStderr}` : ''); + currentDisplayOutput = cumulativeStdout; } // Throttle pending UI updates to avoid excessive re-renders. @@ -192,6 +187,7 @@ export const useShellCommandProcessor = ( } }, abortSignal, + config.getShouldUseNodePtyShell(), ); executionPid = pid; @@ -295,6 +291,10 @@ export const useShellCommandProcessor = ( resolve(); // Resolve the promise to unblock `onExec` } + }; + + const execPromise = new Promise<void>((resolve) => { + executeCommand(resolve); }); onExec(execPromise); |