diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 8d46e2d345..935c65df91 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -4187,8 +4187,8 @@ describe('openNewSessionTile workspace target', () => { it('keeps an unlisted named local legacy-profile tile owned by its bare profile', async () => { const storedSessionId = 'stored-unlisted-omar' - $profiles.set([{ name: 'default' }, { name: 'omar' }] as never) setConnection({ mode: 'local' } as never) + $profiles.set([{ name: 'default' }, { name: 'omar' }] as never) const requestGateway = vi.fn(async (method: string) => { if (method === 'session.create') { @@ -4234,10 +4234,10 @@ describe('openNewSessionTile workspace target', () => { it('records the draft profile owner when tab-strip create omits profile', async () => { const storedSessionId = 'stored-unlisted-draft-omar' + setConnection({ mode: 'local' } as never) $profiles.set([{ name: 'default' }, { name: 'omar' }] as never) $newChatProfile.set('omar') $activeGatewayProfile.set('default') - setConnection({ mode: 'local' } as never) const requestGateway = vi.fn(async (method: string) => { if (method === 'session.create') { diff --git a/apps/desktop/src/store/profile-cache.test.ts b/apps/desktop/src/store/profile-cache.test.ts index cdda7b52d2..2e46a39c8e 100644 --- a/apps/desktop/src/store/profile-cache.test.ts +++ b/apps/desktop/src/store/profile-cache.test.ts @@ -2,9 +2,8 @@ import { atom } from 'nanostores' import { afterEach, expect, it, vi } from 'vitest' import { setApiRequestConnection } from '@/api/client' -import { buildRestGroups } from '@/app/chat/sidebar/fleet-rail' -import type { DesktopAgentRoster, DesktopRegistryConnection, HermesConnection } from '@/global' -import { $fleetRoster, _resetFleetRosterForTests, refreshFleetRoster } from '@/store/fleet-roster' +import type { DesktopAgentRoster, HermesConnection } from '@/global' +import { $fleetRoster, _resetFleetRosterForTests } from '@/store/fleet-roster' import type { ProfileInfo } from '@/types/hermes' vi.mock('@/store/gateway', () => ({ $gateway: atom(null) })) @@ -73,98 +72,49 @@ it('keeps failed incoming profile reads isolated while retaining the outgoing co expect($profiles.get()).toBe(outgoing) }) -it('lets only a subsequent successful roster supersede cached rail names', async () => { - const connections: DesktopRegistryConnection[] = ['source-a', 'source-b'].map(id => ({ - id, - kind: 'remote', - label: id, - tokenSet: false, - tokenPreview: null - })) - - const roster = (names: string[], reachable = true): DesktopAgentRoster => ({ - agents: names.map(name => ({ - connectionId: 'source-a', - connectionKind: 'remote', - connectionLabel: 'source-a', - profile: name, - handle: `${name}-source-a` - })), - sources: connections.map(connection => ({ - connectionId: connection.id, - kind: connection.kind, - label: connection.label, - reachable: connection.id === 'source-a' ? reachable : true - })) - }) - - const restNames = () => - buildRestGroups({ - activeConnectionId: $connection.get()?.connectionId ?? null, - connections, - roster: $fleetRoster.get(), - profilesByConnection: $profilesByConnection.get() - }) - .find(group => group.connectionId === 'source-a')! - .named.map(agent => agent.profile) - - const outgoing = [profile('default'), profile('old')] - const incoming = [profile('default'), profile('builder')] - const api = vi.fn().mockResolvedValue({ profiles: outgoing }) - const getAgentRoster = vi.fn().mockResolvedValue(roster([])) - vi.stubGlobal('window', { hermesDesktop: { api, getAgentRoster } }) - await refreshFleetRoster({ force: true }) - activate('source-a') - await refreshProfiles() - activate('source-b') - api.mockResolvedValue({ profiles: incoming }) - await refreshProfiles() - expect(restNames()).toEqual(['old']) // The older roster must not hide a newly discovered profile. - - getAgentRoster.mockRejectedValueOnce(new Error('enumeration failed')) - vi.spyOn(console, 'warn').mockImplementation(() => undefined) - await refreshFleetRoster({ force: true }) - expect(restNames()).toEqual(['old']) - getAgentRoster.mockResolvedValue(roster([], false)) - await refreshFleetRoster({ force: true }) - expect(restNames()).toEqual(['old']) // A partial roster failure is not a deletion. - expect($profiles.get()).toBe(incoming) - - for (const error of ['HTTP 401', 'connect-on-demand']) { - // The main-process registry restores cached names but retains the probe error. - const cachedFailure = roster(['stale'], true) - cachedFailure.sources[0].error = error - getAgentRoster.mockResolvedValue(cachedFailure) - await refreshFleetRoster({ force: true }) - expect(restNames()).toEqual(['old']) // Cached names are reachable, not freshly enumerated. - } - - for (const names of [['renamed'], ['created', 'renamed'], []]) { - getAgentRoster.mockResolvedValue(roster(names)) - await refreshFleetRoster({ force: true }) - expect(restNames()).toEqual(names) - expect($profiles.get()).toBe(incoming) - } - - // A roster refresh while this source is active must not clear its foreground - // list, but an older HTTP response must not resurrect its invalidated cache. - activate('source-a') - api.mockResolvedValue({ profiles: outgoing }) - await refreshProfiles() +it('lets a fresh active-source list land even when the fleet roster arrives first', async () => { + const list = [profile('default'), profile('writer')] let resolve!: (value: { profiles: ProfileInfo[] }) => void - api.mockReturnValueOnce( - new Promise(done => { - resolve = done - }) - ) - const stale = refreshProfiles() - getAgentRoster.mockResolvedValue(roster(['latest'])) - await refreshFleetRoster({ force: true }) - expect($profiles.get()).toBe(outgoing) - resolve({ profiles: outgoing }) - await stale - activate('source-b') - expect(restNames()).toEqual(['latest']) + const api = vi.fn(() => new Promise<{ profiles: ProfileInfo[] }>(done => (resolve = done))) + vi.stubGlobal('window', { hermesDesktop: { api } }) + activate('source-a') + + const flight = refreshProfiles() + + // After a switch the registry change forces a roster refresh that races + // the active source's own list read; both are reads of the same backend. + const roster: DesktopAgentRoster = { + agents: [], + sources: [{ connectionId: 'source-a', kind: 'remote', label: 'source-a', reachable: true }] + } + + $fleetRoster.set(roster) + resolve({ profiles: list }) + await flight + + expect($profiles.get()).toBe(list) + expect($profilesByConnection.get().get('source-a')).toBe(list) +}) + +it('treats a null descriptor as a reconnect blip, not a source change', async () => { + const list = [profile('default'), profile('writer')] + const api = vi.fn(async () => ({ profiles: list })) + vi.stubGlobal('window', { hermesDesktop: { api } }) + activate('source-a') + await refreshProfiles() + + let resolve!: (value: { profiles: ProfileInfo[] }) => void + api.mockReturnValueOnce(new Promise(done => (resolve = done))) + const flight = refreshProfiles() + $connection.set(null) // failed reconnect attempt publishes null + expect($profiles.get()).toBe(list) + const refreshed = [profile('default'), profile('writer'), profile('editor')] + resolve({ profiles: refreshed }) + await flight + + expect($profiles.get()).toBe(refreshed) + activate('source-a') + expect($profiles.get()).toBe(refreshed) }) it('strands a retry during a same-profile source change without retargeting it to the incoming source', async () => { diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index c397930a96..49dbf6c71d 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -15,7 +15,6 @@ import { } from '@/lib/storage' import { withTimeout } from '@/lib/with-timeout' import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope' -import { $fleetRoster } from '@/store/fleet-roster' import { $gateway, activeGatewayConnectionId, @@ -57,15 +56,23 @@ export const $activeProfile = atom('default') // Cached profile list for the picker. Refreshed lazily; the dropdown also // re-fetches on open so a profile created elsewhere shows up. -export const $profiles = atom([]) +const NO_PROFILES: ProfileInfo[] = [] +export const $profiles = atom(NO_PROFILES) // Successful lists belong to their source, not whichever gateway is active -// when a rail renders. Keep them on re-home; a failed incoming read must not -// borrow the outgoing source's profiles (or discard its cached squares). +// when a rail renders. A re-home repaints from this cache until the incoming +// source serves its own list, so a failed incoming read can neither borrow the +// outgoing source's profiles nor blank a source we already know. export const $profilesByConnection = atom>(new Map()) -function profileListSource(connection: HermesConnection | null): string { - return connection?.connectionId ?? JSON.stringify(['legacy', connection?.mode, connection?.baseUrl]) +// Registry descriptors carry their connection id; legacy primaries are keyed +// by endpoint. Null is a reconnect blip (see setConnection), not a source. +function profileListSource(connection: HermesConnection | null): null | string { + if (!connection) { + return null + } + + return connection.connectionId ?? `${connection.mode ?? 'local'}:${connection.baseUrl}` } export function setActiveProfile(name: string): void { @@ -114,7 +121,10 @@ export function refreshProfiles(): Promise { if (epoch === profileListEpoch) { batch(() => { - $profilesByConnection.set(new Map($profilesByConnection.get()).set(source, profiles)) + if (source !== null) { + $profilesByConnection.set(new Map($profilesByConnection.get()).set(source, profiles)) + } + $profiles.set(profiles) }) } @@ -157,43 +167,27 @@ export function refreshProfiles(): Promise { } // Source changes can keep the same profile name (default → default), including -// direct agent activations that never run the connection-switch wipe. -let profileListOwner = profileListSource($connection.get()) +// direct agent activations that never run the connection-switch wipe. The +// first published descriptor adopts whatever list is already loaded; a null +// descriptor is a reconnect blip and keeps the current owner (setConnection). +let profileListOwner: null | string = null $connection.subscribe(connection => { const source = profileListSource(connection) - if (source === profileListOwner) { + if (source === null || source === profileListOwner) { return } + const adopting = profileListOwner === null profileListOwner = source + + if (adopting) { + return + } + invalidateProfileListFetches() - $profiles.set($profilesByConnection.get().get(source) ?? []) -}) - -// Newly published successful enumerations supersede older per-source lists. -// Do not consult the existing roster on re-home: it may predate the cached list. -$fleetRoster.listen(roster => { - const cached = $profilesByConnection.get() - const next = new Map(cached) - - for (const source of roster?.sources ?? []) { - // Main can mark cached names reachable even when enumeration failed. - if (source.reachable && !source.error) { - next.delete(source.connectionId) - - // A pending older read must not repopulate the cache after invalidation. - if (source.connectionId === profileListOwner && refreshInFlight) { - invalidateProfileListFetches() - } - } - } - - if (next.size !== cached.size) { - $profilesByConnection.set(next) - } - // Leave the active $profiles view alone; the inactive rail falls back to roster. + $profiles.set($profilesByConnection.get().get(source) ?? NO_PROFILES) }) // ── Rail order ─────────────────────────────────────────────────────────────