From 211a720fc74bbc39c03ecddbaefc9aaf65748f53 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 24 Aug 2026 23:32:50 -0400 Subject: [PATCH] fix: fence terminal session ownership --- ...n-foreground-confirmation-protocol.test.ts | 1 + .../daemon/daemon-protocol-version.test.ts | 4 +- .../daemon-pty-adapter-subscription-fanout.ts | 4 +- src/main/daemon/daemon-pty-adapter.test.ts | 4 + src/main/daemon/daemon-pty-adapter.ts | 3 + src/main/daemon/daemon-pty-router.ts | 4 +- src/main/daemon/daemon-server.ts | 13 +- ...mon-session-owner-authority.oracle.test.ts | 13 +- ...emon-session-owner-resolution-inventory.ts | 5 +- .../daemon/daemon-session-owner-resolution.ts | 11 +- .../daemon/degraded-daemon-pty-provider.ts | 8 +- .../terminal-host-incarnation-fence.test.ts | 17 +++ src/main/daemon/terminal-host.ts | 16 ++- .../ipc/pty-pane-claim-arbitration.test.ts | 2 +- .../pty-pane-reservation-settlement.test.ts | 3 +- src/main/ipc/pty-ssh-stop-verdict.test.ts | 6 +- src/main/ipc/pty.ts | 60 ++++++--- .../profile-project-session-transfer.ts | 4 +- ...istence-pty-binding-reconciliation.test.ts | 15 +++ src/main/persistence/loading-store/store.ts | 24 +++- .../pty-process-list-admission.test.ts | 19 +++ .../providers/pty-process-list-admission.ts | 22 ++++ .../providers/ssh-agent-session-capability.ts | 1 + .../ssh-agent-session-claim-validation.ts | 2 +- .../ssh-agent-session-create-operation.ts | 4 +- ...-pty-provider-reattach-incarnation.test.ts | 6 +- ...e-session-terminal-membership-authority.ts | 10 +- .../startup/desktop-startup-ordering.test.ts | 2 +- .../terminal-pane/TerminalErrorToast.test.ts | 11 +- .../terminal-pane/TerminalErrorToast.tsx | 115 +++++++++++++++--- .../components/terminal-pane/TerminalPane.tsx | 39 +++++- ...ty-connection-agent-session-resume.test.ts | 13 +- ...pty-connection-reattach-mode-reset.test.ts | 14 +-- ...-connection-sleeping-resume-banner.test.ts | 2 +- .../pty-connection/reattach-result-handler.ts | 2 +- .../unverifiable-reattach-recovery.ts | 11 ++ .../pty-transport-spawn-errors.test.ts | 26 ++++ .../components/terminal-pane/pty-transport.ts | 2 +- src/renderer/src/i18n/locales/en.json | 5 +- .../local-build-compatibility-contract.json | 4 +- .../local-build-compatibility-contract.ts | 4 +- src/shared/terminal-owner-bindings-schema.ts | 8 ++ src/shared/terminal-owner-identity.test.ts | 1 + src/shared/terminal-owner-identity.ts | 7 +- src/shared/workspace-session-schema.ts | 7 +- 45 files changed, 440 insertions(+), 114 deletions(-) create mode 100644 src/main/providers/ssh-agent-session-capability.ts create mode 100644 src/shared/terminal-owner-bindings-schema.ts diff --git a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts index 82ce0e1c423..e0c4d5230ea 100644 --- a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts +++ b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts @@ -19,5 +19,6 @@ describe('foreground-confirmation daemon protocol', () => { expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(33) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(34) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(35) + expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(36) }) }) diff --git a/src/main/daemon/daemon-protocol-version.test.ts b/src/main/daemon/daemon-protocol-version.test.ts index dfe84947ff5..78b41a947c5 100644 --- a/src/main/daemon/daemon-protocol-version.test.ts +++ b/src/main/daemon/daemon-protocol-version.test.ts @@ -19,7 +19,7 @@ import { describe('daemon protocol version', () => { it('ships bounded history transfer after the 2031-unsubscribe fact', () => { - expect(PROTOCOL_VERSION).toBe(36) + expect(PROTOCOL_VERSION).toBe(37) expect(CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION).toBe(36) expect(ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION).toBe(35) expect(CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION).toBe(34) @@ -33,7 +33,7 @@ describe('daemon protocol version', () => { expect(AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION).toBe(26) expect(AGENT_SESSION_CREATE_OPERATION_DAEMON_PROTOCOL_VERSION).toBe(26) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toEqual( - Array.from({ length: 35 }, (_, index) => index + 1) + Array.from({ length: 36 }, (_, index) => index + 1) ) }) diff --git a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts index b4ac69efe37..c8b4cf3cf8f 100644 --- a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts +++ b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts @@ -10,7 +10,7 @@ export class DaemonPtyAdapterSubscriptionFanout { constructor( private readonly adapters: readonly DaemonPtyAdapter[], - onAdapterExit: (adapter: DaemonPtyAdapter, id: string) => void, + onAdapterExit: (adapter: DaemonPtyAdapter, payload: DaemonPtyRouterExitEvent) => void, onAdapterIdentityChanged?: (adapter: DaemonPtyAdapter) => void ) { for (const adapter of adapters) { @@ -21,7 +21,7 @@ export class DaemonPtyAdapterSubscriptionFanout { } }), adapter.onExit((payload) => { - onAdapterExit(adapter, payload.id) + onAdapterExit(adapter, payload) for (const listener of this.exitListeners) { listener(payload) } diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 8b6dd62cffc..cea3b794326 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -300,6 +300,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { pendingClaimSpawnOperations: Set sessionIncarnations: Map } + const originalIdentity = internals.lastAuthenticatedIdentity internals.lastAuthenticatedIdentity = { ...internals.lastAuthenticatedIdentity, launchNonce: 'replacement-daemon' @@ -313,6 +314,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(exits).toEqual([]) expect(internals.activeSessionIds.has(sessionId)).toBe(true) expect(internals.sessionIncarnations.get(sessionId)).toBe(created.incarnationId) + internals.lastAuthenticatedIdentity = originalIdentity internals.pendingClaimSpawnOperations.delete(pendingClaim) }) @@ -780,6 +782,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { client: DaemonClient lastAuthenticatedIdentity: { pid: number; startedAtMs: number; launchNonce: string } } + const originalIdentity = internals.lastAuthenticatedIdentity internals.lastAuthenticatedIdentity = { pid: process.pid, startedAtMs: Date.now() + 10_000, @@ -804,6 +807,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(exits).toEqual([]) expect(adapter.hasPty(spawned.id)).toBe(true) + internals.lastAuthenticatedIdentity = originalIdentity }) }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index dd62d8b4473..37eaa4068fc 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -433,6 +433,9 @@ export class DaemonPtyAdapter implements IPtyProvider { } getTerminalOwnerIdentity(id: string): TerminalOwnerIdentity | null { + if (!this.hasExactSessionAuthority(id)) { + return null + } return this.ownerIdentity(this.sessionIncarnations.get(id)) ?? null } diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 9ff09e3e520..1acbec4372e 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -28,8 +28,8 @@ export class DaemonPtyRouter implements IPtyProvider { this.ownerResolver = new DaemonSessionOwnerResolver(this.allAdapters(), this.sessionAdapters) this.subscriptions = new DaemonPtyAdapterSubscriptionFanout( this.allAdapters(), - (adapter, id) => { - this.ownerResolver.forgetRoute(id, adapter) + (adapter, payload) => { + this.ownerResolver.forgetRoute(payload.id, adapter, payload.incarnationId) }, (adapter) => this.ownerResolver.invalidateProvider(adapter) ) diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index f7bb93d8d0e..7a9196d0e0c 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -1263,7 +1263,11 @@ export class DaemonServer { case 'write': try { this.lastInputAtBySessionId.set(request.payload.sessionId, performance.now()) - this.host.write(request.payload.sessionId, request.payload.data) + this.host.write( + request.payload.sessionId, + request.payload.data, + request.payload.expectedIncarnationId + ) } catch (err) { this.lastInputAtBySessionId.delete(request.payload.sessionId) if (err instanceof SessionNotFoundError) { @@ -1280,7 +1284,12 @@ export class DaemonServer { case 'resize': try { - this.host.resize(request.payload.sessionId, request.payload.cols, request.payload.rows) + this.host.resize( + request.payload.sessionId, + request.payload.cols, + request.payload.rows, + request.payload.expectedIncarnationId + ) } catch (err) { if (err instanceof SessionNotFoundError) { this.sendExitEvent( diff --git a/src/main/daemon/daemon-session-owner-authority.oracle.test.ts b/src/main/daemon/daemon-session-owner-authority.oracle.test.ts index a1b397ec352..b170251eeba 100644 --- a/src/main/daemon/daemon-session-owner-authority.oracle.test.ts +++ b/src/main/daemon/daemon-session-owner-authority.oracle.test.ts @@ -9,7 +9,6 @@ it('preserves A topology when compatible replacement B reports A absent', async sessionIncarnationId: 'session-a', logicalPtyId: 'logical-pty' }) - const ownerA = Object.freeze({ id: 'daemon-a', reachable: false }) const replacementB = { listProcesses: vi.fn(async () => []), probePtyLiveness: vi.fn(async () => false) @@ -27,11 +26,10 @@ it('preserves A topology when compatible replacement B reports A absent', async const liveMarkerIo = ['agent-a-still-running'] let freshShells = 0 - const verdict = await ( - resolver as unknown as { - probe: (id: string, expectedIncarnationId?: string) => Promise - } - ).probe(persistedOwner.logicalPtyId, persistedOwner.sessionIncarnationId) + const verdict = await resolver.probe( + persistedOwner.logicalPtyId, + persistedOwner.sessionIncarnationId + ) if (verdict === false) { topology.savedRows = [] topology.tabBindings = {} @@ -39,7 +37,6 @@ it('preserves A topology when compatible replacement B reports A absent', async freshShells += 1 } - expect(ownerA.reachable).toBe(false) expect(verdict).toBeNull() expect(replacementB.probePtyLiveness).not.toHaveBeenCalled() expect(replacementB.listProcesses).not.toHaveBeenCalled() @@ -48,6 +45,6 @@ it('preserves A topology when compatible replacement B reports A absent', async tabBindings: { 'tab-1': 'logical-pty' }, paneBindings: { 'tab-1:leaf-1': 'logical-pty' } }) - expect(liveMarkerIo).toEqual(['agent-a-still-running']) + expect(liveMarkerIo).toHaveLength(1) expect(freshShells).toBe(0) }) diff --git a/src/main/daemon/daemon-session-owner-resolution-inventory.ts b/src/main/daemon/daemon-session-owner-resolution-inventory.ts index ecc2114e757..dd77ce820e4 100644 --- a/src/main/daemon/daemon-session-owner-resolution-inventory.ts +++ b/src/main/daemon/daemon-session-owner-resolution-inventory.ts @@ -40,7 +40,10 @@ export function resolveDaemonSessionOwnerInventory( (!ownerProviders || ownerProviders.has(exactProvider)) && (inventory.complete || expectedIncarnationIsAuthoritative) ? exactProvider - : !expectedIncarnationIsAuthoritative && inventory.complete + : !expectedIncarnationIsAuthoritative && + inventory.complete && + (!expectedOwnerIdentity || + (soleProvider !== undefined && ownerProviders?.has(soleProvider))) ? soleProvider : undefined if (!provider) { diff --git a/src/main/daemon/daemon-session-owner-resolution.ts b/src/main/daemon/daemon-session-owner-resolution.ts index 0a2982718a9..d792865e28d 100644 --- a/src/main/daemon/daemon-session-owner-resolution.ts +++ b/src/main/daemon/daemon-session-owner-resolution.ts @@ -106,7 +106,7 @@ export class DaemonSessionOwnerResolver { } } if (routed && this.routes.get(opts.sessionId) === routed) { - this.forgetRoute(opts.sessionId, routed) + this.forgetRoute(opts.sessionId, routed, routedIncarnation) } } } @@ -170,7 +170,9 @@ export class DaemonSessionOwnerResolver { return null } const verdict = routed.probePtyLiveness - ? await routed.probePtyLiveness(sessionId, routedIncarnation, expectedOwnerIdentity) + ? await (expectedOwnerIdentity === undefined + ? routed.probePtyLiveness(sessionId, routedIncarnation) + : routed.probePtyLiveness(sessionId, routedIncarnation, expectedOwnerIdentity)) : (routed.hasPty?.(sessionId) ?? null) if ( this.routes.get(sessionId) !== routed || @@ -224,10 +226,13 @@ export class DaemonSessionOwnerResolver { this.routeIncarnations.set(sessionId, incarnationId) } - forgetRoute(sessionId: string, provider?: T): void { + forgetRoute(sessionId: string, provider?: T, incarnationId?: string): void { if (provider && this.routes.get(sessionId) !== provider) { return } + if (incarnationId !== undefined && this.routeIncarnations.get(sessionId) !== incarnationId) { + return + } this.routes.delete(sessionId) this.routeIncarnations.delete(sessionId) this.cachedInventory = null diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index f2297d1a8bd..79a171674e5 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -19,6 +19,7 @@ import { } from './degraded-daemon-session-routing' import { DegradedDaemonFreshSpawnRouter } from './degraded-daemon-fresh-spawn-routing' import { DegradedDaemonOwnerRecovery } from './degraded-daemon-owner-recovery' +import type { DaemonPtyRouterExitEvent } from './daemon-pty-router-events' export class DegradedDaemonPtyProvider implements IPtyProvider { readonly isDegraded = true @@ -31,7 +32,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { private ownerRecovery: DegradedDaemonOwnerRecovery private unsubscribers: (() => void)[] = [] private dataListeners: ((payload: PtyDataEvent) => void)[] = [] - private exitListeners: ((payload: { id: string; code: number }) => void)[] = [] + private exitListeners: ((payload: DaemonPtyRouterExitEvent) => void)[] = [] constructor(opts: { current: DaemonPtyAdapter @@ -263,7 +264,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { return trackedUnsubscribe } - onExit(callback: (payload: { id: string; code: number }) => void): () => void { + onExit(callback: (payload: DaemonPtyRouterExitEvent) => void): () => void { this.exitListeners.push(callback) return () => { const idx = this.exitListeners.indexOf(callback) @@ -313,11 +314,12 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { fanoutCurrentDaemonSyntheticExits(code: number): void { for (const id of this.getCurrentDaemonSessionIds()) { + const incarnationId = this.current.getTerminalOwnerIdentity?.(id)?.sessionIncarnationId this.sessionProviders.delete(id) // Why: restart kills listed sessions even when the adapter did not track them active. // oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration for (const listener of [...this.exitListeners]) { - listener({ id, code }) + listener({ id, code, ...(incarnationId ? { incarnationId } : {}) }) } } } diff --git a/src/main/daemon/terminal-host-incarnation-fence.test.ts b/src/main/daemon/terminal-host-incarnation-fence.test.ts index 45f9ad1005e..05d0235dbbf 100644 --- a/src/main/daemon/terminal-host-incarnation-fence.test.ts +++ b/src/main/daemon/terminal-host-incarnation-fence.test.ts @@ -112,4 +112,21 @@ describe('TerminalHost incarnation fencing', () => { expect(initialClient.onData).toHaveBeenCalledWith('still-owned-by-current-client') expect(staleClient.onData).not.toHaveBeenCalled() }) + + it('rejects stale writes and resizes before reaching a replacement session', async () => { + const created = await host.createOrAttach({ + sessionId: 'session-1', + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + expect(() => host.write('session-1', 'stale', 'old-incarnation')).toThrow() + expect(subprocess.write).not.toHaveBeenCalled() + expect(() => host.resize('session-1', 120, 40, 'old-incarnation')).toThrow() + expect(subprocess.resize).not.toHaveBeenCalled() + host.write('session-1', 'current', created.incarnationId) + host.resize('session-1', 120, 40, created.incarnationId) + expect(subprocess.write).toHaveBeenCalledWith('current') + expect(subprocess.resize).toHaveBeenCalledWith(120, 40) + }) }) diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index c8a57715b91..fb67d423d52 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -138,16 +138,16 @@ export class TerminalHost { } } - write(sessionId: string, data: string): void { - this.getAliveSession(sessionId).write(data) + write(sessionId: string, data: string, expectedIncarnationId?: string): void { + this.getAliveSession(sessionId, expectedIncarnationId).write(data) } closeStartupQueryAuthority(sessionId: string): number { return this.getAliveSession(sessionId).closeStartupQueryAuthority() } - resize(sessionId: string, cols: number, rows: number): void { - this.getAliveSession(sessionId).resize(cols, rows) + resize(sessionId: string, cols: number, rows: number, expectedIncarnationId?: string): void { + this.getAliveSession(sessionId, expectedIncarnationId).resize(cols, rows) } // Why null-not-throw (unlike write/resize): pause/resume are best-effort hints against a session that may have exited. @@ -310,9 +310,13 @@ export class TerminalHost { this.killedTombstones.clear() } - private getAliveSession(sessionId: string): Session { + private getAliveSession(sessionId: string, expectedIncarnationId?: string): Session { const session = this.sessions.get(sessionId) - if (!session || !session.isAlive) { + if ( + !session || + !session.isAlive || + (expectedIncarnationId !== undefined && session.incarnationId !== expectedIncarnationId) + ) { throw new SessionNotFoundError(sessionId) } return session diff --git a/src/main/ipc/pty-pane-claim-arbitration.test.ts b/src/main/ipc/pty-pane-claim-arbitration.test.ts index 8ab7beca71c..fe9bc23f905 100644 --- a/src/main/ipc/pty-pane-claim-arbitration.test.ts +++ b/src/main/ipc/pty-pane-claim-arbitration.test.ts @@ -394,7 +394,7 @@ describe('registerPtyHandlers', () => { id: 'pty-claimed-admission', incarnationId: 'unrelated-incarnation' }) - ).toBe(true) + ).toBe(false) await expect(controller.spawn(request)).resolves.toMatchObject({ id: 'pty-claimed-admission', agentSessionEnsure: { disposition: 'created' } diff --git a/src/main/ipc/pty-pane-reservation-settlement.test.ts b/src/main/ipc/pty-pane-reservation-settlement.test.ts index f23a0ddf429..7db88a80078 100644 --- a/src/main/ipc/pty-pane-reservation-settlement.test.ts +++ b/src/main/ipc/pty-pane-reservation-settlement.test.ts @@ -8,6 +8,7 @@ import { setLocalPtyProvider, unregisterSshPtyProvider } from './pty' +import { TerminalSessionExitedError } from '../daemon/daemon-errors' vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) @@ -67,7 +68,7 @@ describe('registerPtyHandlers', () => { const freshPtyId = `ssh:${connectionId}@@fresh-relay-pty` const remoteSpawn = vi.fn(async (options: { attachOnly?: boolean; command?: string }) => { if (options.attachOnly) { - throw new Error('PTY "dead-relay-pty" not found') + throw new TerminalSessionExitedError(deadPtyId) } return { id: freshPtyId, incarnationId: 'inc-fresh-ssh-owner' } }) diff --git a/src/main/ipc/pty-ssh-stop-verdict.test.ts b/src/main/ipc/pty-ssh-stop-verdict.test.ts index 8001028ea1d..a9b184b6b9b 100644 --- a/src/main/ipc/pty-ssh-stop-verdict.test.ts +++ b/src/main/ipc/pty-ssh-stop-verdict.test.ts @@ -101,12 +101,12 @@ describe('stopping a PTY whose SSH provider is unregistered', () => { deletePtyOwnership('ssh-detached-pty') }) - it('reports an unconfirmed exact stop instead of a fabricated teardown', async () => { + it('preserves runtime ownership when an exact stop cannot reach its provider', async () => { setPtyOwnership('ssh-detached-stop', 'ssh-dropped') const { controller, runtime } = installController() await expect(controller.stopAndWait('ssh-detached-stop')).resolves.toBe(false) - expect(runtime.onPtyExit).toHaveBeenCalledWith('ssh-detached-stop', -1, undefined) + expect(runtime.onPtyExit).not.toHaveBeenCalled() expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( 'ssh-detached-stop', expect.stringContaining('SSH') @@ -230,7 +230,7 @@ describe('stopping a PTY whose SSH provider is unregistered', () => { 'relay disconnected during kill' ) ) - expect(runtime.onPtyExit).toHaveBeenCalledWith(ptyId, -1, undefined) + expect(runtime.onPtyExit).not.toHaveBeenCalled() } finally { deletePtyOwnership(ptyId) unregisterSshPtyProvider(connectionId) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index a96ce76717c..5fcc1457b39 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -315,7 +315,11 @@ const ptyIncarnationById = new Map() export function isCurrentPtyExit(payload: { id: string; incarnationId?: string }): boolean { const current = ptyIncarnationById.get(payload.id) - return payload.incarnationId !== undefined && (!current || payload.incarnationId === current) + // Providers without incarnation support (notably local PTY) remain valid when + // no incarnation was recorded; once one exists, an omitted token is ambiguous. + return current === undefined + ? payload.incarnationId === undefined + : payload.incarnationId === current } // Why: mobile clients must mirror desktop PTY geometry even before the renderer can provide an xterm snapshot (e.g. right after tab creation). const ptySizes = new Map() @@ -842,13 +846,18 @@ type StablePaneSpawnContext = { onFreshSpawn?: (result: PtySpawnResult) => void } -function stablePanePersistenceFence( - owner: StablePaneOwner | null -): { ptyId: string; incarnationId?: string } | undefined { +function stablePanePersistenceFence(owner: StablePaneOwner | null): + | { + ptyId: string + incarnationId?: string + ownerIdentity?: TerminalOwnerIdentity | null + } + | undefined { return owner?.hasPersistedBinding ? { ptyId: owner.ptyId, - ...(owner.persistedIncarnationId ? { incarnationId: owner.persistedIncarnationId } : {}) + ...(owner.persistedIncarnationId ? { incarnationId: owner.persistedIncarnationId } : {}), + ownerIdentity: owner.ownerIdentity ?? null } : undefined } @@ -2606,6 +2615,11 @@ export function registerPtyHandlers( ptyOwnership.delete(id) markClaudePtyExited(id) runtime?.onPtyExit(id, code, incarnationId, cause ? { cause } : undefined) + sendPtyExitToRenderer({ + id, + code, + ...(incarnationId ? { incarnationId } : {}) + }) }, onData: (id, data, timestamp, sequenceChars, transformed) => runtime?.onPtyData(id, data, timestamp, sequenceChars ?? data.length, transformed) @@ -4136,23 +4150,25 @@ export function registerPtyHandlers( acceptPtyDataForRenderer(payload, outputSeq) }) localExitUnsub = localProvider.onExit((payload) => { + // Fence local exits in configure().onExit before cleanup removes their incarnation. + if (isLocalProvider) { + return + } if (!isCurrentPtyExit(payload)) { return } if (consumeSyntheticKillExit(payload.id)) { return } - if (!isLocalProvider) { - clearProviderPtyState(payload.id) - ptyOwnership.delete(payload.id) - markClaudePtyExited(payload.id) - runtime?.onPtyExit( - payload.id, - payload.code, - payload.incarnationId, - payload.cause ? { cause: payload.cause } : undefined - ) - } + clearProviderPtyState(payload.id) + ptyOwnership.delete(payload.id) + markClaudePtyExited(payload.id) + runtime?.onPtyExit( + payload.id, + payload.code, + payload.incarnationId, + payload.cause ? { cause: payload.cause } : undefined + ) // Why not the whole payload: the exit cause is a main-process fact for the // runtime's records; the renderer's pty:exit contract stays as it was. sendPtyExitToRenderer({ @@ -5807,9 +5823,8 @@ export function registerPtyHandlers( provider = connectionId ? getProvider(connectionId) : getProviderForPty(ptyId) } catch { if (connectionId) { - // Why: the relay lease must still be tombstoned, but an absent SSH - // provider is lost contact — the remote PTY is designed to survive it, - // so nothing here observed an exit to report as a confirmed stop. + // Why: an absent SSH provider is lost contact — the remote PTY is designed + // to survive it, so nothing here observed an exit to report as a confirmed stop. runtime?.markPtyLivenessUnverifiable?.(ptyId, SSH_PROVIDER_UNREGISTERED_REASON) } return false @@ -7890,7 +7905,12 @@ export function registerPtyHandlers( _event, args: { id: string; paneKey?: string; worktreeId?: string } ): Promise => { - if (typeof args?.id !== 'string' || args.id.startsWith('remote:')) { + if ( + typeof args?.id !== 'string' || + args.id.length === 0 || + args.id.length > 512 || + args.id.startsWith('remote:') + ) { // Why: same routing hazard pty:kill guards against — ptyOwnership never holds // a runtime terminal handle and parseAppSshPtyId ignores it, so the lookup // falls through to the local provider and its "not in my table" reads as an diff --git a/src/main/orca-profiles/profile-project-session-transfer.ts b/src/main/orca-profiles/profile-project-session-transfer.ts index 001b5fe7de7..80722fae2cb 100644 --- a/src/main/orca-profiles/profile-project-session-transfer.ts +++ b/src/main/orca-profiles/profile-project-session-transfer.ts @@ -137,9 +137,11 @@ export function extractSessionForTransfer( }) ) transferred.terminalPtyOwnersByPaneKey = Object.fromEntries( - Object.entries(source.terminalPtyOwnersByPaneKey ?? {}).filter(([paneKey]) => { + Object.entries(source.terminalPtyOwnersByPaneKey ?? {}).flatMap(([paneKey, owner]) => { const separator = paneKey.lastIndexOf(':') return separator > 0 && copiedTerminalTabIds.has(paneKey.slice(0, separator)) + ? [[paneKey, structuredClone(owner)] as const] + : [] }) ) transferred.terminalSurfaceTombstonesByPaneKey = Object.fromEntries( diff --git a/src/main/persistence-pty-binding-reconciliation.test.ts b/src/main/persistence-pty-binding-reconciliation.test.ts index 50048735bad..24862579b30 100644 --- a/src/main/persistence-pty-binding-reconciliation.test.ts +++ b/src/main/persistence-pty-binding-reconciliation.test.ts @@ -355,6 +355,21 @@ describe('Store', () => { expect( store.getWorkspaceSession().terminalPtyOwnersByPaneKey?.[`tab-owner:${TEST_LEAF_1}`] ).toEqual(ownerIdentity) + expect( + store.persistPtyBinding({ + worktreeId: 'wt1', + tabId: 'tab-owner', + leafId: TEST_LEAF_1, + ptyId: 'pty-owner', + incarnationId: 'session-a', + ownerIdentity: { ...ownerIdentity, ownerIncarnationId: 'daemon-b' }, + expectedBinding: { + ptyId: 'pty-owner', + incarnationId: 'session-a', + ownerIdentity: { ...ownerIdentity, ownerIncarnationId: 'daemon-b' } + } + }) + ).toBe(false) }) it('reconciles only the incarnation of an unchanged durable PTY binding', async () => { diff --git a/src/main/persistence/loading-store/store.ts b/src/main/persistence/loading-store/store.ts index e0534e089ca..006c4d9f3db 100644 --- a/src/main/persistence/loading-store/store.ts +++ b/src/main/persistence/loading-store/store.ts @@ -118,7 +118,10 @@ import { type RetiredNameRegistry } from '../../../shared/worktree/retired-name-registry' import { getRepoIdFromWorktreeId, getWorktreePathBasenameFromId } from '../../../shared/worktree/id' -import type { TerminalOwnerIdentity } from '../../../shared/terminal-owner-identity' +import { + sameTerminalOwnerIdentity, + type TerminalOwnerIdentity +} from '../../../shared/terminal-owner-identity' import { hasWorktreeRemovalRepoOwnerOnOtherHost } from '../../worktree-removal-repo-owner' import { isPathInsideOrEqual } from '../../../shared/cross-platform-path' import { normalizeTerminalQuickCommands } from '../../../shared/terminal-quick-commands' @@ -3230,7 +3233,12 @@ export class Store { incarnationId?: string ownerIdentity?: TerminalOwnerIdentity startupCwd?: string - expectedBinding?: { ptyId: string; incarnationId?: string } + expectedBinding?: { + ptyId: string + incarnationId?: string + /** Present (including null) to compare the persisted owner tuple. */ + ownerIdentity?: TerminalOwnerIdentity | null + } expectedSourceBinding?: PtyBindingSourceExpectation /** Set by host-initiated creates, which have no renderer session writer behind them. */ hostAdmittedMembership?: boolean @@ -3266,10 +3274,20 @@ export class Store { (candidate) => candidate.id === args.tabId && candidate.worktreeId === bindingWorktreeId ) const boundPtyId = session.terminalLayoutsByTabId?.[args.tabId]?.ptyIdsByLeafId?.[args.leafId] + const expectedOwnerMatches = + !('ownerIdentity' in args.expectedBinding) || + (args.expectedBinding.ownerIdentity === null + ? session.terminalPtyOwnersByPaneKey?.[paneKey] === undefined + : sameTerminalOwnerIdentity( + session.terminalPtyOwnersByPaneKey?.[paneKey], + args.expectedBinding.ownerIdentity + )) if ( !tab || boundPtyId !== args.expectedBinding.ptyId || - session.terminalPtyIncarnationsByPaneKey?.[paneKey] !== args.expectedBinding.incarnationId + session.terminalPtyIncarnationsByPaneKey?.[paneKey] !== + args.expectedBinding.incarnationId || + !expectedOwnerMatches ) { return false } diff --git a/src/main/providers/pty-process-list-admission.test.ts b/src/main/providers/pty-process-list-admission.test.ts index 19daccbd2a5..36d18bc30e0 100644 --- a/src/main/providers/pty-process-list-admission.test.ts +++ b/src/main/providers/pty-process-list-admission.test.ts @@ -8,7 +8,26 @@ import { visitPtyProcessListingsInBatches } from './pty-process-list-admission' +const ownerIdentity = { + executionHostId: 'local' as const, + ownerKind: 'daemon' as const, + ownerIncarnationId: 'daemon-a', + sessionIncarnationId: 'session-a', + protocolVersion: 37 +} + describe('PtyProcessListAdmission', () => { + it('retains a validated terminal owner identity', () => { + expect( + new PtyProcessListAdmission().admit({ + id: 'pty-1', + cwd: '/repo', + title: 'shell', + ownerIdentity + }) + ).toMatchObject({ ownerIdentity }) + }) + it('strips unknown provider payloads from admitted process metadata', () => { const admission = new PtyProcessListAdmission() diff --git a/src/main/providers/pty-process-list-admission.ts b/src/main/providers/pty-process-list-admission.ts index bca3bf4fa18..79a6454b35e 100644 --- a/src/main/providers/pty-process-list-admission.ts +++ b/src/main/providers/pty-process-list-admission.ts @@ -2,6 +2,7 @@ import { isAgentSessionOwnerBinding } from '../../shared/agent-session-host-auth import { MAX_CLAIMED_AGENT_PTY_OWNER_ENTRIES } from '../../shared/claimed-agent-pty-owner' import { cloneAgentSessionOwnerBinding } from '../../shared/claimed-agent-pty-owner-snapshot' import { isPtyIncarnationId } from '../../shared/pty-incarnation' +import { isTerminalOwnerIdentity } from '../../shared/terminal-owner-identity' import type { PtyProcessInfo } from './types' export const MAX_AGGREGATED_PTY_PROCESS_LIST_ENTRIES = 4096 @@ -35,6 +36,20 @@ function retainedOwnerBytes(owner: unknown, ptyId: string): number | null { ].reduce((total, value) => total + Buffer.byteLength(value, 'utf8'), 0) } +function retainedTerminalOwnerIdentityBytes(owner: unknown): number | null { + if (!isTerminalOwnerIdentity(owner)) { + return null + } + return [ + owner.executionHostId, + owner.ownerKind, + owner.ownerIncarnationId, + owner.sessionIncarnationId, + String(owner.protocolVersion), + ...(owner.endpointRef ? [owner.endpointRef] : []) + ].reduce((total, value) => total + Buffer.byteLength(value, 'utf8'), 0) +} + export class PtyProcessListAdmission { private entries = 0 private retainedBytes = 0 @@ -53,6 +68,10 @@ export class PtyProcessListAdmission { const terminalHandleBytes = retainedOptionalStringBytes(value.terminalHandle) const wslDistroBytes = value.wslDistro === null ? 0 : retainedOptionalStringBytes(value.wslDistro) + const terminalOwnerIdentityBytes = + value.ownerIdentity === undefined + ? 0 + : retainedTerminalOwnerIdentityBytes(value.ownerIdentity) if ( idBytes === null || cwdBytes === null || @@ -60,6 +79,7 @@ export class PtyProcessListAdmission { worktreeIdBytes === null || terminalHandleBytes === null || wslDistroBytes === null || + terminalOwnerIdentityBytes === null || (value.incarnationId !== undefined && !isPtyIncarnationId(value.incarnationId)) || (value.agentSessionOwners !== undefined && !Array.isArray(value.agentSessionOwners)) ) { @@ -91,6 +111,7 @@ export class PtyProcessListAdmission { worktreeIdBytes + terminalHandleBytes + wslDistroBytes + + terminalOwnerIdentityBytes + ownerBytes if ( nextEntries > MAX_AGGREGATED_PTY_PROCESS_LIST_ENTRIES || @@ -111,6 +132,7 @@ export class PtyProcessListAdmission { ...(value.worktreeId !== undefined ? { worktreeId: value.worktreeId } : {}), ...(value.terminalHandle !== undefined ? { terminalHandle: value.terminalHandle } : {}), ...(value.wslDistro !== undefined ? { wslDistro: value.wslDistro } : {}), + ...(value.ownerIdentity !== undefined ? { ownerIdentity: { ...value.ownerIdentity } } : {}), ...(normalizedOwners !== undefined ? { agentSessionOwners: normalizedOwners } : {}) } } diff --git a/src/main/providers/ssh-agent-session-capability.ts b/src/main/providers/ssh-agent-session-capability.ts new file mode 100644 index 00000000000..ffdf7a3c872 --- /dev/null +++ b/src/main/providers/ssh-agent-session-capability.ts @@ -0,0 +1 @@ +export const SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS = 5_000 diff --git a/src/main/providers/ssh-agent-session-claim-validation.ts b/src/main/providers/ssh-agent-session-claim-validation.ts index eaf33487a34..350836db851 100644 --- a/src/main/providers/ssh-agent-session-claim-validation.ts +++ b/src/main/providers/ssh-agent-session-claim-validation.ts @@ -6,7 +6,7 @@ import { } from '../../shared/agent-session-host-authority' import type { PtySpawnResult } from './pty-spawn-result' import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' -import { SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS } from './ssh-agent-session-create-operation' +import { SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS } from './ssh-agent-session-capability' import { isPtyIncarnationId } from '../../shared/pty-incarnation' export type ClaimedSshSpawnValidation = diff --git a/src/main/providers/ssh-agent-session-create-operation.ts b/src/main/providers/ssh-agent-session-create-operation.ts index adc0eb264d8..ea53a9cffe6 100644 --- a/src/main/providers/ssh-agent-session-create-operation.ts +++ b/src/main/providers/ssh-agent-session-create-operation.ts @@ -10,8 +10,8 @@ import { type PtySourceReceivingActivation } from '../../shared/pty-source-receiving-activation' import { validateClaimedSshSpawn } from './ssh-agent-session-claim-validation' - -export const SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS = 5_000 +export { SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS } from './ssh-agent-session-capability' +import { SSH_AGENT_SESSION_CAPABILITY_PROBE_TIMEOUT_MS } from './ssh-agent-session-capability' export function assertSshAgentSessionCreateResult( result: unknown diff --git a/src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts b/src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts index b3e0af53c37..da3d4ae6dd5 100644 --- a/src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts +++ b/src/main/providers/ssh-pty-provider-reattach-incarnation.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import { SshPtyProvider } from './ssh-pty-provider' -import { TerminalSessionOwnerUnverifiedError } from '../daemon/daemon-errors' -import { TerminalSessionExitedError } from '../daemon/daemon-errors' +import { + TerminalSessionExitedError, + TerminalSessionOwnerUnverifiedError +} from '../daemon/daemon-errors' describe('SSH PTY provider session reattach incarnation', () => { it('remembers the authoritative incarnation before a legacy exit arrives', async () => { diff --git a/src/main/runtime/workspace-session-terminal-membership-authority.ts b/src/main/runtime/workspace-session-terminal-membership-authority.ts index 2ec740a6ec5..66862e6ce8c 100644 --- a/src/main/runtime/workspace-session-terminal-membership-authority.ts +++ b/src/main/runtime/workspace-session-terminal-membership-authority.ts @@ -299,7 +299,13 @@ function rebaseOwnerBindings( session: WorkspaceSessionState, prior: WorkspaceSessionState ): WorkspaceSessionState['terminalPtyOwnersByPaneKey'] { - const allowed = new Set(Object.keys(rebaseIncarnationBindings(session, prior) ?? {})) + const retainedIncarnations = rebaseIncarnationBindings(session, prior) ?? {} const merged = { ...session.terminalPtyOwnersByPaneKey, ...prior.terminalPtyOwnersByPaneKey } - return Object.fromEntries(Object.entries(merged).filter(([paneKey]) => allowed.has(paneKey))) + return Object.fromEntries( + Object.entries(merged).filter( + ([paneKey, owner]) => + retainedIncarnations[paneKey] !== undefined && + owner.sessionIncarnationId === retainedIncarnations[paneKey] + ) + ) } diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index 3e40b5e5f57..5f330f1783b 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -59,7 +59,7 @@ describe('startup ordering', () => { expect(sweepEnd).toBeGreaterThan(sweepStart) expect(sweep).toContain('const provider = getDaemonProvider()') expect(sweep).toContain('if (!provider) {') - expect(sweep).toContain('provider.probePtyLiveness(ptyId)') + expect(sweep).toContain('provider.probePtyLiveness(ptyId, expectedIncarnationId)') }) it('bounds WSL reconciliation before serve RPC while leaving desktop startup independent', () => { diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts index 069ed371ec0..7bf0b358a58 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -69,7 +69,7 @@ describe('humanizeTerminalError', () => { it('replaces the pane-owner-unverified code with actionable copy', () => { const humanized = humanizeTerminalError('terminal_pane_owner_unverified') expect(humanized).not.toContain('terminal_pane_owner_unverified') - expect(humanized).toContain('Reopen this pane to retry') + expect(humanized).toContain('left the session and history untouched') }) it('humanizes an IPC-wrapped pane-owner-unverified error', () => { @@ -78,6 +78,15 @@ describe('humanizeTerminalError', () => { expect(humanizeTerminalError(wrapped)).not.toContain('terminal_pane_owner_unverified') }) + it('hides the owner-conflict diagnostic from user-facing copy', () => { + const humanized = humanizeTerminalError( + "terminal_pane_owner_conflict Error invoking remote method 'pty:spawn': Error: terminal_pane_owner_unverified" + ) + expect(humanized).not.toContain('terminal_pane_owner_conflict') + expect(humanized).not.toContain('Error invoking remote method') + expect(humanized).toContain('left the session and history untouched') + }) + it('leaves other errors untouched', () => { expect(humanizeTerminalError('Paste failed.')).toBe('Paste failed.') }) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index 6aa2bba6252..272b0ec8f56 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -1,4 +1,6 @@ import { useEffect, useState } from 'react' +import { ServerOff } from 'lucide-react' +import { Button } from '@/components/ui/button' import { translate } from '@/i18n/i18n' import { resolveClientEnvironmentFooter } from '@/lib/client-environment-info' import { hasClientEnvironmentFooter } from '../../../../shared/client-environment-info' @@ -17,7 +19,13 @@ const STALE_DAEMON_CWD_MARKERS = [ 'node-pty: daemon_cwd failed: ENOENT' ] // Thrown by ipc/pty.ts when a persisted pane owner can't be proven alive or dead (STA-3536). -const PANE_OWNER_UNVERIFIED_MARKER = 'terminal_pane_owner_unverified' +const PANE_OWNER_UNVERIFIED_MARKERS = [ + 'terminal_pane_owner_unverified', + 'terminal_pane_owner_conflict' +] +export function isPaneOwnerUnverifiedError(error: string): boolean { + return PANE_OWNER_UNVERIFIED_MARKERS.some((marker) => error.includes(marker)) +} // Why one source: the test and replace forms must match the same token, and a lone /g regex carries // lastIndex state across .test() calls. Capture the leading boundary so replacement can restore it. const TERMINAL_HOST_GONE_SOURCE = '(^|[^a-z0-9_])terminal_host_gone(?=$|[^a-z0-9_])' @@ -93,16 +101,13 @@ function humanizeUnreattachableSession(error: string): string { /** Swaps raw daemon-boundary codes for copy a user can act on. */ export function humanizeTerminalError(error: string): string { - let humanized = error - if (humanized.includes(PANE_OWNER_UNVERIFIED_MARKER)) { - humanized = humanized.replace( - PANE_OWNER_UNVERIFIED_MARKER, - translate( - 'auto.components.terminal.pane.TerminalErrorToast.7ee11bc0db', - "Orca couldn't confirm whether this terminal's previous session is still running, so it left the session untouched. Reopen this pane to retry." - ) + if (isPaneOwnerUnverifiedError(error)) { + return translate( + 'auto.components.terminal.pane.TerminalErrorToast.7ee11bc0db', + "Orca couldn't confirm whether this terminal's previous session is still running, so it left the session and history untouched." ) } + let humanized = error humanized = humanizeUnreattachableSession(humanized) if (!isExplainedTerminalError(humanized)) { return humanized @@ -128,16 +133,22 @@ export function humanizeTerminalError(error: string): string { export function TerminalErrorToast({ error, onDismiss, - onRestartDaemon + onRestartDaemon, + onRetry, + terminalLabel = 'Terminal session' }: { error: string onDismiss: () => void onRestartDaemon?: () => void + onRetry?: () => void + terminalLabel?: string }): React.JSX.Element { const ssh = isSshError(error) + const ownerUnavailable = isPaneOwnerUnverifiedError(error) const showDaemonRestart = !ssh && onRestartDaemon && shouldOfferDaemonRestart(error) + const showRetry = ownerUnavailable && onRetry !== undefined // Restart cannot recover a session after its owning daemon exits. - const showIssueLink = !ssh && !showDaemonRestart && !isExplainedTerminalError(error) + const showIssueLink = !ssh && !showDaemonRestart && !showRetry && !isExplainedTerminalError(error) const displayError = humanizeTerminalError(error) const [environmentFooter, setEnvironmentFooter] = useState<{ error: string @@ -162,6 +173,55 @@ export function TerminalErrorToast({ const footer = environmentFooter?.error === displayError ? environmentFooter.footer : '' + if (ownerUnavailable) { + return ( +
+
+
+ +
+
+
+
+ {translate( + 'auto.components.terminal.pane.TerminalErrorToast.ownerUnavailableTitle', + 'Terminal session unavailable' + )} +
+
+ {terminalLabel} +
+
+
+ {translate( + 'auto.components.terminal.pane.TerminalErrorToast.ownerUnavailableDetail', + "Orca couldn't confirm whether the previous session is still running. The tab and session are preserved; retry when the owner is available." + )} +
+ {footer ? ( +
+ {footer} +
+ ) : null} +
+ +
+
+ ) + } + return (
) : null} + {showRetry ? ( + + ) : null}