diff --git a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts index 7db2fb9fae..10173894b3 100644 --- a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts +++ b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts @@ -59,7 +59,9 @@ const { ensureActiveGatewayOpen, ensureGatewayForAgent, ensureGatewayForProfile, + openGatewayForAgent, openGatewayForProfile, + pruneSecondaryGateways, reconnectSecondaryGateways, retireLocalProfileGateways, setPrimaryGateway @@ -180,6 +182,44 @@ describe('disposeSecondariesForConnection', () => { }) describe('secondary reconnect runtime scope', () => { + it('invalidates stale runtime bindings before a direct secondary reopen publishes open', async () => { + installDesktop({ + getConnectionFor: vi.fn(async ({ connectionId, profile }) => descriptorFor(connectionId, profile)) + }) + + await openGatewayForAgent('homelab', 'writer') + const firstSocket = gatewayMocks.instances[0] + pruneSecondaryGateways(new Set()) + expect(firstSocket.close).toHaveBeenCalledOnce() + + let finishReconnect!: () => void + + const reconnect = new Promise(resolve => { + finishReconnect = resolve + }) + + gatewayMocks.connect.mockImplementationOnce(() => reconnect) + + // A real user action after the renderer/main-process pool reaped an idle + // profile creates a fresh Secondary entry and opens it directly. Runtime + // bindings from the previous backend generation must be gone before the + // new socket can publish `open`, or the request can immediately reuse a + // process-local id the respawned backend never minted. + const reopening = openGatewayForAgent('homelab', 'writer') + + await vi.waitFor(() => expect(gatewayMocks.connect).toHaveBeenCalledTimes(2)) + expect(reconnectStateMocks.resetTileRuntimeBindings).toHaveBeenCalledWith({ + connectionId: 'homelab', + profile: 'writer' + }) + expect(reconnectStateMocks.resetTileRuntimeBindings.mock.invocationCallOrder[0]).toBeLessThan( + gatewayMocks.connect.mock.invocationCallOrder[1] + ) + + finishReconnect() + await reopening + }) + it('rebinds only Bot runtimes owned by the reconnected profile route', async () => { installDesktop({ getConnectionFor: vi.fn(async ({ connectionId, profile }) => descriptorFor(connectionId, profile)) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 6186cefaa6..7de63e8a28 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -47,6 +47,8 @@ interface Secondary { connectionId: null | string connection: HermesConnection | null gateway: HermesGateway + /** True after this entry completed at least one socket connection. */ + openedOnce: boolean activeRequests: number connectPromise: Promise | null offEvent: () => void @@ -106,6 +108,8 @@ interface GatewayRegistryState { activeKey: string activationEpoch: number secondaries: Map + /** Scopes that opened in this renderer generation, even if later pruned. */ + openedSecondaryScopes?: Set $gateway: ReturnType> $activeProfile: ReturnType> } @@ -121,6 +125,7 @@ function createRegistryState(): GatewayRegistryState { activeKey: 'default', activationEpoch: 0, secondaries: new Map(), + openedSecondaryScopes: new Set(), // The active gateway instance, exposed for inline message-stream // components (inline ClarifyTool, model overlays) that call gateway // methods without the instance threaded down through props. @@ -155,6 +160,10 @@ function gatewayState(): GatewayRegistryState { const g = gatewayState() +// Dev HMR can hand a newer module an older state-container shape. Keep the +// generation ledger lazy so an already-open socket still survives the update. +const openedSecondaryScopes = (): Set => (g.openedSecondaryScopes ??= new Set()) + // Re-exported as a stable binding: the atom instance lives in `g`, so every hot // reload of this module hands back the SAME atom subscribers are already wired // to. (A fresh `atom()` per reload would orphan existing subscriptions.) @@ -355,6 +364,35 @@ async function openSecondary(entry: Secondary): Promise { } const pending = (async () => { + // A secondary can be reopened directly by the next routed user action, + // without passing through reconnectSecondary(). Its previous backend may + // have been respawned, so every stored→runtime binding for this exact scope + // is process-local stale state. Invalidate BEFORE connect publishes `open`: + // otherwise an eager route effect / submit can send the old runtime id in + // the narrow window between the new socket opening and post-connect cleanup. + // + // Dynamic import keeps the existing session-states → gateway module cycle + // open. Awaiting it is intentional: correctness at the generation boundary + // outranks the single local-module microtask this adds to a reconnect. + const openedScopes = openedSecondaryScopes() + const reopening = entry.openedOnce || entry.connection !== null || openedScopes.has(entry.scope) + let reconcileBusyAfterOpen: null | (() => void) = null + + if (reopening) { + try { + const { reconcileBusyStatesOnReconnect, resetTileRuntimeBindings } = await import('@/store/session-states') + + resetTileRuntimeBindings({ + connectionId: entry.connectionId || 'local', + profile: entry.profile + }) + reconcileBusyAfterOpen = () => reconcileBusyStatesOnReconnect(entry.scope) + } catch { + // Best effort for partial test/HMR graphs. Production always loads the + // real store; a failed import must not make the transport unrecoverable. + } + } + // Registry-scoped entries dial through getConnectionFor when the bridge has // it. Local/legacy entries retain the existing getConnection path. const conn = @@ -377,6 +415,15 @@ async function openSecondary(entry: Secondary): Promise { const wsUrl = await resolveGatewayWsUrl(wsDeps, conn) await entry.gateway.connect(wsUrl) + entry.openedOnce = true + openedScopes.add(entry.scope) + + try { + reconcileBusyAfterOpen?.() + } catch { + // The socket is already open. A best-effort UI-state reconcile must not + // turn that successful transport recovery into a reported dial failure. + } if (!entry.wantOpen) { entry.gateway.close() @@ -427,27 +474,6 @@ async function reconnectSecondary(entry: Secondary): Promise { try { await openSecondary(entry) entry.reconnectAttempt = 0 - // The re-dialed backend may have respawned and re-minted runtime ids — - // busy flags recorded from THIS socket's pre-drop events would then never - // receive their terminal busy:false, leaving the session's running arc - // armed forever (#53902/#73082 stale-flag half). Scoped: only runtimes - // whose events arrived on this connection are reconciled; live work on - // other sockets is untouched, and a genuinely live turn here re-asserts - // busy on its next event. Lazy import: a static edge here closes a module - // cycle (session-states → … → gateway) that leaves nanostores atoms - // undefined at init for whichever module loads second. Best-effort catch: - // under partial vi.mock('@/hermes') harnesses the transitive graph can - // fail to load — a skipped reconcile there must not surface as an - // unhandled rejection (the real graph always loads in production). - void import('@/store/session-states') - .then(({ reconcileBusyStatesOnReconnect, resetTileRuntimeBindings }) => { - reconcileBusyStatesOnReconnect(entry.scope) - resetTileRuntimeBindings({ - connectionId: entry.connectionId || 'local', - profile: entry.profile - }) - }) - .catch(() => undefined) } catch (error) { // The registry no longer knows this connection (removed while we were // backing off), or Electron's deletion guard reports the profile itself @@ -506,6 +532,7 @@ function createSecondary(profile: string, connectionId: null | string = null): S connectionId, connection: null, gateway, + openedOnce: false, activeRequests: 0, connectPromise: null, offEvent: () => {}, @@ -1209,6 +1236,7 @@ export function closeSecondaryGateways(): void { } g.secondaries.clear() + openedSecondaryScopes().clear() restoreActiveToPrimaryIfEvicted() }