From 014dfdc465a41968b2afb80fca527158abfc088b Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 20 Feb 2026 18:24:45 +0000 Subject: [PATCH] fix: handle externally-triggered terminal abort to prevent stuck cloud UI When handleTerminalOperation("abort") is called via IPC CancelCommand, the execa subprocess is killed and the process promise rejects. Previously, this rejection was unhandled (isUserTimedOut was false), causing the error to propagate up without emitting any TaskEvent, leaving the cloud UI stuck. Changes: - Add isTerminalAbortedExternally flag to Task class - Set the flag in handleTerminalOperation before calling abort() - Check the flag in executeCommandInTerminal catch block alongside isUserTimedOut - When set, handle cleanly: say error, set didToolFailInCurrentTurn, return tool result - Add "cancelled" status to CommandExecutionStatus discriminated union - Add tests for both the external abort case and the unexpected error case --- packages/types/src/terminal.ts | 4 ++ src/core/task/Task.ts | 2 + src/core/tools/ExecuteCommandTool.ts | 12 ++++ .../tools/__tests__/executeCommand.spec.ts | 57 +++++++++++++++++++ 4 files changed, 75 insertions(+) diff --git a/packages/types/src/terminal.ts b/packages/types/src/terminal.ts index 34f7a74e24..8db7ee6df1 100644 --- a/packages/types/src/terminal.ts +++ b/packages/types/src/terminal.ts @@ -29,6 +29,10 @@ export const commandExecutionStatusSchema = z.discriminatedUnion("status", [ executionId: z.string(), status: z.literal("timeout"), }), + z.object({ + executionId: z.string(), + status: z.literal("cancelled"), + }), ]) export type CommandExecutionStatus = z.infer diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9d19248057..dadb64b339 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -299,6 +299,7 @@ export class Task extends EventEmitter implements TaskLike { rooProtectedController?: RooProtectedController fileContextTracker: FileContextTracker terminalProcess?: RooTerminalProcess + isTerminalAbortedExternally: boolean = false // Editing diffViewProvider: DiffViewProvider @@ -1628,6 +1629,7 @@ export class Task extends EventEmitter implements TaskLike { if (terminalOperation === "continue") { this.terminalProcess?.continue() } else if (terminalOperation === "abort") { + this.isTerminalAbortedExternally = true this.terminalProcess?.abort() } } diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index cb6fc6ff02..d045c415cc 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -368,6 +368,18 @@ export async function executeCommandInTerminal( `The command was terminated after exceeding a user-configured ${commandExecutionTimeoutSeconds}s timeout. Do not try to re-run the command.`, ] } + + if (task.isTerminalAbortedExternally) { + task.isTerminalAbortedExternally = false + const status: CommandExecutionStatus = { executionId, status: "cancelled" } + provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) }) + await task.say("error", "The command was cancelled by the user.") + task.didToolFailInCurrentTurn = true + task.terminalProcess = undefined + + return [false, "The command was cancelled by the user."] + } + throw error } finally { clearTimeout(agentTimeoutId) diff --git a/src/core/tools/__tests__/executeCommand.spec.ts b/src/core/tools/__tests__/executeCommand.spec.ts index fd85beb0f4..13f2baade6 100644 --- a/src/core/tools/__tests__/executeCommand.spec.ts +++ b/src/core/tools/__tests__/executeCommand.spec.ts @@ -395,6 +395,63 @@ describe("executeCommand", () => { }) }) + describe("External Abort Handling", () => { + it("should handle externally-triggered abort cleanly when isTerminalAbortedExternally is set", async () => { + // Setup: Process rejects (simulating SIGKILL from external abort) + const rejectingProcess = Promise.reject(new Error("process was killed")) as any + rejectingProcess.continue = vitest.fn() + rejectingProcess.catch(() => {}) // prevent unhandled rejection + + mockTask.isTerminalAbortedExternally = true + + mockTerminal.runCommand.mockReturnValue(rejectingProcess) + mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project") + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "long-running-command", + terminalShellIntegrationDisabled: true, + } + + // Execute + const [rejected, result] = await executeCommandInTerminal(mockTask, options) + + // Verify: should return a clean tool result, not throw + expect(rejected).toBe(false) + expect(result).toBe("The command was cancelled by the user.") + expect(mockTask.say).toHaveBeenCalledWith("error", "The command was cancelled by the user.") + expect(mockTask.didToolFailInCurrentTurn).toBe(true) + expect(mockTask.terminalProcess).toBeUndefined() + // Verify the flag was reset + expect(mockTask.isTerminalAbortedExternally).toBe(false) + // Verify cancelled status was sent to webview + expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({ + type: "commandExecutionStatus", + text: JSON.stringify({ executionId: "test-123", status: "cancelled" }), + }) + }) + + it("should still throw unexpected errors when isTerminalAbortedExternally is not set", async () => { + // Setup: Process rejects but flag is NOT set (unexpected crash) + const rejectingProcess = Promise.reject(new Error("unexpected crash")) as any + rejectingProcess.continue = vitest.fn() + rejectingProcess.catch(() => {}) // prevent unhandled rejection + + mockTask.isTerminalAbortedExternally = false + + mockTerminal.runCommand.mockReturnValue(rejectingProcess) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "crashing-command", + terminalShellIntegrationDisabled: true, + } + + // Execute: should throw since it's not an external abort + await expect(executeCommandInTerminal(mockTask, options)).rejects.toThrow("unexpected crash") + }) + }) + describe("Terminal Working Directory Updates", () => { it("should update working directory when terminal returns different cwd", async () => { // Setup: Terminal initially at project root, but getCurrentWorkingDirectory returns different path