[Fix] Commands stay Running when user closes their terminal

docs/architecture/task-lifecycle-model.md

@@ -6,6 +6,8 @@ Zoo Code checks its persisted task delegation lifecycle with a bounded, exhausti
 pnpm lifecycle:model-check
 ```
 
+`pnpm lifecycle:model` runs the same checks directly; `lifecycle:model-check` is the CI-facing alias.
+
 The check runs in the `compile` CI job after type checking. It fails if it finds an invariant violation, a modeled action becomes unreachable, or exploration exceeds its declared state budget. A violation includes the shortest breadth-first event trace, every intermediate state, and the active bounds so the sequence can be replayed as a focused regression test.
 
 ## Why an executable TypeScript model
@@ -37,6 +39,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:

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 && pnpm cleanup-protocol:model-check",
+		"lifecycle:model": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && pnpm cleanup-protocol:model-check && tsx scripts/check-terminal-lifecycle.ts",
+		"lifecycle:model-check": "pnpm lifecycle:model",
 		"cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts",
 		"test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only",
 		"format": "turbo format --log-order grouped --output-logs new-only",

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<string, (trace: TraceStep[]) => 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<Action>()
+const reachedLandmarks = new Set<string>()
+
+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}`,
+)

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,24 @@ 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
+		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 {
@@ -104,6 +123,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().
@@ -123,13 +147,21 @@ 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)
 
 						// Run the command in the terminal
 						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
@@ -153,22 +185,43 @@ export class Terminal extends BaseTerminal {
 	 * than polling — important for slow-starting shells (heavy .zshrc, nvm, etc.).
 	 */
 	private waitForShellIntegration(timeoutMs: number): Promise<void> {
+		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<void>((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)
 				}
 			})
 		})

src/integrations/terminal/TerminalProcess.ts

@@ -58,6 +58,21 @@ 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 })
+
+		if (executionStarted) {
+			return
+		}
+
+		// 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")
+	}
+
 	public override async run(command: string) {
 		this.command = command
 

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)
 			}
 		})