jcarr
/
gemini-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert #6088 (#6328)

This commit is contained in:
Gal Zahavi 2025-08-15 10:27:33 -07:00 committed by GitHub
parent ab1c483cab
commit 1a2906a8ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 412 additions and 442 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
esbuild.config.js
View File

@ -21,14 +21,7 @@ esbuild
outfile: 'bundle/gemini.js', outfile: 'bundle/gemini.js',
platform: 'node', platform: 'node',
format: 'esm', format: 'esm',
external: [ external: [],
'@lydell/node-pty',
'@lydell/node-pty-darwin-arm64',
'@lydell/node-pty-darwin-x64',
'@lydell/node-pty-linux-x64',
'@lydell/node-pty-win32-arm64',
'@lydell/node-pty-win32-x64',
],
alias: { alias: {
'is-in-ci': path.resolve( 'is-in-ci': path.resolve(
__dirname, __dirname,

116
package-lock.json generated
View File

@ -11,7 +11,6 @@
"packages/*" "packages/*"
], ],
"dependencies": { "dependencies": {
"@lydell/node-pty": "1.1.0",
"node-fetch": "^3.3.2" "node-fetch": "^3.3.2"
}, },
"bin": { "bin": {
@ -51,13 +50,6 @@
}, },
"engines": { "engines": {
"node": ">=20.0.0" "node": ">=20.0.0"
},
"optionalDependencies": {
"@lydell/node-pty-darwin-arm64": "1.1.0",
"@lydell/node-pty-darwin-x64": "1.1.0",
"@lydell/node-pty-linux-x64": "1.1.0",
"@lydell/node-pty-win32-arm64": "1.1.0",
"@lydell/node-pty-win32-x64": "1.1.0"
} }
}, },
"node_modules/@alcalzone/ansi-tokenize": { "node_modules/@alcalzone/ansi-tokenize": {
@ -1463,98 +1455,6 @@
"integrity": "sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw==", "integrity": "sha512-GaHYm+c0O9MjZRu0ongGBRbinu8gVAMd2UZjji6jVmqKtZluZnptXGWhz1E8j8D2HJ3f/yMxKAUC0b+57wncIw==",
"license": "MIT" "license": "MIT"
}, },
"node_modules/@lydell/node-pty": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty/-/node-pty-1.1.0.tgz",
"integrity": "sha512-VDD8LtlMTOrPKWMXUAcB9+LTktzuunqrMwkYR1DMRBkS6LQrCt+0/Ws1o2rMml/n3guePpS7cxhHF7Nm5K4iMw==",
"license": "MIT",
"optionalDependencies": {
"@lydell/node-pty-darwin-arm64": "1.1.0",
"@lydell/node-pty-darwin-x64": "1.1.0",
"@lydell/node-pty-linux-arm64": "1.1.0",
"@lydell/node-pty-linux-x64": "1.1.0",
"@lydell/node-pty-win32-arm64": "1.1.0",
"@lydell/node-pty-win32-x64": "1.1.0"
}
},
"node_modules/@lydell/node-pty-darwin-arm64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-darwin-arm64/-/node-pty-darwin-arm64-1.1.0.tgz",
"integrity": "sha512-7kFD+owAA61qmhJCtoMbqj3Uvff3YHDiU+4on5F2vQdcMI3MuwGi7dM6MkFG/yuzpw8LF2xULpL71tOPUfxs0w==",
"cpu": [
"arm64"
],
"license": "MIT",
"optional": true,
"os": [
"darwin"
]
},
"node_modules/@lydell/node-pty-darwin-x64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-darwin-x64/-/node-pty-darwin-x64-1.1.0.tgz",
"integrity": "sha512-XZdvqj5FjAMjH8bdp0YfaZjur5DrCIDD1VYiE9EkkYVMDQqRUPHYV3U8BVEQVT9hYfjmpr7dNaELF2KyISWSNA==",
"cpu": [
"x64"
],
"license": "MIT",
"optional": true,
"os": [
"darwin"
]
},
"node_modules/@lydell/node-pty-linux-arm64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-linux-arm64/-/node-pty-linux-arm64-1.1.0.tgz",
"integrity": "sha512-yyDBmalCfHpLiQMT2zyLcqL2Fay4Xy7rIs8GH4dqKLnEviMvPGOK7LADVkKAsbsyXBSISL3Lt1m1MtxhPH6ckg==",
"cpu": [
"arm64"
],
"license": "MIT",
"optional": true,
"os": [
"linux"
]
},
"node_modules/@lydell/node-pty-linux-x64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-linux-x64/-/node-pty-linux-x64-1.1.0.tgz",
"integrity": "sha512-NcNqRTD14QT+vXcEuqSSvmWY+0+WUBn2uRE8EN0zKtDpIEr9d+YiFj16Uqds6QfcLCHfZmC+Ls7YzwTaqDnanA==",
"cpu": [
"x64"
],
"license": "MIT",
"optional": true,
"os": [
"linux"
]
},
"node_modules/@lydell/node-pty-win32-arm64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-win32-arm64/-/node-pty-win32-arm64-1.1.0.tgz",
"integrity": "sha512-JOMbCou+0fA7d/m97faIIfIU0jOv8sn2OR7tI45u3AmldKoKoLP8zHY6SAvDDnI3fccO1R2HeR1doVjpS7HM0w==",
"cpu": [
"arm64"
],
"license": "MIT",
"optional": true,
"os": [
"win32"
]
},
"node_modules/@lydell/node-pty-win32-x64": {
"version": "1.1.0",
"resolved": "https://registry.npmjs.org/@lydell/node-pty-win32-x64/-/node-pty-win32-x64-1.1.0.tgz",
"integrity": "sha512-3N56BZ+WDFnUMYRtsrr7Ky2mhWGl9xXcyqR6cexfuCqcz9RNWL+KoXRv/nZylY5dYaXkft4JaR1uVu+roiZDAw==",
"cpu": [
"x64"
],
"license": "MIT",
"optional": true,
"os": [
"win32"
]
},
"node_modules/@modelcontextprotocol/sdk": { "node_modules/@modelcontextprotocol/sdk": {
"version": "1.15.1", "version": "1.15.1",
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.15.1.tgz", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.15.1.tgz",
@ -3362,12 +3262,6 @@
"url": "https://opencollective.com/vitest" "url": "https://opencollective.com/vitest"
} }
}, },
"node_modules/@xterm/headless": {
"version": "5.5.0",
"resolved": "https://registry.npmjs.org/@xterm/headless/-/headless-5.5.0.tgz",
"integrity": "sha512-5xXB7kdQlFBP82ViMJTwwEc3gKCLGKR/eoxQm4zge7GPBl86tCdI0IdPJjoKd8mUSFXz5V7i/25sfsEkP4j46g==",
"license": "MIT"
},
"node_modules/accepts": { "node_modules/accepts": {
"version": "2.0.0", "version": "2.0.0",
"resolved": "https://registry.npmjs.org/accepts/-/accepts-2.0.0.tgz", "resolved": "https://registry.npmjs.org/accepts/-/accepts-2.0.0.tgz",
@ -12599,7 +12493,6 @@
"version": "0.1.21", "version": "0.1.21",
"dependencies": { "dependencies": {
"@google/genai": "1.13.0", "@google/genai": "1.13.0",
"@lydell/node-pty": "1.1.0",
"@modelcontextprotocol/sdk": "^1.11.0", "@modelcontextprotocol/sdk": "^1.11.0",
"@opentelemetry/api": "^1.9.0", "@opentelemetry/api": "^1.9.0",
"@opentelemetry/exporter-logs-otlp-grpc": "^0.52.0", "@opentelemetry/exporter-logs-otlp-grpc": "^0.52.0",
@ -12609,7 +12502,6 @@
"@opentelemetry/sdk-node": "^0.52.0", "@opentelemetry/sdk-node": "^0.52.0",
"@types/glob": "^8.1.0", "@types/glob": "^8.1.0",
"@types/html-to-text": "^9.0.4", "@types/html-to-text": "^9.0.4",
"@xterm/headless": "5.5.0",
"ajv": "^8.17.1", "ajv": "^8.17.1",
"chardet": "^2.1.0", "chardet": "^2.1.0",
"diff": "^7.0.0", "diff": "^7.0.0",
@ -12628,6 +12520,7 @@
"picomatch": "^4.0.1", "picomatch": "^4.0.1",
"shell-quote": "^1.8.3", "shell-quote": "^1.8.3",
"simple-git": "^3.28.0", "simple-git": "^3.28.0",
"strip-ansi": "^7.1.0",
"undici": "^7.10.0", "undici": "^7.10.0",
"ws": "^8.18.0" "ws": "^8.18.0"
}, },
@ -12644,13 +12537,6 @@
}, },
"engines": { "engines": {
"node": ">=20" "node": ">=20"
},
"optionalDependencies": {
"@lydell/node-pty-darwin-arm64": "1.1.0",
"@lydell/node-pty-darwin-x64": "1.1.0",
"@lydell/node-pty-linux-x64": "1.1.0",
"@lydell/node-pty-win32-arm64": "1.1.0",
"@lydell/node-pty-win32-x64": "1.1.0"
} }
}, },
"packages/core/node_modules/ajv": { "packages/core/node_modules/ajv": {

8
package.json
View File

@ -91,14 +91,6 @@
"yargs": "^17.7.2" "yargs": "^17.7.2"
}, },
"dependencies": { "dependencies": {
"@lydell/node-pty": "1.1.0",
"node-fetch": "^3.3.2" "node-fetch": "^3.3.2"
},
"optionalDependencies": {
"@lydell/node-pty-darwin-arm64": "1.1.0",
"@lydell/node-pty-darwin-x64": "1.1.0",
"@lydell/node-pty-linux-x64": "1.1.0",
"@lydell/node-pty-win32-arm64": "1.1.0",
"@lydell/node-pty-win32-x64": "1.1.0"
} }
} }

8
packages/cli/src/ui/hooks/shellCommandProcessor.test.ts
View File

@ -104,6 +104,8 @@ describe('useShellCommandProcessor', () => {
): ShellExecutionResult => ({ ): ShellExecutionResult => ({
rawOutput: Buffer.from(overrides.output || ''), rawOutput: Buffer.from(overrides.output || ''),
output: 'Success', output: 'Success',
stdout: 'Success',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,
@ -221,6 +223,7 @@ describe('useShellCommandProcessor', () => {
act(() => { act(() => {
mockShellOutputCallback({ mockShellOutputCallback({
type: 'data', type: 'data',
stream: 'stdout',
chunk: 'hello', chunk: 'hello',
}); });
}); });
@ -231,9 +234,12 @@ describe('useShellCommandProcessor', () => {
// Advance time and send another event to trigger the throttled update // Advance time and send another event to trigger the throttled update
await act(async () => { await act(async () => {
await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1); await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1);
});
act(() => {
mockShellOutputCallback({ mockShellOutputCallback({
type: 'data', type: 'data',
chunk: 'hello world', stream: 'stdout',
chunk: ' world',
}); });
}); });

11
packages/cli/src/ui/hooks/shellCommandProcessor.ts
View File

@ -104,6 +104,7 @@ export const useShellCommandProcessor = (
const execPromise = new Promise<void>((resolve) => { const execPromise = new Promise<void>((resolve) => {
let lastUpdateTime = Date.now(); let lastUpdateTime = Date.now();
let cumulativeStdout = ''; let cumulativeStdout = '';
let cumulativeStderr = '';
let isBinaryStream = false; let isBinaryStream = false;
let binaryBytesReceived = 0; let binaryBytesReceived = 0;
@ -141,7 +142,11 @@ export const useShellCommandProcessor = (
case 'data': case 'data':
// Do not process text data if we've already switched to binary mode. // Do not process text data if we've already switched to binary mode.
if (isBinaryStream) break; if (isBinaryStream) break;
cumulativeStdout = event.chunk; if (event.stream === 'stdout') {
cumulativeStdout += event.chunk;
} else {
cumulativeStderr += event.chunk;
}
break; break;
case 'binary_detected': case 'binary_detected':
isBinaryStream = true; isBinaryStream = true;
@ -167,7 +172,9 @@ export const useShellCommandProcessor = (
'[Binary output detected. Halting stream...]'; '[Binary output detected. Halting stream...]';
} }
} else { } else {
currentDisplayOutput = cumulativeStdout; currentDisplayOutput =
cumulativeStdout +
(cumulativeStderr ? `\n${cumulativeStderr}` : '');
} }
// Throttle pending UI updates to avoid excessive re-renders. // Throttle pending UI updates to avoid excessive re-renders.

5
packages/cli/src/ui/hooks/useReactToolScheduler.ts
View File

@ -62,8 +62,6 @@ export type TrackedToolCall =
| TrackedCompletedToolCall | TrackedCompletedToolCall
| TrackedCancelledToolCall; | TrackedCancelledToolCall;
import { useTerminalSize } from './useTerminalSize.js';
export function useReactToolScheduler( export function useReactToolScheduler(
onComplete: (tools: CompletedToolCall[]) => Promise<void>, onComplete: (tools: CompletedToolCall[]) => Promise<void>,
config: Config, config: Config,
@ -73,7 +71,6 @@ export function useReactToolScheduler(
getPreferredEditor: () => EditorType | undefined, getPreferredEditor: () => EditorType | undefined,
onEditorClose: () => void, onEditorClose: () => void,
): [TrackedToolCall[], ScheduleFn, MarkToolsAsSubmittedFn] { ): [TrackedToolCall[], ScheduleFn, MarkToolsAsSubmittedFn] {
const terminalSize = useTerminalSize();
const [toolCallsForDisplay, setToolCallsForDisplay] = useState< const [toolCallsForDisplay, setToolCallsForDisplay] = useState<
TrackedToolCall[] TrackedToolCall[]
>([]); >([]);
@ -143,7 +140,6 @@ export function useReactToolScheduler(
onToolCallsUpdate: toolCallsUpdateHandler, onToolCallsUpdate: toolCallsUpdateHandler,
getPreferredEditor, getPreferredEditor,
config, config,
getTerminalSize: () => terminalSize,
onEditorClose, onEditorClose,
}), }),
[ [
@ -152,7 +148,6 @@ export function useReactToolScheduler(
allToolCallsCompleteHandler, allToolCallsCompleteHandler,
toolCallsUpdateHandler, toolCallsUpdateHandler,
getPreferredEditor, getPreferredEditor,
terminalSize,
onEditorClose, onEditorClose,
], ],
); );

15
packages/cli/src/ui/hooks/useToolScheduler.test.ts
View File

@ -36,13 +36,6 @@ import {
HistoryItemToolGroup, HistoryItemToolGroup,
} from '../types.js'; } from '../types.js';
vi.mock('./useTerminalSize', () => ({
useTerminalSize: () => ({
columns: 80,
rows: 24,
}),
}));
// Mocks // Mocks
vi.mock('@google/gemini-cli-core', async () => { vi.mock('@google/gemini-cli-core', async () => {
const actual = await vi.importActual('@google/gemini-cli-core'); const actual = await vi.importActual('@google/gemini-cli-core');
@ -231,8 +224,8 @@ describe('useReactToolScheduler in YOLO Mode', () => {
request.args, request.args,
expect.any(AbortSignal), expect.any(AbortSignal),
undefined, undefined,
80, undefined,
24, undefined,
); );
// Check that onComplete was called with success // Check that onComplete was called with success
@ -383,8 +376,8 @@ describe('useReactToolScheduler', () => {
request.args, request.args,
expect.any(AbortSignal), expect.any(AbortSignal),
undefined, undefined,
80, undefined,
24, undefined,
); );
expect(onComplete).toHaveBeenCalledWith([ expect(onComplete).toHaveBeenCalledWith([
expect.objectContaining({ expect.objectContaining({

12
packages/core/package.json
View File

@ -48,17 +48,9 @@
"picomatch": "^4.0.1", "picomatch": "^4.0.1",
"shell-quote": "^1.8.3", "shell-quote": "^1.8.3",
"simple-git": "^3.28.0", "simple-git": "^3.28.0",
"strip-ansi": "^7.1.0",
"undici": "^7.10.0", "undici": "^7.10.0",
"ws": "^8.18.0", "ws": "^8.18.0"
"@lydell/node-pty": "1.1.0",
"@xterm/headless": "5.5.0"
},
"optionalDependencies": {
"@lydell/node-pty-darwin-arm64": "1.1.0",
"@lydell/node-pty-darwin-x64": "1.1.0",
"@lydell/node-pty-linux-x64": "1.1.0",
"@lydell/node-pty-win32-arm64": "1.1.0",
"@lydell/node-pty-win32-x64": "1.1.0"
}, },
"devDependencies": { "devDependencies": {
"@google/gemini-cli-test-utils": "file:../test-utils", "@google/gemini-cli-test-utils": "file:../test-utils",

6
packages/core/src/core/coreToolScheduler.test.ts
View File

@ -61,7 +61,6 @@ describe('CoreToolScheduler', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });
@ -118,7 +117,6 @@ describe('CoreToolScheduler with payload', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });
@ -415,7 +413,6 @@ describe('CoreToolScheduler edit cancellation', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });
@ -504,7 +501,6 @@ describe('CoreToolScheduler YOLO mode', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });
@ -590,7 +586,6 @@ describe('CoreToolScheduler request queueing', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });
@ -700,7 +695,6 @@ describe('CoreToolScheduler request queueing', () => {
onAllToolCallsComplete, onAllToolCallsComplete,
onToolCallsUpdate, onToolCallsUpdate,
getPreferredEditor: () => 'vscode', getPreferredEditor: () => 'vscode',
getTerminalSize: () => ({ columns: 80, rows: 24 }),
onEditorClose: vi.fn(), onEditorClose: vi.fn(),
}); });

11
packages/core/src/core/coreToolScheduler.ts
View File

@ -232,7 +232,6 @@ interface CoreToolSchedulerOptions {
onToolCallsUpdate?: ToolCallsUpdateHandler; onToolCallsUpdate?: ToolCallsUpdateHandler;
getPreferredEditor: () => EditorType | undefined; getPreferredEditor: () => EditorType | undefined;
config: Config; config: Config;
getTerminalSize: () => { columns: number; rows: number };
onEditorClose: () => void; onEditorClose: () => void;
} }
@ -244,7 +243,6 @@ export class CoreToolScheduler {
private onToolCallsUpdate?: ToolCallsUpdateHandler; private onToolCallsUpdate?: ToolCallsUpdateHandler;
private getPreferredEditor: () => EditorType | undefined; private getPreferredEditor: () => EditorType | undefined;
private config: Config; private config: Config;
private getTerminalSize: () => { columns: number; rows: number };
private onEditorClose: () => void; private onEditorClose: () => void;
private isFinalizingToolCalls = false; private isFinalizingToolCalls = false;
private isScheduling = false; private isScheduling = false;
@ -262,7 +260,6 @@ export class CoreToolScheduler {
this.onAllToolCallsComplete = options.onAllToolCallsComplete; this.onAllToolCallsComplete = options.onAllToolCallsComplete;
this.onToolCallsUpdate = options.onToolCallsUpdate; this.onToolCallsUpdate = options.onToolCallsUpdate;
this.getPreferredEditor = options.getPreferredEditor; this.getPreferredEditor = options.getPreferredEditor;
this.getTerminalSize = options.getTerminalSize;
this.onEditorClose = options.onEditorClose; this.onEditorClose = options.onEditorClose;
} }
@ -831,14 +828,8 @@ export class CoreToolScheduler {
} }
: undefined; : undefined;
const terminalSize = this.getTerminalSize();
invocation invocation
.execute( .execute(signal, liveOutputCallback)
signal,
liveOutputCallback,
terminalSize.columns,
terminalSize.rows,
)
.then(async (toolResult: ToolResult) => { .then(async (toolResult: ToolResult) => {
if (signal.aborted) { if (signal.aborted) {
this.setStatusInternal( this.setStatusInternal(

299
packages/core/src/services/shellExecutionService.test.ts
View File

@ -5,12 +5,14 @@
*/ */
import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest'; import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest';
const mockPtySpawn = vi.hoisted(() => vi.fn()); const mockSpawn = vi.hoisted(() => vi.fn());
vi.mock('@lydell/node-pty', () => ({ vi.mock('child_process', () => ({
spawn: mockPtySpawn, spawn: mockSpawn,
})); }));
import EventEmitter from 'events'; import EventEmitter from 'events';
import { Readable } from 'stream';
import { type ChildProcess } from 'child_process';
import { import {
ShellExecutionService, ShellExecutionService,
ShellOutputEvent, ShellOutputEvent,
@ -29,13 +31,12 @@ vi.mock('os', () => ({
platform: mockPlatform, platform: mockPlatform,
})); }));
const mockProcessKill = vi
.spyOn(process, 'kill')
.mockImplementation(() => true);
describe('ShellExecutionService', () => { describe('ShellExecutionService', () => {
let mockPtyProcess: EventEmitter & { let mockChildProcess: EventEmitter & Partial<ChildProcess>;
pid: number;
kill: Mock;
onData: Mock;
onExit: Mock;
};
let onOutputEventMock: Mock<(event: ShellOutputEvent) => void>; let onOutputEventMock: Mock<(event: ShellOutputEvent) => void>;
beforeEach(() => { beforeEach(() => {
@ -46,27 +47,26 @@ describe('ShellExecutionService', () => {
onOutputEventMock = vi.fn(); onOutputEventMock = vi.fn();
mockPtyProcess = new EventEmitter() as EventEmitter & { mockChildProcess = new EventEmitter() as EventEmitter &
pid: number; Partial<ChildProcess>;
kill: Mock; // FIX: Cast simple EventEmitters to the expected stream type.
onData: Mock; mockChildProcess.stdout = new EventEmitter() as Readable;
onExit: Mock; mockChildProcess.stderr = new EventEmitter() as Readable;
}; mockChildProcess.kill = vi.fn();
mockPtyProcess.pid = 12345;
mockPtyProcess.kill = vi.fn();
mockPtyProcess.onData = vi.fn();
mockPtyProcess.onExit = vi.fn();
mockPtySpawn.mockReturnValue(mockPtyProcess); // FIX: Use Object.defineProperty to set the readonly 'pid' property.
Object.defineProperty(mockChildProcess, 'pid', {
value: 12345,
configurable: true,
});
mockSpawn.mockReturnValue(mockChildProcess);
}); });
// Helper function to run a standard execution simulation // Helper function to run a standard execution simulation
const simulateExecution = async ( const simulateExecution = async (
command: string, command: string,
simulation: ( simulation: (cp: typeof mockChildProcess, ac: AbortController) => void,
ptyProcess: typeof mockPtyProcess,
ac: AbortController,
) => void,
) => { ) => {
const abortController = new AbortController(); const abortController = new AbortController();
const handle = ShellExecutionService.execute( const handle = ShellExecutionService.execute(
@ -77,123 +77,215 @@ describe('ShellExecutionService', () => {
); );
await new Promise((resolve) => setImmediate(resolve)); await new Promise((resolve) => setImmediate(resolve));
simulation(mockPtyProcess, abortController); simulation(mockChildProcess, abortController);
const result = await handle.result; const result = await handle.result;
return { result, handle, abortController }; return { result, handle, abortController };
}; };
describe('Successful Execution', () => { describe('Successful Execution', () => {
it('should execute a command and capture output', async () => { it('should execute a command and capture stdout and stderr', async () => {
const { result, handle } = await simulateExecution('ls -l', (pty) => { const { result, handle } = await simulateExecution('ls -l', (cp) => {
pty.onData.mock.calls[0][0]('file1.txt\n'); cp.stdout?.emit('data', Buffer.from('file1.txt\n'));
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.stderr?.emit('data', Buffer.from('a warning'));
cp.emit('exit', 0, null);
}); });
expect(mockPtySpawn).toHaveBeenCalledWith( expect(mockSpawn).toHaveBeenCalledWith(
'bash', 'ls -l',
['-c', 'ls -l'], [],
expect.any(Object), expect.objectContaining({ shell: 'bash' }),
); );
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
expect(result.signal).toBeNull(); expect(result.signal).toBeNull();
expect(result.error).toBeNull(); expect(result.error).toBeNull();
expect(result.aborted).toBe(false); expect(result.aborted).toBe(false);
expect(result.output).toBe('file1.txt'); expect(result.stdout).toBe('file1.txt\n');
expect(result.stderr).toBe('a warning');
expect(result.output).toBe('file1.txt\n\na warning');
expect(handle.pid).toBe(12345); expect(handle.pid).toBe(12345);
expect(onOutputEventMock).toHaveBeenCalledWith({ expect(onOutputEventMock).toHaveBeenCalledWith({
type: 'data', type: 'data',
chunk: 'file1.txt', stream: 'stdout',
chunk: 'file1.txt\n',
});
expect(onOutputEventMock).toHaveBeenCalledWith({
type: 'data',
stream: 'stderr',
chunk: 'a warning',
}); });
}); });
it('should strip ANSI codes from output', async () => { it('should strip ANSI codes from output', async () => {
const { result } = await simulateExecution('ls --color=auto', (pty) => { const { result } = await simulateExecution('ls --color=auto', (cp) => {
pty.onData.mock.calls[0][0]('a\u001b[31mred\u001b[0mword'); cp.stdout?.emit('data', Buffer.from('a\u001b[31mred\u001b[0mword'));
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.emit('exit', 0, null);
}); });
expect(result.output).toBe('aredword'); expect(result.stdout).toBe('aredword');
expect(onOutputEventMock).toHaveBeenCalledWith({ expect(onOutputEventMock).toHaveBeenCalledWith({
type: 'data', type: 'data',
stream: 'stdout',
chunk: 'aredword', chunk: 'aredword',
}); });
}); });
it('should correctly decode multi-byte characters split across chunks', async () => { it('should correctly decode multi-byte characters split across chunks', async () => {
const { result } = await simulateExecution('echo "你好"', (pty) => { const { result } = await simulateExecution('echo "你好"', (cp) => {
const multiByteChar = '你好'; const multiByteChar = Buffer.from('你好', 'utf-8');
pty.onData.mock.calls[0][0](multiByteChar.slice(0, 1)); cp.stdout?.emit('data', multiByteChar.slice(0, 2));
pty.onData.mock.calls[0][0](multiByteChar.slice(1)); cp.stdout?.emit('data', multiByteChar.slice(2));
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.emit('exit', 0, null);
}); });
expect(result.output).toBe('你好'); expect(result.stdout).toBe('你好');
}); });
it('should handle commands with no output', async () => { it('should handle commands with no output', async () => {
const { result } = await simulateExecution('touch file', (pty) => { const { result } = await simulateExecution('touch file', (cp) => {
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.emit('exit', 0, null);
}); });
expect(result.stdout).toBe('');
expect(result.stderr).toBe('');
expect(result.output).toBe(''); expect(result.output).toBe('');
expect(onOutputEventMock).not.toHaveBeenCalled(); expect(onOutputEventMock).not.toHaveBeenCalled();
}); });
}); });
describe('Failed Execution', () => { describe('Failed Execution', () => {
it('should capture a non-zero exit code', async () => { it('should capture a non-zero exit code and format output correctly', async () => {
const { result } = await simulateExecution('a-bad-command', (pty) => { const { result } = await simulateExecution('a-bad-command', (cp) => {
pty.onData.mock.calls[0][0]('command not found'); cp.stderr?.emit('data', Buffer.from('command not found'));
pty.onExit.mock.calls[0][0]({ exitCode: 127, signal: null }); cp.emit('exit', 127, null);
}); });
expect(result.exitCode).toBe(127); expect(result.exitCode).toBe(127);
expect(result.output).toBe('command not found'); expect(result.stderr).toBe('command not found');
expect(result.stdout).toBe('');
expect(result.output).toBe('\ncommand not found');
expect(result.error).toBeNull(); expect(result.error).toBeNull();
}); });
it('should capture a termination signal', async () => { it('should capture a termination signal', async () => {
const { result } = await simulateExecution('long-process', (pty) => { const { result } = await simulateExecution('long-process', (cp) => {
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: 15 }); cp.emit('exit', null, 'SIGTERM');
}); });
expect(result.exitCode).toBe(0); expect(result.exitCode).toBeNull();
expect(result.signal).toBe(15); expect(result.signal).toBe('SIGTERM');
}); });
it('should handle a synchronous spawn error', async () => { it('should handle a spawn error', async () => {
const spawnError = new Error('spawn ENOENT'); const spawnError = new Error('spawn EACCES');
mockPtySpawn.mockImplementation(() => { const { result } = await simulateExecution('protected-cmd', (cp) => {
throw spawnError; cp.emit('error', spawnError);
cp.emit('exit', 1, null);
}); });
const handle = ShellExecutionService.execute(
'any-command',
'/test/dir',
onOutputEventMock,
new AbortController().signal,
);
const result = await handle.result;
expect(result.error).toBe(spawnError); expect(result.error).toBe(spawnError);
expect(result.exitCode).toBe(1); expect(result.exitCode).toBe(1);
expect(result.output).toBe(''); });
expect(handle.pid).toBeUndefined();
it('handles errors that do not fire the exit event', async () => {
const error = new Error('spawn abc ENOENT');
const { result } = await simulateExecution('touch cat.jpg', (cp) => {
cp.emit('error', error); // No exit event is fired.
});
expect(result.error).toBe(error);
expect(result.exitCode).toBe(1);
}); });
}); });
describe('Aborting Commands', () => { describe('Aborting Commands', () => {
it('should abort a running process and set the aborted flag', async () => { describe.each([
const { result } = await simulateExecution( {
'sleep 10', platform: 'linux',
(pty, abortController) => { expectedSignal: 'SIGTERM',
abortController.abort(); expectedExit: { signal: 'SIGKILL' as const },
pty.onExit.mock.calls[0][0]({ exitCode: 1, signal: null }); },
}, {
platform: 'win32',
expectedCommand: 'taskkill',
expectedExit: { code: 1 },
},
])(
'on $platform',
({ platform, expectedSignal, expectedCommand, expectedExit }) => {
it('should abort a running process and set the aborted flag', async () => {
mockPlatform.mockReturnValue(platform);
const { result } = await simulateExecution(
'sleep 10',
(cp, abortController) => {
abortController.abort();
if (expectedExit.signal)
cp.emit('exit', null, expectedExit.signal);
if (typeof expectedExit.code === 'number')
cp.emit('exit', expectedExit.code, null);
},
);
expect(result.aborted).toBe(true);
if (platform === 'linux') {
expect(mockProcessKill).toHaveBeenCalledWith(
-mockChildProcess.pid!,
expectedSignal,
);
} else {
expect(mockSpawn).toHaveBeenCalledWith(expectedCommand, [
'/pid',
String(mockChildProcess.pid),
'/f',
'/t',
]);
}
});
},
);
it('should gracefully attempt SIGKILL on linux if SIGTERM fails', async () => {
mockPlatform.mockReturnValue('linux');
vi.useFakeTimers();
// Don't await the result inside the simulation block for this specific test.
// We need to control the timeline manually.
const abortController = new AbortController();
const handle = ShellExecutionService.execute(
'unresponsive_process',
'/test/dir',
onOutputEventMock,
abortController.signal,
); );
abortController.abort();
// Check the first kill signal
expect(mockProcessKill).toHaveBeenCalledWith(
-mockChildProcess.pid!,
'SIGTERM',
);
// Now, advance time past the timeout
await vi.advanceTimersByTimeAsync(250);
// Check the second kill signal
expect(mockProcessKill).toHaveBeenCalledWith(
-mockChildProcess.pid!,
'SIGKILL',
);
// Finally, simulate the process exiting and await the result
mockChildProcess.emit('exit', null, 'SIGKILL');
const result = await handle.result;
vi.useRealTimers();
expect(result.aborted).toBe(true); expect(result.aborted).toBe(true);
expect(mockPtyProcess.kill).toHaveBeenCalled(); expect(result.signal).toBe('SIGKILL');
// The individual kill calls were already asserted above.
expect(mockProcessKill).toHaveBeenCalledTimes(2);
}); });
}); });
@ -203,10 +295,10 @@ describe('ShellExecutionService', () => {
const binaryChunk1 = Buffer.from([0x89, 0x50, 0x4e, 0x47]); const binaryChunk1 = Buffer.from([0x89, 0x50, 0x4e, 0x47]);
const binaryChunk2 = Buffer.from([0x0d, 0x0a, 0x1a, 0x0a]); const binaryChunk2 = Buffer.from([0x0d, 0x0a, 0x1a, 0x0a]);
const { result } = await simulateExecution('cat image.png', (pty) => { const { result } = await simulateExecution('cat image.png', (cp) => {
pty.onData.mock.calls[0][0](binaryChunk1); cp.stdout?.emit('data', binaryChunk1);
pty.onData.mock.calls[0][0](binaryChunk2); cp.stdout?.emit('data', binaryChunk2);
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.emit('exit', 0, null);
}); });
expect(result.rawOutput).toEqual( expect(result.rawOutput).toEqual(
@ -229,13 +321,14 @@ describe('ShellExecutionService', () => {
it('should not emit data events after binary is detected', async () => { it('should not emit data events after binary is detected', async () => {
mockIsBinary.mockImplementation((buffer) => buffer.includes(0x00)); mockIsBinary.mockImplementation((buffer) => buffer.includes(0x00));
await simulateExecution('cat mixed_file', (pty) => { await simulateExecution('cat mixed_file', (cp) => {
pty.onData.mock.calls[0][0](Buffer.from('some text')); cp.stdout?.emit('data', Buffer.from('some text'));
pty.onData.mock.calls[0][0](Buffer.from([0x00, 0x01, 0x02])); cp.stdout?.emit('data', Buffer.from([0x00, 0x01, 0x02]));
pty.onData.mock.calls[0][0](Buffer.from('more text')); cp.stdout?.emit('data', Buffer.from('more text'));
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); cp.emit('exit', 0, null);
}); });
// FIX: Provide explicit type for the 'call' parameter in the map function.
const eventTypes = onOutputEventMock.mock.calls.map( const eventTypes = onOutputEventMock.mock.calls.map(
(call: [ShellOutputEvent]) => call[0].type, (call: [ShellOutputEvent]) => call[0].type,
); );
@ -251,27 +344,31 @@ describe('ShellExecutionService', () => {
describe('Platform-Specific Behavior', () => { describe('Platform-Specific Behavior', () => {
it('should use cmd.exe on Windows', async () => { it('should use cmd.exe on Windows', async () => {
mockPlatform.mockReturnValue('win32'); mockPlatform.mockReturnValue('win32');
await simulateExecution('dir "foo bar"', (pty) => await simulateExecution('dir "foo bar"', (cp) =>
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }), cp.emit('exit', 0, null),
); );
expect(mockPtySpawn).toHaveBeenCalledWith( expect(mockSpawn).toHaveBeenCalledWith(
'cmd.exe', 'dir "foo bar"',
['/c', 'dir "foo bar"'], [],
expect.any(Object), expect.objectContaining({
shell: true,
detached: false,
}),
); );
}); });
it('should use bash on Linux', async () => { it('should use bash and detached process group on Linux', async () => {
mockPlatform.mockReturnValue('linux'); mockPlatform.mockReturnValue('linux');
await simulateExecution('ls "foo bar"', (pty) => await simulateExecution('ls "foo bar"', (cp) => cp.emit('exit', 0, null));
pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }),
);
expect(mockPtySpawn).toHaveBeenCalledWith( expect(mockSpawn).toHaveBeenCalledWith(
'bash', 'ls "foo bar"',
['-c', 'ls "foo bar"'], [],
expect.any(Object), expect.objectContaining({
shell: 'bash',
detached: true,
}),
); );
}); });
}); });

279
packages/core/src/services/shellExecutionService.ts
View File

@ -4,35 +4,29 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import * as pty from '@lydell/node-pty'; import { spawn } from 'child_process';
import { TextDecoder } from 'util'; import { TextDecoder } from 'util';
import os from 'os'; import os from 'os';
import stripAnsi from 'strip-ansi';
import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js'; import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js';
import { isBinary } from '../utils/textUtils.js'; import { isBinary } from '../utils/textUtils.js';
import pkg from '@xterm/headless';
const { Terminal } = pkg;
// @ts-expect-error getFullText is not a public API. const SIGKILL_TIMEOUT_MS = 200;
const getFullText = (terminal: Terminal) => {
const buffer = terminal.buffer.active;
const lines: string[] = [];
for (let i = 0; i < buffer.length; i++) {
const line = buffer.getLine(i);
lines.push(line ? line.translateToString(true) : '');
}
return lines.join('\n').trim();
};
/** A structured result from a shell command execution. */ /** A structured result from a shell command execution. */
export interface ShellExecutionResult { export interface ShellExecutionResult {
/** The raw, unprocessed output buffer. */ /** The raw, unprocessed output buffer. */
rawOutput: Buffer; rawOutput: Buffer;
/** The combined, decoded output as a string. */ /** The combined, decoded stdout and stderr as a string. */
output: string; output: string;
/** The decoded stdout as a string. */
stdout: string;
/** The decoded stderr as a string. */
stderr: string;
/** The process exit code, or null if terminated by a signal. */ /** The process exit code, or null if terminated by a signal. */
exitCode: number | null; exitCode: number | null;
/** The signal that terminated the process, if any. */ /** The signal that terminated the process, if any. */
signal: number | null; signal: NodeJS.Signals | null;
/** An error object if the process failed to spawn. */ /** An error object if the process failed to spawn. */
error: Error | null; error: Error | null;
/** A boolean indicating if the command was aborted by the user. */ /** A boolean indicating if the command was aborted by the user. */
@ -56,6 +50,8 @@ export type ShellOutputEvent =
| { | {
/** The event contains a chunk of output data. */ /** The event contains a chunk of output data. */
type: 'data'; type: 'data';
/** The stream from which the data originated. */
stream: 'stdout' | 'stderr';
/** The decoded string chunk. */ /** The decoded string chunk. */
chunk: string; chunk: string;
} }
@ -77,7 +73,7 @@ export type ShellOutputEvent =
*/ */
export class ShellExecutionService { export class ShellExecutionService {
/** /**
* Executes a shell command using `node-pty`, capturing all output and lifecycle events. * Executes a shell command using `spawn`, capturing all output and lifecycle events.
* *
* @param commandToExecute The exact command string to run. * @param commandToExecute The exact command string to run.
* @param cwd The working directory to execute the command in. * @param cwd The working directory to execute the command in.
@ -91,150 +87,167 @@ export class ShellExecutionService {
cwd: string, cwd: string,
onOutputEvent: (event: ShellOutputEvent) => void, onOutputEvent: (event: ShellOutputEvent) => void,
abortSignal: AbortSignal, abortSignal: AbortSignal,
terminalColumns?: number,
terminalRows?: number,
): ShellExecutionHandle { ): ShellExecutionHandle {
const isWindows = os.platform() === 'win32'; const isWindows = os.platform() === 'win32';
const shell = isWindows ? 'cmd.exe' : 'bash';
const args = isWindows
? ['/c', commandToExecute]
: ['-c', commandToExecute];
let ptyProcess; const child = spawn(commandToExecute, [], {
try { cwd,
ptyProcess = pty.spawn(shell, args, { stdio: ['ignore', 'pipe', 'pipe'],
cwd, // Use bash unless in Windows (since it doesn't support bash).
name: 'xterm-color', // For windows, just use the default.
cols: terminalColumns ?? 200, shell: isWindows ? true : 'bash',
rows: terminalRows ?? 20, // Use process groups on non-Windows for robust killing.
env: { // Windows process termination is handled by `taskkill /t`.
...process.env, detached: !isWindows,
GEMINI_CLI: '1', env: {
}, ...process.env,
handleFlowControl: true, GEMINI_CLI: '1',
}); },
} catch (e) { });
const error = e as Error;
return {
pid: undefined,
result: Promise.resolve({
rawOutput: Buffer.from(''),
output: '',
exitCode: 1,
signal: null,
error,
aborted: false,
pid: undefined,
}),
};
}
const result = new Promise<ShellExecutionResult>((resolve) => { const result = new Promise<ShellExecutionResult>((resolve) => {
const headlessTerminal = new Terminal({ // Use decoders to handle multi-byte characters safely (for streaming output).
allowProposedApi: true, let stdoutDecoder: TextDecoder | null = null;
cols: terminalColumns ?? 200, let stderrDecoder: TextDecoder | null = null;
rows: terminalRows ?? 20,
}); let stdout = '';
let processingChain = Promise.resolve(); let stderr = '';
let decoder: TextDecoder | null = null;
let output = '';
const outputChunks: Buffer[] = []; const outputChunks: Buffer[] = [];
const error: Error | null = null; let error: Error | null = null;
let exited = false; let exited = false;
let isStreamingRawContent = true; let isStreamingRawContent = true;
const MAX_SNIFF_SIZE = 4096; const MAX_SNIFF_SIZE = 4096;
let sniffedBytes = 0; let sniffedBytes = 0;
const handleOutput = (data: Buffer) => { const handleOutput = (data: Buffer, stream: 'stdout' | 'stderr') => {
// NOTE: The migration from `child_process` to `node-pty` means we if (!stdoutDecoder || !stderrDecoder) {
// no longer have separate `stdout` and `stderr` streams. The `data` const encoding = getCachedEncodingForBuffer(data);
// buffer contains the merged output. If a drop in LLM quality is try {
// observed after this change, we may need to revisit this and stdoutDecoder = new TextDecoder(encoding);
// explore ways to re-introduce that distinction. stderrDecoder = new TextDecoder(encoding);
processingChain = processingChain.then( } catch {
() => // If the encoding is not supported, fall back to utf-8.
new Promise<void>((resolve) => { // This can happen on some platforms for certain encodings like 'utf-32le'.
if (!decoder) { stdoutDecoder = new TextDecoder('utf-8');
const encoding = getCachedEncodingForBuffer(data); stderrDecoder = new TextDecoder('utf-8');
try { }
decoder = new TextDecoder(encoding); }
} catch {
decoder = new TextDecoder('utf-8');
}
}
outputChunks.push(data); outputChunks.push(data);
// First, check if we need to switch to binary mode. // Binary detection logic. This only runs until we've made a determination.
if (isStreamingRawContent && sniffedBytes < MAX_SNIFF_SIZE) { if (isStreamingRawContent && sniffedBytes < MAX_SNIFF_SIZE) {
const sniffBuffer = Buffer.concat(outputChunks.slice(0, 20)); const sniffBuffer = Buffer.concat(outputChunks.slice(0, 20));
sniffedBytes = sniffBuffer.length; sniffedBytes = sniffBuffer.length;
if (isBinary(sniffBuffer)) { if (isBinary(sniffBuffer)) {
isStreamingRawContent = false; // Change state to stop streaming raw content.
onOutputEvent({ type: 'binary_detected' }); isStreamingRawContent = false;
} onOutputEvent({ type: 'binary_detected' });
} }
}
// Now, based on the *current* state, either process as text or binary. const decodedChunk =
if (isStreamingRawContent) { stream === 'stdout'
const decodedChunk = decoder.decode(data, { stream: true }); ? stdoutDecoder.decode(data, { stream: true })
headlessTerminal.write(decodedChunk, () => { : stderrDecoder.decode(data, { stream: true });
const newStrippedOutput = getFullText(headlessTerminal); const strippedChunk = stripAnsi(decodedChunk);
output = newStrippedOutput;
onOutputEvent({ type: 'data', chunk: newStrippedOutput }); if (stream === 'stdout') {
resolve(); stdout += strippedChunk;
}); } else {
} else { stderr += strippedChunk;
// Once in binary mode, we only emit progress events. }
const totalBytes = outputChunks.reduce(
(sum, chunk) => sum + chunk.length, if (isStreamingRawContent) {
0, onOutputEvent({ type: 'data', stream, chunk: strippedChunk });
); } else {
onOutputEvent({ const totalBytes = outputChunks.reduce(
type: 'binary_progress', (sum, chunk) => sum + chunk.length,
bytesReceived: totalBytes, 0,
}); );
resolve(); onOutputEvent({ type: 'binary_progress', bytesReceived: totalBytes });
} }
}),
);
}; };
ptyProcess.onData((data) => { child.stdout.on('data', (data) => handleOutput(data, 'stdout'));
const bufferData = Buffer.from(data, 'utf-8'); child.stderr.on('data', (data) => handleOutput(data, 'stderr'));
handleOutput(bufferData); child.on('error', (err) => {
}); const { stdout, stderr, finalBuffer } = cleanup();
error = err;
ptyProcess.onExit(({ exitCode, signal }) => { resolve({
exited = true; error,
abortSignal.removeEventListener('abort', abortHandler); stdout,
stderr,
processingChain.then(() => { rawOutput: finalBuffer,
const finalBuffer = Buffer.concat(outputChunks); output: stdout + (stderr ? `\n${stderr}` : ''),
exitCode: 1,
resolve({ signal: null,
rawOutput: finalBuffer, aborted: false,
output, pid: child.pid,
exitCode,
signal: signal ?? null,
error,
aborted: abortSignal.aborted,
pid: ptyProcess.pid,
});
}); });
}); });
const abortHandler = async () => { const abortHandler = async () => {
if (ptyProcess.pid && !exited) { if (child.pid && !exited) {
ptyProcess.kill('SIGHUP'); if (isWindows) {
spawn('taskkill', ['/pid', child.pid.toString(), '/f', '/t']);
} else {
try {
// Kill the entire process group (negative PID).
// SIGTERM first, then SIGKILL if it doesn't die.
process.kill(-child.pid, 'SIGTERM');
await new Promise((res) => setTimeout(res, SIGKILL_TIMEOUT_MS));
if (!exited) {
process.kill(-child.pid, 'SIGKILL');
}
} catch (_e) {
// Fall back to killing just the main process if group kill fails.
if (!exited) child.kill('SIGKILL');
}
}
} }
}; };
abortSignal.addEventListener('abort', abortHandler, { once: true }); abortSignal.addEventListener('abort', abortHandler, { once: true });
child.on('exit', (code: number, signal: NodeJS.Signals) => {
const { stdout, stderr, finalBuffer } = cleanup();
resolve({
rawOutput: finalBuffer,
output: stdout + (stderr ? `\n${stderr}` : ''),
stdout,
stderr,
exitCode: code,
signal,
error,
aborted: abortSignal.aborted,
pid: child.pid,
});
});
/**
* Cleans up a process (and it's accompanying state) that is exiting or
* erroring and returns output formatted output buffers and strings
*/
function cleanup() {
exited = true;
abortSignal.removeEventListener('abort', abortHandler);
if (stdoutDecoder) {
stdout += stripAnsi(stdoutDecoder.decode());
}
if (stderrDecoder) {
stderr += stripAnsi(stderrDecoder.decode());
}
const finalBuffer = Buffer.concat(outputChunks);
return { stdout, stderr, finalBuffer };
}
}); });
return { pid: ptyProcess.pid, result }; return { pid: child.pid, result };
} }
} }

34
packages/core/src/tools/shell.test.ts
View File

@ -66,6 +66,7 @@ describe('ShellTool', () => {
Buffer.from('abcdef', 'hex'), Buffer.from('abcdef', 'hex'),
); );
// Capture the output callback to simulate streaming events from the service
mockShellExecutionService.mockImplementation((_cmd, _cwd, callback) => { mockShellExecutionService.mockImplementation((_cmd, _cwd, callback) => {
mockShellOutputCallback = callback; mockShellOutputCallback = callback;
return { return {
@ -122,6 +123,8 @@ describe('ShellTool', () => {
const fullResult: ShellExecutionResult = { const fullResult: ShellExecutionResult = {
rawOutput: Buffer.from(result.output || ''), rawOutput: Buffer.from(result.output || ''),
output: 'Success', output: 'Success',
stdout: 'Success',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,
@ -138,7 +141,7 @@ describe('ShellTool', () => {
resolveShellExecution({ pid: 54321 }); resolveShellExecution({ pid: 54321 });
vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n'); vi.mocked(fs.readFileSync).mockReturnValue('54321\n54322\n'); // Service PID and background PID
const result = await promise; const result = await promise;
@ -149,8 +152,6 @@ describe('ShellTool', () => {
expect.any(String), expect.any(String),
expect.any(Function), expect.any(Function),
mockAbortSignal, mockAbortSignal,
undefined,
undefined,
); );
expect(result.llmContent).toContain('Background PIDs: 54322'); expect(result.llmContent).toContain('Background PIDs: 54322');
expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile); expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile);
@ -163,6 +164,8 @@ describe('ShellTool', () => {
resolveShellExecution({ resolveShellExecution({
rawOutput: Buffer.from(''), rawOutput: Buffer.from(''),
output: '', output: '',
stdout: '',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,
@ -175,8 +178,6 @@ describe('ShellTool', () => {
expect.any(String), expect.any(String),
expect.any(Function), expect.any(Function),
mockAbortSignal, mockAbortSignal,
undefined,
undefined,
); );
}); });
@ -188,13 +189,16 @@ describe('ShellTool', () => {
error, error,
exitCode: 1, exitCode: 1,
output: 'err', output: 'err',
stderr: 'err',
rawOutput: Buffer.from('err'), rawOutput: Buffer.from('err'),
stdout: '',
signal: null, signal: null,
aborted: false, aborted: false,
pid: 12345, pid: 12345,
}); });
const result = await promise; const result = await promise;
// The final llmContent should contain the user's command, not the wrapper
expect(result.llmContent).toContain('Error: wrapped command failed'); expect(result.llmContent).toContain('Error: wrapped command failed');
expect(result.llmContent).not.toContain('pgrep'); expect(result.llmContent).not.toContain('pgrep');
}); });
@ -227,6 +231,8 @@ describe('ShellTool', () => {
resolveExecutionPromise({ resolveExecutionPromise({
output: 'long output', output: 'long output',
rawOutput: Buffer.from('long output'), rawOutput: Buffer.from('long output'),
stdout: 'long output',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,
@ -251,7 +257,7 @@ describe('ShellTool', () => {
mockShellExecutionService.mockImplementation(() => { mockShellExecutionService.mockImplementation(() => {
throw error; throw error;
}); });
vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.existsSync).mockReturnValue(true); // Pretend the file exists
const invocation = shellTool.build({ command: 'a-command' }); const invocation = shellTool.build({ command: 'a-command' });
await expect(invocation.execute(mockAbortSignal)).rejects.toThrow(error); await expect(invocation.execute(mockAbortSignal)).rejects.toThrow(error);
@ -274,26 +280,33 @@ describe('ShellTool', () => {
const invocation = shellTool.build({ command: 'stream' }); const invocation = shellTool.build({ command: 'stream' });
const promise = invocation.execute(mockAbortSignal, updateOutputMock); const promise = invocation.execute(mockAbortSignal, updateOutputMock);
// First chunk, should be throttled.
mockShellOutputCallback({ mockShellOutputCallback({
type: 'data', type: 'data',
stream: 'stdout',
chunk: 'hello ', chunk: 'hello ',
}); });
expect(updateOutputMock).not.toHaveBeenCalled(); expect(updateOutputMock).not.toHaveBeenCalled();
// Advance time past the throttle interval.
await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1); await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1);
// Send a second chunk. THIS event triggers the update with the CUMULATIVE content.
mockShellOutputCallback({ mockShellOutputCallback({
type: 'data', type: 'data',
chunk: 'hello world', stream: 'stderr',
chunk: 'world',
}); });
// It should have been called once now with the combined output. // It should have been called once now with the combined output.
expect(updateOutputMock).toHaveBeenCalledOnce(); expect(updateOutputMock).toHaveBeenCalledOnce();
expect(updateOutputMock).toHaveBeenCalledWith('hello world'); expect(updateOutputMock).toHaveBeenCalledWith('hello \nworld');
resolveExecutionPromise({ resolveExecutionPromise({
rawOutput: Buffer.from(''), rawOutput: Buffer.from(''),
output: '', output: '',
stdout: '',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,
@ -319,13 +332,16 @@ describe('ShellTool', () => {
}); });
expect(updateOutputMock).toHaveBeenCalledOnce(); expect(updateOutputMock).toHaveBeenCalledOnce();
// Advance time past the throttle interval.
await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1); await vi.advanceTimersByTimeAsync(OUTPUT_UPDATE_INTERVAL_MS + 1);
// Send a SECOND progress event. This one will trigger the flush.
mockShellOutputCallback({ mockShellOutputCallback({
type: 'binary_progress', type: 'binary_progress',
bytesReceived: 2048, bytesReceived: 2048,
}); });
// Now it should be called a second time with the latest progress.
expect(updateOutputMock).toHaveBeenCalledTimes(2); expect(updateOutputMock).toHaveBeenCalledTimes(2);
expect(updateOutputMock).toHaveBeenLastCalledWith( expect(updateOutputMock).toHaveBeenLastCalledWith(
'[Receiving binary output... 2.0 KB received]', '[Receiving binary output... 2.0 KB received]',
@ -334,6 +350,8 @@ describe('ShellTool', () => {
resolveExecutionPromise({ resolveExecutionPromise({
rawOutput: Buffer.from(''), rawOutput: Buffer.from(''),
output: '', output: '',
stdout: '',
stderr: '',
exitCode: 0, exitCode: 0,
signal: null, signal: null,
error: null, error: null,

25
packages/core/src/tools/shell.ts
View File

@ -97,8 +97,6 @@ class ShellToolInvocation extends BaseToolInvocation<
async execute( async execute(
signal: AbortSignal, signal: AbortSignal,
updateOutput?: (output: string) => void, updateOutput?: (output: string) => void,
terminalColumns?: number,
terminalRows?: number,
): Promise<ToolResult> { ): Promise<ToolResult> {
const strippedCommand = stripShellWrapper(this.params.command); const strippedCommand = stripShellWrapper(this.params.command);
@ -131,7 +129,9 @@ class ShellToolInvocation extends BaseToolInvocation<
this.params.directory || '', this.params.directory || '',
); );
let cumulativeOutput = ''; let cumulativeStdout = '';
let cumulativeStderr = '';
let lastUpdateTime = Date.now(); let lastUpdateTime = Date.now();
let isBinaryStream = false; let isBinaryStream = false;
@ -148,9 +148,15 @@ class ShellToolInvocation extends BaseToolInvocation<
switch (event.type) { switch (event.type) {
case 'data': case 'data':
if (isBinaryStream) break; if (isBinaryStream) break; // Don't process text if we are in binary mode
cumulativeOutput = event.chunk; if (event.stream === 'stdout') {
currentDisplayOutput = cumulativeOutput; cumulativeStdout += event.chunk;
} else {
cumulativeStderr += event.chunk;
}
currentDisplayOutput =
cumulativeStdout +
(cumulativeStderr ? `\n${cumulativeStderr}` : '');
if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) { if (Date.now() - lastUpdateTime > OUTPUT_UPDATE_INTERVAL_MS) {
shouldUpdate = true; shouldUpdate = true;
} }
@ -181,8 +187,6 @@ class ShellToolInvocation extends BaseToolInvocation<
} }
}, },
signal, signal,
terminalColumns,
terminalRows,
); );
const result = await resultPromise; const result = await resultPromise;
@ -214,7 +218,7 @@ class ShellToolInvocation extends BaseToolInvocation<
if (result.aborted) { if (result.aborted) {
llmContent = 'Command was cancelled by user before it could complete.'; llmContent = 'Command was cancelled by user before it could complete.';
if (result.output.trim()) { if (result.output.trim()) {
llmContent += ` Below is the output before it was cancelled:\n${result.output}`; llmContent += ` Below is the output (on stdout and stderr) before it was cancelled:\n${result.output}`;
} else { } else {
llmContent += ' There was no output before it was cancelled.'; llmContent += ' There was no output before it was cancelled.';
} }
@ -228,7 +232,8 @@ class ShellToolInvocation extends BaseToolInvocation<
llmContent = [ llmContent = [
`Command: ${this.params.command}`, `Command: ${this.params.command}`,
`Directory: ${this.params.directory || '(root)'}`, `Directory: ${this.params.directory || '(root)'}`,
`Output: ${result.output || '(empty)'}`, `Stdout: ${result.stdout || '(empty)'}`,
`Stderr: ${result.stderr || '(empty)'}`,
`Error: ${finalError}`, // Use the cleaned error string. `Error: ${finalError}`, // Use the cleaned error string.
`Exit Code: ${result.exitCode ?? '(none)'}`, `Exit Code: ${result.exitCode ?? '(none)'}`,
`Signal: ${result.signal ?? '(none)'}`, `Signal: ${result.signal ?? '(none)'}`,

13
packages/core/src/tools/tools.ts
View File

@ -50,8 +50,6 @@ export interface ToolInvocation<
execute( execute(
signal: AbortSignal, signal: AbortSignal,
updateOutput?: (output: string) => void, updateOutput?: (output: string) => void,
terminalColumns?: number,
terminalRows?: number,
): Promise<TResult>; ): Promise<TResult>;
} }
@ -80,8 +78,6 @@ export abstract class BaseToolInvocation<
abstract execute( abstract execute(
signal: AbortSignal, signal: AbortSignal,
updateOutput?: (output: string) => void, updateOutput?: (output: string) => void,
terminalColumns?: number,
terminalRows?: number,
): Promise<TResult>; ): Promise<TResult>;
} }
@ -200,16 +196,9 @@ export abstract class DeclarativeTool<
params: TParams, params: TParams,
signal: AbortSignal, signal: AbortSignal,
updateOutput?: (output: string) => void, updateOutput?: (output: string) => void,
terminalColumns?: number,
terminalRows?: number,
): Promise<TResult> { ): Promise<TResult> {
const invocation = this.build(params); const invocation = this.build(params);
return invocation.execute( return invocation.execute(signal, updateOutput);
signal,
updateOutput,
terminalColumns,
terminalRows,
);
} }
} }

3
packages/vscode-ide-companion/tsconfig.json
View File

@ -6,8 +6,7 @@
"lib": ["ES2022", "dom"], "lib": ["ES2022", "dom"],
"sourceMap": true, "sourceMap": true,
"rootDir": "src", "rootDir": "src",
"strict": true /* enable all strict type-checking options */, "strict": true /* enable all strict type-checking options */
"skipLibCheck": true /* Helps avoid potential type conflicts between different third-party libraries in node_modules. */
/* Additional Checks */ /* Additional Checks */
// "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */ // "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */
// "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */ // "noFallthroughCasesInSwitch": true, /* Report errors for fallthrough cases in switch statement. */