mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-02-01 22:48:03 +00:00
feat(core): Add configurable inactivity timeout for shell commands (#13531)
This commit is contained in:
@@ -600,6 +600,11 @@ their corresponding top-level category object in your `settings.json` file.
|
||||
- **Description:** Show color in shell output.
|
||||
- **Default:** `false`
|
||||
|
||||
- **`tools.shell.inactivityTimeout`** (number):
|
||||
- **Description:** The maximum time in seconds allowed without output from the
|
||||
shell command. Defaults to 5 minutes.
|
||||
- **Default:** `300`
|
||||
|
||||
- **`tools.autoAccept`** (boolean):
|
||||
- **Description:** Automatically accept and execute tool calls that are
|
||||
considered safe (e.g., read-only operations).
|
||||
|
||||
@@ -643,6 +643,7 @@ export async function loadCliConfig(
|
||||
useRipgrep: settings.tools?.useRipgrep,
|
||||
enableInteractiveShell:
|
||||
settings.tools?.shell?.enableInteractiveShell ?? true,
|
||||
shellToolInactivityTimeout: settings.tools?.shell?.inactivityTimeout,
|
||||
skipNextSpeakerCheck: settings.model?.skipNextSpeakerCheck,
|
||||
enablePromptCompletion: settings.general?.enablePromptCompletion ?? false,
|
||||
truncateToolOutputThreshold: settings.tools?.truncateToolOutputThreshold,
|
||||
|
||||
@@ -116,6 +116,7 @@ const MIGRATION_MAP: Record<string, string> = {
|
||||
enableInteractiveShell: 'tools.shell.enableInteractiveShell',
|
||||
shellPager: 'tools.shell.pager',
|
||||
shellShowColor: 'tools.shell.showColor',
|
||||
shellInactivityTimeout: 'tools.shell.inactivityTimeout',
|
||||
skipNextSpeakerCheck: 'model.skipNextSpeakerCheck',
|
||||
summarizeToolOutput: 'model.summarizeToolOutput',
|
||||
telemetry: 'telemetry',
|
||||
|
||||
@@ -930,6 +930,16 @@ const SETTINGS_SCHEMA = {
|
||||
description: 'Show color in shell output.',
|
||||
showInDialog: true,
|
||||
},
|
||||
inactivityTimeout: {
|
||||
type: 'number',
|
||||
label: 'Inactivity Timeout',
|
||||
category: 'Tools',
|
||||
requiresRestart: false,
|
||||
default: 300,
|
||||
description:
|
||||
'The maximum time in seconds allowed without output from the shell command. Defaults to 5 minutes.',
|
||||
showInDialog: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
autoAccept: {
|
||||
|
||||
@@ -750,6 +750,22 @@ describe('Server Config (config.ts)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Shell Tool Inactivity Timeout', () => {
|
||||
it('should default to 300000ms (300 seconds) when not provided', () => {
|
||||
const config = new Config(baseParams);
|
||||
expect(config.getShellToolInactivityTimeout()).toBe(300000);
|
||||
});
|
||||
|
||||
it('should convert provided seconds to milliseconds', () => {
|
||||
const params: ConfigParameters = {
|
||||
...baseParams,
|
||||
shellToolInactivityTimeout: 10, // 10 seconds
|
||||
};
|
||||
const config = new Config(params);
|
||||
expect(config.getShellToolInactivityTimeout()).toBe(10000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ContinueOnFailedApiCall Configuration', () => {
|
||||
it('should default continueOnFailedApiCall to false when not provided', () => {
|
||||
const config = new Config(baseParams);
|
||||
|
||||
@@ -297,6 +297,7 @@ export interface ConfigParameters {
|
||||
continueOnFailedApiCall?: boolean;
|
||||
retryFetchErrors?: boolean;
|
||||
enableShellOutputEfficiency?: boolean;
|
||||
shellToolInactivityTimeout?: number;
|
||||
fakeResponses?: string;
|
||||
recordResponses?: string;
|
||||
ptyInfo?: string;
|
||||
@@ -411,6 +412,7 @@ export class Config {
|
||||
private readonly continueOnFailedApiCall: boolean;
|
||||
private readonly retryFetchErrors: boolean;
|
||||
private readonly enableShellOutputEfficiency: boolean;
|
||||
private readonly shellToolInactivityTimeout: number;
|
||||
readonly fakeResponses?: string;
|
||||
readonly recordResponses?: string;
|
||||
private readonly disableYoloMode: boolean;
|
||||
@@ -547,6 +549,8 @@ export class Config {
|
||||
this.continueOnFailedApiCall = params.continueOnFailedApiCall ?? true;
|
||||
this.enableShellOutputEfficiency =
|
||||
params.enableShellOutputEfficiency ?? true;
|
||||
this.shellToolInactivityTimeout =
|
||||
(params.shellToolInactivityTimeout ?? 300) * 1000; // 5 minutes
|
||||
this.extensionManagement = params.extensionManagement ?? true;
|
||||
this.enableExtensionReloading = params.enableExtensionReloading ?? false;
|
||||
this.storage = new Storage(this.targetDir);
|
||||
@@ -1308,6 +1312,10 @@ export class Config {
|
||||
return this.enableShellOutputEfficiency;
|
||||
}
|
||||
|
||||
getShellToolInactivityTimeout(): number {
|
||||
return this.shellToolInactivityTimeout;
|
||||
}
|
||||
|
||||
getShellExecutionConfig(): ShellExecutionConfig {
|
||||
return this.shellExecutionConfig;
|
||||
}
|
||||
|
||||
@@ -92,6 +92,7 @@ describe('ShellTool', () => {
|
||||
getGeminiClient: vi.fn(),
|
||||
getEnableInteractiveShell: vi.fn().mockReturnValue(false),
|
||||
isInteractive: vi.fn().mockReturnValue(true),
|
||||
getShellToolInactivityTimeout: vi.fn().mockReturnValue(300000),
|
||||
} as unknown as Config;
|
||||
|
||||
shellTool = new ShellTool(mockConfig);
|
||||
@@ -219,7 +220,7 @@ describe('ShellTool', () => {
|
||||
wrappedCommand,
|
||||
tempRootDir,
|
||||
expect.any(Function),
|
||||
mockAbortSignal,
|
||||
expect.any(AbortSignal),
|
||||
false,
|
||||
{},
|
||||
);
|
||||
@@ -244,7 +245,7 @@ describe('ShellTool', () => {
|
||||
wrappedCommand,
|
||||
subdir,
|
||||
expect.any(Function),
|
||||
mockAbortSignal,
|
||||
expect.any(AbortSignal),
|
||||
false,
|
||||
{},
|
||||
);
|
||||
@@ -265,7 +266,7 @@ describe('ShellTool', () => {
|
||||
wrappedCommand,
|
||||
path.join(tempRootDir, 'subdir'),
|
||||
expect.any(Function),
|
||||
mockAbortSignal,
|
||||
expect.any(AbortSignal),
|
||||
false,
|
||||
{},
|
||||
);
|
||||
@@ -292,7 +293,7 @@ describe('ShellTool', () => {
|
||||
'dir',
|
||||
tempRootDir,
|
||||
expect.any(Function),
|
||||
mockAbortSignal,
|
||||
expect.any(AbortSignal),
|
||||
false,
|
||||
{},
|
||||
);
|
||||
@@ -376,6 +377,30 @@ describe('ShellTool', () => {
|
||||
expect(result.returnDisplay).toBe('long output');
|
||||
});
|
||||
|
||||
it('should NOT start a timeout if timeoutMs is <= 0', async () => {
|
||||
// Mock the timeout config to be 0
|
||||
(mockConfig.getShellToolInactivityTimeout as Mock).mockReturnValue(0);
|
||||
|
||||
vi.useFakeTimers();
|
||||
|
||||
const invocation = shellTool.build({ command: 'sleep 10' });
|
||||
const promise = invocation.execute(mockAbortSignal);
|
||||
|
||||
// Verify no timeout logic is triggered even after a long time
|
||||
resolveShellExecution({
|
||||
output: 'finished',
|
||||
exitCode: 0,
|
||||
});
|
||||
|
||||
await promise;
|
||||
// If we got here without aborting/timing out logic interfering, we're good.
|
||||
// We can also verify that setTimeout was NOT called for the inactivity timeout.
|
||||
// However, since we don't have direct access to the internal `resetTimeout`,
|
||||
// we can infer success by the fact it didn't abort.
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('should clean up the temp file on synchronous execution error', async () => {
|
||||
const error = new Error('sync spawn error');
|
||||
mockShellExecutionService.mockImplementation(() => {
|
||||
|
||||
@@ -150,6 +150,15 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
.toString('hex')}.tmp`;
|
||||
const tempFilePath = path.join(os.tmpdir(), tempFileName);
|
||||
|
||||
const timeoutMs = this.config.getShellToolInactivityTimeout();
|
||||
const timeoutController = new AbortController();
|
||||
let timeoutTimer: NodeJS.Timeout | undefined;
|
||||
|
||||
// Handle signal combination manually to avoid TS issues or runtime missing features
|
||||
const combinedController = new AbortController();
|
||||
|
||||
const onAbort = () => combinedController.abort();
|
||||
|
||||
try {
|
||||
// pgrep is not available on Windows, so we can't get background PIDs
|
||||
const commandToExecute = isWindows
|
||||
@@ -169,11 +178,30 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
let lastUpdateTime = Date.now();
|
||||
let isBinaryStream = false;
|
||||
|
||||
const resetTimeout = () => {
|
||||
if (timeoutMs <= 0) {
|
||||
return;
|
||||
}
|
||||
if (timeoutTimer) clearTimeout(timeoutTimer);
|
||||
timeoutTimer = setTimeout(() => {
|
||||
timeoutController.abort();
|
||||
}, timeoutMs);
|
||||
};
|
||||
|
||||
signal.addEventListener('abort', onAbort, { once: true });
|
||||
timeoutController.signal.addEventListener('abort', onAbort, {
|
||||
once: true,
|
||||
});
|
||||
|
||||
// Start timeout
|
||||
resetTimeout();
|
||||
|
||||
const { result: resultPromise, pid } =
|
||||
await ShellExecutionService.execute(
|
||||
commandToExecute,
|
||||
cwd,
|
||||
(event: ShellOutputEvent) => {
|
||||
resetTimeout(); // Reset timeout on any event
|
||||
if (!updateOutput) {
|
||||
return;
|
||||
}
|
||||
@@ -211,7 +239,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
lastUpdateTime = Date.now();
|
||||
}
|
||||
},
|
||||
signal,
|
||||
combinedController.signal,
|
||||
this.config.getEnableInteractiveShell(),
|
||||
shellExecutionConfig ?? {},
|
||||
);
|
||||
@@ -246,8 +274,17 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
}
|
||||
|
||||
let llmContent = '';
|
||||
let timeoutMessage = '';
|
||||
if (result.aborted) {
|
||||
llmContent = 'Command was cancelled by user before it could complete.';
|
||||
if (timeoutController.signal.aborted) {
|
||||
timeoutMessage = `Command was automatically cancelled because it exceeded the timeout of ${(
|
||||
timeoutMs / 60000
|
||||
).toFixed(1)} minutes without output.`;
|
||||
llmContent = timeoutMessage;
|
||||
} else {
|
||||
llmContent =
|
||||
'Command was cancelled by user before it could complete.';
|
||||
}
|
||||
if (result.output.trim()) {
|
||||
llmContent += ` Below is the output before it was cancelled:\n${result.output}`;
|
||||
} else {
|
||||
@@ -282,7 +319,11 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
returnDisplayMessage = result.output;
|
||||
} else {
|
||||
if (result.aborted) {
|
||||
returnDisplayMessage = 'Command cancelled by user.';
|
||||
if (timeoutMessage) {
|
||||
returnDisplayMessage = timeoutMessage;
|
||||
} else {
|
||||
returnDisplayMessage = 'Command cancelled by user.';
|
||||
}
|
||||
} else if (result.signal) {
|
||||
returnDisplayMessage = `Command terminated by signal: ${result.signal}`;
|
||||
} else if (result.error) {
|
||||
@@ -327,6 +368,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
|
||||
...executionError,
|
||||
};
|
||||
} finally {
|
||||
if (timeoutTimer) clearTimeout(timeoutTimer);
|
||||
signal.removeEventListener('abort', onAbort);
|
||||
timeoutController.signal.removeEventListener('abort', onAbort);
|
||||
if (fs.existsSync(tempFilePath)) {
|
||||
fs.unlinkSync(tempFilePath);
|
||||
}
|
||||
|
||||
@@ -993,6 +993,13 @@
|
||||
"markdownDescription": "Show color in shell output.\n\n- Category: `Tools`\n- Requires restart: `no`\n- Default: `false`",
|
||||
"default": false,
|
||||
"type": "boolean"
|
||||
},
|
||||
"inactivityTimeout": {
|
||||
"title": "Inactivity Timeout",
|
||||
"description": "The maximum time in seconds allowed without output from the shell command. Defaults to 5 minutes.",
|
||||
"markdownDescription": "The maximum time in seconds allowed without output from the shell command. Defaults to 5 minutes.\n\n- Category: `Tools`\n- Requires restart: `no`\n- Default: `300`",
|
||||
"default": 300,
|
||||
"type": "number"
|
||||
}
|
||||
},
|
||||
"additionalProperties": false
|
||||
|
||||
Reference in New Issue
Block a user