mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-23 11:34:44 -07:00
chore: finish truncation and stream logging logic
This commit is contained in:
@@ -27,6 +27,7 @@ export interface ExecutionResult {
|
||||
pid: number | undefined;
|
||||
executionMethod: ExecutionMethod;
|
||||
backgrounded?: boolean;
|
||||
fullOutputFilePath?: string;
|
||||
}
|
||||
|
||||
export interface ExecutionHandle {
|
||||
|
||||
@@ -33,7 +33,16 @@ const mockIsBinary = vi.hoisted(() => vi.fn());
|
||||
const mockPlatform = vi.hoisted(() => vi.fn());
|
||||
const mockHomedir = vi.hoisted(() => vi.fn());
|
||||
const mockMkdirSync = vi.hoisted(() => vi.fn());
|
||||
const mockCreateWriteStream = vi.hoisted(() => vi.fn());
|
||||
const mockCreateWriteStream = vi.hoisted(() =>
|
||||
vi.fn().mockReturnValue({
|
||||
write: vi.fn(),
|
||||
end: vi.fn().mockImplementation((cb?: () => void) => {
|
||||
if (cb) cb();
|
||||
}),
|
||||
destroy: vi.fn(),
|
||||
closed: false,
|
||||
}),
|
||||
);
|
||||
const mockGetPty = vi.hoisted(() => vi.fn());
|
||||
const mockSerializeTerminalToObject = vi.hoisted(() => vi.fn());
|
||||
const mockResolveExecutable = vi.hoisted(() => vi.fn());
|
||||
@@ -92,6 +101,7 @@ vi.mock('node:os', () => ({
|
||||
default: {
|
||||
platform: mockPlatform,
|
||||
homedir: mockHomedir,
|
||||
tmpdir: () => '/tmp',
|
||||
constants: {
|
||||
signals: {
|
||||
SIGTERM: 15,
|
||||
@@ -208,6 +218,15 @@ describe('ShellExecutionService', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockCreateWriteStream.mockReturnValue({
|
||||
write: vi.fn(),
|
||||
end: vi.fn().mockImplementation((cb?: () => void) => {
|
||||
if (cb) cb();
|
||||
}),
|
||||
destroy: vi.fn(),
|
||||
closed: false,
|
||||
on: vi.fn(),
|
||||
});
|
||||
ExecutionLifecycleService.resetForTest();
|
||||
ShellExecutionService.resetForTest();
|
||||
mockSerializeTerminalToObject.mockReturnValue([]);
|
||||
@@ -621,7 +640,7 @@ describe('ShellExecutionService', () => {
|
||||
});
|
||||
|
||||
it('should handle a synchronous spawn error', async () => {
|
||||
mockGetPty.mockImplementation(() => null);
|
||||
mockGetPty.mockResolvedValue(null);
|
||||
|
||||
mockCpSpawn.mockImplementation(() => {
|
||||
throw new Error('Simulated PTY spawn error');
|
||||
@@ -725,7 +744,13 @@ describe('ShellExecutionService', () => {
|
||||
});
|
||||
|
||||
describe('Backgrounding', () => {
|
||||
let mockWriteStream: { write: Mock; end: Mock; on: Mock };
|
||||
let mockWriteStream: {
|
||||
write: Mock;
|
||||
end: Mock;
|
||||
on: Mock;
|
||||
destroy: Mock;
|
||||
closed: boolean;
|
||||
};
|
||||
let mockBgChildProcess: EventEmitter & Partial<ChildProcess>;
|
||||
|
||||
beforeEach(async () => {
|
||||
@@ -733,6 +758,8 @@ describe('ShellExecutionService', () => {
|
||||
write: vi.fn(),
|
||||
end: vi.fn().mockImplementation((cb) => cb?.()),
|
||||
on: vi.fn(),
|
||||
destroy: vi.fn(),
|
||||
closed: false,
|
||||
};
|
||||
|
||||
mockMkdirSync.mockReturnValue(undefined);
|
||||
|
||||
@@ -12,6 +12,7 @@ import type { Writable } from 'node:stream';
|
||||
import os from 'node:os';
|
||||
import fs, { mkdirSync } from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import crypto from 'node:crypto';
|
||||
import type { IPty } from '@lydell/node-pty';
|
||||
import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js';
|
||||
import {
|
||||
@@ -370,32 +371,114 @@ export class ShellExecutionService {
|
||||
shouldUseNodePty: boolean,
|
||||
shellExecutionConfig: ShellExecutionConfig,
|
||||
): Promise<ShellExecutionHandle> {
|
||||
const outputFileName = `gemini_shell_output_${crypto.randomBytes(6).toString('hex')}.log`;
|
||||
const outputFilePath = path.join(os.tmpdir(), outputFileName);
|
||||
const outputStream = fs.createWriteStream(outputFilePath);
|
||||
|
||||
let isBinaryStream = false;
|
||||
let totalBytesWritten = 0;
|
||||
|
||||
const interceptedOnOutputEvent = (event: ShellOutputEvent) => {
|
||||
switch (event.type) {
|
||||
case 'raw_data':
|
||||
break;
|
||||
case 'file_data':
|
||||
if (!isBinaryStream) {
|
||||
outputStream.write(event.chunk);
|
||||
totalBytesWritten += Buffer.byteLength(event.chunk);
|
||||
}
|
||||
break;
|
||||
case 'binary_detected':
|
||||
case 'binary_progress':
|
||||
isBinaryStream = true;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
onOutputEvent(event);
|
||||
};
|
||||
|
||||
let handlePromise: Promise<ShellExecutionHandle>;
|
||||
|
||||
if (shouldUseNodePty) {
|
||||
const ptyInfo = await getPty();
|
||||
if (ptyInfo) {
|
||||
try {
|
||||
return await this.executeWithPty(
|
||||
handlePromise = getPty().then((ptyInfo) => {
|
||||
if (ptyInfo) {
|
||||
return this.executeWithPty(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
onOutputEvent,
|
||||
interceptedOnOutputEvent,
|
||||
abortSignal,
|
||||
shellExecutionConfig,
|
||||
ptyInfo,
|
||||
).catch(() =>
|
||||
this.childProcessFallback(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
interceptedOnOutputEvent,
|
||||
abortSignal,
|
||||
shellExecutionConfig,
|
||||
shouldUseNodePty,
|
||||
),
|
||||
);
|
||||
} catch {
|
||||
// Fallback to child_process
|
||||
}
|
||||
}
|
||||
return this.childProcessFallback(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
interceptedOnOutputEvent,
|
||||
abortSignal,
|
||||
shellExecutionConfig,
|
||||
shouldUseNodePty,
|
||||
);
|
||||
});
|
||||
} else {
|
||||
handlePromise = this.childProcessFallback(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
interceptedOnOutputEvent,
|
||||
abortSignal,
|
||||
shellExecutionConfig,
|
||||
shouldUseNodePty,
|
||||
);
|
||||
}
|
||||
|
||||
return this.childProcessFallback(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
onOutputEvent,
|
||||
abortSignal,
|
||||
shellExecutionConfig,
|
||||
shouldUseNodePty,
|
||||
);
|
||||
const handle = await handlePromise;
|
||||
|
||||
const wrappedResultPromise = handle.result
|
||||
.then(async (result) => {
|
||||
await new Promise<void>((resolve) => {
|
||||
outputStream.end(resolve);
|
||||
});
|
||||
// The threshold logic is handled later by ToolExecutor/caller, so we just return the full file path if anything was written
|
||||
if (
|
||||
totalBytesWritten > 0 &&
|
||||
!result.backgrounded &&
|
||||
!abortSignal.aborted &&
|
||||
!result.error
|
||||
) {
|
||||
return {
|
||||
...result,
|
||||
fullOutputFilePath: outputFilePath,
|
||||
};
|
||||
} else {
|
||||
if (!outputStream.closed) {
|
||||
outputStream.destroy();
|
||||
}
|
||||
await fs.promises.unlink(outputFilePath).catch(() => undefined);
|
||||
return result;
|
||||
}
|
||||
})
|
||||
.catch(async (err) => {
|
||||
if (!outputStream.closed) {
|
||||
outputStream.destroy();
|
||||
}
|
||||
await fs.promises.unlink(outputFilePath).catch(() => undefined);
|
||||
throw err;
|
||||
});
|
||||
|
||||
return {
|
||||
pid: handle.pid,
|
||||
result: wrappedResultPromise,
|
||||
};
|
||||
}
|
||||
|
||||
private static appendAndTruncate(
|
||||
|
||||
Reference in New Issue
Block a user