From d71e1981e05bf3c8237a12332ef0e0bc8d8ca7e5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:19:27 -0700 Subject: [PATCH] fix(daemon): a terminal the old daemon still runs is never handed to the new daemon after an update (#29418) * fix(daemon): a daemon update never hands a live old-daemon session id to the new daemon Refs #26296 * fix(daemon): reconnect an older daemon through its inventory before refusing a spawn * fix(daemon): reattach the current daemon's own session before consulting older daemons * fix(daemon): reconnect the current daemon too before an older daemon can refuse its session --- ...on-pty-adapter-subscription-fanout.test.ts | 2 +- .../daemon-pty-adapter-subscription-fanout.ts | 4 +- .../daemon-pty-router-legacy-owner.test.ts | 97 +++++++++++++++++++ src/main/daemon/daemon-pty-router.test.ts | 4 +- src/main/daemon/daemon-pty-router.ts | 10 +- src/main/daemon/daemon-router-legacy-owner.ts | 62 ++++++++++++ 6 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 src/main/daemon/daemon-pty-router-legacy-owner.test.ts create mode 100644 src/main/daemon/daemon-router-legacy-owner.ts diff --git a/src/main/daemon/daemon-pty-adapter-subscription-fanout.test.ts b/src/main/daemon/daemon-pty-adapter-subscription-fanout.test.ts index e9348324f6b..45c68370c80 100644 --- a/src/main/daemon/daemon-pty-adapter-subscription-fanout.test.ts +++ b/src/main/daemon/daemon-pty-adapter-subscription-fanout.test.ts @@ -69,7 +69,7 @@ describe('DaemonPtyAdapterSubscriptionFanout.dropAdapter', () => { expect(data).toEqual(['before', 'kept']) expect(exits).toEqual(['live']) expect(onAdapterExit).toHaveBeenCalledTimes(1) - expect(onAdapterExit).toHaveBeenCalledWith('live') + expect(onAdapterExit).toHaveBeenCalledWith('live', kept.adapter) fanout.dispose() expect(dropped.unsubscribed()).toBe(2) diff --git a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts index ed99103e394..59da6912e0a 100644 --- a/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts +++ b/src/main/daemon/daemon-pty-adapter-subscription-fanout.ts @@ -13,7 +13,7 @@ export class DaemonPtyAdapterSubscriptionFanout { constructor( adapters: readonly DaemonPtyAdapter[], - onAdapterExit: (id: string) => void, + onAdapterExit: (id: string, adapter: DaemonPtyAdapter) => void, onAdapterIdentityChanged?: (adapter: DaemonPtyAdapter) => void ) { this.adapters = [...adapters] @@ -25,7 +25,7 @@ export class DaemonPtyAdapterSubscriptionFanout { } }), adapter.onExit((payload) => { - onAdapterExit(payload.id) + onAdapterExit(payload.id, adapter) for (const listener of this.exitListeners) { listener(payload) } diff --git a/src/main/daemon/daemon-pty-router-legacy-owner.test.ts b/src/main/daemon/daemon-pty-router-legacy-owner.test.ts new file mode 100644 index 00000000000..2becdce72bc --- /dev/null +++ b/src/main/daemon/daemon-pty-router-legacy-owner.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAdapter } from './daemon-pty-router-test-fixture' +import { DaemonPtyRouter } from './daemon-pty-router' +import { TerminalSessionOwnerUnverifiedError } from './daemon-errors' + +const SPAWN = { cols: 80, rows: 24 } + +// #26296: after a daemon update, the old daemon keeps the sessions it runs; the new one takes only new ids. +describe('DaemonPtyRouter keeps live sessions on their older daemon', () => { + it('reattaches on the older daemon when startup discovery could not route the id', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['claude-session']) + vi.mocked(legacy.listProcesses).mockRejectedValueOnce(new Error('slow inventory')) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + await router.spawn({ ...SPAWN, sessionId: 'claude-session' }) + + expect(legacy.spawn).toHaveBeenCalledOnce() + expect(current.spawn).not.toHaveBeenCalled() + await router.shutdown('claude-session', { immediate: true }) + expect(legacy.shutdown).toHaveBeenCalledOnce() + expect(current.shutdown).not.toHaveBeenCalled() + }) + + it('keeps the route when another daemon generation reports an exit for the same id', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['claude-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + vi.mocked(legacy.probePtyLiveness).mockResolvedValue(null) + + current.emitExit('claude-session', -1) + router.write('claude-session', 'x') + + expect(legacy.write).toHaveBeenCalledWith('claude-session', 'x') + expect(current.write).not.toHaveBeenCalled() + }) + + it('refuses rather than duplicating the id when the older daemon cannot answer', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['claude-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() + + await expect(router.spawn({ ...SPAWN, sessionId: 'claude-session' })).rejects.toBeInstanceOf( + TerminalSessionOwnerUnverifiedError + ) + expect(current.spawn).not.toHaveBeenCalled() + }) + + it('reconnects through the inventory when the liveness probe cannot reach the older daemon', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['claude-session']) + vi.mocked(legacy.probePtyLiveness).mockResolvedValue(null) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + await router.spawn({ ...SPAWN, sessionId: 'claude-session' }) + await router.spawn({ ...SPAWN, sessionId: 'other-session' }) + + expect(legacy.spawn).toHaveBeenCalledExactlyOnceWith({ ...SPAWN, sessionId: 'claude-session' }) + expect(current.spawn).toHaveBeenCalledExactlyOnceWith({ ...SPAWN, sessionId: 'other-session' }) + }) + + it('reattaches a current-daemon session while an unrelated older daemon cannot answer', async () => { + const current = createAdapter('current', ['current-session']) + const legacy = createAdapter('legacy') + vi.mocked(legacy.listProcesses).mockRejectedValue(new Error('wedged')) + vi.mocked(legacy.probePtyLiveness).mockResolvedValue(null) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + // A dropped current connection answers the probe with null; the inventory reconnects. + vi.mocked(current.probePtyLiveness).mockResolvedValue(null) + + await router.spawn({ ...SPAWN, sessionId: 'current-session' }) + + expect(current.spawn).toHaveBeenCalledOnce() + expect(legacy.spawn).not.toHaveBeenCalled() + }) + + it('creates on the current daemon once the older daemon no longer runs the id', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', []) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + await router.spawn({ ...SPAWN, sessionId: 'exited-session' }) + await router.spawn({ ...SPAWN, sessionId: 'fresh-id', isNewSession: true }) + await router.spawn(SPAWN) + + expect(current.spawn).toHaveBeenCalledTimes(3) + expect(legacy.spawn).not.toHaveBeenCalled() + expect(legacy.probePtyLiveness).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index ce32a447cf7..205555ef3d9 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -463,11 +463,13 @@ describe('DaemonPtyRouter', () => { it('drops a legacy mapping after the routed session exits', async () => { const current = createAdapter('current') - const legacy = createAdapter('legacy', ['legacy-session']) + const legacySessions = ['legacy-session'] + const legacy = createAdapter('legacy', legacySessions) const router = new DaemonPtyRouter({ current, legacy: [legacy] }) await router.discoverLegacySessions() + legacySessions.length = 0 legacy.emitExit('legacy-session', 0) await router.spawn({ sessionId: 'legacy-session', cols: 80, rows: 24 }) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 8e52980bc96..a7a12affa50 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -18,6 +18,7 @@ import { DaemonSessionOwnerResolver } from './daemon-session-owner-resolution' import type { DaemonIdleRetirementResult } from './daemon-pty-runtime-state' import { DaemonRouterRetirement } from './daemon-router-retirement' import { routeSpawnOrHold } from './daemon-router-newer-daemon-hold' +import { findLegacySessionOwner } from './daemon-router-legacy-owner' import type { WriteSettlement } from '../../shared/pty-write-settlement' import type { TerminalOscColorQueryReplyColors } from '../../shared/terminal-osc-color-reply' @@ -36,8 +37,9 @@ export class DaemonPtyRouter implements IPtyProvider { this.ownerResolver = new DaemonSessionOwnerResolver(this.allAdapters(), this.sessionAdapters) this.subscriptions = new DaemonPtyAdapterSubscriptionFanout( this.allAdapters(), - (id) => { - this.ownerResolver.forgetRoute(id) + // Why owner-only: another generation's exit for a reused id must not unroute a live legacy session. + (id, adapter) => { + this.ownerResolver.forgetRoute(id, adapter) }, (adapter) => this.ownerResolver.invalidateProvider(adapter) ) @@ -59,7 +61,9 @@ export class DaemonPtyRouter implements IPtyProvider { 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 adapter = + (opts.sessionId ? this.sessionAdapters.get(opts.sessionId) : undefined) ?? + (await findLegacySessionOwner(opts, this.current, this.legacy, this.legacyExitProof)) const target = await routeSpawnOrHold(opts, adapter, this.current, this.ownerResolver) const result = await target.spawn(opts) // Why: the adapter filters intentional recovery exits and canonical-ID races before publishing proof. diff --git a/src/main/daemon/daemon-router-legacy-owner.ts b/src/main/daemon/daemon-router-legacy-owner.ts new file mode 100644 index 00000000000..fed94f3be14 --- /dev/null +++ b/src/main/daemon/daemon-router-legacy-owner.ts @@ -0,0 +1,62 @@ +import type { PtySpawnOptions } from '../providers/pty-provider-contract' +import { TerminalSessionOwnerUnverifiedError } from './daemon-errors' +import type { LegacyDaemonExitProof } from './daemon-legacy-exit-proof' +import type { DaemonPtyAdapter } from './daemon-pty-adapter' + +/** + * The daemon still running an unrouted session id, or undefined when none does. Why: a spawn of + * that id on the current daemon starts a second session under it, and the next stop of the id + * lands on the old one's live agent. A legacy daemon that cannot answer is not proof of death, + * so it refuses unless its pid is proven gone. + */ +export async function findLegacySessionOwner( + opts: Pick, + current: DaemonPtyAdapter, + legacy: readonly DaemonPtyAdapter[], + exitProof: LegacyDaemonExitProof +): Promise { + const { sessionId } = opts + if (!sessionId || opts.isNewSession || opts.attachOnly || legacy.length === 0) { + return undefined + } + // Why first: an unreachable legacy daemon must not block reattaching the current daemon's own session. + if ((await probeOwnership(current, sessionId)) === true) { + return current + } + const verdicts = await Promise.all( + legacy.map(async (adapter) => ({ + adapter, + live: await probeOwnership(adapter, sessionId) + })) + ) + exitProof.record(legacy) + const owner = verdicts.find(({ live }) => live === true) + if (owner) { + return owner.adapter + } + if (verdicts.some(({ adapter, live }) => live === null && !exitProof.hasExited(adapter))) { + throw new TerminalSessionOwnerUnverifiedError(sessionId) + } + return undefined +} + +const LEGACY_OWNER_INVENTORY_MS = 2_000 + +async function probeOwnership( + adapter: DaemonPtyAdapter, + sessionId: string +): Promise { + const live = await adapter.probePtyLiveness(sessionId).catch(() => null) + if (live !== null) { + return live + } + // Why: the probe never reconnects, so a daemon whose startup connect failed would refuse forever. + try { + const processes = await adapter.listProcesses({ + deadlineMs: Date.now() + LEGACY_OWNER_INVENTORY_MS + }) + return processes.some((process) => process.id === sessionId) + } catch { + return null + } +}