feat: Add configurable timeout for command execution (#5668)

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: Matt Rubens <[email protected]>
Co-authored-by: Daniel Riccio <[email protected]>
Co-authored-by: Daniel <[email protected]>

Author: 4 people

packages/types/src/terminal.ts

@@ -25,6 +25,10 @@ export const commandExecutionStatusSchema = z.discriminatedUnion("status", [
 		executionId: z.string(),
 		status: z.literal("fallback"),
 	}),
+	z.object({
+		executionId: z.string(),
+		status: z.literal("timeout"),
+	}),
 ])
 
 export type CommandExecutionStatus = z.infer<typeof commandExecutionStatusSchema>

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

@@ -0,0 +1,189 @@
+// Integration tests for command execution timeout functionality
+// npx vitest run src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts
+
+import * as vscode from "vscode"
+import * as fs from "fs/promises"
+import { executeCommand, ExecuteCommandOptions } from "../executeCommandTool"
+import { Task } from "../../task/Task"
+import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry"
+
+// Mock dependencies
+vitest.mock("vscode", () => ({
+	workspace: {
+		getConfiguration: vitest.fn(),
+	},
+}))
+
+vitest.mock("fs/promises")
+vitest.mock("../../../integrations/terminal/TerminalRegistry")
+vitest.mock("../../task/Task")
+
+describe("Command Execution Timeout Integration", () => {
+	let mockTask: any
+	let mockTerminal: any
+	let mockProcess: any
+
+	beforeEach(() => {
+		vitest.clearAllMocks()
+
+		// Mock fs.access to resolve successfully for working directory
+		;(fs.access as any).mockResolvedValue(undefined)
+
+		// Mock task
+		mockTask = {
+			cwd: "/test/directory",
+			terminalProcess: undefined,
+			providerRef: {
+				deref: vitest.fn().mockResolvedValue({
+					postMessageToWebview: vitest.fn(),
+				}),
+			},
+			say: vitest.fn().mockResolvedValue(undefined),
+		}
+
+		// Mock terminal process
+		mockProcess = {
+			abort: vitest.fn(),
+			then: vitest.fn(),
+			catch: vitest.fn(),
+		}
+
+		// Mock terminal
+		mockTerminal = {
+			runCommand: vitest.fn().mockReturnValue(mockProcess),
+			getCurrentWorkingDirectory: vitest.fn().mockReturnValue("/test/directory"),
+		}
+
+		// Mock TerminalRegistry
+		;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal)
+
+		// Mock VSCode configuration
+		const mockGetConfiguration = vitest.fn().mockReturnValue({
+			get: vitest.fn().mockReturnValue(0), // Default 0 (no timeout)
+		})
+		;(vscode.workspace.getConfiguration as any).mockReturnValue(mockGetConfiguration())
+	})
+
+	it("should pass timeout configuration to executeCommand", async () => {
+		const customTimeoutMs = 15000 // 15 seconds in milliseconds
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "echo test",
+			commandExecutionTimeout: customTimeoutMs,
+		}
+
+		// Mock a quick-completing process
+		const quickProcess = Promise.resolve()
+		mockTerminal.runCommand.mockReturnValue(quickProcess)
+
+		await executeCommand(mockTask as Task, options)
+
+		// Verify that the terminal was called with the command
+		expect(mockTerminal.runCommand).toHaveBeenCalledWith("echo test", expect.any(Object))
+	})
+
+	it("should handle timeout scenario", async () => {
+		const shortTimeoutMs = 100 // Very short timeout in milliseconds
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "sleep 10",
+			commandExecutionTimeout: shortTimeoutMs,
+		}
+
+		// Create a process that never resolves but has an abort method
+		const longRunningProcess = new Promise(() => {
+			// Never resolves to simulate a hanging command
+		})
+
+		// Add abort method to the promise
+		;(longRunningProcess as any).abort = vitest.fn()
+
+		mockTerminal.runCommand.mockReturnValue(longRunningProcess)
+
+		// Execute with timeout
+		const result = await executeCommand(mockTask as Task, options)
+
+		// Should return timeout error
+		expect(result[0]).toBe(false) // Not rejected by user
+		expect(result[1]).toContain("terminated after exceeding")
+		expect(result[1]).toContain("0.1s") // Should show seconds in error message
+	}, 10000) // Increase test timeout to 10 seconds
+
+	it("should abort process on timeout", async () => {
+		const shortTimeoutMs = 50 // Short timeout in milliseconds
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "sleep 10",
+			commandExecutionTimeout: shortTimeoutMs,
+		}
+
+		// Create a process that can be aborted
+		const abortSpy = vitest.fn()
+
+		// Mock the process to never resolve but be abortable
+		const neverResolvingPromise = new Promise(() => {})
+		;(neverResolvingPromise as any).abort = abortSpy
+
+		mockTerminal.runCommand.mockReturnValue(neverResolvingPromise)
+
+		await executeCommand(mockTask as Task, options)
+
+		// Verify abort was called
+		expect(abortSpy).toHaveBeenCalled()
+	}, 5000) // Increase test timeout to 5 seconds
+
+	it("should clean up timeout on successful completion", async () => {
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "echo test",
+			commandExecutionTimeout: 5000,
+		}
+
+		// Mock a process that completes quickly
+		const quickProcess = Promise.resolve()
+		mockTerminal.runCommand.mockReturnValue(quickProcess)
+
+		const result = await executeCommand(mockTask as Task, options)
+
+		// Should complete successfully without timeout
+		expect(result[0]).toBe(false) // Not rejected
+		expect(result[1]).not.toContain("terminated after exceeding")
+	})
+
+	it("should use default timeout when not specified (0 = no timeout)", async () => {
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "echo test",
+			// commandExecutionTimeout not specified, should use default (0)
+		}
+
+		const quickProcess = Promise.resolve()
+		mockTerminal.runCommand.mockReturnValue(quickProcess)
+
+		await executeCommand(mockTask as Task, options)
+
+		// Should complete without issues using default (no timeout)
+		expect(mockTerminal.runCommand).toHaveBeenCalled()
+	})
+
+	it("should not timeout when commandExecutionTimeout is 0", async () => {
+		const options: ExecuteCommandOptions = {
+			executionId: "test-execution",
+			command: "sleep 10",
+			commandExecutionTimeout: 0, // No timeout
+		}
+
+		// Create a process that resolves after a delay to simulate a long-running command
+		const longRunningProcess = new Promise((resolve) => {
+			setTimeout(resolve, 200) // 200ms delay
+		})
+
+		mockTerminal.runCommand.mockReturnValue(longRunningProcess)
+
+		const result = await executeCommand(mockTask as Task, options)
+
+		// Should complete successfully without timeout
+		expect(result[0]).toBe(false) // Not rejected
+		expect(result[1]).not.toContain("terminated after exceeding")
+	})
+})

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

@@ -1,6 +1,7 @@
 // npx vitest run src/core/tools/__tests__/executeCommandTool.spec.ts
 
 import type { ToolUsage } from "@roo-code/types"
+import * as vscode from "vscode"
 
 import { Task } from "../../task/Task"
 import { formatResponse } from "../../prompts/responses"
@@ -12,6 +13,12 @@ vitest.mock("execa", () => ({
 	execa: vitest.fn(),
 }))
 
+vitest.mock("vscode", () => ({
+	workspace: {
+		getConfiguration: vitest.fn(),
+	},
+}))
+
 vitest.mock("../../task/Task")
 vitest.mock("../../prompts/responses")
 
@@ -266,4 +273,40 @@ describe("executeCommandTool", () => {
 			expect(mockExecuteCommand).not.toHaveBeenCalled()
 		})
 	})
+
+	describe("Command execution timeout configuration", () => {
+		it("should include timeout parameter in ExecuteCommandOptions", () => {
+			// This test verifies that the timeout configuration is properly typed
+			// The actual timeout logic is tested in integration tests
+			// Note: timeout is stored internally in milliseconds but configured in seconds
+			const timeoutSeconds = 15
+			const options = {
+				executionId: "test-id",
+				command: "echo test",
+				commandExecutionTimeout: timeoutSeconds * 1000, // Convert to milliseconds
+			}
+
+			// Verify the options object has the expected structure
+			expect(options.commandExecutionTimeout).toBe(15000)
+			expect(typeof options.commandExecutionTimeout).toBe("number")
+		})
+
+		it("should handle timeout parameter in function signature", () => {
+			// Test that the executeCommand function accepts timeout parameter
+			// This is a compile-time check that the types are correct
+			const mockOptions = {
+				executionId: "test-id",
+				command: "echo test",
+				customCwd: undefined,
+				terminalShellIntegrationDisabled: false,
+				terminalOutputLineLimit: 500,
+				commandExecutionTimeout: 0,
+			}
+
+			// Verify all required properties exist
+			expect(mockOptions.executionId).toBeDefined()
+			expect(mockOptions.command).toBeDefined()
+			expect(mockOptions.commandExecutionTimeout).toBeDefined()
+		})
+	})
 })

src/core/tools/executeCommandTool.ts

@@ -1,5 +1,6 @@
 import fs from "fs/promises"
 import * as path from "path"
+import * as vscode from "vscode"
 
 import delay from "delay"
 
@@ -14,6 +15,8 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization"
 import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../../integrations/terminal/types"
 import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
 import { Terminal } from "../../integrations/terminal/Terminal"
+import { Package } from "../../shared/package"
+import { t } from "../../i18n"
 
 class ShellIntegrationError extends Error {}
 
@@ -62,12 +65,21 @@ export async function executeCommandTool(
 			const clineProviderState = await clineProvider?.getState()
 			const { terminalOutputLineLimit = 500, terminalShellIntegrationDisabled = false } = clineProviderState ?? {}
 
+			// Get command execution timeout from VSCode configuration (in seconds)
+			const commandExecutionTimeoutSeconds = vscode.workspace
+				.getConfiguration(Package.name)
+				.get<number>("commandExecutionTimeout", 0)
+
+			// Convert seconds to milliseconds for internal use
+			const commandExecutionTimeout = commandExecutionTimeoutSeconds * 1000
+
 			const options: ExecuteCommandOptions = {
 				executionId,
 				command,
 				customCwd,
 				terminalShellIntegrationDisabled,
 				terminalOutputLineLimit,
+				commandExecutionTimeout,
 			}
 
 			try {
@@ -113,6 +125,7 @@ export type ExecuteCommandOptions = {
 	customCwd?: string
 	terminalShellIntegrationDisabled?: boolean
 	terminalOutputLineLimit?: number
+	commandExecutionTimeout?: number
 }
 
 export async function executeCommand(
@@ -123,8 +136,11 @@ export async function executeCommand(
 		customCwd,
 		terminalShellIntegrationDisabled = false,
 		terminalOutputLineLimit = 500,
+		commandExecutionTimeout = 0,
 	}: ExecuteCommandOptions,
 ): Promise<[boolean, ToolResponse]> {
+	// Convert milliseconds back to seconds for display purposes
+	const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000
 	let workingDir: string
 
 	if (!customCwd) {
@@ -211,8 +227,55 @@ export async function executeCommand(
 	const process = terminal.runCommand(command, callbacks)
 	cline.terminalProcess = process
 
-	await process
-	cline.terminalProcess = undefined
+	// Implement command execution timeout (skip if timeout is 0)
+	if (commandExecutionTimeout > 0) {
+		let timeoutId: NodeJS.Timeout | undefined
+		let isTimedOut = false
+
+		const timeoutPromise = new Promise<void>((_, reject) => {
+			timeoutId = setTimeout(() => {
+				isTimedOut = true
+				// Try to abort the process
+				if (cline.terminalProcess) {
+					cline.terminalProcess.abort()
+				}
+				reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
+			}, commandExecutionTimeout)
+		})
+
+		try {
+			await Promise.race([process, timeoutPromise])
+		} catch (error) {
+			if (isTimedOut) {
+				// Handle timeout case
+				const status: CommandExecutionStatus = { executionId, status: "timeout" }
+				clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
+
+				// Add visual feedback for timeout
+				await cline.say("text", t("common:command_timeout", { seconds: commandExecutionTimeoutSeconds }))
+
+				cline.terminalProcess = undefined
+
+				return [
+					false,
+					`The command was terminated after exceeding a user-configured ${commandExecutionTimeoutSeconds}s timeout. Do not try to re-run the command.`,
+				]
+			}
+			throw error
+		} finally {
+			if (timeoutId) {
+				clearTimeout(timeoutId)
+			}
+			cline.terminalProcess = undefined
+		}
+	} else {
+		// No timeout - just wait for the process to complete
+		try {
+			await process
+		} finally {
+			cline.terminalProcess = undefined
+		}
+	}
 
 	if (shellIntegrationError) {
 		throw new ShellIntegrationError(shellIntegrationError)

src/i18n/locales/ca/common.json

@@ -71,6 +71,7 @@
 		"url_page_not_found": "No s'ha trobat la pàgina. Comprova si la URL és correcta.",
 		"url_fetch_failed": "Error en obtenir el contingut de la URL: {{error}}",
 		"url_fetch_error_with_url": "Error en obtenir contingut per {{url}}: {{error}}",
+		"command_timeout": "L'execució de la comanda ha superat el temps d'espera de {{seconds}} segons",
 		"share_task_failed": "Ha fallat compartir la tasca. Si us plau, torna-ho a provar.",
 		"share_no_active_task": "No hi ha cap tasca activa per compartir",
 		"share_auth_required": "Es requereix autenticació. Si us plau, inicia sessió per compartir tasques.",

src/i18n/locales/de/common.json

@@ -67,6 +67,7 @@
 		"url_page_not_found": "Die Seite wurde nicht gefunden. Bitte prüfe, ob die URL korrekt ist.",
 		"url_fetch_failed": "Fehler beim Abrufen des URL-Inhalts: {{error}}",
 		"url_fetch_error_with_url": "Fehler beim Abrufen des Inhalts für {{url}}: {{error}}",
+		"command_timeout": "Zeitüberschreitung bei der Befehlsausführung nach {{seconds}} Sekunden",
 		"share_task_failed": "Teilen der Aufgabe fehlgeschlagen. Bitte versuche es erneut.",
 		"share_no_active_task": "Keine aktive Aufgabe zum Teilen",
 		"share_auth_required": "Authentifizierung erforderlich. Bitte melde dich an, um Aufgaben zu teilen.",