From 39bf984765e4a2c75e49b0c52019ca41ed3667c9 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:44:35 -0700 Subject: [PATCH] fix(desktop): sync connection atoms and share the switch mutex for agent activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureGatewayForAgent (the SDK ensureAgent door) skipped the two invariants the profile path provides: - $connection / $activeGatewayProfile were only updated when a socket was freshly dialed (setConnection inside openSecondary), so activating an ALREADY-OPEN registry agent left both describing the previous backend — /api/fs, /api/media and image.attach routed to the wrong machine (same class as #46651) and newSessionInProfile targeted the stale profile. - Activations bypassed the gatewaySwitch mutex, so a rapid agent/profile interleave could complete out of order with the earlier setActive() landing last. Add profile.ts ensureGatewayAgent: the (connectionId, profile) analogue of ensureGatewayProfile that shares the same gatewaySwitch mutex, moves $activeGatewayProfile on every activation, and resyncs $connection from getConnectionFor (best-effort, like the profile path). The SDK ensureAgent now routes through it; local/null connectionId falls through to the profile path unchanged. --- apps/desktop/src/sdk/index.ts | 9 +- .../store/profile-agent-activation.test.ts | 173 ++++++++++++++++++ apps/desktop/src/store/profile.test.ts | 3 +- apps/desktop/src/store/profile.ts | 63 ++++++- 4 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/src/store/profile-agent-activation.test.ts diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 05c9245147..2ad2e22bdf 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -24,10 +24,11 @@ import { openSession, type OpenSessionIntent } from '@/app/open-session' import { $narrowViewport } from '@/components/pane-shell/tree/store' import { onGatewayEvent } from '@/contrib/events' import { deleteProfile, getLogs, getStatus, type HermesGateway } from '@/hermes' -import { $gateway, ensureGatewayForAgent, openGatewayForAgent, openGatewayForProfile } from '@/store/gateway' +import { $gateway, openGatewayForAgent, openGatewayForProfile } from '@/store/gateway' import { notify, notifyError } from '@/store/notifications' import { $activeGatewayProfile, + ensureGatewayAgent, ensureGatewayProfile, newSessionInProfile, normalizeProfileKey, @@ -190,11 +191,13 @@ export const host = { }, /** Activate an agent's gateway (dialing it if needed) so subsequent - * host.request calls hit that agent's backend. The local source falls + * host.request calls hit that agent's backend. Goes through the store's + * serialized activation path so $connection / $activeGatewayProfile follow + * and rapid switches can't land out of order. The local source falls * through to the profile path — single-source plugins keep working * against older behavior unchanged. */ ensureAgent: async (connectionId: null | string, profile: string): Promise => - ensureGatewayForAgent(connectionId, (profile ?? '').trim() || 'default'), + ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default'), openSession: async ( storedSessionId: string, diff --git a/apps/desktop/src/store/profile-agent-activation.test.ts b/apps/desktop/src/store/profile-agent-activation.test.ts new file mode 100644 index 0000000000..5d79ccb30e --- /dev/null +++ b/apps/desktop/src/store/profile-agent-activation.test.ts @@ -0,0 +1,173 @@ +import { atom } from 'nanostores' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { HermesConnection } from '@/global' + +// Registry-agent activation (ensureGatewayAgent — the SDK ensureAgent door). +// Two regressions pinned here: +// 1. Activating an ALREADY-OPEN registry agent must still resync +// $connection (via getConnectionFor) and move $activeGatewayProfile — +// previously only a freshly-dialed socket synced $connection (inside +// openSecondary), so re-activating an open agent left REST/fs/media and +// image-attach routing on the previous backend (same class as #46651). +// 2. Agent activations share the gatewaySwitch mutex with profile switches — +// without it, two rapid activations could complete out of order and the +// EARLIER setActive() landed last. + +const ensureGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => undefined) +const ensureGatewayForProfile = vi.fn(async (_profile: string) => undefined) +const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) +const $gateway = atom({ id: 'live-socket' }) +const resetStarmapGraph = vi.fn() + +vi.mock('@/store/gateway', () => ({ + $gateway, + ensureGatewayForAgent, + ensureGatewayForProfile, + openGatewayForProfile +})) +vi.mock('@/hermes', () => ({ + getProfiles: vi.fn(async () => ({ profiles: [] })), + setApiRequestProfile: vi.fn() +})) +vi.mock('@/lib/query-client', () => ({ invalidateProfileScopedQueries: vi.fn() })) +vi.mock('@/store/starmap', () => ({ resetStarmapGraph })) + +const { $activeGatewayProfile, ensureGatewayAgent, ensureGatewayProfile } = await import('./profile') +const { $connection } = await import('./session') + +const agentConn = (over: Partial = {}): HermesConnection => + ({ baseUrl: 'https://homelab.invalid', mode: 'remote', profile: 'research', ...over }) as HermesConnection + +const localConn = (over: Partial = {}): HermesConnection => + ({ baseUrl: '', mode: 'local', profile: 'default', ...over }) as HermesConnection + +const getConnection = vi.fn<(profile?: string | null) => Promise>() +const getConnectionFor = vi.fn<(payload: { connectionId?: null | string; profile?: null | string }) => Promise>() + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void + + const promise = new Promise(r => { + resolve = r + }) + + return { promise, resolve } +} + +beforeEach(() => { + getConnection.mockReset() + getConnectionFor.mockReset() + ensureGatewayForAgent.mockClear() + ensureGatewayForProfile.mockClear() + $gateway.set({ id: 'live-socket' }) + $activeGatewayProfile.set('default') + $connection.set(localConn()) + vi.stubGlobal('window', { hermesDesktop: { getConnection, getConnectionFor } }) +}) + +afterEach(() => { + vi.unstubAllGlobals() + $connection.set(null) +}) + +describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () => { + it('resyncs $connection and $activeGatewayProfile even when the agent socket is already open', async () => { + // The store-level activation resolves instantly (socket already open) — + // exactly the case that used to skip the sync entirely. + getConnectionFor.mockResolvedValue(agentConn()) + + await ensureGatewayAgent('homelab', 'research') + + expect(ensureGatewayForAgent).toHaveBeenCalledWith('homelab', 'research') + expect(getConnectionFor).toHaveBeenCalledWith({ connectionId: 'homelab', profile: 'research' }) + expect($activeGatewayProfile.get()).toBe('research') + expect($connection.get()?.mode).toBe('remote') + expect($connection.get()?.profile).toBe('research') + }) + + it('leaves the prior connection intact when the descriptor fetch fails', async () => { + getConnectionFor.mockRejectedValue(new Error('source unreachable')) + + await ensureGatewayAgent('homelab', 'research') + + expect($activeGatewayProfile.get()).toBe('research') + // Best-effort: boot/reconnect resyncs later; we must not null it out here. + expect($connection.get()?.mode).toBe('local') + }) + + it('falls through to the profile path for a local/null connectionId', async () => { + getConnection.mockResolvedValue(agentConn({ mode: 'local', profile: 'research' })) + + await ensureGatewayAgent(null, 'research') + + expect(ensureGatewayForProfile).toHaveBeenCalledWith('research') + expect(ensureGatewayForAgent).not.toHaveBeenCalled() + expect(getConnectionFor).not.toHaveBeenCalled() + }) +}) + +describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switches', () => { + it('serializes an agent activation behind an in-flight profile switch', async () => { + const profileGate = deferred() + const order: string[] = [] + + ensureGatewayForProfile.mockImplementation(async (profile: string) => { + order.push(`profile:${profile}`) + await profileGate.promise + }) + ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => { + order.push(`agent:${profile}`) + }) + getConnection.mockResolvedValue(localConn({ profile: 'worker' })) + getConnectionFor.mockResolvedValue(agentConn()) + + // Start a profile switch that stalls mid-flight, then an agent + // activation. The agent activation must NOT start until the profile + // switch settles — otherwise the earlier setActive could land last. + const profileSwitch = ensureGatewayProfile('worker') + await Promise.resolve() + const agentSwitch = ensureGatewayAgent('homelab', 'research') + await Promise.resolve() + + expect(order).toEqual(['profile:worker']) + + profileGate.resolve() + await profileSwitch + await agentSwitch + + expect(order).toEqual(['profile:worker', 'agent:research']) + // The LAST activation wins the active pointer. + expect($activeGatewayProfile.get()).toBe('research') + expect($connection.get()?.profile).toBe('research') + }) + + it('serializes a profile switch behind an in-flight agent activation', async () => { + const agentGate = deferred() + const order: string[] = [] + + ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => { + order.push(`agent:${profile}`) + await agentGate.promise + }) + ensureGatewayForProfile.mockImplementation(async (profile: string) => { + order.push(`profile:${profile}`) + }) + getConnection.mockResolvedValue(localConn({ profile: 'worker' })) + getConnectionFor.mockResolvedValue(agentConn()) + + const agentSwitch = ensureGatewayAgent('homelab', 'research') + await Promise.resolve() + const profileSwitch = ensureGatewayProfile('worker') + await Promise.resolve() + + expect(order).toEqual(['agent:research']) + + agentGate.resolve() + await agentSwitch + await profileSwitch + + expect(order).toEqual(['agent:research', 'profile:worker']) + expect($activeGatewayProfile.get()).toBe('worker') + }) +}) diff --git a/apps/desktop/src/store/profile.test.ts b/apps/desktop/src/store/profile.test.ts index 49d6186a43..fbf9939643 100644 --- a/apps/desktop/src/store/profile.test.ts +++ b/apps/desktop/src/store/profile.test.ts @@ -7,11 +7,12 @@ import type { ProfileInfo } from '@/types/hermes' // Keep profile.ts's side-effecting imports inert: the gateway socket layer and // the REST query client must not run for real in a unit test. const ensureGatewayForProfile = vi.fn(async () => undefined) +const ensureGatewayForAgent = vi.fn(async () => undefined) const openGatewayForProfile = vi.fn(async (_profile: string) => undefined) const $gateway = atom({ id: 'live-socket' }) const resetStarmapGraph = vi.fn() -vi.mock('@/store/gateway', () => ({ $gateway, ensureGatewayForProfile, openGatewayForProfile })) +vi.mock('@/store/gateway', () => ({ $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile })) vi.mock('@/hermes', () => ({ getProfiles: vi.fn(async () => ({ profiles: [] })), setApiRequestProfile: vi.fn() diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index 3d8cf0a630..73ab65e5f5 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -1,3 +1,4 @@ +import { backendScopeKey } from '@hermes/shared' import { atom, computed } from 'nanostores' import { getProfiles, setApiRequestProfile, STARTUP_REQUEST_TIMEOUT_MS } from '@/hermes' @@ -12,7 +13,7 @@ import { storedStringRecord } from '@/lib/storage' import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' -import { $gateway, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway' +import { $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway' import { setConnection } from '@/store/session' import { resetStarmapGraph } from '@/store/starmap' import type { ProfileInfo } from '@/types/hermes' @@ -307,6 +308,66 @@ export async function ensureGatewayProfile(profile: string | null | undefined): } } +// Registry-aware sibling of syncConnectionToActiveProfile: a connection-scoped +// agent's descriptor comes from getConnectionFor (its SOURCE connection), not +// getConnection (the local pool). Same best-effort contract. +async function syncConnectionToActiveAgent(connectionId: string, profile: string): Promise { + const getConnectionFor = window.hermesDesktop?.getConnectionFor + + if (!getConnectionFor) { + return + } + + try { + setConnection(await getConnectionFor({ connectionId, profile })) + } catch { + // Leave the prior connection in place; boot/reconnect resyncs it later. + } +} + +// Activate a connection-scoped agent's gateway — the (connectionId, profile) +// analogue of ensureGatewayProfile, and the door the SDK's ensureAgent goes +// through. Two invariants the raw store call (ensureGatewayForAgent) does not +// provide on its own: +// - Every activation moves $activeGatewayProfile and resyncs $connection, +// exactly like the profile path — otherwise activating an ALREADY-OPEN +// registry agent left both describing the previous backend, routing +// /api/fs, /api/media and image.attach to the wrong machine (the same +// class as #46651) and pointing newSessionInProfile at the stale profile. +// - Activations share the gatewaySwitch mutex with profile switches, so a +// rapid agent↔profile (or agent↔agent) interleave can't finish out of +// order and leave the EARLIER setActive() as the last write. +// A local/null connectionId falls through to the profile path verbatim. +export async function ensureGatewayAgent(connectionId: null | string, profile: string): Promise { + const target = normalizeProfileKey(profile) + const connection = (connectionId ?? '').trim() || null + + if (!connection || backendScopeKey(connection, target) === target) { + return ensureGatewayProfile(target) + } + + // Serialize against any in-flight profile/agent switch (shared mutex). + if (gatewaySwitch) { + await gatewaySwitch.catch(() => undefined) + } + + $gatewaySwapTarget.set(target) + gatewaySwitch = (async () => { + await ensureGatewayForAgent(connection, target) + $activeGatewayProfile.set(target) + // The active backend just changed; resync $connection so remote-aware + // paths (image.attach_bytes vs image.attach, /api/fs/*, /api/media) follow. + await syncConnectionToActiveAgent(connection, target) + })() + + try { + await gatewaySwitch + } finally { + gatewaySwitch = null + $gatewaySwapTarget.set(null) + } +} + // ── Sidebar profile scope (the "workspace switcher" model) ───────────────── // Mirrors how Slack/VS Code/Linear do multi-context: you're "in" one profile at // a time and the sidebar shows only that profile's sessions (clean rows, no