diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 9266d49987..bc777b9c38 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. @@ -50,6 +51,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, 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. + ## 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..c830f49e2f 100644 --- a/package.json +++ b/package.json @@ -13,7 +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-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-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", diff --git a/scripts/check-terminal-lifecycle.ts b/scripts/check-terminal-lifecycle.ts new file mode 100644 index 0000000000..e6dababbbf --- /dev/null +++ b/scripts/check-terminal-lifecycle.ts @@ -0,0 +1,269 @@ +type Phase = "idle" | "waiting" | "running" | "completed" | "failed" | "closed" +type Action = "run" | "wait" | "track-process" | "activate" | "output" | "end" | "error" | "error-superseded" | "close" + +interface ModelState { + phase: Phase + processAttached: boolean + commandSubmitted: boolean + completionCount: number + output: string + deliveredOutput: string + iteratorReleased: boolean + waitsCreated: number + pendingWaits: number + settledWaits: number + processesCreated: number + trackedProcesses: number + settledProcesses: number +} + +interface TraceStep { + action: Action | "initial" + state: ModelState +} + +const actions: Action[] = [ + "run", + "wait", + "track-process", + "activate", + "output", + "end", + "error", + "error-superseded", + "close", +] +const MAX_DEPTH = 9 +const MAX_STATES = 500 + +function initialState(): ModelState { + return { + phase: "idle", + processAttached: false, + commandSubmitted: false, + completionCount: 0, + output: "", + deliveredOutput: "", + iteratorReleased: false, + waitsCreated: 0, + pendingWaits: 0, + settledWaits: 0, + processesCreated: 0, + trackedProcesses: 0, + settledProcesses: 0, + } +} + +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, + 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.processAttached && state.processesCreated < 2 + ? { + ...state, + processesCreated: state.processesCreated + 1, + trackedProcesses: state.trackedProcesses + 1, + } + : state + case "activate": + 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"), + trackedProcesses: state.trackedProcesses - 1, + settledProcesses: state.settledProcesses + 1, + } + : 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 "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 + : { + ...complete(state, "closed"), + pendingWaits: 0, + settledWaits: state.settledWaits + state.pendingWaits, + trackedProcesses: 0, + settledProcesses: state.settledProcesses + state.trackedProcesses, + } + } +} + +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.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.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") + } + 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, + "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, + "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" && + 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() +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() +const frontier: ModelState[] = [] + +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) { + frontier.push(node.state) + 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 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)) +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/Terminal.ts b/src/integrations/terminal/Terminal.ts index 21f98b86c6..b985d43fcb 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -11,6 +11,9 @@ import { mergePromise } from "./mergePromise" 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 @@ -74,7 +77,31 @@ 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 + } + + /** Finalizes any attached command when VS Code disposes this terminal. */ + public handleClose(): void { + if (this.closed) { + return + } + + this.closed = true + for (const cancel of this.cancelShellIntegrationWaits) { + cancel() + } + + if (this.activeProcesses.size > 0) { + for (const process of this.activeProcesses) { + process.handleTerminalClosed() + } + } else if (this.process instanceof TerminalProcess) { + this.process.handleError() + } else { + this.activeShellExecution = undefined + this.setActiveStream(undefined) + this.shellExecutionComplete({ exitCode: undefined }) + } } public override runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise { @@ -102,8 +129,14 @@ export class Terminal extends BaseTerminal { process.once("error", (error) => { console.error(`[Terminal ${this.id}] error:`, error) reject(error) + process.handleError() }) + 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(). @@ -123,6 +156,10 @@ export class Terminal extends BaseTerminal { // customised startup that suppresses the OSC 633;A marker). this.waitForShellIntegration(Terminal.getShellIntegrationTimeout()) .then(() => { + if (this.isClosed()) { + return + } + // Clean up temporary directory if shell integration is available, zsh did its job: ShellIntegrationManager.zshCleanupTmpDir(this.id) @@ -130,6 +167,10 @@ export class Terminal extends BaseTerminal { void process.run(command).catch((error) => process.emit("error", error)) }) .catch(() => { + if (this.isClosed()) { + return + } + console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`) // Clean up temporary directory if shell integration is not available @@ -146,6 +187,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 @@ -153,22 +204,41 @@ 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`)) + + this.cancelShellIntegrationWaits.delete(cancel) + + 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.cancelShellIntegrationWaits.add(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..c53b609cea 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -30,21 +30,27 @@ 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 + private errorHandled = false constructor(terminal: Terminal) { super() this.terminalRef = new WeakRef(terminal) + terminal.trackProcess(this) this.once("completed", () => { - this.terminal.busy = false + this.terminal.releaseProcess(this) + if (this.terminal.process === this) { + this.terminal.busy = false + } }) + this.once("shell_execution_complete", () => this.terminal.releaseProcess(this)) + this.once("no_shell_integration", () => { - this.emit("completed", "") - this.terminal.busy = false - this.terminal.setActiveStream(undefined) - this.continue() + this.completeBeforeExecution("") }) } @@ -58,6 +64,66 @@ export class TerminalProcess extends BaseTerminalProcess { return terminal } + /** Completes this process when its terminal closes without an execution-end event. */ + public handleTerminalClosed(): void { + if (this.terminalCloseHandled || this.finalizedBeforeExecution || this.errorHandled) { + return + } + this.terminalCloseHandled = true + + const executionStarted = this.ownExecution !== undefined + if (this.terminal.process === this) { + this.terminal.shellExecutionComplete({ exitCode: undefined }) + } else { + this.emit("shell_execution_complete", { 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.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.busy = false + terminal.running = false + terminal.process = undefined + } + terminal.releaseProcess(this) + this.stopHotTimer() + this.removeAllListeners() + } + + private completeBeforeExecution(output: string): void { + this.finalizedBeforeExecution = true + + const terminal = this.terminal + if (terminal.process === this) { + terminal.activeShellExecution = undefined + terminal.setActiveStream(undefined) + terminal.busy = false + terminal.running = false + terminal.process = undefined + } + this.emit("completed", output) + this.continue() + this.stopHotTimer() + this.removeAllListeners() + } + public override async run(command: string) { this.command = command @@ -76,13 +142,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 } @@ -184,6 +243,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 @@ -228,7 +288,6 @@ export class TerminalProcess extends BaseTerminalProcess { "", ) - this.terminal.busy = false this.cleanupScriptFile() // Emit continue event to allow execution to proceed @@ -298,6 +357,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), @@ -307,13 +367,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. @@ -506,15 +570,13 @@ 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", ``, ) this.emit("continue") + this.handleError() } } } 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__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 31bb806be3..1a16ce086e 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -59,10 +59,60 @@ 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("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) const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined) + const initialProcessCount = mockTerminalInfo["activeProcesses"].size const commandPromise = mockTerminalInfo.runCommand("test command", { onLine: vi.fn(), @@ -73,11 +123,126 @@ 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() }) + 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("finalizes an error only once", () => { + mockTerminalInfo.busy = true + mockTerminalInfo.running = true + terminalProcess.isHot = 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.isHot).toBe(false) + 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() @@ -107,6 +272,109 @@ 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?.isHot).toBe(false) + expect(process?.eventNames()).toEqual([]) + expect(vi.getTimerCount()).toBe(0) + } finally { + Terminal.setShellIntegrationTimeout(previousTimeout) + vi.useRealTimers() + } + }) + + 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("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 oldProcess = new TerminalProcess(mockTerminalInfo) + mockTerminalInfo.process = oldProcess + + const oldRun = oldProcess.run("old command") + oldProcess.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(oldProcess) + 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. diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index f60c0d0722..35359c4947 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(), })) @@ -209,6 +216,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 +230,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() } @@ -237,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 () => { @@ -291,6 +310,719 @@ 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(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") + const cleanupSpy = vi.spyOn(ShellIntegrationManager, "zshCleanupTmpDir") + + 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(closed.isStreamClosed).toBe(true) + expect(cleanupSpy).toHaveBeenCalledWith(closed.id) + 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 + const closeSpy = vi.spyOn(registered, "handleClose") + + closeHandler(foreign) + + expect(closeSpy).not.toHaveBeenCalled() + expect(TerminalRegistry["terminals"]).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 + 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(), + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + + await vi.waitFor(() => expect(executeCommand).toHaveBeenCalledOnce()) + await startHandler({ terminal: terminal.terminal, execution }) + await waitingForNext + closeHandler(terminal.terminal) + await settleWithin(result) + + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("hello\n", process) + 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() + 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, + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + Object.defineProperty(terminal.terminal, "exitStatus", { + value: { code: undefined, reason: 3 }, + configurable: true, + }) + + closeHandler(terminal.terminal) + await settleWithin(result) + + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + 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, + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + 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 settleWithin(result) + + expect(executeCommand).not.toHaveBeenCalled() + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + 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, + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + + expect(terminal.terminal.exitStatus).toBeUndefined() + const shellCompleteSpy = vi.spyOn(terminal, "shellExecutionComplete") + terminal.handleClose() + terminal.handleClose() + await settleWithin(result) + + expect(terminal.isClosed()).toBe(true) + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + 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 () => { + 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, + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + await settleWithin(result) + + expect(executeCommand).not.toHaveBeenCalled() + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + 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 settleWithin(wait) + + expect(settledSpy).toHaveBeenCalledOnce() + expect(disposeSpy).toHaveBeenCalledOnce() + expect(terminal["cancelShellIntegrationWaits"].size).toBe(0) + }) + + 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 second = terminal["waitForShellIntegration"](100) + + expect(terminal["cancelShellIntegrationWaits"].size).toBe(2) + handlers[0]({ terminal: terminal.terminal, shellIntegration: {} as never }) + await settleWithin(first) + expect(terminal["cancelShellIntegrationWaits"].size).toBe(1) + + handlers[1]({ terminal: terminal.terminal, shellIntegration: {} as never }) + await settleWithin(second) + expect(terminal["cancelShellIntegrationWaits"].size).toBe(0) + }) + + 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("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() + 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: completedSpy, + onShellExecutionStarted: vi.fn(), + onShellExecutionComplete: completionSpy, + onNoShellIntegration: vi.fn(), + }) + const process = terminal.process + expect(process).toBeInstanceOf(TerminalProcess) + const emitSpy = vi.spyOn(process!, "emit") + + terminal.handleClose() + expect(completedSpy).toHaveBeenCalledOnce() + expect(completedSpy).toHaveBeenCalledWith("", process) + expect(completionSpy).toHaveBeenCalledOnce() + expect(completionSpy).toHaveBeenCalledWith({ exitCode: undefined }, process) + await result + + expect(disposeSpy).toHaveBeenCalledOnce() + expect(vi.getTimerCount()).toBe(0) + expect(emitSpy).not.toHaveBeenCalledWith("no_shell_integration", expect.anything()) + 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 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]) + + 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(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) + }) + + 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 }) + 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() + 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(completedSpy).toHaveBeenCalledWith("") + expect(continueSpy).toHaveBeenCalledOnce() + 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([]) + 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 + 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) + }) + + 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) + }) + + 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) + + const [cancel] = terminal["cancelShellIntegrationWaits"] + cancel?.() + 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) + }) + + 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 + 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",