From 90a0b346285afa179cd2fbcbcaea9be068349501 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:02:25 -0700 Subject: [PATCH] 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= expected= 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. --- .../plugins/hermes-bots/data.roster.test.tsx | 67 ++++++++++++++++++- apps/desktop/src/plugins/hermes-bots/data.ts | 28 ++++++-- 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/plugins/hermes-bots/data.roster.test.tsx b/apps/desktop/src/plugins/hermes-bots/data.roster.test.tsx index ba9679ed52..1c9ad85998 100644 --- a/apps/desktop/src/plugins/hermes-bots/data.roster.test.tsx +++ b/apps/desktop/src/plugins/hermes-bots/data.roster.test.tsx @@ -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 }) => ( + {children} + ) + + 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) + }) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/data.ts b/apps/desktop/src/plugins/hermes-bots/data.ts index 18323ce7ca..ae7bb22c83 100644 --- a/apps/desktop/src/plugins/hermes-bots/data.ts +++ b/apps/desktop/src/plugins/hermes-bots/data.ts @@ -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(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(route, 'profiles.list', {}) + : await requestForBot(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.