diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 5e5e0de2bef..23110d05e19 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3474,7 +3474,7 @@ "providers": ["local", "daemon", "ssh", "remote-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": ["local", "ssh"], - "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, persistent single-timer stale-spawn and graceful fallback ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local and daemon providers retain pane/tab identity and reject stale teardown before native or daemon kill; daemon native failures propagate and accepted kills remain listed until onExit or conservative OS proof, while protocol v22 carries identity across process boundaries and legacy adapters fail closed unless main observed the exact pane. Lease-less SSH retry owners survive transport replacement and re-drive the replacement provider. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; unresolved terminal creation may continue for an unrelated host while the removed host remains fenced. Legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", + "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, persistent single-timer stale-spawn and graceful fallback ownership, retry after native failure without deleting the only handle before close is proven, and accept OS process-death proof even when native close throws without onExit. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local and daemon providers retain pane/tab identity and reject stale teardown before native or daemon kill; daemon native failures propagate and accepted kills remain listed until onExit or conservative OS proof. Protocol v22 carries pane identity across process boundaries, v23 carries immutable daemon-session generations through create/list/size/exit, and admission revisions protect preserved legacy daemons while replacement creation is in flight. Lease-less SSH retry owners survive transport replacement and re-drive the replacement provider. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; unresolved terminal creation may continue for an unrelated host while the removed host remains fenced. Legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/8260", "https://github.com/stablyai/orca/pull/8279" @@ -3492,7 +3492,8 @@ "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts" + "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/pty-subprocess.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-host-session-reaping-leak.test.ts src/main/daemon/daemon-server.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-session-exit-fence.test.ts" ], "testFiles": [ "config/scripts/build-relay-windows-node-pty-patch.test.mjs", @@ -3509,6 +3510,7 @@ "src/shared/ssh-types.test.ts", "src/main/providers/local-pty-provider.test.ts", "src/main/daemon/daemon-pty-adapter.test.ts", + "src/main/daemon/daemon-session-exit-fence.test.ts", "src/main/daemon/daemon-pty-router.test.ts", "src/main/daemon/degraded-daemon-pty-provider.test.ts", "src/main/daemon/session.test.ts", @@ -3568,6 +3570,7 @@ "every Windows shutdown attempt stays signal-free and repeats only while physical exit remains unproved", "stale spawn native failures retain one managed PTY and one retry timer beyond the eighth failure", "graceful fallback throws stay contained and re-arm instead of reaching relay-global uncaughtException", + "native close throws followed by OS process-death proof reap immediate and stale-spawn fallback owners without another timer", "whole-relay shutdown retries native failure and waits for exit proof before clearing ownership", "51 concurrent spawn requests produce exactly 50 live PTYs and one cap rejection", "explicit shutdown retains one managed PTY until onExit or OS liveness proves the child is gone" @@ -3651,7 +3654,23 @@ "assertions": [ "daemon shutdown carries pane identity across RPC and rejects a stale kill", "accepted daemon shutdown remains provider-visible until physical exit proof", - "preserved pre-v22 sessions fail closed when restart cannot recover pane identity for attach or shutdown" + "preserved pre-v22 sessions fail closed when restart cannot recover pane identity for attach or shutdown", + "a stale daemon list started before same-id replacement completion cannot finalize the old exit", + "a protocol-v23 exit from an older immutable session generation is rejected without probing" + ] + }, + { + "file": "src/main/daemon/daemon-session-exit-fence.test.ts", + "assertions": [ + "same-id admission completion invalidates a stale absent-list snapshot and recognizes the older immutable generation", + "2,000 finalized session identities leave zero admission, revision, generation, or pending-exit entries", + "an admission finishing after dispose-style clear cannot recreate fence state" + ] + }, + { + "file": "src/main/daemon/daemon-server.test.ts", + "assertions": [ + "one immutable protocol-v23 generation crosses create, list, size, and exit and changes on same-id replacement" ] }, { @@ -3837,6 +3856,15 @@ } ], "evidenceRuns": [ + { + "date": "2026-07-13", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/daemon/daemon-session-exit-fence.test.ts", + "result": "passed", + "durationSeconds": 1, + "summary": "3 deterministic tests passed: replacement admission invalidated a stale absent-list snapshot, immutable generation rejected the old exit identity, 2,000 finalized identities retained zero fence entries, and a post-clear completion could not recreate state." + }, { "date": "2026-07-12", "runner": "local", @@ -4016,6 +4044,15 @@ "result": "passed", "durationSeconds": 10, "summary": "1784 cross-provider ownership tests passed, including bounded pre-handler exit overflow with targeted liveness recovery and both persistence-failure cleanup entry points." + }, + { + "date": "2026-07-13", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-runtime-vm-removal.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts", + "result": "passed", + "durationSeconds": 2, + "summary": "215 tests passed, including immediate and stale-spawn fallback native-throw paths that accept independent OS death proof, release ownership, and retain zero retry timers." } ], "runtimeBudget": { diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 8573cf27004..dd2ffce0844 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -580,6 +580,25 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(exits).toEqual([]) }) + it('rejects an exit from an older daemon session generation without probing', async () => { + const exits: { id: string; code: number }[] = [] + adapter.onExit((payload) => exits.push(payload)) + const { id } = await adapter.spawn({ sessionId: 'generation-reuse', cols: 80, rows: 24 }) + const internals = adapter as unknown as { + readAppliedSize(sessionId: string): Promise<{ status: 'alive' | 'absent' | 'unknown' }> + handleExitEvent(sessionId: string, code: number, sessionGeneration?: string): Promise + exitFence: { rememberGeneration(sessionId: string, generation: string): void } + } + internals.exitFence.rememberGeneration(id, 'replacement-generation') + const readback = vi.spyOn(internals, 'readAppliedSize') + + await internals.handleExitEvent(id, 7, 'old-generation') + + expect(readback).not.toHaveBeenCalled() + expect(adapter.hasPty(id)).toBe(true) + expect(exits).toEqual([]) + }) + it('retains an unknown exit until authoritative listing reconciles it', async () => { const exits: { id: string; code: number }[] = [] adapter.onExit((payload) => exits.push(payload)) @@ -587,7 +606,10 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { const internals = adapter as unknown as { readAppliedSize(sessionId: string): Promise<{ status: 'alive' | 'absent' | 'unknown' }> handleExitEvent(sessionId: string, code: number): Promise - pendingExitProofs: Map + exitFence: { + getPending(sessionId: string): { code: number } | undefined + defer(sessionId: string, exit: { code: number }): void + } activeSessionIds: Set } vi.spyOn(internals, 'readAppliedSize').mockResolvedValueOnce({ status: 'unknown' }) @@ -595,17 +617,49 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { await internals.handleExitEvent(id, 7) expect(adapter.hasPty(id)).toBe(true) - expect(internals.pendingExitProofs.get(id)).toBe(7) + expect(internals.exitFence.getPending(id)?.code).toBe(7) expect(exits).toEqual([]) await adapter.listProcesses() - expect(internals.pendingExitProofs.has(id)).toBe(false) + expect(internals.exitFence.getPending(id)).toBeUndefined() internals.activeSessionIds.add('confirmed-absent') - internals.pendingExitProofs.set('confirmed-absent', 9) + internals.exitFence.defer('confirmed-absent', { code: 9 }) await adapter.listProcesses() expect(exits).toContainEqual({ id: 'confirmed-absent', code: 9 }) }) + + it('retains an old exit when an absent list races a same-id replacement admission', async () => { + const exits: { id: string; code: number }[] = [] + adapter.onExit((payload) => exits.push(payload)) + const { id } = await adapter.spawn({ sessionId: 'same-id-race', cols: 80, rows: 24 }) + const internals = adapter as unknown as { + client: { + request(type: string, payload: unknown): Promise + } + readAppliedSize(sessionId: string): Promise<{ status: 'alive' | 'absent' | 'unknown' }> + handleExitEvent(sessionId: string, code: number): Promise + exitFence: { + beginAdmission(sessionId: string): () => void + rememberGeneration(sessionId: string, generation: string): void + getPending(sessionId: string): { code: number } | undefined + } + } + const completeAdmission = internals.exitFence.beginAdmission(id) + vi.spyOn(internals, 'readAppliedSize').mockResolvedValueOnce({ status: 'unknown' }) + await internals.handleExitEvent(id, 7) + + const request = vi.spyOn(internals.client, 'request') + request.mockResolvedValueOnce({ sessions: [] }) + const staleList = adapter.listProcesses() + completeAdmission() + internals.exitFence.rememberGeneration(id, 'replacement-generation') + await staleList + + expect(adapter.hasPty(id)).toBe(true) + expect(internals.exitFence.getPending(id)?.code).toBe(7) + expect(exits).toEqual([]) + }) }) describe('spawn with sessionId (reattach)', () => { diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 554f7a82013..12bfa0f1122 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -5,6 +5,7 @@ import { basename } from 'node:path' import { existsSync } from 'node:fs' import { DaemonClient } from './client' import { getMacDaemonSystemResolverHealth } from './daemon-health' +import { DaemonSessionExitFence, type PendingDaemonExit } from './daemon-session-exit-fence' import { HistoryManager } from './history-manager' import { HistoryReader, type ColdRestoreInfo } from './history-reader' import { mintPtySessionId, parsePtySessionId } from './pty-session-id' @@ -45,7 +46,7 @@ type ColdRestorePayload = { } type AppliedSizeReadback = - | { status: 'alive'; size: { cols: number; rows: number } } + | { status: 'alive'; size: { cols: number; rows: number }; sessionGeneration?: string } | { status: 'absent' } | { status: 'unknown' } @@ -143,7 +144,7 @@ export class DaemonPtyAdapter implements IPtyProvider { // resume died with the connection. Owe those sessions a resume on the next // connect; the daemon's 5s failsafe covers the window in between. private producerResumesOwedOnReconnect = new Set() - private pendingExitProofs = new Map() + private exitFence = new DaemonSessionExitFence() private static CHECKPOINT_INTERVAL_MS = 5_000 // Why: a streaming session (build logs, `yes`) re-triggers a full multi-MB // snapshot checkpoint on every 5s tick via pending-buffer overflow or the @@ -192,11 +193,18 @@ export class DaemonPtyAdapter implements IPtyProvider { } async spawn(opts: PtySpawnOptions): Promise { - return this.withDaemonRetry(() => this.doSpawn(opts)) + const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId) + const completeAdmission = this.exitFence.beginAdmission(sessionId) + try { + return await this.withDaemonRetry(() => this.doSpawn({ ...opts, sessionId })) + } finally { + completeAdmission() + await this.reconcilePendingExitAfterAdmission(sessionId) + } } private async doSpawn(opts: PtySpawnOptions): Promise { - const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId) + const sessionId = opts.sessionId! const requestedIdentity = getPtyPaneIdentityFromEnv(opts.env) if (this.killedSessionTombstones.has(sessionId)) { @@ -292,6 +300,7 @@ export class DaemonPtyAdapter implements IPtyProvider { let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null let result = await createOrAttach(scrollback) + this.exitFence.rememberGeneration(sessionId, result.sessionGeneration) this.paneIdentityBySessionId.set(sessionId, requestedIdentity) const launchIdentity = (): { launchAgent?: NonNullable } => result.launchAgent ? { launchAgent: result.launchAgent } : {} @@ -347,6 +356,7 @@ export class DaemonPtyAdapter implements IPtyProvider { effectiveCols = restoreInfo.cols effectiveRows = restoreInfo.rows result = await createOrAttach(scrollback) + this.exitFence.rememberGeneration(sessionId, result.sessionGeneration) pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null this.initialCwds.set(sessionId, effectiveCwd) } @@ -465,16 +475,23 @@ export class DaemonPtyAdapter implements IPtyProvider { } async attach(id: string): Promise { - await this.ensureConnected() - if (!this.supportsAuthoritativeBufferSnapshots) { - this.setPtyBackgrounded(id, false) - } + const completeAdmission = this.exitFence.beginAdmission(id) + try { + await this.ensureConnected() + if (!this.supportsAuthoritativeBufferSnapshots) { + this.setPtyBackgrounded(id, false) + } - await this.client.request('createOrAttach', { - sessionId: id, - cols: 80, - rows: 24 - }) + const result = await this.client.request('createOrAttach', { + sessionId: id, + cols: 80, + rows: 24 + }) + this.exitFence.rememberGeneration(id, result.sessionGeneration) + } finally { + completeAdmission() + await this.reconcilePendingExitAfterAdmission(id) + } } hasPty(id: string): boolean { @@ -647,11 +664,17 @@ export class DaemonPtyAdapter implements IPtyProvider { private async readAppliedSize(id: string): Promise { try { - const result = await this.client.request<{ size: { cols: number; rows: number } | null }>( - 'getSize', - { sessionId: id } - ) - return result.size ? { status: 'alive', size: result.size } : { status: 'absent' } + const result = await this.client.request<{ + size: { cols: number; rows: number } | null + sessionGeneration?: string + }>('getSize', { sessionId: id }) + return result.size + ? { + status: 'alive', + size: result.size, + ...(result.sessionGeneration ? { sessionGeneration: result.sessionGeneration } : {}) + } + : { status: 'absent' } } catch { return { status: 'unknown' } } @@ -799,11 +822,15 @@ export class DaemonPtyAdapter implements IPtyProvider { } async listProcesses(): Promise { + const pendingSnapshots = new Map( + this.exitFence.pendingEntries().map(([id]) => [id, this.exitFence.snapshot(id)]) + ) await this.ensureConnected() const result = await this.client.request('listSessions', undefined) for (const session of result.sessions) { this.rememberSessionIdentity(session) if (session.isAlive) { + this.exitFence.rememberGeneration(session.sessionId, session.sessionGeneration) // Why: router startup discovers preserved legacy sessions through this // method; spawn must know the ID is already live before createOrAttach. this.activeSessionIds.add(session.sessionId) @@ -814,11 +841,14 @@ export class DaemonPtyAdapter implements IPtyProvider { const liveIds = new Set( result.sessions.filter((session) => session.isAlive).map((session) => session.sessionId) ) - for (const [id, code] of this.pendingExitProofs) { + for (const [id, exit] of this.exitFence.pendingEntries()) { if (liveIds.has(id)) { - this.pendingExitProofs.delete(id) - } else { - this.finalizeExitEvent(id, code) + this.exitFence.clearPending(id) + } else if ( + pendingSnapshots.has(id) && + this.exitFence.isStable(id, pendingSnapshots.get(id)) + ) { + this.finalizeExitEvent(id, exit.code) } } return result.sessions @@ -863,7 +893,7 @@ export class DaemonPtyAdapter implements IPtyProvider { this.sessionsNeedingFullCheckpoint.clear() this.pausedProducerSessionIds.clear() this.producerResumesOwedOnReconnect.clear() - this.pendingExitProofs.clear() + this.exitFence.clear() this.stopCheckpointTimer() for (const id of ids) { this.coldRestoreCache.delete(id) @@ -939,7 +969,7 @@ export class DaemonPtyAdapter implements IPtyProvider { this.coldRestoreCache.clear() this.pausedProducerSessionIds.clear() this.producerResumesOwedOnReconnect.clear() - this.pendingExitProofs.clear() + this.exitFence.clear() this.removeEventListener?.() this.removeEventListener = null // Why: final checkpoints are written daemon-side in TerminalHost.dispose() @@ -983,7 +1013,7 @@ export class DaemonPtyAdapter implements IPtyProvider { } this.pausedProducerSessionIds.clear() this.producerResumesOwedOnReconnect.clear() - this.pendingExitProofs.clear() + this.exitFence.clear() this.removeEventListener?.() this.removeEventListener = null this.client.disconnect() @@ -1397,17 +1427,41 @@ export class DaemonPtyAdapter implements IPtyProvider { fact: event.payload }) } else if (event.event === 'exit') { - void this.handleExitEvent(event.sessionId, event.payload.code) + void this.handleExitEvent( + event.sessionId, + event.payload.code, + event.payload.sessionGeneration + ) } }) } - private async handleExitEvent(sessionId: string, code: number): Promise { + private async handleExitEvent( + sessionId: string, + code: number, + sessionGeneration?: string + ): Promise { + if (this.exitFence.isStaleGeneration(sessionId, sessionGeneration)) { + return + } + const fenceSnapshot = this.exitFence.snapshot(sessionId) // Why: control and stream sockets can reorder a replacement create reply // ahead of the old process's queued exit. Targeted liveness proof prevents // that stale exit from deleting the replacement's same-id ownership. const readback = await this.readAppliedSize(sessionId) if (readback.status === 'alive') { + if (this.exitFence.isStable(sessionId, fenceSnapshot)) { + this.exitFence.rememberGeneration(sessionId, readback.sessionGeneration) + } + this.exitFence.clearPending(sessionId) + return + } + const exit: PendingDaemonExit = { code, ...(sessionGeneration ? { sessionGeneration } : {}) } + if (!this.exitFence.isStable(sessionId, fenceSnapshot)) { + this.exitFence.defer(sessionId, exit) + // Why: admission may have completed while the old readback was in + // flight, before spawn's finally block could observe this deferred exit. + await this.reconcilePendingExitAfterAdmission(sessionId) return } if (readback.status === 'unknown') { @@ -1418,14 +1472,14 @@ export class DaemonPtyAdapter implements IPtyProvider { // Why: a failed control-socket probe is not absence proof. Retain the // active id until authoritative listing; this also bounds retention to // the adapter's already-owned sessions instead of arbitrary exit ids. - this.pendingExitProofs.set(sessionId, code) + this.exitFence.defer(sessionId, exit) return } this.finalizeExitEvent(sessionId, code) } private finalizeExitEvent(sessionId: string, code: number): void { - this.pendingExitProofs.delete(sessionId) + this.exitFence.forget(sessionId) this.activeSessionIds.delete(sessionId) this.dirtySessionVersions.delete(sessionId) this.pausedProducerSessionIds.delete(sessionId) @@ -1449,6 +1503,25 @@ export class DaemonPtyAdapter implements IPtyProvider { listener({ id: sessionId, code }) } } + + private async reconcilePendingExitAfterAdmission(sessionId: string): Promise { + const pending = this.exitFence.getPending(sessionId) + if (!pending || this.exitFence.isStaleGeneration(sessionId, pending.sessionGeneration)) { + this.exitFence.clearPending(sessionId) + return + } + const fenceSnapshot = this.exitFence.snapshot(sessionId) + const readback = await this.readAppliedSize(sessionId) + if (!this.exitFence.isStable(sessionId, fenceSnapshot)) { + return + } + if (readback.status === 'alive') { + this.exitFence.rememberGeneration(sessionId, readback.sessionGeneration) + this.exitFence.clearPending(sessionId) + } else if (readback.status === 'absent') { + this.finalizeExitEvent(sessionId, pending.code) + } + } } // Why: ENOENT/ECONNREFUSED with syscall 'connect' mean the socket is diff --git a/src/main/daemon/daemon-server.test.ts b/src/main/daemon/daemon-server.test.ts index 3ee224afc4e..b06d1882160 100644 --- a/src/main/daemon/daemon-server.test.ts +++ b/src/main/daemon/daemon-server.test.ts @@ -176,10 +176,52 @@ describe('DaemonServer', () => { expect(result).toMatchObject({ isNew: true, - pid: 55555 + pid: 55555, + sessionGeneration: expect.any(String) }) }) + it('carries one immutable session generation through create, list, size, and exit', async () => { + let subprocess: ReturnType + server = new DaemonServer({ + socketPath, + tokenPath, + spawnSubprocess: () => { + subprocess = createMockSubprocess() + return subprocess + } + }) + await server.start() + const c = await connectClient() + const created = await c.request<{ sessionGeneration: string }>('createOrAttach', { + sessionId: 'generation-session', + cols: 80, + rows: 24 + }) + const listed = await c.request<{ + sessions: { sessionGeneration: string }[] + }>('listSessions', undefined) + const sized = await c.request<{ sessionGeneration: string }>('getSize', { + sessionId: 'generation-session' + }) + const exitEvent = new Promise((resolve) => c.onEvent(resolve)) + + subprocess!._simulateExit(42) + + expect(listed.sessions[0]?.sessionGeneration).toBe(created.sessionGeneration) + expect(sized.sessionGeneration).toBe(created.sessionGeneration) + await expect(exitEvent).resolves.toMatchObject({ + event: 'exit', + payload: { code: 42, sessionGeneration: created.sessionGeneration } + }) + const replacement = await c.request<{ sessionGeneration: string }>('createOrAttach', { + sessionId: 'generation-session', + cols: 80, + rows: 24 + }) + expect(replacement.sessionGeneration).not.toBe(created.sessionGeneration) + }) + it('persists only an allowlisted launch identity across reattach', async () => { await startServer() const c = await connectClient() diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index 9f81a4ad630..2e77f6d7d8e 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -383,7 +383,7 @@ export class DaemonServer { flushMaxChars: DaemonServer.INTERACTIVE_OUTPUT_MAX_CHARS }) }, - onExit: (code) => { + onExit: (code, sessionGeneration) => { // Why: exit tears down renderer handlers, so it must ride the // ordered queue behind final output even when the shallow socket // gate holds that output for a later drain pass. @@ -392,7 +392,7 @@ export class DaemonServer { type: 'event', event: 'exit', sessionId: p.sessionId, - payload: { code } + payload: { code, sessionGeneration } }) this.streamDataBatcher.flush(clientId) recordDaemonStreamBacklogEvent('sessionExit', { @@ -426,7 +426,8 @@ export class DaemonServer { pid: result.pid, shellState: result.shellState, ...(result.launchAgent ? { launchAgent: result.launchAgent } : {}), - ...(result.historySeeded !== undefined ? { historySeeded: result.historySeeded } : {}) + ...(result.historySeeded !== undefined ? { historySeeded: result.historySeeded } : {}), + sessionGeneration: result.sessionGeneration } } @@ -570,7 +571,7 @@ export class DaemonServer { } case 'getSize': - return { size: this.host.getAppliedSize(request.payload.sessionId) } + return this.host.getAppliedSizeWithGeneration(request.payload.sessionId) ?? { size: null } case 'takePendingOutput': // Why no await before this call: with includeSnapshot, drain and diff --git a/src/main/daemon/daemon-session-exit-fence.test.ts b/src/main/daemon/daemon-session-exit-fence.test.ts new file mode 100644 index 00000000000..a925cd14c7c --- /dev/null +++ b/src/main/daemon/daemon-session-exit-fence.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import { DaemonSessionExitFence } from './daemon-session-exit-fence' + +describe('DaemonSessionExitFence', () => { + it('invalidates an absent-list snapshot across same-id admission completion', () => { + const fence = new DaemonSessionExitFence() + fence.rememberGeneration('same-id', 'old-generation') + fence.defer('same-id', { code: 7, sessionGeneration: 'old-generation' }) + const staleListSnapshot = fence.snapshot('same-id') + + const completeAdmission = fence.beginAdmission('same-id') + fence.rememberGeneration('same-id', 'replacement-generation') + completeAdmission() + + expect(fence.isStable('same-id', staleListSnapshot)).toBe(false) + expect(fence.isStaleGeneration('same-id', 'old-generation')).toBe(true) + expect(fence.getPending('same-id')).toEqual({ + code: 7, + sessionGeneration: 'old-generation' + }) + }) + + it('releases every per-session map after repeated exit finalization', () => { + const fence = new DaemonSessionExitFence() + for (let i = 0; i < 2_000; i++) { + const id = `session-${i}` + const completeAdmission = fence.beginAdmission(id) + fence.rememberGeneration(id, `generation-${i}`) + fence.defer(id, { code: i }) + completeAdmission() + fence.forget(id) + } + const internals = fence as unknown as Record< + 'revisions' | 'admissions' | 'sessionGenerations' | 'pendingExits', + Map + > + + expect(internals.revisions.size).toBe(0) + expect(internals.admissions.size).toBe(0) + expect(internals.sessionGenerations.size).toBe(0) + expect(internals.pendingExits.size).toBe(0) + }) + + it('does not recreate fence state when an admission completes after clear', () => { + const fence = new DaemonSessionExitFence() + const completeAdmission = fence.beginAdmission('late-session') + fence.clear() + + completeAdmission() + + expect(fence.snapshot('late-session')).toBeUndefined() + }) +}) diff --git a/src/main/daemon/daemon-session-exit-fence.ts b/src/main/daemon/daemon-session-exit-fence.ts new file mode 100644 index 00000000000..c0213166e25 --- /dev/null +++ b/src/main/daemon/daemon-session-exit-fence.ts @@ -0,0 +1,97 @@ +export type PendingDaemonExit = { + code: number + sessionGeneration?: string +} + +export class DaemonSessionExitFence { + // Why global-monotonic: forgetting and then reusing the same id must not + // recreate a revision that an older async list snapshot can mistake as live. + private nextRevision = 0 + private clearEpoch = 0 + private revisions = new Map() + private admissions = new Map() + private sessionGenerations = new Map() + private pendingExits = new Map() + + beginAdmission(sessionId: string): () => void { + const admissionEpoch = this.clearEpoch + this.bump(sessionId) + this.admissions.set(sessionId, (this.admissions.get(sessionId) ?? 0) + 1) + let completed = false + return () => { + if (completed) { + return + } + completed = true + // Why: dispose/fanout clears every owner; a late spawn finally block + // must not recreate per-session fence state after that terminal event. + if (admissionEpoch !== this.clearEpoch) { + return + } + const remaining = (this.admissions.get(sessionId) ?? 1) - 1 + if (remaining === 0) { + this.admissions.delete(sessionId) + } else { + this.admissions.set(sessionId, remaining) + } + this.bump(sessionId) + } + } + + rememberGeneration(sessionId: string, generation: string | undefined): void { + if (!generation || this.sessionGenerations.get(sessionId) === generation) { + return + } + this.sessionGenerations.set(sessionId, generation) + this.bump(sessionId) + } + + isStaleGeneration(sessionId: string, generation: string | undefined): boolean { + const current = this.sessionGenerations.get(sessionId) + return generation !== undefined && current !== undefined && generation !== current + } + + snapshot(sessionId: string): number | undefined { + return this.revisions.get(sessionId) + } + + isStable(sessionId: string, snapshot: number | undefined): boolean { + return !this.admissions.has(sessionId) && this.revisions.get(sessionId) === snapshot + } + + defer(sessionId: string, exit: PendingDaemonExit): void { + this.pendingExits.set(sessionId, exit) + } + + getPending(sessionId: string): PendingDaemonExit | undefined { + return this.pendingExits.get(sessionId) + } + + pendingEntries(): [string, PendingDaemonExit][] { + return [...this.pendingExits] + } + + clearPending(sessionId: string): void { + this.pendingExits.delete(sessionId) + } + + forget(sessionId: string): void { + this.bump(sessionId) + this.pendingExits.delete(sessionId) + this.sessionGenerations.delete(sessionId) + this.admissions.delete(sessionId) + this.revisions.delete(sessionId) + } + + clear(): void { + this.clearEpoch++ + this.pendingExits.clear() + this.sessionGenerations.clear() + this.admissions.clear() + this.revisions.clear() + } + + private bump(sessionId: string): void { + this.revisions.set(sessionId, ++this.nextRevision) + } +} diff --git a/src/main/daemon/daemon-stream-events.ts b/src/main/daemon/daemon-stream-events.ts index 1644642bd09..2f3e4ac9554 100644 --- a/src/main/daemon/daemon-stream-events.ts +++ b/src/main/daemon/daemon-stream-events.ts @@ -12,7 +12,7 @@ export type ExitEvent = { type: 'event' event: 'exit' sessionId: string - payload: { code: number } + payload: { code: number; sessionGeneration?: string } } export type TerminalErrorEvent = { diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index 7631c776f34..c8927ce941a 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -510,7 +510,7 @@ describe('Session', () => { expect(onData).not.toHaveBeenCalled() expect(onExit).toHaveBeenCalledTimes(1) - expect(onExit).toHaveBeenCalledWith(-1) + expect(onExit).toHaveBeenCalledWith(-1, expect.any(String)) expect(session.exitCode).toBe(-1) }) }) diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 98fc2f9f173..68900850f78 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -1,4 +1,5 @@ /* oxlint-disable max-lines */ +import { randomUUID } from 'node:crypto' import { HeadlessEmulator } from './headless-emulator' import { isValidPtySize, normalizePtySize } from './daemon-pty-size' import { PostReadyFlushGate } from './post-ready-flush-gate' @@ -104,11 +105,12 @@ export type SessionOptions = { type AttachedClient = { token: symbol onData: (data: string) => void - onExit: (code: number) => void + onExit: (code: number, sessionGeneration: string) => void } export class Session { readonly sessionId: string + readonly generation = randomUUID() readonly paneKey: string | null readonly tabId: string | null readonly terminalHandle: string | null @@ -338,7 +340,10 @@ export class Session { this.subprocess.signal(sig) } - attachClient(client: { onData: (data: string) => void; onExit: (code: number) => void }): symbol { + attachClient(client: { + onData: (data: string) => void + onExit: (code: number, sessionGeneration: string) => void + }): symbol { const token = Symbol('attach') this.attachedClients.push({ token, ...client }) return token @@ -503,7 +508,7 @@ export class Session { this.emulator.dispose() for (const client of clientsToNotify) { - client.onExit(-1) + client.onExit(-1, this.generation) } } @@ -678,7 +683,7 @@ export class Session { } for (const client of this.attachedClients) { - client.onExit(code) + client.onExit(code, this.generation) } // Why: hand off to the owner's reaper so the emulator is disposed and the diff --git a/src/main/daemon/terminal-host-create-contract.ts b/src/main/daemon/terminal-host-create-contract.ts index 3fa99a108a5..412c13e0528 100644 --- a/src/main/daemon/terminal-host-create-contract.ts +++ b/src/main/daemon/terminal-host-create-contract.ts @@ -21,7 +21,10 @@ export type CreateOrAttachOptions = { shellReadySupported?: boolean shellReadyTimeoutMs?: number historySeed?: string - streamClient: { onData: (data: string) => void; onExit: (code: number) => void } + streamClient: { + onData: (data: string) => void + onExit: (code: number, sessionGeneration: string) => void + } } export type CreateOrAttachResult = { @@ -31,5 +34,6 @@ export type CreateOrAttachResult = { shellState: ShellReadyState historySeeded?: boolean launchAgent?: TuiAgent + sessionGeneration: string attachToken: symbol } diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 3e1ecf64379..3cfda1241b6 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -84,6 +84,7 @@ export class TerminalHost { shellState: existing.shellState, ...(existing.launchAgent ? { launchAgent: existing.launchAgent } : {}), ...(existing.historySeeded !== undefined ? { historySeeded: existing.historySeeded } : {}), + sessionGeneration: existing.generation, attachToken: token } } @@ -176,6 +177,7 @@ export class TerminalHost { shellState: session.shellState, ...(session.launchAgent ? { launchAgent: session.launchAgent } : {}), ...(session.historySeeded !== undefined ? { historySeeded: session.historySeeded } : {}), + sessionGeneration: session.generation, attachToken: token } } @@ -312,6 +314,14 @@ export class TerminalHost { return session.getAppliedSize() } + getAppliedSizeWithGeneration( + sessionId: string + ): { size: { cols: number; rows: number }; sessionGeneration: string } | null { + const session = this.sessions.get(sessionId) + const size = session?.isAlive ? session.getAppliedSize() : null + return session && size ? { size, sessionGeneration: session.generation } : null + } + // Why: same null-not-throw semantics as getSnapshot — incremental // checkpoints are best-effort against sessions that may have just exited. takePendingOutput( @@ -339,6 +349,7 @@ export class TerminalHost { const size = session.getAppliedSize() result.push({ sessionId: session.sessionId, + sessionGeneration: session.generation, ...(session.paneKey ? { paneKey: session.paneKey } : {}), ...(session.tabId ? { tabId: session.tabId } : {}), state: session.state, diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index 7941bf1ba52..4148d5e945e 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -15,9 +15,9 @@ import type { TuiAgent } from '../../shared/types' import type * as PtyIdentity from '../pty/pty-shutdown-identity' // Why: daemons survive app updates; bump for wire changes so old daemons do // not accept handshakes for unsupported RPC behavior. -export const PROTOCOL_VERSION = 22 +export const PROTOCOL_VERSION = 23 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22 ] as const // ─── Session State Machine ────────────────────────────────────────── @@ -364,7 +364,7 @@ export type CreateOrAttachResult = { shellState: ShellReadyState historySeeded?: boolean launchAgent?: TuiAgent -} +} & { sessionGeneration?: string } export type GetSnapshotResult = { snapshot: TerminalSnapshot | null } @@ -379,7 +379,7 @@ export type SystemResolverHealthResult = { health: SystemResolverHealth } -export type SessionInfo = PtyIdentity.PtyPaneIdentityFields & { +export type SessionInfo = PtyIdentity.PtyPaneIdentityFields & { sessionGeneration?: string } & { sessionId: string state: SessionState shellState: ShellReadyState diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index 9b69fb40f66..21174660461 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -774,6 +774,32 @@ describe('PtyHandler', () => { } }) + it('reaps a stale spawn when fallback kill throws after OS exit proof', async () => { + const killSpy = vi + .fn() + .mockImplementationOnce(() => {}) + .mockImplementationOnce(() => { + throw new Error('native handle already closed') + }) + mockPtySpawn.mockReturnValue({ + ...mockPtyInstance, + kill: killSpy, + onData: vi.fn(), + onExit: vi.fn() + }) + const aliveSpy = vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(false) + try { + await dispatcher.callRequest('pty.spawn', {}, { isStale: () => true }) + vi.advanceTimersByTime(5000) + } finally { + aliveSpy.mockRestore() + } + + expect(killSpy).toHaveBeenCalledTimes(2) + expect(handler.activePtyCount).toBe(0) + expect(vi.getTimerCount()).toBe(0) + }) + it('retains stale-spawn ownership beyond the eighth native failure', async () => { let onExitCb: ((event: { exitCode: number }) => void) | null = null const killSpy = vi.fn() @@ -1379,6 +1405,31 @@ describe('PtyHandler', () => { } }) + it('accepts OS exit proof when immediate native shutdown throws after process death', async () => { + const killSpy = vi.fn(() => { + throw new Error('native handle already closed') + }) + mockPtySpawn.mockReturnValue({ + ...mockPtyInstance, + kill: killSpy, + onData: vi.fn(), + onExit: vi.fn() + }) + const aliveSpy = vi.spyOn(ptyShellUtils, 'isProcessAlive').mockReturnValue(false) + try { + await dispatcher.callRequest('pty.spawn', {}) + await expect( + dispatcher.callRequest('pty.shutdown', { id: 'pty-1', immediate: true }) + ).resolves.toBeUndefined() + } finally { + aliveSpy.mockRestore() + } + + expect(killSpy).toHaveBeenCalledTimes(1) + expect(handler.activePtyCount).toBe(0) + expect(vi.getTimerCount()).toBe(0) + }) + it('does not arm Windows graceful fallback until native shutdown succeeds', async () => { const platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') let onExitCb: ((event: { exitCode: number }) => void) | null = null diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 4ddba5c6229..9958f18ca89 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -534,6 +534,11 @@ export class PtyHandler { killPtyForShutdown(still, 'SIGKILL') state.forceKillAccepted = true } catch (error) { + // Why: native close can throw after the OS has already reaped the + // child. Accept that independent proof before retaining a retry. + if (this.reapManagedPtyIfProvablyExited(still)) { + return + } state.failedAttempts += 1 if (state.failedAttempts <= 2 || state.failedAttempts === RETAINED_KILL_LOG_MILESTONE) { process.stderr.write( @@ -936,13 +941,27 @@ export class PtyHandler { if (immediate) { this.releaseStartupCommand(managed) this.flushPtyOutput(id) - killPtyForShutdown(managed, 'SIGKILL') + try { + killPtyForShutdown(managed, 'SIGKILL') + } catch (error) { + if (this.reapManagedPtyIfProvablyExited(managed)) { + return + } + throw error + } // Why: kill returning is only acceptance. Preserve the PTY and listeners // unless synchronous onExit or an ESRCH readback proves physical exit. this.reapManagedPtyIfProvablyExited(managed) } else { this.releaseStartupCommand(managed) - killPtyForShutdown(managed, 'SIGTERM') + try { + killPtyForShutdown(managed, 'SIGTERM') + } catch (error) { + if (this.reapManagedPtyIfProvablyExited(managed)) { + return + } + throw error + } // Why: Some processes ignore SIGTERM (e.g. a hung child, a custom signal // handler). Without a SIGKILL fallback the PTY process would leak and the