fix(desktop): profile-list ownership follows the published source, without a roster reconciler
The per-connection list cache is kept only to repaint $profiles on re-home. The $fleetRoster listener is dropped: a roster landing while the active source's own /api/profiles read was in flight invalidated that read, so $profiles stayed empty/stale after every switch or focus refresh that the roster IPC won. Both are reads of the same backend; neither is "older". A null descriptor is a reconnect blip (setConnection's contract) and keeps the current owner instead of blanking the rail; the first published descriptor adopts whatever list is already loaded. Legacy sources are keyed by endpoint rather than a JSON tuple. Tests cover the roster race and the null blip; the two use-session-actions tests now publish the descriptor before seeding $profiles, matching the runtime order.
This commit is contained in:
@@ -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') {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<string>('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<ProfileInfo[]>([])
|
||||
const NO_PROFILES: ProfileInfo[] = []
|
||||
export const $profiles = atom<ProfileInfo[]>(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<ReadonlyMap<string, ProfileInfo[]>>(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<ProfileInfo[]> {
|
||||
|
||||
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<ProfileInfo[]> {
|
||||
}
|
||||
|
||||
// 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 ─────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user