Skip to content

Commit d96cd4c

Browse files
fix(terminal): terminate running process when task is cancelled (#245) (#261)
releaseTerminalsForTask only disassociated the terminal (taskId = undefined) without aborting a still-running command, so cancel (✕) left the process orphaned and the terminal stuck "busy" until a manual kill. Now abort the running process for busy terminals on release. Adds TerminalRegistry tests. Co-authored-by: Armando Vaquera <263793884+proyectoauraorg@users.noreply.github.com>
1 parent f8d7b79 commit d96cd4c

3 files changed

Lines changed: 83 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"zoo-code": patch
3+
---
4+
5+
Terminate the running command when a task is cancelled or torn down (#245). Pressing cancel (✕) now aborts the underlying terminal process instead of leaving it running while the terminal stays stuck "busy" until a manual kill.

src/integrations/terminal/TerminalRegistry.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,22 @@ export class TerminalRegistry {
284284
public static releaseTerminalsForTask(taskId: string): void {
285285
this.terminals.forEach((terminal) => {
286286
if (terminal.taskId === taskId) {
287+
// #245: If the terminal is still executing a command when its task is torn
288+
// down (user pressed cancel ✕, or the task was switched/removed), abort the
289+
// process. Otherwise the command keeps running orphaned and the terminal stays
290+
// stuck "busy" — the cancel-doesn't-terminate bug. abort() is safe when idle
291+
// (Ctrl+C is gated on an active stream; Execa abort is idempotent).
292+
if (terminal.busy) {
293+
try {
294+
terminal.process?.abort()
295+
} catch (error) {
296+
console.error(
297+
`[TerminalRegistry] Error aborting process for terminal ${terminal.id} on release:`,
298+
error,
299+
)
300+
}
301+
}
302+
287303
terminal.taskId = undefined
288304
}
289305
})

src/integrations/terminal/__tests__/TerminalRegistry.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,66 @@ describe("TerminalRegistry", () => {
123123
}
124124
})
125125
})
126+
127+
describe("releaseTerminalsForTask", () => {
128+
it("aborts a busy terminal's running process and disassociates it from the task (#245)", () => {
129+
const terminal = TerminalRegistry.createTerminal("/test/path", "vscode")
130+
const abort = vi.fn()
131+
terminal.taskId = "task-245"
132+
terminal.busy = true
133+
terminal.process = { abort } as any
134+
135+
TerminalRegistry.releaseTerminalsForTask("task-245")
136+
137+
expect(abort).toHaveBeenCalledTimes(1)
138+
expect(terminal.taskId).toBeUndefined()
139+
})
140+
141+
it("does not abort an idle (not busy) terminal but still disassociates it", () => {
142+
const terminal = TerminalRegistry.createTerminal("/test/path", "vscode")
143+
const abort = vi.fn()
144+
terminal.taskId = "task-idle"
145+
terminal.busy = false
146+
terminal.process = { abort } as any
147+
148+
TerminalRegistry.releaseTerminalsForTask("task-idle")
149+
150+
expect(abort).not.toHaveBeenCalled()
151+
expect(terminal.taskId).toBeUndefined()
152+
})
153+
154+
it("only releases terminals belonging to the given task", () => {
155+
const a = TerminalRegistry.createTerminal("/a", "vscode")
156+
const b = TerminalRegistry.createTerminal("/b", "vscode")
157+
const abortA = vi.fn()
158+
const abortB = vi.fn()
159+
a.taskId = "task-A"
160+
a.busy = true
161+
a.process = { abort: abortA } as any
162+
b.taskId = "task-B"
163+
b.busy = true
164+
b.process = { abort: abortB } as any
165+
166+
TerminalRegistry.releaseTerminalsForTask("task-A")
167+
168+
expect(abortA).toHaveBeenCalledTimes(1)
169+
expect(a.taskId).toBeUndefined()
170+
expect(abortB).not.toHaveBeenCalled()
171+
expect(b.taskId).toBe("task-B")
172+
})
173+
174+
it("swallows errors thrown by process.abort() and still disassociates the terminal", () => {
175+
const terminal = TerminalRegistry.createTerminal("/test/path", "vscode")
176+
terminal.taskId = "task-throw"
177+
terminal.busy = true
178+
terminal.process = {
179+
abort: vi.fn(() => {
180+
throw new Error("boom")
181+
}),
182+
} as any
183+
184+
expect(() => TerminalRegistry.releaseTerminalsForTask("task-throw")).not.toThrow()
185+
expect(terminal.taskId).toBeUndefined()
186+
})
187+
})
126188
})

0 commit comments

Comments
 (0)