Skip to content

Commit 9956cc1

Browse files
authored
Use SIGKILL for command execution timeouts in the "execa" variant (#6071)
1 parent 2411c8f commit 9956cc1

File tree

5 files changed

+79
-64
lines changed

5 files changed

+79
-64
lines changed

apps/web-evals/src/actions/runs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export async function createRun({ suite, exercises = [], systemPrompt, timeout,
5656

5757
const dockerArgs = [
5858
`--name evals-controller-${run.id}`,
59-
// "--rm",
59+
"--rm",
6060
"--network evals_default",
6161
"-v /var/run/docker.sock:/var/run/docker.sock",
6262
"-v /tmp/evals:/var/log/evals",

packages/evals/src/cli/runTask.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@ import * as os from "node:os"
55
import pWaitFor from "p-wait-for"
66
import { execa } from "execa"
77

8-
import { type TaskEvent, TaskCommandName, RooCodeEventName, IpcMessageType, EVALS_SETTINGS } from "@roo-code/types"
8+
import {
9+
type TaskEvent,
10+
type ClineSay,
11+
TaskCommandName,
12+
RooCodeEventName,
13+
IpcMessageType,
14+
EVALS_SETTINGS,
15+
} from "@roo-code/types"
916
import { IpcClient } from "@roo-code/ipc"
1017

1118
import {
@@ -203,6 +210,15 @@ export const runTask = async ({ run, task, publish, logger }: RunTaskOptions) =>
203210
log: [RooCodeEventName.TaskTokenUsageUpdated, RooCodeEventName.TaskAskResponded],
204211
}
205212

213+
const loggableSays: ClineSay[] = [
214+
"error",
215+
"command_output",
216+
"rooignore_error",
217+
"diff_error",
218+
"condense_context",
219+
"condense_context_error",
220+
]
221+
206222
client.on(IpcMessageType.TaskEvent, async (taskEvent) => {
207223
const { eventName, payload } = taskEvent
208224

@@ -215,7 +231,9 @@ export const runTask = async ({ run, task, publish, logger }: RunTaskOptions) =>
215231
// For message events we only log non-partial messages.
216232
if (
217233
!ignoreEvents.log.includes(eventName) &&
218-
(eventName !== RooCodeEventName.Message || payload[0].message.partial !== true)
234+
(eventName !== RooCodeEventName.Message ||
235+
(payload[0].message.say && loggableSays.includes(payload[0].message.say)) ||
236+
payload[0].message.partial !== true)
219237
) {
220238
logger.info(`${eventName} ->`, payload)
221239
}

packages/types/src/global-settings.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
222222
alwaysAllowUpdateTodoList: true,
223223
followupAutoApproveTimeoutMs: 0,
224224
allowedCommands: ["*"],
225-
commandExecutionTimeout: 30_000,
225+
commandExecutionTimeout: 20,
226226
commandTimeoutAllowlist: [],
227227
preventCompletionWithOpenTodos: false,
228228

@@ -266,7 +266,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
266266

267267
mcpEnabled: false,
268268

269-
mode: "code",
269+
mode: "code", // "architect",
270270

271271
customModes: [],
272272
}

src/core/tools/executeCommandTool.ts

Lines changed: 43 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { t } from "../../i18n"
2121
class ShellIntegrationError extends Error {}
2222

2323
export async function executeCommandTool(
24-
cline: Task,
24+
task: Task,
2525
block: ToolUse,
2626
askApproval: AskApproval,
2727
handleError: HandleError,
@@ -33,25 +33,25 @@ export async function executeCommandTool(
3333

3434
try {
3535
if (block.partial) {
36-
await cline.ask("command", removeClosingTag("command", command), block.partial).catch(() => {})
36+
await task.ask("command", removeClosingTag("command", command), block.partial).catch(() => {})
3737
return
3838
} else {
3939
if (!command) {
40-
cline.consecutiveMistakeCount++
41-
cline.recordToolError("execute_command")
42-
pushToolResult(await cline.sayAndCreateMissingParamError("execute_command", "command"))
40+
task.consecutiveMistakeCount++
41+
task.recordToolError("execute_command")
42+
pushToolResult(await task.sayAndCreateMissingParamError("execute_command", "command"))
4343
return
4444
}
4545

46-
const ignoredFileAttemptedToAccess = cline.rooIgnoreController?.validateCommand(command)
46+
const ignoredFileAttemptedToAccess = task.rooIgnoreController?.validateCommand(command)
4747

4848
if (ignoredFileAttemptedToAccess) {
49-
await cline.say("rooignore_error", ignoredFileAttemptedToAccess)
49+
await task.say("rooignore_error", ignoredFileAttemptedToAccess)
5050
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess)))
5151
return
5252
}
5353

54-
cline.consecutiveMistakeCount = 0
54+
task.consecutiveMistakeCount = 0
5555

5656
command = unescapeHtmlEntities(command) // Unescape HTML entities.
5757
const didApprove = await askApproval("command", command)
@@ -60,14 +60,15 @@ export async function executeCommandTool(
6060
return
6161
}
6262

63-
const executionId = cline.lastMessageTs?.toString() ?? Date.now().toString()
64-
const clineProvider = await cline.providerRef.deref()
65-
const clineProviderState = await clineProvider?.getState()
63+
const executionId = task.lastMessageTs?.toString() ?? Date.now().toString()
64+
const provider = await task.providerRef.deref()
65+
const providerState = await provider?.getState()
66+
6667
const {
6768
terminalOutputLineLimit = 500,
6869
terminalOutputCharacterLimit = DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT,
6970
terminalShellIntegrationDisabled = false,
70-
} = clineProviderState ?? {}
71+
} = providerState ?? {}
7172

7273
// Get command execution timeout from VSCode configuration (in seconds)
7374
const commandExecutionTimeoutSeconds = vscode.workspace
@@ -96,26 +97,26 @@ export async function executeCommandTool(
9697
}
9798

9899
try {
99-
const [rejected, result] = await executeCommand(cline, options)
100+
const [rejected, result] = await executeCommand(task, options)
100101

101102
if (rejected) {
102-
cline.didRejectTool = true
103+
task.didRejectTool = true
103104
}
104105

105106
pushToolResult(result)
106107
} catch (error: unknown) {
107108
const status: CommandExecutionStatus = { executionId, status: "fallback" }
108-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
109-
await cline.say("shell_integration_warning")
109+
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
110+
await task.say("shell_integration_warning")
110111

111112
if (error instanceof ShellIntegrationError) {
112-
const [rejected, result] = await executeCommand(cline, {
113+
const [rejected, result] = await executeCommand(task, {
113114
...options,
114115
terminalShellIntegrationDisabled: true,
115116
})
116117

117118
if (rejected) {
118-
cline.didRejectTool = true
119+
task.didRejectTool = true
119120
}
120121

121122
pushToolResult(result)
@@ -143,7 +144,7 @@ export type ExecuteCommandOptions = {
143144
}
144145

145146
export async function executeCommand(
146-
cline: Task,
147+
task: Task,
147148
{
148149
executionId,
149150
command,
@@ -154,16 +155,16 @@ export async function executeCommand(
154155
commandExecutionTimeout = 0,
155156
}: ExecuteCommandOptions,
156157
): Promise<[boolean, ToolResponse]> {
157-
// Convert milliseconds back to seconds for display purposes
158+
// Convert milliseconds back to seconds for display purposes.
158159
const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000
159160
let workingDir: string
160161

161162
if (!customCwd) {
162-
workingDir = cline.cwd
163+
workingDir = task.cwd
163164
} else if (path.isAbsolute(customCwd)) {
164165
workingDir = customCwd
165166
} else {
166-
workingDir = path.resolve(cline.cwd, customCwd)
167+
workingDir = path.resolve(task.cwd, customCwd)
167168
}
168169

169170
try {
@@ -180,7 +181,7 @@ export async function executeCommand(
180181
let shellIntegrationError: string | undefined
181182

182183
const terminalProvider = terminalShellIntegrationDisabled ? "execa" : "vscode"
183-
const clineProvider = await cline.providerRef.deref()
184+
const provider = await task.providerRef.deref()
184185

185186
let accumulatedOutput = ""
186187
const callbacks: RooTerminalCallbacks = {
@@ -192,14 +193,14 @@ export async function executeCommand(
192193
terminalOutputCharacterLimit,
193194
)
194195
const status: CommandExecutionStatus = { executionId, status: "output", output: compressedOutput }
195-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
196+
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
196197

197198
if (runInBackground) {
198199
return
199200
}
200201

201202
try {
202-
const { response, text, images } = await cline.ask("command_output", "")
203+
const { response, text, images } = await task.ask("command_output", "")
203204
runInBackground = true
204205

205206
if (response === "messageResponse") {
@@ -214,29 +215,30 @@ export async function executeCommand(
214215
terminalOutputLineLimit,
215216
terminalOutputCharacterLimit,
216217
)
217-
cline.say("command_output", result)
218+
219+
task.say("command_output", result)
218220
completed = true
219221
},
220222
onShellExecutionStarted: (pid: number | undefined) => {
221223
console.log(`[executeCommand] onShellExecutionStarted: ${pid}`)
222224
const status: CommandExecutionStatus = { executionId, status: "started", pid, command }
223-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
225+
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
224226
},
225227
onShellExecutionComplete: (details: ExitCodeDetails) => {
226228
const status: CommandExecutionStatus = { executionId, status: "exited", exitCode: details.exitCode }
227-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
229+
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
228230
exitDetails = details
229231
},
230232
}
231233

232234
if (terminalProvider === "vscode") {
233235
callbacks.onNoShellIntegration = async (error: string) => {
234-
TelemetryService.instance.captureShellIntegrationError(cline.taskId)
236+
TelemetryService.instance.captureShellIntegrationError(task.taskId)
235237
shellIntegrationError = error
236238
}
237239
}
238240

239-
const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, cline.taskId, terminalProvider)
241+
const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, task.taskId, terminalProvider)
240242

241243
if (terminal instanceof Terminal) {
242244
terminal.terminal.show(true)
@@ -248,20 +250,17 @@ export async function executeCommand(
248250
}
249251

250252
const process = terminal.runCommand(command, callbacks)
251-
cline.terminalProcess = process
253+
task.terminalProcess = process
252254

253-
// Implement command execution timeout (skip if timeout is 0)
255+
// Implement command execution timeout (skip if timeout is 0).
254256
if (commandExecutionTimeout > 0) {
255257
let timeoutId: NodeJS.Timeout | undefined
256258
let isTimedOut = false
257259

258260
const timeoutPromise = new Promise<void>((_, reject) => {
259261
timeoutId = setTimeout(() => {
260262
isTimedOut = true
261-
// Try to abort the process
262-
if (cline.terminalProcess) {
263-
cline.terminalProcess.abort()
264-
}
263+
task.terminalProcess?.abort()
265264
reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
266265
}, commandExecutionTimeout)
267266
})
@@ -270,17 +269,10 @@ export async function executeCommand(
270269
await Promise.race([process, timeoutPromise])
271270
} catch (error) {
272271
if (isTimedOut) {
273-
// Handle timeout case
274272
const status: CommandExecutionStatus = { executionId, status: "timeout" }
275-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
276-
277-
// Add visual feedback for timeout
278-
await cline.say(
279-
"error",
280-
t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }),
281-
)
282-
283-
cline.terminalProcess = undefined
273+
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
274+
await task.say("error", t("common:errors:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
275+
task.terminalProcess = undefined
284276

285277
return [
286278
false,
@@ -292,14 +284,15 @@ export async function executeCommand(
292284
if (timeoutId) {
293285
clearTimeout(timeoutId)
294286
}
295-
cline.terminalProcess = undefined
287+
288+
task.terminalProcess = undefined
296289
}
297290
} else {
298-
// No timeout - just wait for the process to complete
291+
// No timeout - just wait for the process to complete.
299292
try {
300293
await process
301294
} finally {
302-
cline.terminalProcess = undefined
295+
task.terminalProcess = undefined
303296
}
304297
}
305298

@@ -316,7 +309,7 @@ export async function executeCommand(
316309

317310
if (message) {
318311
const { text, images } = message
319-
await cline.say("user_feedback", text, images)
312+
await task.say("user_feedback", text, images)
320313

321314
return [
322315
true,

src/integrations/terminal/ExecaTerminalProcess.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
7373
let timeoutId: NodeJS.Timeout | undefined
7474

7575
const kill = new Promise<void>((resolve) => {
76+
console.log(`[ExecaTerminalProcess#run] SIGKILL -> ${this.pid}`)
77+
7678
timeoutId = setTimeout(() => {
7779
try {
7880
subprocess.kill("SIGKILL")
@@ -86,7 +88,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
8688
await Promise.race([subprocess, kill])
8789
} catch (error) {
8890
console.log(
89-
`[ExecaTerminalProcess] subprocess termination error: ${error instanceof Error ? error.message : String(error)}`,
91+
`[ExecaTerminalProcess#run] subprocess termination error: ${error instanceof Error ? error.message : String(error)}`,
9092
)
9193
}
9294

@@ -98,12 +100,13 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
98100
this.emit("shell_execution_complete", { exitCode: 0 })
99101
} catch (error) {
100102
if (error instanceof ExecaError) {
101-
console.error(`[ExecaTerminalProcess] shell execution error: ${error.message}`)
103+
console.error(`[ExecaTerminalProcess#run] shell execution error: ${error.message}`)
102104
this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal })
103105
} else {
104106
console.error(
105-
`[ExecaTerminalProcess] shell execution error: ${error instanceof Error ? error.message : String(error)}`,
107+
`[ExecaTerminalProcess#run] shell execution error: ${error instanceof Error ? error.message : String(error)}`,
106108
)
109+
107110
this.emit("shell_execution_complete", { exitCode: 1 })
108111
}
109112
}
@@ -128,29 +131,30 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
128131
psTree(this.pid, async (err, children) => {
129132
if (!err) {
130133
const pids = children.map((p) => parseInt(p.PID))
134+
console.error(`[ExecaTerminalProcess#abort] SIGKILL children -> ${pids.join(", ")}`)
131135

132136
for (const pid of pids) {
133137
try {
134-
process.kill(pid, "SIGINT")
138+
process.kill(pid, "SIGKILL")
135139
} catch (e) {
136140
console.warn(
137-
`[ExecaTerminalProcess] Failed to send SIGINT to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
141+
`[ExecaTerminalProcess#abort] Failed to send SIGKILL to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
138142
)
139-
// Optionally try SIGTERM or SIGKILL on failure, depending on desired behavior.
140143
}
141144
}
142145
} else {
143146
console.error(
144-
`[ExecaTerminalProcess] Failed to get process tree for PID ${this.pid}: ${err.message}`,
147+
`[ExecaTerminalProcess#abort] Failed to get process tree for PID ${this.pid}: ${err.message}`,
145148
)
146149
}
147150
})
148151

149152
try {
150-
process.kill(this.pid, "SIGINT")
153+
console.error(`[ExecaTerminalProcess#abort] SIGKILL parent -> ${this.pid}`)
154+
process.kill(this.pid, "SIGKILL")
151155
} catch (e) {
152156
console.warn(
153-
`[ExecaTerminalProcess] Failed to send SIGINT to main PID ${this.pid}: ${e instanceof Error ? e.message : String(e)}`,
157+
`[ExecaTerminalProcess#abort] Failed to send SIGKILL to main PID ${this.pid}: ${e instanceof Error ? e.message : String(e)}`,
154158
)
155159
}
156160
}

0 commit comments

Comments
 (0)