feat: Implement background shell commands (#14849)

This commit is contained in:
Gal Zahavi
2026-01-30 09:53:09 -08:00
committed by GitHub
parent fc90f581b2
commit 2eb8dc3042
52 changed files with 3957 additions and 470 deletions
+80 -4
View File
@@ -18,8 +18,13 @@ import {
const mockPlatform = vi.hoisted(() => vi.fn());
const mockShellExecutionService = vi.hoisted(() => vi.fn());
const mockShellBackground = vi.hoisted(() => vi.fn());
vi.mock('../services/shellExecutionService.js', () => ({
ShellExecutionService: { execute: mockShellExecutionService },
ShellExecutionService: {
execute: mockShellExecutionService,
background: mockShellBackground,
},
}));
vi.mock('node:os', async (importOriginal) => {
@@ -38,6 +43,7 @@ vi.mock('../utils/summarizer.js');
import { initializeShellParsers } from '../utils/shell-utils.js';
import { ShellTool } from './shell.js';
import { debugLogger } from '../index.js';
import { type Config } from '../config/config.js';
import {
type ShellExecutionResult,
@@ -168,6 +174,20 @@ describe('ShellTool', () => {
}),
};
});
mockShellBackground.mockImplementation(() => {
resolveExecutionPromise({
output: '',
rawOutput: Buffer.from(''),
exitCode: null,
signal: null,
error: null,
aborted: false,
pid: 12345,
executionMethod: 'child_process',
backgrounded: true,
});
});
});
afterEach(() => {
@@ -305,6 +325,25 @@ describe('ShellTool', () => {
);
});
it('should handle is_background parameter by calling ShellExecutionService.background', async () => {
vi.useFakeTimers();
const invocation = shellTool.build({
command: 'sleep 10',
is_background: true,
});
const promise = invocation.execute(mockAbortSignal);
// We need to provide a PID for the background logic to trigger
resolveShellExecution({ pid: 12345 });
// Advance time to trigger the background timeout
await vi.advanceTimersByTimeAsync(250);
expect(mockShellBackground).toHaveBeenCalledWith(12345);
await promise;
});
itWindowsOnly(
'should not wrap command on windows',
async () => {
@@ -430,8 +469,6 @@ describe('ShellTool', () => {
// 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 () => {
@@ -450,10 +487,28 @@ describe('ShellTool', () => {
expect(fs.existsSync(tmpFile)).toBe(false);
});
it('should not log "missing pgrep output" when process is backgrounded', async () => {
vi.useFakeTimers();
const debugErrorSpy = vi.spyOn(debugLogger, 'error');
const invocation = shellTool.build({
command: 'sleep 10',
is_background: true,
});
const promise = invocation.execute(mockAbortSignal);
// Advance time to trigger backgrounding
await vi.advanceTimersByTimeAsync(200);
await promise;
expect(debugErrorSpy).not.toHaveBeenCalledWith('missing pgrep output');
});
describe('Streaming to `updateOutput`', () => {
let updateOutputMock: Mock;
beforeEach(() => {
vi.useFakeTimers({ toFake: ['Date'] });
vi.useFakeTimers({ toFake: ['Date', 'setTimeout', 'clearTimeout'] });
updateOutputMock = vi.fn();
});
afterEach(() => {
@@ -503,6 +558,27 @@ describe('ShellTool', () => {
});
await promise;
});
it('should NOT call updateOutput if the command is backgrounded', async () => {
const invocation = shellTool.build({
command: 'sleep 10',
is_background: true,
});
const promise = invocation.execute(mockAbortSignal, updateOutputMock);
mockShellOutputCallback({ type: 'data', chunk: 'some output' });
expect(updateOutputMock).not.toHaveBeenCalled();
// We need to provide a PID for the background logic to trigger
resolveShellExecution({ pid: 12345 });
// Advance time to trigger the background timeout
await vi.advanceTimersByTimeAsync(250);
expect(mockShellBackground).toHaveBeenCalledWith(12345);
await promise;
});
});
});
+50 -9
View File
@@ -46,10 +46,14 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
export const OUTPUT_UPDATE_INTERVAL_MS = 1000;
// Delay so user does not see the output of the process before the process is moved to the background.
const BACKGROUND_DELAY_MS = 200;
export interface ShellToolParams {
command: string;
description?: string;
dir_path?: string;
is_background?: boolean;
}
export class ShellToolInvocation extends BaseToolInvocation<
@@ -79,6 +83,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
if (this.params.description) {
description += ` (${this.params.description.replace(/\n/g, ' ')})`;
}
if (this.params.is_background) {
description += ' [background]';
}
return description;
}
@@ -249,12 +256,14 @@ export class ShellToolInvocation extends BaseToolInvocation<
shouldUpdate = true;
}
break;
case 'exit':
break;
default: {
throw new Error('An unhandled ShellOutputEvent was found.');
}
}
if (shouldUpdate) {
if (shouldUpdate && !this.params.is_background) {
updateOutput(cumulativeOutput);
lastUpdateTime = Date.now();
}
@@ -270,8 +279,17 @@ export class ShellToolInvocation extends BaseToolInvocation<
},
);
if (pid && setPidCallback) {
setPidCallback(pid);
if (pid) {
if (setPidCallback) {
setPidCallback(pid);
}
// If the model requested to run in the background, do so after a short delay.
if (this.params.is_background) {
setTimeout(() => {
ShellExecutionService.background(pid);
}, BACKGROUND_DELAY_MS);
}
}
const result = await resultPromise;
@@ -299,12 +317,14 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
}
} else {
if (!signal.aborted) {
if (!signal.aborted && !result.backgrounded) {
debugLogger.error('missing pgrep output');
}
}
}
let data: Record<string, unknown> | undefined;
let llmContent = '';
let timeoutMessage = '';
if (result.aborted) {
@@ -322,6 +342,13 @@ export class ShellToolInvocation extends BaseToolInvocation<
} else {
llmContent += ' There was no output before it was cancelled.';
}
} else if (this.params.is_background || result.backgrounded) {
llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`;
data = {
pid: result.pid,
command: this.params.command,
initialOutput: result.output,
};
} else {
// Create a formatted error string for display, replacing the wrapper command
// with the user-facing command.
@@ -356,7 +383,9 @@ export class ShellToolInvocation extends BaseToolInvocation<
if (this.config.getDebugMode()) {
returnDisplayMessage = llmContent;
} else {
if (result.output.trim()) {
if (this.params.is_background || result.backgrounded) {
returnDisplayMessage = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`;
} else if (result.output.trim()) {
returnDisplayMessage = result.output;
} else {
if (result.aborted) {
@@ -406,6 +435,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
return {
llmContent,
returnDisplay: returnDisplayMessage,
data,
...executionError,
};
} finally {
@@ -421,7 +451,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
}
function getShellToolDescription(): string {
function getShellToolDescription(enableInteractiveShell: boolean): string {
const returnedInfo = `
The following information is returned:
@@ -434,9 +464,15 @@ function getShellToolDescription(): string {
Process Group PGID: Only included if available.`;
if (os.platform() === 'win32') {
return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command <command>\`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`.${returnedInfo}`;
const backgroundInstructions = enableInteractiveShell
? 'To run a command in the background, set the `is_background` parameter to true. Do NOT use PowerShell background constructs.'
: 'Command can start background processes using PowerShell constructs such as `Start-Process -NoNewWindow` or `Start-Job`.';
return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command <command>\`. ${backgroundInstructions}${returnedInfo}`;
} else {
return `This tool executes a given shell command as \`bash -c <command>\`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${returnedInfo}`;
const backgroundInstructions = enableInteractiveShell
? 'To run a command in the background, set the `is_background` parameter to true. Do NOT use `&` to background commands.'
: 'Command can start background processes using `&`.';
return `This tool executes a given shell command as \`bash -c <command>\`. ${backgroundInstructions} Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${returnedInfo}`;
}
}
@@ -464,7 +500,7 @@ export class ShellTool extends BaseDeclarativeTool<
super(
ShellTool.Name,
'Shell',
getShellToolDescription(),
getShellToolDescription(config.getEnableInteractiveShell()),
Kind.Execute,
{
type: 'object',
@@ -483,6 +519,11 @@ export class ShellTool extends BaseDeclarativeTool<
description:
'(OPTIONAL) The path of the directory to run the command in. If not provided, the project root directory is used. Must be a directory within the workspace and must already exist.',
},
is_background: {
type: 'boolean',
description:
'Set to true if this command should be run in the background (e.g. for long-running servers or watchers). The command will be started, allowed to run for a brief moment to check for immediate errors, and then moved to the background.',
},
},
required: ['command'],
},
+5
View File
@@ -550,6 +550,11 @@ export interface ToolResult {
message: string; // raw error message
type?: ToolErrorType; // An optional machine-readable error type (e.g., 'FILE_NOT_FOUND').
};
/**
* Optional data payload for passing structured information back to the caller.
*/
data?: Record<string, unknown>;
}
/**