From b2c8a148e83e41e949527afc0a5724ac9bfdf3f4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:11:46 -0700 Subject: [PATCH] fix(desktop): fail-stop deleted-profile reconnects and evict the stale rail badge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the renderer half of #88769, found while live-verifying the profile-lifecycle fixes: - The secondary-socket reconnect loop now fail-stops when Electron's spawn guard rejects with "no longer exists" / "is being deleted" — a permanent condition for that scope, previously retried forever on the 15s-cap backoff (40+ guard hits observed in 3 minutes after clicking a stale badge). The entry is disposed, evicted, and the active key restored to the primary, mirroring the existing missing-connection fail-stop. - The Bot Mode SDK deleteProfile path now refreshes $profiles after a successful delete, so the profile rail drops the dead badge instead of keeping a clickable ghost. (The Desktop dialog path already refreshed via onDeleted; the SDK path was the gap.) Tests: two fail-stop regression tests (profile-gone + mid-delete guard rejections, sabotage-verified to fail without the fix) and a refresh assertion on the SDK delete ordering test. --- apps/desktop/src/sdk/index.ts | 6 ++ apps/desktop/src/sdk/profile-routing.test.ts | 3 + .../gateway-connection-lifecycle.test.ts | 67 +++++++++++++++++++ apps/desktop/src/store/gateway.ts | 26 +++++-- 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 0d037b809e..e7f9da15ea 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -303,6 +303,12 @@ export const host = { retireLocalProfileGateways(name) await deleteProfile(name) + // The profile rail paints from the shared $profiles cache; without a + // refresh the deleted profile's badge survives and clicking it starts a + // doomed spawn-retry loop against Electron's deletion guard (#88769). + // Best-effort: the delete itself already succeeded. + await refreshProfiles().catch(() => undefined) + if (wasActive) { selectProfile('default') setActiveProfile('default') diff --git a/apps/desktop/src/sdk/profile-routing.test.ts b/apps/desktop/src/sdk/profile-routing.test.ts index 812eb7846b..ed1b928d46 100644 --- a/apps/desktop/src/sdk/profile-routing.test.ts +++ b/apps/desktop/src/sdk/profile-routing.test.ts @@ -130,6 +130,9 @@ describe('connection-aware plugin host APIs', () => { expect(order).toEqual(['retire', 'delete']) expect(retireLocalProfileGateways).toHaveBeenCalledWith('worker') + // The rail paints from $profiles; skipping the refresh leaves a stale + // badge whose click hot-loops against the deletion guard (#88769). + expect(refreshProfiles).toHaveBeenCalled() }) it('refreshes the profile inventory before asking Electron for routes', async () => { diff --git a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts index 345f52216b..0feed4309b 100644 --- a/apps/desktop/src/store/gateway-connection-lifecycle.test.ts +++ b/apps/desktop/src/store/gateway-connection-lifecycle.test.ts @@ -51,6 +51,7 @@ const { disposeSecondariesForConnection, ensureActiveGatewayOpen, ensureGatewayForAgent, + ensureGatewayForProfile, openGatewayForProfile, reconnectSecondaryGateways, retireLocalProfileGateways, @@ -241,4 +242,70 @@ describe('reconnect fail-stop on a removed connection', () => { expect(reopened).not.toBeNull() }) + + it('evicts a LOCAL profile entry when the deletion guard reports the profile gone (#88769)', async () => { + // A stale rail badge clicked after deletion drives reconnects against + // Electron's spawn guard, which rejects every attempt. That rejection is + // permanent — the loop must fail-stop, not hammer the guard on backoff. + // sharedPrimaryRoute probes getConnection too, so resolve enough calls to + // get the socket open before the guard starts rejecting. + let connectionCalls = 0 + + const getConnection = vi.fn(async () => { + connectionCalls += 1 + + if (connectionCalls <= 3) { + return descriptorFor('legacy-local', 'selena') + } + + throw new Error('Profile "selena" no longer exists.') + }) + + installDesktop({ getConnection }) + + await openGatewayForProfile('selena') + await ensureGatewayForProfile('selena') + expect(gatewayMocks.instances).toHaveLength(1) + connectionCalls = 99 + + const socket = gatewayMocks.instances[0] as unknown as { connectionState: string } + socket.connectionState = 'closed' + + // Drive the reconnect: the guard rejection must dispose + evict. + const result = await ensureActiveGatewayOpen() + + expect(result).toBeNull() + const callsAfterFailStop = getConnection.mock.calls.length + await ensureActiveGatewayOpen() + expect(getConnection.mock.calls.length).toBe(callsAfterFailStop) + }) + + it('fail-stops on the mid-delete guard rejection too', async () => { + let connectionCalls = 0 + + const getConnection = vi.fn(async () => { + connectionCalls += 1 + + if (connectionCalls <= 3) { + return descriptorFor('legacy-local', 'selena') + } + + throw new Error('Profile "selena" is being deleted.') + }) + + installDesktop({ getConnection }) + + await openGatewayForProfile('selena') + await ensureGatewayForProfile('selena') + connectionCalls = 99 + + const socket = gatewayMocks.instances[0] as unknown as { connectionState: string } + socket.connectionState = 'closed' + + await ensureActiveGatewayOpen() + + const callsAfterFailStop = getConnection.mock.calls.length + await ensureActiveGatewayOpen() + expect(getConnection.mock.calls.length).toBe(callsAfterFailStop) + }) }) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index e753f7474e..e50f20c41e 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -325,12 +325,20 @@ async function reconnectSecondary(entry: Secondary): Promise { entry.reconnectAttempt = 0 } catch (error) { // The registry no longer knows this connection (removed while we were - // backing off). Retrying forever can never succeed — fail-stop: dispose - // the entry and evict it instead of an infinite 15s-cap retry loop. - if (entry.connectionId && isMissingConnectionError(error)) { + // backing off), or Electron's deletion guard reports the profile itself + // gone/mid-delete. Both are permanent for this scoped socket — retrying + // forever can never succeed and hammers the spawn guard every backoff + // tick (#88769). Fail-stop: dispose the entry and evict it instead of an + // infinite 15s-cap retry loop. + if ((entry.connectionId && isMissingConnectionError(error)) || isMissingProfileError(error)) { entry.reconnecting = false disposeSecondary(entry) - g.secondaries.delete(entry.scope) + + if (g.secondaries.get(entry.scope) === entry) { + g.secondaries.delete(entry.scope) + } + + restoreActiveToPrimaryIfEvicted() return } @@ -353,6 +361,16 @@ function isMissingConnectionError(error: unknown): boolean { return message.includes('No connection with id') } +// Electron's spawn guard (assertLocalProfileCanStart) rejects with these when +// the profile's directory is gone or its DELETE is still in flight. For a +// renderer socket that condition is permanent: the backend it reconnects to +// can never come back, and every retry hammers the guard (#88769). +function isMissingProfileError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error ?? '') + + return message.includes('no longer exists') || message.includes('is being deleted') +} + function createSecondary(profile: string, connectionId: null | string = null): Secondary { const gateway = new HermesGateway() const scope = registryBackendScopeKey(connectionId, profile)