From 761ad9639f886f9207109098bb2154759cee775c Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Thu, 24 Sep 2026 18:57:43 -0500 Subject: [PATCH] fix(desktop): spin the bot row while a cold chat opens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a bot whose chat is not already a tab waits on source prep and the registry open before anything changes, so on a cold backend the click looked dead. openRosterBot now publishes $pendingBotOpen after the fronted-tab miss, and the row shows a GlyphSpinner with aria-busy and a translated "Opening chat…" label. The highlight still follows the chat on screen. Every exit settles the mark behind a generation guard, and bumpBotOpenGeneration clears it, so a group open, a return to Sessions, or the next click drops it. Closes #120277 Co-authored-by: finn763 <165816600+finn763@users.noreply.github.com> Co-authored-by: Vaibhav Arora --- .../src/plugins/hermes-bots/bot-row.tsx | 8 + .../src/plugins/hermes-bots/bot-state.ts | 1 + .../cold-bot-switch-pending.test.ts | 163 ++++++++++++++++++ .../hermes-bots/cold-bot-switch-row.test.tsx | 59 +++++++ apps/desktop/src/plugins/hermes-bots/i18n.ts | 6 + .../plugins/hermes-bots/plugin-panes.test.tsx | 20 +++ .../src/plugins/hermes-bots/roster-actions.ts | 33 +++- .../desktop/src/plugins/hermes-bots/shared.ts | 10 +- 8 files changed, 298 insertions(+), 2 deletions(-) create mode 100644 apps/desktop/src/plugins/hermes-bots/cold-bot-switch-pending.test.ts create mode 100644 apps/desktop/src/plugins/hermes-bots/cold-bot-switch-row.test.tsx diff --git a/apps/desktop/src/plugins/hermes-bots/bot-row.tsx b/apps/desktop/src/plugins/hermes-bots/bot-row.tsx index 2c528b898f..c5d24667ec 100644 --- a/apps/desktop/src/plugins/hermes-bots/bot-row.tsx +++ b/apps/desktop/src/plugins/hermes-bots/bot-row.tsx @@ -19,6 +19,7 @@ import { ContextMenuSubContent, ContextMenuSubTrigger, ContextMenuTrigger, + GlyphSpinner, haptic, host, queryClient, @@ -35,6 +36,7 @@ import { isBackfilledFacePng } from './avatar-image' import { $botChatFocused, $focusedBotOwner, + $pendingBotOpen, $selectedRosterKey, focusedRosterOwner, saveSelectedRosterBot @@ -116,6 +118,8 @@ export function BotRow({ bot, onDelete, onEdit, onGroup, onNewSection, showHandl const b = useBots() const focusedOwner = focusedRosterOwner(useValue($focusedBotOwner)) const selectedRosterKey = useValue($selectedRosterKey) + const pendingOpenKey = useValue($pendingBotOpen)?.key + const isOpening = pendingOpenKey === botRosterKey(bot) const botChatFocused = useValue($botChatFocused) const activeGroup = useValue($groupChatWorkspace) const allMeta = useValue($botMeta) @@ -236,6 +240,7 @@ export function BotRow({ bot, onDelete, onEdit, onGroup, onNewSection, showHandl const row = ( ) : null} + {isOpening ? ( + + ) : null} {rowAgeTs ? ( {rowAge(rowAgeTs * 1000, t.sidebar.row)} diff --git a/apps/desktop/src/plugins/hermes-bots/bot-state.ts b/apps/desktop/src/plugins/hermes-bots/bot-state.ts index 96fb48df71..840fea42e0 100644 --- a/apps/desktop/src/plugins/hermes-bots/bot-state.ts +++ b/apps/desktop/src/plugins/hermes-bots/bot-state.ts @@ -52,6 +52,7 @@ export const $botsPaneVisible = atom(false) * canonical chat was resolved). This transient view observation is never an * identity preference. */ export const $openBotChat = atom<{ key: string; openedRegistryId: string; openedSessionId?: string } | null>(null) +export { $pendingBotOpen } from './shared' /** A session owns the main workspace. The roster highlight and the Cronjobs * lifecycle both key off this rather than reading host.state conditionally * from render. */ diff --git a/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-pending.test.ts b/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-pending.test.ts new file mode 100644 index 0000000000..0fb6131d49 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-pending.test.ts @@ -0,0 +1,163 @@ +/** + * Cold bot switch acknowledges the clicked row before the backend answers + * (hermes-agent#120277). + * + * The mark is published only after the fronted-tab check misses, and before + * prepareBotSource. It is not chat ownership: highlight, routing, drafts, and + * running turns stay on their existing paths. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { RosterRow } from './types' + +const { openBotCanonicalChat, prepareBotSource } = vi.hoisted(() => ({ + openBotCanonicalChat: vi.fn(), + prepareBotSource: vi.fn() +})) + +vi.mock('./canonical-chat', () => ({ + CANONICAL_CHAT_TITLE: 'Bot Chat', + ensureBotMetadata: vi.fn(async () => ({})), + notifyBotOpenFailure: vi.fn(), + openBotCanonicalChat, + prepareBotSource, + PROFILE_SESSION_LIST_LIMIT: 200 +})) + +const { host } = await import('@hermes/plugin-sdk') +const { $openBotChat, $pendingBotOpen } = await import('./bot-state') +const { $groupChats, $groupChatWorkspace } = await import('./group-chat') +const { openGroupChat } = await import('./group-chat-view') +const { bumpBotOpenGeneration } = await import('./shared') +const { openRosterBot } = await import('./roster-actions') + +const botB = { + connectionId: 'local', + name: 'bravo', + canonical_session: { id: 'b-chat', resolved_id: 'b-tip' } +} as RosterRow + +const botA = { + connectionId: 'local', + name: 'alpha', + canonical_session: { id: 'a-chat', resolved_id: 'a-tip' } +} as RosterRow + +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + + const promise = new Promise((yes, no) => { + resolve = yes + reject = no + }) + + return { promise, resolve, reject } +} + +beforeEach(() => { + vi.clearAllMocks() + $openBotChat.set(null) + $pendingBotOpen.set(null) + $groupChats.set({}) + $groupChatWorkspace.set(null) + // @ts-expect-error — restore the harness default (no focus verb). + delete host.focusOpenWorkspaceSession +}) + +describe('cold bot switch publishes its target after the fronted-tab miss', () => { + it('marks the target before prepareBotSource, and not when a tab is already fronted', async () => { + const seenAtPrepare: Array = [] + + prepareBotSource.mockImplementation(() => { + seenAtPrepare.push($pendingBotOpen.get()?.key ?? null) + + return Promise.resolve() + }) + openBotCanonicalChat.mockResolvedValue({ openedId: 'b-tip', registryId: 'b-chat' }) + + host.focusOpenWorkspaceSession = vi.fn(() => 'a-tip') as never + await openRosterBot(botA) + + expect(prepareBotSource).not.toHaveBeenCalled() + expect($pendingBotOpen.get()).toBeNull() + + // @ts-expect-error — cold path: the shell cannot front a tab. + delete host.focusOpenWorkspaceSession + const flight = openRosterBot(botB) + + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + await flight + expect(seenAtPrepare).toEqual(['local::bravo']) + expect($pendingBotOpen.get()).toBeNull() + expect($openBotChat.get()?.openedSessionId).toBe('b-tip') + }) + + it('clears the mark when source prep fails, and a retry marks and clears again', async () => { + const gate = deferred() + + prepareBotSource.mockReturnValueOnce(gate.promise) + const flight = openRosterBot(botB) + + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + gate.reject(new Error('backend away')) + await expect(flight).resolves.toBe(false) + expect($pendingBotOpen.get()).toBeNull() + + const retryGate = deferred() + + prepareBotSource.mockReturnValueOnce(retryGate.promise) + openBotCanonicalChat.mockResolvedValueOnce({ openedId: 'b-tip', registryId: 'b-chat' }) + const retry = openRosterBot(botB) + + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + retryGate.resolve() + await expect(retry).resolves.toBe(true) + expect($pendingBotOpen.get()).toBeNull() + }) + + it('opening a group drops the mark at once, and the late flight cannot reclaim it', async () => { + const gate = deferred() + + prepareBotSource.mockReturnValueOnce(gate.promise) + const flight = openRosterBot(botB) + + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + $groupChats.set({ Team: { log: [], sessions: {}, watermarks: {} } }) + openGroupChat('Team') + expect($pendingBotOpen.get()).toBeNull() + + gate.resolve() + await expect(flight).resolves.toBe(false) + expect($pendingBotOpen.get()).toBeNull() + expect(openBotCanonicalChat).not.toHaveBeenCalled() + expect($groupChatWorkspace.get()).toBe('Team') + }) + + it('a superseded flight never clears its successor, and an external supersede drops the mark', async () => { + const gateA = deferred() + const gateB = deferred() + + prepareBotSource.mockReturnValueOnce(gateA.promise).mockReturnValueOnce(gateB.promise) + openBotCanonicalChat.mockResolvedValue({ openedId: 'b-tip', registryId: 'b-chat' }) + + const flightA = openRosterBot(botA) + + expect($pendingBotOpen.get()?.key).toBe('local::alpha') + + const flightB = openRosterBot(botB) + + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + gateA.resolve() + await flightA + expect($pendingBotOpen.get()?.key).toBe('local::bravo') + + bumpBotOpenGeneration() + expect($pendingBotOpen.get()).toBeNull() + gateB.resolve() + await flightB + expect($pendingBotOpen.get()).toBeNull() + expect($openBotChat.get()?.key).not.toBe('local::bravo') + }) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-row.test.tsx b/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-row.test.tsx new file mode 100644 index 0000000000..b737678ea5 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/cold-bot-switch-row.test.tsx @@ -0,0 +1,59 @@ +/** + * The pending cold-open mark is a spinner on the clicked row, not a new owner. + * Highlight still follows the chat on screen (hermes-agent#120277). + */ + +import type * as HermesSdk from '@hermes/plugin-sdk' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +import { BotRow } from './bot-row' +import { $botChatFocused, $pendingBotOpen, $selectedRosterKey } from './bot-state' +import { $groupChatWorkspace } from './group-chat' +import { translateBotsIn } from './i18n-test-helper' +import type { RosterRow } from './types' + +vi.mock('@hermes/plugin-sdk', async importOriginal => { + const sdk = await importOriginal() + + return { + ...sdk, + usePluginI18n: () => translateBotsIn('en') + } +}) + +const noop = () => undefined + +const alpha = { connectionId: 'local', name: 'alpha' } as RosterRow +const bravo = { connectionId: 'local', name: 'bravo' } as RosterRow + +beforeEach(() => { + $groupChatWorkspace.set(null) + $botChatFocused.set(false) + $selectedRosterKey.set('local::alpha') + $pendingBotOpen.set({ generation: 1, key: 'local::bravo' }) +}) + +afterEach(() => { + $pendingBotOpen.set(null) + $selectedRosterKey.set('') + cleanup() +}) + +it('spins the pending target row without stealing the highlight', () => { + render( + <> + + + + ) + + const [alphaRow, bravoRow] = screen.getAllByRole('button') + + expect(bravoRow.getAttribute('aria-busy')).toBe('true') + expect(bravoRow.querySelector('[role="status"]')?.getAttribute('aria-label')).toBe('Opening chat…') + expect(alphaRow.getAttribute('aria-busy')).not.toBe('true') + expect(alphaRow.querySelector('[role="status"]')).toBeNull() + expect(alphaRow.className).toContain('bg-(--ui-row-active-background)') + expect(bravoRow.className).not.toContain('bg-(--ui-row-active-background)') +}) diff --git a/apps/desktop/src/plugins/hermes-bots/i18n.ts b/apps/desktop/src/plugins/hermes-bots/i18n.ts index ea116ee5af..c4c270a593 100644 --- a/apps/desktop/src/plugins/hermes-bots/i18n.ts +++ b/apps/desktop/src/plugins/hermes-bots/i18n.ts @@ -174,6 +174,8 @@ type BotsMessages = { /** Re-opens the forever-chat on purpose. A plain row click only returns to * the tabs already open, so a closed Bot Chat needs an explicit ask. */ openBotChat: string + /** Screen-reader label for the row spinner while a cold bot chat opens. */ + openingChat: string /** Row context menu: pin/hide toggles, their toasts, and the groups entry. */ pinToTop: string unpin: string @@ -617,6 +619,7 @@ const en: BotsMessages = { descriptionHint: 'Leave blank to generate from the bot’s name and description.', newChatWith: 'New chat with this bot', openBotChat: 'Open Bot Chat', + openingChat: 'Opening chat…', pinToTop: 'Pin to top', unpin: 'Unpin', pinnedToast: name => `${name} pinned to top`, @@ -1046,6 +1049,7 @@ const ja: BotsMessages = { descriptionHint: '空欄のままにすると、ボットの名前と説明から生成します。', newChatWith: 'このボットと新しいチャット', openBotChat: 'ボットチャットを開く', + openingChat: 'チャットを開いています…', pinToTop: '先頭にピン留め', unpin: 'ピン留めを解除', pinnedToast: name => `${name}を先頭にピン留めしました`, @@ -1468,6 +1472,7 @@ const zh: BotsMessages = { descriptionHint: '留空则根据机器人的名称和描述生成。', newChatWith: '与此机器人开新聊天', openBotChat: '打开机器人聊天', + openingChat: '正在打开聊天…', pinToTop: '置顶', unpin: '取消置顶', pinnedToast: name => `已将 ${name} 置顶`, @@ -1884,6 +1889,7 @@ const zhHant: BotsMessages = { descriptionHint: '留空則依機器人的名稱和描述產生。', newChatWith: '與此機器人開新聊天', openBotChat: '開啟機器人聊天', + openingChat: '正在開啟聊天…', pinToTop: '釘選到頂端', unpin: '取消釘選', pinnedToast: name => `已將 ${name} 釘選到頂端`, diff --git a/apps/desktop/src/plugins/hermes-bots/plugin-panes.test.tsx b/apps/desktop/src/plugins/hermes-bots/plugin-panes.test.tsx index ac07b81e72..16445869e1 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin-panes.test.tsx +++ b/apps/desktop/src/plugins/hermes-bots/plugin-panes.test.tsx @@ -305,6 +305,26 @@ describe('the Scheduled jobs pane', () => { }) }) +describe('returning to Sessions', () => { + it('drops a cold bot open still pending (#120277)', async () => { + const store = paneStores() + const harness = recordingContext() + const { $pendingBotOpen } = await import('./shared') + + plugin.register(harness.ctx) + await settle() + store(`hermes-bots:pane`).set(true) + $pendingBotOpen.set({ generation: 1, key: 'local::bravo' }) + + store(`hermes-bots:pane`).set(false) + + expect($pendingBotOpen.get()).toBeNull() + expect(mocks.setWorkspaceScope).toHaveBeenCalledWith('sessions') + + harness.dispose() + }) +}) + describe('a desktop without host.paneVisibility', () => { it('keeps the always-registered pane', async () => { const { host } = await import('@hermes/plugin-sdk') diff --git a/apps/desktop/src/plugins/hermes-bots/roster-actions.ts b/apps/desktop/src/plugins/hermes-bots/roster-actions.ts index 4f8bc99315..d2ddec8139 100644 --- a/apps/desktop/src/plugins/hermes-bots/roster-actions.ts +++ b/apps/desktop/src/plugins/hermes-bots/roster-actions.ts @@ -10,7 +10,14 @@ import { ackStoredSessionId, atom, haptic, host, markSessionUnreadFinished } from '@hermes/plugin-sdk' -import { $openBotChat, $selectedBot, lastToastedPreview, rosterWatermarks, saveSelectedRosterBot } from './bot-state' +import { + $openBotChat, + $pendingBotOpen, + $selectedBot, + lastToastedPreview, + rosterWatermarks, + saveSelectedRosterBot +} from './bot-state' import { CANONICAL_CHAT_TITLE, notifyBotOpenFailure, openBotCanonicalChat, prepareBotSource } from './canonical-chat' import { $botMeta, botActivitySession, botRosterKey, botSelectionKey, newBotChat } from './data' import { $groupChats, $groupChatWorkspace } from './group-chat' @@ -147,6 +154,14 @@ function refreshOpenBotChat(bot: RosterRow, { allowWhileBusy = false }: { allowW }) } +/** Release the pending-open mark, but only for the flight that set it: a + * superseded flight settling late must not clear its successor's mark. */ +function settlePendingBotOpen(generation: number) { + if ($pendingBotOpen.get()?.generation === generation) { + $pendingBotOpen.set(null) + } +} + /** Front the bot's canonical Bot Chat when it is ALREADY open as a tab — * presentation only, no registry round-trip. Returns the fronted stored id, * or null when the chat is not on screen (or this shell cannot tell) and the @@ -274,6 +289,11 @@ export async function openRosterBot(bot: RosterRow): Promise { return true } + // The click missed an already-open tab. Publish the target before the cold + // backend start so the row can acknowledge it in this same turn (#120277). + // Highlight, routing, drafts, and running turns are unchanged. + $pendingBotOpen.set({ generation, key }) + try { // Activation selects this row's source only. Canonical identity is resolved // after that by the owner profile's "Bot Chat" title registry. @@ -285,10 +305,14 @@ export async function openRosterBot(bot: RosterRow): Promise { notifyBotOpenFailure(error, bot, 'reach') } + settlePendingBotOpen(generation) + return false } if (generation !== getBotOpenGeneration()) { + settlePendingBotOpen(generation) + return false } @@ -296,6 +320,8 @@ export async function openRosterBot(bot: RosterRow): Promise { const opened = await openBotCanonicalChat(bot, () => generation === getBotOpenGeneration()) if (generation !== getBotOpenGeneration()) { + settlePendingBotOpen(generation) + return false } @@ -312,6 +338,7 @@ export async function openRosterBot(bot: RosterRow): Promise { openedRegistryId: opened.registryId, openedSessionId: opened.openedId }) + settlePendingBotOpen(generation) return true } @@ -322,6 +349,8 @@ export async function openRosterBot(bot: RosterRow): Promise { notifyBotOpenFailure(error, bot, 'open', displayName(bot, meta)) } + settlePendingBotOpen(generation) + return false } @@ -330,6 +359,7 @@ export async function openRosterBot(bot: RosterRow): Promise { if (typeof host.newChat !== 'function') { $openBotChat.set(null) restorePreviousGroup() + settlePendingBotOpen(generation) return false } @@ -339,6 +369,7 @@ export async function openRosterBot(bot: RosterRow): Promise { openedRegistryId: '' }) newBotChat(bot) + settlePendingBotOpen(generation) return true } diff --git a/apps/desktop/src/plugins/hermes-bots/shared.ts b/apps/desktop/src/plugins/hermes-bots/shared.ts index 2eb8ea8a69..f64552856d 100644 --- a/apps/desktop/src/plugins/hermes-bots/shared.ts +++ b/apps/desktop/src/plugins/hermes-bots/shared.ts @@ -8,7 +8,7 @@ * `setPluginCtx`, and every reader goes through `getPluginCtx()`. */ -import type { PluginContext } from '@hermes/plugin-sdk' +import { atom, type PluginContext } from '@hermes/plugin-sdk' export const ID = 'hermes-bots' @@ -28,11 +28,19 @@ export function setPluginCtx(ctx: PluginContext | null) { * live in the same module. */ let botOpenGeneration = 0 +/** Cold open still in flight. Published after the fronted-tab miss and before + * source prep. Not chat ownership — never route or highlight from this key. + * The generation guards the clear: a superseded flight never releases its + * successor's mark. A bump (another open, a group, or Sessions) drops it. */ +export const $pendingBotOpen = atom(null) + export function getBotOpenGeneration() { return botOpenGeneration } /** Invalidate every in-flight open; returns the generation that now owns it. */ export function bumpBotOpenGeneration() { + $pendingBotOpen.set(null) + return ++botOpenGeneration }