fix(core): Improve shell tool reliability and test portability (#1036)
This commit is contained in:
parent
2b799cbbf0
commit
7352cb403c
|
@ -174,8 +174,8 @@ export class Config {
|
||||||
setGeminiMdFilename(params.contextFileName);
|
setGeminiMdFilename(params.contextFileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
this.toolRegistry = createToolRegistry(this);
|
|
||||||
this.geminiClient = new GeminiClient(this);
|
this.geminiClient = new GeminiClient(this);
|
||||||
|
this.toolRegistry = createToolRegistry(this);
|
||||||
|
|
||||||
if (this.telemetrySettings.enabled) {
|
if (this.telemetrySettings.enabled) {
|
||||||
initializeTelemetry(this);
|
initializeTelemetry(this);
|
||||||
|
|
|
@ -7,6 +7,7 @@
|
||||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||||
import { getCoreSystemPrompt } from './prompts.js'; // Adjust import path
|
import { getCoreSystemPrompt } from './prompts.js'; // Adjust import path
|
||||||
import * as process from 'node:process';
|
import * as process from 'node:process';
|
||||||
|
import * as os from 'node:os';
|
||||||
import { isGitRepository } from '../utils/gitUtils.js';
|
import { isGitRepository } from '../utils/gitUtils.js';
|
||||||
|
|
||||||
// Mock tool names if they are dynamically generated or complex
|
// Mock tool names if they are dynamically generated or complex
|
||||||
|
@ -24,6 +25,9 @@ vi.mock('../tools/shell', () => ({
|
||||||
vi.mock('../tools/write-file', () => ({
|
vi.mock('../tools/write-file', () => ({
|
||||||
WriteFileTool: { Name: 'write_file' },
|
WriteFileTool: { Name: 'write_file' },
|
||||||
}));
|
}));
|
||||||
|
vi.mock('node:os', () => ({
|
||||||
|
platform: vi.fn(),
|
||||||
|
}));
|
||||||
vi.mock('../utils/gitUtils', () => ({
|
vi.mock('../utils/gitUtils', () => ({
|
||||||
isGitRepository: vi.fn(),
|
isGitRepository: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
@ -35,6 +39,7 @@ describe('Core System Prompt (prompts.ts)', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
// Store original value before each test
|
// Store original value before each test
|
||||||
originalSandboxEnv = process.env.SANDBOX;
|
originalSandboxEnv = process.env.SANDBOX;
|
||||||
|
vi.mocked(os.platform).mockReturnValue('darwin');
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
|
|
@ -170,17 +170,16 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
|
||||||
}
|
}
|
||||||
|
|
||||||
const isWindows = os.platform() === 'win32';
|
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
|
// pgrep is not available on Windows, so we can't get background PIDs
|
||||||
const command = isWindows
|
const command = isWindows
|
||||||
? params.command
|
? params.command
|
||||||
: (() => {
|
: (() => {
|
||||||
// wrap command to append subprocess pids (via pgrep) to temporary file
|
// 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();
|
let command = params.command.trim();
|
||||||
if (!command.endsWith('&')) command += ';';
|
if (!command.endsWith('&')) command += ';';
|
||||||
return `{ ${command} }; __code=$?; pgrep -g 0 >${tempFilePath} 2>&1; exit $__code;`;
|
return `{ ${command} }; __code=$?; pgrep -g 0 >${tempFilePath} 2>&1; exit $__code;`;
|
||||||
|
@ -293,10 +292,6 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> {
|
||||||
// parse pids (pgrep output) from temporary file and remove it
|
// parse pids (pgrep output) from temporary file and remove it
|
||||||
const backgroundPIDs: number[] = [];
|
const backgroundPIDs: number[] = [];
|
||||||
if (os.platform() !== 'win32') {
|
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)) {
|
if (fs.existsSync(tempFilePath)) {
|
||||||
const pgrepLines = fs
|
const pgrepLines = fs
|
||||||
.readFileSync(tempFilePath, 'utf8')
|
.readFileSync(tempFilePath, 'utf8')
|
||||||
|
|
Loading…
Reference in New Issue