Use SIGKILL for command execution timeouts in the "execa" variant (#6071)

This commit is contained in:
Chris Estreich 2025-07-23 01:47:10 -07:00 committed by GitHub
parent 2411c8faa4
commit 9956cc1f48
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 79 additions and 64 deletions

View file

@ -56,7 +56,7 @@ export async function createRun({ suite, exercises = [], systemPrompt, timeout,
const dockerArgs = [ const dockerArgs = [
`--name evals-controller-${run.id}`, `--name evals-controller-${run.id}`,
// "--rm", "--rm",
"--network evals_default", "--network evals_default",
"-v /var/run/docker.sock:/var/run/docker.sock", "-v /var/run/docker.sock:/var/run/docker.sock",
"-v /tmp/evals:/var/log/evals", "-v /tmp/evals:/var/log/evals",

View file

@ -5,7 +5,14 @@ import * as os from "node:os"
import pWaitFor from "p-wait-for" import pWaitFor from "p-wait-for"
import { execa } from "execa" import { execa } from "execa"
import { type TaskEvent, TaskCommandName, RooCodeEventName, IpcMessageType, EVALS_SETTINGS } from "@roo-code/types" import {
type TaskEvent,
type ClineSay,
TaskCommandName,
RooCodeEventName,
IpcMessageType,
EVALS_SETTINGS,
} from "@roo-code/types"
import { IpcClient } from "@roo-code/ipc" import { IpcClient } from "@roo-code/ipc"
import { import {
@ -203,6 +210,15 @@ export const runTask = async ({ run, task, publish, logger }: RunTaskOptions) =>
log: [RooCodeEventName.TaskTokenUsageUpdated, RooCodeEventName.TaskAskResponded], log: [RooCodeEventName.TaskTokenUsageUpdated, RooCodeEventName.TaskAskResponded],
} }
const loggableSays: ClineSay[] = [
"error",
"command_output",
"rooignore_error",
"diff_error",
"condense_context",
"condense_context_error",
]
client.on(IpcMessageType.TaskEvent, async (taskEvent) => { client.on(IpcMessageType.TaskEvent, async (taskEvent) => {
const { eventName, payload } = taskEvent const { eventName, payload } = taskEvent
@ -215,7 +231,9 @@ export const runTask = async ({ run, task, publish, logger }: RunTaskOptions) =>
// For message events we only log non-partial messages. // For message events we only log non-partial messages.
if ( if (
!ignoreEvents.log.includes(eventName) && !ignoreEvents.log.includes(eventName) &&
(eventName !== RooCodeEventName.Message || payload[0].message.partial !== true) (eventName !== RooCodeEventName.Message ||
(payload[0].message.say && loggableSays.includes(payload[0].message.say)) ||
payload[0].message.partial !== true)
) { ) {
logger.info(`${eventName} ->`, payload) logger.info(`${eventName} ->`, payload)
} }

View file

@ -222,7 +222,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
alwaysAllowUpdateTodoList: true, alwaysAllowUpdateTodoList: true,
followupAutoApproveTimeoutMs: 0, followupAutoApproveTimeoutMs: 0,
allowedCommands: ["*"], allowedCommands: ["*"],
commandExecutionTimeout: 30_000, commandExecutionTimeout: 20,
commandTimeoutAllowlist: [], commandTimeoutAllowlist: [],
preventCompletionWithOpenTodos: false, preventCompletionWithOpenTodos: false,
@ -266,7 +266,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
mcpEnabled: false, mcpEnabled: false,
mode: "code", mode: "code", // "architect",
customModes: [], customModes: [],
} }

View file

@ -21,7 +21,7 @@ import { t } from "../../i18n"
class ShellIntegrationError extends Error {} class ShellIntegrationError extends Error {}
export async function executeCommandTool( export async function executeCommandTool(
cline: Task, task: Task,
block: ToolUse, block: ToolUse,
askApproval: AskApproval, askApproval: AskApproval,
handleError: HandleError, handleError: HandleError,
@ -33,25 +33,25 @@ export async function executeCommandTool(
try { try {
if (block.partial) { if (block.partial) {
await cline.ask("command", removeClosingTag("command", command), block.partial).catch(() => {}) await task.ask("command", removeClosingTag("command", command), block.partial).catch(() => {})
return return
} else { } else {
if (!command) { if (!command) {
cline.consecutiveMistakeCount++ task.consecutiveMistakeCount++
cline.recordToolError("execute_command") task.recordToolError("execute_command")
pushToolResult(await cline.sayAndCreateMissingParamError("execute_command", "command")) pushToolResult(await task.sayAndCreateMissingParamError("execute_command", "command"))
return return
} }
const ignoredFileAttemptedToAccess = cline.rooIgnoreController?.validateCommand(command) const ignoredFileAttemptedToAccess = task.rooIgnoreController?.validateCommand(command)
if (ignoredFileAttemptedToAccess) { if (ignoredFileAttemptedToAccess) {
await cline.say("rooignore_error", ignoredFileAttemptedToAccess) await task.say("rooignore_error", ignoredFileAttemptedToAccess)
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess))) pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess)))
return return
} }
cline.consecutiveMistakeCount = 0 task.consecutiveMistakeCount = 0
command = unescapeHtmlEntities(command) // Unescape HTML entities. command = unescapeHtmlEntities(command) // Unescape HTML entities.
const didApprove = await askApproval("command", command) const didApprove = await askApproval("command", command)
@ -60,14 +60,15 @@ export async function executeCommandTool(
return return
} }
const executionId = cline.lastMessageTs?.toString() ?? Date.now().toString() const executionId = task.lastMessageTs?.toString() ?? Date.now().toString()
const clineProvider = await cline.providerRef.deref() const provider = await task.providerRef.deref()
const clineProviderState = await clineProvider?.getState() const providerState = await provider?.getState()
const { const {
terminalOutputLineLimit = 500, terminalOutputLineLimit = 500,
terminalOutputCharacterLimit = DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT, terminalOutputCharacterLimit = DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT,
terminalShellIntegrationDisabled = false, terminalShellIntegrationDisabled = false,
} = clineProviderState ?? {} } = providerState ?? {}
// Get command execution timeout from VSCode configuration (in seconds) // Get command execution timeout from VSCode configuration (in seconds)
const commandExecutionTimeoutSeconds = vscode.workspace const commandExecutionTimeoutSeconds = vscode.workspace
@ -96,26 +97,26 @@ export async function executeCommandTool(
} }
try { try {
const [rejected, result] = await executeCommand(cline, options) const [rejected, result] = await executeCommand(task, options)
if (rejected) { if (rejected) {
cline.didRejectTool = true task.didRejectTool = true
} }
pushToolResult(result) pushToolResult(result)
} catch (error: unknown) { } catch (error: unknown) {
const status: CommandExecutionStatus = { executionId, status: "fallback" } const status: CommandExecutionStatus = { executionId, status: "fallback" }
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
await cline.say("shell_integration_warning") await task.say("shell_integration_warning")
if (error instanceof ShellIntegrationError) { if (error instanceof ShellIntegrationError) {
const [rejected, result] = await executeCommand(cline, { const [rejected, result] = await executeCommand(task, {
...options, ...options,
terminalShellIntegrationDisabled: true, terminalShellIntegrationDisabled: true,
}) })
if (rejected) { if (rejected) {
cline.didRejectTool = true task.didRejectTool = true
} }
pushToolResult(result) pushToolResult(result)
@ -143,7 +144,7 @@ export type ExecuteCommandOptions = {
} }
export async function executeCommand( export async function executeCommand(
cline: Task, task: Task,
{ {
executionId, executionId,
command, command,
@ -154,16 +155,16 @@ export async function executeCommand(
commandExecutionTimeout = 0, commandExecutionTimeout = 0,
}: ExecuteCommandOptions, }: ExecuteCommandOptions,
): Promise<[boolean, ToolResponse]> { ): Promise<[boolean, ToolResponse]> {
// Convert milliseconds back to seconds for display purposes // Convert milliseconds back to seconds for display purposes.
const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000 const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000
let workingDir: string let workingDir: string
if (!customCwd) { if (!customCwd) {
workingDir = cline.cwd workingDir = task.cwd
} else if (path.isAbsolute(customCwd)) { } else if (path.isAbsolute(customCwd)) {
workingDir = customCwd workingDir = customCwd
} else { } else {
workingDir = path.resolve(cline.cwd, customCwd) workingDir = path.resolve(task.cwd, customCwd)
} }
try { try {
@ -180,7 +181,7 @@ export async function executeCommand(
let shellIntegrationError: string | undefined let shellIntegrationError: string | undefined
const terminalProvider = terminalShellIntegrationDisabled ? "execa" : "vscode" const terminalProvider = terminalShellIntegrationDisabled ? "execa" : "vscode"
const clineProvider = await cline.providerRef.deref() const provider = await task.providerRef.deref()
let accumulatedOutput = "" let accumulatedOutput = ""
const callbacks: RooTerminalCallbacks = { const callbacks: RooTerminalCallbacks = {
@ -192,14 +193,14 @@ export async function executeCommand(
terminalOutputCharacterLimit, terminalOutputCharacterLimit,
) )
const status: CommandExecutionStatus = { executionId, status: "output", output: compressedOutput } const status: CommandExecutionStatus = { executionId, status: "output", output: compressedOutput }
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
if (runInBackground) { if (runInBackground) {
return return
} }
try { try {
const { response, text, images } = await cline.ask("command_output", "") const { response, text, images } = await task.ask("command_output", "")
runInBackground = true runInBackground = true
if (response === "messageResponse") { if (response === "messageResponse") {
@ -214,29 +215,30 @@ export async function executeCommand(
terminalOutputLineLimit, terminalOutputLineLimit,
terminalOutputCharacterLimit, terminalOutputCharacterLimit,
) )
cline.say("command_output", result)
task.say("command_output", result)
completed = true completed = true
}, },
onShellExecutionStarted: (pid: number | undefined) => { onShellExecutionStarted: (pid: number | undefined) => {
console.log(`[executeCommand] onShellExecutionStarted: ${pid}`) console.log(`[executeCommand] onShellExecutionStarted: ${pid}`)
const status: CommandExecutionStatus = { executionId, status: "started", pid, command } const status: CommandExecutionStatus = { executionId, status: "started", pid, command }
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
}, },
onShellExecutionComplete: (details: ExitCodeDetails) => { onShellExecutionComplete: (details: ExitCodeDetails) => {
const status: CommandExecutionStatus = { executionId, status: "exited", exitCode: details.exitCode } const status: CommandExecutionStatus = { executionId, status: "exited", exitCode: details.exitCode }
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
exitDetails = details exitDetails = details
}, },
} }
if (terminalProvider === "vscode") { if (terminalProvider === "vscode") {
callbacks.onNoShellIntegration = async (error: string) => { callbacks.onNoShellIntegration = async (error: string) => {
TelemetryService.instance.captureShellIntegrationError(cline.taskId) TelemetryService.instance.captureShellIntegrationError(task.taskId)
shellIntegrationError = error shellIntegrationError = error
} }
} }
const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, cline.taskId, terminalProvider) const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, task.taskId, terminalProvider)
if (terminal instanceof Terminal) { if (terminal instanceof Terminal) {
terminal.terminal.show(true) terminal.terminal.show(true)
@ -248,9 +250,9 @@ export async function executeCommand(
} }
const process = terminal.runCommand(command, callbacks) const process = terminal.runCommand(command, callbacks)
cline.terminalProcess = process task.terminalProcess = process
// Implement command execution timeout (skip if timeout is 0) // Implement command execution timeout (skip if timeout is 0).
if (commandExecutionTimeout > 0) { if (commandExecutionTimeout > 0) {
let timeoutId: NodeJS.Timeout | undefined let timeoutId: NodeJS.Timeout | undefined
let isTimedOut = false let isTimedOut = false
@ -258,10 +260,7 @@ export async function executeCommand(
const timeoutPromise = new Promise<void>((_, reject) => { const timeoutPromise = new Promise<void>((_, reject) => {
timeoutId = setTimeout(() => { timeoutId = setTimeout(() => {
isTimedOut = true isTimedOut = true
// Try to abort the process task.terminalProcess?.abort()
if (cline.terminalProcess) {
cline.terminalProcess.abort()
}
reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`)) reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
}, commandExecutionTimeout) }, commandExecutionTimeout)
}) })
@ -270,17 +269,10 @@ export async function executeCommand(
await Promise.race([process, timeoutPromise]) await Promise.race([process, timeoutPromise])
} catch (error) { } catch (error) {
if (isTimedOut) { if (isTimedOut) {
// Handle timeout case
const status: CommandExecutionStatus = { executionId, status: "timeout" } const status: CommandExecutionStatus = { executionId, status: "timeout" }
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
await task.say("error", t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
// Add visual feedback for timeout task.terminalProcess = undefined
await cline.say(
"error",
t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }),
)
cline.terminalProcess = undefined
return [ return [
false, false,
@ -292,14 +284,15 @@ export async function executeCommand(
if (timeoutId) { if (timeoutId) {
clearTimeout(timeoutId) clearTimeout(timeoutId)
} }
cline.terminalProcess = undefined
task.terminalProcess = undefined
} }
} else { } else {
// No timeout - just wait for the process to complete // No timeout - just wait for the process to complete.
try { try {
await process await process
} finally { } finally {
cline.terminalProcess = undefined task.terminalProcess = undefined
} }
} }
@ -316,7 +309,7 @@ export async function executeCommand(
if (message) { if (message) {
const { text, images } = message const { text, images } = message
await cline.say("user_feedback", text, images) await task.say("user_feedback", text, images)
return [ return [
true, true,

View file

@ -73,6 +73,8 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
let timeoutId: NodeJS.Timeout | undefined let timeoutId: NodeJS.Timeout | undefined
const kill = new Promise<void>((resolve) => { const kill = new Promise<void>((resolve) => {
console.log(`[ExecaTerminalProcess#run] SIGKILL -> ${this.pid}`)
timeoutId = setTimeout(() => { timeoutId = setTimeout(() => {
try { try {
subprocess.kill("SIGKILL") subprocess.kill("SIGKILL")
@ -86,7 +88,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
await Promise.race([subprocess, kill]) await Promise.race([subprocess, kill])
} catch (error) { } catch (error) {
console.log( console.log(
`[ExecaTerminalProcess] subprocess termination error: ${error instanceof Error ? error.message : String(error)}`, `[ExecaTerminalProcess#run] subprocess termination error: ${error instanceof Error ? error.message : String(error)}`,
) )
} }
@ -98,12 +100,13 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
this.emit("shell_execution_complete", { exitCode: 0 }) this.emit("shell_execution_complete", { exitCode: 0 })
} catch (error) { } catch (error) {
if (error instanceof ExecaError) { if (error instanceof ExecaError) {
console.error(`[ExecaTerminalProcess] shell execution error: ${error.message}`) console.error(`[ExecaTerminalProcess#run] shell execution error: ${error.message}`)
this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal }) this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal })
} else { } else {
console.error( console.error(
`[ExecaTerminalProcess] shell execution error: ${error instanceof Error ? error.message : String(error)}`, `[ExecaTerminalProcess#run] shell execution error: ${error instanceof Error ? error.message : String(error)}`,
) )
this.emit("shell_execution_complete", { exitCode: 1 }) this.emit("shell_execution_complete", { exitCode: 1 })
} }
} }
@ -128,29 +131,30 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
psTree(this.pid, async (err, children) => { psTree(this.pid, async (err, children) => {
if (!err) { if (!err) {
const pids = children.map((p) => parseInt(p.PID)) const pids = children.map((p) => parseInt(p.PID))
console.error(`[ExecaTerminalProcess#abort] SIGKILL children -> ${pids.join(", ")}`)
for (const pid of pids) { for (const pid of pids) {
try { try {
process.kill(pid, "SIGINT") process.kill(pid, "SIGKILL")
} catch (e) { } catch (e) {
console.warn( console.warn(
`[ExecaTerminalProcess] Failed to send SIGINT to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`, `[ExecaTerminalProcess#abort] Failed to send SIGKILL to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
) )
// Optionally try SIGTERM or SIGKILL on failure, depending on desired behavior.
} }
} }
} else { } else {
console.error( console.error(
`[ExecaTerminalProcess] Failed to get process tree for PID ${this.pid}: ${err.message}`, `[ExecaTerminalProcess#abort] Failed to get process tree for PID ${this.pid}: ${err.message}`,
) )
} }
}) })
try { try {
process.kill(this.pid, "SIGINT") console.error(`[ExecaTerminalProcess#abort] SIGKILL parent -> ${this.pid}`)
process.kill(this.pid, "SIGKILL")
} catch (e) { } catch (e) {
console.warn( console.warn(
`[ExecaTerminalProcess] Failed to send SIGINT to main PID ${this.pid}: ${e instanceof Error ? e.message : String(e)}`, `[ExecaTerminalProcess#abort] Failed to send SIGKILL to main PID ${this.pid}: ${e instanceof Error ? e.message : String(e)}`,
) )
} }
} }