fix(desktop): Bots roster only caches rows from the connection it is keyed under
With a local and a remote backend that both run profile `default`, the Bots pane showed the remote bot's title on "This device" (and the remote profiles as "Status unknown" ghost rows) after a primary-connection switch, and mergeServerMeta could carry that title into bot-meta-v2. fetchRosterSnapshot binds activeConnectionId when it is called, but it sent profiles.list over the ambient socket resolved later. A retry or late run of a query keyed to the connection the window just left was answered by the backend it switched to and cached under the old key. Live trace: published connection=vps [bot-roster] MISROUTED active=local answered=<vps install> expected=<local install> Now the fetch refuses when its key is no longer the published connection (stale retries never send), and asks the keyed connection by its exact (connection, profile) route instead of the ambient socket. Without an exact route (older Desktop host) it keeps the ambient request.
This commit is contained in:
@@ -25,7 +25,7 @@ import { renderHook, waitFor } from '@testing-library/react'
|
||||
import type { ReactNode } from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { $lastRoster, useRoster } from './data'
|
||||
import { $lastRoster, ROSTER_KEY, useRoster } from './data'
|
||||
import type { RosterRow } from './types'
|
||||
|
||||
const { hostMock } = vi.hoisted(() => ({
|
||||
@@ -633,3 +633,68 @@ describe('a stalled profiles.list cannot pin the spinner forever', () => {
|
||||
expect(hostMock.request.mock.calls.length).toBeGreaterThan(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('the roster only caches rows from the connection it is keyed under', () => {
|
||||
const routes = [
|
||||
{ connectionId: 'local', mode: 'local', primary: true, profile: 'default', targetProfile: 'default' },
|
||||
{ connectionId: 'vps', mode: 'remote', profile: 'default', targetProfile: 'default' }
|
||||
]
|
||||
|
||||
const vpsList = { profiles: [{ display_name: 'Agent A', name: 'default' }] }
|
||||
const localList = { profiles: [{ display_name: 'Agent B', name: 'default' }] }
|
||||
|
||||
function renderRoster() {
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } })
|
||||
|
||||
const wrapper = ({ children }: { children: ReactNode }) => (
|
||||
<QueryClientProvider client={client}>{children}</QueryClientProvider>
|
||||
)
|
||||
|
||||
return { client, result: renderHook(() => useRoster(), { wrapper }).result }
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
hostMock.profileRoutes = undefined
|
||||
})
|
||||
|
||||
it("asks the keyed connection by route, not an ambient socket that sits on another machine", async () => {
|
||||
hostMock.state.connectionId.get.mockReturnValue('vps')
|
||||
hostMock.agents.mockRejectedValue(new Error('no union roster on this build'))
|
||||
hostMock.profileRoutes = vi.fn(async () => routes)
|
||||
hostMock.request.mockResolvedValue(localList)
|
||||
hostMock.requestProfile.mockImplementation(async (route: { connectionId: string }) =>
|
||||
route.connectionId === 'vps' ? vpsList : localList
|
||||
)
|
||||
|
||||
const { result } = renderRoster()
|
||||
|
||||
await waitFor(() => expect(result.current.data).toBeTruthy())
|
||||
expect(result.current.data?.profiles?.map(row => row.display_name)).toEqual(['Agent A'])
|
||||
expect(hostMock.request).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('never answers a fetch whose connection was switched away mid-flight', async () => {
|
||||
// Keyed to 'local' at render; the window switches to the VPS while the
|
||||
// fetch is underway, so whatever socket answers now is not 'local'.
|
||||
let published = 'local'
|
||||
hostMock.state.connectionId.get.mockImplementation(() => published)
|
||||
hostMock.agents.mockRejectedValue(new Error('no union roster on this build'))
|
||||
hostMock.profileRoutes = vi.fn(async () => {
|
||||
published = 'vps'
|
||||
|
||||
return routes
|
||||
})
|
||||
hostMock.request.mockResolvedValue(vpsList)
|
||||
hostMock.requestProfile.mockResolvedValue(vpsList)
|
||||
|
||||
const { client } = renderRoster()
|
||||
const localKey = [...ROSTER_KEY, 'local']
|
||||
|
||||
// The fetch and its bounded retries (1s, 2s) all refuse: none may send a
|
||||
// request, so the VPS answer is never cached as this device's roster.
|
||||
await waitFor(() => expect(client.getQueryState(localKey)?.status).toBe('error'), { timeout: 6000 })
|
||||
expect(client.getQueryData(localKey)).toBeUndefined()
|
||||
expect(hostMock.request).not.toHaveBeenCalled()
|
||||
expect(hostMock.requestProfile.mock.calls.every(([route]) => route.connectionId === 'vps')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -695,23 +695,43 @@ async function fetchRosterSnapshot(activeConnectionId: null | string | undefined
|
||||
// keep its configured friendly identity after activation (#89131).
|
||||
// Best-effort and feature-detected — a failed read keeps the last
|
||||
// good index rather than dropping identities mid-session.
|
||||
let routes: ProfileRoute[] = []
|
||||
|
||||
if (typeof host.profileRoutes === 'function') {
|
||||
const epoch = beginAliasRouteIndex()
|
||||
|
||||
try {
|
||||
indexAliasRoutes(await host.profileRoutes(), epoch)
|
||||
routes = await host.profileRoutes()
|
||||
indexAliasRoutes(routes, epoch)
|
||||
} catch {
|
||||
/* keep the previous alias index */
|
||||
}
|
||||
}
|
||||
|
||||
// Owner routing is ambient in the SDK now (post-#92731): requestForBot
|
||||
// resolves the active owner itself, no captured route needed here.
|
||||
// The rows are cached under `activeConnectionId`, so they must come from
|
||||
// that connection. A retry or late run of a query keyed to the connection
|
||||
// the window just switched away from would otherwise be answered by the
|
||||
// one it switched to and cached under the old key — the cross-machine
|
||||
// swap that relabelled a VPS bot with the local bot's title.
|
||||
if ((host.state.connectionId?.get?.() || null) !== (activeConnectionId || null)) {
|
||||
throw new Error('Bot roster query outlived its connection')
|
||||
}
|
||||
|
||||
const activeBot = {
|
||||
name: String(host.state.profile?.get?.() || 'default').trim() || 'default'
|
||||
}
|
||||
|
||||
const local = await requestForBot<RosterSnapshot>(activeBot, 'profiles.list', {})
|
||||
// Ask the keyed connection by name rather than the ambient socket, which
|
||||
// can sit on another machine (a secondary foregrounded across a connection
|
||||
// apply). No exact route (older Desktop host) → the ambient owner.
|
||||
const route = Array.isArray(routes)
|
||||
? routes.find(r => r?.connectionId === activeConnectionId && r?.profile === activeBot.name)
|
||||
: undefined
|
||||
|
||||
const local = route
|
||||
? await host.requestProfile<RosterSnapshot>(route, 'profiles.list', {})
|
||||
: await requestForBot<RosterSnapshot>(activeBot, 'profiles.list', {})
|
||||
|
||||
// Newer backends inject the teammate-messaging protocol into every
|
||||
// session's system prompt (agent.bot_mode_protocol) — SOUL.md must not
|
||||
// carry a second copy. Older gateways lack the flag: keep appending.
|
||||
|
||||
Reference in New Issue
Block a user