From 22de53d1f7d60de7acab8d43a34e5c15dc7ec878 Mon Sep 17 00:00:00 2001 From: Roomote Date: Mon, 24 Aug 2026 03:56:52 +0000 Subject: [PATCH 01/23] fix(terminal): finalize commands when terminal closes --- src/integrations/terminal/Terminal.ts | 67 +++++++++- src/integrations/terminal/TerminalProcess.ts | 17 +++ src/integrations/terminal/TerminalRegistry.ts | 13 +- .../__tests__/TerminalRegistry.spec.ts | 122 ++++++++++++++++++ 4 files changed, 208 insertions(+), 11 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 21f98b86c6..fc80dd311f 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -11,6 +11,8 @@ import { mergePromise } from "./mergePromise" export class Terminal extends BaseTerminal { public terminal: vscode.Terminal + private closed = false + private cancelShellIntegrationWait?: () => void public cmdCounter: number = 0 @@ -74,7 +76,23 @@ export class Terminal extends BaseTerminal { * active. (This value is set when onDidCloseTerminal is fired.) */ public override isClosed(): boolean { - return this.terminal.exitStatus !== undefined + return this.closed || this.terminal.exitStatus !== undefined + } + + public handleClose(): void { + if (this.closed) { + return + } + + this.closed = true + this.cancelShellIntegrationWait?.() + this.cancelShellIntegrationWait = undefined + + if (this.process instanceof TerminalProcess) { + this.process.handleTerminalClosed() + } else { + this.shellExecutionComplete({ exitCode: undefined }) + } } public override runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise { @@ -123,6 +141,14 @@ export class Terminal extends BaseTerminal { // customised startup that suppresses the OSC 633;A marker). this.waitForShellIntegration(Terminal.getShellIntegrationTimeout()) .then(() => { + if (this.isClosed()) { + if (this.process === process) { + process.handleTerminalClosed() + } + + return + } + // Clean up temporary directory if shell integration is available, zsh did its job: ShellIntegrationManager.zshCleanupTmpDir(this.id) @@ -130,6 +156,14 @@ export class Terminal extends BaseTerminal { void process.run(command).catch((error) => process.emit("error", error)) }) .catch(() => { + if (this.isClosed()) { + if (this.process === process) { + process.handleTerminalClosed() + } + + return + } + console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`) // Clean up temporary directory if shell integration is not available @@ -153,22 +187,43 @@ export class Terminal extends BaseTerminal { * than polling — important for slow-starting shells (heavy .zshrc, nvm, etc.). */ private waitForShellIntegration(timeoutMs: number): Promise { + if (this.isClosed()) { + return Promise.reject(new Error("Terminal closed before shell integration became available")) + } + if (this.terminal.shellIntegration) { return Promise.resolve() } return new Promise((resolve, reject) => { const ref = { disposable: null as vscode.Disposable | null } - const timer = setTimeout(() => { + let settled = false + let cancel = () => {} + const finish = (callback: () => void) => { + if (settled) { + return + } + + settled = true + clearTimeout(timer) ref.disposable?.dispose() - reject(new Error(`Shell integration did not activate within ${timeoutMs / 1000}s`)) + + if (this.cancelShellIntegrationWait === cancel) { + this.cancelShellIntegrationWait = undefined + } + + callback() + } + const timer = setTimeout(() => { + finish(() => reject(new Error(`Shell integration did not activate within ${timeoutMs / 1000}s`))) }, timeoutMs) + cancel = () => finish(() => reject(new Error("Terminal closed before shell integration became available"))) + this.cancelShellIntegrationWait = cancel + ref.disposable = vscode.window.onDidChangeTerminalShellIntegration((e) => { if (e.terminal === this.terminal) { - clearTimeout(timer) - ref.disposable?.dispose() - resolve() + finish(resolve) } }) }) diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index d1643dec3a..991318697c 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -58,6 +58,23 @@ export class TerminalProcess extends BaseTerminalProcess { return terminal } + public handleTerminalClosed(): void { + const executionStarted = this.ownExecution !== undefined + this.terminal.shellExecutionComplete({ exitCode: undefined }) + + if (executionStarted) { + return + } + + // run() has not installed its completion listener yet, so finish the + // startup-wait path directly instead of leaving runCommand() pending. + this.terminal.activeShellExecution = undefined + this.cleanupScriptFile() + this.stopHotTimer() + this.emit("completed", "") + this.emit("continue") + } + public override async run(command: string) { this.command = command diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index da4b3dd16d..d7385af1b5 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.ts @@ -33,13 +33,16 @@ export class TerminalRegistry { // TODO: This initialization code is VSCode specific, and therefore // should probably live elsewhere. - // Register handler for terminal close events to clean up temporary - // directories. + // Treat terminal closure as a completion path because VS Code may not emit + // onDidEndTerminalShellExecution after the terminal is disposed. const closeDisposable = vscode.window.onDidCloseTerminal((vsceTerminal) => { - const terminal = this.getTerminalByVSCETerminal(vsceTerminal) + // Do not use getTerminalByVSCETerminal here: exitStatus is already set when + // this event fires, so that helper removes closed terminals before returning. + const terminal = this.terminals.find((t) => t instanceof Terminal && t.terminal === vsceTerminal) - if (terminal) { - ShellIntegrationManager.zshCleanupTmpDir(terminal.id) + if (terminal instanceof Terminal) { + terminal.handleClose() + this.removeTerminal(terminal.id) } }) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index f60c0d0722..47ea940bec 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -209,6 +209,8 @@ describe("TerminalRegistry", () => { }) describe("onDidEndTerminalShellExecution race condition (#489, #622)", () => { + let closeHandler: (terminal: vscode.Terminal) => void + let shellIntegrationHandler: (event: vscode.TerminalShellIntegrationChangeEvent) => void let startHandler: (e: any) => Promise let endHandler: (e: any) => Promise @@ -221,6 +223,15 @@ describe("TerminalRegistry", () => { ;(vscode.window as any).onDidStartTerminalShellExecution ??= () => ({ dispose: () => {} }) ;(vscode.window as any).onDidEndTerminalShellExecution ??= () => ({ dispose: () => {} }) + vi.spyOn(vscode.window, "onDidCloseTerminal").mockImplementation((handler) => { + closeHandler = handler + return { dispose: vi.fn() } + }) + vi.spyOn(vscode.window, "onDidChangeTerminalShellIntegration").mockImplementation((handler) => { + shellIntegrationHandler = handler + return { dispose: vi.fn() } + }) + vi.spyOn(vscode.window, "onDidStartTerminalShellExecution" as any).mockImplementation((handler: any) => { startHandler = handler return { dispose: vi.fn() } @@ -291,6 +302,117 @@ describe("TerminalRegistry", () => { expect(completeSpy).not.toHaveBeenCalled() }) + it("finalizes an active process when its terminal closes (#1362)", () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const process = new TerminalProcess(terminal) + process.ownExecution = { commandLine: { value: "git status" } } as vscode.TerminalShellExecution + terminal.process = process + terminal.busy = true + terminal.running = true + const completionSpy = vi.fn() + process.on("shell_execution_complete", completionSpy) + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: undefined, reason: 3 }, + configurable: true, + }) + + closeHandler(terminal.terminal) + + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }) + expect(terminal.process).toBeUndefined() + expect(terminal.busy).toBe(false) + expect(terminal.running).toBe(false) + }) + + it("unblocks a process when its terminal closes while shell integration is initializing (#1362)", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const completedSpy = vi.fn() + const completionSpy = vi.fn() + const noShellIntegrationSpy = vi.fn() + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const result = terminal.runCommand("git status", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + onNoShellIntegration: noShellIntegrationSpy, + }) + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: undefined, reason: 3 }, + configurable: true, + }) + + closeHandler(terminal.terminal) + await result + + expect(completionSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledOnce() + expect(noShellIntegrationSpy).not.toHaveBeenCalled() + expect(terminal.process).toBeUndefined() + expect(terminal.busy).toBe(false) + }) + + it("does not submit a command when shell integration resolves immediately before terminal closure", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const executeCommand = vi.fn(() => { + throw new Error("command should not execute after terminal closure") + }) + const completedSpy = vi.fn() + const completionSpy = vi.fn() + const noShellIntegrationSpy = vi.fn() + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const result = terminal.runCommand("git status", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + onNoShellIntegration: noShellIntegrationSpy, + }) + Object.defineProperty(terminal.terminal, "shellIntegration", { + value: { executeCommand }, + configurable: true, + }) + + shellIntegrationHandler({ + terminal: terminal.terminal, + shellIntegration: terminal.terminal.shellIntegration!, + }) + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: undefined, reason: 3 }, + configurable: true, + }) + closeHandler(terminal.terminal) + await result + + expect(executeCommand).not.toHaveBeenCalled() + expect(completionSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledOnce() + expect(noShellIntegrationSpy).not.toHaveBeenCalled() + }) + + it("does not finalize a process twice when its terminal closes after the end event", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const execution = { commandLine: { value: "git status" } } as vscode.TerminalShellExecution + const process = new TerminalProcess(terminal) + process.ownExecution = execution + terminal.process = process + terminal.busy = true + terminal.running = true + const completionSpy = vi.fn() + process.on("shell_execution_complete", completionSpy) + + await endHandler({ terminal: terminal.terminal, execution, exitCode: 0 }) + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: 0, reason: 2 }, + configurable: true, + }) + closeHandler(terminal.terminal) + + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith(expect.objectContaining({ exitCode: 0 })) + }) + it( "ignores a late end event for a superseded execution instead of completing " + "the next command on the same reused terminal", From 246daa1a12616772358d3feac50f11b8dd99aaea Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 5 Sep 2026 00:32:26 +0000 Subject: [PATCH 02/23] test(terminal): model close lifecycle --- docs/architecture/task-lifecycle-model.md | 15 +- package.json | 3 +- scripts/check-terminal-lifecycle.ts | 141 ++++++++++++++++++ .../__tests__/TerminalRegistry.spec.ts | 84 +++++++++++ 4 files changed, 239 insertions(+), 4 deletions(-) create mode 100644 scripts/check-terminal-lifecycle.ts diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 9266d49987..f59285594e 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -6,14 +6,15 @@ Zoo Code checks task lifecycle protocols through one compositional verification pnpm lifecycle:model-check ``` -The command runs six independent bounded submodels in sequence: +The command runs seven independent bounded submodels in sequence: 1. the persisted task delegation lifecycle; 2. shared-store concurrency across task-history hosts; 3. production-backed provider handoff and scheduler ordering; 4. the task cleanup protocol; -5. request-stream parser scoping; and -6. completion persistence. +5. request-stream parser scoping; +6. completion persistence; and +7. the terminal command lifecycle. This umbrella command is the single model-check entry point in the `compile` CI job after type checking. Command-level composition does not merge the submodels' state spaces: each checker retains its own bounds, transitions, invariant ownership, reachability requirements, and counterexample format. In particular, parser state is not part of the persisted lifecycle graph. The focused parser checker remains directly runnable with `pnpm parser-scope:model-check` for debugging. @@ -21,6 +22,8 @@ An individual checker fails if it finds an invariant violation, a modeled action Executable cross-model composition should be added only when a correctness claim genuinely spans two or more submodels and there is an explicit, production-grounded boundary mapping between their events or state. That composition must state a bounded joint exploration strategy and own cross-model invariants that cannot be proved within either child model alone. Shared command orchestration or conceptual adjacency is not sufficient reason to multiply independent state spaces. +`pnpm lifecycle:model` runs the same seven checks directly; `lifecycle:model-check` is the CI-facing alias. + ## Why an executable TypeScript model The models use small explicit-state explorers rather than adding Quint, TLA+/TLC, or Alloy. This is deliberate: @@ -50,6 +53,12 @@ The model has three fixed task slots, enough to cover competing siblings and a n Production completion also accepts a recovery-compatible `active` parent that still awaits the returning child, then clears the stale pointers. Normal model transitions never create that intermediate state, so it is covered by a focused reducer test rather than admitted as a generally valid reachable state. +## Terminal command lifecycle model + +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, and duplicate closure. + +This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. + ## Shared-store concurrency model The same `pnpm lifecycle:model-check` command also runs a second bounded explorer over two `TaskHistoryStore` hosts. It imports the production `computeHistoryDelta` and `mergeHistoryDelta` functions, so its semantics match the store rather than assuming coherent caches or transactional pair writes: diff --git a/package.json b/package.json index 1fd9ddc8fe..d9c252501b 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", - "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts", + "lifecycle:model": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-terminal-lifecycle.ts", + "lifecycle:model-check": "pnpm lifecycle:model", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", "parser-scope:model-check": "node scripts/run-native-tool-call-parser-scoping.mjs", "mcp:integration-check": "tsx scripts/check-mcp-oauth-integration.ts", diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts new file mode 100644 index 0000000000..f9a14afacc --- /dev/null +++ b/scripts/check-terminal-lifecycle.ts @@ -0,0 +1,141 @@ +type Phase = "idle" | "waiting" | "running" | "completed" | "closed" +type Action = "run" | "activate" | "output" | "end" | "close" + +interface ModelState { + phase: Phase + processAttached: boolean + commandSubmitted: boolean + completionCount: number + output: string + deliveredOutput: string + iteratorReleased: boolean +} + +interface TraceStep { + action: Action | "initial" + state: ModelState +} + +const actions: Action[] = ["run", "activate", "output", "end", "close"] +const MAX_DEPTH = 7 +const MAX_STATES = 100 + +function initialState(): ModelState { + return { + phase: "idle", + processAttached: false, + commandSubmitted: false, + completionCount: 0, + output: "", + deliveredOutput: "", + iteratorReleased: false, + } +} + +function complete(state: ModelState, phase: "completed" | "closed"): ModelState { + return { + ...state, + phase, + processAttached: false, + completionCount: state.processAttached ? state.completionCount + 1 : state.completionCount, + deliveredOutput: state.output, + iteratorReleased: state.iteratorReleased || state.phase === "running", + } +} + +function transition(state: ModelState, action: Action): ModelState { + switch (action) { + case "run": + return state.phase === "idle" ? { ...state, phase: "waiting", processAttached: true } : state + case "activate": + return state.phase === "waiting" ? { ...state, phase: "running", commandSubmitted: true } : state + case "output": + return state.phase === "running" ? { ...state, output: `${state.output}chunk` } : state + case "end": + return state.phase === "waiting" || state.phase === "running" ? complete(state, "completed") : state + case "close": + return state.phase === "closed" ? state : complete(state, "closed") + } +} + +function violations(state: ModelState): string[] { + const result: string[] = [] + if (state.completionCount > 1) result.push("a command completed more than once") + if (state.phase === "closed" && state.processAttached) result.push("a closed terminal retained its process") + if (state.phase === "closed" && state.commandSubmitted && !state.iteratorReleased) { + result.push("closing a submitted command did not release its stream iterator") + } + if ((state.phase === "completed" || state.phase === "closed") && state.deliveredOutput !== state.output) { + result.push("completion did not deliver all buffered output") + } + return result +} + +function formatCounterexample(message: string, trace: TraceStep[]): string { + return [ + `Terminal lifecycle invariant failed: ${message}`, + `Bounds: depth=${MAX_DEPTH}, states=${MAX_STATES}`, + ...trace.map((step, index) => `${index}. ${step.action}: ${JSON.stringify(step.state)}`), + ].join("\n") +} + +const landmarks = { + "waiting-close-without-submit": (trace: TraceStep[]) => + trace.some((step) => step.action === "run") && + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.commandSubmitted === false && + trace.at(-1)?.state.completionCount === 1, + "running-close-after-output": (trace: TraceStep[]) => + trace.some((step) => step.action === "output") && + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.deliveredOutput === "chunk" && + trace.at(-1)?.state.iteratorReleased === true, + "end-then-close": (trace: TraceStep[]) => + trace.some((step) => step.action === "end") && + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.completionCount === 1, + "duplicate-close": (trace: TraceStep[]) => trace.filter((step) => step.action === "close").length >= 2, +} satisfies Record boolean> + +const start = initialState() +const queue: Array<{ state: ModelState; trace: TraceStep[] }> = [ + { state: start, trace: [{ action: "initial", state: start }] }, +] +const visited = new Set([JSON.stringify(start)]) +const reachedActions = new Set() +const reachedLandmarks = new Set() + +for (let index = 0; index < queue.length; index++) { + const node = queue[index]! + const stateViolations = violations(node.state) + if (stateViolations.length) throw new Error(formatCounterexample(stateViolations.join("; "), node.trace)) + for (const [name, predicate] of Object.entries(landmarks)) { + if (predicate(node.trace)) reachedLandmarks.add(name) + } + if (node.trace.length - 1 === MAX_DEPTH) continue + + for (const action of actions) { + const next = transition(node.state, action) + const trace = [...node.trace, { action, state: next }] + for (const [name, predicate] of Object.entries(landmarks)) { + if (predicate(trace)) reachedLandmarks.add(name) + } + if (next === node.state) continue + reachedActions.add(action) + const key = JSON.stringify(next) + if (visited.has(key)) continue + visited.add(key) + queue.push({ state: next, trace }) + if (visited.size > MAX_STATES) throw new Error(`Terminal lifecycle exceeded its ${MAX_STATES}-state budget`) + } +} + +const missingActions = actions.filter((action) => !reachedActions.has(action)) +if (missingActions.length) throw new Error(`Terminal lifecycle has unreachable actions: ${missingActions.join(", ")}`) +const missingLandmarks = Object.keys(landmarks).filter((name) => !reachedLandmarks.has(name)) +if (missingLandmarks.length) + throw new Error(`Terminal lifecycle has unreachable landmarks: ${missingLandmarks.join(", ")}`) + +console.log( + `Terminal lifecycle model check passed: ${visited.size} reachable states, ${actions.length}/${actions.length} actions reachable, ${Object.keys(landmarks).length}/${Object.keys(landmarks).length} landmarks reached, depth <= ${MAX_DEPTH}`, +) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 47ea940bec..16e1d44fb2 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -325,6 +325,61 @@ describe("TerminalRegistry", () => { expect(terminal.running).toBe(false) }) + it("delivers buffered output and releases the stream iterator when an active terminal closes", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + let nextCall = 0 + let signalWaitingForNext: () => void = () => {} + const waitingForNext = new Promise((resolve) => { + signalWaitingForNext = resolve + }) + const returnSpy = vi.fn().mockResolvedValue({ done: true, value: undefined }) + const stream: AsyncIterable = { + [Symbol.asyncIterator]() { + return { + next: vi.fn(() => { + nextCall++ + if (nextCall === 1) { + return Promise.resolve({ done: false, value: "\x1b]633;C\x07hello\n" }) + } + + signalWaitingForNext() + return new Promise>(() => {}) + }), + return: returnSpy, + } + }, + } + const execution = { + commandLine: { value: "printf hello" }, + read: vi.fn().mockReturnValue(stream), + } as unknown as vscode.TerminalShellExecution + const executeCommand = vi.fn().mockReturnValue(execution) + Object.defineProperty(terminal.terminal, "shellIntegration", { + value: { executeCommand }, + configurable: true, + }) + const completedSpy = vi.fn() + const result = terminal.runCommand("printf hello", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + }) + + await vi.waitFor(() => expect(executeCommand).toHaveBeenCalledOnce()) + await startHandler({ terminal: terminal.terminal, execution }) + await waitingForNext + closeHandler(terminal.terminal) + await result + + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("hello\n", expect.any(TerminalProcess)) + expect(returnSpy).toHaveBeenCalledOnce() + expect(terminal.process).toBeUndefined() + expect(terminal.busy).toBe(false) + expect(terminal.running).toBe(false) + }) + it("unblocks a process when its terminal closes while shell integration is initializing (#1362)", async () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal const completedSpy = vi.fn() @@ -348,6 +403,7 @@ describe("TerminalRegistry", () => { expect(completionSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) expect(noShellIntegrationSpy).not.toHaveBeenCalled() expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) @@ -388,9 +444,37 @@ describe("TerminalRegistry", () => { expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) expect(noShellIntegrationSpy).not.toHaveBeenCalled() }) + it("marks closure explicitly and completes only once when exitStatus remains undefined", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const completedSpy = vi.fn() + const completionSpy = vi.fn() + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const result = terminal.runCommand("git status", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + }) + + expect(terminal.terminal.exitStatus).toBeUndefined() + terminal.handleClose() + terminal.handleClose() + await result + + expect(terminal.isClosed()).toBe(true) + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, expect.any(TerminalProcess)) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(terminal.process).toBeUndefined() + expect(terminal.busy).toBe(false) + expect(terminal.running).toBe(false) + }) + it("does not finalize a process twice when its terminal closes after the end event", async () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal const execution = { commandLine: { value: "git status" } } as vscode.TerminalShellExecution From f41b4cec7e7c0e08ef96739d2c010a304d1b0a0e Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 5 Sep 2026 01:29:04 +0000 Subject: [PATCH 03/23] test(terminal): cover close lifecycle branches --- src/integrations/terminal/Terminal.ts | 13 +- src/integrations/terminal/TerminalProcess.ts | 3 - .../__tests__/TerminalRegistry.spec.ts | 117 ++++++++++++++++++ 3 files changed, 122 insertions(+), 11 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index fc80dd311f..da84170d21 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -122,6 +122,11 @@ export class Terminal extends BaseTerminal { reject(error) }) + if (this.isClosed()) { + process.handleTerminalClosed() + return + } + if (Terminal.isActiveShellCmdExe()) { // Keep this defensive fallback for callers that invoke Terminal.runCommand() // directly instead of routing through executeCommandInTerminal(). @@ -142,10 +147,6 @@ export class Terminal extends BaseTerminal { this.waitForShellIntegration(Terminal.getShellIntegrationTimeout()) .then(() => { if (this.isClosed()) { - if (this.process === process) { - process.handleTerminalClosed() - } - return } @@ -157,10 +158,6 @@ export class Terminal extends BaseTerminal { }) .catch(() => { if (this.isClosed()) { - if (this.process === process) { - process.handleTerminalClosed() - } - return } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 991318697c..8d310ec5bd 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -68,9 +68,6 @@ export class TerminalProcess extends BaseTerminalProcess { // run() has not installed its completion listener yet, so finish the // startup-wait path directly instead of leaving runCommand() pending. - this.terminal.activeShellExecution = undefined - this.cleanupScriptFile() - this.stopHotTimer() this.emit("completed", "") this.emit("continue") } diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 16e1d44fb2..0d16ed728b 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -320,11 +320,41 @@ describe("TerminalRegistry", () => { expect(completionSpy).toHaveBeenCalledOnce() expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }) + expect(Object.hasOwn(completionSpy.mock.calls[0][0], "exitCode")).toBe(true) expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) expect(terminal.running).toBe(false) }) + it("removes only the closed registered terminal when no process is attached", () => { + const closed = TerminalRegistry.createTerminal("/closed", "vscode") as Terminal + const open = TerminalRegistry.createTerminal("/open", "vscode") as Terminal + closed.busy = true + closed.running = true + const completionSpy = vi.spyOn(closed, "shellExecutionComplete") + + closeHandler(closed.terminal) + + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }) + expect(Object.hasOwn(completionSpy.mock.calls[0][0], "exitCode")).toBe(true) + expect(closed.isClosed()).toBe(true) + expect(closed.busy).toBe(false) + expect(closed.running).toBe(false) + expect(TerminalRegistry.getAllTerminals()).toEqual([open]) + }) + + it("ignores close events from unregistered terminals", () => { + const registered = TerminalRegistry.createTerminal("/registered", "vscode") as Terminal + const foreign = { name: "foreign" } as vscode.Terminal + const closeSpy = vi.spyOn(registered, "handleClose") + + closeHandler(foreign) + + expect(closeSpy).not.toHaveBeenCalled() + expect(TerminalRegistry.getAllTerminals()).toEqual([registered]) + }) + it("delivers buffered output and releases the stream iterator when an active terminal closes", async () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal let nextCall = 0 @@ -475,6 +505,93 @@ describe("TerminalRegistry", () => { expect(terminal.running).toBe(false) }) + it("does not start a command invoked after the terminal has already closed", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const executeCommand = vi.fn() + Object.defineProperty(terminal.terminal, "shellIntegration", { + value: { executeCommand }, + configurable: true, + }) + terminal.handleClose() + const completedSpy = vi.fn() + const completionSpy = vi.fn() + + const result = terminal.runCommand("git status", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + }) + await result + + expect(executeCommand).not.toHaveBeenCalled() + expect(completionSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(terminal.busy).toBe(false) + }) + + it("settles a shell-integration wait once and ignores unrelated terminal events", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + let waitHandler: (event: vscode.TerminalShellIntegrationChangeEvent) => void = () => {} + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce((handler) => { + waitHandler = handler + return { dispose: disposeSpy } + }) + const wait = terminal["waitForShellIntegration"](100) + const settledSpy = vi.fn() + void wait.then(settledSpy) + + waitHandler({ terminal: { name: "foreign" } as vscode.Terminal, shellIntegration: {} as never }) + await Promise.resolve() + expect(settledSpy).not.toHaveBeenCalled() + + const event = { terminal: terminal.terminal, shellIntegration: {} as never } + waitHandler(event) + waitHandler(event) + await wait + + expect(settledSpy).toHaveBeenCalledOnce() + expect(disposeSpy).toHaveBeenCalledOnce() + expect(terminal["cancelShellIntegrationWait"]).toBeUndefined() + }) + + it("does not let an older shell-integration wait clear a newer cancellation", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const handlers: Array<(event: vscode.TerminalShellIntegrationChangeEvent) => void> = [] + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementation((handler) => { + handlers.push(handler) + return { dispose: vi.fn() } + }) + const first = terminal["waitForShellIntegration"](100) + const firstCancel = terminal["cancelShellIntegrationWait"] + const second = terminal["waitForShellIntegration"](100) + const secondCancel = terminal["cancelShellIntegrationWait"] + + expect(firstCancel).not.toBe(secondCancel) + handlers[0]({ terminal: terminal.terminal, shellIntegration: {} as never }) + await first + expect(terminal["cancelShellIntegrationWait"]).toBe(secondCancel) + + handlers[1]({ terminal: terminal.terminal, shellIntegration: {} as never }) + await second + expect(terminal["cancelShellIntegrationWait"]).toBeUndefined() + }) + + it("uses the native exit status to recognize closure before the close event is handled", () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + expect(terminal.isClosed()).toBe(false) + + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: 0, reason: 2 }, + configurable: true, + }) + + expect(terminal.isClosed()).toBe(true) + }) + it("does not finalize a process twice when its terminal closes after the end event", async () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal const execution = { commandLine: { value: "git status" } } as vscode.TerminalShellExecution From 20dfbd9d3b6e2352d348bc04c4a531d23c9a9b42 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 5 Sep 2026 01:34:40 +0000 Subject: [PATCH 04/23] test(terminal): verify wait cleanup edges --- .../__tests__/TerminalRegistry.spec.ts | 101 ++++++++++++++++-- 1 file changed, 92 insertions(+), 9 deletions(-) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 0d16ed728b..9bc8f4fa5b 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -11,6 +11,13 @@ import { TerminalRegistry } from "../TerminalRegistry" const PAGER = process.platform === "win32" ? "" : "cat" +function settleWithin(promise: PromiseLike): Promise { + return Promise.race([ + Promise.resolve(promise), + new Promise((_, reject) => setTimeout(() => reject(new Error("terminal lifecycle did not settle")), 250)), + ]) +} + vi.mock("execa", () => ({ execa: vi.fn(), })) @@ -332,6 +339,7 @@ describe("TerminalRegistry", () => { closed.busy = true closed.running = true const completionSpy = vi.spyOn(closed, "shellExecutionComplete") + const cleanupSpy = vi.spyOn(ShellIntegrationManager, "zshCleanupTmpDir") closeHandler(closed.terminal) @@ -341,7 +349,8 @@ describe("TerminalRegistry", () => { expect(closed.isClosed()).toBe(true) expect(closed.busy).toBe(false) expect(closed.running).toBe(false) - expect(TerminalRegistry.getAllTerminals()).toEqual([open]) + expect(cleanupSpy).toHaveBeenCalledWith(closed.id) + expect(TerminalRegistry["terminals"]).toEqual([open]) }) it("ignores close events from unregistered terminals", () => { @@ -400,7 +409,7 @@ describe("TerminalRegistry", () => { await startHandler({ terminal: terminal.terminal, execution }) await waitingForNext closeHandler(terminal.terminal) - await result + await settleWithin(result) expect(completedSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledWith("hello\n", expect.any(TerminalProcess)) @@ -429,7 +438,7 @@ describe("TerminalRegistry", () => { }) closeHandler(terminal.terminal) - await result + await settleWithin(result) expect(completionSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() @@ -469,7 +478,7 @@ describe("TerminalRegistry", () => { configurable: true, }) closeHandler(terminal.terminal) - await result + await settleWithin(result) expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() @@ -491,9 +500,10 @@ describe("TerminalRegistry", () => { }) expect(terminal.terminal.exitStatus).toBeUndefined() + const shellCompleteSpy = vi.spyOn(terminal, "shellExecutionComplete") terminal.handleClose() terminal.handleClose() - await result + await settleWithin(result) expect(terminal.isClosed()).toBe(true) expect(completionSpy).toHaveBeenCalledOnce() @@ -503,6 +513,7 @@ describe("TerminalRegistry", () => { expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) expect(terminal.running).toBe(false) + expect(shellCompleteSpy).toHaveBeenCalledOnce() }) it("does not start a command invoked after the terminal has already closed", async () => { @@ -522,7 +533,7 @@ describe("TerminalRegistry", () => { onShellExecutionStarted: vi.fn(), onShellExecutionComplete: completionSpy, }) - await result + await settleWithin(result) expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() @@ -550,7 +561,7 @@ describe("TerminalRegistry", () => { const event = { terminal: terminal.terminal, shellIntegration: {} as never } waitHandler(event) waitHandler(event) - await wait + await settleWithin(wait) expect(settledSpy).toHaveBeenCalledOnce() expect(disposeSpy).toHaveBeenCalledOnce() @@ -572,14 +583,86 @@ describe("TerminalRegistry", () => { expect(firstCancel).not.toBe(secondCancel) handlers[0]({ terminal: terminal.terminal, shellIntegration: {} as never }) - await first + await settleWithin(first) expect(terminal["cancelShellIntegrationWait"]).toBe(secondCancel) handlers[1]({ terminal: terminal.terminal, shellIntegration: {} as never }) - await second + await settleWithin(second) expect(terminal["cancelShellIntegrationWait"]).toBeUndefined() }) + it("rejects a direct shell-integration wait when the terminal is already closed", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + terminal.handleClose() + + await expect(terminal["waitForShellIntegration"](100)).rejects.toThrow( + "Terminal closed before shell integration became available", + ) + }) + + it("clears the timeout and disposes the listener when shell integration activates", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + let waitHandler: (event: vscode.TerminalShellIntegrationChangeEvent) => void = () => {} + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce((handler) => { + waitHandler = handler + return { dispose: disposeSpy } + }) + const wait = terminal["waitForShellIntegration"](1_000) + + waitHandler({ terminal: terminal.terminal, shellIntegration: {} as never }) + await wait + + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + + it("reports the configured timeout and releases wait resources", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce(() => ({ + dispose: disposeSpy, + })) + const rejectedSpy = vi.fn() + void terminal["waitForShellIntegration"](1_500).catch(rejectedSpy) + + await vi.advanceTimersByTimeAsync(1_500) + + expect(rejectedSpy).toHaveBeenCalledOnce() + expect(rejectedSpy.mock.calls[0][0]).toEqual(new Error("Shell integration did not activate within 1.5s")) + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + + it("cancels a pending shell-integration wait with the terminal-close reason", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce(() => ({ + dispose: disposeSpy, + })) + const rejectedSpy = vi.fn() + void terminal["waitForShellIntegration"](1_000).catch(rejectedSpy) + + terminal["cancelShellIntegrationWait"]?.() + await Promise.resolve() + + expect(rejectedSpy).toHaveBeenCalledOnce() + expect(rejectedSpy.mock.calls[0][0]).toEqual( + new Error("Terminal closed before shell integration became available"), + ) + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + it("uses the native exit status to recognize closure before the close event is handled", () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal expect(terminal.isClosed()).toBe(false) From ad1e7bf18a9665c410e55ad52e8a40bf947dd9e6 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 5 Sep 2026 01:38:53 +0000 Subject: [PATCH 05/23] test(terminal): keep registry assertion typed --- src/integrations/terminal/__tests__/TerminalRegistry.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 9bc8f4fa5b..6926fa1768 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -361,7 +361,7 @@ describe("TerminalRegistry", () => { closeHandler(foreign) expect(closeSpy).not.toHaveBeenCalled() - expect(TerminalRegistry.getAllTerminals()).toEqual([registered]) + expect(TerminalRegistry["terminals"]).toEqual([registered]) }) it("delivers buffered output and releases the stream iterator when an active terminal closes", async () => { From f20c82087d0a212feb1117ade9d6135eab504cac Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 5 Sep 2026 01:48:37 +0000 Subject: [PATCH 06/23] test(terminal): assert exact process identity --- src/integrations/terminal/Terminal.ts | 1 + src/integrations/terminal/TerminalProcess.ts | 1 + .../__tests__/TerminalRegistry.spec.ts | 23 ++++++++++++++----- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index da84170d21..de68c4284c 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -79,6 +79,7 @@ export class Terminal extends BaseTerminal { return this.closed || this.terminal.exitStatus !== undefined } + /** Finalizes any attached command when VS Code disposes this terminal. */ public handleClose(): void { if (this.closed) { return diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 8d310ec5bd..c32805b53d 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -58,6 +58,7 @@ export class TerminalProcess extends BaseTerminalProcess { return terminal } + /** Completes this process when its terminal closes without an execution-end event. */ public handleTerminalClosed(): void { const executionStarted = this.ownExecution !== undefined this.terminal.shellExecutionComplete({ exitCode: undefined }) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 6926fa1768..36a0468c54 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -404,6 +404,8 @@ describe("TerminalRegistry", () => { onShellExecutionStarted: vi.fn(), onShellExecutionComplete: vi.fn(), }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) await vi.waitFor(() => expect(executeCommand).toHaveBeenCalledOnce()) await startHandler({ terminal: terminal.terminal, execution }) @@ -412,7 +414,7 @@ describe("TerminalRegistry", () => { await settleWithin(result) expect(completedSpy).toHaveBeenCalledOnce() - expect(completedSpy).toHaveBeenCalledWith("hello\n", expect.any(TerminalProcess)) + expect(completedSpy).toHaveBeenCalledWith("hello\n", process) expect(returnSpy).toHaveBeenCalledOnce() expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) @@ -432,6 +434,8 @@ describe("TerminalRegistry", () => { onShellExecutionComplete: completionSpy, onNoShellIntegration: noShellIntegrationSpy, }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) Object.defineProperty(terminal.terminal, "exitStatus", { value: { code: undefined, reason: 3 }, configurable: true, @@ -442,7 +446,7 @@ describe("TerminalRegistry", () => { expect(completionSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() - expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(completedSpy).toHaveBeenCalledWith("", process) expect(noShellIntegrationSpy).not.toHaveBeenCalled() expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) @@ -464,6 +468,8 @@ describe("TerminalRegistry", () => { onShellExecutionComplete: completionSpy, onNoShellIntegration: noShellIntegrationSpy, }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) Object.defineProperty(terminal.terminal, "shellIntegration", { value: { executeCommand }, configurable: true, @@ -483,7 +489,7 @@ describe("TerminalRegistry", () => { expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() - expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(completedSpy).toHaveBeenCalledWith("", process) expect(noShellIntegrationSpy).not.toHaveBeenCalled() }) @@ -498,6 +504,8 @@ describe("TerminalRegistry", () => { onShellExecutionStarted: vi.fn(), onShellExecutionComplete: completionSpy, }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) expect(terminal.terminal.exitStatus).toBeUndefined() const shellCompleteSpy = vi.spyOn(terminal, "shellExecutionComplete") @@ -507,9 +515,9 @@ describe("TerminalRegistry", () => { expect(terminal.isClosed()).toBe(true) expect(completionSpy).toHaveBeenCalledOnce() - expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, expect.any(TerminalProcess)) + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) expect(completedSpy).toHaveBeenCalledOnce() - expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(completedSpy).toHaveBeenCalledWith("", process) expect(terminal.process).toBeUndefined() expect(terminal.busy).toBe(false) expect(terminal.running).toBe(false) @@ -534,10 +542,13 @@ describe("TerminalRegistry", () => { onShellExecutionComplete: completionSpy, }) await settleWithin(result) + const completedProcess = completedSpy.mock.calls[0][1] expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() - expect(completedSpy).toHaveBeenCalledWith("", expect.any(TerminalProcess)) + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, completedProcess) + expect(completedSpy).toHaveBeenCalledWith("", completedProcess) + expect(completedProcess).toBeInstanceOf(TerminalProcess) expect(terminal.busy).toBe(false) }) From 1aedb7168dfd7bfd2a11b46f36c386100b62096f Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 10 Sep 2026 00:20:56 +0000 Subject: [PATCH 07/23] test(terminal): tighten close regression cleanup --- src/integrations/terminal/Terminal.ts | 3 ++- .../terminal/__tests__/TerminalRegistry.spec.ts | 13 ++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index de68c4284c..466cc187ed 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -124,7 +124,8 @@ export class Terminal extends BaseTerminal { }) if (this.isClosed()) { - process.handleTerminalClosed() + // Keep the newly created process observable to the caller until runCommand returns. + queueMicrotask(() => process.handleTerminalClosed()) return } diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 36a0468c54..397dc8af02 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -255,6 +255,7 @@ describe("TerminalRegistry", () => { afterEach(() => { // Reset so other test blocks aren't affected. TerminalRegistry["isInitialized"] = false + vi.useRealTimers() }) it("calls shellExecutionComplete when end event fires before running is set (race)", async () => { @@ -541,14 +542,15 @@ describe("TerminalRegistry", () => { onShellExecutionStarted: vi.fn(), onShellExecutionComplete: completionSpy, }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) await settleWithin(result) - const completedProcess = completedSpy.mock.calls[0][1] expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() - expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, completedProcess) - expect(completedSpy).toHaveBeenCalledWith("", completedProcess) - expect(completedProcess).toBeInstanceOf(TerminalProcess) + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) expect(terminal.busy).toBe(false) }) @@ -628,7 +630,6 @@ describe("TerminalRegistry", () => { expect(disposeSpy).toHaveBeenCalledOnce() expect(vi.getTimerCount()).toBe(0) - vi.useRealTimers() }) it("reports the configured timeout and releases wait resources", async () => { @@ -648,7 +649,6 @@ describe("TerminalRegistry", () => { expect(rejectedSpy.mock.calls[0][0]).toEqual(new Error("Shell integration did not activate within 1.5s")) expect(disposeSpy).toHaveBeenCalledOnce() expect(vi.getTimerCount()).toBe(0) - vi.useRealTimers() }) it("cancels a pending shell-integration wait with the terminal-close reason", async () => { @@ -671,7 +671,6 @@ describe("TerminalRegistry", () => { ) expect(disposeSpy).toHaveBeenCalledOnce() expect(vi.getTimerCount()).toBe(0) - vi.useRealTimers() }) it("uses the native exit status to recognize closure before the close event is handled", () => { From dd35105a0ac96621d05b25c237fa3a2dcb8ac6b1 Mon Sep 17 00:00:00 2001 From: Roomote Date: Thu, 10 Sep 2026 22:52:09 +0000 Subject: [PATCH 08/23] refactor(ci): consolidate lifecycle model command --- docs/architecture/task-lifecycle-model.md | 2 -- package.json | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index f59285594e..f28d7abfe6 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -22,8 +22,6 @@ An individual checker fails if it finds an invariant violation, a modeled action Executable cross-model composition should be added only when a correctness claim genuinely spans two or more submodels and there is an explicit, production-grounded boundary mapping between their events or state. That composition must state a bounded joint exploration strategy and own cross-model invariants that cannot be proved within either child model alone. Shared command orchestration or conceptual adjacency is not sufficient reason to multiply independent state spaces. -`pnpm lifecycle:model` runs the same seven checks directly; `lifecycle:model-check` is the CI-facing alias. - ## Why an executable TypeScript model The models use small explicit-state explorers rather than adding Quint, TLA+/TLC, or Alloy. This is deliberate: diff --git a/package.json b/package.json index d9c252501b..c830f49e2f 100644 --- a/package.json +++ b/package.json @@ -13,8 +13,7 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", - "lifecycle:model": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-terminal-lifecycle.ts", - "lifecycle:model-check": "pnpm lifecycle:model", + "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-terminal-lifecycle.ts", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", "parser-scope:model-check": "node scripts/run-native-tool-call-parser-scoping.mjs", "mcp:integration-check": "tsx scripts/check-mcp-oauth-integration.ts", From a52025a766100581573e297f45e15f049719f09d Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 01:18:40 +0000 Subject: [PATCH 09/23] fix(terminal): settle all startup waits on close --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 39 ++++- src/integrations/terminal/Terminal.ts | 16 +- src/integrations/terminal/TerminalProcess.ts | 44 ++++-- .../__tests__/TerminalRegistry.spec.ts | 143 +++++++++++++++++- 5 files changed, 211 insertions(+), 33 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index f28d7abfe6..cf775b3599 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, and duplicate closure. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, concurrent shell-integration waits, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the process and settle every pending wait, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, and closure with two pending waits. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index f9a14afacc..358897f609 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -1,5 +1,5 @@ type Phase = "idle" | "waiting" | "running" | "completed" | "closed" -type Action = "run" | "activate" | "output" | "end" | "close" +type Action = "run" | "wait" | "activate" | "output" | "end" | "close" interface ModelState { phase: Phase @@ -9,6 +9,9 @@ interface ModelState { output: string deliveredOutput: string iteratorReleased: boolean + waitsCreated: number + pendingWaits: number + settledWaits: number } interface TraceStep { @@ -16,7 +19,7 @@ interface TraceStep { state: ModelState } -const actions: Action[] = ["run", "activate", "output", "end", "close"] +const actions: Action[] = ["run", "wait", "activate", "output", "end", "close"] const MAX_DEPTH = 7 const MAX_STATES = 100 @@ -29,6 +32,9 @@ function initialState(): ModelState { output: "", deliveredOutput: "", iteratorReleased: false, + waitsCreated: 0, + pendingWaits: 0, + settledWaits: 0, } } @@ -47,14 +53,32 @@ function transition(state: ModelState, action: Action): ModelState { switch (action) { case "run": return state.phase === "idle" ? { ...state, phase: "waiting", processAttached: true } : state + case "wait": + return state.phase !== "closed" && state.waitsCreated < 2 + ? { ...state, waitsCreated: state.waitsCreated + 1, pendingWaits: state.pendingWaits + 1 } + : state case "activate": - return state.phase === "waiting" ? { ...state, phase: "running", commandSubmitted: true } : state + return state.phase === "waiting" + ? { + ...state, + phase: "running", + commandSubmitted: true, + pendingWaits: 0, + settledWaits: state.settledWaits + state.pendingWaits, + } + : state case "output": return state.phase === "running" ? { ...state, output: `${state.output}chunk` } : state case "end": return state.phase === "waiting" || state.phase === "running" ? complete(state, "completed") : state case "close": - return state.phase === "closed" ? state : complete(state, "closed") + return state.phase === "closed" + ? state + : { + ...complete(state, "closed"), + pendingWaits: 0, + settledWaits: state.settledWaits + state.pendingWaits, + } } } @@ -62,6 +86,8 @@ function violations(state: ModelState): string[] { const result: string[] = [] if (state.completionCount > 1) result.push("a command completed more than once") if (state.phase === "closed" && state.processAttached) result.push("a closed terminal retained its process") + if (state.phase === "closed" && state.pendingWaits !== 0) result.push("a closed terminal retained pending waits") + if (state.settledWaits > state.waitsCreated) result.push("more waits settled than were created") if (state.phase === "closed" && state.commandSubmitted && !state.iteratorReleased) { result.push("closing a submitted command did not release its stream iterator") } @@ -95,6 +121,11 @@ const landmarks = { trace.at(-1)?.action === "close" && trace.at(-1)?.state.completionCount === 1, "duplicate-close": (trace: TraceStep[]) => trace.filter((step) => step.action === "close").length >= 2, + "concurrent-waits-close": (trace: TraceStep[]) => + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.waitsCreated === 2 && + trace.at(-1)?.state.pendingWaits === 0 && + trace.at(-1)?.state.settledWaits === 2, } satisfies Record boolean> const start = initialState() diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 466cc187ed..0d1e6a21dc 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -12,7 +12,7 @@ import { mergePromise } from "./mergePromise" export class Terminal extends BaseTerminal { public terminal: vscode.Terminal private closed = false - private cancelShellIntegrationWait?: () => void + private cancelShellIntegrationWaits = new Set<() => void>() public cmdCounter: number = 0 @@ -86,8 +86,10 @@ export class Terminal extends BaseTerminal { } this.closed = true - this.cancelShellIntegrationWait?.() - this.cancelShellIntegrationWait = undefined + for (const cancel of this.cancelShellIntegrationWaits) { + cancel() + } + this.cancelShellIntegrationWaits.clear() if (this.process instanceof TerminalProcess) { this.process.handleTerminalClosed() @@ -149,6 +151,7 @@ export class Terminal extends BaseTerminal { this.waitForShellIntegration(Terminal.getShellIntegrationTimeout()) .then(() => { if (this.isClosed()) { + process.handleTerminalClosed() return } @@ -160,6 +163,7 @@ export class Terminal extends BaseTerminal { }) .catch(() => { if (this.isClosed()) { + process.handleTerminalClosed() return } @@ -207,9 +211,7 @@ export class Terminal extends BaseTerminal { clearTimeout(timer) ref.disposable?.dispose() - if (this.cancelShellIntegrationWait === cancel) { - this.cancelShellIntegrationWait = undefined - } + this.cancelShellIntegrationWaits.delete(cancel) callback() } @@ -218,7 +220,7 @@ export class Terminal extends BaseTerminal { }, timeoutMs) cancel = () => finish(() => reject(new Error("Terminal closed before shell integration became available"))) - this.cancelShellIntegrationWait = cancel + this.cancelShellIntegrationWaits.add(cancel) ref.disposable = vscode.window.onDidChangeTerminalShellIntegration((e) => { if (e.terminal === this.terminal) { diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index c32805b53d..f0c8519b54 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -30,6 +30,8 @@ export class TerminalProcess extends BaseTerminalProcess { // whatever command is currently running on the same reused terminal -- see the // self-finalize grace period in run()'s finalize(). public ownExecution?: vscode.TerminalShellExecution + private terminalCloseHandled = false + private finalizedBeforeExecution = false constructor(terminal: Terminal) { super() @@ -41,10 +43,7 @@ export class TerminalProcess extends BaseTerminalProcess { }) this.once("no_shell_integration", () => { - this.emit("completed", "") - this.terminal.busy = false - this.terminal.setActiveStream(undefined) - this.continue() + this.completeBeforeExecution("") }) } @@ -60,8 +59,17 @@ export class TerminalProcess extends BaseTerminalProcess { /** Completes this process when its terminal closes without an execution-end event. */ public handleTerminalClosed(): void { + if (this.terminalCloseHandled || this.finalizedBeforeExecution) { + return + } + this.terminalCloseHandled = true + const executionStarted = this.ownExecution !== undefined - this.terminal.shellExecutionComplete({ exitCode: undefined }) + if (this.terminal.process === this) { + this.terminal.shellExecutionComplete({ exitCode: undefined }) + } else { + this.emit("shell_execution_complete", { exitCode: undefined }) + } if (executionStarted) { return @@ -69,8 +77,23 @@ export class TerminalProcess extends BaseTerminalProcess { // run() has not installed its completion listener yet, so finish the // startup-wait path directly instead of leaving runCommand() pending. - this.emit("completed", "") - this.emit("continue") + this.completeBeforeExecution("") + } + + private completeBeforeExecution(output: string): void { + this.finalizedBeforeExecution = true + + const terminal = this.terminal + terminal.busy = false + terminal.running = false + terminal.activeShellExecution = undefined + terminal.setActiveStream(undefined) + if (terminal.process === this) { + terminal.process = undefined + } + this.emit("completed", output) + this.continue() + this.removeAllListeners() } public override async run(command: string) { @@ -91,13 +114,6 @@ export class TerminalProcess extends BaseTerminalProcess { message: "Command was submitted; output is not available, as shell integration is inactive.", commandSubmitted: true, }) - - this.emit( - "completed", - "", - ) - - this.emit("continue") return } diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 397dc8af02..3c3e2f9f2e 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -578,7 +578,7 @@ describe("TerminalRegistry", () => { expect(settledSpy).toHaveBeenCalledOnce() expect(disposeSpy).toHaveBeenCalledOnce() - expect(terminal["cancelShellIntegrationWait"]).toBeUndefined() + expect(terminal["cancelShellIntegrationWaits"].size).toBe(0) }) it("does not let an older shell-integration wait clear a newer cancellation", async () => { @@ -590,18 +590,16 @@ describe("TerminalRegistry", () => { return { dispose: vi.fn() } }) const first = terminal["waitForShellIntegration"](100) - const firstCancel = terminal["cancelShellIntegrationWait"] const second = terminal["waitForShellIntegration"](100) - const secondCancel = terminal["cancelShellIntegrationWait"] - expect(firstCancel).not.toBe(secondCancel) + expect(terminal["cancelShellIntegrationWaits"].size).toBe(2) handlers[0]({ terminal: terminal.terminal, shellIntegration: {} as never }) await settleWithin(first) - expect(terminal["cancelShellIntegrationWait"]).toBe(secondCancel) + expect(terminal["cancelShellIntegrationWaits"].size).toBe(1) handlers[1]({ terminal: terminal.terminal, shellIntegration: {} as never }) await settleWithin(second) - expect(terminal["cancelShellIntegrationWait"]).toBeUndefined() + expect(terminal["cancelShellIntegrationWaits"].size).toBe(0) }) it("rejects a direct shell-integration wait when the terminal is already closed", async () => { @@ -613,6 +611,136 @@ describe("TerminalRegistry", () => { ) }) + it("releases all startup resources when handleClose interrupts runCommand", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce(() => ({ + dispose: disposeSpy, + })) + const result = terminal.runCommand("git status", { + onLine: vi.fn(), + onCompleted: vi.fn(), + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + onNoShellIntegration: vi.fn(), + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + + terminal.handleClose() + await result + + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + for (const event of [ + "line", + "completed", + "continue", + "error", + "shell_execution_started", + "shell_execution_complete", + "no_shell_integration", + ]) { + expect(process?.listenerCount(event), event).toBe(0) + } + }) + + it("cancels every concurrent shell-integration wait when the terminal closes", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpies = [vi.fn(), vi.fn()] + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration) + .mockImplementationOnce(() => ({ dispose: disposeSpies[0] })) + .mockImplementationOnce(() => ({ dispose: disposeSpies[1] })) + const rejectedSpies = [vi.fn(), vi.fn()] + void terminal["waitForShellIntegration"](1_000).catch(rejectedSpies[0]) + void terminal["waitForShellIntegration"](1_000).catch(rejectedSpies[1]) + + terminal.handleClose() + await Promise.resolve() + + expect(rejectedSpies[0]).toHaveBeenCalledOnce() + expect(rejectedSpies[1]).toHaveBeenCalledOnce() + expect(disposeSpies[0]).toHaveBeenCalledOnce() + expect(disposeSpies[1]).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + }) + + it("settles every concurrent runCommand startup when the terminal closes", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpies = [vi.fn(), vi.fn()] + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration) + .mockImplementationOnce(() => ({ dispose: disposeSpies[0] })) + .mockImplementationOnce(() => ({ dispose: disposeSpies[1] })) + const completedSpies = [vi.fn(), vi.fn()] + const completionSpies = [vi.fn(), vi.fn()] + const results = [ + terminal.runCommand("first", { + onLine: vi.fn(), + onCompleted: completedSpies[0], + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpies[0], + }), + terminal.runCommand("second", { + onLine: vi.fn(), + onCompleted: completedSpies[1], + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpies[1], + }), + ] + const settledSpies = [vi.fn(), vi.fn()] + void results[0].then(settledSpies[0]) + void results[1].then(settledSpies[1]) + + terminal.handleClose() + await Promise.all(results) + + expect(settledSpies[0]).toHaveBeenCalledOnce() + expect(settledSpies[1]).toHaveBeenCalledOnce() + expect(completedSpies[0]).toHaveBeenCalledOnce() + expect(completedSpies[1]).toHaveBeenCalledOnce() + expect(completionSpies[0]).toHaveBeenCalledOnce() + expect(completionSpies[1]).toHaveBeenCalledOnce() + expect(disposeSpies[0]).toHaveBeenCalledOnce() + expect(disposeSpies[1]).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + }) + + it("does not complete again when a no-shell process is followed by terminal closure", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const process = new TerminalProcess(terminal) + terminal.process = process + terminal.busy = true + const noShellSpy = vi.fn() + const completedSpy = vi.fn() + const continueSpy = vi.fn() + const shellCompleteSpy = vi.fn() + process.on("no_shell_integration", noShellSpy) + process.on("completed", completedSpy) + process.on("continue", continueSpy) + process.on("shell_execution_complete", shellCompleteSpy) + + await process.run("git status") + + expect(noShellSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledOnce() + expect(continueSpy).toHaveBeenCalledOnce() + expect(terminal.process).toBeUndefined() + expect(process.eventNames()).toEqual([]) + + terminal.handleClose() + + expect(shellCompleteSpy).not.toHaveBeenCalled() + expect(completedSpy).toHaveBeenCalledOnce() + expect(continueSpy).toHaveBeenCalledOnce() + }) + it("clears the timeout and disposes the listener when shell integration activates", async () => { vi.useFakeTimers() const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal @@ -662,7 +790,8 @@ describe("TerminalRegistry", () => { const rejectedSpy = vi.fn() void terminal["waitForShellIntegration"](1_000).catch(rejectedSpy) - terminal["cancelShellIntegrationWait"]?.() + const [cancel] = terminal["cancelShellIntegrationWaits"] + cancel?.() await Promise.resolve() expect(rejectedSpy).toHaveBeenCalledOnce() From 1297d818e2aa0732e9d6138800a4c36a674e63b3 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 01:33:31 +0000 Subject: [PATCH 10/23] test(terminal): cover close cleanup invariants --- src/integrations/terminal/Terminal.ts | 1 - .../__tests__/TerminalRegistry.spec.ts | 111 +++++++++++++++--- 2 files changed, 95 insertions(+), 17 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 0d1e6a21dc..bf9dcbda6f 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -89,7 +89,6 @@ export class Terminal extends BaseTerminal { for (const cancel of this.cancelShellIntegrationWaits) { cancel() } - this.cancelShellIntegrationWaits.clear() if (this.process instanceof TerminalProcess) { this.process.handleTerminalClosed() diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 3c3e2f9f2e..9f07e8e9bd 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -616,20 +616,26 @@ describe("TerminalRegistry", () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) const disposeSpy = vi.fn() + const completedSpy = vi.fn() + const completionSpy = vi.fn() vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce(() => ({ dispose: disposeSpy, })) const result = terminal.runCommand("git status", { onLine: vi.fn(), - onCompleted: vi.fn(), + onCompleted: completedSpy, onShellExecutionStarted: vi.fn(), - onShellExecutionComplete: vi.fn(), + onShellExecutionComplete: completionSpy, onNoShellIntegration: vi.fn(), }) const process = terminal.process expect(process).toBeInstanceOf(TerminalProcess) terminal.handleClose() + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) await result expect(disposeSpy).toHaveBeenCalledOnce() @@ -679,20 +685,21 @@ describe("TerminalRegistry", () => { .mockImplementationOnce(() => ({ dispose: disposeSpies[1] })) const completedSpies = [vi.fn(), vi.fn()] const completionSpies = [vi.fn(), vi.fn()] - const results = [ - terminal.runCommand("first", { - onLine: vi.fn(), - onCompleted: completedSpies[0], - onShellExecutionStarted: vi.fn(), - onShellExecutionComplete: completionSpies[0], - }), - terminal.runCommand("second", { - onLine: vi.fn(), - onCompleted: completedSpies[1], - onShellExecutionStarted: vi.fn(), - onShellExecutionComplete: completionSpies[1], - }), - ] + const first = terminal.runCommand("first", { + onLine: vi.fn(), + onCompleted: completedSpies[0], + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpies[0], + }) + const firstProcess = terminal.process + const second = terminal.runCommand("second", { + onLine: vi.fn(), + onCompleted: completedSpies[1], + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpies[1], + }) + const secondProcess = terminal.process + const results = [first, second] const settledSpies = [vi.fn(), vi.fn()] void results[0].then(settledSpies[0]) void results[1].then(settledSpies[1]) @@ -706,6 +713,8 @@ describe("TerminalRegistry", () => { expect(completedSpies[1]).toHaveBeenCalledOnce() expect(completionSpies[0]).toHaveBeenCalledOnce() expect(completionSpies[1]).toHaveBeenCalledOnce() + expect(completionSpies[0]).toHaveBeenCalledWith({ exitCode: undefined }, firstProcess) + expect(completionSpies[1]).toHaveBeenCalledWith({ exitCode: undefined }, secondProcess) expect(disposeSpies[0]).toHaveBeenCalledOnce() expect(disposeSpies[1]).toHaveBeenCalledOnce() expect(vi.getTimerCount()).toBe(0) @@ -730,17 +739,87 @@ describe("TerminalRegistry", () => { expect(noShellSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("") expect(continueSpy).toHaveBeenCalledOnce() expect(terminal.process).toBeUndefined() + expect(terminal.busy).toBe(false) + expect(terminal.isStreamClosed).toBe(true) + expect(process["finalizedBeforeExecution"]).toBe(true) expect(process.eventNames()).toEqual([]) + const emitSpy = vi.spyOn(process, "emit") terminal.handleClose() + expect(emitSpy).not.toHaveBeenCalled() expect(shellCompleteSpy).not.toHaveBeenCalled() expect(completedSpy).toHaveBeenCalledOnce() expect(continueSpy).toHaveBeenCalledOnce() }) + it("keeps terminal-close completion idempotent when invoked directly", () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const process = new TerminalProcess(terminal) + terminal.process = process + const shellCompleteSpy = vi.spyOn(terminal, "shellExecutionComplete") + const completedSpy = vi.fn() + process.on("completed", completedSpy) + + process.handleTerminalClosed() + process.handleTerminalClosed() + + expect(shellCompleteSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledOnce() + expect(process["terminalCloseHandled"]).toBe(true) + }) + + it("finalizes a superseded startup process without clearing the current process", () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + const superseded = new TerminalProcess(terminal) + const current = new TerminalProcess(terminal) + terminal.process = current + const completionSpy = vi.fn() + const completedSpy = vi.fn() + superseded.on("shell_execution_complete", completionSpy) + superseded.on("completed", completedSpy) + + superseded.handleTerminalClosed() + + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }) + expect(Object.hasOwn(completionSpy.mock.calls[0][0], "exitCode")).toBe(true) + expect(completedSpy).toHaveBeenCalledWith("") + expect(terminal.process).toBe(current) + }) + + it("settles a superseded command whose integration wait resolved immediately before closure", async () => { + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const handlers: Array<(event: vscode.TerminalShellIntegrationChangeEvent) => void> = [] + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementation((handler) => { + handlers.push(handler) + return { dispose: vi.fn() } + }) + const firstCompleted = vi.fn() + const firstResult = terminal.runCommand("first", { + onLine: vi.fn(), + onCompleted: firstCompleted, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + }) + const firstProcess = terminal.process + handlers[0]({ terminal: terminal.terminal, shellIntegration: {} as never }) + + const secondResult = terminal.runCommand("second", { + onLine: vi.fn(), + onCompleted: vi.fn(), + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + }) + terminal.handleClose() + await Promise.all([firstResult, secondResult]) + + expect(firstCompleted).toHaveBeenCalledWith("", firstProcess) + }) + it("clears the timeout and disposes the listener when shell integration activates", async () => { vi.useFakeTimers() const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal From c4461c86ed30ed49f1996598a39e63789dfb446b Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 01:47:05 +0000 Subject: [PATCH 11/23] test(terminal): prove close finalization idempotency --- .../__tests__/TerminalProcess.spec.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 31bb806be3..ab0004b9ad 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -59,6 +59,42 @@ describe("TerminalProcess", () => { }) describe("run", () => { + it("does not execute a command started after the terminal is already closed", async () => { + mockTerminalInfo.handleClose() + const completedSpy = vi.fn() + const completionSpy = vi.fn() + + const result = mockTerminalInfo.runCommand("test command", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + }) + const process = mockTerminalInfo.process + expect(process).toBeInstanceOf(TerminalProcess) + await result + + expect(mockTerminal.shellIntegration.executeCommand).not.toHaveBeenCalled() + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + }) + + it("emits the startup-close completion sequence once and resets running state", () => { + mockTerminalInfo.running = true + const emitSpy = vi.spyOn(terminalProcess, "emit") + + terminalProcess.handleTerminalClosed() + terminalProcess.handleTerminalClosed() + + expect(emitSpy).toHaveBeenCalledTimes(3) + expect(emitSpy).toHaveBeenCalledWith("shell_execution_complete", { exitCode: undefined }) + expect(emitSpy).toHaveBeenCalledWith("completed", "") + expect(emitSpy).toHaveBeenCalledWith("continue") + expect(mockTerminalInfo.running).toBe(false) + }) + it("rejects the command promise when terminal process startup rejects", async () => { const startupError = new Error("terminal startup failed") const runSpy = vi.spyOn(TerminalProcess.prototype, "run").mockRejectedValueOnce(startupError) From 83f57e8d1ed0bcc952d68bbcfdb62ea2404ce240 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 02:01:19 +0000 Subject: [PATCH 12/23] refactor(terminal): remove redundant close state --- src/integrations/terminal/Terminal.ts | 6 ------ src/integrations/terminal/TerminalProcess.ts | 6 ++---- .../terminal/__tests__/TerminalProcess.spec.ts | 13 +++++++++++++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index bf9dcbda6f..1a7063e1fb 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -124,12 +124,6 @@ export class Terminal extends BaseTerminal { reject(error) }) - if (this.isClosed()) { - // Keep the newly created process observable to the caller until runCommand returns. - queueMicrotask(() => process.handleTerminalClosed()) - return - } - if (Terminal.isActiveShellCmdExe()) { // Keep this defensive fallback for callers that invoke Terminal.runCommand() // directly instead of routing through executeCommandInTerminal(). diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index f0c8519b54..3fd3bf514b 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -84,11 +84,9 @@ export class TerminalProcess extends BaseTerminalProcess { this.finalizedBeforeExecution = true const terminal = this.terminal - terminal.busy = false - terminal.running = false - terminal.activeShellExecution = undefined - terminal.setActiveStream(undefined) if (terminal.process === this) { + terminal.activeShellExecution = undefined + terminal.setActiveStream(undefined) terminal.process = undefined } this.emit("completed", output) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index ab0004b9ad..2d03d6b690 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -95,6 +95,19 @@ describe("TerminalProcess", () => { expect(mockTerminalInfo.running).toBe(false) }) + it("delivers only one close signal after shell execution has started", () => { + terminalProcess.ownExecution = { commandLine: { value: "test command" } } as vscode.TerminalShellExecution + const emitSpy = vi.spyOn(terminalProcess, "emit") + + terminalProcess.handleTerminalClosed() + terminalProcess.handleTerminalClosed() + + expect(emitSpy).toHaveBeenCalledOnce() + expect(emitSpy).toHaveBeenCalledWith("shell_execution_complete", { exitCode: undefined }) + expect(terminalProcess["terminalCloseHandled"]).toBe(true) + expect(terminalProcess["finalizedBeforeExecution"]).toBe(false) + }) + it("rejects the command promise when terminal process startup rejects", async () => { const startupError = new Error("terminal startup failed") const runSpy = vi.spyOn(TerminalProcess.prototype, "run").mockRejectedValueOnce(startupError) From 7ea0ca42b235c86248ad95d3aa02c4d93de926d0 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 04:09:41 +0000 Subject: [PATCH 13/23] fix(terminal): finalize superseded active processes --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 31 ++++++- src/integrations/terminal/Terminal.ts | 17 +++- src/integrations/terminal/TerminalProcess.ts | 11 ++- .../__tests__/TerminalRegistry.spec.ts | 80 +++++++++++++++++++ 5 files changed, 132 insertions(+), 9 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index cf775b3599..284e392446 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, concurrent shell-integration waits, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the process and settle every pending wait, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, and closure with two pending waits. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, closure with two pending waits, and closure with a superseded process. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index 358897f609..be2dd5e001 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -1,5 +1,5 @@ type Phase = "idle" | "waiting" | "running" | "completed" | "closed" -type Action = "run" | "wait" | "activate" | "output" | "end" | "close" +type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "close" interface ModelState { phase: Phase @@ -12,6 +12,9 @@ interface ModelState { waitsCreated: number pendingWaits: number settledWaits: number + processesCreated: number + trackedProcesses: number + settledProcesses: number } interface TraceStep { @@ -19,9 +22,9 @@ interface TraceStep { state: ModelState } -const actions: Action[] = ["run", "wait", "activate", "output", "end", "close"] +const actions: Action[] = ["run", "wait", "track-process", "activate", "output", "end", "close"] const MAX_DEPTH = 7 -const MAX_STATES = 100 +const MAX_STATES = 500 function initialState(): ModelState { return { @@ -35,6 +38,9 @@ function initialState(): ModelState { waitsCreated: 0, pendingWaits: 0, settledWaits: 0, + processesCreated: 0, + trackedProcesses: 0, + settledProcesses: 0, } } @@ -57,6 +63,14 @@ function transition(state: ModelState, action: Action): ModelState { return state.phase !== "closed" && state.waitsCreated < 2 ? { ...state, waitsCreated: state.waitsCreated + 1, pendingWaits: state.pendingWaits + 1 } : state + case "track-process": + return state.phase !== "closed" && state.processesCreated < 2 + ? { + ...state, + processesCreated: state.processesCreated + 1, + trackedProcesses: state.trackedProcesses + 1, + } + : state case "activate": return state.phase === "waiting" ? { @@ -78,6 +92,8 @@ function transition(state: ModelState, action: Action): ModelState { ...complete(state, "closed"), pendingWaits: 0, settledWaits: state.settledWaits + state.pendingWaits, + trackedProcesses: 0, + settledProcesses: state.settledProcesses + state.trackedProcesses, } } } @@ -87,7 +103,11 @@ function violations(state: ModelState): string[] { if (state.completionCount > 1) result.push("a command completed more than once") if (state.phase === "closed" && state.processAttached) result.push("a closed terminal retained its process") if (state.phase === "closed" && state.pendingWaits !== 0) result.push("a closed terminal retained pending waits") + if (state.phase === "closed" && state.trackedProcesses !== 0) { + result.push("a closed terminal retained tracked processes") + } if (state.settledWaits > state.waitsCreated) result.push("more waits settled than were created") + if (state.settledProcesses > state.processesCreated) result.push("more processes settled than were created") if (state.phase === "closed" && state.commandSubmitted && !state.iteratorReleased) { result.push("closing a submitted command did not release its stream iterator") } @@ -126,6 +146,11 @@ const landmarks = { trace.at(-1)?.state.waitsCreated === 2 && trace.at(-1)?.state.pendingWaits === 0 && trace.at(-1)?.state.settledWaits === 2, + "superseded-process-close": (trace: TraceStep[]) => + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.processesCreated === 2 && + trace.at(-1)?.state.trackedProcesses === 0 && + trace.at(-1)?.state.settledProcesses === 2, } satisfies Record boolean> const start = initialState() diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 1a7063e1fb..9108b4c604 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -13,6 +13,7 @@ export class Terminal extends BaseTerminal { public terminal: vscode.Terminal private closed = false private cancelShellIntegrationWaits = new Set<() => void>() + private activeProcesses = new Set() public cmdCounter: number = 0 @@ -90,8 +91,15 @@ export class Terminal extends BaseTerminal { cancel() } + const processes = new Set(this.activeProcesses) if (this.process instanceof TerminalProcess) { - this.process.handleTerminalClosed() + processes.add(this.process) + } + + if (processes.size > 0) { + for (const process of processes) { + process.handleTerminalClosed() + } } else { this.shellExecutionComplete({ exitCode: undefined }) } @@ -106,12 +114,16 @@ export class Terminal extends BaseTerminal { const process = new TerminalProcess(this) process.command = command this.process = process + this.activeProcesses.add(process) // Set up event handlers from callbacks before starting process. // This ensures that we don't miss any events because they are // configured before the process starts. process.on("line", (line) => callbacks.onLine(line, process)) - process.once("completed", (output) => callbacks.onCompleted(output, process)) + process.once("completed", (output) => { + this.activeProcesses.delete(process) + void callbacks.onCompleted(output, process) + }) process.once("shell_execution_started", (pid) => callbacks.onShellExecutionStarted(pid, process)) process.once("shell_execution_complete", (details) => callbacks.onShellExecutionComplete(details, process)) process.once("no_shell_integration", (details) => callbacks.onNoShellIntegration?.(details, process)) @@ -120,6 +132,7 @@ export class Terminal extends BaseTerminal { // Set up event handlers process.once("continue", () => resolve()) process.once("error", (error) => { + this.activeProcesses.delete(process) console.error(`[Terminal ${this.id}] error:`, error) reject(error) }) diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 3fd3bf514b..d735c2db6b 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -327,6 +327,7 @@ export class TerminalProcess extends BaseTerminalProcess { // and silently drops the first output chunk). let nextChunk = iterator.next() while (true) { + let idleTimer: NodeJS.Timeout | undefined const racers: Promise>[] = [ nextChunk, shellExecutionComplete.then(() => DONE_SENTINEL as typeof DONE_SENTINEL), @@ -336,13 +337,17 @@ export class TerminalProcess extends BaseTerminalProcess { // flowing we trust the stream to close normally (or the D-marker path). if (chunkCount === 0) { racers.push( - new Promise((resolve) => - setTimeout(() => resolve(IDLE_SENTINEL as typeof IDLE_SENTINEL), IDLE_TIMEOUT_MS), - ), + new Promise((resolve) => { + idleTimer = setTimeout( + () => resolve(IDLE_SENTINEL as typeof IDLE_SENTINEL), + IDLE_TIMEOUT_MS, + ) + }), ) } const raceResult = await Promise.race(racers) + clearTimeout(idleTimer) if (raceResult === DONE_SENTINEL) { // onDidEndTerminalShellExecution fired — the shell says we're done. diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 9f07e8e9bd..eb7121fc87 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -446,6 +446,7 @@ describe("TerminalRegistry", () => { await settleWithin(result) expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) expect(completedSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledWith("", process) expect(noShellIntegrationSpy).not.toHaveBeenCalled() @@ -489,6 +490,7 @@ describe("TerminalRegistry", () => { expect(executeCommand).not.toHaveBeenCalled() expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) expect(completedSpy).toHaveBeenCalledOnce() expect(completedSpy).toHaveBeenCalledWith("", process) expect(noShellIntegrationSpy).not.toHaveBeenCalled() @@ -720,6 +722,84 @@ describe("TerminalRegistry", () => { expect(vi.getTimerCount()).toBe(0) }) + it("settles a superseded active stream and the current startup command on close", async () => { + vi.useFakeTimers() + const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal + let nextCall = 0 + let signalWaitingForNext: () => void = () => {} + const waitingForNext = new Promise((resolve) => { + signalWaitingForNext = resolve + }) + const returnSpy = vi.fn().mockResolvedValue({ done: true, value: undefined }) + const stream: AsyncIterable = { + [Symbol.asyncIterator]() { + return { + next: vi.fn(() => { + nextCall++ + if (nextCall === 1) { + return Promise.resolve({ done: false, value: "\x1b]633;C\x07first\n" }) + } + signalWaitingForNext() + return new Promise>(() => {}) + }), + return: returnSpy, + } + }, + } + const execution = { + commandLine: { value: "first" }, + read: vi.fn().mockReturnValue(stream), + } as unknown as vscode.TerminalShellExecution + const executeCommand = vi.fn().mockReturnValue(execution) + Object.defineProperty(terminal.terminal, "shellIntegration", { + value: { executeCommand }, + configurable: true, + }) + const firstCompleted = vi.fn() + const firstCompletion = vi.fn() + const firstResult = terminal.runCommand("first", { + onLine: vi.fn(), + onCompleted: firstCompleted, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: firstCompletion, + }) + const firstProcess = terminal.process + await Promise.resolve() + await Promise.resolve() + expect(executeCommand).toHaveBeenCalledOnce() + await startHandler({ terminal: terminal.terminal, execution }) + await waitingForNext + + Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) + const disposeSpy = vi.fn() + vi.mocked(vscode.window.onDidChangeTerminalShellIntegration).mockImplementationOnce(() => ({ + dispose: disposeSpy, + })) + const secondCompleted = vi.fn() + const secondCompletion = vi.fn() + const secondResult = terminal.runCommand("second", { + onLine: vi.fn(), + onCompleted: secondCompleted, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: secondCompletion, + }) + const secondProcess = terminal.process + + terminal.handleClose() + const settled = vi.fn() + void Promise.all([firstResult, secondResult]).then(settled) + await vi.advanceTimersByTimeAsync(0) + + expect(settled).toHaveBeenCalledOnce() + expect(firstCompletion).toHaveBeenCalledWith({ exitCode: undefined }, firstProcess) + expect(secondCompletion).toHaveBeenCalledWith({ exitCode: undefined }, secondProcess) + expect(firstCompleted).toHaveBeenCalledWith("first\n", firstProcess) + expect(secondCompleted).toHaveBeenCalledWith("", secondProcess) + expect(returnSpy).toHaveBeenCalledOnce() + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + }) + it("does not complete again when a no-shell process is followed by terminal closure", async () => { const terminal = TerminalRegistry.createTerminal("/test/path", "vscode") as Terminal Object.defineProperty(terminal.terminal, "shellIntegration", { value: undefined, configurable: true }) From 34d343e64709c7c3e3ac759ea88a5774c5e7b0ca Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 04:21:47 +0000 Subject: [PATCH 14/23] refactor(terminal): centralize process tracking --- src/integrations/terminal/Terminal.ts | 33 +++++++++++--------- src/integrations/terminal/TerminalProcess.ts | 5 +++ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 9108b4c604..fe5c664d54 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -91,13 +91,8 @@ export class Terminal extends BaseTerminal { cancel() } - const processes = new Set(this.activeProcesses) - if (this.process instanceof TerminalProcess) { - processes.add(this.process) - } - - if (processes.size > 0) { - for (const process of processes) { + if (this.activeProcesses.size > 0) { + for (const process of this.activeProcesses) { process.handleTerminalClosed() } } else { @@ -114,16 +109,12 @@ export class Terminal extends BaseTerminal { const process = new TerminalProcess(this) process.command = command this.process = process - this.activeProcesses.add(process) // Set up event handlers from callbacks before starting process. // This ensures that we don't miss any events because they are // configured before the process starts. process.on("line", (line) => callbacks.onLine(line, process)) - process.once("completed", (output) => { - this.activeProcesses.delete(process) - void callbacks.onCompleted(output, process) - }) + process.once("completed", (output) => callbacks.onCompleted(output, process)) process.once("shell_execution_started", (pid) => callbacks.onShellExecutionStarted(pid, process)) process.once("shell_execution_complete", (details) => callbacks.onShellExecutionComplete(details, process)) process.once("no_shell_integration", (details) => callbacks.onNoShellIntegration?.(details, process)) @@ -132,11 +123,15 @@ export class Terminal extends BaseTerminal { // Set up event handlers process.once("continue", () => resolve()) process.once("error", (error) => { - this.activeProcesses.delete(process) console.error(`[Terminal ${this.id}] error:`, error) reject(error) }) + if (this.isClosed()) { + queueMicrotask(() => process.handleTerminalClosed()) + return + } + if (Terminal.isActiveShellCmdExe()) { // Keep this defensive fallback for callers that invoke Terminal.runCommand() // directly instead of routing through executeCommandInTerminal(). @@ -157,7 +152,6 @@ export class Terminal extends BaseTerminal { this.waitForShellIntegration(Terminal.getShellIntegrationTimeout()) .then(() => { if (this.isClosed()) { - process.handleTerminalClosed() return } @@ -169,7 +163,6 @@ export class Terminal extends BaseTerminal { }) .catch(() => { if (this.isClosed()) { - process.handleTerminalClosed() return } @@ -189,6 +182,16 @@ export class Terminal extends BaseTerminal { return mergePromise(process, promise) } + /** Registers a process so terminal closure can finalize it even after supersession. */ + public trackProcess(process: TerminalProcess): void { + this.activeProcesses.add(process) + } + + /** Removes a process after its completion or error path has settled. */ + public releaseProcess(process: TerminalProcess): void { + this.activeProcesses.delete(process) + } + /** * Resolves when this terminal's shell integration becomes active, or rejects * after timeoutMs if the shell never signals readiness. Uses the diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index d735c2db6b..5b8674b4ed 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -37,11 +37,16 @@ export class TerminalProcess extends BaseTerminalProcess { super() this.terminalRef = new WeakRef(terminal) + terminal.trackProcess(this) this.once("completed", () => { + this.terminal.releaseProcess(this) this.terminal.busy = false }) + this.once("error", () => this.terminal.releaseProcess(this)) + this.once("shell_execution_complete", () => this.terminal.releaseProcess(this)) + this.once("no_shell_integration", () => { this.completeBeforeExecution("") }) From fbe2f67c49154613ab44d97e8b38b75afb0b1197 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 04:32:51 +0000 Subject: [PATCH 15/23] test(terminal): verify process tracking cleanup --- src/integrations/terminal/__tests__/TerminalProcess.spec.ts | 2 ++ src/integrations/terminal/__tests__/TerminalRegistry.spec.ts | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 2d03d6b690..47684d4745 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -112,6 +112,7 @@ describe("TerminalProcess", () => { const startupError = new Error("terminal startup failed") const runSpy = vi.spyOn(TerminalProcess.prototype, "run").mockRejectedValueOnce(startupError) const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined) + const initialProcessCount = mockTerminalInfo["activeProcesses"].size const commandPromise = mockTerminalInfo.runCommand("test command", { onLine: vi.fn(), @@ -122,6 +123,7 @@ describe("TerminalProcess", () => { await expect(commandPromise).rejects.toThrow("terminal startup failed") expect(runSpy).toHaveBeenCalledWith("test command") + expect(mockTerminalInfo["activeProcesses"].size).toBe(initialProcessCount) runSpy.mockRestore() consoleErrorSpy.mockRestore() diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index eb7121fc87..41872720cc 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -632,6 +632,7 @@ describe("TerminalRegistry", () => { }) const process = terminal.process expect(process).toBeInstanceOf(TerminalProcess) + const emitSpy = vi.spyOn(process!, "emit") terminal.handleClose() expect(completedSpy).toHaveBeenCalledOnce() @@ -642,6 +643,7 @@ describe("TerminalRegistry", () => { expect(disposeSpy).toHaveBeenCalledOnce() expect(vi.getTimerCount()).toBe(0) + expect(emitSpy).not.toHaveBeenCalledWith("no_shell_integration", expect.anything()) for (const event of [ "line", "completed", @@ -822,6 +824,7 @@ describe("TerminalRegistry", () => { expect(completedSpy).toHaveBeenCalledWith("") expect(continueSpy).toHaveBeenCalledOnce() expect(terminal.process).toBeUndefined() + expect(terminal["activeProcesses"].size).toBe(0) expect(terminal.busy).toBe(false) expect(terminal.isStreamClosed).toBe(true) expect(process["finalizedBeforeExecution"]).toBe(true) From 3ce63db9ebc38600984f2669cc47f9cbc00f217d Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 01:39:35 +0000 Subject: [PATCH 16/23] test(terminal): reject truncated lifecycle exploration --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 284e392446..fc7fc17abf 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, streamed output, normal completion, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, closure with two pending waits, and closure with a superseded process. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index be2dd5e001..b5b82aefbf 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -23,7 +23,7 @@ interface TraceStep { } const actions: Action[] = ["run", "wait", "track-process", "activate", "output", "end", "close"] -const MAX_DEPTH = 7 +const MAX_DEPTH = 8 const MAX_STATES = 500 function initialState(): ModelState { @@ -82,7 +82,7 @@ function transition(state: ModelState, action: Action): ModelState { } : state case "output": - return state.phase === "running" ? { ...state, output: `${state.output}chunk` } : state + return state.phase === "running" && state.output === "" ? { ...state, output: "chunk" } : state case "end": return state.phase === "waiting" || state.phase === "running" ? complete(state, "completed") : state case "close": @@ -160,6 +160,7 @@ const queue: Array<{ state: ModelState; trace: TraceStep[] }> = [ const visited = new Set([JSON.stringify(start)]) const reachedActions = new Set() const reachedLandmarks = new Set() +const frontier: ModelState[] = [] for (let index = 0; index < queue.length; index++) { const node = queue[index]! @@ -168,7 +169,10 @@ for (let index = 0; index < queue.length; index++) { for (const [name, predicate] of Object.entries(landmarks)) { if (predicate(node.trace)) reachedLandmarks.add(name) } - if (node.trace.length - 1 === MAX_DEPTH) continue + if (node.trace.length - 1 === MAX_DEPTH) { + frontier.push(node.state) + continue + } for (const action of actions) { const next = transition(node.state, action) @@ -186,6 +190,15 @@ for (let index = 0; index < queue.length; index++) { } } +const unexploredSuccessor = frontier + .flatMap((state) => actions.map((action) => ({ action, next: transition(state, action) }))) + .find(({ next }) => !visited.has(JSON.stringify(next))) +if (unexploredSuccessor) { + throw new Error( + `Terminal lifecycle exploration reached depth ${MAX_DEPTH} with an unseen successor (${unexploredSuccessor.action}); increase the depth bound`, + ) +} + const missingActions = actions.filter((action) => !reachedActions.has(action)) if (missingActions.length) throw new Error(`Terminal lifecycle has unreachable actions: ${missingActions.join(", ")}`) const missingLandmarks = Object.keys(landmarks).filter((name) => !reachedLandmarks.has(name)) From d1945c8b30d1b7ee843ac24c268aa3eb47ef2685 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 03:05:22 +0000 Subject: [PATCH 17/23] fix(terminal): finalize failed processes once --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 26 +++++-- src/integrations/terminal/Terminal.ts | 9 ++- src/integrations/terminal/TerminalProcess.ts | 29 +++++++- .../__tests__/TerminalProcess.spec.ts | 69 +++++++++++++++++++ 5 files changed, 127 insertions(+), 8 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index fc7fc17abf..601d5ba86d 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, startup or stream failure, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, closure after an error without a late completion, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index b5b82aefbf..e7fbdf3964 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -1,5 +1,5 @@ -type Phase = "idle" | "waiting" | "running" | "completed" | "closed" -type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "close" +type Phase = "idle" | "waiting" | "running" | "completed" | "failed" | "closed" +type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "error" | "close" interface ModelState { phase: Phase @@ -22,8 +22,8 @@ interface TraceStep { state: ModelState } -const actions: Action[] = ["run", "wait", "track-process", "activate", "output", "end", "close"] -const MAX_DEPTH = 8 +const actions: Action[] = ["run", "wait", "track-process", "activate", "output", "end", "error", "close"] +const MAX_DEPTH = 9 const MAX_STATES = 500 function initialState(): ModelState { @@ -85,6 +85,19 @@ function transition(state: ModelState, action: Action): ModelState { return state.phase === "running" && state.output === "" ? { ...state, output: "chunk" } : state case "end": return state.phase === "waiting" || state.phase === "running" ? complete(state, "completed") : state + case "error": + return state.phase === "waiting" || state.phase === "running" + ? { + ...state, + phase: "failed", + processAttached: false, + iteratorReleased: state.iteratorReleased || state.phase === "running", + pendingWaits: 0, + settledWaits: state.settledWaits + state.pendingWaits, + trackedProcesses: Math.max(0, state.trackedProcesses - 1), + settledProcesses: state.settledProcesses + (state.trackedProcesses > 0 ? 1 : 0), + } + : state case "close": return state.phase === "closed" ? state @@ -140,6 +153,11 @@ const landmarks = { trace.some((step) => step.action === "end") && trace.at(-1)?.action === "close" && trace.at(-1)?.state.completionCount === 1, + "error-then-close": (trace: TraceStep[]) => + trace.some((step) => step.action === "error") && + trace.at(-1)?.action === "close" && + trace.at(-1)?.state.processAttached === false && + trace.at(-1)?.state.completionCount === 0, "duplicate-close": (trace: TraceStep[]) => trace.filter((step) => step.action === "close").length >= 2, "concurrent-waits-close": (trace: TraceStep[]) => trace.at(-1)?.action === "close" && diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index fe5c664d54..b8de1cc1bb 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -96,7 +96,13 @@ export class Terminal extends BaseTerminal { process.handleTerminalClosed() } } else { - this.shellExecutionComplete({ exitCode: undefined }) + this.busy = false + this.running = false + this.activeShellExecution = undefined + this.setActiveStream(undefined) + if (!this.process) { + this.shellExecutionComplete({ exitCode: undefined }) + } } } @@ -125,6 +131,7 @@ export class Terminal extends BaseTerminal { process.once("error", (error) => { console.error(`[Terminal ${this.id}] error:`, error) reject(error) + process.handleError() }) if (this.isClosed()) { diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 5b8674b4ed..79691cd477 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -32,6 +32,7 @@ export class TerminalProcess extends BaseTerminalProcess { public ownExecution?: vscode.TerminalShellExecution private terminalCloseHandled = false private finalizedBeforeExecution = false + private errorHandled = false constructor(terminal: Terminal) { super() @@ -44,7 +45,6 @@ export class TerminalProcess extends BaseTerminalProcess { this.terminal.busy = false }) - this.once("error", () => this.terminal.releaseProcess(this)) this.once("shell_execution_complete", () => this.terminal.releaseProcess(this)) this.once("no_shell_integration", () => { @@ -64,7 +64,7 @@ export class TerminalProcess extends BaseTerminalProcess { /** Completes this process when its terminal closes without an execution-end event. */ public handleTerminalClosed(): void { - if (this.terminalCloseHandled || this.finalizedBeforeExecution) { + if (this.terminalCloseHandled || this.finalizedBeforeExecution || this.errorHandled) { return } this.terminalCloseHandled = true @@ -85,6 +85,28 @@ export class TerminalProcess extends BaseTerminalProcess { this.completeBeforeExecution("") } + /** Releases a failed process without allowing terminal closure to complete it later. */ + public handleError(): void { + if (this.errorHandled || this.terminalCloseHandled || this.finalizedBeforeExecution) { + return + } + this.errorHandled = true + + const terminal = this.terminal + if (terminal.process === this) { + terminal.activeShellExecution = undefined + terminal.setActiveStream(undefined) + terminal.process = undefined + terminal.busy = false + terminal.running = false + } + terminal.releaseProcess(this) + this.isHot = false + this.stopHotTimer() + this.cleanupScriptFile() + this.removeAllListeners() + } + private completeBeforeExecution(output: string): void { this.finalizedBeforeExecution = true @@ -218,6 +240,7 @@ export class TerminalProcess extends BaseTerminalProcess { // that misses output: the execution begins after the stream was opened, // VSCode doesn't buffer retroactively, and zero chunks arrive. } catch (error) { + cancelStreamWait() this.terminal.activeShellExecution = undefined this.cleanupScriptFile() throw error @@ -267,6 +290,7 @@ export class TerminalProcess extends BaseTerminalProcess { // Emit continue event to allow execution to proceed this.emit("continue") + this.handleError() return } @@ -554,6 +578,7 @@ export class TerminalProcess extends BaseTerminalProcess { ``, ) this.emit("continue") + this.handleError() } } } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 47684d4745..3a987e521f 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -129,6 +129,75 @@ describe("TerminalProcess", () => { consoleErrorSpy.mockRestore() }) + it("cleans up a failed process without completing it when the terminal closes later", async () => { + vi.useFakeTimers() + const startupError = new Error("terminal startup failed") + mockTerminal.shellIntegration.executeCommand.mockImplementationOnce(() => { + throw startupError + }) + vi.spyOn(console, "error").mockImplementation(() => undefined) + const completionSpy = vi.fn() + + try { + const commandPromise = mockTerminalInfo.runCommand("test command", { + onLine: vi.fn(), + onCompleted: vi.fn(), + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + }) + const process = mockTerminalInfo.process + mockTerminalInfo.running = true + + await expect(commandPromise).rejects.toBe(startupError) + + expect(mockTerminalInfo.process).toBeUndefined() + expect(mockTerminalInfo.activeShellExecution).toBeUndefined() + expect(mockTerminalInfo.busy).toBe(false) + expect(mockTerminalInfo.running).toBe(false) + expect(mockTerminalInfo.isStreamClosed).toBe(true) + expect(mockTerminalInfo["activeProcesses"]).not.toContain(process) + expect(process?.eventNames()).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + + mockTerminalInfo.handleClose() + + expect(completionSpy).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it("does not clear a newer process when a superseded process fails", async () => { + const startupError = new Error("first process failed") + vi.spyOn(TerminalProcess.prototype, "run") + .mockRejectedValueOnce(startupError) + .mockResolvedValueOnce(undefined) + vi.spyOn(console, "error").mockImplementation(() => undefined) + const callbacks = { + onLine: vi.fn(), + onCompleted: vi.fn(), + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + } + + const firstCommand = mockTerminalInfo.runCommand("first", callbacks) + const firstProcess = mockTerminalInfo.process + const secondCommand = mockTerminalInfo.runCommand("second", callbacks) + const secondProcess = mockTerminalInfo.process + mockTerminalInfo.running = true + + await expect(firstCommand).rejects.toBe(startupError) + + expect(mockTerminalInfo.process).toBe(secondProcess) + expect(mockTerminalInfo.busy).toBe(true) + expect(mockTerminalInfo.running).toBe(true) + expect(mockTerminalInfo["activeProcesses"]).not.toContain(firstProcess) + expect(mockTerminalInfo["activeProcesses"]).toContain(secondProcess) + + mockTerminalInfo.handleClose() + await secondCommand + }) + it("emits no_shell_integration with commandSubmitted=false when shell integration startup times out", async () => { vi.useFakeTimers() const previousTimeout = Terminal.getShellIntegrationTimeout() From c0aaff84b33a0694b489e3dea1013c17e6829c3a Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 03:16:35 +0000 Subject: [PATCH 18/23] test(terminal): prove error cleanup ownership --- src/integrations/terminal/Terminal.ts | 8 ++- src/integrations/terminal/TerminalProcess.ts | 2 - .../__tests__/TerminalProcess.spec.ts | 49 +++++++++++++++++++ .../__tests__/TerminalRegistry.spec.ts | 22 +++++++++ 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index b8de1cc1bb..b985d43fcb 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -95,14 +95,12 @@ export class Terminal extends BaseTerminal { for (const process of this.activeProcesses) { process.handleTerminalClosed() } + } else if (this.process instanceof TerminalProcess) { + this.process.handleError() } else { - this.busy = false - this.running = false this.activeShellExecution = undefined this.setActiveStream(undefined) - if (!this.process) { - this.shellExecutionComplete({ exitCode: undefined }) - } + this.shellExecutionComplete({ exitCode: undefined }) } } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 79691cd477..03e9251765 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -101,9 +101,7 @@ export class TerminalProcess extends BaseTerminalProcess { terminal.running = false } terminal.releaseProcess(this) - this.isHot = false this.stopHotTimer() - this.cleanupScriptFile() this.removeAllListeners() } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 3a987e521f..774785e8f3 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -198,6 +198,27 @@ describe("TerminalProcess", () => { await secondCommand }) + it("finalizes an error only once", () => { + mockTerminalInfo.busy = true + mockTerminalInfo.running = true + mockTerminalInfo.activeShellExecution = { + commandLine: { value: "failed" }, + } as vscode.TerminalShellExecution + const releaseSpy = vi.spyOn(mockTerminalInfo, "releaseProcess") + + terminalProcess.handleError() + terminalProcess.handleError() + + expect(releaseSpy).toHaveBeenCalledOnce() + expect(terminalProcess["errorHandled"]).toBe(true) + expect(mockTerminalInfo.process).toBeUndefined() + expect(mockTerminalInfo.activeShellExecution).toBeUndefined() + expect(mockTerminalInfo.busy).toBe(false) + expect(mockTerminalInfo.running).toBe(false) + expect(mockTerminalInfo.isStreamClosed).toBe(true) + expect(terminalProcess.eventNames()).toEqual([]) + }) + it("emits no_shell_integration with commandSubmitted=false when shell integration startup times out", async () => { vi.useFakeTimers() const previousTimeout = Terminal.getShellIntegrationTimeout() @@ -227,6 +248,34 @@ describe("TerminalProcess", () => { } }) + it("releases a command when its shell stream never becomes available", async () => { + vi.useFakeTimers() + const previousTimeout = Terminal.getShellIntegrationTimeout() + Terminal.setShellIntegrationTimeout(10) + mockTerminal.shellIntegration.executeCommand.mockReturnValue({}) + + try { + const commandPromise = mockTerminalInfo.runCommand("test command", { + onLine: vi.fn(), + onCompleted: vi.fn(), + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: vi.fn(), + }) + const process = mockTerminalInfo.process + + await vi.advanceTimersByTimeAsync(10) + await commandPromise + + expect(mockTerminalInfo.process).toBeUndefined() + expect(mockTerminalInfo["activeProcesses"]).not.toContain(process) + expect(process?.eventNames()).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + } finally { + Terminal.setShellIntegrationTimeout(previousTimeout) + vi.useRealTimers() + } + }) + it("runs command after shell integration activates via onDidChangeTerminalShellIntegration event", async () => { // Cover Terminal.runCommand's waitForShellIntegration resolve path: shell // integration is initially absent but arrives via the VSCode event before timeout. diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 41872720cc..a0aaaa343f 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -354,6 +354,28 @@ describe("TerminalRegistry", () => { expect(TerminalRegistry["terminals"]).toEqual([open]) }) + it("cleans a stale untracked process without emitting close completion", () => { + const terminal = TerminalRegistry.createTerminal("/closed", "vscode") as Terminal + const process = new TerminalProcess(terminal) + terminal.process = process + terminal.releaseProcess(process) + terminal.busy = true + terminal.running = true + terminal.activeShellExecution = { commandLine: { value: "failed" } } as vscode.TerminalShellExecution + const completionSpy = vi.fn() + process.on("shell_execution_complete", completionSpy) + + terminal.handleClose() + + expect(completionSpy).not.toHaveBeenCalled() + expect(terminal.process).toBeUndefined() + expect(terminal.activeShellExecution).toBeUndefined() + expect(terminal.busy).toBe(false) + expect(terminal.running).toBe(false) + expect(terminal.isStreamClosed).toBe(true) + expect(process.eventNames()).toEqual([]) + }) + it("ignores close events from unregistered terminals", () => { const registered = TerminalRegistry.createTerminal("/registered", "vscode") as Terminal const foreign = { name: "foreign" } as vscode.Terminal From 2015a4e8a850963bcf420bafd7385157591491f2 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 03:28:02 +0000 Subject: [PATCH 19/23] test(terminal): cover stream error cleanup --- src/integrations/terminal/TerminalProcess.ts | 1 - .../__tests__/TerminalProcess.spec.ts | 36 +++++++++++++++++++ .../__tests__/TerminalRegistry.spec.ts | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 03e9251765..32436a0e46 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -288,7 +288,6 @@ export class TerminalProcess extends BaseTerminalProcess { // Emit continue event to allow execution to proceed this.emit("continue") - this.handleError() return } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 774785e8f3..56734097b4 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -201,6 +201,7 @@ describe("TerminalProcess", () => { it("finalizes an error only once", () => { mockTerminalInfo.busy = true mockTerminalInfo.running = true + terminalProcess.isHot = true mockTerminalInfo.activeShellExecution = { commandLine: { value: "failed" }, } as vscode.TerminalShellExecution @@ -216,6 +217,7 @@ describe("TerminalProcess", () => { expect(mockTerminalInfo.busy).toBe(false) expect(mockTerminalInfo.running).toBe(false) expect(mockTerminalInfo.isStreamClosed).toBe(true) + expect(terminalProcess.isHot).toBe(false) expect(terminalProcess.eventNames()).toEqual([]) }) @@ -276,6 +278,40 @@ describe("TerminalProcess", () => { } }) + it("releases a command when its active stream throws", async () => { + const streamError = new Error("stream failed") + const stream: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ next: () => Promise.reject(streamError) }), + } + mockTerminal.shellIntegration.executeCommand.mockReturnValue({}) + vi.spyOn(console, "error").mockImplementation(() => undefined) + const completedSpy = vi.fn() + const completionSpy = vi.fn() + + const commandPromise = mockTerminalInfo.runCommand("test command", { + onLine: vi.fn(), + onCompleted: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + }) + const process = mockTerminalInfo.process + await Promise.resolve() + mockTerminalInfo.setActiveStream(stream) + + await commandPromise + + expect(completedSpy).toHaveBeenCalledWith("", process) + expect(mockTerminalInfo.process).toBeUndefined() + expect(mockTerminalInfo.activeShellExecution).toBeUndefined() + expect(mockTerminalInfo.busy).toBe(false) + expect(mockTerminalInfo.running).toBe(false) + expect(mockTerminalInfo["activeProcesses"]).not.toContain(process) + expect(process?.eventNames()).toEqual([]) + + mockTerminalInfo.handleClose() + expect(completionSpy).not.toHaveBeenCalled() + }) + it("runs command after shell integration activates via onDidChangeTerminalShellIntegration event", async () => { // Cover Terminal.runCommand's waitForShellIntegration resolve path: shell // integration is initially absent but arrives via the VSCode event before timeout. diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index a0aaaa343f..05112019e2 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -350,6 +350,7 @@ describe("TerminalRegistry", () => { expect(closed.isClosed()).toBe(true) expect(closed.busy).toBe(false) expect(closed.running).toBe(false) + expect(closed.isStreamClosed).toBe(true) expect(cleanupSpy).toHaveBeenCalledWith(closed.id) expect(TerminalRegistry["terminals"]).toEqual([open]) }) From f3e4ddd525ebc9c8d078573e77b3ce53d811dc16 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 03:45:03 +0000 Subject: [PATCH 20/23] fix(terminal): preserve current process ownership --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 27 ++++++++++++- src/integrations/terminal/TerminalProcess.ts | 12 +++--- .../__tests__/TerminalProcess.spec.ts | 38 +++++++++++++++++++ 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 601d5ba86d..01d3d8268d 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, startup or stream failure, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, closure after an error without a late completion, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, startup or stream failure, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, closure after an error without a late completion, a superseded process failing without clearing the current owner, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index e7fbdf3964..bfc01e1675 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -1,5 +1,5 @@ type Phase = "idle" | "waiting" | "running" | "completed" | "failed" | "closed" -type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "error" | "close" +type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "error" | "error-superseded" | "close" interface ModelState { phase: Phase @@ -22,7 +22,17 @@ interface TraceStep { state: ModelState } -const actions: Action[] = ["run", "wait", "track-process", "activate", "output", "end", "error", "close"] +const actions: Action[] = [ + "run", + "wait", + "track-process", + "activate", + "output", + "end", + "error", + "error-superseded", + "close", +] const MAX_DEPTH = 9 const MAX_STATES = 500 @@ -98,6 +108,14 @@ function transition(state: ModelState, action: Action): ModelState { settledProcesses: state.settledProcesses + (state.trackedProcesses > 0 ? 1 : 0), } : state + case "error-superseded": + return state.processAttached && state.trackedProcesses >= 2 + ? { + ...state, + trackedProcesses: state.trackedProcesses - 1, + settledProcesses: state.settledProcesses + 1, + } + : state case "close": return state.phase === "closed" ? state @@ -158,6 +176,11 @@ const landmarks = { trace.at(-1)?.action === "close" && trace.at(-1)?.state.processAttached === false && trace.at(-1)?.state.completionCount === 0, + "superseded-error-preserves-owner": (trace: TraceStep[]) => + trace.at(-1)?.action === "error-superseded" && + trace.at(-1)?.state.processAttached === true && + trace.at(-1)?.state.phase === "running" && + trace.at(-1)?.state.trackedProcesses === 1, "duplicate-close": (trace: TraceStep[]) => trace.filter((step) => step.action === "close").length >= 2, "concurrent-waits-close": (trace: TraceStep[]) => trace.at(-1)?.action === "close" && diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 32436a0e46..95b6f96ebe 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -42,7 +42,9 @@ export class TerminalProcess extends BaseTerminalProcess { this.once("completed", () => { this.terminal.releaseProcess(this) - this.terminal.busy = false + if (this.terminal.process === this) { + this.terminal.busy = false + } }) this.once("shell_execution_complete", () => this.terminal.releaseProcess(this)) @@ -96,9 +98,9 @@ export class TerminalProcess extends BaseTerminalProcess { if (terminal.process === this) { terminal.activeShellExecution = undefined terminal.setActiveStream(undefined) - terminal.process = undefined terminal.busy = false terminal.running = false + terminal.process = undefined } terminal.releaseProcess(this) this.stopHotTimer() @@ -112,6 +114,8 @@ export class TerminalProcess extends BaseTerminalProcess { if (terminal.process === this) { terminal.activeShellExecution = undefined terminal.setActiveStream(undefined) + terminal.busy = false + terminal.running = false terminal.process = undefined } this.emit("completed", output) @@ -283,7 +287,6 @@ export class TerminalProcess extends BaseTerminalProcess { "", ) - this.terminal.busy = false this.cleanupScriptFile() // Emit continue event to allow execution to proceed @@ -566,9 +569,6 @@ export class TerminalProcess extends BaseTerminalProcess { if (streamProcessingError !== undefined) { // Ensure cleanup and caller unblocking happen even when the loop throws. - this.terminal.activeShellExecution = undefined - this.terminal.busy = false - this.isHot = false this.cleanupScriptFile() this.emit( "completed", diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 56734097b4..88e4976bf2 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -312,6 +312,44 @@ describe("TerminalProcess", () => { expect(completionSpy).not.toHaveBeenCalled() }) + it("does not clear a newer process when a superseded active stream throws", async () => { + const streamError = new Error("old stream failed") + let rejectNext: (error: Error) => void = () => undefined + const next = new Promise>((_, reject) => (rejectNext = reject)) + const stream: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + next: () => next, + }), + } + const oldExecution = { commandLine: { value: "old" } } as vscode.TerminalShellExecution + mockTerminal.shellIntegration.executeCommand.mockReturnValue(oldExecution) + vi.spyOn(console, "error").mockImplementation(() => undefined) + + const oldRun = terminalProcess.run("old command") + terminalProcess.emit("stream_available", stream) + await Promise.resolve() + + const currentProcess = new TerminalProcess(mockTerminalInfo) + const currentExecution = { commandLine: { value: "current" } } as vscode.TerminalShellExecution + currentProcess.ownExecution = currentExecution + mockTerminalInfo.process = currentProcess + mockTerminalInfo.activeShellExecution = currentExecution + mockTerminalInfo.busy = true + mockTerminalInfo.running = true + + rejectNext(streamError) + await oldRun + + expect(mockTerminalInfo.process).toBe(currentProcess) + expect(mockTerminalInfo.activeShellExecution).toBe(currentExecution) + expect(mockTerminalInfo.busy).toBe(true) + expect(mockTerminalInfo.running).toBe(true) + expect(mockTerminalInfo["activeProcesses"]).not.toContain(terminalProcess) + expect(mockTerminalInfo["activeProcesses"]).toContain(currentProcess) + + mockTerminalInfo.handleClose() + }) + it("runs command after shell integration activates via onDidChangeTerminalShellIntegration event", async () => { // Cover Terminal.runCommand's waitForShellIntegration resolve path: shell // integration is initially absent but arrives via the VSCode event before timeout. From b2180f851fd5835befa8e4ba8d6b7b75590f0385 Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 03:55:49 +0000 Subject: [PATCH 21/23] test(terminal): assert completion ownership --- .../terminal/__tests__/TerminalProcess.spec.ts | 8 +++++--- .../terminal/__tests__/TerminalRegistry.spec.ts | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 88e4976bf2..877258d780 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -324,9 +324,11 @@ describe("TerminalProcess", () => { const oldExecution = { commandLine: { value: "old" } } as vscode.TerminalShellExecution mockTerminal.shellIntegration.executeCommand.mockReturnValue(oldExecution) vi.spyOn(console, "error").mockImplementation(() => undefined) + const oldProcess = new TerminalProcess(mockTerminalInfo) + mockTerminalInfo.process = oldProcess - const oldRun = terminalProcess.run("old command") - terminalProcess.emit("stream_available", stream) + const oldRun = oldProcess.run("old command") + oldProcess.emit("stream_available", stream) await Promise.resolve() const currentProcess = new TerminalProcess(mockTerminalInfo) @@ -344,7 +346,7 @@ describe("TerminalProcess", () => { expect(mockTerminalInfo.activeShellExecution).toBe(currentExecution) expect(mockTerminalInfo.busy).toBe(true) expect(mockTerminalInfo.running).toBe(true) - expect(mockTerminalInfo["activeProcesses"]).not.toContain(terminalProcess) + expect(mockTerminalInfo["activeProcesses"]).not.toContain(oldProcess) expect(mockTerminalInfo["activeProcesses"]).toContain(currentProcess) mockTerminalInfo.handleClose() diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 05112019e2..35359c4947 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -831,6 +831,7 @@ describe("TerminalRegistry", () => { const process = new TerminalProcess(terminal) terminal.process = process terminal.busy = true + terminal.running = true const noShellSpy = vi.fn() const completedSpy = vi.fn() const continueSpy = vi.fn() @@ -849,6 +850,7 @@ describe("TerminalRegistry", () => { expect(terminal.process).toBeUndefined() expect(terminal["activeProcesses"].size).toBe(0) expect(terminal.busy).toBe(false) + expect(terminal.running).toBe(false) expect(terminal.isStreamClosed).toBe(true) expect(process["finalizedBeforeExecution"]).toBe(true) expect(process.eventNames()).toEqual([]) From c7f2f16cd749f1927fc694a8dad4142394d8d50b Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 04:06:07 +0000 Subject: [PATCH 22/23] test(terminal): assert completion owner state --- .../__tests__/TerminalProcess.spec.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 877258d780..90554b4235 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -221,6 +221,28 @@ describe("TerminalProcess", () => { expect(terminalProcess.eventNames()).toEqual([]) }) + it("clears busy when the matching process completes", () => { + const process = new TerminalProcess(mockTerminalInfo) + mockTerminalInfo.process = process + mockTerminalInfo.busy = true + + process.emit("completed", "") + + expect(mockTerminalInfo.busy).toBe(false) + }) + + it("keeps a newer owner busy when a superseded process completes", () => { + const superseded = new TerminalProcess(mockTerminalInfo) + const current = new TerminalProcess(mockTerminalInfo) + mockTerminalInfo.process = current + mockTerminalInfo.busy = true + + superseded.emit("completed", "") + + expect(mockTerminalInfo.process).toBe(current) + expect(mockTerminalInfo.busy).toBe(true) + }) + it("emits no_shell_integration with commandSubmitted=false when shell integration startup times out", async () => { vi.useFakeTimers() const previousTimeout = Terminal.getShellIntegrationTimeout() From b0d6ecbbf5231c7991614162638b1ae52072ff6b Mon Sep 17 00:00:00 2001 From: Roomote Date: Sat, 12 Sep 2026 21:41:47 +0000 Subject: [PATCH 23/23] fix(terminal): align model cleanup ownership --- docs/architecture/task-lifecycle-model.md | 2 +- scripts/check-terminal-lifecycle.ts | 24 ++++++++++++++++--- src/integrations/terminal/TerminalProcess.ts | 1 + .../__tests__/TerminalProcess.spec.ts | 1 + 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 01d3d8268d..bc777b9c38 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -53,7 +53,7 @@ Production completion also accepts a recovery-compatible `active` parent that st ## Terminal command lifecycle model -The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, startup or stream failure, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, closure after an error without a late completion, a superseded process failing without clearing the current owner, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. +The same command runs a bounded terminal lifecycle explorer for issue #1362. It models command startup, shell activation, one representative buffered output chunk, normal completion, startup or stream failure, concurrent shell-integration waits, tracked superseded processes, and terminal closure. Process registration is atomic with command startup, normal completion and failure release the current process, and a superseded process can only be introduced while a current process exists. Bounding output presence keeps the reachable state space finite without weakening the cleanup property. Its invariants require completion to remain at-most-once, every attached current process to remain tracked, closure to detach the current process and settle every pending wait and tracked process, buffered output to be delivered, and an active stream iterator to be released. Named landmarks retain the important interleavings: closure before command submission, closure after output, closure after a normal end event, closure after an error without a late completion, a superseded process failing without clearing the current owner, duplicate closure, closure with two pending waits, and closure with a superseded process. The checker fails rather than reporting success if the depth boundary still has an unseen enabled successor. This terminal model is intentionally separate from persisted task delegation state because VS Code terminal events are an extension-host adapter protocol rather than `HistoryItem` transitions. Focused `TerminalRegistry` tests bind the abstract properties to production behavior, including omitted `onDidEndTerminalShellExecution` events and an undefined `exitStatus` during the close callback. diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts index bfc01e1675..e6dababbbf 100644 --- a/scripts/check-terminal-lifecycle.ts +++ b/scripts/check-terminal-lifecycle.ts @@ -68,13 +68,21 @@ function complete(state: ModelState, phase: "completed" | "closed"): ModelState function transition(state: ModelState, action: Action): ModelState { switch (action) { case "run": - return state.phase === "idle" ? { ...state, phase: "waiting", processAttached: true } : state + return state.phase === "idle" + ? { + ...state, + phase: "waiting", + processAttached: true, + processesCreated: state.processesCreated + 1, + trackedProcesses: state.trackedProcesses + 1, + } + : state case "wait": return state.phase !== "closed" && state.waitsCreated < 2 ? { ...state, waitsCreated: state.waitsCreated + 1, pendingWaits: state.pendingWaits + 1 } : state case "track-process": - return state.phase !== "closed" && state.processesCreated < 2 + return state.processAttached && state.processesCreated < 2 ? { ...state, processesCreated: state.processesCreated + 1, @@ -94,7 +102,13 @@ function transition(state: ModelState, action: Action): ModelState { case "output": return state.phase === "running" && state.output === "" ? { ...state, output: "chunk" } : state case "end": - return state.phase === "waiting" || state.phase === "running" ? complete(state, "completed") : state + return state.phase === "waiting" || state.phase === "running" + ? { + ...complete(state, "completed"), + trackedProcesses: state.trackedProcesses - 1, + settledProcesses: state.settledProcesses + 1, + } + : state case "error": return state.phase === "waiting" || state.phase === "running" ? { @@ -139,6 +153,10 @@ function violations(state: ModelState): string[] { } if (state.settledWaits > state.waitsCreated) result.push("more waits settled than were created") if (state.settledProcesses > state.processesCreated) result.push("more processes settled than were created") + if (state.trackedProcesses + state.settledProcesses !== state.processesCreated) { + result.push("process registration and settlement accounting diverged") + } + if (state.processAttached && state.trackedProcesses === 0) result.push("the current process was not tracked") if (state.phase === "closed" && state.commandSubmitted && !state.iteratorReleased) { result.push("closing a submitted command did not release its stream iterator") } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 95b6f96ebe..c53b609cea 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -120,6 +120,7 @@ export class TerminalProcess extends BaseTerminalProcess { } this.emit("completed", output) this.continue() + this.stopHotTimer() this.removeAllListeners() } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 90554b4235..1a16ce086e 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -292,6 +292,7 @@ describe("TerminalProcess", () => { expect(mockTerminalInfo.process).toBeUndefined() expect(mockTerminalInfo["activeProcesses"]).not.toContain(process) + expect(process?.isHot).toBe(false) expect(process?.eventNames()).toEqual([]) expect(vi.getTimerCount()).toBe(0) } finally {