From 7352cb403c3ba0d70863f4daefefe0fddb56200e Mon Sep 17 00:00:00 2001 From: Sijie Wang <3463757+sijieamoy@users.noreply.github.com> Date: Sun, 15 Jun 2025 02:19:19 -0700 Subject: [PATCH] fix(core): Improve shell tool reliability and test portability (#1036) --- packages/core/src/config/config.ts | 2 +- packages/core/src/core/prompts.test.ts | 5 +++++ packages/core/src/tools/shell.ts | 13 ++++--------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 891b6302..c4f46f5e 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -174,8 +174,8 @@ export class Config { setGeminiMdFilename(params.contextFileName); } - this.toolRegistry = createToolRegistry(this); this.geminiClient = new GeminiClient(this); + this.toolRegistry = createToolRegistry(this); if (this.telemetrySettings.enabled) { initializeTelemetry(this); diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 23735525..cb22e8a1 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { getCoreSystemPrompt } from './prompts.js'; // Adjust import path import * as process from 'node:process'; +import * as os from 'node:os'; import { isGitRepository } from '../utils/gitUtils.js'; // Mock tool names if they are dynamically generated or complex @@ -24,6 +25,9 @@ vi.mock('../tools/shell', () => ({ vi.mock('../tools/write-file', () => ({ WriteFileTool: { Name: 'write_file' }, })); +vi.mock('node:os', () => ({ + platform: vi.fn(), +})); vi.mock('../utils/gitUtils', () => ({ isGitRepository: vi.fn(), })); @@ -35,6 +39,7 @@ describe('Core System Prompt (prompts.ts)', () => { beforeEach(() => { // Store original value before each test originalSandboxEnv = process.env.SANDBOX; + vi.mocked(os.platform).mockReturnValue('darwin'); }); afterEach(() => { diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 4bae498b..62dd8429 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -170,17 +170,16 @@ export class ShellTool extends BaseTool { } const isWindows = os.platform() === 'win32'; + const tempFileName = `shell_pgrep_${crypto + .randomBytes(6) + .toString('hex')}.tmp`; + const tempFilePath = path.join(os.tmpdir(), tempFileName); // pgrep is not available on Windows, so we can't get background PIDs const command = isWindows ? params.command : (() => { // wrap command to append subprocess pids (via pgrep) to temporary file - const tempFileName = `shell_pgrep_${crypto - .randomBytes(6) - .toString('hex')}.tmp`; - const tempFilePath = path.join(os.tmpdir(), tempFileName); - let command = params.command.trim(); if (!command.endsWith('&')) command += ';'; return `{ ${command} }; __code=$?; pgrep -g 0 >${tempFilePath} 2>&1; exit $__code;`; @@ -293,10 +292,6 @@ export class ShellTool extends BaseTool { // parse pids (pgrep output) from temporary file and remove it const backgroundPIDs: number[] = []; if (os.platform() !== 'win32') { - const tempFileName = `shell_pgrep_${crypto - .randomBytes(6) - .toString('hex')}.tmp`; - const tempFilePath = path.join(os.tmpdir(), tempFileName); if (fs.existsSync(tempFilePath)) { const pgrepLines = fs .readFileSync(tempFilePath, 'utf8')