mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
3b27135cf7
commit
014dfdc465
4 changed files with 75 additions and 0 deletions
|
|
@ -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<typeof commandExecutionStatusSchema>
|
||||
|
|
|
|||
|
|
@ -299,6 +299,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
rooProtectedController?: RooProtectedController
|
||||
fileContextTracker: FileContextTracker
|
||||
terminalProcess?: RooTerminalProcess
|
||||
isTerminalAbortedExternally: boolean = false
|
||||
|
||||
// Editing
|
||||
diffViewProvider: DiffViewProvider
|
||||
|
|
@ -1628,6 +1629,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
if (terminalOperation === "continue") {
|
||||
this.terminalProcess?.continue()
|
||||
} else if (terminalOperation === "abort") {
|
||||
this.isTerminalAbortedExternally = true
|
||||
this.terminalProcess?.abort()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue