fix(desktop): sync connection atoms and share the switch mutex for agent activation
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.
This commit is contained in:
@@ -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<void> =>
|
||||
ensureGatewayForAgent(connectionId, (profile ?? '').trim() || 'default'),
|
||||
ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default'),
|
||||
|
||||
openSession: async (
|
||||
storedSessionId: string,
|
||||
|
||||
173
apps/desktop/src/store/profile-agent-activation.test.ts
Normal file
173
apps/desktop/src/store/profile-agent-activation.test.ts
Normal file
@@ -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<unknown>({ 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> = {}): HermesConnection =>
|
||||
({ baseUrl: 'https://homelab.invalid', mode: 'remote', profile: 'research', ...over }) as HermesConnection
|
||||
|
||||
const localConn = (over: Partial<HermesConnection> = {}): HermesConnection =>
|
||||
({ baseUrl: '', mode: 'local', profile: 'default', ...over }) as HermesConnection
|
||||
|
||||
const getConnection = vi.fn<(profile?: string | null) => Promise<HermesConnection>>()
|
||||
const getConnectionFor = vi.fn<(payload: { connectionId?: null | string; profile?: null | string }) => Promise<HermesConnection>>()
|
||||
|
||||
function deferred(): { promise: Promise<void>; resolve: () => void } {
|
||||
let resolve!: () => void
|
||||
|
||||
const promise = new Promise<void>(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')
|
||||
})
|
||||
})
|
||||
@@ -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<unknown>({ 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()
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user