Skip to content

Commit 3df6abd

Browse files
committed
feat: change default command execution timeout to 0 (no timeout)
- Change default commandExecutionTimeout from 30000ms to 0 (no timeout) - Update minimum value from 1000ms to 0ms in package.json - Update package.nls.json description to clarify 0 = no timeout - Modify executeCommandTool.ts to skip timeout logic when timeout is 0 - Update tests to reflect new default behavior and add test for no timeout - Timeout functionality works for both VSCode terminal and execa execution Addresses @mrubens feedback in PR #5668
1 parent ac332f3 commit 3df6abd

File tree

5 files changed

+71
-41
lines changed

5 files changed

+71
-41
lines changed

src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe("Command Execution Timeout Integration", () => {
5858

5959
// Mock VSCode configuration
6060
const mockGetConfiguration = vitest.fn().mockReturnValue({
61-
get: vitest.fn().mockReturnValue(30000), // Default 30 second timeout
61+
get: vitest.fn().mockReturnValue(0), // Default 0 (no timeout)
6262
})
6363
;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
6464
})
@@ -149,19 +149,40 @@ describe("Command Execution Timeout Integration", () => {
149149
expect(result[1]).not.toContain("timed out")
150150
})
151151

152-
it("should use default timeout when not specified", async () => {
152+
it("should use default timeout when not specified (0 = no timeout)", async () => {
153153
const options: ExecuteCommandOptions = {
154154
executionId: "test-execution",
155155
command: "echo test",
156-
// commandExecutionTimeout not specified, should use default
156+
// commandExecutionTimeout not specified, should use default (0)
157157
}
158158

159159
const quickProcess = Promise.resolve()
160160
mockTerminal.runCommand.mockReturnValue(quickProcess)
161161

162162
await executeCommand(mockTask as Task, options)
163163

164-
// Should complete without issues using default timeout
164+
// Should complete without issues using default (no timeout)
165165
expect(mockTerminal.runCommand).toHaveBeenCalled()
166166
})
167+
168+
it("should not timeout when commandExecutionTimeout is 0", async () => {
169+
const options: ExecuteCommandOptions = {
170+
executionId: "test-execution",
171+
command: "sleep 10",
172+
commandExecutionTimeout: 0, // No timeout
173+
}
174+
175+
// Create a process that resolves after a delay to simulate a long-running command
176+
const longRunningProcess = new Promise((resolve) => {
177+
setTimeout(resolve, 200) // 200ms delay
178+
})
179+
180+
mockTerminal.runCommand.mockReturnValue(longRunningProcess)
181+
182+
const result = await executeCommand(mockTask as Task, options)
183+
184+
// Should complete successfully without timeout
185+
expect(result[0]).toBe(false) // Not rejected
186+
expect(result[1]).not.toContain("timed out")
187+
})
167188
})

src/core/tools/__tests__/executeCommandTool.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ describe("executeCommandTool", () => {
298298
customCwd: undefined,
299299
terminalShellIntegrationDisabled: false,
300300
terminalOutputLineLimit: 500,
301-
commandExecutionTimeout: 30000,
301+
commandExecutionTimeout: 0,
302302
}
303303

304304
// Verify all required properties exist

src/core/tools/executeCommandTool.ts

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export async function executeCommandTool(
6767
// Get command execution timeout from VSCode configuration
6868
const commandExecutionTimeout = vscode.workspace
6969
.getConfiguration(Package.name)
70-
.get<number>("commandExecutionTimeout", 30000)
70+
.get<number>("commandExecutionTimeout", 0)
7171

7272
const options: ExecuteCommandOptions = {
7373
executionId,
@@ -132,7 +132,7 @@ export async function executeCommand(
132132
customCwd,
133133
terminalShellIntegrationDisabled = false,
134134
terminalOutputLineLimit = 500,
135-
commandExecutionTimeout = 30000,
135+
commandExecutionTimeout = 0,
136136
}: ExecuteCommandOptions,
137137
): Promise<[boolean, ToolResponse]> {
138138
let workingDir: string
@@ -221,42 +221,51 @@ export async function executeCommand(
221221
const process = terminal.runCommand(command, callbacks)
222222
cline.terminalProcess = process
223223

224-
// Implement command execution timeout
225-
let timeoutId: NodeJS.Timeout | undefined
226-
let isTimedOut = false
227-
228-
const timeoutPromise = new Promise<void>((_, reject) => {
229-
timeoutId = setTimeout(() => {
230-
isTimedOut = true
231-
// Try to abort the process
232-
if (cline.terminalProcess) {
233-
cline.terminalProcess.abort()
234-
}
235-
reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
236-
}, commandExecutionTimeout)
237-
})
224+
// Implement command execution timeout (skip if timeout is 0)
225+
if (commandExecutionTimeout > 0) {
226+
let timeoutId: NodeJS.Timeout | undefined
227+
let isTimedOut = false
228+
229+
const timeoutPromise = new Promise<void>((_, reject) => {
230+
timeoutId = setTimeout(() => {
231+
isTimedOut = true
232+
// Try to abort the process
233+
if (cline.terminalProcess) {
234+
cline.terminalProcess.abort()
235+
}
236+
reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
237+
}, commandExecutionTimeout)
238+
})
239+
240+
try {
241+
await Promise.race([process, timeoutPromise])
242+
} catch (error) {
243+
if (isTimedOut) {
244+
// Handle timeout case
245+
const status: CommandExecutionStatus = { executionId, status: "timeout" }
246+
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
238247

239-
try {
240-
await Promise.race([process, timeoutPromise])
241-
} catch (error) {
242-
if (isTimedOut) {
243-
// Handle timeout case
244-
const status: CommandExecutionStatus = { executionId, status: "timeout" }
245-
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
248+
cline.terminalProcess = undefined
246249

250+
return [
251+
false,
252+
`Command execution timed out after ${commandExecutionTimeout}ms. The command was terminated.`,
253+
]
254+
}
255+
throw error
256+
} finally {
257+
if (timeoutId) {
258+
clearTimeout(timeoutId)
259+
}
247260
cline.terminalProcess = undefined
248-
249-
return [
250-
false,
251-
`Command execution timed out after ${commandExecutionTimeout}ms. The command was terminated.`,
252-
]
253261
}
254-
throw error
255-
} finally {
256-
if (timeoutId) {
257-
clearTimeout(timeoutId)
262+
} else {
263+
// No timeout - just wait for the process to complete
264+
try {
265+
await process
266+
} finally {
267+
cline.terminalProcess = undefined
258268
}
259-
cline.terminalProcess = undefined
260269
}
261270

262271
if (shellIntegrationError) {

src/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,8 @@
340340
},
341341
"roo-cline.commandExecutionTimeout": {
342342
"type": "number",
343-
"default": 30000,
344-
"minimum": 1000,
343+
"default": 0,
344+
"minimum": 0,
345345
"maximum": 300000,
346346
"description": "%commands.commandExecutionTimeout.description%"
347347
},

src/package.nls.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
"configuration.title": "Roo Code",
2929
"commands.allowedCommands.description": "Commands that can be auto-executed when 'Always approve execute operations' is enabled",
3030
"commands.deniedCommands.description": "Command prefixes that will be automatically denied without asking for approval. In case of conflicts with allowed commands, the longest prefix match takes precedence. Add * to deny all commands.",
31-
"commands.commandExecutionTimeout.description": "Maximum time in milliseconds to wait for command execution to complete before timing out (1000-300000ms, default: 30000ms)",
31+
"commands.commandExecutionTimeout.description": "Maximum time in milliseconds to wait for command execution to complete before timing out (0 = no timeout, 1000-300000ms, default: 0ms)",
3232
"settings.vsCodeLmModelSelector.description": "Settings for VSCode Language Model API",
3333
"settings.vsCodeLmModelSelector.vendor.description": "The vendor of the language model (e.g. copilot)",
3434
"settings.vsCodeLmModelSelector.family.description": "The family of the language model (e.g. gpt-4)",

0 commit comments

Comments
 (0)