diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts index f5ee2ca66c..bbb560257b 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -72,6 +72,7 @@ import { $sessionTiles, $workingSessionIds, foregroundSessionScopes, + forgetProfileOnlyRuntimeOwners, liveSessionScopes, openTileGatewayScopes, reconcileBusyStatesOnReconnect, @@ -760,6 +761,7 @@ export function useGatewayBoot({ // primary thread or a just-created session's owner hold is bound to // (#93892). foregroundScopes: foregroundSessionScopes, + onLocalProfileRetired: forgetProfileOnlyRuntimeOwners, onActiveConnectionChanged: publish, // Keep $activeGatewayProfile in lockstep with the registry's OWN record // of which profile the active socket serves. The registry is the only diff --git a/apps/desktop/src/store/gateway-profile-only-owner.integration.test.ts b/apps/desktop/src/store/gateway-profile-only-owner.integration.test.ts new file mode 100644 index 0000000000..931180a353 --- /dev/null +++ b/apps/desktop/src/store/gateway-profile-only-owner.integration.test.ts @@ -0,0 +1,185 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const gatewayMocks = vi.hoisted(() => { + const instances: Array<{ + connectionState: string + emit: (event: { payload?: Record; session_id?: string; type: string }) => void + request: ReturnType + }> = [] + + return { instances } +}) + +vi.mock('@/hermes', async importActual => ({ + ...(await importActual>()), + setApiRequestConnection: vi.fn(), + HermesGateway: class { + connectionState = 'closed' + private eventHandlers = new Set<(event: { payload?: Record; session_id?: string; type: string }) => void>() + request = vi.fn(async (method: string, params: Record) => ({ method, params })) + + constructor() { + gatewayMocks.instances.push(this as never) + } + + connect = async (): Promise => { + this.connectionState = 'open' + } + + close = (): void => { + this.connectionState = 'closed' + } + + onEvent = ( + handler: (event: { payload?: Record; session_id?: string; type: string }) => void + ): (() => void) => { + this.eventHandlers.add(handler) + + return () => this.eventHandlers.delete(handler) + } + + onState = (): (() => void) => () => undefined + + emit = (event: { payload?: Record; session_id?: string; type: string }): void => { + for (const handler of this.eventHandlers) { + handler(event) + } + } + } +})) + +const { + closeSecondaryGateways, + configureGatewayRegistry, + ensureGatewayForProfile, + retireLocalProfileGateways, + setPrimaryGateway +} = await import('./gateway') + +const { $profiles } = await import('./profile') + +const { $activeSessionId, _resetSessionOwnerHintsForTests, setSessionOwnerHint, setSessions } = await import('./session') +const { $gateway } = await import('./gateway') +const { clearAllPrompts } = await import('./prompts') + +const { + $sessionStates, + $sessionTiles, + clearAllSessionStates, + forgetProfileOnlyRuntimeOwners, + knownOwnerForSession, + recordSessionEventScope, + requestForOwnedSession +} = await import('./session-states') + +const { isSessionOwnerResolutionError } = await import('./session-owner-resolution') + +function installDesktop(): void { + ;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = { + getConnection: vi.fn(async (profile: string) => ({ + authMode: 'token', + mode: 'local', + profile, + token: 'test-token', + wsUrl: `wss://${profile}.invalid/ws` + })), + notify: vi.fn() + } +} + +beforeEach(() => { + installDesktop() + gatewayMocks.instances.length = 0 + $profiles.set([{ name: 'default' }, { name: 'research' }] as never) + $sessionTiles.set([]) + setSessions([]) + clearAllSessionStates() + clearAllPrompts() + $activeSessionId.set(null) + _resetSessionOwnerHintsForTests({ storage: true }) + + // This is the exact event fan-in installed by useGatewayBoot: the secondary + // producer calls config.onEvent, which records scope before UI dispatch. + configureGatewayRegistry({ + onLocalProfileRetired: forgetProfileOnlyRuntimeOwners, + onEvent: event => { + recordSessionEventScope(event) + } + }) + const primary = { connectionState: 'open', request: vi.fn() } + $gateway.set(primary as never) + setPrimaryGateway(primary as never, 'default') +}) + +afterEach(() => { + closeSecondaryGateways() + clearAllSessionStates() + $sessionTiles.set([]) + $profiles.set([]) + setSessions([]) + _resetSessionOwnerHintsForTests({ storage: true }) + clearAllPrompts() + $activeSessionId.set(null) + $gateway.set(null) + delete (window as { hermesDesktop?: unknown }).hermesDesktop +}) + +describe('profile-only secondary approval ownership', () => { + it('keeps a profile-only secondary event owner across a focus switch and dispatches approval on that secondary', async () => { + await expect(ensureGatewayForProfile('research')).resolves.toBeUndefined() + const secondary = gatewayMocks.instances[0] + + expect(secondary).toBeTruthy() + // No tile, row, hint, or runtime→stored state mirror is available. + expect($sessionTiles.get()).toEqual([]) + expect($sessionStates.get()).toEqual({}) + + secondary.emit({ session_id: 'rt-secondary', type: 'approval.request' }) + await expect(ensureGatewayForProfile('default')).resolves.toBeUndefined() + + expect(knownOwnerForSession('rt-secondary')).toBe('research') + + const ambient = vi.fn(async () => ({ via: 'ambient' })) + await expect( + requestForOwnedSession('rt-secondary', ambient as never, 'approval.respond', { + choice: 'once', + session_id: 'rt-secondary' + }) + ).resolves.toEqual({ + method: 'approval.respond', + params: { choice: 'once', session_id: 'rt-secondary' } + }) + + expect(secondary.request).toHaveBeenCalledWith('approval.respond', { + choice: 'once', + session_id: 'rt-secondary' + }) + expect(ambient).not.toHaveBeenCalled() + }) + + it('rejects unproven profiles and retires only transient local ownership, never durable exact routes', async () => { + recordSessionEventScope({ profile: 'research', session_id: 'rt-unproven' }) + expect(knownOwnerForSession('rt-unproven')).toBeUndefined() + await expect(requestForOwnedSession('rt-unproven', vi.fn() as never, 'approval.respond', {})) + .rejects.toSatisfy(isSessionOwnerResolutionError) + + await ensureGatewayForProfile('research') + const secondary = gatewayMocks.instances[0] + secondary.emit({ session_id: 'rt-retired', type: 'approval.request' }) + secondary.emit({ session_id: 'rt-durable', type: 'approval.request' }) + expect(knownOwnerForSession('rt-retired')).toBe('research') + const exact = { connectionId: 'remote-same-name', profile: 'research' } + setSessionOwnerHint('rt-durable', exact) + recordSessionEventScope({ ...exact, session_id: 'rt-remote' }) + expect(knownOwnerForSession('rt-durable')).toEqual(exact) + retireLocalProfileGateways('research') + expect(knownOwnerForSession('rt-retired')).toBeUndefined() + expect(knownOwnerForSession('rt-durable')).toEqual(exact) + expect(knownOwnerForSession('rt-remote')).toEqual(exact) + const getConnection = window.hermesDesktop!.getConnection + vi.mocked(getConnection).mockClear() + await expect(requestForOwnedSession('rt-retired', vi.fn() as never, 'approval.respond', {})) + .rejects.toSatisfy(isSessionOwnerResolutionError) + expect(getConnection).not.toHaveBeenCalled() + }) +}) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 0812aec00f..f9ce195cb1 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -7,6 +7,7 @@ import { reconnectBackoffDelayMs } from '@/lib/reconnect-backoff' import { RECONNECT_ATTEMPT_TIMEOUT_MS, withTimeout } from '@/lib/with-timeout' import { markNativeNotifyBaseline } from '@/store/notify-baseline' import { setConnection, setGatewayState } from '@/store/session' +import { stampSecondaryProfileOwner } from '@/store/session-event-provenance' // ── Multi-profile gateway routing ────────────────────────────────────────── // Concurrent sessions across profiles need concurrent sockets: the renderer's @@ -61,6 +62,10 @@ interface RegistryConfig { * (#89206: the stale-profile split-brain that stranded bot wake-ups). */ onActiveRouteChanged?: (profile: string) => void + /** Drop transient profile-pool runtime routes when local profile teardown + * permanently retires their owning secondary. Exact registry routes are + * deliberately outside this callback: a remote source may share the name. */ + onLocalProfileRetired?: (profile: string) => void /** * Scopes a FOREGROUND surface is bound to right now — every mounted * session tile's owner and the primary thread's (foregroundSessionScopes in @@ -757,9 +762,13 @@ function createSecondary(profile: string, connectionId: null | string = null): S } // Events keep carrying the bare profile — session routing is profile-keyed - // everywhere. connectionId rides along for surfaces that need the source. + // everywhere. A pool secondary with no registry connection has no exact + // connection id, so stamp this closure-owned profile before registry fan-in; + // the recorder must not promote an arbitrary wire `profile` field instead. entry.offEvent = gateway.onEvent(event => { - g.config?.onEvent({ ...event, profile, ...(connectionId ? { connectionId } : {}) }) + const scopedEvent = stampSecondaryProfileOwner({ ...event, ...(connectionId ? { connectionId } : {}) }, profile) + + g.config?.onEvent(scopedEvent) releaseTerminalTurnLease(entry.scope, event) }) entry.offState = gateway.onState(state => { @@ -1828,6 +1837,12 @@ export function retireLocalProfileGateways(profile: string): void { const scopes = new Set([key, registryBackendScopeKey('local', key)]) let activeInvalidated = false + // A profile-only owner is a claim about the legacy local pool, not durable + // session identity. Clear it with that pool before a delayed session action + // can recreate the deleted/old-name backend. Exact remote owners are + // descriptors and remain routable even when they share this profile name. + g.config?.onLocalProfileRetired?.(key) + for (const scope of scopes) { const entry = g.secondaries.get(scope) diff --git a/apps/desktop/src/store/session-event-provenance.ts b/apps/desktop/src/store/session-event-provenance.ts new file mode 100644 index 0000000000..0be840405a --- /dev/null +++ b/apps/desktop/src/store/session-event-provenance.ts @@ -0,0 +1,31 @@ +import type { GatewayEvent } from '@hermes/shared' + +// A profile name received over the wire is descriptive, not an ownership +// claim: primary and arbitrary synthetic events can carry one. The secondary +// socket closure is the authority for profile-only pool routes, so stamp its +// outgoing copy with a non-serializable marker before it crosses registry +// fan-in. JSON-RPC payloads cannot forge a Symbol property. +const secondaryProfileOwner = Symbol('secondaryProfileOwner') + +type SecondaryProfileOwnedEvent = GatewayEvent & { + [secondaryProfileOwner]?: string +} + +export function stampSecondaryProfileOwner(event: GatewayEvent, profile: string): GatewayEvent { + const scoped = { ...event, profile } as SecondaryProfileOwnedEvent + + Object.defineProperty(scoped, secondaryProfileOwner, { + configurable: false, + enumerable: false, + value: profile, + writable: false + }) + + return scoped +} + +export function secondaryProfileOwnerForEvent(event: GatewayEvent): string | undefined { + const profile = (event as SecondaryProfileOwnedEvent)[secondaryProfileOwner] + + return typeof profile === 'string' && profile.trim() ? profile.trim() : undefined +} diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index afd217b79b..19c54c58e9 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -16,7 +16,7 @@ * itself here as the delegate so tile UI stays dependency-light. */ -import { LOCAL_CONNECTION_ID, registryBackendScopeKey } from '@hermes/shared' +import { type GatewayEvent, LOCAL_CONNECTION_ID, registryBackendScopeKey } from '@hermes/shared' import { atom, computed } from 'nanostores' import type { ClientSessionState } from '@/app/types' @@ -56,6 +56,7 @@ import { setBusy, setSessions } from './session' +import { secondaryProfileOwnerForEvent } from './session-event-provenance' import { assertSessionOwnerResolved } from './session-owner-resolution' import { requestForSessionProfile, @@ -84,22 +85,50 @@ export const $sessionStates = atom>({}) const sessionScopeByRuntimeId = new Map() -// Structured twin of the scope ledger: the same inbound events also carry the -// exact (connectionId, profile) owner, which the composite scope string -// cannot give back. Consumed as the LAST rung of knownOwnerForSession so a -// runtime whose event source already proved its owner can still route -// session-scoped RPCs (approval.respond) when every durable binding -// (tile / hint / row) is absent — while durable stored identity keeps -// outranking it (#97511). -const sessionOwnerByRuntimeId = new Map() +// Structured twin of the scope ledger: inbound events can carry either an +// exact (connectionId, profile) owner or a producer-proven profile-only pool +// owner. Consumed as the LAST rung of knownOwnerForSession so a runtime whose +// event source already proved its owner can still route session-scoped RPCs +// (approval.respond) when every durable binding (tile / hint / row) is absent +// — while durable stored identity keeps outranking it (#97511). +const sessionOwnerByRuntimeId = new Map() export function recordSessionEventScope(event: { connectionId?: string; profile?: string; session_id?: string }): void { - if (event.session_id && event.connectionId) { + if (!event.session_id) { + return + } + + if (event.connectionId) { sessionScopeByRuntimeId.set(event.session_id, registryBackendScopeKey(event.connectionId, event.profile)) sessionOwnerByRuntimeId.set(event.session_id, { connectionId: event.connectionId, profile: String(event.profile ?? '').trim() || 'default' }) + + return + } + + // Only gateway.ts's secondary closure can add this non-serializable marker. + // A profile field from a primary or arbitrary inbound event is descriptive, + // not an owner route, and must keep failing closed in multi-profile installs. + const profile = secondaryProfileOwnerForEvent(event as GatewayEvent) + + if (profile) { + sessionOwnerByRuntimeId.set(event.session_id, profile) + } +} + +/** Forget only profile-pool runtime owners during permanent LOCAL profile + * teardown. These string routes came exclusively from the legacy secondary + * producer; exact connection descriptors must survive a same-named remote + * profile's local delete/rename. */ +export function forgetProfileOnlyRuntimeOwners(profile: string): void { + const retired = normalizeProfileKey(profile) + + for (const [runtimeId, owner] of sessionOwnerByRuntimeId) { + if (typeof owner === 'string' && normalizeProfileKey(owner) === retired) { + sessionOwnerByRuntimeId.delete(runtimeId) + } } } @@ -989,10 +1018,10 @@ export function openTileGatewayScopes(): Set { * Last rung: the owner recorded from the inbound runtime event itself * (sessionOwnerByRuntimeId, #97511) — an orphan runtime whose tile/hint/row * binding is absent or stale still routes through the exact - * (connectionId, profile) its events proved, while every durable rung above - * keeps outranking it, so a stored-id collision never inherits a stale - * runtime ledger entry. Untagged events record nothing, so unknown owners in - * multi-profile topology still fail closed. + * (connectionId, profile) or secondary socket's proven local profile. Every + * durable rung above keeps outranking it, so a stored-id collision never + * inherits a stale runtime ledger entry. Unproven profile fields record + * nothing, so unknown owners in multi-profile topology still fail closed. * Returns undefined when no owner is known — the caller fails closed * (assertSessionOwnerResolved), never falls to "active". */ diff --git a/website/docs/user-guide/multi-connection-desktop.md b/website/docs/user-guide/multi-connection-desktop.md index 13ce2e88b6..9746d02cdc 100644 --- a/website/docs/user-guide/multi-connection-desktop.md +++ b/website/docs/user-guide/multi-connection-desktop.md @@ -157,6 +157,13 @@ Each `(connection, profile)` pair gets its own backend and socket, pooled with the same idle-reaping as local per-profile backends — background agents keep streaming while you look at another gateway. +Approval buttons route back to the session's owning backend, not whichever +profile is currently selected. For a local secondary profile, Desktop can use +the socket that delivered the request even when the cached session binding is +missing. Saved session ownership still takes precedence, and deleting or +renaming that local profile clears this temporary route rather than reconnecting +an obsolete backend. + ### Switching and scoping The sidebar foot follows one hierarchy: **gateway → profile → sessions**.