Skip to content

fix: add default timeout for terminal command tool execution #10550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amabito wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: add default timeout for terminal command tool execution #10550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 186 additions & 0 deletions core/tools/implementations/runTerminalCommand.timeout.vitest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
import { EventEmitter } from "node:events";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { IDE, ToolExtras } from "../..";
import { runTerminalCommandImpl } from "./runTerminalCommand";

// Hoist mock function so it can be referenced in vi.mock factory
const { mockSpawn } = vi.hoisted(() => ({
mockSpawn: vi.fn(),
}));

vi.mock("node:child_process", () => ({
default: {
spawn: mockSpawn,
},
spawn: mockSpawn,
}));

describe("runTerminalCommand timeout functionality", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let mockChildProc: any;
let mockGetIdeInfo: ReturnType<typeof vi.fn>;
let mockGetWorkspaceDirs: ReturnType<typeof vi.fn>;
let mockOnPartialOutput: ReturnType<typeof vi.fn>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let setTimeoutSpy: any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let clearTimeoutSpy: any;

beforeEach(() => {
vi.resetAllMocks();
vi.clearAllTimers();
vi.useFakeTimers();

// Spy on setTimeout and clearTimeout
setTimeoutSpy = vi.spyOn(global, "setTimeout");
clearTimeoutSpy = vi.spyOn(global, "clearTimeout");

// Create mock child process with EventEmitter behavior
mockChildProc = new EventEmitter();
mockChildProc.stdout = new EventEmitter();
mockChildProc.stderr = new EventEmitter();
mockChildProc.killed = false;
mockChildProc.kill = vi.fn((signal?: NodeJS.Signals) => {
mockChildProc.killed = true;
setTimeout(() => {
mockChildProc.emit("close", signal === "SIGKILL" ? 137 : 143);
}, 100);
return true;
});

mockSpawn.mockReturnValue(mockChildProc);

mockGetIdeInfo = vi.fn().mockResolvedValue({ remoteName: "local" });
mockGetWorkspaceDirs = vi
.fn()
.mockResolvedValue(["file:///tmp/test-workspace"]);
mockOnPartialOutput = vi.fn();
});

afterEach(() => {
vi.restoreAllMocks();
vi.useRealTimers();
});

const createMockExtras = (
overrides: Partial<ToolExtras> = {},
): ToolExtras => {
const mockIde = {
getIdeInfo: mockGetIdeInfo,
getWorkspaceDirs: mockGetWorkspaceDirs,
getIdeSettings: vi.fn(),
getDiff: vi.fn(),
readFile: vi.fn(),
readRangeInFile: vi.fn(),
isTelemetryEnabled: vi.fn(),
getProblems: vi.fn(),
subprocess: vi.fn(),
getWorkspaceConfigs: vi.fn(),
showToast: vi.fn(),
listWorkspaceContents: vi.fn(),
getTerminalContents: vi.fn(),
listFolders: vi.fn(),
getSessionId: vi.fn(),
runCommand: vi.fn(),
showLines: vi.fn(),
saveFile: vi.fn(),
getBranch: vi.fn(),
showDiff: vi.fn(),
getOpenFiles: vi.fn(),
showVirtualFile: vi.fn(),
openFile: vi.fn(),
getRepo: vi.fn(),
pathSep: vi.fn(),
fileExists: vi.fn(),
} as unknown as IDE;

return {
ide: mockIde,
llm: {} as any,
fetch: vi.fn() as any,
tool: {} as any,
config: {} as any,
onPartialOutput: mockOnPartialOutput,
toolCallId: "test-tool-call-id",
...overrides,
};
};

it("should set up timeout when waitForCompletion is true", async () => {
const extras = createMockExtras();
const args = { command: "echo test", waitForCompletion: true };
const resultPromise = runTerminalCommandImpl(args, extras);
await vi.runOnlyPendingTimersAsync();
expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 120_000);
mockChildProc.emit("close", 0);
await resultPromise;
expect(clearTimeoutSpy).toHaveBeenCalled();
});

it("should NOT set up timeout when waitForCompletion is false", async () => {
const extras = createMockExtras();
const args = { command: "sleep 10", waitForCompletion: false };
const result = await runTerminalCommandImpl(args, extras);
const timeoutCalls = setTimeoutSpy.mock.calls.filter(
(call: any[]) => call[1] === 120_000,
);
expect(timeoutCalls.length).toBe(0);
expect(result[0].status).toContain("background");
});

it("should kill process when timeout fires (streaming)", async () => {
const extras = createMockExtras();
const args = { command: "sleep 300", waitForCompletion: true };
const resultPromise = runTerminalCommandImpl(args, extras);
await vi.runOnlyPendingTimersAsync();
await vi.advanceTimersByTimeAsync(120_000);
expect(mockChildProc.kill).toHaveBeenCalledWith("SIGTERM");
await vi.advanceTimersByTimeAsync(5_000);
await vi.runAllTimersAsync();
const result = await resultPromise;
expect(result[0].content).toContain(
"[Timeout: process killed after 2 minutes]",
);
});

it("should clear timeout on normal process exit", async () => {
const extras = createMockExtras();
const args = { command: "echo quick", waitForCompletion: true };
const resultPromise = runTerminalCommandImpl(args, extras);
// Flush async setup (getIdeInfo, getWorkspaceDirs) without advancing timer time
await vi.advanceTimersByTimeAsync(0);
mockChildProc.stdout.emit("data", Buffer.from("quick\n"));
mockChildProc.emit("close", 0);
await resultPromise;
expect(clearTimeoutSpy).toHaveBeenCalled();
expect(mockChildProc.kill).not.toHaveBeenCalled();
});

it("should clear SIGKILL timeout when process exits between SIGTERM and SIGKILL grace period", async () => {
const extras = createMockExtras();
const args = { command: "sleep 300", waitForCompletion: true };
const resultPromise = runTerminalCommandImpl(args, extras);

// Let initial setup complete
await vi.runOnlyPendingTimersAsync();

// Advance to trigger main timeout (120s) — SIGTERM sent, SIGKILL timer started
await vi.advanceTimersByTimeAsync(120_000);
expect(mockChildProc.kill).toHaveBeenCalledWith("SIGTERM");
expect(mockChildProc.kill).toHaveBeenCalledTimes(1);

// Process exits gracefully after 2 seconds (before 5s grace period)
await vi.advanceTimersByTimeAsync(2_000);
mockChildProc.emit("close", 143); // SIGTERM exit code

// Wait for promise to resolve
await resultPromise;

// Advance past the SIGKILL grace period (3 more seconds)
await vi.advanceTimersByTimeAsync(3_000);

// Verify SIGKILL was NOT called (timer was cleared)
expect(mockChildProc.kill).toHaveBeenCalledTimes(1); // Only SIGTERM
expect(mockChildProc.kill).not.toHaveBeenCalledWith("SIGKILL");
});
});
86 changes: 86 additions & 0 deletions core/tools/implementations/runTerminalCommand.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import iconv from "iconv-lite";
import childProcess from "node:child_process";
import os from "node:os";
import { ContinueError, ContinueErrorReason } from "../../util/errors";

// Default timeout for terminal commands (2 minutes)
const DEFAULT_TOOL_TIMEOUT_MS = 120_000;

// Automatically decode the buffer according to the platform to avoid garbled Chinese
function getDecodedOutput(data: Buffer): string {
if (process.platform === "win32") {
Expand Down Expand Up @@ -133,6 +137,8 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {

return new Promise((resolve, reject) => {
let terminalOutput = "";
let timeoutId: ReturnType<typeof setTimeout> | undefined;
let sigkillTimeoutId: ReturnType<typeof setTimeout> | undefined;

if (!waitForCompletion) {
const status = "Command is running in the background...";
Expand Down Expand Up @@ -168,6 +174,41 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
);
}

// Set up timeout for waitForCompletion mode
if (waitForCompletion) {
timeoutId = setTimeout(() => {
if (!childProc.killed) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: SIGKILL fallback never runs because childProc.killed becomes true immediately after SIGTERM; it does not indicate process exit. Use exitCode/signalCode to detect whether the process is still running.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At core/tools/implementations/runTerminalCommand.ts, line 180:

<comment>SIGKILL fallback never runs because childProc.killed becomes true immediately after SIGTERM; it does not indicate process exit. Use exitCode/signalCode to detect whether the process is still running.</comment>

<file context>
@@ -168,6 +174,42 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
+          // Set up timeout for waitForCompletion mode
+          if (waitForCompletion) {
+            timeoutId = setTimeout(() => {
+              if (!childProc.killed) {
+                terminalOutput += "
+[Timeout: process killed after 2 minutes]
</file context>
Fix with Cubic

terminalOutput +=
"\n[Timeout: process killed after 2 minutes]\n";

// Update UI with timeout message
if (extras.onPartialOutput) {
extras.onPartialOutput({
toolCallId,
contextItems: [
{
name: "Terminal",
description: "Terminal command output",
content: terminalOutput,
status: "Command timed out",
},
],
});
}

// Try graceful termination first
childProc.kill("SIGTERM");

// Force kill after 5 seconds if still running
sigkillTimeoutId = setTimeout(() => {
if (!childProc.killed) {
childProc.kill("SIGKILL");
}
}, 5_000);
}
}, DEFAULT_TOOL_TIMEOUT_MS);
}

childProc.stdout?.on("data", (data) => {
// Skip if this process has been backgrounded
if (isProcessBackgrounded(toolCallId)) return;
Expand Down Expand Up @@ -240,6 +281,16 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
}

childProc.on("close", (code) => {
// Clear timeout on normal completion
if (timeoutId) {
clearTimeout(timeoutId);
}

// Clear inner SIGKILL timeout if process exits before grace period
if (sigkillTimeoutId) {
clearTimeout(sigkillTimeoutId);
}

// Clean up process tracking
if (toolCallId) {
if (isProcessBackgrounded(toolCallId)) {
Expand Down Expand Up @@ -296,6 +347,11 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
});

childProc.on("error", (error) => {
// Clear timeout on error
if (timeoutId) {
clearTimeout(timeoutId);
}

// Clean up process tracking
if (toolCallId) {
if (isProcessBackgrounded(toolCallId)) {
Expand Down Expand Up @@ -325,6 +381,9 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
getShellCommand(command);
const output = await new Promise<{ stdout: string; stderr: string }>(
(resolve, reject) => {
let timeoutId: ReturnType<typeof setTimeout> | undefined;
let sigkillTimeoutId: ReturnType<typeof setTimeout> | undefined;

const childProc = childProcess.spawn(
nonStreamingShell,
nonStreamingArgs,
Expand All @@ -342,6 +401,23 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
let stdout = "";
let stderr = "";

// Set up timeout
timeoutId = setTimeout(() => {
if (!childProc.killed) {
stderr += "\n[Timeout: process killed after 2 minutes]\n";

// Try graceful termination first
childProc.kill("SIGTERM");

// Force kill after 5 seconds if still running
sigkillTimeoutId = setTimeout(() => {
if (!childProc.killed) {
childProc.kill("SIGKILL");
}
}, 5_000);
}
}, DEFAULT_TOOL_TIMEOUT_MS);

childProc.stdout?.on("data", (data) => {
stdout += getDecodedOutput(data);
});
Expand All @@ -351,6 +427,16 @@ export const runTerminalCommandImpl: ToolImpl = async (args, extras) => {
});

childProc.on("close", (code) => {
// Clear outer timeout
if (timeoutId) {
clearTimeout(timeoutId);
}

// Clear inner SIGKILL timeout if process exits before grace period
if (sigkillTimeoutId) {
clearTimeout(sigkillTimeoutId);
}

// Clean up process tracking
if (toolCallId) {
removeRunningProcess(toolCallId);
Expand Down
Loading