From 3bb2ac71ec8c11f6022ff0aca2c6035ed06ac3be Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 13 Sep 2026 21:55:11 -0700 Subject: [PATCH] fix(native-chat): stop the router reporting a stop it never observed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `closeAll` cleared the route table and set one boolean, after which that boolean was the only surviving evidence about any session. Two call sites then spent it: `releaseAcquisition` and the stop path each turned a route-lookup MISS into reported success. Eviction reads a `true` from the stop path as proof the provider child is gone and releases the durable lease on it, so a session the router never routed could have its lease handed back on the strength of "I have no record, but everything is closed." Loss of contact is not evidence of process death. The fix keeps the evidence instead of the inference: adapter shutdown only resolves once every child is proven stopped, so `closeAll` now marks each routed session `stopped` rather than forgetting it. A routed session still answers `true` from its own retained proof; a session with no route answers `false`, which leaves it indexed for a real retry. `releaseAcquisition` drops its short-circuit and asks the adapters, which answer from their own session maps. The acquire-side latch is unchanged: once closed, the router stays closed and refuses new work. Behaviour that changed: a post-`closeAll` stop for a session the router never routed, or one the host already acknowledged as released, now reports unproven instead of proven. That matches what the same call already answered before `closeAll`, and no real flow reaches it — quit evicts every owned session before `closeAll` runs, and eviction only asks the adapter for sessions whose provider child this host acquired through the router. --- ...tured-agent-session-adapter-router.test.ts | 27 +++++++++++++++---- ...structured-agent-session-adapter-router.ts | 16 +++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts index 02e73e8e73b..e08c289c87a 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.test.ts @@ -187,7 +187,7 @@ describe('StructuredAgentSessionAdapterRouter.closeAll', () => { expect(acquire).not.toHaveBeenCalled() }) - it('publishes one global close proof without retaining session ids', async () => { + it('keeps a per-session stop proof and reports no stop for a session it never routed', async () => { const claude = adapterOf(vi.fn(async () => true)) const closeAdapters = vi.fn(async () => undefined) const router = new StructuredAgentSessionAdapterRouter( @@ -202,14 +202,30 @@ describe('StructuredAgentSessionAdapterRouter.closeAll', () => { await router.closeAll() + // The routed session carries the shutdown's own exit proof; the other two are sessions this + // router has no record of, and an absent record is not a stop it can report. await expect(router.closeSession('session-1')).resolves.toBe(true) - await expect(router.closeSession('never-routed')).resolves.toBe(true) + await expect(router.closeSession('never-routed')).resolves.toBe(false) router.acknowledgeSessionRelease('session-1') - await expect(router.closeSession('session-1')).resolves.toBe(true) + await expect(router.closeSession('session-1')).resolves.toBe(false) await router.closeAll() expect(closeAdapters).toHaveBeenCalledOnce() }) + it('asks the adapters to release an unrouted session rather than answering from the close proof', async () => { + const claudeRelease = vi.fn(async () => true) + const codexRelease = vi.fn(async () => false) + const router = new StructuredAgentSessionAdapterRouter( + { claude: adapterOf(claudeRelease), codex: adapterOf(codexRelease) }, + async () => undefined + ) + await router.closeAll() + + await expect(router.releaseAcquisition({ sessionId: 'never-routed' })).resolves.toBe(true) + expect(claudeRelease).toHaveBeenCalledWith({ sessionId: 'never-routed' }) + expect(codexRelease).toHaveBeenCalledWith({ sessionId: 'never-routed' }) + }) + it('retains live routes and publishes no global proof when closeAll fails', async () => { const failure = new Error('adapter shutdown failed') const claude = adapterOf(vi.fn(async () => true)) @@ -268,9 +284,10 @@ describe('StructuredAgentSessionAdapterRouter.closeAll', () => { await router.closeAll() resolveAcquire(acquisition(2, 'spawn-2')) - // The route is NOT published behind a closed adapter, so nothing routes back out to it. + // The route is NOT published behind a closed adapter, so nothing routes back out to it — and + // with no route the router has nothing to stop and no stop to report. await expect(acquiring).rejects.toThrow('router is closed') - await expect(router.closeSession('session-1')).resolves.toBe(true) + await expect(router.closeSession('session-1')).resolves.toBe(false) expect(closeSession).not.toHaveBeenCalled() }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts index 041d9adc11e..1cc571d39aa 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.ts @@ -51,9 +51,6 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi this.routes.delete(input.sessionId) } } - if (this.allAdaptersClosed) { - return true - } let released = false for (const candidate of Object.values(this.adapters)) { released = (await candidate.releaseAcquisition?.(input)) === true || released @@ -145,7 +142,10 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi ): Promise { const route = this.routes.get(sessionId) if (!route) { - return this.allAdaptersClosed + // No route is loss of contact, never proof of a stop. Answering `true` here would hand a + // caller a receipt for a session this router never acted on — and the caller spends that + // receipt by releasing the durable lease. + return false } if (route.state === 'stopped') { return true @@ -169,7 +169,13 @@ export class StructuredAgentSessionAdapterRouter implements StructuredAgentSessi this.closePromise = (async () => { try { await this.closeAdapters() - this.routes.clear() + // Adapter shutdown only resolves once every child is PROVEN stopped, so each routed + // session inherits that proof and keeps it per session. Clearing the map instead would + // leave one boolean as the only surviving evidence, and an empty map cannot tell a + // session this router stopped from one it never saw. + for (const route of this.routes.values()) { + route.state = 'stopped' + } this.allAdaptersClosed = true } finally { this.closePromise = null