diff --git a/src/main/daemon/client.ts b/src/main/daemon/client.ts index b6229521c..dd1d9066a 100644 --- a/src/main/daemon/client.ts +++ b/src/main/daemon/client.ts @@ -18,6 +18,7 @@ import type { DaemonEvent } from './types' import { addNodePtyRecoveryHint } from './node-pty-error-hints' +import { decodeDaemonResponseError } from './daemon-errors' const CONNECT_TIMEOUT_MS = 5000 const CONNECTION_ATTEMPT_WAIT_MS = CONNECT_TIMEOUT_MS * 4 @@ -429,7 +430,12 @@ export class DaemonClient { if (response.ok) { pending.resolve(response.payload) } else { - pending.reject(new DaemonProtocolError(addNodePtyRecoveryHint(response.error))) + const decoded = decodeDaemonResponseError(response.error) + pending.reject( + decoded instanceof DaemonProtocolError + ? new DaemonProtocolError(addNodePtyRecoveryHint(response.error)) + : decoded + ) } } } diff --git a/src/main/daemon/daemon-errors.test.ts b/src/main/daemon/daemon-errors.test.ts new file mode 100644 index 000000000..d12ce645e --- /dev/null +++ b/src/main/daemon/daemon-errors.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' +import { + DaemonProtocolError, + decodeDaemonResponseError, + SessionNotFoundError +} from './daemon-errors' + +describe('decodeDaemonResponseError', () => { + it('types the exact legacy session-absence response', () => { + expect(decodeDaemonResponseError('Session not found: pty-1')).toBeInstanceOf( + SessionNotFoundError + ) + }) + + it('keeps unrelated daemon failures non-authoritative', () => { + expect(decodeDaemonResponseError('proxy failed: Session not found: pty-1')).toBeInstanceOf( + DaemonProtocolError + ) + }) +}) diff --git a/src/main/daemon/daemon-errors.ts b/src/main/daemon/daemon-errors.ts index 8077add31..bddb3690a 100644 --- a/src/main/daemon/daemon-errors.ts +++ b/src/main/daemon/daemon-errors.ts @@ -20,3 +20,17 @@ export class SessionNotFoundError extends Error { this.name = 'SessionNotFoundError' } } + +export class TerminalSessionOwnerUnverifiedError extends Error { + constructor(sessionId: string) { + super(`Terminal session owner could not be verified: ${sessionId}`) + this.name = 'TerminalSessionOwnerUnverifiedError' + } +} + +export function decodeDaemonResponseError(message: string): Error { + const prefix = 'Session not found: ' + return message.startsWith(prefix) + ? new SessionNotFoundError(message.slice(prefix.length)) + : new DaemonProtocolError(message) +} diff --git a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts index e4855640a..1d82ef70b 100644 --- a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts +++ b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts @@ -10,7 +10,8 @@ export class DaemonPtyAdapterSubscriptionFanout { constructor( private readonly adapters: readonly DaemonPtyAdapter[], - onAdapterExit: (id: string) => void + onAdapterExit: (id: string) => void, + onAdapterIdentityChanged?: (adapter: DaemonPtyAdapter) => void ) { for (const adapter of adapters) { this.unsubscribers.push( @@ -24,7 +25,10 @@ export class DaemonPtyAdapterSubscriptionFanout { for (const listener of this.exitListeners) { listener(payload) } - }) + }), + ...(onAdapterIdentityChanged && typeof adapter.onDaemonIdentityChanged === 'function' + ? [adapter.onDaemonIdentityChanged(() => onAdapterIdentityChanged(adapter))] + : []) ) } } diff --git a/src/main/daemon/daemon-pty-liveness-probe.ts b/src/main/daemon/daemon-pty-liveness-probe.ts deleted file mode 100644 index ba2c448bb..000000000 --- a/src/main/daemon/daemon-pty-liveness-probe.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { IPtyProvider } from '../providers/types' -import type { DaemonPtyAdapter } from './daemon-pty-adapter' - -export async function probePtyOwners( - id: string, - routed: IPtyProvider | undefined, - possibleOwners: readonly DaemonPtyAdapter[] -): Promise { - if (routed) { - return routed.probePtyLiveness - ? await routed.probePtyLiveness(id) - : (routed.hasPty?.(id) ?? null) - } - const results = await Promise.all(possibleOwners.map((provider) => provider.probePtyLiveness(id))) - return results.some((result) => result === true) - ? true - : results.every((result) => result === false) - ? false - : null -} diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index 2bee5164c..1930224c2 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { DaemonPtyRouter } from './daemon-pty-router' +import { SessionNotFoundError, TerminalSessionOwnerUnverifiedError } from './daemon-errors' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import type { PtyBackgroundStreamEvent, PtySpawnOptions, PtySpawnResult } from '../providers/types' import { @@ -18,6 +19,7 @@ type AdapterMock = DaemonPtyAdapter & { emitData: (id: string, data: string, sequenceChars?: number) => void emitBackground: (event: PtyBackgroundStreamEvent) => void emitExit: (id: string, code: number, incarnationId?: string) => void + emitIdentityChange: () => void triggerWriteUnavailable: (id: string) => void } @@ -44,6 +46,7 @@ function createAdapter( const writeUnavailableListeners: ((payload: { id: string }) => void)[] = [] const exitListeners: ((payload: { id: string; code: number; incarnationId?: string }) => void)[] = [] + const identityChangeListeners: (() => void)[] = [] return { protocolVersion, supportsGitCredentialGuardHost: () => @@ -136,6 +139,15 @@ function createAdapter( } } ), + onDaemonIdentityChanged: vi.fn((callback: () => void) => { + identityChangeListeners.push(callback) + return () => { + const idx = identityChangeListeners.indexOf(callback) + if (idx !== -1) { + identityChangeListeners.splice(idx, 1) + } + } + }), ackColdRestore: vi.fn(), clearTombstone: vi.fn(), reconcileOnStartup: vi.fn(async () => reconcileResult ?? { alive: sessions, killed: [] }), @@ -156,6 +168,7 @@ function createAdapter( listener({ id, code, ...(incarnationId ? { incarnationId } : {}) }) } }, + emitIdentityChange: () => identityChangeListeners.forEach((listener) => listener()), triggerWriteUnavailable: (id: string) => { for (const listener of writeUnavailableListeners) { listener({ id }) @@ -498,14 +511,16 @@ describe('DaemonPtyRouter', () => { expect(current.hasPty).not.toHaveBeenCalledWith('legacy-session') }) - it('probes every possible daemon owner for an unmapped session', async () => { + it('discovers an unmapped live session from one coalesced inventory', async () => { const current = createAdapter('current') const legacy = createAdapter('legacy', ['surviving-session']) const router = new DaemonPtyRouter({ current, legacy: [legacy] }) await expect(router.probePtyLiveness('surviving-session')).resolves.toBe(true) - expect(current.probePtyLiveness).toHaveBeenCalledExactlyOnceWith('surviving-session') - expect(legacy.probePtyLiveness).toHaveBeenCalledExactlyOnceWith('surviving-session') + expect(current.listProcesses).toHaveBeenCalledOnce() + expect(legacy.listProcesses).toHaveBeenCalledOnce() + expect(current.probePtyLiveness).not.toHaveBeenCalled() + expect(legacy.probePtyLiveness).not.toHaveBeenCalled() }) it('does not report absence while any possible daemon owner is unavailable', async () => { @@ -517,6 +532,102 @@ describe('DaemonPtyRouter', () => { await expect(router.probePtyLiveness('unknown-session')).resolves.toBeNull() }) + it('routes an attach to a legacy session created after startup inventory', async () => { + const current = createAdapter('current') + const legacySessions = ['legacy-at-startup'] + const legacy = createAdapter('legacy', legacySessions) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + legacySessions.push('legacy-created-later') + + await router.spawn({ + sessionId: 'legacy-created-later', + attachOnly: true, + cols: 80, + rows: 24 + }) + + expect(legacy.spawn).toHaveBeenCalledExactlyOnceWith({ + sessionId: 'legacy-created-later', + attachOnly: true, + cols: 80, + rows: 24 + }) + expect(current.spawn).not.toHaveBeenCalled() + }) + + it('still routes probes for a discovered session to its owning legacy daemon', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['legacy-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + await expect(router.probePtyLiveness('legacy-session')).resolves.toBe(true) + expect(legacy.probePtyLiveness).toHaveBeenCalledExactlyOnceWith('legacy-session') + }) + + it('keeps consulting a legacy daemon whose inventory listing failed', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['legacy-session']) + vi.mocked(legacy.listProcesses).mockRejectedValue(new Error('wedged')) + vi.mocked(legacy.probePtyLiveness).mockResolvedValue(null) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + // Without an inventory nothing proves the legacy daemon doesn't own this id. + await expect(router.probePtyLiveness('unknown-session')).resolves.toBeNull() + expect(legacy.listProcesses).toHaveBeenCalledTimes(2) + expect(legacy.probePtyLiveness).not.toHaveBeenCalled() + }) + + it('keeps an attach unresolved when any possible owner inventory fails', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy') + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + vi.mocked(legacy.listProcesses).mockRejectedValue(new Error('wedged')) + + await expect( + router.spawn({ sessionId: 'unknown-session', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(TerminalSessionOwnerUnverifiedError) + expect(current.spawn).not.toHaveBeenCalled() + expect(legacy.spawn).not.toHaveBeenCalled() + }) + + it('re-resolves a stale positive route before declaring an owner absent', async () => { + const current = createAdapter('current') + const firstSessions = ['moved-session'] + const secondSessions: string[] = [] + const first = createAdapter('first', firstSessions) + const second = createAdapter('second', secondSessions) + const router = new DaemonPtyRouter({ current, legacy: [first, second] }) + await router.discoverLegacySessions() + firstSessions.length = 0 + secondSessions.push('moved-session') + vi.mocked(first.spawn).mockRejectedValueOnce(new SessionNotFoundError('moved-session')) + + await router.spawn({ sessionId: 'moved-session', attachOnly: true, cols: 80, rows: 24 }) + + expect(first.spawn).toHaveBeenCalledOnce() + expect(second.spawn).toHaveBeenCalledOnce() + }) + + it('invalidates positive routes when a daemon endpoint identity changes', async () => { + const currentSessions: string[] = [] + const legacySessions = ['moved-session'] + const current = createAdapter('current', currentSessions) + const legacy = createAdapter('legacy', legacySessions) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + legacySessions.length = 0 + currentSessions.push('moved-session') + + legacy.emitIdentityChange() + await router.spawn({ sessionId: 'moved-session', attachOnly: true, cols: 80, rows: 24 }) + + expect(current.spawn).toHaveBeenCalledOnce() + expect(legacy.spawn).not.toHaveBeenCalled() + }) + it('hands a checkpointed pre-v30 session to the current daemon on wake', async () => { const current = createAdapter('current', [], undefined, HISTORY_SEED_TRANSFER_PROTOCOL_VERSION) const legacy = createAdapter( @@ -582,7 +693,7 @@ describe('DaemonPtyRouter', () => { await expect(router.listProcesses()).rejects.toThrow('legacy exited') await expect(router.listProcesses()).rejects.toThrow('legacy exited') expect(router.getLegacyAdapters()).toEqual([legacy]) - expect(current.listProcesses).toHaveBeenCalledTimes(2) + expect(current.listProcesses).toHaveBeenCalledTimes(3) }) it('pins colliding unmapped legacy ids falling through to the current daemon', async () => { diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 0f07da4dc..8429495ed 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -9,44 +9,44 @@ import type { PtySpawnResult } from '../providers/types' import type { PtyProcessInspection } from '../providers/pty-process-inspection' -import { probePtyOwners } from './daemon-pty-liveness-probe' import { shouldHandoffDaemonHistory } from './daemon-history-handoff' import type { DaemonPtyRouterDataEvent, DaemonPtyRouterExitEvent } from './daemon-pty-router-events' +import { DaemonSessionOwnerResolver } from './daemon-session-owner-resolution' export class DaemonPtyRouter implements IPtyProvider { private current: DaemonPtyAdapter private legacy: DaemonPtyAdapter[] private sessionAdapters = new Map() + private readonly ownerResolver: DaemonSessionOwnerResolver private readonly subscriptions: DaemonPtyAdapterSubscriptionFanout constructor(opts: { current: DaemonPtyAdapter; legacy: DaemonPtyAdapter[] }) { this.current = opts.current this.legacy = opts.legacy - this.subscriptions = new DaemonPtyAdapterSubscriptionFanout(this.allAdapters(), (id) => { - this.sessionAdapters.delete(id) - }) + this.ownerResolver = new DaemonSessionOwnerResolver(this.allAdapters(), this.sessionAdapters) + this.subscriptions = new DaemonPtyAdapterSubscriptionFanout( + this.allAdapters(), + (id) => { + this.ownerResolver.forgetRoute(id) + }, + (adapter) => this.ownerResolver.invalidateProvider(adapter) + ) } async discoverLegacySessions(): Promise { - for (const adapter of this.legacy) { - try { - const sessions = await adapter.listProcesses() - for (const session of sessions) { - this.sessionAdapters.set(session.id, adapter) - } - } catch (error) { - console.warn('[daemon] Failed to discover legacy daemon sessions', error) - } - } + await this.ownerResolver.discoverRoutes() } async spawn(opts: PtySpawnOptions): Promise { + if (opts.attachOnly && opts.sessionId) { + return await this.ownerResolver.spawnAttachOnly({ ...opts, sessionId: opts.sessionId }) + } const adapter = opts.sessionId ? this.sessionAdapters.get(opts.sessionId) : undefined const target = adapter ?? this.current const result = await target.spawn(opts) // Why: the adapter filters intentional recovery exits and canonical-ID races before publishing proof. if (!result.exitedBeforeSpawnReply) { - this.sessionAdapters.set(result.id, target) + this.ownerResolver.recordRoute(result.id, target, result.incarnationId) } return result } @@ -86,7 +86,7 @@ export class DaemonPtyRouter implements IPtyProvider { } async probePtyLiveness(id: string): Promise { - return await probePtyOwners(id, this.sessionAdapters.get(id), this.allAdapters()) + return await this.ownerResolver.probe(id) } write(id: string, data: string): void { @@ -121,7 +121,7 @@ export class DaemonPtyRouter implements IPtyProvider { adapter.ackColdRestore(id) } if (this.sessionAdapters.get(id) === adapter) { - this.sessionAdapters.delete(id) + this.ownerResolver.forgetRoute(id, adapter) } } } @@ -240,6 +240,7 @@ export class DaemonPtyRouter implements IPtyProvider { }> { const alive: string[] = [] const killed: string[] = [] + const aliveProviders = new Map>() for (const adapter of this.allAdapters()) { const result = await adapter.reconcileOnStartup(validWorktreeIds) // Why: daemon startup can reconcile many restored sessions; spreading @@ -251,10 +252,17 @@ export class DaemonPtyRouter implements IPtyProvider { killed.push(id) } for (const id of result.alive) { - this.sessionAdapters.set(id, adapter) + const providers = aliveProviders.get(id) ?? new Set() + providers.add(adapter) + aliveProviders.set(id, providers) } - for (const id of result.killed) { - this.sessionAdapters.delete(id) + } + for (const id of new Set([...alive, ...killed])) { + const providers = aliveProviders.get(id) + if (providers?.size === 1) { + this.ownerResolver.recordRoute(id, providers.values().next().value!) + } else { + this.ownerResolver.forgetRoute(id) } } return { alive, killed } diff --git a/src/main/daemon/daemon-session-owner-resolution.test.ts b/src/main/daemon/daemon-session-owner-resolution.test.ts new file mode 100644 index 000000000..277c0750a --- /dev/null +++ b/src/main/daemon/daemon-session-owner-resolution.test.ts @@ -0,0 +1,450 @@ +import { describe, expect, it, vi } from 'vitest' +import type { + IPtyProvider, + PtyProcessInfo, + PtySpawnOptions, + PtySpawnResult +} from '../providers/types' +import { SessionNotFoundError, TerminalSessionOwnerUnverifiedError } from './daemon-errors' +import { DaemonSessionOwnerResolver } from './daemon-session-owner-resolution' + +function provider( + processes: () => Promise, + spawn: (opts: PtySpawnOptions) => Promise = async (opts) => ({ + id: opts.sessionId ?? 'new' + }) +): IPtyProvider { + return { + listProcesses: vi.fn(processes), + spawn: vi.fn(spawn) + } as unknown as IPtyProvider +} + +describe('DaemonSessionOwnerResolver', () => { + it('coalesces concurrent all-provider inventories', async () => { + let release!: () => void + const gate = new Promise((resolve) => { + release = resolve + }) + const firstInventory = vi.fn(async () => { + await gate + return [] + }) + const secondInventory = vi.fn(async () => { + await gate + return [] + }) + const first = provider(firstInventory) + const second = provider(secondInventory) + const resolver = new DaemonSessionOwnerResolver([first, second], new Map()) + + const resolutions = Promise.all([resolver.resolve('one'), resolver.resolve('two')]) + await vi.waitFor(() => expect(firstInventory).toHaveBeenCalledOnce()) + expect(secondInventory).toHaveBeenCalledOnce() + release() + + await expect(resolutions).resolves.toEqual([{ kind: 'unknown' }, { kind: 'unknown' }]) + expect(firstInventory).toHaveBeenCalledOnce() + expect(secondInventory).toHaveBeenCalledOnce() + }) + + it('bounds restore inventory requests across many panes and daemon generations', async () => { + const inventories = Array.from({ length: 51 }, (_, index) => + vi.fn(async () => { + if (index === 50) { + throw new Error('offline') + } + return [] + }) + ) + const resolver = new DaemonSessionOwnerResolver( + inventories.map((inventory) => provider(inventory)), + new Map() + ) + + for (let index = 0; index < 40; index += 1) { + await resolver.resolve(`pane-${index}`) + } + + for (const inventory of inventories) { + expect(inventory).toHaveBeenCalledOnce() + } + }) + + it('reuses incomplete inventory across serialized restores', async () => { + const firstInventory = vi.fn(async () => []) + const secondInventory = vi.fn(async () => { + throw new Error('offline') + }) + const resolver = new DaemonSessionOwnerResolver( + [provider(firstInventory), provider(secondInventory)], + new Map() + ) + + await resolver.resolve('first') + await resolver.resolve('second') + + expect(firstInventory).toHaveBeenCalledOnce() + expect(secondInventory).toHaveBeenCalledOnce() + }) + + it('does not extend incomplete inventory lifetime on cache hits', async () => { + let now = 1_000 + const clock = vi.spyOn(Date, 'now').mockImplementation(() => now) + const inventory = vi.fn(async () => { + throw new Error('offline') + }) + const resolver = new DaemonSessionOwnerResolver( + [provider(async () => []), provider(inventory)], + new Map() + ) + + try { + await resolver.resolve('first') + now += 900 + await resolver.resolve('second') + now += 101 + await resolver.resolve('third') + + expect(inventory).toHaveBeenCalledTimes(2) + } finally { + clock.mockRestore() + } + }) + + it('does not let startup inventory hide a session created before restore', async () => { + const sessions: PtyProcessInfo[] = [] + const inventory = vi.fn(async () => sessions) + const owner = provider(inventory, async (opts) => ({ + id: opts.sessionId!, + incarnationId: 'live', + isReattach: true + })) + const resolver = new DaemonSessionOwnerResolver([owner, provider(async () => [])], new Map()) + + await resolver.discoverRoutes() + sessions.push({ id: 'session', incarnationId: 'live', cwd: '', title: 'live' }) + + await expect( + resolver.spawnAttachOnly({ sessionId: 'session', attachOnly: true, cols: 80, rows: 24 }) + ).resolves.toMatchObject({ id: 'session', incarnationId: 'live', isReattach: true }) + expect(inventory).toHaveBeenCalledTimes(2) + }) + + it('uses the expected incarnation to disambiguate duplicate session ids', async () => { + const oldOwner = provider(async () => [ + { id: 'session', incarnationId: 'old', cwd: '', title: 'old' } + ]) + const exactOwner = provider(async () => [ + { id: 'session', incarnationId: 'expected', cwd: '', title: 'exact' } + ]) + const resolver = new DaemonSessionOwnerResolver([oldOwner, exactOwner], new Map()) + + await expect(resolver.resolve('session', 'expected')).resolves.toMatchObject({ + kind: 'owner', + provider: exactOwner + }) + }) + + it('accepts one exact-id owner when persisted incarnation evidence is stale', async () => { + const liveOwner = provider(async () => [ + { id: 'session', incarnationId: 'live', cwd: '', title: 'live' } + ]) + const resolver = new DaemonSessionOwnerResolver([liveOwner], new Map()) + + await expect(resolver.resolve('session', 'stale')).resolves.toMatchObject({ + kind: 'owner', + provider: liveOwner + }) + }) + + it('does not inventory unrelated providers for a routed stale persisted incarnation', async () => { + const routedOwner = provider( + async () => [{ id: 'session', incarnationId: 'live', cwd: '', title: 'live' }], + async () => ({ id: 'session', incarnationId: 'live', isReattach: true }) + ) + const unrelatedInventory = vi.fn(async () => { + throw new Error('offline') + }) + const resolver = new DaemonSessionOwnerResolver( + [routedOwner, provider(unrelatedInventory)], + new Map([['session', routedOwner]]) + ) + + await expect( + resolver.spawnAttachOnly({ + sessionId: 'session', + expectedIncarnationId: 'stale', + attachOnly: true, + cols: 80, + rows: 24 + }) + ).resolves.toMatchObject({ id: 'session', incarnationId: 'live', isReattach: true }) + expect(unrelatedInventory).not.toHaveBeenCalled() + }) + + it('re-resolves a routed incarnation mismatch against every possible owner', async () => { + const staleRoute = provider( + async () => [{ id: 'session', incarnationId: 'other', cwd: '', title: 'stale' }], + async () => ({ id: 'session', incarnationId: 'other', isReattach: true }) + ) + const exactOwner = provider( + async () => [{ id: 'session', incarnationId: 'expected', cwd: '', title: 'exact' }], + async () => ({ id: 'session', incarnationId: 'expected', isReattach: true }) + ) + const resolver = new DaemonSessionOwnerResolver( + [staleRoute, exactOwner], + new Map([['session', staleRoute]]) + ) + + await expect( + resolver.spawnAttachOnly({ + sessionId: 'session', + expectedIncarnationId: 'expected', + expectedIncarnationIsAuthoritative: true, + attachOnly: true, + cols: 80, + rows: 24 + }) + ).resolves.toMatchObject({ incarnationId: 'expected' }) + expect(staleRoute.spawn).not.toHaveBeenCalled() + }) + + it('refreshes incomplete inventory when a routed owner refuses a moved session', async () => { + let firstOwnsSession = true + let secondOwnsSession = false + const first = provider( + async () => + firstOwnsSession + ? [{ id: 'session', incarnationId: 'runtime', cwd: '', title: 'first' }] + : [], + async () => { + throw new SessionNotFoundError('session') + } + ) + const second = provider( + async () => + secondOwnsSession + ? [{ id: 'session', incarnationId: 'runtime', cwd: '', title: 'second' }] + : [], + async () => ({ id: 'session', incarnationId: 'runtime', isReattach: true }) + ) + const unavailable = provider(async () => { + throw new Error('offline') + }) + const resolver = new DaemonSessionOwnerResolver([first, second, unavailable], new Map()) + + await expect(resolver.resolve('session', 'runtime', true)).resolves.toMatchObject({ + kind: 'owner', + provider: first + }) + firstOwnsSession = false + secondOwnsSession = true + + await expect( + resolver.spawnAttachOnly({ + sessionId: 'session', + expectedIncarnationId: 'runtime', + expectedIncarnationIsAuthoritative: true, + attachOnly: true, + cols: 80, + rows: 24 + }) + ).resolves.toMatchObject({ id: 'session', incarnationId: 'runtime', isReattach: true }) + expect(first.spawn).toHaveBeenCalledOnce() + expect(second.spawn).toHaveBeenCalledOnce() + expect(second.listProcesses).toHaveBeenCalledTimes(2) + }) + + it('accepts exact runtime proof from an incomplete inventory', async () => { + const exactOwner = provider(async () => [ + { id: 'session', incarnationId: 'runtime', cwd: '', title: 'exact' } + ]) + const resolver = new DaemonSessionOwnerResolver( + [ + exactOwner, + provider(async () => { + throw new Error('offline') + }) + ], + new Map() + ) + + await expect(resolver.resolve('session', 'runtime', true)).resolves.toMatchObject({ + kind: 'owner', + provider: exactOwner + }) + }) + + it('does not trust persisted incarnation proof from an incomplete inventory', async () => { + const resolver = new DaemonSessionOwnerResolver( + [ + provider(async () => [ + { id: 'session', incarnationId: 'persisted', cwd: '', title: 'candidate' } + ]), + provider(async () => { + throw new Error('offline') + }) + ], + new Map() + ) + + await expect(resolver.resolve('session', 'persisted')).resolves.toEqual({ kind: 'unknown' }) + }) + + it('accepts positive liveness proof from an incomplete inventory', async () => { + const resolver = new DaemonSessionOwnerResolver( + [ + provider(async () => [{ id: 'session', cwd: '', title: 'live' }]), + provider(async () => { + throw new Error('offline') + }) + ], + new Map() + ) + + await expect(resolver.probe('session')).resolves.toBe(true) + }) + + it('fails closed when duplicate owners cannot be disambiguated', async () => { + const first = provider(async () => [{ id: 'session', cwd: '', title: 'first' }]) + const second = provider(async () => [{ id: 'session', cwd: '', title: 'second' }]) + const resolver = new DaemonSessionOwnerResolver([first, second], new Map()) + + await expect(resolver.resolve('session')).resolves.toEqual({ kind: 'unknown' }) + }) + + it('reports confirmed absence from a sole owner', async () => { + const owner = provider( + async () => [], + async () => { + throw new SessionNotFoundError('missing') + } + ) + const resolver = new DaemonSessionOwnerResolver([owner], new Map()) + + await expect( + resolver.spawnAttachOnly({ sessionId: 'missing', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(SessionNotFoundError) + expect(owner.spawn).toHaveBeenCalledOnce() + expect(owner.listProcesses).not.toHaveBeenCalled() + }) + + it('keeps absence unverified when there are no possible owners', async () => { + const resolver = new DaemonSessionOwnerResolver([], new Map()) + + await expect(resolver.resolve('missing')).resolves.toEqual({ kind: 'unknown' }) + }) + + it('preserves an unresolved owner when any provider inventory fails', async () => { + const resolver = new DaemonSessionOwnerResolver( + [ + provider(async () => []), + provider(async () => { + throw new Error('offline') + }) + ], + new Map() + ) + + await expect( + resolver.spawnAttachOnly({ sessionId: 'missing', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(TerminalSessionOwnerUnverifiedError) + }) + + it('does not turn a raced owner refusal into absence while another provider is unresolved', async () => { + const candidate = provider( + async () => [{ id: 'session', cwd: '', title: 'candidate' }], + async () => { + throw new SessionNotFoundError('session') + } + ) + const resolver = new DaemonSessionOwnerResolver( + [ + candidate, + provider(async () => { + throw new Error('offline') + }) + ], + new Map() + ) + + await expect( + resolver.spawnAttachOnly({ sessionId: 'session', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(TerminalSessionOwnerUnverifiedError) + }) + + it('does not turn a post-inventory owner refusal into aggregate absence', async () => { + const candidate = provider( + async () => [{ id: 'session', cwd: '', title: 'candidate' }], + async () => { + throw new SessionNotFoundError('session') + } + ) + const resolver = new DaemonSessionOwnerResolver( + [candidate, provider(async () => [])], + new Map() + ) + + await expect( + resolver.spawnAttachOnly({ sessionId: 'session', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(TerminalSessionOwnerUnverifiedError) + }) + + it('pre-routes every uniquely inventoried session for serialized restores', async () => { + const inventory = vi.fn(async () => [ + { id: 'first', cwd: '', title: 'first' }, + { id: 'second', cwd: '', title: 'second' } + ]) + const owner = provider(inventory) + const emptyInventory = vi.fn(async () => []) + const resolver = new DaemonSessionOwnerResolver([owner, provider(emptyInventory)], new Map()) + + await resolver.spawnAttachOnly({ sessionId: 'first', attachOnly: true, cols: 80, rows: 24 }) + await resolver.spawnAttachOnly({ sessionId: 'second', attachOnly: true, cols: 80, rows: 24 }) + + expect(inventory).toHaveBeenCalledOnce() + expect(emptyInventory).toHaveBeenCalledOnce() + }) + + it('discards an inventory completed after daemon identity replacement', async () => { + let release!: () => void + const gate = new Promise((resolve) => { + release = resolve + }) + const inventory = vi.fn(async () => { + await gate + return [{ id: 'session', cwd: '', title: 'stale' }] + }) + const owner = provider(inventory) + const resolver = new DaemonSessionOwnerResolver([owner], new Map()) + + const resolution = resolver.resolve('session') + await vi.waitFor(() => expect(inventory).toHaveBeenCalledOnce()) + resolver.invalidateProvider(owner) + release() + + await expect(resolution).resolves.toEqual({ kind: 'unknown' }) + await resolver.resolve('session') + expect(inventory).toHaveBeenCalledTimes(2) + }) + + it('restores a proven route when identity changes during exact reattach', async () => { + const routes = new Map() + let resolver!: DaemonSessionOwnerResolver + let owner!: IPtyProvider + owner = provider( + async () => [], + async () => { + resolver.invalidateProvider(owner) + return { id: 'session', incarnationId: 'inc', isReattach: true } + } + ) + routes.set('session', owner) + resolver = new DaemonSessionOwnerResolver([owner], routes) + + await resolver.spawnAttachOnly({ sessionId: 'session', attachOnly: true, cols: 80, rows: 24 }) + + expect(routes.get('session')).toBe(owner) + }) +}) diff --git a/src/main/daemon/daemon-session-owner-resolution.ts b/src/main/daemon/daemon-session-owner-resolution.ts new file mode 100644 index 000000000..d35d2795d --- /dev/null +++ b/src/main/daemon/daemon-session-owner-resolution.ts @@ -0,0 +1,300 @@ +import type { + IPtyProvider, + PtyProcessInfo, + PtySpawnOptions, + PtySpawnResult +} from '../providers/types' +import { SessionNotFoundError, TerminalSessionOwnerUnverifiedError } from './daemon-errors' + +export type DaemonSessionOwnerResolution = + | { kind: 'owner'; provider: T } + | { kind: 'absent' } + | { kind: 'unknown' } + +type ProviderInventory = { provider: T; processes: PtyProcessInfo[] | null } + +type OwnerInventory = { + candidatesBySessionId: Map + complete: boolean + epoch: number +} + +const OWNER_RESOLUTION_TIMEOUT_MS = 2_000 +const OWNER_INVENTORY_CACHE_MS = 1_000 +const FAILED_PROVIDER_COOLDOWN_MS = 1_000 + +function assertClientConnected(signal: PtySpawnOptions['signal']): void { + if (signal?.aborted) { + throw new Error('client_disconnected') + } +} + +export class DaemonSessionOwnerResolver { + private inventoryInFlight: Promise> | null = null + private cachedInventory: { value: OwnerInventory; expiresAt: number } | null = null + private readonly failedProviderCooldowns = new Map() + private readonly routeIncarnations = new Map() + private epoch = 0 + + constructor( + private readonly providers: readonly T[], + private readonly routes: Map + ) {} + + invalidateProvider(provider: T): void { + this.epoch += 1 + this.inventoryInFlight = null + this.cachedInventory = null + this.failedProviderCooldowns.clear() + for (const [sessionId, routed] of this.routes) { + if (routed === provider) { + this.routes.delete(sessionId) + this.routeIncarnations.delete(sessionId) + } + } + } + + async spawnAttachOnly(opts: PtySpawnOptions & { sessionId: string }): Promise { + assertClientConnected(opts.signal) + const routed = this.providers.find((provider) => provider === this.routes.get(opts.sessionId)) + const direct = routed ?? (this.providers.length === 1 ? this.providers[0] : undefined) + const routedIncarnation = this.routeIncarnations.get(opts.sessionId) + const routeNeedsAuthoritativeResolution = + direct && + routed && + opts.expectedIncarnationIsAuthoritative === true && + routedIncarnation !== opts.expectedIncarnationId + if (direct && !routeNeedsAuthoritativeResolution) { + try { + const result = await direct.spawn(opts) + if ( + !result.exitedBeforeSpawnReply && + result.id === opts.sessionId && + result.isReattach === true + ) { + this.recordRoute(result.id, direct, result.incarnationId) + } + return result + } catch (error) { + if (!(error instanceof SessionNotFoundError)) { + throw error + } + if (this.providers.length === 1) { + throw error + } + if (routed && this.routes.get(opts.sessionId) === routed) { + this.forgetRoute(opts.sessionId, routed) + } + } + } + + assertClientConnected(opts.signal) + const resolution = await this.resolve( + opts.sessionId, + opts.expectedIncarnationId, + opts.expectedIncarnationIsAuthoritative + ) + assertClientConnected(opts.signal) + if (resolution.kind === 'unknown') { + throw new TerminalSessionOwnerUnverifiedError(opts.sessionId) + } + if (resolution.kind === 'absent') { + throw new SessionNotFoundError(opts.sessionId) + } + try { + const result = await resolution.provider.spawn(opts) + if ( + !result.exitedBeforeSpawnReply && + result.id === opts.sessionId && + result.isReattach === true + ) { + this.recordRoute(result.id, resolution.provider, result.incarnationId) + } + return result + } catch (error) { + if (error instanceof SessionNotFoundError && this.providers.length > 1) { + throw new TerminalSessionOwnerUnverifiedError(opts.sessionId) + } + throw error + } + } + + async probe(sessionId: string): Promise { + const routed = this.providers.find((provider) => provider === this.routes.get(sessionId)) + const direct = routed ?? (this.providers.length === 1 ? this.providers[0] : undefined) + if (direct) { + const verdict = direct.probePtyLiveness + ? await direct.probePtyLiveness(sessionId) + : (direct.hasPty?.(sessionId) ?? null) + if (verdict !== false || this.providers.length === 1) { + return verdict + } + if (this.routes.get(sessionId) === routed) { + this.routes.delete(sessionId) + this.routeIncarnations.delete(sessionId) + } + } + const inventory = await this.inventory(false) + if (inventory.epoch !== this.epoch) { + return null + } + if ((inventory.candidatesBySessionId.get(sessionId)?.length ?? 0) > 0) { + return true + } + const resolution = this.resolveInventory(inventory, sessionId) + if (resolution.kind === 'owner') { + return resolution.provider === routed ? null : true + } + return resolution.kind === 'absent' ? false : null + } + + async resolve( + sessionId: string, + expectedIncarnationId?: string, + expectedIncarnationIsAuthoritative = false + ): Promise> { + const inventory = await this.inventory() + if (inventory.epoch !== this.epoch) { + return { kind: 'unknown' } + } + const resolution = this.resolveInventory( + inventory, + sessionId, + expectedIncarnationId, + expectedIncarnationIsAuthoritative + ) + if ( + (!inventory.complete || resolution.kind === 'unknown') && + this.cachedInventory?.value !== inventory + ) { + this.cachedInventory = { + value: inventory, + expiresAt: Date.now() + OWNER_INVENTORY_CACHE_MS + } + } + return resolution + } + + async discoverRoutes(): Promise { + await this.inventory() + this.failedProviderCooldowns.clear() + this.cachedInventory = null + } + + recordRoute(sessionId: string, provider: T, incarnationId?: string): void { + this.routes.set(sessionId, provider) + this.routeIncarnations.set(sessionId, incarnationId) + } + + forgetRoute(sessionId: string, provider?: T): void { + if (provider && this.routes.get(sessionId) !== provider) { + return + } + this.routes.delete(sessionId) + this.routeIncarnations.delete(sessionId) + this.cachedInventory = null + } + + private resolveInventory( + inventory: OwnerInventory, + sessionId: string, + expectedIncarnationId?: string, + expectedIncarnationIsAuthoritative = false + ): DaemonSessionOwnerResolution { + const candidates = inventory.candidatesBySessionId.get(sessionId) ?? [] + const providers = new Set(candidates.map(({ provider }) => provider)) + const exactProviders = new Set( + candidates + .filter(({ process }) => process.incarnationId === expectedIncarnationId) + .map(({ provider }) => provider) + ) + const exactProvider = + expectedIncarnationId && exactProviders.size === 1 + ? exactProviders.values().next().value + : undefined + const soleProvider = providers.size === 1 ? providers.values().next().value : undefined + const provider = + (exactProvider && (inventory.complete || expectedIncarnationIsAuthoritative) + ? exactProvider + : undefined) ?? + (!expectedIncarnationIsAuthoritative && inventory.complete ? soleProvider : undefined) + if (provider) { + const process = candidates.find((candidate) => candidate.provider === provider)?.process + this.recordRoute(sessionId, provider, process?.incarnationId) + return { kind: 'owner', provider } + } + if (!inventory.complete || providers.size > 1 || this.providers.length !== 1) { + return { kind: 'unknown' } + } + return { kind: 'absent' } + } + + private inventory(allowCached = true): Promise> { + if (this.inventoryInFlight) { + return this.inventoryInFlight + } + if ( + allowCached && + this.cachedInventory?.value.epoch === this.epoch && + this.cachedInventory.expiresAt > Date.now() + ) { + return Promise.resolve(this.cachedInventory.value) + } + this.cachedInventory = null + const deadlineMs = Date.now() + OWNER_RESOLUTION_TIMEOUT_MS + const epoch = this.epoch + const inventory = Promise.all( + this.providers.map(async (provider): Promise> => { + if ((this.failedProviderCooldowns.get(provider) ?? 0) > Date.now()) { + return { provider, processes: null } + } + this.failedProviderCooldowns.delete(provider) + try { + return { provider, processes: await provider.listProcesses({ deadlineMs }) } + } catch { + if (epoch === this.epoch) { + this.failedProviderCooldowns.set(provider, Date.now() + FAILED_PROVIDER_COOLDOWN_MS) + } + return { provider, processes: null } + } + }) + ) + .then((entries) => this.indexInventory(entries, epoch)) + .finally(() => { + if (this.inventoryInFlight === inventory) { + this.inventoryInFlight = null + } + }) + this.inventoryInFlight = inventory + return inventory + } + + private indexInventory(entries: ProviderInventory[], epoch: number): OwnerInventory { + const candidatesBySessionId = new Map() + let complete = true + for (const entry of entries) { + if (!entry.processes) { + complete = false + continue + } + for (const process of entry.processes) { + const candidates = candidatesBySessionId.get(process.id) ?? [] + candidates.push({ provider: entry.provider, process }) + candidatesBySessionId.set(process.id, candidates) + } + } + if (complete && epoch === this.epoch) { + for (const [sessionId, candidates] of candidatesBySessionId) { + const providers = new Set(candidates.map(({ provider }) => provider)) + if (providers.size === 1) { + const provider = providers.values().next().value! + const process = candidates.find((candidate) => candidate.provider === provider)?.process + this.recordRoute(sessionId, provider, process?.incarnationId) + } else { + this.forgetRoute(sessionId) + } + } + } + return { candidatesBySessionId, complete, epoch } + } +} diff --git a/src/main/daemon/degraded-daemon-owner-recovery.ts b/src/main/daemon/degraded-daemon-owner-recovery.ts new file mode 100644 index 000000000..ec8798a5c --- /dev/null +++ b/src/main/daemon/degraded-daemon-owner-recovery.ts @@ -0,0 +1,82 @@ +import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types' +import type { DaemonPtyAdapter } from './daemon-pty-adapter' +import { DaemonSessionOwnerResolver } from './daemon-session-owner-resolution' + +export class DegradedDaemonOwnerRecovery { + private readonly attachResolver: DaemonSessionOwnerResolver + private readonly livenessResolver: DaemonSessionOwnerResolver + + constructor( + providers: readonly IPtyProvider[], + private readonly daemonAdapters: readonly DaemonPtyAdapter[], + routes: Map, + private readonly spawnFresh: (opts: PtySpawnOptions) => Promise + ) { + this.attachResolver = new DaemonSessionOwnerResolver(providers, routes) + this.livenessResolver = new DaemonSessionOwnerResolver(daemonAdapters, routes) + } + + spawn = async (opts: PtySpawnOptions): Promise => { + return opts.attachOnly && opts.sessionId + ? await this.attachResolver.spawnAttachOnly({ ...opts, sessionId: opts.sessionId }) + : await this.spawnFresh(opts) + } + + probe = async (sessionId: string): Promise => + await this.livenessResolver.probe(sessionId) + + discoverRoutes = async (): Promise => await this.attachResolver.discoverRoutes() + + async reconcileOnStartup(validWorktreeIds: Set): Promise<{ + alive: string[] + killed: string[] + }> { + const alive: string[] = [] + const killed: string[] = [] + const aliveProviders = new Map>() + for (const adapter of this.daemonAdapters) { + const result = await adapter.reconcileOnStartup(validWorktreeIds) + for (const id of result.alive) { + alive.push(id) + const providers = aliveProviders.get(id) ?? new Set() + providers.add(adapter) + aliveProviders.set(id, providers) + } + killed.push(...result.killed) + } + for (const id of new Set([...alive, ...killed])) { + const providers = aliveProviders.get(id) + if (providers?.size === 1) { + this.recordRoute(id, providers.values().next().value!) + } else { + this.forgetRoute(id) + } + } + return { alive, killed } + } + + recordRoute(sessionId: string, provider: IPtyProvider): void { + this.attachResolver.recordRoute(sessionId, provider) + if (this.daemonAdapters.includes(provider as DaemonPtyAdapter)) { + this.livenessResolver.recordRoute(sessionId, provider as DaemonPtyAdapter) + } + } + + forgetRoute(sessionId: string): void { + this.attachResolver.forgetRoute(sessionId) + this.livenessResolver.forgetRoute(sessionId) + } + + subscribeIdentityChanges(): (() => void)[] { + return this.daemonAdapters.flatMap((adapter) => + typeof adapter.onDaemonIdentityChanged === 'function' + ? [ + adapter.onDaemonIdentityChanged(() => { + this.attachResolver.invalidateProvider(adapter) + this.livenessResolver.invalidateProvider(adapter) + }) + ] + : [] + ) + } +} diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index 644f732be..af7c30d1c 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -4,6 +4,7 @@ import { DEGRADED_DAEMON_RECOVERY_RETRY_MS } from './degraded-daemon-fresh-spawn import type { DaemonPtyAdapter } from './daemon-pty-adapter' import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types' import type { PtyProcessInspection } from '../providers/pty-process-inspection' +import { TerminalSessionOwnerUnverifiedError } from './daemon-errors' type ProviderMock = IPtyProvider & { probePtyLiveness: (id: string) => Promise @@ -158,6 +159,47 @@ it('forwards dead-endpoint write-unavailable signals from the daemon adapters', expect(recovered).toEqual(['daemon-pane', 'legacy-pane']) }) +it('routes attach-only to a legacy session created after startup inventory', async () => { + const current = createDaemonAdapter('daemon') + const legacySessions = ['legacy-at-startup'] + const legacy = createDaemonAdapter('legacy', legacySessions) + const fallback = createProvider('fallback') + const provider = new DegradedDaemonPtyProvider({ + current, + legacy: [legacy], + fallback + }) + await provider.discoverDaemonSessions() + legacySessions.push('legacy-created-later') + + await provider.spawn({ + sessionId: 'legacy-created-later', + attachOnly: true, + cols: 80, + rows: 24 + }) + + expect(legacy.spawn).toHaveBeenCalledOnce() + expect(current.spawn).not.toHaveBeenCalled() + expect(fallback.spawn).not.toHaveBeenCalled() +}) + +it('keeps an attach unresolved when a legacy inventory listing fails', async () => { + const current = createDaemonAdapter('daemon') + const legacy = createDaemonAdapter('legacy') + vi.mocked(legacy.listProcesses).mockRejectedValue(new Error('wedged')) + const provider = new DegradedDaemonPtyProvider({ + current, + legacy: [legacy], + fallback: createProvider('fallback') + }) + + await expect( + provider.spawn({ sessionId: 'unknown-session', attachOnly: true, cols: 80, rows: 24 }) + ).rejects.toBeInstanceOf(TerminalSessionOwnerUnverifiedError) + expect(current.spawn).not.toHaveBeenCalled() +}) + it('rejects completion inspection instead of borrowing the fallback provider', async () => { const provider = new DegradedDaemonPtyProvider({ current: createDaemonAdapter('daemon'), @@ -368,7 +410,8 @@ describe('DegradedDaemonPtyProvider', () => { }) it('probes daemon owners without borrowing fallback liveness', async () => { - const current = createDaemonAdapter('current') + const currentSessions: string[] = [] + const current = createDaemonAdapter('current', currentSessions) const legacy = createDaemonAdapter('legacy') const fallback = createProvider('fallback', ['unknown-session']) const provider = new DegradedDaemonPtyProvider({ current, legacy: [legacy], fallback }) @@ -377,7 +420,7 @@ describe('DegradedDaemonPtyProvider', () => { await expect(provider.probePtyLiveness('unknown-session')).resolves.toBeNull() expect(fallback.probePtyLiveness).not.toHaveBeenCalled() - vi.mocked(current.probePtyLiveness).mockResolvedValue(true) + currentSessions.push('unknown-session') await expect(provider.probePtyLiveness('unknown-session')).resolves.toBe(true) }) @@ -529,6 +572,6 @@ describe('DegradedDaemonPtyProvider', () => { await expect(provider.listProcesses()).rejects.toThrow('legacy exited') expect(provider.getLegacyAdapters()).toEqual([legacy]) expect(current.listProcesses).toHaveBeenCalledTimes(3) - expect(fallback.listProcesses).toHaveBeenCalledTimes(2) + expect(fallback.listProcesses).toHaveBeenCalledTimes(3) }) }) diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index 4fef029f0..6b1c738b3 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -14,12 +14,11 @@ import type { import { adoptOwningProvider, attachDaemonOwnedSession, - discoverDegradedDaemonSessions, findDaemonAdapter, listProviderSessionIds } from './degraded-daemon-session-routing' -import { probePtyOwners } from './daemon-pty-liveness-probe' import { DegradedDaemonFreshSpawnRouter } from './degraded-daemon-fresh-spawn-routing' +import { DegradedDaemonOwnerRecovery } from './degraded-daemon-owner-recovery' export class DegradedDaemonPtyProvider implements IPtyProvider { readonly isDegraded = true @@ -29,6 +28,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { private fallback: IPtyProvider private sessionProviders = new Map() private freshSpawns: DegradedDaemonFreshSpawnRouter + private ownerRecovery: DegradedDaemonOwnerRecovery private unsubscribers: (() => void)[] = [] private dataListeners: ((payload: PtyDataEvent) => void)[] = [] private exitListeners: ((payload: { id: string; code: number }) => void)[] = [] @@ -48,26 +48,27 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { this.sessionProviders, opts.probeCurrentDaemonSpawn ?? null ) + this.ownerRecovery = new DegradedDaemonOwnerRecovery( + this.allProviders(), + this.allDaemonAdapters(), + this.sessionProviders, + (spawnOpts) => this.freshSpawns.spawn(spawnOpts) + ) for (const provider of this.allProviders()) { this.unsubscribers.push( - provider.onData((payload) => { - for (const listener of this.dataListeners) { - listener(payload) - } - }), + provider.onData((payload) => this.dataListeners.forEach((listener) => listener(payload))), provider.onExit((payload) => { - this.sessionProviders.delete(payload.id) - for (const listener of this.exitListeners) { - listener(payload) - } + this.ownerRecovery.forgetRoute(payload.id) + this.exitListeners.forEach((listener) => listener(payload)) }) ) } + this.unsubscribers.push(...this.ownerRecovery.subscribeIdentityChanges()) } - discoverDaemonSessions(): Promise { - return discoverDegradedDaemonSessions(this.allDaemonAdapters(), this.sessionProviders) + async discoverDaemonSessions(): Promise { + await this.ownerRecovery.discoverRoutes() } get routesFreshSpawnsToLocalProvider(): true | undefined { @@ -82,7 +83,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { canProvideAuthoritativeBufferSnapshot = (id: string): boolean => this.freshSpawns.canProvideSnapshot(id) - spawn = (opts: PtySpawnOptions): Promise => this.freshSpawns.spawn(opts) + spawn = (opts: PtySpawnOptions): Promise => this.ownerRecovery.spawn(opts) // Why refuse the fallback route (unknown ids resolve to it): see attachDaemonOwnedSession. attach = (id: string): Promise => @@ -94,7 +95,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } async probePtyLiveness(id: string): Promise { - return await probePtyOwners(id, this.sessionProviders.get(id), this.allDaemonAdapters()) + return await this.ownerRecovery.probe(id) } // Why: an unknown id cannot borrow listing authority from the fresh-spawn provider. @@ -275,20 +276,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { alive: string[] killed: string[] }> { - const alive: string[] = [] - const killed: string[] = [] - for (const adapter of this.allDaemonAdapters()) { - const result = await adapter.reconcileOnStartup(validWorktreeIds) - for (const id of result.alive) { - alive.push(id) - this.sessionProviders.set(id, adapter) - } - for (const id of result.killed) { - killed.push(id) - this.sessionProviders.delete(id) - } - } - return { alive, killed } + return await this.ownerRecovery.reconcileOnStartup(validWorktreeIds) } dispose(): void { diff --git a/src/main/daemon/degraded-daemon-session-routing.ts b/src/main/daemon/degraded-daemon-session-routing.ts index 8f0ef61d2..688098c05 100644 --- a/src/main/daemon/degraded-daemon-session-routing.ts +++ b/src/main/daemon/degraded-daemon-session-routing.ts @@ -2,21 +2,6 @@ import type { IPtyProvider } from '../providers/types' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import { SessionNotFoundError } from './daemon-errors' -export async function discoverDegradedDaemonSessions( - adapters: readonly DaemonPtyAdapter[], - sessionProviders: Map -): Promise { - for (const adapter of adapters) { - try { - for (const session of await adapter.listProcesses()) { - sessionProviders.set(session.id, adapter) - } - } catch (error) { - console.warn('[daemon] Failed to discover degraded daemon sessions', error) - } - } -} - export function listProviderSessionIds( sessionProviders: ReadonlyMap, provider: IPtyProvider diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index c91c7d23d..877d7d886 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -14,6 +14,7 @@ import type { TuiAgent } from '../../shared/types' import type { AgentSessionOwnerBinding } from '../../shared/agent-session-host-authority' import { AGENT_SESSION_CLAIM_DIGEST_VERSION } from '../../shared/agent-session-host-authority' import { PtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' +import { TerminalSessionOwnerUnverifiedError } from '../daemon/daemon-errors' const isWindowsHost = process.platform === 'win32' const posixOnlyIt = isWindowsHost ? it.skip : it @@ -9009,14 +9010,15 @@ describe('registerPtyHandlers', () => { } ) - it('adopts an exact persisted owner when the runtime projection is missing', async () => { + it('repairs a stale persisted incarnation after exact same-id reattach', async () => { const tabId = 'tab-persisted-owner' const leafId = '88888888-8888-4888-8888-888888888888' const paneKey = makePaneKey(tabId, leafId) const worktreeId = 'repo-1::/tmp/persisted-owner' + let attachAttempt = 0 const providerSpawn = vi.fn(async (options: { attachOnly?: boolean; sessionId?: string }) => ({ id: options.sessionId ?? 'unexpected-fresh-id', - incarnationId: 'inc-persisted-owner', + incarnationId: attachAttempt++ === 1 ? 'inc-wrong-owner' : 'inc-live-owner', isReattach: options.attachOnly === true, snapshot: 'persisted-owner-output' })) @@ -9067,10 +9069,10 @@ describe('registerPtyHandlers', () => { [tabId]: { ptyIdsByLeafId: { [leafId]: 'pty-persisted-owner' } } }, terminalPtyIncarnationsByPaneKey: { - [paneKey]: 'inc-persisted-owner' + [paneKey]: 'inc-stale-owner' } })), - persistPtyBinding: vi.fn() + persistPtyBinding: vi.fn(() => true) } registerPtyHandlers( @@ -9081,7 +9083,7 @@ describe('registerPtyHandlers', () => { undefined, store as never ) - const mounted = await handlers.get('pty:spawn')!(null, { + const spawnArgs = { cols: 80, rows: 24, cwd: '/tmp/persisted-owner', @@ -9094,18 +9096,22 @@ describe('registerPtyHandlers', () => { ORCA_TAB_ID: tabId, ORCA_WORKTREE_ID: worktreeId } - }) + } + + const mounted = await handlers.get('pty:spawn')!(null, spawnArgs) expect(mounted).toMatchObject({ id: 'pty-persisted-owner', - incarnationId: 'inc-persisted-owner', + incarnationId: 'inc-live-owner', isReattach: true }) - expect(providerSpawn).toHaveBeenCalledOnce() - expect(providerSpawn).toHaveBeenCalledWith( + expect(providerSpawn).toHaveBeenNthCalledWith( + 1, expect.objectContaining({ attachOnly: true, sessionId: 'pty-persisted-owner', + expectedIncarnationId: 'inc-stale-owner', + expectedIncarnationIsAuthoritative: false, command: undefined }) ) @@ -9115,10 +9121,50 @@ describe('registerPtyHandlers', () => { ) expect(runtime.noteTerminalSpawnCommand).not.toHaveBeenCalled() expect(store.persistPtyBinding).toHaveBeenCalledOnce() + expect(store.persistPtyBinding).toHaveBeenCalledWith( + expect.objectContaining({ + worktreeId, + tabId, + leafId, + ptyId: 'pty-persisted-owner', + incarnationId: 'inc-live-owner', + expectedBinding: { + ptyId: 'pty-persisted-owner', + incarnationId: 'inc-stale-owner' + } + }), + undefined + ) expect( mainWindow.webContents.send.mock.calls.filter(([channel]) => channel === 'pty:spawned') ).toHaveLength(1) expect(runtime.onPtyExit).not.toHaveBeenCalled() + + store.persistPtyBinding.mockClear() + await expect(handlers.get('pty:spawn')!(null, spawnArgs)).rejects.toThrow( + 'terminal_pane_owner_changed' + ) + expect(store.persistPtyBinding).not.toHaveBeenCalled() + + runtime.assertPtyRegistrationAllowed.mockImplementationOnce(() => { + throw new Error('agent_session_exited_during_start') + }) + await expect(handlers.get('pty:spawn')!(null, spawnArgs)).rejects.toThrow( + 'agent_session_exited_during_start' + ) + expect(store.persistPtyBinding).not.toHaveBeenCalled() + expect(providerSpawn).toHaveBeenCalledTimes(3) + expect(providerSpawn).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + attachOnly: true, + sessionId: 'pty-persisted-owner', + expectedIncarnationId: 'inc-live-owner', + expectedIncarnationIsAuthoritative: true, + command: undefined + }) + ) + clearProviderPtyState('pty-persisted-owner') }) it.each([ @@ -9295,29 +9341,22 @@ describe('registerPtyHandlers', () => { } ) - it.each([ - { label: 'another owner reports it alive', liveness: true }, - { label: 'no owner could answer', liveness: null } - ])('keeps a persisted owner whose absence is unproven ($label)', async ({ liveness }) => { + it('keeps a persisted owner when daemon routing is unresolved', async () => { const worktreeId = 'repo-1::/tmp/unproven-owner' const cwd = '/tmp/unproven-owner' const tabId = 'tab-unproven-owner' const leafId = '56565656-5656-4656-8656-565656565656' const paneKey = makePaneKey(tabId, leafId) - // Why: a degraded router answers unmapped ids from the local fallback, which never - // owned this daemon session — the same "Session not found" a truly dead PTY yields. const providerSpawn = vi.fn( async (options: { attachOnly?: boolean; command?: string; sessionId?: string }) => { if (options.attachOnly) { - throw new Error('Session not found: pty-unproven-owner') + throw new TerminalSessionOwnerUnverifiedError('pty-unproven-owner') } return { id: 'pty-fresh-unproven', incarnationId: 'inc-fresh-unproven' } } ) - const probePtyLiveness = vi.fn(async () => liveness) setLocalPtyProvider({ spawn: providerSpawn, - probePtyLiveness, write: vi.fn(), resize: vi.fn(), kill: vi.fn(), @@ -9410,7 +9449,6 @@ describe('registerPtyHandlers', () => { }) ).rejects.toThrow('terminal_pane_owner_unverified') - expect(probePtyLiveness).toHaveBeenCalledWith('pty-unproven-owner') // The live PTY keeps its pane binding, gets no synthetic exit, and is not duplicated. expect(providerSpawn).toHaveBeenCalledOnce() expect(providerSpawn.mock.calls[0]?.[0]).toMatchObject({ attachOnly: true }) @@ -9420,11 +9458,7 @@ describe('registerPtyHandlers', () => { expect(session.tabsByWorktree[worktreeId]).toHaveLength(1) }) - // Why the positive direction needs its own case: the sibling retire tests use providers with - // no `probePtyLiveness`, so they skip this guard entirely. Without this, the guard could be - // strengthened into a permanent veto — no daemon-backed pane could ever recover from a dead - // owner — and every suite would stay green. - it('still retires and respawns when a provider proves the owner is absent', async () => { + it('still retires and respawns when the routed provider confirms absence', async () => { const worktreeId = 'repo-1::/tmp/proven-absent-owner' const cwd = '/tmp/proven-absent-owner' const tabId = 'tab-proven-absent-owner' @@ -9438,10 +9472,8 @@ describe('registerPtyHandlers', () => { return { id: 'pty-fresh-proven', incarnationId: 'inc-fresh-proven' } } ) - const probePtyLiveness = vi.fn(async () => false) setLocalPtyProvider({ spawn: providerSpawn, - probePtyLiveness, write: vi.fn(), resize: vi.fn(), kill: vi.fn(), @@ -9532,8 +9564,6 @@ describe('registerPtyHandlers', () => { } }) - expect(probePtyLiveness).toHaveBeenCalledWith('pty-proven-absent-owner') - // Proven absence is the one answer that authorizes retirement, so recovery must proceed. expect(mounted).toMatchObject({ id: 'pty-fresh-proven' }) expect(providerSpawn).toHaveBeenCalledTimes(2) expect(providerSpawn.mock.calls[1]?.[0]).toMatchObject({ @@ -9548,6 +9578,124 @@ describe('registerPtyHandlers', () => { expect(store.flushOrThrow).toHaveBeenCalledOnce() }) + it('does not poll after the routed provider confirms absence', async () => { + const worktreeId = 'repo-1::/tmp/probe-blip-owner' + const cwd = '/tmp/probe-blip-owner' + const tabId = 'tab-probe-blip-owner' + const leafId = '67676767-6767-4767-8767-676767676767' + const paneKey = makePaneKey(tabId, leafId) + const providerSpawn = vi.fn( + async (options: { attachOnly?: boolean; command?: string; sessionId?: string }) => { + if (options.attachOnly) { + throw new Error('Session not found: pty-probe-blip-owner') + } + return { id: 'pty-fresh-probe-blip', incarnationId: 'inc-fresh-probe-blip' } + } + ) + const probePtyLiveness = vi.fn(async () => null) + setLocalPtyProvider({ + spawn: providerSpawn, + probePtyLiveness, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown: vi.fn(), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}), + listProcesses: vi.fn(async () => []), + attach: vi.fn(), + getDefaultShell: vi.fn(), + getProfiles: vi.fn() + } as never) + let session = { + tabsByWorktree: { + [worktreeId]: [{ id: tabId, worktreeId, ptyId: 'pty-probe-blip-owner' }] + }, + terminalLayoutsByTabId: { + [tabId]: { + root: { type: 'leaf' as const, leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: 'pty-probe-blip-owner' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-probe-blip-owner' } + } + const store = { + getWorkspaceSession: vi.fn(() => session), + setWorkspaceSession: vi.fn((next) => { + session = next + }), + flushOrThrow: vi.fn(), + persistPtyBinding: vi.fn(), + getFolderWorkspace: vi.fn(() => undefined), + getFolderWorkspaces: vi.fn(() => []), + getProjectGroups: vi.fn(() => []), + getRepos: vi.fn(() => []) + } + const runtime = { + setPtyController: vi.fn(), + resolveTerminalPane: vi.fn(() => { + throw new Error('terminal_not_found') + }), + createPreAllocatedTerminalHandle: vi.fn(() => 'term-probe-blip'), + preAllocateHandleForPty: vi.fn(() => 'term-probe-blip'), + registerPreAllocatedHandleForPty: vi.fn(), + beginPtyRegistration: vi.fn(), + cancelPendingPtyRegistration: vi.fn(), + assertPtyRegistrationAllowed: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + seedHeadlessTerminal: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn() + } + + registerPtyHandlers( + mainWindow as never, + runtime as never, + undefined, + undefined, + undefined, + store as never + ) + + const mounted = await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd, + command: 'codex resume probe-blip-session', + worktreeId, + tabId, + leafId, + env: { + ORCA_PANE_KEY: paneKey, + ORCA_TAB_ID: tabId, + ORCA_WORKTREE_ID: worktreeId + } + }) + + expect(probePtyLiveness).not.toHaveBeenCalled() + expect(mounted).toMatchObject({ id: 'pty-fresh-probe-blip' }) + expect(providerSpawn).toHaveBeenCalledTimes(2) + expect(runtime.onPtyExit).toHaveBeenCalledWith( + 'pty-probe-blip-owner', + 0, + 'inc-probe-blip-owner' + ) + }) + // Why: a parked pane (stopped with keepHistory) leaves the runtime holding the binding while // persistence has already dropped it. Reading "nothing left to retire" as a competing owner // aborted materialization *after* signalling the exit, which destroyed the pane instead of @@ -9676,7 +9824,7 @@ describe('registerPtyHandlers', () => { } }) - expect(probePtyLiveness).toHaveBeenCalledWith('pty-already-retired-owner') + expect(probePtyLiveness).not.toHaveBeenCalled() expect(mounted).toMatchObject({ id: 'pty-fresh-already-retired' }) expect(providerSpawn).toHaveBeenCalledTimes(2) expect(providerSpawn.mock.calls[1]?.[0]).toMatchObject({ diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index e4f50a625..5d858cac7 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -93,6 +93,7 @@ import { import { resolveWslSessionContext } from '../daemon/wsl-session-context' import { addNodePtyRecoveryHint } from '../daemon/node-pty-error-hints' import { recordDaemonStreamBacklogEvent } from '../daemon/daemon-stream-backlog-probe' +import { TerminalSessionOwnerUnverifiedError } from '../daemon/daemon-errors' import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service' import type { ClaudeAccountSelectionTarget } from '../claude-accounts/runtime-selection' import { CLAUDE_AUTH_ENV_VARS, hasClaudeAuthEnvConflict } from '../claude-accounts/environment' @@ -599,6 +600,9 @@ type StablePaneOwner = { leafId: string ptyId: string incarnationId?: string + hasPersistedBinding?: true + persistedIncarnationId?: string + runtimeIncarnationId?: string } type StablePaneAdoption = { result: PtySpawnResult @@ -677,13 +681,6 @@ function resolveStablePaneOwner( throw new Error('terminal_pane_owner_host_mismatch') } const runtimeIncarnationId = ptyIncarnationById.get(ptyId) - if ( - runtimeIncarnationId && - persisted?.incarnationId && - runtimeIncarnationId !== persisted.incarnationId - ) { - throw new Error('terminal_pane_owner_conflict') - } const parsed = parsePaneKey(paneKey) if (!parsed) { return null @@ -695,7 +692,10 @@ function resolveStablePaneOwner( ptyId, ...(runtimeIncarnationId || persisted?.incarnationId ? { incarnationId: runtimeIncarnationId ?? persisted?.incarnationId } - : {}) + : {}), + ...(persisted ? { hasPersistedBinding: true as const } : {}), + ...(persisted?.incarnationId ? { persistedIncarnationId: persisted.incarnationId } : {}), + ...(runtimeIncarnationId ? { runtimeIncarnationId } : {}) } } @@ -717,7 +717,7 @@ function retirePersistedStablePaneOwner( // not a competing owner. Reporting failure here strands the pane after its PTY is proven dead. return true } - if (current.ptyId !== owner.ptyId || current.incarnationId !== owner.incarnationId) { + if (current.ptyId !== owner.ptyId || current.incarnationId !== owner.persistedIncarnationId) { return false } const session = store.getWorkspaceSession(hostId) @@ -726,7 +726,7 @@ function retirePersistedStablePaneOwner( parentTabId: owner.tabId, leafId: owner.leafId, ptyId: owner.ptyId, - ...(owner.incarnationId ? { incarnationId: owner.incarnationId } : {}) + ...(current.incarnationId ? { incarnationId: current.incarnationId } : {}) }) if (retired === session) { return false @@ -748,6 +748,47 @@ type StablePaneSpawnContext = { onFreshSpawn?: (result: PtySpawnResult) => void } +function stablePanePersistenceFence( + owner: StablePaneOwner | null +): { ptyId: string; incarnationId?: string } | undefined { + return owner?.hasPersistedBinding + ? { + ptyId: owner.ptyId, + ...(owner.persistedIncarnationId ? { incarnationId: owner.persistedIncarnationId } : {}) + } + : undefined +} + +function persistAdmittedStablePaneBinding(args: { + store: Store | undefined + owner: StablePaneOwner | null + result: PtySpawnResult + worktreeId: string | undefined + startupCwd: string | undefined + connectionId: string | null | undefined +}): boolean { + const expectedBinding = stablePanePersistenceFence(args.owner) + if (!args.store || !args.owner || !args.worktreeId || !expectedBinding) { + return false + } + const persisted = args.store.persistPtyBinding( + { + worktreeId: args.worktreeId, + tabId: args.owner.tabId, + leafId: args.owner.leafId, + ptyId: args.result.id, + ...(args.result.incarnationId ? { incarnationId: args.result.incarnationId } : {}), + ...(args.startupCwd ? { startupCwd: args.startupCwd } : {}), + expectedBinding + }, + args.connectionId ? toSshExecutionHostId(args.connectionId) : undefined + ) + if (persisted === false) { + throw new Error('terminal_pane_owner_changed') + } + return true +} + async function attachStablePaneOwner( args: StablePaneSpawnContext & { owner: StablePaneOwner } ): Promise<{ result: PtySpawnResult; owner: StablePaneOwner } | null> { @@ -758,6 +799,8 @@ async function attachStablePaneOwner( ...spawnOptions, sessionId: owner.ptyId, attachOnly: true, + expectedIncarnationId: owner.runtimeIncarnationId ?? owner.persistedIncarnationId, + expectedIncarnationIsAuthoritative: owner.runtimeIncarnationId !== undefined, isNewSession: undefined, command: undefined, commandDelivery: undefined, @@ -769,25 +812,19 @@ async function attachStablePaneOwner( onPtySpawnCommitted: undefined }) } catch (error) { + if (error instanceof TerminalSessionOwnerUnverifiedError) { + throw new Error('terminal_pane_owner_unverified') + } if (!isPtyAlreadyGoneError(error)) { throw error } - // Why: "Session not found" only proves the provider we asked has no such PTY — and a - // degraded router answers unmapped ids from the local fallback, which never owned a - // daemon session. Retiring on that would signal exit and delete a live agent's pane - // binding. Absence must be proven across every possible owner first; `null` (nobody - // could answer) is not absence. Providers without a probe are their own sole owner, - // so their refusal stays authoritative. - if (provider.probePtyLiveness && (await provider.probePtyLiveness(owner.ptyId)) !== false) { - throw new Error('terminal_pane_owner_unverified') - } const ownerBeforeRetire = args.resolveOwner?.() if ( ownerBeforeRetire && (ownerBeforeRetire.ptyId !== owner.ptyId || - (ownerBeforeRetire.incarnationId !== undefined && - owner.incarnationId !== undefined && - ownerBeforeRetire.incarnationId !== owner.incarnationId)) + ownerBeforeRetire.runtimeIncarnationId !== owner.runtimeIncarnationId || + ownerBeforeRetire.hasPersistedBinding !== owner.hasPersistedBinding || + ownerBeforeRetire.persistedIncarnationId !== owner.persistedIncarnationId) ) { throw new Error('terminal_pane_owner_changed') } @@ -808,7 +845,9 @@ async function attachStablePaneOwner( if ( result.id !== owner.ptyId || result.isReattach !== true || - (owner.incarnationId !== undefined && result.incarnationId !== owner.incarnationId) + (owner.runtimeIncarnationId !== undefined && + result.incarnationId !== owner.runtimeIncarnationId) || + (result.incarnationId === undefined && owner.incarnationId !== undefined) ) { throw new Error('terminal_pane_owner_changed') } @@ -4680,6 +4719,7 @@ export function registerPtyHandlers( : null let result: PtySpawnResult let stablePaneOwner: StablePaneOwner | null = null + let stablePaneBindingPersisted = false let rejectedRegistrationCandidate: PtySpawnResult | null = null let pendingRegistrationPtyId: string | null = null let preparedProvisionalExecutionContext = false @@ -4902,6 +4942,24 @@ export function registerPtyHandlers( trustedTerminalHandleEnv.delete(args.preAllocatedHandle) } } + try { + stablePaneBindingPersisted = persistAdmittedStablePaneBinding({ + store: hostSessionBinding?.store, + owner: stablePaneOwner, + result, + worktreeId: hostSessionBinding?.worktreeId, + startupCwd: cwd, + connectionId: args.connectionId + }) + } catch (error) { + if (error instanceof Error && error.message === 'terminal_pane_owner_changed') { + throw error + } + console.error('[pty] failed to persist runtime PTY binding after attach:', error) + throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage()), { + agentSessionOperationOutcome: 'unknown' as const + }) + } if (result.agentSessionEnsure?.disposition === 'adopted') { const owner = result.agentSessionEnsure.owner ptyOwnership.set(result.id, args.connectionId ?? ptyOwnership.get(result.id) ?? null) @@ -4969,7 +5027,7 @@ export function registerPtyHandlers( target: codexSelectionTarget, settings: getSettings?.() }) - if (hostSessionBinding) { + if (hostSessionBinding && !stablePaneBindingPersisted) { try { const binding = { worktreeId: hostSessionBinding.worktreeId, @@ -5625,6 +5683,7 @@ export function registerPtyHandlers( let finishTerminalInstall = (): void => {} let result: PtySpawnResult let stablePaneOwner: StablePaneOwner | null = null + let stablePaneBindingPersisted = false let rejectedRegistrationCandidate: PtySpawnResult | null = null let pendingRegistrationPtyId: string | null = null let preparedProvisionalExecutionContext = false @@ -6245,6 +6304,24 @@ export function registerPtyHandlers( trustedTerminalHandleEnv.delete(preAllocatedHandle) } } + try { + stablePaneBindingPersisted = persistAdmittedStablePaneBinding({ + store, + owner: stablePaneOwner, + result, + worktreeId: args.worktreeId, + startupCwd: cwd, + connectionId: args.connectionId + }) + } catch (error) { + if (error instanceof Error && error.message === 'terminal_pane_owner_changed') { + throw error + } + console.error('[pty] failed to persist PTY binding after attach:', error) + throw Object.assign(new Error(createTerminalSessionStateSaveFailureMessage()), { + agentSessionOperationOutcome: 'unknown' as const + }) + } spawnTiming.log(result.id, { daemon: isDaemonHostSpawn, reattach: result.isReattach ?? false @@ -6303,7 +6380,8 @@ export function registerPtyHandlers( store && typeof args.worktreeId === 'string' && typeof args.tabId === 'string' && - validatedLeafId !== null + validatedLeafId !== null && + !stablePaneBindingPersisted ) { try { const binding = { diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 24cc6e512..0a48bbbb3 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -9033,6 +9033,221 @@ describe('Store', () => { ).toBeUndefined() }) + it('reconciles only the incarnation of an unchanged durable PTY binding', async () => { + const store = await createStore() + const paneKey = `tab1:${TEST_LEAF_1}` + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-1' })] + }, + terminalLayoutsByTabId: { + tab1: { + root: { type: 'leaf', leafId: TEST_LEAF_1 }, + activeLeafId: TEST_LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-1' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-stale' } + }) + + expect( + store.persistPtyBinding({ + worktreeId: 'wt1', + tabId: 'tab1', + leafId: TEST_LEAF_1, + ptyId: 'pty-1', + incarnationId: 'inc-live', + expectedBinding: { ptyId: 'pty-1', incarnationId: 'inc-stale' } + }) + ).toBe(true) + + expect(store.getWorkspaceSession().terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe('inc-live') + const reloaded = await createStore() + expect(reloaded.getWorkspaceSession().terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-live' + ) + }) + + it('rejects competing PTY and incarnation changes during reconciliation', async () => { + const store = await createStore() + const paneKey = `tab1:${TEST_LEAF_1}` + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-current' })] + }, + terminalLayoutsByTabId: { + tab1: { + root: { type: 'leaf', leafId: TEST_LEAF_1 }, + activeLeafId: TEST_LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-current' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-current' } + }) + + for (const competing of [ + { ptyId: 'pty-replaced', expectedIncarnationId: 'inc-current' }, + { ptyId: 'pty-current', expectedIncarnationId: 'inc-replaced' } + ]) { + expect( + store.persistPtyBinding({ + worktreeId: 'wt1', + tabId: 'tab1', + leafId: TEST_LEAF_1, + ptyId: competing.ptyId, + incarnationId: 'inc-live', + expectedBinding: { + ptyId: competing.ptyId, + incarnationId: competing.expectedIncarnationId + } + }) + ).toBe(false) + } + expect(store.getWorkspaceSession().terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-current' + ) + }) + + it('reconciles only the requested execution-host partition', async () => { + const store = await createStore() + const paneKey = `tab1:${TEST_LEAF_1}` + const session = { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-1' })] + }, + terminalLayoutsByTabId: { + tab1: { + root: { type: 'leaf' as const, leafId: TEST_LEAF_1 }, + activeLeafId: TEST_LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-1' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-stale' } + } + store.setWorkspaceSession(structuredClone(session), 'local') + store.setWorkspaceSession(structuredClone(session), 'ssh:ssh-1') + + expect( + store.persistPtyBinding( + { + worktreeId: 'wt1', + tabId: 'tab1', + leafId: TEST_LEAF_1, + ptyId: 'pty-1', + incarnationId: 'inc-remote-live', + expectedBinding: { ptyId: 'pty-1', incarnationId: 'inc-stale' } + }, + 'ssh:ssh-1' + ) + ).toBe(true) + + expect(store.getWorkspaceSession('local').terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-stale' + ) + expect(store.getWorkspaceSession('ssh:ssh-1').terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-remote-live' + ) + }) + + it.each([ + { label: 'local', hostId: undefined }, + { label: 'SSH', hostId: 'ssh:ssh-1' } + ])( + 'preserves a reconciled incarnation across a renderer snapshot ($label)', + async ({ hostId }) => { + const store = await createStore() + const paneKey = `tab1:${TEST_LEAF_1}` + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-1' })] + }, + terminalLayoutsByTabId: { + tab1: { + root: { type: 'leaf', leafId: TEST_LEAF_1 }, + activeLeafId: TEST_LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-1' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-stale' } + }, + hostId + ) + + expect( + store.persistPtyBinding( + { + worktreeId: 'wt1', + tabId: 'tab1', + leafId: TEST_LEAF_1, + ptyId: 'pty-1', + incarnationId: 'inc-live', + expectedBinding: { ptyId: 'pty-1', incarnationId: 'inc-stale' } + }, + hostId + ) + ).toBe(true) + + const rendererSnapshot = structuredClone(store.getWorkspaceSession(hostId)) + delete rendererSnapshot.terminalPtyIncarnationsByPaneKey + delete rendererSnapshot.terminalTopologyRevisionByRepoId + store.setWorkspaceSession(rendererSnapshot, hostId) + + expect(store.getWorkspaceSession(hostId).terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-live' + ) + const reloaded = await createStore() + expect(reloaded.getWorkspaceSession(hostId).terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-live' + ) + } + ) + + it('rolls back incarnation reconciliation when the durability barrier fails', async () => { + const store = await createStore() + const paneKey = `tab1:${TEST_LEAF_1}` + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + wt1: [makeTerminalTab({ id: 'tab1', worktreeId: 'wt1', ptyId: 'pty-1' })] + }, + terminalLayoutsByTabId: { + tab1: { + root: { type: 'leaf', leafId: TEST_LEAF_1 }, + activeLeafId: TEST_LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [TEST_LEAF_1]: 'pty-1' } + } + }, + terminalPtyIncarnationsByPaneKey: { [paneKey]: 'inc-stale' } + }) + vi.spyOn(store, 'flushOrThrow').mockImplementationOnce(() => { + throw new Error('disk full') + }) + + expect(() => + store.persistPtyBinding({ + worktreeId: 'wt1', + tabId: 'tab1', + leafId: TEST_LEAF_1, + ptyId: 'pty-1', + incarnationId: 'inc-live', + expectedBinding: { ptyId: 'pty-1', incarnationId: 'inc-stale' } + }) + ).toThrow('disk full') + expect(store.getWorkspaceSession().terminalPtyIncarnationsByPaneKey?.[paneKey]).toBe( + 'inc-stale' + ) + }) + it('adds a missing split leaf to the durable root when a new pane spawns before layout debounce', async () => { const store = await createStore() store.setWorkspaceSession({ diff --git a/src/main/persistence.ts b/src/main/persistence.ts index af82bcbdd..be3f10fd7 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -6621,11 +6621,26 @@ export class Store { ptyId: string incarnationId?: string startupCwd?: string + expectedBinding?: { ptyId: string; incarnationId?: string } }, hostId?: string | null - ): void { + ): boolean { const resolvedHostId = this.resolveHostId(hostId) const session = this.getWorkspaceSession(resolvedHostId) + const paneKey = `${args.tabId}:${args.leafId}` + if (args.expectedBinding) { + const tab = session.tabsByWorktree?.[args.worktreeId]?.find( + (candidate) => candidate.id === args.tabId && candidate.worktreeId === args.worktreeId + ) + const boundPtyId = session.terminalLayoutsByTabId?.[args.tabId]?.ptyIdsByLeafId?.[args.leafId] + if ( + !tab || + boundPtyId !== args.expectedBinding.ptyId || + session.terminalPtyIncarnationsByPaneKey?.[paneKey] !== args.expectedBinding.incarnationId + ) { + return false + } + } if (resolvedHostId !== LOCAL_EXECUTION_HOST_ID) { this.state.workspaceSessionsByHostId = { ...this.state.workspaceSessionsByHostId, @@ -6633,15 +6648,17 @@ export class Store { } } const sessionBeforeBinding = cloneWorkspaceSessionState(session) - const paneKey = `${args.tabId}:${args.leafId}` + const reconciledIncarnation = + args.expectedBinding !== undefined && + args.incarnationId !== args.expectedBinding.incarnationId let terminalMembershipChanged = false - const advanceTopologyAfterMembershipChange = (): void => { + const advanceTopologyFence = (): void => { const repoId = getRepoIdFromWorktreeId(args.worktreeId) const currentRevision = session.terminalTopologyRevisionByRepoId?.[repoId] ?? 0 - if (!terminalMembershipChanged || currentRevision <= 0) { + if (!reconciledIncarnation && (!terminalMembershipChanged || currentRevision <= 0)) { return } - // Why: a real host-admitted spawn after a retirement must be distinguishable from a stale renderer replay. + // Why: host-admitted membership or incarnation changes must outrank a stale renderer replay. session.terminalTopologyRevisionByRepoId = { ...session.terminalTopologyRevisionByRepoId, [repoId]: currentRevision + 1 @@ -6696,14 +6713,14 @@ export class Store { } if (!isTerminalLeafId(args.leafId)) { // Why: keep legacy renderer-local pane ids out of durable leaf-keyed layout state after the UUID migration. - advanceTopologyAfterMembershipChange() + advanceTopologyFence() try { this.flushOrThrow() } catch (err) { restoreSession() throw err } - return + return true } const layout = session.terminalLayoutsByTabId?.[args.tabId] if (layout) { @@ -6744,13 +6761,14 @@ export class Store { } } } - advanceTopologyAfterMembershipChange() + advanceTopologyFence() try { this.flushOrThrow() } catch (err) { restoreSession() throw err } + return true } // ── SSH Targets ──────────────────────────────────────────────────── diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index d43158482..64dd98509 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -38,6 +38,7 @@ import type { ShellReadySignal } from './local-pty-shell-ready' import { removeInheritedNoColor } from '../pty/terminal-color-env' import { removeAppImageRuntimeEnv } from '../pty/appimage-terminal-env' import { stripInheritedBuildModeEnv } from '../pty/build-mode-env' +import { SessionNotFoundError } from '../daemon/daemon-errors' import { resolvePathEnvKey } from '../pty/windows-environment-path' import { isHostCodexHomeForWsl, isWslCodexHomeForHost } from '../pty/codex-home-wsl-env' import { addWslEnvKeys } from '../wsl-env' @@ -546,7 +547,7 @@ export class LocalPtyProvider implements IPtyProvider { } } if (args.attachOnly) { - throw new Error(`Session not found: ${args.sessionId ?? ''}`) + throw new SessionNotFoundError(args.sessionId ?? '') } const id = allocatePtyId(reattachId ?? undefined) const incarnationId = randomUUID() diff --git a/src/main/providers/pty-provider-contract.ts b/src/main/providers/pty-provider-contract.ts index b766b1149..98ab3d9b6 100644 --- a/src/main/providers/pty-provider-contract.ts +++ b/src/main/providers/pty-provider-contract.ts @@ -64,6 +64,10 @@ export type PtySpawnOptions = { isNewSession?: boolean /** Attach the named session atomically or fail without creating a process. */ attachOnly?: boolean + /** Exact persisted owner expected by an attach-only routing decision. */ + expectedIncarnationId?: PtyIncarnationId + /** True when runtime state makes the expected incarnation a hard attach fence. */ + expectedIncarnationIsAuthoritative?: boolean /** Why: allows the renderer to request a specific shell for a single new * terminal tab (e.g. "open this tab in WSL" from the "+" submenu) without * changing the user's persistent default shell setting. Only consulted on diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts index 56e7ffd0a..bd9aedd37 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { + humanizeTerminalError, isSshReconnectOwnedTerminalError, shouldOfferDaemonRestart, stripSshReconnectOwnedErrorLines @@ -36,6 +37,24 @@ describe('isSshReconnectOwnedTerminalError', () => { }) }) +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') + }) + + it('humanizes an IPC-wrapped pane-owner-unverified error', () => { + const wrapped = + "Error invoking remote method 'pty:spawn': Error: terminal_pane_owner_unverified" + expect(humanizeTerminalError(wrapped)).not.toContain('terminal_pane_owner_unverified') + }) + + it('leaves other errors untouched', () => { + expect(humanizeTerminalError('Paste failed.')).toBe('Paste failed.') + }) +}) + describe('stripSshReconnectOwnedErrorLines', () => { it('clears an error that is only SSH reconnect text', () => { expect(stripSshReconnectOwnedErrorLines(SSH_FAILURE)).toBeNull() diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index 7f38733fa..99a00e54f 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -12,6 +12,8 @@ const STALE_DAEMON_CWD_MARKERS = [ "Daemon's working directory is gone", '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' function isSshError(error: string): boolean { return error.startsWith(SSH_PREFIX) || error.includes(SSH_RELAY_LOST_MARKER) @@ -42,6 +44,20 @@ export function shouldOfferDaemonRestart(error: string): boolean { ) } +/** Swaps the raw pane-owner-unverified code for copy a user can act on. */ +export function humanizeTerminalError(error: string): string { + if (!error.includes(PANE_OWNER_UNVERIFIED_MARKER)) { + return error + } + return error.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." + ) + ) +} + export function TerminalErrorToast({ error, onDismiss, @@ -53,6 +69,7 @@ export function TerminalErrorToast({ }): React.JSX.Element { const ssh = isSshError(error) const showDaemonRestart = !ssh && onRestartDaemon && shouldOfferDaemonRestart(error) + const displayError = humanizeTerminalError(error) return (
- {error} + {displayError} {showDaemonRestart ? ( <> {'\n'} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 3f9400767..3792f6915 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -2801,7 +2801,8 @@ "e4aa243f8c": "Restart daemon", "a7e2fd2699": "file an issue", "5c8ce20be6": "If this persists, please", - "cc6d997c65": "Restart the terminal daemon from here to clear stale daemon state." + "cc6d997c65": "Restart the terminal daemon from here to clear stale daemon state.", + "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." }, "TerminalPane": { "ac112e9036": "Remove title",