diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index a23aab79de9..2446b0b1438 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3491,7 +3491,8 @@ "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts", "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/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" ], "testFiles": [ "config/scripts/build-relay-windows-node-pty-patch.test.mjs", @@ -3508,6 +3509,8 @@ "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-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", @@ -3715,7 +3718,10 @@ }, { "file": "src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts", - "assertions": ["ownerless pre-handler exits evict oldest state at the 64-id bound"] + "assertions": [ + "ownerless pre-handler exits evict oldest state at the 64-id bound", + "an evicted legitimate exit triggers one targeted liveness readback and disconnects the dead pane" + ] }, { "file": "src/renderer/src/lib/pty-kill-retry-ownership.test.ts", @@ -3743,10 +3749,24 @@ "relay generation reuse removes stale retry ownership without touching the new PTY", "lease-less SSH shutdown survives provider unregister and re-drives the replacement transport", "accepted local and SSH kills retain retry ownership until provider readback proves physical exit", + "runtime-owned and renderer SSH spawn persistence failures retain exact retry ownership after cleanup rejection and clear only after retry success", "an old provider exit readback cannot settle replacement-provider retry ownership", "unprovable pre-v22 restart intent retires once without synthesizing exit, retrying forever, or killing by recycled id" ] }, + { + "file": "src/main/daemon/daemon-pty-router.test.ts", + "assertions": [ + "upgrade routing advertises exit-proof shutdown and retains the legacy adapter until onExit or readback", + "a late legacy exit cannot delete a replacement route after PTY id reuse" + ] + }, + { + "file": "src/main/daemon/degraded-daemon-pty-provider.test.ts", + "assertions": [ + "degraded routing advertises exit-proof shutdown and retains the exact child provider until onExit or readback" + ] + }, { "file": "src/main/ipc/runtime-terminal-close-retry-owner.test.ts", "assertions": [ @@ -3978,6 +3998,24 @@ "result": "passed", "durationSeconds": 9.8, "summary": "1781 cross-provider ownership tests passed, including whole-relay exit-proof drain and an atomic 50-live-PTY cap under 51 concurrent spawn requests." + }, + { + "date": "2026-07-12", + "runner": "local", + "platform": "macos", + "command": "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", + "result": "passed", + "durationSeconds": 9.3, + "summary": "1414 daemon/provider/runtime tests passed, adding composite-provider exit-proof routing, late-exit generation fencing, and retained persistence-failure cleanup retries." + }, + { + "date": "2026-07-12", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts", + "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." } ], "runtimeBudget": { diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index eb8acb86488..f088889e7bb 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -122,6 +122,41 @@ function createAdapter( } describe('DaemonPtyRouter', () => { + it('retains the exact adapter route until exit proof', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['shared-session']) + vi.mocked(legacy.shutdown).mockResolvedValueOnce(undefined) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + expect(router.requiresShutdownExitProof).toBe(true) + await router.shutdown('shared-session', { immediate: true }) + router.write('shared-session', 'before-exit') + + expect(legacy.write).toHaveBeenCalledWith('shared-session', 'before-exit') + legacy.emitExit('shared-session', 0) + router.write('shared-session', 'after-exit') + expect(current.write).toHaveBeenCalledWith('shared-session', 'after-exit') + }) + + it('fences a late legacy exit after readback and id reuse', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['shared-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + const exit = vi.fn() + router.onExit(exit) + await router.discoverLegacySessions() + + await router.shutdown('shared-session', { immediate: true }) + await router.listProcesses() + await router.spawn({ sessionId: 'shared-session', cols: 80, rows: 24 }) + legacy.emitExit('shared-session', 0) + router.write('shared-session', 'replacement') + + expect(current.write).toHaveBeenCalledWith('shared-session', 'replacement') + expect(exit).not.toHaveBeenCalled() + }) + it('routes fresh foreground confirmation to the session-owning daemon', async () => { const current = createAdapter('current', ['current-session']) const legacy = createAdapter('legacy', ['legacy-session']) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 4f94432a65b..a5286e9537b 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -1,4 +1,5 @@ import type { DaemonPtyAdapter } from './daemon-pty-adapter' +import { listProviderProcessesAndReconcileRoutes } from './pty-provider-route-reconciliation' import type { IPtyProvider, PtyBackgroundStreamEvent, @@ -10,6 +11,7 @@ import type { } from '../providers/types' export class DaemonPtyRouter implements IPtyProvider { + readonly requiresShutdownExitProof = true private current: DaemonPtyAdapter private legacy: DaemonPtyAdapter[] private sessionAdapters = new Map() @@ -33,7 +35,15 @@ export class DaemonPtyRouter implements IPtyProvider { } }), adapter.onExit((payload) => { - this.sessionAdapters.delete(payload.id) + // Why: a late exit from a legacy daemon must not delete a route that + // has already been rebound to a replacement provider generation. + const routed = this.sessionAdapters.get(payload.id) + if (routed && routed !== adapter) { + return + } + if (routed === adapter) { + this.sessionAdapters.delete(payload.id) + } for (const listener of this.exitListeners) { listener(payload) } @@ -97,15 +107,8 @@ export class DaemonPtyRouter implements IPtyProvider { async shutdown(id: string, opts: PtyShutdownOptions): Promise { await this.adapterFor(id).shutdown(id, opts) - // Why: sleep passes keepHistory=true and re-spawns against the same - // sessionId on wake. If we delete the routing entry here, adapterFor() - // falls back to `this.current` on wake — for a session that originally - // lived on a legacy adapter (different protocolVersion), the wake-side - // createOrAttach lands on the wrong adapter and creates a fresh session, - // losing the cold-restore from the legacy adapter's history dir. - if (!opts.keepHistory) { - this.sessionAdapters.delete(id) - } + // Why: native shutdown acceptance is not exit proof. onExit or a later + // successful listProcesses readback retires the exact adapter route. } async sendSignal(id: string, signal: string): Promise { @@ -162,8 +165,7 @@ export class DaemonPtyRouter implements IPtyProvider { async listProcesses(): Promise { // Why: runtime exact-stop/liveness flows must fail closed if any adapter // cannot provide a trustworthy process list. - const results = await Promise.all(this.allAdapters().map((adapter) => adapter.listProcesses())) - return results.flat() + return listProviderProcessesAndReconcileRoutes(this.allAdapters(), this.sessionAdapters) } async getDefaultShell(): Promise { diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index 188173816db..ef9abccbf31 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -109,6 +109,25 @@ function createDaemonAdapter( } describe('DegradedDaemonPtyProvider', () => { + it('retains the exact child route until exit proof', async () => { + const daemonSessions = ['shared-session'] + const current = createDaemonAdapter('daemon', daemonSessions) + const fallback = createProvider('fallback') + vi.mocked(current.shutdown).mockResolvedValueOnce(undefined) + const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) + await provider.discoverDaemonSessions() + + expect(provider.requiresShutdownExitProof).toBe(true) + await provider.shutdown('shared-session', { immediate: true }) + provider.write('shared-session', 'before-exit') + + expect(current.write).toHaveBeenCalledWith('shared-session', 'before-exit') + daemonSessions.length = 0 + current.emitExit('shared-session', 0) + provider.write('shared-session', 'after-exit') + expect(fallback.write).toHaveBeenCalledWith('shared-session', 'after-exit') + }) + it('routes fresh foreground confirmation to the session owner', async () => { const current = createDaemonAdapter('daemon', ['daemon-session']) const fallback = createProvider('fallback') diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index 572cb0aca37..054ba1db04c 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -1,5 +1,6 @@ import type { DaemonPtyAdapter } from './daemon-pty-adapter' import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shutdown' +import * as PtyRoutes from './pty-provider-route-reconciliation' import type { IPtyProvider, PtyBackgroundStreamEvent, @@ -15,12 +16,12 @@ type ManagedPtyProvider = IPtyProvider & { } export class DegradedDaemonPtyProvider implements IPtyProvider { + readonly requiresShutdownExitProof = true readonly routesFreshSpawnsToLocalProvider = true // Why: the preserved daemon answers protocol but cannot spawn fresh PTYs. // Surfaced (e.g. via pty:management:listSessions) so the UI can warn that // new terminals are running without daemon persistence until a restart. readonly isDegraded = true - private current: DaemonPtyAdapter private legacy: DaemonPtyAdapter[] private fallback: ManagedPtyProvider @@ -50,7 +51,15 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } }), provider.onExit((payload) => { - this.sessionProviders.delete(payload.id) + // Why: a late exit from an old daemon must not delete a route that + // has already been rebound to the local fallback or another daemon. + const routed = this.sessionProviders.get(payload.id) + if (routed && routed !== provider) { + return + } + if (routed === provider) { + this.sessionProviders.delete(payload.id) + } for (const listener of this.exitListeners) { listener(payload) } @@ -114,9 +123,8 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise { await this.providerFor(id).shutdown(id, opts) - if (!opts.keepHistory) { - this.sessionProviders.delete(id) - } + // Why: shutdown acceptance is not physical-exit proof. Keep routing until + // the owning provider emits onExit or listProcesses proves the id absent. } async sendSignal(id: string, signal: string): Promise { @@ -173,10 +181,10 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } async listProcesses(): Promise { - const results = await Promise.all( - this.allProviders().map((provider) => provider.listProcesses()) + return PtyRoutes.listProviderProcessesAndReconcileRoutes( + this.allProviders(), + this.sessionProviders ) - return results.flat() } async getDefaultShell(): Promise { @@ -286,7 +294,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } getCurrentDaemonSessionIds(): string[] { - return this.sessionIdsForProvider(this.current) + return PtyRoutes.providerSessionIds(this.sessionProviders, this.current) } fanoutCurrentDaemonSyntheticExits(code: number): void { @@ -336,12 +344,6 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { return null } - private sessionIdsForProvider(provider: ManagedPtyProvider): string[] { - return [...this.sessionProviders] - .filter(([, mappedProvider]) => mappedProvider === provider) - .map(([id]) => id) - } - private daemonAdapterFor(sessionId: string): DaemonPtyAdapter | null { const provider = this.sessionProviders.get(sessionId) return provider && this.allDaemonAdapters().includes(provider as DaemonPtyAdapter) diff --git a/src/main/daemon/pty-provider-route-reconciliation.ts b/src/main/daemon/pty-provider-route-reconciliation.ts new file mode 100644 index 00000000000..2f84b144661 --- /dev/null +++ b/src/main/daemon/pty-provider-route-reconciliation.ts @@ -0,0 +1,26 @@ +import type { IPtyProvider, PtyProcessInfo } from '../providers/types' + +export async function listProviderProcessesAndReconcileRoutes( + providers: readonly T[], + routes: Map +): Promise { + const routesAtStart = [...routes] + const listings = await Promise.all( + providers.map(async (provider) => ({ provider, sessions: await provider.listProcesses() })) + ) + const liveIdsByProvider = new Map( + listings.map(({ provider, sessions }) => [provider, new Set(sessions.map(({ id }) => id))]) + ) + for (const [id, provider] of routesAtStart) { + // Why: spawn/exit can mutate routing while remote listings are in flight. + // Never let that older snapshot delete a route rebound to another owner. + if (routes.get(id) === provider && !liveIdsByProvider.get(provider)?.has(id)) { + routes.delete(id) + } + } + return listings.flatMap(({ sessions }) => sessions) +} + +export function providerSessionIds(routes: Map, provider: T): string[] { + return [...routes].filter(([, routedProvider]) => routedProvider === provider).map(([id]) => id) +} diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index a4447ff4eb4..216e83760d4 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -6679,6 +6679,7 @@ describe('registerPtyHandlers', () => { }) it('cleans up fresh runtime-owned SSH spawns when binding persistence fails', async () => { + vi.useFakeTimers() type RuntimeSpawnController = { spawn(args: { cols: number @@ -6692,7 +6693,10 @@ describe('registerPtyHandlers', () => { }): Promise<{ id: string }> } const appPtyId = 'ssh:ssh-fresh-fail@@relay-pty' - const remoteShutdown = vi.fn() + const remoteShutdown = vi + .fn() + .mockRejectedValueOnce(new Error('relay cleanup unavailable')) + .mockResolvedValueOnce(undefined) registerSshPtyProvider('ssh-fresh-fail', { spawn: vi.fn(async () => ({ id: appPtyId })), write: vi.fn(), @@ -6720,7 +6724,8 @@ describe('registerPtyHandlers', () => { throw new Error('disk full') }), removeSshRemotePtyLease: vi.fn(), - markSshRemotePtyLease: vi.fn() + markSshRemotePtyLease: vi.fn(), + markSshRemotePtyShutdownRequested: vi.fn(() => true) } let controller: RuntimeSpawnController | null = null const runtime = { @@ -6761,9 +6766,24 @@ describe('registerPtyHandlers', () => { }) ).rejects.toThrow(/ORCA_TERMINAL_SESSION_STATE_SAVE_FAILED/) - expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, { immediate: true }) - expect(store.upsertSshRemotePtyLease).not.toHaveBeenCalled() + expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, { + immediate: true, + expectedPaneKey: makePaneKey('tab-remote', leafId), + expectedTabId: 'tab-remote' + }) + expect(store.upsertSshRemotePtyLease).toHaveBeenCalled() expect(store.removeSshRemotePtyLease).not.toHaveBeenCalled() + expect(openCodeClearPtyMock).not.toHaveBeenCalledWith(appPtyId) + expect(piClearPtyMock).not.toHaveBeenCalledWith(appPtyId) + + await vi.advanceTimersByTimeAsync(250) + + expect(remoteShutdown).toHaveBeenCalledTimes(2) + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-fresh-fail', + appPtyId, + 'terminated' + ) expect(openCodeClearPtyMock).toHaveBeenCalledWith(appPtyId) expect(piClearPtyMock).toHaveBeenCalledWith(appPtyId) } finally { @@ -6771,6 +6791,78 @@ describe('registerPtyHandlers', () => { } }) + it('retains renderer SSH cleanup when binding persistence and first shutdown fail', async () => { + vi.useFakeTimers() + const appPtyId = 'ssh:ssh-1@@renderer-persist-fail' + const remoteShutdown = vi + .fn() + .mockRejectedValueOnce(new Error('relay cleanup unavailable')) + .mockResolvedValueOnce(undefined) + registerSshPtyProvider('ssh-1', { + spawn: vi.fn(async () => ({ id: appPtyId })), + write: vi.fn(), + resize: vi.fn(), + shutdown: remoteShutdown, + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}), + listProcesses: vi.fn(async () => []), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + getDefaultShell: vi.fn(), + getProfiles: vi.fn() + } as never) + const store = { + upsertSshRemotePtyLease: vi.fn(), + persistPtyBinding: vi.fn(() => { + throw new Error('disk full') + }), + markSshRemotePtyShutdownRequested: vi.fn(() => true), + markSshRemotePtyLease: vi.fn(), + removeSshRemotePtyLease: vi.fn() + } + registerPtyHandlers( + mainWindow as never, + undefined, + undefined, + undefined, + undefined, + store as never + ) + const leafId = '11111111-1111-4111-8111-111111111111' + const paneKey = makePaneKey('tab-renderer', leafId) + + await expect( + handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + connectionId: 'ssh-1', + worktreeId: 'wt-remote', + tabId: 'tab-renderer', + leafId, + env: { ORCA_PANE_KEY: paneKey } + }) + ).rejects.toThrow(/ORCA_TERMINAL_SESSION_STATE_SAVE_FAILED/) + + expect(remoteShutdown).toHaveBeenCalledWith(appPtyId, { + immediate: true, + expectedPaneKey: paneKey, + expectedTabId: 'tab-renderer' + }) + expect(openCodeClearPtyMock).not.toHaveBeenCalledWith(appPtyId) + await vi.advanceTimersByTimeAsync(250) + expect(remoteShutdown).toHaveBeenCalledTimes(2) + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', appPtyId, 'terminated') + expect(openCodeClearPtyMock).toHaveBeenCalledWith(appPtyId) + }) + it('maps runtime-owned spawn paneKeys for renderer serializer settlement', async () => { type RuntimeSpawnController = { spawn(args: { diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index fbed685ef2b..f2425307838 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -856,6 +856,21 @@ function retainSshPtyShutdown( return retained } +async function retainFailedSpawnCleanup( + id: string, + connectionId: string | null | undefined, + provider: IPtyProvider, + options: PtyShutdownOptions +): Promise { + const retained = connectionId + ? retainSshPtyShutdown(id, connectionId, provider, options) + : retainLocalPtyShutdown(id, options) + if (!retained) { + throw new Error('PTY cleanup could not retain shutdown ownership') + } + await attemptRetainedPtyShutdownForCaller(retained) +} + // ─── Host PTY env assembly ────────────────────────────────────────── // Why: both the LocalPtyProvider.buildSpawnEnv closure and the daemon-active // fallback in pty:spawn need the same set of host-local env injections @@ -3690,14 +3705,27 @@ export function registerPtyHandlers( }) } catch (err) { console.error('[pty] failed to persist runtime PTY binding after spawn:', err) - deletePtyOwnership(result.id) - if (!result.isReattach) { + if (result.isReattach) { + // Why: the caller rejected this attach and must not keep routing + // writes, but the pre-existing provider session remains owned. + deletePtyOwnership(result.id) + } else { + if (args.connectionId) { + // Why: the binding failed, so the SSH lease is the only durable + // identity that can carry cleanup across an app restart. + try { + persistSshLease() + } catch {} + } try { - await provider.shutdown(result.id, { immediate: true }) + await retainFailedSpawnCleanup(result.id, args.connectionId, provider, { + immediate: true, + ...(metadataPaneKey ? { expectedPaneKey: metadataPaneKey } : {}), + ...(typeof args.tabId === 'string' ? { expectedTabId: args.tabId } : {}) + }) } catch (shutdownErr) { console.warn('[pty] failed to clean up PTY after persistence failure:', shutdownErr) } - clearProviderPtyState(result.id) } throw new Error(createTerminalSessionStateSaveFailureMessage()) } @@ -4697,18 +4725,17 @@ export function registerPtyHandlers( console.error('[pty] failed to persist PTY binding after spawn:', err) if (!result.isReattach) { try { - await provider.shutdown(result.id, { immediate: true }) + await retainFailedSpawnCleanup(result.id, args.connectionId, provider, { + immediate: true, + ...(validatedPaneKey ? { expectedPaneKey: validatedPaneKey } : {}), + ...(typeof args.tabId === 'string' ? { expectedTabId: args.tabId } : {}) + }) } catch (shutdownErr) { console.warn( '[pty] failed to clean up PTY after persistence failure:', shutdownErr ) } - clearProviderPtyState(result.id) - deletePtyOwnership(result.id) - } - if (!result.isReattach && args.connectionId && store) { - store.removeSshRemotePtyLease(args.connectionId, relayResultId) } throw new Error(createTerminalSessionStateSaveFailureMessage()) } diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index b5071209b00..a9481f6aeaf 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -18,7 +18,8 @@ import { bufferPreHandlerPtyExit, clearPreHandlerPtyState, drainPreHandlerPtyData, - drainPreHandlerPtyExit + drainPreHandlerPtyExit, + reconcilePreHandlerPtyExitAfterOverflow } from './pty-pre-handler-buffer' import { clearReceivedPtyCharTotal, @@ -392,7 +393,14 @@ export function registerEagerPtyBuffer( // caller receives that handle before onExit fires. queueMicrotask(() => { if (ptyExitHandlers.get(ptyId) === exitHandler) { - drainPreHandlerPtyExit(ptyId, exitHandler) + if (!drainPreHandlerPtyExit(ptyId, exitHandler)) { + reconcilePreHandlerPtyExitAfterOverflow( + ptyId, + window.api.pty.hasPty, + exitHandler, + () => ptyExitHandlers.get(ptyId) === exitHandler + ) + } } else { clearPreHandlerPtyState(ptyId) } diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts index 18bd6de7603..9b74e3d07ad 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts @@ -4,7 +4,8 @@ import { bufferPreHandlerPtyExit, clearPreHandlerPtyState, drainPreHandlerPtyData, - drainPreHandlerPtyExit + drainPreHandlerPtyExit, + reconcilePreHandlerPtyExitAfterOverflow } from './pty-pre-handler-buffer' const RESCAN_PTY_ID = 'pty-pre-handler-rescan' @@ -90,4 +91,19 @@ describe('pre-handler PTY buffer', () => { expect(oldest).not.toHaveBeenCalled() expect(newest).toHaveBeenCalledWith(64) }) + + it('reconciles an evicted pending exit with one targeted liveness readback', async () => { + for (let index = 0; index <= 64; index += 1) { + bufferPreHandlerPtyExit(`pty-exit-${index}`, index) + } + const handler = vi.fn() + const hasPty = vi.fn(async () => false) + + reconcilePreHandlerPtyExitAfterOverflow('pty-exit-0', hasPty, handler, () => true) + await Promise.resolve() + + expect(hasPty).toHaveBeenCalledOnce() + expect(hasPty).toHaveBeenCalledWith('pty-exit-0') + expect(handler).toHaveBeenCalledWith(-1) + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts index a58d7a76b27..d6e5feca472 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.ts @@ -22,6 +22,7 @@ const preHandlerPtyExit = new Map() const PRE_HANDLER_PTY_DATA_MAX_BYTES = 512 * 1024 const PRE_HANDLER_PTY_DATA_MAX_PTYS = 64 const PRE_HANDLER_PTY_EXIT_MAX_PTYS = 64 +let preHandlerPtyExitOverflowed = false // Why: legit pre-attach windows drain within milliseconds and hold little // data. Sustained accumulation means a pane lost its data handler (the // frozen-pane detach/attach race) — leave a breadcrumb for trace capture. @@ -96,18 +97,40 @@ export function bufferPreHandlerPtyExit(ptyId: string, code: number): void { const oldestPtyId = preHandlerPtyExit.keys().next().value if (typeof oldestPtyId === 'string') { preHandlerPtyExit.delete(oldestPtyId) + preHandlerPtyExitOverflowed = true } } preHandlerPtyExit.set(ptyId, code) } -export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) => void): void { +export function drainPreHandlerPtyExit(ptyId: string, handler: (code: number) => void): boolean { const code = preHandlerPtyExit.get(ptyId) if (code === undefined) { - return + return false } preHandlerPtyExit.delete(ptyId) handler(code) + return true +} + +export function reconcilePreHandlerPtyExitAfterOverflow( + ptyId: string, + hasPty: ((id: string) => Promise) | undefined, + handler: (code: number) => void, + isCurrent: () => boolean +): void { + if (!preHandlerPtyExitOverflowed || !hasPty) { + return + } + // Why: the bounded exit buffer may evict a legitimate pre-registration exit. + // One targeted readback after overflow recovers truth without retaining IDs. + void hasPty(ptyId) + .then((alive) => { + if (alive === false && isCurrent()) { + handler(-1) + } + }) + .catch(() => {}) } export function clearPreHandlerPtyData(ptyId: string): void { diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 6c4585fa931..607687820a6 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -42,6 +42,7 @@ describe('createIpcPtyTransport', () => { writeAccepted: vi.fn().mockResolvedValue(true), resize: vi.fn(), kill: vi.fn().mockResolvedValue(undefined), + hasPty: vi.fn().mockResolvedValue(true), onData: vi.fn((callback: (payload: { id: string; data: string }) => void) => { onData = callback return () => {} @@ -932,6 +933,24 @@ describe('createIpcPtyTransport', () => { expect(onEagerExit).toHaveBeenCalledWith('pty-fast-setup', 1) }) + it('reconciles an exit evicted by the bounded pre-handler buffer', async () => { + const { ensurePtyDispatcher, registerEagerPtyBuffer } = await import('./pty-transport') + const hasPty = window.api.pty.hasPty as unknown as ReturnType + const onEagerExit = vi.fn() + hasPty.mockResolvedValue(false) + ensurePtyDispatcher() + for (let index = 0; index <= 64; index += 1) { + onExit?.({ id: `pty-overflow-${index}`, code: index }) + } + + registerEagerPtyBuffer('pty-overflow-0', onEagerExit) + await Promise.resolve() + await Promise.resolve() + + expect(hasPty).toHaveBeenCalledWith('pty-overflow-0') + expect(onEagerExit).toHaveBeenCalledWith('pty-overflow-0', -1) + }) + it('enforces the eager buffer cap in UTF-8 bytes for multi-byte output', async () => { const { registerEagerPtyBuffer } = await import('./pty-transport') const cap = 512 * 1024 diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 66ae20c0c3b..34fd9a61a0a 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -22,7 +22,11 @@ import { ensurePtyDispatcher, getEagerPtyBufferHandle } from './pty-dispatcher' -import { drainPreHandlerPtyData, drainPreHandlerPtyExit } from './pty-pre-handler-buffer' +import { + drainPreHandlerPtyData, + drainPreHandlerPtyExit, + reconcilePreHandlerPtyExitAfterOverflow +} from './pty-pre-handler-buffer' import { createPtyInputWriteQueue } from './pty-input-write-queue' import type { PtyDataMeta } from './pty-dispatcher' import type { IpcPtyTransportOptions, PtyConnectResult, PtyTransport } from './pty-transport-types' @@ -682,7 +686,14 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra // would otherwise fire stale notifications after the data handler // is removed but before the exit event arrives. ptyTeardownHandlers.set(id, clearAccumulatedState) - drainPreHandlerPtyExit(id, exitHandler) + if (!drainPreHandlerPtyExit(id, exitHandler)) { + reconcilePreHandlerPtyExitAfterOverflow( + id, + window.api.pty.hasPty, + exitHandler, + () => ownedExitHandlers.get(id) === exitHandler + ) + } } return {