From ad08688bc68bc97695701c484e2c32b6408342f3 Mon Sep 17 00:00:00 2001 From: sprmn24 Date: Wed, 2 Sep 2026 19:29:00 +0300 Subject: [PATCH 1/4] fix(bot-mode): derive group clarify/approval attention from $groupClarify instead of duplicating it into $groupNeedsYou $groupNeedsYou was written by two independent sources: an @user mention (appendGroupChatEntry) and, until now, syncGroupClarify whenever a member blocked on a clarify or approval. Nothing kept the two in sync, so every path that consumed a clarify -- answering it, the server resolving it, disbanding or renaming the room -- left a stale badge lit with nothing behind it, because none of them cleared the copy syncGroupClarify had written. A naive fix (writing false back into $groupNeedsYou on every clarify cleanup path) would have created the opposite bug: clearing an unrelated, still-unresolved @user mention. $groupClarify is already the source of truth for pending clarify/ approval attention. syncGroupClarify no longer writes $groupNeedsYou; a new groupHasPendingClarify(clarifies, group) derives the same signal from a $groupClarify snapshot. It's intentionally pure -- the caller (roster-pane) subscribes to $groupClarify itself via useValue and passes the live snapshot in, so the subscription actually drives the recalculation instead of existing only to force a re-render. A prompt resolving, being answered, or its room being disbanded all self-correct through the existing $groupClarify cleanup paths, with nothing left to keep in sync. Rename needed its own fix: the old clearGroupClarify(oldName) call on rename dropped a clarify-only room's attention entirely, since clarify no longer lives in $groupNeedsYou to be carried over by the existing old->new key swap there. New renameGroupClarify(oldName, newName) re-keys matching mirrors onto the new name in two passes -- unrelated entries first, migrated entries last -- so a stale mirror already stranded at the destination key (left behind by a known, separate in-flight-poll race) can never clobber the just-migrated current prompt. The roster now reads groupNeedsYou[group] || groupHasPendingClarify(...) and subscribes to both stores so either one repaints the row. Tests: multi-clarify sequencing, mention+clarify independence (resolving one must not clear the other), server-side resolution cleanup, rename migration, rename destination-collision (red-green verified against the prior single-pass implementation), a GroupRow badge render test, and a subscription harness using the real stores/useValue/helper end-to-end. (cherry picked from commit e5038f12ccc1a4d1fb56ecc5f0e1a884f1e8b795) --- .../src/plugins/hermes-bots/bot-row.test.tsx | 22 ++- .../plugins/hermes-bots/group-chat-view.tsx | 8 +- .../plugins/hermes-bots/group-turns.test.ts | 141 +++++++++++++++++- .../src/plugins/hermes-bots/group-turns.ts | 62 +++++++- .../hermes-bots/roster-attention.test.tsx | 92 ++++++++++++ .../src/plugins/hermes-bots/roster-pane.tsx | 6 +- 6 files changed, 310 insertions(+), 21 deletions(-) create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx diff --git a/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx b/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx index 55dcc8e2df..481bcb4c16 100644 --- a/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx +++ b/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx @@ -19,7 +19,7 @@ import type * as HermesSdk from '@hermes/plugin-sdk' import { fireEvent, render, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { BotRow } from './bot-row' +import { BotRow, GroupRow } from './bot-row' import { translateBots } from './i18n-test-helper' import type { RosterRow } from './types' @@ -175,3 +175,23 @@ describe('context-menu mutations hydrate the alias first', () => { expect(params).toMatchObject({ name: 'backend-worker', ui_meta: { 'hermes-bots': { pinned: false } } }) }) }) + +describe('GroupRow renders its needs-you badge from the needsYou prop', () => { + const noopGroup = () => undefined + + it('shows the question badge when needsYou is true', () => { + render( + + ) + + expect(screen.getByLabelText('Needs your input')).toBeTruthy() + }) + + it('hides the question badge when needsYou is false', () => { + render( + + ) + + expect(screen.queryByLabelText('Needs your input')).toBeNull() + }) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx b/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx index 776576e34b..059c895fd8 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx +++ b/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx @@ -94,7 +94,7 @@ import { } from './group-panes' import type { GroupComposerDraft, GroupDraftSetter } from './group-panes' import { sendToGroupChat, stopGroupThread } from './group-rounds' -import { clearGroupClarify } from './group-turns' +import { clearGroupClarify, renameGroupClarify } from './group-turns' import { botsText, useBots } from './i18n' import { displayName, slugify } from './labels' import { botRosterMeta, setBotsWorkspaceOwner } from './routing' @@ -299,9 +299,9 @@ async function renameGroupChat(oldName: string, newName: string, members: GroupM $groupNeedsYou.set(needs) } - // Mirrored clarify cards key by group name; drop the old room's — the - // next poll re-mirrors any still-blocking question under the new name. - clearGroupClarify(oldName) + // Mirrored clarify cards key by group name; a pending prompt's attention + // must follow the room to its new name, not disappear. + renameGroupClarify(oldName, next) // Local memberships: swap the name inside each member's canonical groups // list (syncs cross-machine via ui_meta). Remote members' seating lives in diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts index b956c4883c..948af00790 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts @@ -359,11 +359,23 @@ describe('clarify and approvals (#90694)', () => { } it('holds the turn open while a member is blocked on clarify, then lands the reply', async () => { + let live: Awaited> | null = null + let sawPendingAttention = false + const room = await loadRoom({ clarifyUntil: { research: { payload: CLARIFY, until: 3 } }, + // The mirror pass runs while the question is still blocking — this is + // the observable proof the gate inspected pending_clarify. Asserting + // on $groupNeedsYou/$groupClarify AFTER the turn lands proves nothing: + // the clarify has already resolved and its mirror is gone by then. + onResumePoll: () => { + sawPendingAttention = sawPendingAttention || live!.turns.groupHasPendingClarify(live!.chat.$groupClarify.get(), 'Core') + }, turn: () => 'targeting staging' }) + live = room + const thread = room.rounds.sendToGroupChat( 'Core', [{ name: 'research', title: '' }], @@ -379,10 +391,9 @@ describe('clarify and approvals (#90694)', () => { expect(replies).toHaveLength(1) expect(replies[0].text).toBe('targeting staging') expect(Object.keys(room.chat.$groupClarify.get())).toHaveLength(0) - // The mirror pass ran while the question was blocking, badging the room. - // A poll that never inspects pending_clarify leaves this unset — it is the - // observable proof the gate executed. - expect(room.chat.$groupNeedsYou.get().Core).toBe(true) + expect(sawPendingAttention).toBe(true) + // Resolved and mirrored away — nothing left to badge. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) }) it('mirrors a question, badges needs-you, and is idempotent per request', async () => { @@ -397,16 +408,20 @@ describe('clarify and approvals (#90694)', () => { expect(mirrored[0].requestId).toBe('req-clarify-1') expect(mirrored[0].question).toBe('Which env should I target?') expect(mirrored[0].choices).toEqual(['staging', 'prod']) - expect(chat.$groupNeedsYou.get().Core).toBe(true) + // Badge is derived from $groupClarify, not a copy — nothing writes + // $groupNeedsYou here, so there is nothing to keep in sync. + expect(turns.groupHasPendingClarify(chat.$groupClarify.get(), 'Core')).toBe(true) // Same request again: no new entry, identity preserved. turns.syncGroupClarify('Core', member, { pending_clarify: CLARIFY }) expect(Object.values(chat.$groupClarify.get())[0]).toBe(mirrored[0]) - // Question resolved server-side: the mirror clears. + // Question resolved server-side: the mirror clears, and so does the + // derived badge — no separate cleanup path required. expect(turns.syncGroupClarify('Core', member, {})).toBe(false) expect(Object.keys(chat.$groupClarify.get())).toHaveLength(0) + expect(turns.groupHasPendingClarify(chat.$groupClarify.get(), 'Core')).toBe(false) }) it('never mirrors a question for older backends without pending_clarify', async () => { @@ -465,14 +480,125 @@ describe('clarify and approvals (#90694)', () => { expect(remaining).toHaveLength(1) expect(remaining[0].group).toBe('Other') + // The derived badge follows $groupClarify with no separate cleanup step. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Other')).toBe(true) + }) + + it('keeps the derived badge lit until every clarify in the room is answered', async () => { + const room = await loadRoom() + const research: GroupMember = { name: 'research', title: '' } + const ops: GroupMember = { name: 'ops', title: '' } + + room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) + room.turns.syncGroupClarify('Core', ops, { pending_clarify: { ...CLARIFY, request_id: 'req-2' } }) + + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) + + const [first, second] = Object.values(room.chat.$groupClarify.get()) + await room.turns.answerGroupClarify(first, research, 'staging') + + // One of two questions answered — the room still owes the user a reply. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) + + await room.turns.answerGroupClarify(second, ops, 'staging') + + // Last pending clarify answered — nothing left to badge. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) + }) + + // $groupNeedsYou (mention attention) and + // $groupClarify (pending clarify/approval attention) are independent + // sources — resolving one must never clear the other. The roster reads + // BOTH: groupNeedsYou[group] || groupHasPendingClarify(clarifies, group). + it('resolving the last clarify preserves independent @user mention attention', async () => { + const room = await loadRoom() + const research: GroupMember = { name: 'research', title: '' } + + // A member addressing @user badges the room — independent of clarify. + room.chat.appendGroupChatEntry('Core', { kind: 'member', name: 'research' }, '@user deployment needs a call') + expect(room.chat.$groupNeedsYou.get().Core).toBe(true) + + room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) + + await room.turns.answerGroupClarify(Object.values(room.chat.$groupClarify.get())[0], research, 'staging') + + // Clarify resolved — but the unrelated mention attention must survive. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) + expect(room.chat.$groupNeedsYou.get().Core).toBe(true) + }) + + it('preserves pending clarify attention across a room rename', async () => { + const room = await loadRoom() + const research: GroupMember = { name: 'research', title: '' } + + // Core has a pending clarify and no @user mention — attention is + // entirely clarify-derived, the case a plain group-key swap loses. + room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) + expect(room.chat.$groupNeedsYou.get().Core).toBeFalsy() + + room.turns.renameGroupClarify('Core', 'Renamed') + + // The prompt now belongs to the new name — not the old one. + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Renamed')).toBe(true) + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) + const [prompt] = Object.values(room.chat.$groupClarify.get()) + expect(prompt.group).toBe('Renamed') + + // Resolving it under the new name clears attention there. + await room.turns.answerGroupClarify(prompt, research, 'staging') + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Renamed')).toBe(false) + }) + + it('lets the renamed room\'s current prompt replace a stale mirror already at the destination', async () => { + const room = await loadRoom() + + // Current prompt inserted FIRST, stale destination mirror SECOND — a + // single-pass rekey that iterates Object.entries in insertion order + // would let the stale entry (visited last) clobber the just-migrated + // current one. Two passes must let the live room win regardless of + // insertion order. + room.turns.syncGroupClarify('Core', { name: 'research', title: '' }, { pending_clarify: CLARIFY }) + room.chat.$groupClarify.set({ + ...room.chat.$groupClarify.get(), + 'Renamed::research': { + at: Date.now(), + choices: [], + group: 'Renamed', + kind: 'clarify', + member: 'research', + memberKey: 'research', + multiSelect: false, + question: 'stale question', + questions: null, + requestId: 'req-stale', + sessionId: null + } + }) + + room.turns.renameGroupClarify('Core', 'Renamed') + + const migrated = room.chat.$groupClarify.get()['Renamed::research'] + expect(migrated.requestId).toBe(CLARIFY.request_id) + expect(Object.keys(room.chat.$groupClarify.get())).toEqual(['Renamed::research']) }) it('holds the turn open on a command approval too', async () => { + let live: Awaited> | null = null + let sawPendingAttention = false + const room = await loadRoom({ approvalUntil: { research: { payload: APPROVAL, until: 3 } }, + onResumePoll: () => { + sawPendingAttention = sawPendingAttention || live!.turns.groupHasPendingClarify(live!.chat.$groupClarify.get(), 'Core') + }, turn: () => 'build cleaned' }) + live = room + const thread = room.rounds.sendToGroupChat( 'Core', [{ name: 'research', title: '' }], @@ -488,7 +614,8 @@ describe('clarify and approvals (#90694)', () => { expect(replies).toHaveLength(1) expect(replies[0].text).toBe('build cleaned') expect(Object.keys(room.chat.$groupClarify.get())).toHaveLength(0) - expect(room.chat.$groupNeedsYou.get().Core).toBe(true) + expect(sawPendingAttention).toBe(true) + expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) }) it('mirrors an approval with its kind, command and server choices', async () => { diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.ts index 7a79a37c14..e3e6cfdec6 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.ts @@ -9,7 +9,7 @@ import { host } from '@hermes/plugin-sdk' import { recordGroupActivity } from './group-activity' -import { $groupChats, $groupClarify, $groupNeedsYou, appendGroupChatEntry, updateGroupChat } from './group-chat' +import { $groupChats, $groupClarify, appendGroupChatEntry, updateGroupChat } from './group-chat' import type { GroupChatRoom } from './group-chat' import { groupMemberKey, groupSessionOwner } from './group-membership' import { botConnectionRoute, requestForBot } from './routing' @@ -493,16 +493,24 @@ export function syncGroupClarify(group: string, member: GroupMember, state: Grou questions: null } }) - // A blocked member is a question for the human — badge the room. - $groupNeedsYou.set({ - ...$groupNeedsYou.get(), - [group]: true - }) return true } -/** Drop every mirrored clarify belonging to `group` (disband/rename). */ +/** Whether `group` has any member currently blocked on a clarify or + * approval, given a $groupClarify snapshot. Pure by design: the caller + * (roster-pane) subscribes to $groupClarify itself via useValue and passes + * the live snapshot in, so the subscription actually drives the + * recalculation instead of existing only to force a re-render. $groupClarify + * is the single source of truth for this kind of attention — nothing copies + * it into a second boolean, so there is nothing to keep in sync when a + * prompt resolves, is answered, or the room is disbanded/renamed. */ +export function groupHasPendingClarify(clarifies: Record, group: string): boolean { + return Object.values(clarifies).some(entry => entry?.group === group) +} + +/** Drop every mirrored clarify belonging to `group` (disband — the room is + * gone, nothing to move the attention to). */ export function clearGroupClarify(group: string) { const all = $groupClarify.get() const next: Record = {} @@ -521,6 +529,46 @@ export function clearGroupClarify(group: string) { } } +/** Re-key every mirrored clarify belonging to `oldName` onto `newName` + * (rename). Unlike disband, the room still exists under its new name — a + * pending clarify's attention must follow it, not disappear. Known gap: + * an in-flight member turn captured `oldName` as its closure argument + * (runGroupChatMemberTurnLeased) and keeps polling under that name, so a + * poll that lands after this rename can still re-mirror a NEW entry under + * oldName. That race predates this function and isn't fixed here. */ +export function renameGroupClarify(oldName: string, newName: string) { + const all = $groupClarify.get() + const next: Record = {} + let changed = false + + // Preserve every mirror that isn't being renamed. Iteration order matters: + // a single pass keyed by insertion order can let a STALE mirror already + // stranded at newName (left behind by the in-flight-poll race noted + // below) clobber the just-migrated CURRENT mirror if the stale entry + // happens to iterate after it. Copying unrelated entries first and + // writing the migrated ones last guarantees the live room's prompt + // always wins its destination key. + for (const [key, value] of Object.entries(all)) { + if (value?.group !== oldName) { + next[key] = value + } + } + + for (const value of Object.values(all)) { + if (value?.group === oldName) { + changed = true + // Rebuild the key from the mirror's own memberKey rather than + // string-replacing oldName in place — a group name that happens to + // be a substring of the member key must not corrupt the rekey. + next[`${newName}::${value.memberKey}`] = { ...value, group: newName } + } + } + + if (changed) { + $groupClarify.set(next) + } +} + /** Answer a member's pending prompt from the room. Routes to the member's * OWN source (requestForBot), so cross-connection members work. * - clarify: `clarify.respond`; batch questions send one respond per diff --git a/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx b/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx new file mode 100644 index 0000000000..f8265028a6 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx @@ -0,0 +1,92 @@ +/** + * The needs-you attention badge the roster shows for a Group Chat row is + * derived from TWO independent stores: $groupNeedsYou (an @user mention) + * and $groupClarify (a pending clarify/approval, via groupHasPendingClarify). + * roster-pane.tsx subscribes to both and combines them with ||. + * + * This harness proves the REAL composition works end-to-end: real stores, + * real useValue subscriptions, and the real groupHasPendingClarify helper — + * not a re-implementation of the derive expression. Mutating a store here + * must be enough to flip what the harness renders, with no manual refresh. + */ +import { useValue } from '@hermes/plugin-sdk' +import { act, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import { $groupClarify, $groupNeedsYou } from './group-chat' +import { groupHasPendingClarify } from './group-turns' +import type { GroupPrompt } from './types' + +const GROUP = 'Core' + +function mockPrompt(overrides: Partial = {}): GroupPrompt { + return { + at: Date.now(), + choices: ['staging', 'prod'], + group: GROUP, + kind: 'clarify', + member: 'research', + memberKey: 'research', + multiSelect: false, + question: 'Which env?', + questions: null, + requestId: 'req-1', + sessionId: null, + ...overrides + } +} + +function AttentionHarness({ group }: { group: string }) { + const mentions = useValue($groupNeedsYou) + const clarifies = useValue($groupClarify) + const needsYou = Boolean(mentions[group]) || groupHasPendingClarify(clarifies, group) + + return
{String(needsYou)}
+} + +describe('roster attention badge — real stores, real useValue, real helper', () => { + afterEach(() => { + $groupNeedsYou.set({}) + $groupClarify.set({}) + }) + + it('reflects a clarify being added and then removed', () => { + act(() => { + render() + }) + + expect(screen.getByTestId('attention').textContent).toBe('false') + + act(() => { + $groupClarify.set({ 'Core::research': mockPrompt() }) + }) + + expect(screen.getByTestId('attention').textContent).toBe('true') + + act(() => { + $groupClarify.set({}) + }) + + expect(screen.getByTestId('attention').textContent).toBe('false') + }) + + it('keeps attention true when clarify clears but the mention persists', () => { + act(() => { + render() + }) + + act(() => { + $groupNeedsYou.set({ [GROUP]: true }) + $groupClarify.set({ 'Core::research': mockPrompt() }) + }) + + expect(screen.getByTestId('attention').textContent).toBe('true') + + // Clarify resolves — the unrelated mention must keep the badge lit. + act(() => { + $groupClarify.set({}) + }) + + expect(screen.getByTestId('attention').textContent).toBe('true') + }) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx index 8d82d7dd36..cd94ee134a 100644 --- a/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx @@ -56,10 +56,11 @@ import { useRoster } from './data' import { EditProfileDialog } from './edit-profile-dialog' -import { $groupChats, $groupChatWorkspace, $groupNeedsYou } from './group-chat' +import { $groupChats, $groupChatWorkspace, $groupClarify, $groupNeedsYou } from './group-chat' import { disbandGroupChat, GroupChatWorkspace, openGroupChat } from './group-chat-view' import { groupChatMemberBots, groupChatNames, groupLastActivity } from './group-membership' import { $groupMainTabsRev, shouldRenderGroupChatInPane } from './group-panes' +import { groupHasPendingClarify } from './group-turns' import { $showHiddenBots, isBotHidden, isBotPinned } from './hidden-bots' import { useBots } from './i18n' import { displayName } from './labels' @@ -289,6 +290,7 @@ export function BotsPane() { // room beside a live main tab and stick). useValue($groupMainTabsRev) const groupNeedsYou = useValue($groupNeedsYou) + const groupClarify = useValue($groupClarify) const groupRooms = useValue($groupChats) const rememberedSources = useValue($lastSources) const rosterHydrated = useValue($rosterHydrated) @@ -572,7 +574,7 @@ export function BotsPane() { group={row.name} key={`group:${row.name}`} members={row.members} - needsYou={Boolean(groupNeedsYou[row.name])} + needsYou={Boolean(groupNeedsYou[row.name]) || groupHasPendingClarify(groupClarify, row.name)} onDisband={setDeletingGroup} onOpen={openGroupChat} /> From 613ec20304326a1816516fee6742d9cddccf7dbb Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:35:04 -0700 Subject: [PATCH 2/4] fix(bot-mode): keep pending group attention bound to its live room Complete the PR #101452 salvage, preserving sprmn24 authorship. Credit @kokhlo PR #101591 for independently diagnosing the in-flight rename race; use scoped authoritative room bindings rather than persistent aliases. Keep mention attention independent and retire late work after disband. --- .../src/plugins/hermes-bots/bot-row.test.tsx | 22 +- .../plugins/hermes-bots/group-chat-view.tsx | 2 +- .../plugins/hermes-bots/group-membership.ts | 36 + .../src/plugins/hermes-bots/group-rounds.ts | 110 ++- .../plugins/hermes-bots/group-turns.test.ts | 256 ++++-- .../src/plugins/hermes-bots/group-turns.ts | 770 ++++++++++-------- .../hermes-bots/roster-attention.test.tsx | 92 --- website/docs/user-guide/bot-mode.md | 2 +- 8 files changed, 698 insertions(+), 592 deletions(-) delete mode 100644 apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx diff --git a/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx b/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx index 481bcb4c16..55dcc8e2df 100644 --- a/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx +++ b/apps/desktop/src/plugins/hermes-bots/bot-row.test.tsx @@ -19,7 +19,7 @@ import type * as HermesSdk from '@hermes/plugin-sdk' import { fireEvent, render, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { BotRow, GroupRow } from './bot-row' +import { BotRow } from './bot-row' import { translateBots } from './i18n-test-helper' import type { RosterRow } from './types' @@ -175,23 +175,3 @@ describe('context-menu mutations hydrate the alias first', () => { expect(params).toMatchObject({ name: 'backend-worker', ui_meta: { 'hermes-bots': { pinned: false } } }) }) }) - -describe('GroupRow renders its needs-you badge from the needsYou prop', () => { - const noopGroup = () => undefined - - it('shows the question badge when needsYou is true', () => { - render( - - ) - - expect(screen.getByLabelText('Needs your input')).toBeTruthy() - }) - - it('hides the question badge when needsYou is false', () => { - render( - - ) - - expect(screen.queryByLabelText('Needs your input')).toBeNull() - }) -}) diff --git a/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx b/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx index 059c895fd8..c93ca55793 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx +++ b/apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx @@ -238,7 +238,7 @@ export async function disbandGroupChat(group: string, members: RosterRow[]) { * rename, so even a member whose sid is later lost falls back to the same * "Group: " title lookup instead of a fresh "Group: ". * Returns the new name, or null when the target name is taken. */ -async function renameGroupChat(oldName: string, newName: string, members: GroupMember[] | null | undefined) { +export async function renameGroupChat(oldName: string, newName: string, members: GroupMember[] | null | undefined) { const next = String(newName || '') .trim() .slice(0, 64) diff --git a/apps/desktop/src/plugins/hermes-bots/group-membership.ts b/apps/desktop/src/plugins/hermes-bots/group-membership.ts index 8d8e8a00e4..bfbe5814f2 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-membership.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-membership.ts @@ -9,6 +9,42 @@ import { $groupChats, groupChatRoomKey } from './group-chat' import { botConnectionRoute, botRosterMeta, resolveBotConnectionRoute } from './routing' import type { BotMeta, GroupChat, GroupMember, RosterRow } from './types' +/** Follow the authoritative room record for one async operation. Rename moves + * the record wholesale (including legacy rooms without a roomId); disband + * retires this binding permanently, even if the same display name is reused. */ +export function followGroupChat(group: string, onRename: (name: string) => void) { + let live = !$groupChats.get()[group]?.tombstone + + const dispose = $groupChats.listen((rooms, previous) => { + const prior = previous?.[group] + + if (!live || !prior) { + return + } + + const current = rooms[group] + + if (current && !current.tombstone && current.roomId === prior.roomId) { + return + } + + const moved = Object.entries(rooms).find(([, room]) => + !room.tombstone && (prior.roomId ? room.roomId === prior.roomId : room === prior) + ) + + if (!moved) { + live = false + + return + } + + group = moved[0] + onRename(group) + }) + + return { dispose, isLive: () => live } +} + export function groupWorkspaceOwnerKey(group: string) { return `group:${groupChatRoomKey(group, $groupChats.get()[group])}` } diff --git a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts index ff65ff2745..476921c0cd 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts @@ -21,7 +21,7 @@ import { updateGroupChat } from './group-chat' import type { GroupChatRoom, GroupHoldStamp } from './group-chat' -import { durableGroupChatMembers, groupMemberKey } from './group-membership' +import { durableGroupChatMembers, followGroupChat, groupMemberKey } from './group-membership' import { harvestStrandedGroupReply, isGroupPassText, runGroupChatMemberTurn } from './group-turns' import { requestForBot } from './routing' import type { Attachment, GroupMember, GroupMessage } from './types' @@ -493,8 +493,9 @@ export async function stopGroupThread(group: string, thread: null | string, memb * Watermarks are per thread+member (`${thread}::${memberKey}`), so parallel * topics never eat each other's deltas. */ export async function runGroupChatRounds(group: string, members: GroupMember[], thread: string) { + const binding = followGroupChat(group, name => { group = name }) const startEpoch = ($groupChats.get()[group] || {}).epoch || 0 - const isCurrent = () => (($groupChats.get()[group] || {}).epoch || 0) === startEpoch + const isCurrent = () => binding.isLive() && (($groupChats.get()[group] || {}).epoch || 0) === startEpoch let posted = 0 let continuations = 0 // #94478: how this drive ended. 'settled' means quiet consensus (everyone @@ -519,6 +520,10 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], } await harvestStrandedGroupReply(group, member) + + if (!binding.isLive()) { + return + } } const roomLog = (($groupChats.get()[group] || {}).log || []).filter( @@ -676,6 +681,10 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], // during-turn tail is anchored by entry id, not index — the history // trim drops entries from the FRONT, so an index slice could // overshoot after a mid-turn trim and silently commit a stale turn. + if (!binding.isLive()) { + return + } + const roomNow = $groupChats.get()[group] || { log: [] } @@ -911,6 +920,8 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], void harvestStrandedUntilSettled(group, members, thread) } } + + binding.dispose() } } @@ -919,39 +930,53 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], * stranded, a new loop takes the room over (it harvests on its own), or the * room record disappears (disband). */ async function harvestStrandedUntilSettled(group: string, members: GroupMember[], thread: string) { - const HARVEST_INTERVAL_MS = 5000 - const HARVEST_MAX_TRIES = 60 + const binding = followGroupChat(group, name => { group = name }) - for (let attempt = 0; attempt < HARVEST_MAX_TRIES; attempt++) { - await new Promise(resolve => window.setTimeout(resolve, HARVEST_INTERVAL_MS)) - const room = $groupChats.get()[group] + try { + const HARVEST_INTERVAL_MS = 5000 + const HARVEST_MAX_TRIES = 60 - if (!room || room.running) { - return - } + for (let attempt = 0; attempt < HARVEST_MAX_TRIES; attempt++) { + await new Promise(resolve => window.setTimeout(resolve, HARVEST_INTERVAL_MS)) + const room = $groupChats.get()[group] - const stranded = room.stranded || {} + if (!binding.isLive() || !room || room.running) { + return + } - if (!Object.keys(stranded).length) { - return - } + const stranded = room.stranded || {} - for (const member of members) { - if (Object.prototype.hasOwnProperty.call(stranded, groupMemberKey(member))) { - try { - await harvestStrandedGroupReply(group, member) - } catch { - // Best-effort: the next tick retries; the bound stops runaways. + if (!Object.keys(stranded).length) { + return + } + + for (const member of members) { + if (!binding.isLive()) { + return + } + + if (Object.prototype.hasOwnProperty.call(stranded, groupMemberKey(member))) { + try { + await harvestStrandedGroupReply(group, member) + } catch { + // Best-effort: the next tick retries; the bound stops runaways. + } } } } - } - recordGroupActivity(group, { - kind: 'failed', - member: null, - thread - }) + if (!binding.isLive()) { + return + } + + recordGroupActivity(group, { + kind: 'failed', + member: null, + thread + }) + } finally { + binding.dispose() + } } /** User send into a group room. `thread` continues that thread (its reply @@ -1026,26 +1051,31 @@ export function sendToGroupChat( thread: target }) - if (!wasRunning) { - void runGroupChatRounds(group, members, target).catch(() => { - updateGroupChat(group, (r: GroupChatRoom) => { - r.running = false + const binding = followGroupChat(group, name => { group = name }) - return r - }) - }) - } else { - // A loop is live; it bails at its next boundary. Chain the fresh loop - // after a short settle so exactly one drive owns the room. - setTimeout(() => { - void runGroupChatRounds(group, members, target).catch(() => { + const drive = () => { + if (!binding.isLive()) { + binding.dispose() + + return + } + + void runGroupChatRounds(group, members, target).catch(() => { + if (binding.isLive()) { updateGroupChat(group, (r: GroupChatRoom) => { r.running = false return r }) - }) - }, 250) + } + }).finally(binding.dispose) + } + + if (!wasRunning) { + drive() + } else { + // Preserve the existing newer-send handoff delay, without pinning its name. + setTimeout(drive, 250) } return target diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts index 948af00790..c7414b74dc 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts @@ -485,104 +485,194 @@ describe('clarify and approvals (#90694)', () => { expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Other')).toBe(true) }) - it('keeps the derived badge lit until every clarify in the room is answered', async () => { - const room = await loadRoom() - const research: GroupMember = { name: 'research', title: '' } - const ops: GroupMember = { name: 'ops', title: '' } - - room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) - room.turns.syncGroupClarify('Core', ops, { pending_clarify: { ...CLARIFY, request_id: 'req-2' } }) - - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) - - const [first, second] = Object.values(room.chat.$groupClarify.get()) - await room.turns.answerGroupClarify(first, research, 'staging') - - // One of two questions answered — the room still owes the user a reply. - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) - - await room.turns.answerGroupClarify(second, ops, 'staging') - - // Last pending clarify answered — nothing left to badge. - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) + it('keeps pending prompts independent from mention attention through their lifecycle', async () => { + const { chat, turns } = await loadRoom() + const member = { name: 'research', title: '' } + turns.syncGroupClarify('Core', member, { pending_clarify: CLARIFY }) + expect(chat.$groupNeedsYou.get().Core).toBeFalsy() + turns.syncGroupClarify('Core', { name: 'ops' }, { pending_approval: APPROVAL }) + expect(Object.values(chat.$groupClarify.get())).toHaveLength(2) + turns.syncGroupClarify('Core', member, {}) + expect(turns.groupHasPendingClarify(chat.$groupClarify.get(), 'Core')).toBe(true) + turns.syncGroupClarify('Core', { name: 'ops' }, {}) + expect(turns.groupHasPendingClarify(chat.$groupClarify.get(), 'Core')).toBe(false) + chat.appendGroupChatEntry('Core', { kind: 'member', name: 'research' }, '@user please review') + turns.syncGroupClarify('Core', member, { pending_approval: APPROVAL }) + await turns.answerGroupClarify(Object.values(chat.$groupClarify.get())[0], member, 'deny') + expect(Object.values(chat.$groupClarify.get())).toHaveLength(0) + expect(chat.$groupNeedsYou.get().Core).toBe(true) }) - // $groupNeedsYou (mention attention) and - // $groupClarify (pending clarify/approval attention) are independent - // sources — resolving one must never clear the other. The roster reads - // BOTH: groupNeedsYou[group] || groupHasPendingClarify(clarifies, group). - it('resolving the last clarify preserves independent @user mention attention', async () => { - const room = await loadRoom() - const research: GroupMember = { name: 'research', title: '' } + it('keeps late prompt snapshots on the live room and never revives a disbanded room', async ({ onTestFinished }) => { + for (const roomId of ['stable-room', undefined]) { + for (const disband of [false, true]) { + const room = await loadRoom({ turn: ({ n }) => n === 1 ? 'Completed reply' : '(pass)' }) + const view = await import('./group-chat-view') + const member = { name: 'research', title: '' } + room.chat.updateGroupChat('Core', current => ({ + ...current, roomId, running: true, epoch: 1, + log: [{ id: 'input', at: 1, from: { kind: 'user', name: 'You' }, text: '@research check', thread: 'thread' }] + })) + let entered!: () => void + let release!: () => void + const polled = new Promise(resolve => { entered = resolve }) + const held = new Promise(resolve => { release = resolve }) + const original = host.request as (method: string, params: Record) => Promise + let submitted = false + let answered = false + let polls = 0 - // A member addressing @user badges the room — independent of clarify. - room.chat.appendGroupChatEntry('Core', { kind: 'member', name: 'research' }, '@user deployment needs a call') - expect(room.chat.$groupNeedsYou.get().Core).toBe(true) + host.request = async (method: string, params: Record) => { + const result = await original(method, params) - room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) + if (method === 'prompt.submit') {submitted = true} - await room.turns.answerGroupClarify(Object.values(room.chat.$groupClarify.get())[0], research, 'staging') + if (method === 'clarify.respond') {answered = true} - // Clarify resolved — but the unrelated mention attention must survive. - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) - expect(room.chat.$groupNeedsYou.get().Core).toBe(true) - }) + if (method === 'session.resume' && submitted && !answered) { + if (++polls === 1) { entered(); await held } - it('preserves pending clarify attention across a room rename', async () => { - const room = await loadRoom() - const research: GroupMember = { name: 'research', title: '' } + return { ...result, pending_clarify: CLARIFY } + } - // Core has a pending clarify and no @user mention — attention is - // entirely clarify-derived, the case a plain group-key swap loses. - room.turns.syncGroupClarify('Core', research, { pending_clarify: CLARIFY }) - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(true) - expect(room.chat.$groupNeedsYou.get().Core).toBeFalsy() + return result + } - room.turns.renameGroupClarify('Core', 'Renamed') + const drive = room.rounds.runGroupChatRounds('Core', [member], 'thread') + await polled - // The prompt now belongs to the new name — not the old one. - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Renamed')).toBe(true) - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Core')).toBe(false) - const [prompt] = Object.values(room.chat.$groupClarify.get()) - expect(prompt.group).toBe('Renamed') + if (disband) { + await view.disbandGroupChat('Core', []) + release() + await drive + expect(Object.values(room.chat.$groupClarify.get())).toHaveLength(0) + expect(room.chat.$groupChats.get().Core === undefined || room.chat.$groupChats.get().Core.tombstone).toBe(true) + expect(room.chat.$groupChats.get().Core?.log || []).toHaveLength(0) + } else { + await view.renameGroupChat('Core', 'Renamed', []) - // Resolving it under the new name clears attention there. - await room.turns.answerGroupClarify(prompt, research, 'staging') - expect(room.turns.groupHasPendingClarify(room.chat.$groupClarify.get(), 'Renamed')).toBe(false) - }) + const mirrored = new Promise(resolve => { + const stop = room.chat.$groupClarify.listen(entries => { + if (Object.keys(entries).length) { stop(); resolve() } + }) + }) - it('lets the renamed room\'s current prompt replace a stale mirror already at the destination', async () => { - const room = await loadRoom() - - // Current prompt inserted FIRST, stale destination mirror SECOND — a - // single-pass rekey that iterates Object.entries in insertion order - // would let the stale entry (visited last) clobber the just-migrated - // current one. Two passes must let the live room win regardless of - // insertion order. - room.turns.syncGroupClarify('Core', { name: 'research', title: '' }, { pending_clarify: CLARIFY }) - room.chat.$groupClarify.set({ - ...room.chat.$groupClarify.get(), - 'Renamed::research': { - at: Date.now(), - choices: [], - group: 'Renamed', - kind: 'clarify', - member: 'research', - memberKey: 'research', - multiSelect: false, - question: 'stale question', - questions: null, - requestId: 'req-stale', - sessionId: null + release() + await mirrored + const [prompt] = Object.values(room.chat.$groupClarify.get()) + const correctRoom = prompt.group + await room.turns.answerGroupClarify(prompt, member, 'staging') + await drive + expect(correctRoom).toBe('Renamed') + expect(Object.keys(room.chat.$groupChats.get())).toEqual(['Renamed']) + expect(room.chat.$groupChats.get().Renamed.running).toBe(false) + expect(room.chat.$groupChats.get().Renamed.log.filter(entry => entry.from.kind === 'member').map(entry => entry.text)).toEqual(['Completed reply']) + expect(Object.values(room.chat.$groupClarify.get())).toHaveLength(0) + } } - }) + } - room.turns.renameGroupClarify('Core', 'Renamed') + const clock = vi.spyOn(Date, 'now').mockReturnValue(0) + onTestFinished(() => clock.mockRestore()) - const migrated = room.chat.$groupClarify.get()['Renamed::research'] - expect(migrated.requestId).toBe(CLARIFY.request_id) - expect(Object.keys(room.chat.$groupClarify.get())).toEqual(['Renamed::research']) + for (const recreate of [false, true]) { + clock.mockReturnValue(0) + const room = await loadRoom() + const view = await import('./group-chat-view') + const member = { name: 'research', title: '' } + room.chat.updateGroupChat('Core', current => ({ ...current, roomId: 'retired-room' })) + let entered!: () => void + let release!: () => void + const polled = new Promise(resolve => { entered = resolve }) + const held = new Promise(resolve => { release = resolve }) + const original = host.request as (method: string, params: Record) => Promise + let submitted = false + + host.request = async (method: string, params: Record) => { + if (method === 'session.resume' && submitted) { + entered() + await held + throw new Error('poll rejected after deadline') + } + + const result = await original(method, params) + + if (method === 'prompt.submit') { submitted = true } + + return result + } + + const turn = room.turns.runGroupChatMemberTurn('Core', member, 'check', 'thread', []) + await polled + await view.disbandGroupChat('Core', []) + + if (recreate) { + room.chat.updateGroupChat('Core', current => ({ ...current, roomId: 'replacement-room' })) + room.turns.syncGroupClarify('Core', member, { pending_clarify: CLARIFY }) + } + + const roomsBefore = structuredClone(room.chat.$groupChats.get()) + const promptsBefore = structuredClone(room.chat.$groupClarify.get()) + // Cross even the hard cap while the rejected poll is still in flight. + clock.mockReturnValue(24 * 60 * 60 * 1000) + release() + expect(await turn).toBeNull() + expect(room.chat.$groupChats.get()).toEqual(roomsBefore) + expect(room.chat.$groupClarify.get()).toEqual(promptsBefore) + } + + // Retirement during one background harvest must fence the next member too. + const room = await loadRoom() + const view = await import('./group-chat-view') + const members = [{ name: 'research' }, { name: 'ops' }] + room.chat.updateGroupChat('Core', current => ({ + ...current, roomId: 'old-harvest-room', running: true, + stranded: { research: 0, ops: 0 } + })) + let tick!: () => void + const previousWindow = globalThis.window + vi.stubGlobal('window', { setTimeout: (callback: () => void) => { tick = callback; + + return 0 } }) + onTestFinished(() => { vi.stubGlobal('window', previousWindow) }) + let entered!: () => void + let release!: () => void + const polled = new Promise(resolve => { entered = resolve }) + const held = new Promise(resolve => { release = resolve }) + let background = false + const backgroundProfiles: unknown[] = [] + const original = host.request as (method: string, params: Record) => Promise + + host.request = async (method: string, params: Record) => { + if (method !== 'session.resume') { return original(method, params) } + + if (background) { + backgroundProfiles.push(params.profile) + + if (params.profile === 'research') { entered(); await held } + } + + return { running: true, pending_clarify: CLARIFY } + } + + await room.rounds.runGroupChatRounds('Core', members, 'thread') + background = true + tick() + await polled + await view.disbandGroupChat('Core', []) + const { setImmediate } = await import('node:timers/promises') + await setImmediate() + room.chat.updateGroupChat('Core', current => ({ + ...current, roomId: 'new-harvest-room', stranded: { research: 0, ops: 0 } + })) + const roomsBefore = structuredClone(room.chat.$groupChats.get()) + const promptsBefore = structuredClone(room.chat.$groupClarify.get()) + release() + // Let the released RPC and its background caller finish their microtasks. + await setImmediate() + expect(backgroundProfiles).toEqual(['research']) + expect(room.chat.$groupChats.get()).toEqual(roomsBefore) + expect(room.chat.$groupClarify.get()).toEqual(promptsBefore) }) it('holds the turn open on a command approval too', async () => { diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.ts index e3e6cfdec6..67f737ba87 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.ts @@ -11,7 +11,7 @@ import { host } from '@hermes/plugin-sdk' import { recordGroupActivity } from './group-activity' import { $groupChats, $groupClarify, appendGroupChatEntry, updateGroupChat } from './group-chat' import type { GroupChatRoom } from './group-chat' -import { groupMemberKey, groupSessionOwner } from './group-membership' +import { followGroupChat, groupMemberKey, groupSessionOwner } from './group-membership' import { botConnectionRoute, requestForBot } from './routing' import type { Attachment, GroupMember, GroupPrompt, GroupPromptQuestion, ProfileRoute } from './types' @@ -123,110 +123,128 @@ interface GroupMemberSessionHandle { * it after restarts. Cross-connection members route to their OWN source * via requestForBot; the window's gateway never switches. */ export async function ensureGroupChatSession(group: string, member: GroupMember): Promise { - const room = $groupChats.get()[group] || {} - // New rooms title member sessions by their immutable roomId so a - // same-name recreate never resumes the old room's sessions by title; - // legacy rooms without a roomId fall back to the display name. - const title = `Group: ${room.roomId || group}` - const key = groupMemberKey(member) - const known = room.sessions && room.sessions[key] + const binding = followGroupChat(group, name => { group = name }) - // Try resuming what we know (stored sid first, then title lookup). - // - // FAIL CLOSED on a transient lookup failure — mirrors the sibling fix in - // findExistingCanonicalChat (87b645f52c). session.resume signals "this - // target genuinely doesn't exist" with JSON-RPC code 4007; every other - // failure (network blip, the backend still warming up after a restart, - // an oversized-resume refusal) means the real session might still be - // there and must not be read as "no session, mint a new one" — that - // forks the member's real history, and the fork silently overwrites - // room.sessions[key] so the old session becomes unreachable from the - // room. Only a genuine 4007 on BOTH targets means there truly is nothing - // to resume yet, so the loop falls through to session.create below. - for (const target of [known, title]) { - if (!target || target === true) { - continue - } + try { + const room = $groupChats.get()[group] || {} + // New rooms title member sessions by their immutable roomId so a + // same-name recreate never resumes the old room's sessions by title; + // legacy rooms without a roomId fall back to the display name. + const title = `Group: ${room.roomId || group}` + const key = groupMemberKey(member) + const known = room.sessions && room.sessions[key] - try { - const res = (await requestForBot(member, 'session.resume', { - session_id: target, - profile: member.name, - omit_messages: true - })) as GroupSessionSnapshot + // Try resuming what we know (stored sid first, then title lookup). + // + // FAIL CLOSED on a transient lookup failure — mirrors the sibling fix in + // findExistingCanonicalChat (87b645f52c). session.resume signals "this + // target genuinely doesn't exist" with JSON-RPC code 4007; every other + // failure (network blip, the backend still warming up after a restart, + // an oversized-resume refusal) means the real session might still be + // there and must not be read as "no session, mint a new one" — that + // forks the member's real history, and the fork silently overwrites + // room.sessions[key] so the old session becomes unreachable from the + // room. Only a genuine 4007 on BOTH targets means there truly is nothing + // to resume yet, so the loop falls through to session.create below. + for (const target of [known, title]) { + if (!target || target === true) { + continue + } - if (res?.session_id) { - // TODO(bot-mode-types): `known` is `room.sessions[key]`, which the - // domain model types `string | true` — and the `target === true` skip - // above shows the legacy `true` sentinel is expected here. A backend - // that answers the title resume without a `session_key` therefore - // stores `true` back into room.sessions and hands `true` on as the - // durable id, which later rides into `session_id` on the recovery - // resume and on session.interrupt. Typed as-written. - const stored = res.session_key || known + try { + const res = (await requestForBot(member, 'session.resume', { + session_id: target, + profile: member.name, + omit_messages: true + })) as GroupSessionSnapshot - if (stored) { - updateGroupChat(group, (current: GroupChatRoom) => { - current.sessions = { - ...(current.sessions || {}), - [key]: stored - } - current.sessionOwners = { - ...(current.sessionOwners || {}), - [key]: groupSessionOwner(member) - } - - return current - }) + if (!binding.isLive()) { + return { runtime: null } } - return { - runtime: res.session_id, - stored + if (res?.session_id) { + // TODO(bot-mode-types): `known` is `room.sessions[key]`, which the + // domain model types `string | true` — and the `target === true` skip + // above shows the legacy `true` sentinel is expected here. A backend + // that answers the title resume without a `session_key` therefore + // stores `true` back into room.sessions and hands `true` on as the + // durable id, which later rides into `session_id` on the recovery + // resume and on session.interrupt. Typed as-written. + const stored = res.session_key || known + + if (stored) { + updateGroupChat(group, (current: GroupChatRoom) => { + current.sessions = { + ...(current.sessions || {}), + [key]: stored + } + current.sessionOwners = { + ...(current.sessionOwners || {}), + [key]: groupSessionOwner(member) + } + + return current + }) + } + + return { + runtime: res.session_id, + stored + } } + } catch (error: any) { + if (error?.code !== 4007) { + const detail = error instanceof Error && error.message ? ` (${error.message})` : '' + throw new Error(`Could not check ${member?.name || 'member'}'s group session${detail} — not starting a new one`) + } + /* genuinely doesn't exist (4007) — try the next target / fall through to create */ } - } catch (error: any) { - if (error?.code !== 4007) { - const detail = error instanceof Error && error.message ? ` (${error.message})` : '' - throw new Error(`Could not check ${member?.name || 'member'}'s group session${detail} — not starting a new one`) - } - /* genuinely doesn't exist (4007) — try the next target / fall through to create */ } - } - const created = (await requestForBot(member, 'session.create', { - profile: member.name, - title, - // Room member sessions are plumbing — always hidden from the sidebar. - hidden: true, - // Explicit contracts (PR #97008): room plumbing sessions always rebuild - // from the member profile's CURRENT config on resume, never a stale - // stored model/provider pin. Older gateways ignore the unknown params; - // the server's hidden + "Group: " title fallback then covers legacy. - room_plumbing: true, - follow_profile_config: true - })) as { session_id?: string; stored_session_id?: string } + if (!binding.isLive()) { + return { runtime: null } + } - const stored = created?.stored_session_id || null + const created = (await requestForBot(member, 'session.create', { + profile: member.name, + title, + // Room member sessions are plumbing — always hidden from the sidebar. + hidden: true, + // Explicit contracts (PR #97008): room plumbing sessions always rebuild + // from the member profile's CURRENT config on resume, never a stale + // stored model/provider pin. Older gateways ignore the unknown params; + // the server's hidden + "Group: " title fallback then covers legacy. + room_plumbing: true, + follow_profile_config: true + })) as { session_id?: string; stored_session_id?: string } - if (stored) { - updateGroupChat(group, (r: GroupChatRoom) => { - r.sessions = { - ...(r.sessions || {}), - [key]: stored - } - r.sessionOwners = { - ...(r.sessionOwners || {}), - [key]: groupSessionOwner(member) - } + if (!binding.isLive()) { + return { runtime: null } + } - return r - }) - } + const stored = created?.stored_session_id || null - return { - runtime: created?.session_id || null, - stored + if (stored) { + updateGroupChat(group, (r: GroupChatRoom) => { + r.sessions = { + ...(r.sessions || {}), + [key]: stored + } + r.sessionOwners = { + ...(r.sessionOwners || {}), + [key]: groupSessionOwner(member) + } + + return r + }) + } + + return { + runtime: created?.session_id || null, + stored + } + } finally { + binding.dispose() } } @@ -529,13 +547,8 @@ export function clearGroupClarify(group: string) { } } -/** Re-key every mirrored clarify belonging to `oldName` onto `newName` - * (rename). Unlike disband, the room still exists under its new name — a - * pending clarify's attention must follow it, not disappear. Known gap: - * an in-flight member turn captured `oldName` as its closure argument - * (runGroupChatMemberTurnLeased) and keeps polling under that name, so a - * poll that lands after this rename can still re-mirror a NEW entry under - * oldName. That race predates this function and isn't fixed here. */ +/** Move existing prompts with the renamed room; active operations follow + * the same room through their scoped followGroupChat binding. */ export function renameGroupClarify(oldName: string, newName: string) { const all = $groupClarify.get() const next: Record = {} @@ -583,40 +596,51 @@ export async function answerGroupClarify( member: GroupMember, answers: Record | string | undefined ) { - if (entry.kind === 'approval') { - await requestForBot(member, 'approval.respond', { - session_id: entry.sessionId || undefined, - request_id: entry.requestId, - choice: typeof answers === 'string' && answers ? answers : 'deny' - }) - } else if (entry.questions && entry.questions.length) { - for (const question of entry.questions) { - // Question ids are opaque on the wire (`GroupPrompt.questions` types - // them `unknown`); the batch card keys its answer bag by exactly them. - const qid = (question?.qid ?? question?.id) as string + let group = entry.group + const binding = followGroupChat(group, name => { group = name }) + + try { + if (entry.kind === 'approval') { + await requestForBot(member, 'approval.respond', { + session_id: entry.sessionId || undefined, + request_id: entry.requestId, + choice: typeof answers === 'string' && answers ? answers : 'deny' + }) + } else if (entry.questions && entry.questions.length) { + for (const question of entry.questions) { + // Question ids are opaque on the wire (`GroupPrompt.questions` types + // them `unknown`); the batch card keys its answer bag by exactly them. + const qid = (question?.qid ?? question?.id) as string + await requestForBot(member, 'clarify.respond', { + request_id: entry.requestId, + question_id: qid, + answer: (answers as Record)?.[qid] ?? '' + }) + } + } else { await requestForBot(member, 'clarify.respond', { request_id: entry.requestId, - question_id: qid, - answer: (answers as Record)?.[qid] ?? '' + answer: typeof answers === 'string' ? answers : '' }) } - } else { - await requestForBot(member, 'clarify.respond', { - request_id: entry.requestId, - answer: typeof answers === 'string' ? answers : '' - }) - } - const all = $groupClarify.get() - const key = `${entry.group}::${entry.memberKey}` - - if (all[key]?.requestId === entry.requestId) { - const next = { - ...all + if (!binding.isLive()) { + return } - delete next[key] - $groupClarify.set(next) + const all = $groupClarify.get() + const key = `${group}::${entry.memberKey}` + + if (all[key]?.requestId === entry.requestId) { + const next = { + ...all + } + + delete next[key] + $groupClarify.set(next) + } + } finally { + binding.dispose() } } @@ -638,12 +662,16 @@ export async function runGroupChatMemberTurn( // lease, every RPC below rides its own request-scoped socket lease; the // socket that minted `runtime` can close between RPCs, the gateway reaps // the runtime session, and prompt.submit dies 4001 — the bot goes silent. - const releaseTurnLease = await retainGroupTurnRoute(member) + const binding = followGroupChat(group, name => { group = name }) + let releaseTurnLease: (() => void) | undefined try { - return await runGroupChatMemberTurnLeased(group, member, prompt, thread, images) + releaseTurnLease = await retainGroupTurnRoute(member) + + return binding.isLive() ? await runGroupChatMemberTurnLeased(group, member, prompt, thread, images) : null } finally { - releaseTurnLease() + releaseTurnLease?.() + binding.dispose() } } @@ -654,281 +682,315 @@ async function runGroupChatMemberTurnLeased( thread: string, images?: Attachment[] ): Promise { - const { runtime, stored } = await ensureGroupChatSession(group, member) - - if (!runtime) { - return null - } - - // #91868/#94569: remember the epoch this turn was dispatched under so the - // poll loop below can tell an explicit stop from ordinary room churn. - const dispatchEpoch = ($groupChats.get()[group] || {}).epoch || 0 - const memberKey = groupMemberKey(member) - recordGroupActivity(group, { - kind: 'working', - member: member.name, - thread - }) - - // Baseline: how many messages exist before our submit. - let before = 0 - // Every runtime id this turn has seen for the member's session. Terminal - // frames are keyed by runtime id, and a resume can hand back a fresh one. - const runtimeIds = new Set([runtime]) + const binding = followGroupChat(group, name => { group = name }) try { - const pre = (await requestForBot(member, 'session.resume', { - session_id: stored || runtime, - profile: member.name - })) as GroupSessionSnapshot + const { runtime, stored } = await ensureGroupChatSession(group, member) - before = Array.isArray(pre?.messages) ? pre.messages.length : pre?.message_count || 0 - - if (pre?.session_id) { - runtimeIds.add(pre.session_id) - } - } catch { - /* lazy session — zero messages */ - } - - // Stage this delta's attachments into the member's session so the model - // receives the actual payload with the prompt — the same attach RPCs the - // 1:1 chat uses (they also work cross-connection, where the member's - // gateway can't see this machine's files). Images queue as vision tiles, - // PDFs render per-page via pdf.attach, and other files materialize in the - // session workspace (their @file: refs are appended to the prompt so the - // member's file tools can read them). A failed attach degrades that - // member to text-only; the transcript line still names the attachment so - // the member knows something was shared. - const fileRefs: string[] = [] - - for (const img of Array.isArray(images) ? images : []) { - if (!img || typeof img.data !== 'string' || !img.data) { - continue - } - - try { - if (img.kind === 'pdf') { - await requestForBot(member, 'pdf.attach', { - session_id: runtime, - content_base64: img.data, - filename: img.name || 'attachment.pdf' - }) - } else if (img.kind === 'file') { - const res = (await requestForBot(member, 'file.attach', { - session_id: runtime, - data_url: img.data, - name: img.name || 'attachment' - })) as { ref_text?: string } - - if (res?.ref_text) { - fileRefs.push(`${img.name || 'attachment'} → ${res.ref_text}`) - } - } else { - await requestForBot(member, 'image.attach_bytes', { - session_id: runtime, - content_base64: img.data, - filename: img.name || 'attachment.png' - }) - } - } catch { - /* text-only fallback for this member */ - } - } - - const turnText = fileRefs.length - ? `${prompt}\n\nAttached files staged in your session workspace:\n${fileRefs.join('\n')}` - : prompt - - // #93602: one-shot recovery when the runtime session was reaped between - // minting and submitting. Tracks the runtime id the submit landed on so - // the poll fallback below targets a live session. - const liveRuntime = await submitGroupTurnPrompt(member, runtime, stored, turnText) - runtimeIds.add(liveRuntime) - const started = Date.now() - let deadline = started + GROUP_TURN_TIMEOUT_MS - // After the terminal frame fires, the gateway still has to flip - // session.running off in its turn `finally` — re-check quickly for a few - // beats instead of falling back to the slow backstop cadence. - let quickRechecks = 0 - - while (Date.now() < deadline) { - const signalled = quickRechecks - ? await waitForTurnSignal([], GROUP_TURN_SETTLE_RECHECK_MS) - : await waitForTurnSignal([...runtimeIds], GROUP_TURN_POLL_MS) - - quickRechecks = signalled ? GROUP_TURN_SETTLE_RECHECKS : Math.max(0, quickRechecks - 1) - - // #91868/#94569: an explicit stop (stopGroupThread) bumped the epoch AND - // held this member — the member's session was interrupted, so nothing is - // coming; abandon the poll instead of grinding until the deadline. Both - // conditions on purpose: an ordinary newer send bumps the epoch WITHOUT - // a hold, and that turn must keep polling so finished work can still be - // delivered (the #93127 commit check decides its fate, not this loop). - const roomDuringPoll = $groupChats.get()[group] || {} - - if ((roomDuringPoll.epoch || 0) !== dispatchEpoch && (roomDuringPoll.holds || {})[memberKey]) { + if (!runtime || !binding.isLive()) { return null } - let state: GroupSessionSnapshot | null = null + // #91868/#94569: remember the epoch this turn was dispatched under so the + // poll loop below can tell an explicit stop from ordinary room churn. + const dispatchEpoch = ($groupChats.get()[group] || {}).epoch || 0 + const memberKey = groupMemberKey(member) + recordGroupActivity(group, { + kind: 'working', + member: member.name, + thread + }) + + // Baseline: how many messages exist before our submit. + let before = 0 + // Every runtime id this turn has seen for the member's session. Terminal + // frames are keyed by runtime id, and a resume can hand back a fresh one. + const runtimeIds = new Set([runtime]) try { - state = (await requestForBot(member, 'session.resume', { - session_id: stored || liveRuntime, + const pre = (await requestForBot(member, 'session.resume', { + session_id: stored || runtime, profile: member.name })) as GroupSessionSnapshot + + before = Array.isArray(pre?.messages) ? pre.messages.length : pre?.message_count || 0 + + if (pre?.session_id) { + runtimeIds.add(pre.session_id) + } } catch { - continue + /* lazy session — zero messages */ } - if (state?.session_id) { - runtimeIds.add(state.session_id) + // Stage this delta's attachments into the member's session so the model + // receives the actual payload with the prompt — the same attach RPCs the + // 1:1 chat uses (they also work cross-connection, where the member's + // gateway can't see this machine's files). Images queue as vision tiles, + // PDFs render per-page via pdf.attach, and other files materialize in the + // session workspace (their @file: refs are appended to the prompt so the + // member's file tools can read them). A failed attach degrades that + // member to text-only; the transcript line still names the attachment so + // the member knows something was shared. + const fileRefs: string[] = [] + + for (const img of Array.isArray(images) ? images : []) { + if (!img || typeof img.data !== 'string' || !img.data) { + continue + } + + try { + if (img.kind === 'pdf') { + await requestForBot(member, 'pdf.attach', { + session_id: runtime, + content_base64: img.data, + filename: img.name || 'attachment.pdf' + }) + } else if (img.kind === 'file') { + const res = (await requestForBot(member, 'file.attach', { + session_id: runtime, + data_url: img.data, + name: img.name || 'attachment' + })) as { ref_text?: string } + + if (res?.ref_text) { + fileRefs.push(`${img.name || 'attachment'} → ${res.ref_text}`) + } + } else { + await requestForBot(member, 'image.attach_bytes', { + session_id: runtime, + content_base64: img.data, + filename: img.name || 'attachment.png' + }) + } + } catch { + /* text-only fallback for this member */ + } } - const messages = Array.isArray(state?.messages) ? state.messages : [] - const busy = Boolean(state?.inflight || state?.running) - // A clarify blocking inside the member's session is a question for the - // HUMAN (#90694) — mirror it into the room store so a card renders, and - // hold the turn open: the member isn't stalling, it's waiting on us. - const awaitingUser = syncGroupClarify(group, member, state) - const done = !busy && !awaitingUser + if (!binding.isLive()) { + return null + } - if (messages.length > before && done) { - const replyText = pickGroupTurnReply(messages, before) + const turnText = fileRefs.length + ? `${prompt}\n\nAttached files staged in your session workspace:\n${fileRefs.join('\n')}` + : prompt + + // #93602: one-shot recovery when the runtime session was reaped between + // minting and submitting. Tracks the runtime id the submit landed on so + // the poll fallback below targets a live session. + const liveRuntime = await submitGroupTurnPrompt(member, runtime, stored, turnText) + + if (!binding.isLive()) { + return null + } + + runtimeIds.add(liveRuntime) + const started = Date.now() + let deadline = started + GROUP_TURN_TIMEOUT_MS + // After the terminal frame fires, the gateway still has to flip + // session.running off in its turn `finally` — re-check quickly for a few + // beats instead of falling back to the slow backstop cadence. + let quickRechecks = 0 + + while (Date.now() < deadline) { + const signalled = quickRechecks + ? await waitForTurnSignal([], GROUP_TURN_SETTLE_RECHECK_MS) + : await waitForTurnSignal([...runtimeIds], GROUP_TURN_POLL_MS) + + quickRechecks = signalled ? GROUP_TURN_SETTLE_RECHECKS : Math.max(0, quickRechecks - 1) + + // #91868/#94569: an explicit stop (stopGroupThread) bumped the epoch AND + // held this member — the member's session was interrupted, so nothing is + // coming; abandon the poll instead of grinding until the deadline. Both + // conditions on purpose: an ordinary newer send bumps the epoch WITHOUT + // a hold, and that turn must keep polling so finished work can still be + // delivered (the #93127 commit check decides its fate, not this loop). + if (!binding.isLive()) { + return null + } + + const roomDuringPoll = $groupChats.get()[group] || {} + + if ((roomDuringPoll.epoch || 0) !== dispatchEpoch && (roomDuringPoll.holds || {})[memberKey]) { + return null + } + + let state: GroupSessionSnapshot | null = null + + try { + state = (await requestForBot(member, 'session.resume', { + session_id: stored || liveRuntime, + profile: member.name + })) as GroupSessionSnapshot + } catch { + continue + } + + if (!binding.isLive()) { + return null + } + + if (state?.session_id) { + runtimeIds.add(state.session_id) + } + + const messages = Array.isArray(state?.messages) ? state.messages : [] + const busy = Boolean(state?.inflight || state?.running) + // A clarify blocking inside the member's session is a question for the + // HUMAN (#90694) — mirror it into the room store so a card renders, and + // hold the turn open: the member isn't stalling, it's waiting on us. + const awaitingUser = syncGroupClarify(group, member, state) + const done = !busy && !awaitingUser + + if (messages.length > before && done) { + const replyText = pickGroupTurnReply(messages, before) + + if (replyText !== null) { + recordGroupActivity(group, { + kind: isGroupPassText(replyText) ? 'passed' : 'replied', + member: member.name, + thread + }) + + return replyText + } - if (replyText !== null) { recordGroupActivity(group, { - kind: isGroupPassText(replyText) ? 'passed' : 'replied', + kind: 'passed', member: member.name, thread }) - return replyText + return null } - recordGroupActivity(group, { - kind: 'passed', - member: member.name, - thread - }) + // Still visibly working — or waiting on the user's answer to a clarify: + // extend the deadline (never past the hard cap). A pending question must + // outlive the base turn timeout or it dies unanswered at 3 minutes. + if (busy || awaitingUser) { + deadline = Math.min(started + GROUP_TURN_HARD_CAP_MS, Math.max(deadline, Date.now() + GROUP_TURN_TIMEOUT_MS)) + } + } + if (!binding.isLive()) { return null } - // Still visibly working — or waiting on the user's answer to a clarify: - // extend the deadline (never past the hard cap). A pending question must - // outlive the base turn timeout or it dies unanswered at 3 minutes. - if (busy || awaitingUser) { - deadline = Math.min(started + GROUP_TURN_HARD_CAP_MS, Math.max(deadline, Date.now() + GROUP_TURN_TIMEOUT_MS)) - } - } - - // Timeout — clear any still-mirrored question card (the server-side - // clarify timeout runs its own course) and read as a pass, but remember the baseline + thread - // (runtime-only) so the finished reply can be posted late into the RIGHT - // thread instead of vanishing. - recordGroupActivity(group, { - kind: 'timed-out', - member: member.name, - thread - }) - syncGroupClarify(group, member, null) - updateGroupChat(group, (r: GroupChatRoom) => { - r.stranded = { - ...(r.stranded || {}), - [groupMemberKey(member)]: { - before, - thread + // Timeout — clear any still-mirrored question card (the server-side + // clarify timeout runs its own course) and read as a pass, but remember the baseline + thread + // (runtime-only) so the finished reply can be posted late into the RIGHT + // thread instead of vanishing. + recordGroupActivity(group, { + kind: 'timed-out', + member: member.name, + thread + }) + syncGroupClarify(group, member, null) + updateGroupChat(group, (r: GroupChatRoom) => { + r.stranded = { + ...(r.stranded || {}), + [groupMemberKey(member)]: { + before, + thread + } } - } - return r - }) + return r + }) - return null + return null + } finally { + binding.dispose() + } } /** Post a timed-out member's finished reply into the room, if it landed * after we stopped waiting. Called at the member's next turn boundary and * on user sends, so long-running work is delivered late rather than lost. */ export async function harvestStrandedGroupReply(group: string, member: GroupMember) { - const memberKey = groupMemberKey(member) - const room = $groupChats.get()[group] || {} - const marker = room.stranded?.[memberKey] - // Markers were a bare number before threads; normalize both shapes. - const strandedBefore = typeof marker === 'number' ? marker : marker?.before - const strandedThread = (typeof marker === 'object' && marker?.thread) || 'legacy' - - if (typeof strandedBefore !== 'number') { - return - } - - let state: GroupSessionSnapshot | null = null + const binding = followGroupChat(group, name => { group = name }) try { - const stored = room.sessions?.[memberKey] - state = (await requestForBot(member, 'session.resume', { - session_id: stored || `Group: ${room.roomId || group}`, - profile: member.name - })) as GroupSessionSnapshot - } catch { - return // source unreachable — leave the marker for the next boundary - } + const memberKey = groupMemberKey(member) + const room = $groupChats.get()[group] || {} + const marker = room.stranded?.[memberKey] + // Markers were a bare number before threads; normalize both shapes. + const strandedBefore = typeof marker === 'number' ? marker : marker?.before + const strandedThread = (typeof marker === 'object' && marker?.thread) || 'legacy' - if (state?.inflight || state?.running) { - return // still grinding — keep waiting - } - - // A stranded member blocked on a clarify is not "grinding" — surface the - // question card (#90694) and keep the marker until it resolves. - if (syncGroupClarify(group, member, state)) { - return - } - - // Done (or dead): the marker is consumed either way. - updateGroupChat(group, (r: GroupChatRoom) => { - const next = { - ...(r.stranded || {}) + if (typeof strandedBefore !== 'number') { + return } - delete next[memberKey] - r.stranded = next + let state: GroupSessionSnapshot | null = null - return r - }) - const messages = Array.isArray(state?.messages) ? state.messages : [] + try { + const stored = room.sessions?.[memberKey] + state = (await requestForBot(member, 'session.resume', { + session_id: stored || `Group: ${room.roomId || group}`, + profile: member.name + })) as GroupSessionSnapshot + } catch { + return // source unreachable — leave the marker for the next boundary + } - if (messages.length <= strandedBefore) { - return - } + if (!binding.isLive()) { + return + } - const reply = pickGroupTurnReply(messages, strandedBefore) + // Pending prompts are authoritative even while the session is running. + const awaitingUser = syncGroupClarify(group, member, state) - if (reply && !isGroupPassText(reply)) { - recordGroupActivity(group, { - kind: 'delivered', - member: member.name, - thread: strandedThread - }) - appendGroupChatEntry( - group, - { - kind: 'member', - name: member.name, - ...(member.remoteSource - ? { - source: member.connectionLabel || member.connectionId - } - : {}) - }, - reply, - strandedThread - ) + if (state?.inflight || state?.running || awaitingUser) { + return + } + + // Done (or dead): the marker is consumed either way. updateGroupChat(group, (r: GroupChatRoom) => { - r.watermarks[`${strandedThread}::${memberKey}`] = r.log.length + const next = { + ...(r.stranded || {}) + } + + delete next[memberKey] + r.stranded = next return r }) + const messages = Array.isArray(state?.messages) ? state.messages : [] + + if (messages.length <= strandedBefore) { + return + } + + const reply = pickGroupTurnReply(messages, strandedBefore) + + if (reply && !isGroupPassText(reply)) { + recordGroupActivity(group, { + kind: 'delivered', + member: member.name, + thread: strandedThread + }) + appendGroupChatEntry( + group, + { + kind: 'member', + name: member.name, + ...(member.remoteSource + ? { + source: member.connectionLabel || member.connectionId + } + : {}) + }, + reply, + strandedThread + ) + updateGroupChat(group, (r: GroupChatRoom) => { + r.watermarks[`${strandedThread}::${memberKey}`] = r.log.length + + return r + }) + } + } finally { + binding.dispose() } } diff --git a/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx b/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx deleted file mode 100644 index f8265028a6..0000000000 --- a/apps/desktop/src/plugins/hermes-bots/roster-attention.test.tsx +++ /dev/null @@ -1,92 +0,0 @@ -/** - * The needs-you attention badge the roster shows for a Group Chat row is - * derived from TWO independent stores: $groupNeedsYou (an @user mention) - * and $groupClarify (a pending clarify/approval, via groupHasPendingClarify). - * roster-pane.tsx subscribes to both and combines them with ||. - * - * This harness proves the REAL composition works end-to-end: real stores, - * real useValue subscriptions, and the real groupHasPendingClarify helper — - * not a re-implementation of the derive expression. Mutating a store here - * must be enough to flip what the harness renders, with no manual refresh. - */ -import { useValue } from '@hermes/plugin-sdk' -import { act, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, it } from 'vitest' - -import { $groupClarify, $groupNeedsYou } from './group-chat' -import { groupHasPendingClarify } from './group-turns' -import type { GroupPrompt } from './types' - -const GROUP = 'Core' - -function mockPrompt(overrides: Partial = {}): GroupPrompt { - return { - at: Date.now(), - choices: ['staging', 'prod'], - group: GROUP, - kind: 'clarify', - member: 'research', - memberKey: 'research', - multiSelect: false, - question: 'Which env?', - questions: null, - requestId: 'req-1', - sessionId: null, - ...overrides - } -} - -function AttentionHarness({ group }: { group: string }) { - const mentions = useValue($groupNeedsYou) - const clarifies = useValue($groupClarify) - const needsYou = Boolean(mentions[group]) || groupHasPendingClarify(clarifies, group) - - return
{String(needsYou)}
-} - -describe('roster attention badge — real stores, real useValue, real helper', () => { - afterEach(() => { - $groupNeedsYou.set({}) - $groupClarify.set({}) - }) - - it('reflects a clarify being added and then removed', () => { - act(() => { - render() - }) - - expect(screen.getByTestId('attention').textContent).toBe('false') - - act(() => { - $groupClarify.set({ 'Core::research': mockPrompt() }) - }) - - expect(screen.getByTestId('attention').textContent).toBe('true') - - act(() => { - $groupClarify.set({}) - }) - - expect(screen.getByTestId('attention').textContent).toBe('false') - }) - - it('keeps attention true when clarify clears but the mention persists', () => { - act(() => { - render() - }) - - act(() => { - $groupNeedsYou.set({ [GROUP]: true }) - $groupClarify.set({ 'Core::research': mockPrompt() }) - }) - - expect(screen.getByTestId('attention').textContent).toBe('true') - - // Clarify resolves — the unrelated mention must keep the badge lit. - act(() => { - $groupClarify.set({}) - }) - - expect(screen.getByTestId('attention').textContent).toBe('true') - }) -}) diff --git a/website/docs/user-guide/bot-mode.md b/website/docs/user-guide/bot-mode.md index 02a2070b84..6f7a8794a1 100644 --- a/website/docs/user-guide/bot-mode.md +++ b/website/docs/user-guide/bot-mode.md @@ -96,7 +96,7 @@ Groups are standalone rows in the same activity-ordered roster as Bot DMs. A Bot - **One visible conversation.** Public messages and each member's reply stay readable in arrival order, with the speaker's name and timestamp. Starting another topic does not collapse earlier replies. **Reply in thread** continues that topic without reordering the room; **Activity** is a secondary status view, not a replacement for messages. Private Bot Chats remain separate. - Your message triggers up to **three serial rounds** of member turns. @-mentioned Bots respond (everyone responds when nobody is mentioned); each Bot replies briefly or passes, and the room settles when a full round stays silent. -- Bots pull each other in with `@name`, and escalate real judgment calls to you with `@user` — the group row shows a **needs you** badge when that happens. +- Bots pull each other in with `@name`, and escalate real judgment calls to you with `@user` — the group row shows a **needs you** badge when that happens. Pending questions and command approvals also light that badge; resolving the last prompt clears only prompt attention, not an independent mention. Prompts follow a renamed room, while disbanding retires them even if a member's in-flight poll arrives later. - Hard caps (10 messages per send, 3 rounds) keep rooms from spinning. - Each member keeps its own persistent `Group: ` session, so room context survives like any other conversation. - **Not every Bot replies to every message.** Speaking is each member's own choice — a Bot replies only when it has something new to add and passes otherwise, and @-mentioning specific members scopes the round to them. Expect the members you addressed (or whoever has something to say) to speak, and the rest to stay quiet. From e1c8764d18de9e79dbc6b016a1814b9d89bd0c4f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:53:51 -0700 Subject: [PATCH 3/4] fix: fence retired group member failure cues --- .../src/plugins/hermes-bots/group-rounds.ts | 8 ++++ .../plugins/hermes-bots/group-turns.test.ts | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts index 476921c0cd..78a9e90c2a 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts @@ -655,6 +655,10 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], clearBotAttention(groupMemberKey(member)) } } catch (error: any) { + if (!binding.isLive()) { + return + } + const reason = String(error?.data?.reason || '').trim() recordGroupActivity(group, { kind: 'failed', @@ -822,6 +826,10 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], clearBotAttention(memberKey) } } catch (error: any) { + if (!binding.isLive()) { + return + } + recordGroupActivity(group, { kind: 'failed', member: member.name, diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts index c7414b74dc..bf85c51a62 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.test.ts @@ -572,6 +572,47 @@ describe('clarify and approvals (#90694)', () => { } } + // Rejected member setup/submit must not publish failure cues into a new room. + for (const continuation of [false, true]) { + for (const rejectedMethod of ['session.resume', 'prompt.submit']) { + const room = await loadRoom() + const view = await import('./group-chat-view') + const activity = await import('./group-activity') + const data = await import('./data') + const members = [{ name: 'research' }, { name: 'ops' }] + room.chat.updateGroupChat('Core', current => ({ + ...current, roomId: 'old-rejection-room', running: true, + log: continuation + ? [{ id: 'handoff', at: 1, from: { kind: 'member', name: 'research' }, text: '@ops check', thread: 'thread' }, + { id: 'input', at: 2, from: { kind: 'user', name: 'You' }, text: '@research check', thread: 'thread' }] + : [{ id: 'input', at: 1, from: { kind: 'user', name: 'You' }, text: 'check', thread: 'thread' }], + watermarks: { 'thread::research': continuation ? 2 : 0 } + })) + // The normal responder has no delta; the earlier unanswered @ops + // handoff is driven by the continuation phase. + let phaseEntered!: () => void + let release!: () => void + const entered = new Promise(resolve => { phaseEntered = resolve }) + const held = new Promise(resolve => { release = resolve }) + const original = host.request as (method: string, params: Record) => Promise + host.request = async (method: string, params: Record) => { + if (method === rejectedMethod) { phaseEntered(); await held; throw new Error('401 unauthorized late rejection') } + + return original(method, params) + } + const drive = room.rounds.runGroupChatRounds('Core', members, 'thread') + await entered + await view.disbandGroupChat('Core', []) + room.chat.updateGroupChat('Core', current => ({ ...current, roomId: 'replacement-rejection-room', tombstone: false })) + room.chat.appendGroupChatEntry('Core', { kind: 'member', name: 'research' }, '@user replacement needs you') + const before = structuredClone({ rooms: room.chat.$groupChats.get(), activity: activity.$groupActivity.get(), attention: data.$botAttention.get(), needsYou: room.chat.$groupNeedsYou.get() }) + release() + await drive + expect({ rooms: room.chat.$groupChats.get(), activity: activity.$groupActivity.get(), attention: data.$botAttention.get(), needsYou: room.chat.$groupNeedsYou.get() }).toEqual(before) + expect(room.gateway.rpcFor('prompt.submit')).toHaveLength(0) + } + } + const clock = vi.spyOn(Date, 'now').mockReturnValue(0) onTestFinished(() => clock.mockRestore()) From f9993d5be25e664ef802b5fcd3c012a732eff2a7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:24:09 -0700 Subject: [PATCH 4/4] refactor: extract group member phases and roster presentation --- .../hermes-bots/cross-connection-bots.test.ts | 4 +- .../plugins/hermes-bots/group-chat.test.ts | 10 +- .../hermes-bots/group-round-members.ts | 377 +++++++ .../plugins/hermes-bots/group-round-prompt.ts | 68 ++ .../plugins/hermes-bots/group-rounds.test.ts | 20 +- .../src/plugins/hermes-bots/group-rounds.ts | 422 +------- .../src/plugins/hermes-bots/group-turns.ts | 426 ++++---- .../hermes-bots/roster-pane-content.tsx | 178 ++++ .../hermes-bots/roster-pane-derivation.ts | 248 +++++ .../hermes-bots/roster-pane-dialogs.tsx | 161 +++ .../hermes-bots/roster-pane-lifecycle.ts | 54 + .../hermes-bots/roster-pane-sections.tsx | 195 ++++ .../hermes-bots/roster-pane-toolbar.tsx | 227 +++++ .../src/plugins/hermes-bots/roster-pane.tsx | 954 ++++-------------- 14 files changed, 2005 insertions(+), 1339 deletions(-) create mode 100644 apps/desktop/src/plugins/hermes-bots/group-round-members.ts create mode 100644 apps/desktop/src/plugins/hermes-bots/group-round-prompt.ts create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-content.tsx create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-derivation.ts create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-dialogs.tsx create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-lifecycle.ts create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-sections.tsx create mode 100644 apps/desktop/src/plugins/hermes-bots/roster-pane-toolbar.tsx diff --git a/apps/desktop/src/plugins/hermes-bots/cross-connection-bots.test.ts b/apps/desktop/src/plugins/hermes-bots/cross-connection-bots.test.ts index 2c075e8348..b241c223a4 100644 --- a/apps/desktop/src/plugins/hermes-bots/cross-connection-bots.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/cross-connection-bots.test.ts @@ -43,8 +43,8 @@ vi.mock('@hermes/plugin-sdk', async importOriginal => { const { botBackendProfileScope, botConnectionRoute, requestForBot } = await import('./routing') const { groupMemberKey } = await import('./group-membership') -const { buildGroupChatTurnPrompt, formatGroupChatLine, parseGroupChatMentions, resolveGroupResponders } = - await import('./group-rounds') +const { buildGroupChatTurnPrompt, formatGroupChatLine } = await import('./group-round-prompt') +const { parseGroupChatMentions, resolveGroupResponders } = await import('./group-rounds') const aliasBot = { name: 'worker', diff --git a/apps/desktop/src/plugins/hermes-bots/group-chat.test.ts b/apps/desktop/src/plugins/hermes-bots/group-chat.test.ts index b88c7056d1..4c25e67e9b 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-chat.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-chat.test.ts @@ -102,8 +102,9 @@ describe('room naming', () => { describe('speaker labels', () => { it('the default profile speaks as Hermes in transcripts, not @default', async () => { const { rounds } = await loadRoom() + const { formatGroupChatLine } = await import('./group-round-prompt') - const line = rounds.formatGroupChatLine( + const line = formatGroupChatLine( { from: { kind: 'member', name: 'default' }, text: 'hello room' } as GroupMessage, 'builder' ) @@ -112,15 +113,16 @@ describe('speaker labels', () => { // Other members keep their profile name; the (you) suffix survives. expect( - rounds.formatGroupChatLine({ from: { kind: 'member', name: 'default' }, text: 'hi' } as GroupMessage, 'default') + formatGroupChatLine({ from: { kind: 'member', name: 'default' }, text: 'hi' } as GroupMessage, 'default') ).toBe('Hermes (you): hi') expect( - rounds.formatGroupChatLine({ from: { kind: 'member', name: 'builder' }, text: 'yo' } as GroupMessage, 'research') + formatGroupChatLine({ from: { kind: 'member', name: 'builder' }, text: 'yo' } as GroupMessage, 'research') ).toBe('builder: yo') }) it('honor friendly identity: Bot Mode title, then display_name, never a stale Hermes', async () => { const { chat, rounds } = await loadRoom() + const { formatGroupChatLine } = await import('./group-round-prompt') const data = await import('./data') // A renamed default (core display_name via `hermes profile rename`) must @@ -130,7 +132,7 @@ describe('speaker labels', () => { expect(chat.groupSpeakerLabel('default')).toBe('Lucy') expect( - rounds.formatGroupChatLine({ from: { kind: 'member', name: 'default' }, text: 'hi' } as GroupMessage, 'builder') + formatGroupChatLine({ from: { kind: 'member', name: 'default' }, text: 'hi' } as GroupMessage, 'builder') ).toBe('Lucy: hi') // A Bot Mode title outranks display_name (same precedence as displayName). diff --git a/apps/desktop/src/plugins/hermes-bots/group-round-members.ts b/apps/desktop/src/plugins/hermes-bots/group-round-members.ts new file mode 100644 index 0000000000..65838ada67 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/group-round-members.ts @@ -0,0 +1,377 @@ +import { clearBotAttention, noteBotAttention } from './data' +import { recordGroupActivity } from './group-activity' +import { + $groupChats, + appendGroupChatEntry, + GROUP_CHAT_HISTORY_LIMIT, + GROUP_CHAT_MAX_CONTINUATIONS, + GROUP_CHAT_MAX_MESSAGES, + groupThreadOf, + shouldCommitMemberTurn, + updateGroupChat +} from './group-chat' +import type { GroupChatRoom } from './group-chat' +import { groupMemberKey } from './group-membership' +import { buildGroupChatTurnPrompt, formatGroupChatLine } from './group-round-prompt' +import { isGroupPassText, runGroupChatMemberTurn } from './group-turns' +import type { GroupMember, GroupMessage } from './types' + +export interface GroupRoundMemberContext { + group: string + members: GroupMember[] + thread: string + startEpoch: number + binding: { isLive(): boolean } + isCurrent(): boolean +} + +/** #93129: a held member's skip must consume its delta exactly once — + * advance the watermark past the current log so the same entries never + * re-trigger the skip. Null = nothing to consume (no write, no spin). */ +export function heldMemberWatermarkAdvance(seen: number | undefined, logLength: number): null | number { + return logLength > (seen || 0) ? logLength : null +} + +function prepareGroupRoundMember(context: GroupRoundMemberContext, member: GroupMember) { + const { members, thread } = context + + const room = $groupChats.get()[context.group] || { + log: [], + watermarks: {} + } + + const memberKey = groupMemberKey(member) + const markKey = `${thread}::${memberKey}` + const seen = room.watermarks[markKey] || 0 + // Delta: NEW room entries, narrowed to this thread — the member's + // turn sees only the conversation it's part of. + const delta = room.log.slice(seen).filter((e: GroupMessage) => groupThreadOf(e) === thread) + + if (!delta.length) { + return null + } + + // #93129: a member the user told to stop is HELD — no turn until an + // explicit release (resume / @all resume / a direct non-stop + // mention). Consume the delta exactly once (watermark past the + // current log) so the same entries never re-trigger this skip, and + // surface WHY the bot is silent in the activity feed the first time. + const heldEntry = (room.holds || {})[memberKey] + + if (heldEntry) { + const advance = heldMemberWatermarkAdvance(seen, room.log.length) + updateGroupChat(context.group, (r: GroupChatRoom) => { + if (advance !== null) { + r.watermarks[markKey] = advance + } + + if (r.holds?.[memberKey] && !r.holds[memberKey].noted) { + r.holds = { + ...r.holds, + [memberKey]: { + ...r.holds[memberKey], + noted: true + } + } + } + + return r + }) + + if (!heldEntry.noted) { + recordGroupActivity(context.group, { + kind: 'held', + member: member.name, + thread + }) + } + + return null + } + + const prompt = buildGroupChatTurnPrompt({ + groupName: context.group, + members, + viewer: member, + deltaLines: delta.slice(-GROUP_CHAT_HISTORY_LIMIT).map((e: GroupMessage) => formatGroupChatLine(e, member.name)) + }) + + // Images riding this delta (user attachments — member entries don't + // carry images today, but flatMap keeps this future-proof) get staged + // into the member's session so the model sees the pixels, not just + // the transcript's [attached image: …] marker. + const deltaImages = delta.flatMap((e: GroupMessage) => (Array.isArray(e.images) ? e.images : [])) + + return { room, memberKey, markKey, prompt, deltaImages } +} + +export async function runGroupRoundMember( + context: GroupRoundMemberContext, + member: GroupMember +): Promise { + const { thread, startEpoch, binding } = context + const prepared = prepareGroupRoundMember(context, member) + + if (!prepared) { + return false + } + + const { room, markKey, prompt, deltaImages } = prepared + // Surface WHO is on turn (runtime-only, like running/epoch) so the + // room shows "Radar is thinking…" instead of a generic working line — + // long model turns otherwise read as the room being stuck. + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.turn = member.name + + return r + }) + let reply: null | string = null + + try { + reply = await runGroupChatMemberTurn(context.group, member, prompt, thread, deltaImages) + + // Needs-attention hook (#93091 item 3): a turn that produced a real + // reply (or an explicit pass) is a good turn — clear the badge. + // A timed-out turn also returns null but never threw; leaving any + // prior badge in place there is the conservative choice. + if (reply !== null) { + clearBotAttention(groupMemberKey(member)) + } + } catch (error: any) { + if (!binding.isLive()) { + return null + } + + const reason = String(error?.data?.reason || '').trim() + recordGroupActivity(context.group, { + kind: 'failed', + member: member.name, + thread, + ...(reason + ? { + reason + } + : {}) + }) + noteBotAttention(groupMemberKey(member), reason || error?.message || error) + reply = null // a failed turn is a pass, never a room error + } + + // #93127: the turn may have finished AFTER a newer user send bumped + // the room epoch. That newer send's loop re-drives this member with + // the full delta, so committing this stale result (watermark advance + // + append) would double-deliver the same reply. Drop it here — + // BEFORE the watermark advance and BEFORE the append. Only a newer + // USER entry in THIS thread makes the re-drive premise true: a + // cross-thread send bumps the epoch too, but its loop filters this + // thread out and would never regenerate the finished reply. The + // during-turn tail is anchored by entry id, not index — the history + // trim drops entries from the FRONT, so an index slice could + // overshoot after a mid-turn trim and silently commit a stale turn. + if (!binding.isLive()) { + return null + } + + const roomNow = $groupChats.get()[context.group] || { + log: [] + } + + const epochNow = roomNow.epoch || 0 + const anchorId = room.log.length ? room.log[room.log.length - 1].id : null + const anchorIdx = anchorId === null ? -1 : roomNow.log.findIndex((e: GroupMessage) => e.id === anchorId) + // Anchor trimmed away ⇒ every pre-turn entry was dropped, so every + // surviving entry is newer — scanning the whole log stays exact. + const turnTail = anchorIdx >= 0 ? roomNow.log.slice(anchorIdx + 1) : roomNow.log + + const newerUserEntryInThread = turnTail.some( + (e: GroupMessage) => e.from?.kind === 'user' && groupThreadOf(e) === thread + ) + + if (!shouldCommitMemberTurn(startEpoch, epochNow, newerUserEntryInThread)) { + recordGroupActivity(context.group, { + kind: 'cancelled', + member: member.name, + thread + }) + + return null + } + + // The member has now seen everything up to the pre-reply log length. + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.watermarks[markKey] = r.log.length + + return r + }) + + if (reply !== null && !isGroupPassText(reply)) { + appendGroupChatEntry( + context.group, + { + kind: 'member', + name: member.name, + ...(member.remoteSource + ? { + source: member.connectionLabel || member.connectionId + } + : {}) + }, + reply, + thread + ) + // Its own message counts as seen too. + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.watermarks[markKey] = r.log.length + + return r + }) + + return true + } + + return false +} + +async function runGroupContinuationMember( + context: GroupRoundMemberContext, + member: GroupMember +): Promise { + const { members, thread, binding, isCurrent } = context + + const room = $groupChats.get()[context.group] || { + log: [], + watermarks: {} + } + + const memberKey = groupMemberKey(member) + const markKey = `${thread}::${memberKey}` + const seen = room.watermarks[markKey] || 0 + const delta = room.log.slice(seen).filter((e: GroupMessage) => groupThreadOf(e) === thread) + + // A cited member always has delta here (the citing reply IS in + // its tail); skip defensively anyway so an empty prompt never + // fires. + if (!delta.length) { + return false + } + + const heldEntry = (room.holds || {})[memberKey] + + if (heldEntry) { + return false // holds still apply to continuation turns (#93129) + } + + const prompt = buildGroupChatTurnPrompt({ + groupName: context.group, + members, + viewer: member, + // The continuation prompt centers on what the member missed: + // everything since its watermark, which includes the reply + // that cites it. + deltaLines: delta.slice(-GROUP_CHAT_HISTORY_LIMIT).map((e: GroupMessage) => formatGroupChatLine(e, member.name)) + }) + + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.turn = member.name + + return r + }) + let continuationReply: null | string = null + + try { + continuationReply = await runGroupChatMemberTurn(context.group, member, prompt, thread) + + if (continuationReply !== null) { + clearBotAttention(memberKey) + } + } catch (error: any) { + if (!binding.isLive()) { + return null + } + + recordGroupActivity(context.group, { + kind: 'failed', + member: member.name, + thread + }) + noteBotAttention(memberKey, error?.message || error) + continuationReply = null + } + + if (!isCurrent()) { + return null + } + + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.watermarks[markKey] = r.log.length + + return r + }) + + if (continuationReply !== null && !isGroupPassText(continuationReply)) { + appendGroupChatEntry( + context.group, + { + kind: 'member', + name: member.name, + ...(member.remoteSource + ? { + source: member.connectionLabel || member.connectionId + } + : {}) + }, + continuationReply, + thread + ) + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.watermarks[markKey] = r.log.length + + return r + }) + + return true + } + + return false +} + +export async function runGroupContinuationMembers( + context: GroupRoundMemberContext, + pendingKeys: string[], + continuations: number, + posted: number +): Promise { + const { members, isCurrent } = context + let spokeThisRound = 0 + + if (pendingKeys.length && continuations <= GROUP_CHAT_MAX_CONTINUATIONS) { + const citedMembers = members.filter((member: GroupMember) => pendingKeys.includes(groupMemberKey(member))) + + if (citedMembers.length && posted < GROUP_CHAT_MAX_MESSAGES) { + const strandedNow = ($groupChats.get()[context.group] || {}).stranded || {} + + const continuationResponders = citedMembers.filter( + (member: GroupMember) => !Object.prototype.hasOwnProperty.call(strandedNow, groupMemberKey(member)) + ) + + for (const member of continuationResponders) { + if (!isCurrent() || posted >= GROUP_CHAT_MAX_MESSAGES || continuations > GROUP_CHAT_MAX_CONTINUATIONS) { + break + } + + const result = await runGroupContinuationMember(context, member) + + if (result === null) { + return null + } + + if (result) { + posted += 1 + spokeThisRound += 1 + } + } + } + } + + return spokeThisRound +} diff --git a/apps/desktop/src/plugins/hermes-bots/group-round-prompt.ts b/apps/desktop/src/plugins/hermes-bots/group-round-prompt.ts new file mode 100644 index 0000000000..1ee31a6fa1 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/group-round-prompt.ts @@ -0,0 +1,68 @@ +import { botHandle } from './data' +import { groupSpeakerLabel } from './group-chat' +import { groupMemberKey } from './group-membership' +import type { GroupMember, GroupMessage } from './types' + +/** Room-log line as a member sees it: `Name (user): …` / `Name: …` / + * `Name (you): …`. */ +export function formatGroupChatLine(entry: GroupMessage, viewerName: string) { + // Attachments are staged into each member's session as real payloads; the + // transcript line names them so the delta text and the bytes line up. + const attached = + Array.isArray(entry.images) && entry.images.length + ? ` ${entry.images + .map(img => { + const label = img.kind === 'pdf' ? 'attached PDF' : img.kind === 'file' ? 'attached file' : 'attached image' + + return `[${label}: ${img.name || 'image'}]` + }) + .join(' ')}` + : '' + + if (entry.from.kind === 'user') { + return `${entry.from.name || 'User'} (user): ${entry.text}${attached}` + } + + const suffix = entry.from.name === viewerName ? ' (you)' : '' + // Cross-connection speakers carry their device so same-named agents on + // two machines stay tellable apart in every member's transcript. + const source = entry.from.source ? ` [${entry.from.source}]` : '' + + return `${groupSpeakerLabel(entry.from.name)}${suffix}${source}: ${entry.text}${attached}` +} + +interface GroupChatTurnPromptInput { + deltaLines: string[] + groupName: string + members: GroupMember[] + viewer: GroupMember +} + +/** The full per-turn payload for one member: participation rules + the room + * delta. Rules travel in the turn payload (not SOUL) so every existing bot + * can join a group chat without a profile migration. */ +export function buildGroupChatTurnPrompt({ groupName, members, viewer, deltaLines }: GroupChatTurnPromptInput) { + const viewerKey = groupMemberKey(viewer) + const peers = members.filter(m => groupMemberKey(m) !== viewerKey) + + const peerNames = peers + .map(m => { + const handle = m.title ? `${m.title} (@${botHandle(m.name, m)})` : `@${botHandle(m.name, m)}` + + return m.remoteSource ? `${handle} [on ${m.connectionLabel || m.connectionId}]` : handle + }) + .join(', ') + + return [ + `[Group chat: "${groupName}"] You are @${botHandle(viewer.name, viewer)}, one participant in a group chat with ${peerNames || 'no one else yet'} and the user.`, + '', + 'New messages in the room since your last turn (oldest first):', + ...deltaLines.map(line => ` ${line}`), + '', + 'Rules for this room:', + '- Reply with ONE conversational message ONLY if you have something new worth adding: build on what was just said, claim or hand off work, answer a question aimed at you, or report a real result. Keep chatter short (1-3 sentences) — but when you are delivering a result, an answer the user asked for, or substantive work, give it at full quality and length; never thin out real content to fit the room.', + '- If you have nothing new to add, reply with exactly "(pass)". Passing is good — it lets the conversation settle.', + '- Mention a teammate as @name to pull them in; mention @user only for a judgment call or a result the user needs. Do not repeat points already made.', + '- Never reveal content from your private 1:1 chats. Your reply text goes to the room verbatim — no preamble, no meta-commentary.' + ].join('\n') +} diff --git a/apps/desktop/src/plugins/hermes-bots/group-rounds.test.ts b/apps/desktop/src/plugins/hermes-bots/group-rounds.test.ts index fd9ec778bf..eb99b09e32 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-rounds.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-rounds.test.ts @@ -349,13 +349,14 @@ describe('threads', () => { describe('turn prompt', () => { it('addresses the default profile as @hermes', async () => { const { rounds } = await loadRoom() + const { buildGroupChatTurnPrompt } = await import('./group-round-prompt') const members: GroupMember[] = [ { name: 'default', title: '' }, { name: 'builder', title: '' } ] - const own = rounds.buildGroupChatTurnPrompt({ + const own = buildGroupChatTurnPrompt({ deltaLines: [], groupName: 'Core', members, @@ -365,7 +366,7 @@ describe('turn prompt', () => { expect(own).toMatch(/You are @hermes,/) expect(own).not.toMatch(/@default\b/) - const peer = rounds.buildGroupChatTurnPrompt({ + const peer = buildGroupChatTurnPrompt({ deltaLines: [], groupName: 'Core', members, @@ -377,8 +378,9 @@ describe('turn prompt', () => { it('asks for full-quality results and short chatter, not short results', async () => { const { rounds } = await loadRoom() + const { buildGroupChatTurnPrompt } = await import('./group-round-prompt') - const prompt = rounds.buildGroupChatTurnPrompt({ + const prompt = buildGroupChatTurnPrompt({ deltaLines: [], groupName: 'Core', members: [ @@ -560,9 +562,10 @@ describe('attachments', () => { it('names attachments in the transcript line, labelling PDFs and files distinctly', async () => { const { rounds } = await loadRoom() + const { formatGroupChatLine } = await import('./group-round-prompt') const pdf: Attachment = { data: 'data:application/pdf;base64,JVBERi0=', kind: 'pdf', name: 'spec.pdf' } const doc: Attachment = { data: 'data:text/plain;base64,aGVsbG8=', kind: 'file', name: 'notes.txt' } - const line = (entry: Partial) => rounds.formatGroupChatLine(entry as GroupMessage, 'research') + const line = (entry: Partial) => formatGroupChatLine(entry as GroupMessage, 'research') expect(line({ from: { kind: 'user', name: 'You' }, images: [IMG], text: 'see attached' })).toBe( 'You (user): see attached [attached image: screenshot.png]' @@ -692,14 +695,15 @@ describe('member holds (#93129)', () => { it('consumes a held skip exactly once so the loop cannot spin', async () => { const { rounds } = await loadRoom() + const { heldMemberWatermarkAdvance } = await import('./group-round-members') // Fresh delta → advance to log length. - expect(rounds.heldMemberWatermarkAdvance(3, 7)).toBe(7) + expect(heldMemberWatermarkAdvance(3, 7)).toBe(7) // Already consumed → no write, no spin. - expect(rounds.heldMemberWatermarkAdvance(7, 7)).toBeNull() - expect(rounds.heldMemberWatermarkAdvance(9, 7)).toBeNull() + expect(heldMemberWatermarkAdvance(7, 7)).toBeNull() + expect(heldMemberWatermarkAdvance(9, 7)).toBeNull() // Unset watermark treated as 0. - expect(rounds.heldMemberWatermarkAdvance(undefined, 2)).toBe(2) + expect(heldMemberWatermarkAdvance(undefined, 2)).toBe(2) }) }) diff --git a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts index 78a9e90c2a..da258c7474 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-rounds.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-rounds.ts @@ -1,28 +1,25 @@ +import { botFriendlyNames, botHandle, mentionNameForms } from './data' /** * Room-level coordination: who speaks, in what order, for how long — the * @mention parse, the round-robin driver, the #93129 member holds, the stop * path, and the user send that starts it all. */ - -import { botFriendlyNames, botHandle, clearBotAttention, mentionNameForms, noteBotAttention } from './data' import { recordGroupActivity } from './group-activity' import { $groupChats, $groupNeedsYou, appendGroupChatEntry, - GROUP_CHAT_HISTORY_LIMIT, GROUP_CHAT_MAX_CONTINUATIONS, GROUP_CHAT_MAX_MESSAGES, GROUP_CHAT_MAX_ROUNDS, - groupSpeakerLabel, groupThreadOf, mintGroupThreadId, - shouldCommitMemberTurn, updateGroupChat } from './group-chat' import type { GroupChatRoom, GroupHoldStamp } from './group-chat' import { durableGroupChatMembers, followGroupChat, groupMemberKey } from './group-membership' -import { harvestStrandedGroupReply, isGroupPassText, runGroupChatMemberTurn } from './group-turns' +import { runGroupContinuationMembers, runGroupRoundMember } from './group-round-members' +import { harvestStrandedGroupReply } from './group-turns' import { requestForBot } from './routing' import type { Attachment, GroupMember, GroupMessage } from './types' @@ -154,70 +151,6 @@ export function rotateGroupSpeakers(members: GroupMember[], round: number) { return [...members.slice(shift), ...members.slice(0, shift)] } -/** Room-log line as a member sees it: `Name (user): …` / `Name: …` / - * `Name (you): …`. */ -export function formatGroupChatLine(entry: GroupMessage, viewerName: string) { - // Attachments are staged into each member's session as real payloads; the - // transcript line names them so the delta text and the bytes line up. - const attached = - Array.isArray(entry.images) && entry.images.length - ? ` ${entry.images - .map(img => { - const label = img.kind === 'pdf' ? 'attached PDF' : img.kind === 'file' ? 'attached file' : 'attached image' - - return `[${label}: ${img.name || 'image'}]` - }) - .join(' ')}` - : '' - - if (entry.from.kind === 'user') { - return `${entry.from.name || 'User'} (user): ${entry.text}${attached}` - } - - const suffix = entry.from.name === viewerName ? ' (you)' : '' - // Cross-connection speakers carry their device so same-named agents on - // two machines stay tellable apart in every member's transcript. - const source = entry.from.source ? ` [${entry.from.source}]` : '' - - return `${groupSpeakerLabel(entry.from.name)}${suffix}${source}: ${entry.text}${attached}` -} - -interface GroupChatTurnPromptInput { - deltaLines: string[] - groupName: string - members: GroupMember[] - viewer: GroupMember -} - -/** The full per-turn payload for one member: participation rules + the room - * delta. Rules travel in the turn payload (not SOUL) so every existing bot - * can join a group chat without a profile migration. */ -export function buildGroupChatTurnPrompt({ groupName, members, viewer, deltaLines }: GroupChatTurnPromptInput) { - const viewerKey = groupMemberKey(viewer) - const peers = members.filter(m => groupMemberKey(m) !== viewerKey) - - const peerNames = peers - .map(m => { - const handle = m.title ? `${m.title} (@${botHandle(m.name, m)})` : `@${botHandle(m.name, m)}` - - return m.remoteSource ? `${handle} [on ${m.connectionLabel || m.connectionId}]` : handle - }) - .join(', ') - - return [ - `[Group chat: "${groupName}"] You are @${botHandle(viewer.name, viewer)}, one participant in a group chat with ${peerNames || 'no one else yet'} and the user.`, - '', - 'New messages in the room since your last turn (oldest first):', - ...deltaLines.map(line => ` ${line}`), - '', - 'Rules for this room:', - '- Reply with ONE conversational message ONLY if you have something new worth adding: build on what was just said, claim or hand off work, answer a question aimed at you, or report a real result. Keep chatter short (1-3 sentences) — but when you are delivering a result, an answer the user asked for, or substantive work, give it at full quality and length; never thin out real content to fit the room.', - '- If you have nothing new to add, reply with exactly "(pass)". Passing is good — it lets the conversation settle.', - '- Mention a teammate as @name to pull them in; mention @user only for a judgment call or a result the user needs. Do not repeat points already made.', - '- Never reveal content from your private 1:1 chats. Your reply text goes to the room verbatim — no preamble, no meta-commentary.' - ].join('\n') -} - // --- member-hold helpers (#93129) — pure, unit-tested --- /** #93129: classify a USER room message's effect on member holds. Only user @@ -325,13 +258,6 @@ export function applyGroupHoldDirective( return next } -/** #93129: a held member's skip must consume its delta exactly once — - * advance the watermark past the current log so the same entries never - * re-trigger the skip. Null = nothing to consume (no write, no spin). */ -export function heldMemberWatermarkAdvance(seen: number | undefined, logLength: number): null | number { - return logLength > (seen || 0) ? logLength : null -} - // --- end member-hold helpers --- /** Members cited by @mention in a thread who have not posted any entry after @@ -493,9 +419,24 @@ export async function stopGroupThread(group: string, thread: null | string, memb * Watermarks are per thread+member (`${thread}::${memberKey}`), so parallel * topics never eat each other's deltas. */ export async function runGroupChatRounds(group: string, members: GroupMember[], thread: string) { - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) + const startEpoch = ($groupChats.get()[group] || {}).epoch || 0 const isCurrent = () => binding.isLive() && (($groupChats.get()[group] || {}).epoch || 0) === startEpoch + + const context = { + get group() { + return group + }, + members, + thread, + startEpoch, + binding, + isCurrent + } + let posted = 0 let continuations = 0 // #94478: how this drive ended. 'settled' means quiet consensus (everyone @@ -565,183 +506,13 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], return } - const room = $groupChats.get()[group] || { - log: [], - watermarks: {} - } + const result = await runGroupRoundMember(context, member) - const memberKey = groupMemberKey(member) - const markKey = `${thread}::${memberKey}` - const seen = room.watermarks[markKey] || 0 - // Delta: NEW room entries, narrowed to this thread — the member's - // turn sees only the conversation it's part of. - const delta = room.log.slice(seen).filter((e: GroupMessage) => groupThreadOf(e) === thread) - - if (!delta.length) { - continue - } - - // #93129: a member the user told to stop is HELD — no turn until an - // explicit release (resume / @all resume / a direct non-stop - // mention). Consume the delta exactly once (watermark past the - // current log) so the same entries never re-trigger this skip, and - // surface WHY the bot is silent in the activity feed the first time. - const heldEntry = (room.holds || {})[memberKey] - - if (heldEntry) { - const advance = heldMemberWatermarkAdvance(seen, room.log.length) - updateGroupChat(group, (r: GroupChatRoom) => { - if (advance !== null) { - r.watermarks[markKey] = advance - } - - if (r.holds?.[memberKey] && !r.holds[memberKey].noted) { - r.holds = { - ...r.holds, - [memberKey]: { - ...r.holds[memberKey], - noted: true - } - } - } - - return r - }) - - if (!heldEntry.noted) { - recordGroupActivity(group, { - kind: 'held', - member: member.name, - thread - }) - } - - continue - } - - const prompt = buildGroupChatTurnPrompt({ - groupName: group, - members, - viewer: member, - deltaLines: delta - .slice(-GROUP_CHAT_HISTORY_LIMIT) - .map((e: GroupMessage) => formatGroupChatLine(e, member.name)) - }) - - // Images riding this delta (user attachments — member entries don't - // carry images today, but flatMap keeps this future-proof) get staged - // into the member's session so the model sees the pixels, not just - // the transcript's [attached image: …] marker. - const deltaImages = delta.flatMap((e: GroupMessage) => (Array.isArray(e.images) ? e.images : [])) - - // Surface WHO is on turn (runtime-only, like running/epoch) so the - // room shows "Radar is thinking…" instead of a generic working line — - // long model turns otherwise read as the room being stuck. - updateGroupChat(group, (r: GroupChatRoom) => { - r.turn = member.name - - return r - }) - let reply: null | string = null - - try { - reply = await runGroupChatMemberTurn(group, member, prompt, thread, deltaImages) - - // Needs-attention hook (#93091 item 3): a turn that produced a real - // reply (or an explicit pass) is a good turn — clear the badge. - // A timed-out turn also returns null but never threw; leaving any - // prior badge in place there is the conservative choice. - if (reply !== null) { - clearBotAttention(groupMemberKey(member)) - } - } catch (error: any) { - if (!binding.isLive()) { - return - } - - const reason = String(error?.data?.reason || '').trim() - recordGroupActivity(group, { - kind: 'failed', - member: member.name, - thread, - ...(reason - ? { - reason - } - : {}) - }) - noteBotAttention(groupMemberKey(member), reason || error?.message || error) - reply = null // a failed turn is a pass, never a room error - } - - // #93127: the turn may have finished AFTER a newer user send bumped - // the room epoch. That newer send's loop re-drives this member with - // the full delta, so committing this stale result (watermark advance - // + append) would double-deliver the same reply. Drop it here — - // BEFORE the watermark advance and BEFORE the append. Only a newer - // USER entry in THIS thread makes the re-drive premise true: a - // cross-thread send bumps the epoch too, but its loop filters this - // thread out and would never regenerate the finished reply. The - // during-turn tail is anchored by entry id, not index — the history - // trim drops entries from the FRONT, so an index slice could - // overshoot after a mid-turn trim and silently commit a stale turn. - if (!binding.isLive()) { + if (!binding.isLive() || result === null) { return } - const roomNow = $groupChats.get()[group] || { - log: [] - } - - const epochNow = roomNow.epoch || 0 - const anchorId = room.log.length ? room.log[room.log.length - 1].id : null - const anchorIdx = anchorId === null ? -1 : roomNow.log.findIndex((e: GroupMessage) => e.id === anchorId) - // Anchor trimmed away ⇒ every pre-turn entry was dropped, so every - // surviving entry is newer — scanning the whole log stays exact. - const turnTail = anchorIdx >= 0 ? roomNow.log.slice(anchorIdx + 1) : roomNow.log - - const newerUserEntryInThread = turnTail.some( - (e: GroupMessage) => e.from?.kind === 'user' && groupThreadOf(e) === thread - ) - - if (!shouldCommitMemberTurn(startEpoch, epochNow, newerUserEntryInThread)) { - recordGroupActivity(group, { - kind: 'cancelled', - member: member.name, - thread - }) - - return - } - - // The member has now seen everything up to the pre-reply log length. - updateGroupChat(group, (r: GroupChatRoom) => { - r.watermarks[markKey] = r.log.length - - return r - }) - - if (reply !== null && !isGroupPassText(reply)) { - appendGroupChatEntry( - group, - { - kind: 'member', - name: member.name, - ...(member.remoteSource - ? { - source: member.connectionLabel || member.connectionId - } - : {}) - }, - reply, - thread - ) - // Its own message counts as seen too. - updateGroupChat(group, (r: GroupChatRoom) => { - r.watermarks[markKey] = r.log.length - - return r - }) + if (result) { posted += 1 spokeThisRound += 1 } @@ -762,126 +533,15 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], // room's entire budget on back-and-forth handoffs. continuations += 1 - if (pendingKeys.length && continuations <= GROUP_CHAT_MAX_CONTINUATIONS) { - const citedMembers = members.filter((member: GroupMember) => pendingKeys.includes(groupMemberKey(member))) + const continued = await runGroupContinuationMembers(context, pendingKeys, continuations, posted) - if (citedMembers.length && posted < GROUP_CHAT_MAX_MESSAGES) { - const strandedNow = ($groupChats.get()[group] || {}).stranded || {} - - const continuationResponders = citedMembers.filter( - (member: GroupMember) => !Object.prototype.hasOwnProperty.call(strandedNow, groupMemberKey(member)) - ) - - for (const member of continuationResponders) { - if (!isCurrent() || posted >= GROUP_CHAT_MAX_MESSAGES || continuations > GROUP_CHAT_MAX_CONTINUATIONS) { - break - } - - const room = $groupChats.get()[group] || { - log: [], - watermarks: {} - } - - const memberKey = groupMemberKey(member) - const markKey = `${thread}::${memberKey}` - const seen = room.watermarks[markKey] || 0 - const delta = room.log.slice(seen).filter((e: GroupMessage) => groupThreadOf(e) === thread) - - // A cited member always has delta here (the citing reply IS in - // its tail); skip defensively anyway so an empty prompt never - // fires. - if (!delta.length) { - continue - } - - const heldEntry = (room.holds || {})[memberKey] - - if (heldEntry) { - continue // holds still apply to continuation turns (#93129) - } - - const prompt = buildGroupChatTurnPrompt({ - groupName: group, - members, - viewer: member, - // The continuation prompt centers on what the member missed: - // everything since its watermark, which includes the reply - // that cites it. - deltaLines: delta - .slice(-GROUP_CHAT_HISTORY_LIMIT) - .map((e: GroupMessage) => formatGroupChatLine(e, member.name)) - }) - - updateGroupChat(group, (r: GroupChatRoom) => { - r.turn = member.name - - return r - }) - let continuationReply: null | string = null - - try { - continuationReply = await runGroupChatMemberTurn(group, member, prompt, thread) - - if (continuationReply !== null) { - clearBotAttention(memberKey) - } - } catch (error: any) { - if (!binding.isLive()) { - return - } - - recordGroupActivity(group, { - kind: 'failed', - member: member.name, - thread - }) - noteBotAttention(memberKey, error?.message || error) - continuationReply = null - } - - if (!isCurrent()) { - return - } - - updateGroupChat(group, (r: GroupChatRoom) => { - r.watermarks[markKey] = r.log.length - - return r - }) - - if (continuationReply !== null && !isGroupPassText(continuationReply)) { - appendGroupChatEntry( - group, - { - kind: 'member', - name: member.name, - ...(member.remoteSource - ? { - source: member.connectionLabel || member.connectionId - } - : {}) - }, - continuationReply, - thread - ) - updateGroupChat(group, (r: GroupChatRoom) => { - r.watermarks[markKey] = r.log.length - - return r - }) - posted += 1 - - // The continuation's own reply may cite someone else — fall - // through to the normal loop so the next round handles it via - // the same responder machinery. Reaching here means the loop - // continues rather than settling; the outer for-loop's next - // iteration re-evaluates everything. - spokeThisRound += 1 - } - } - } + if (!binding.isLive() || continued === null) { + return } + posted += continued + spokeThisRound += continued + if (spokeThisRound === 0) { // Genuinely nothing left to say — including after the continuation // attempt above produced no spoken turns. Settle honestly, but if @@ -938,7 +598,9 @@ export async function runGroupChatRounds(group: string, members: GroupMember[], * stranded, a new loop takes the room over (it harvests on its own), or the * room record disappears (disband). */ async function harvestStrandedUntilSettled(group: string, members: GroupMember[], thread: string) { - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) try { const HARVEST_INTERVAL_MS = 5000 @@ -1059,7 +721,9 @@ export function sendToGroupChat( thread: target }) - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) const drive = () => { if (!binding.isLive()) { @@ -1068,15 +732,17 @@ export function sendToGroupChat( return } - void runGroupChatRounds(group, members, target).catch(() => { - if (binding.isLive()) { - updateGroupChat(group, (r: GroupChatRoom) => { - r.running = false + void runGroupChatRounds(group, members, target) + .catch(() => { + if (binding.isLive()) { + updateGroupChat(group, (r: GroupChatRoom) => { + r.running = false - return r - }) - } - }).finally(binding.dispose) + return r + }) + } + }) + .finally(binding.dispose) } if (!wasRunning) { diff --git a/apps/desktop/src/plugins/hermes-bots/group-turns.ts b/apps/desktop/src/plugins/hermes-bots/group-turns.ts index 67f737ba87..e2cc962203 100644 --- a/apps/desktop/src/plugins/hermes-bots/group-turns.ts +++ b/apps/desktop/src/plugins/hermes-bots/group-turns.ts @@ -123,7 +123,9 @@ interface GroupMemberSessionHandle { * it after restarts. Cross-connection members route to their OWN source * via requestForBot; the window's gateway never switches. */ export async function ensureGroupChatSession(group: string, member: GroupMember): Promise { - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) try { const room = $groupChats.get()[group] || {} @@ -195,7 +197,9 @@ export async function ensureGroupChatSession(group: string, member: GroupMember) } catch (error: any) { if (error?.code !== 4007) { const detail = error instanceof Error && error.message ? ` (${error.message})` : '' - throw new Error(`Could not check ${member?.name || 'member'}'s group session${detail} — not starting a new one`) + throw new Error( + `Could not check ${member?.name || 'member'}'s group session${detail} — not starting a new one` + ) } /* genuinely doesn't exist (4007) — try the next target / fall through to create */ } @@ -597,7 +601,10 @@ export async function answerGroupClarify( answers: Record | string | undefined ) { let group = entry.group - const binding = followGroupChat(group, name => { group = name }) + + const binding = followGroupChat(group, name => { + group = name + }) try { if (entry.kind === 'approval') { @@ -662,7 +669,10 @@ export async function runGroupChatMemberTurn( // lease, every RPC below rides its own request-scoped socket lease; the // socket that minted `runtime` can close between RPCs, the gateway reaps // the runtime session, and prompt.submit dies 4001 — the bot goes silent. - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) + let releaseTurnLease: (() => void) | undefined try { @@ -675,6 +685,215 @@ export async function runGroupChatMemberTurn( } } +async function stageGroupTurnAttachments(member: GroupMember, runtime: string, images?: Attachment[]) { + // Stage this delta's attachments into the member's session so the model + // receives the actual payload with the prompt — the same attach RPCs the + // 1:1 chat uses (they also work cross-connection, where the member's + // gateway can't see this machine's files). Images queue as vision tiles, + // PDFs render per-page via pdf.attach, and other files materialize in the + // session workspace (their @file: refs are appended to the prompt so the + // member's file tools can read them). A failed attach degrades that + // member to text-only; the transcript line still names the attachment so + // the member knows something was shared. + const fileRefs: string[] = [] + + for (const img of Array.isArray(images) ? images : []) { + if (!img || typeof img.data !== 'string' || !img.data) { + continue + } + + try { + if (img.kind === 'pdf') { + await requestForBot(member, 'pdf.attach', { + session_id: runtime, + content_base64: img.data, + filename: img.name || 'attachment.pdf' + }) + } else if (img.kind === 'file') { + const res = (await requestForBot(member, 'file.attach', { + session_id: runtime, + data_url: img.data, + name: img.name || 'attachment' + })) as { ref_text?: string } + + if (res?.ref_text) { + fileRefs.push(`${img.name || 'attachment'} → ${res.ref_text}`) + } + } else { + await requestForBot(member, 'image.attach_bytes', { + session_id: runtime, + content_base64: img.data, + filename: img.name || 'attachment.png' + }) + } + } catch { + /* text-only fallback for this member */ + } + } + + return fileRefs +} + +interface GroupTurnPollContext { + group: string + member: GroupMember + thread: string + dispatchEpoch: number + stored: GroupMemberSessionHandle['stored'] + liveRuntime: string + runtimeIds: Set + before: number + binding: { isLive(): boolean } +} + +async function pollGroupMemberTurn(context: GroupTurnPollContext): Promise { + const { member, thread, dispatchEpoch, stored, liveRuntime, runtimeIds, before, binding } = context + const memberKey = groupMemberKey(member) + const started = Date.now() + let deadline = started + GROUP_TURN_TIMEOUT_MS + // After the terminal frame fires, the gateway still has to flip + // session.running off in its turn `finally` — re-check quickly for a few + // beats instead of falling back to the slow backstop cadence. + let quickRechecks = 0 + + while (Date.now() < deadline) { + const signalled = quickRechecks + ? await waitForTurnSignal([], GROUP_TURN_SETTLE_RECHECK_MS) + : await waitForTurnSignal([...runtimeIds], GROUP_TURN_POLL_MS) + + quickRechecks = signalled ? GROUP_TURN_SETTLE_RECHECKS : Math.max(0, quickRechecks - 1) + + // #91868/#94569: an explicit stop (stopGroupThread) bumped the epoch AND + // held this member — the member's session was interrupted, so nothing is + // coming; abandon the poll instead of grinding until the deadline. Both + // conditions on purpose: an ordinary newer send bumps the epoch WITHOUT + // a hold, and that turn must keep polling so finished work can still be + // delivered (the #93127 commit check decides its fate, not this loop). + if (!binding.isLive()) { + return null + } + + const roomDuringPoll = $groupChats.get()[context.group] || {} + + if ((roomDuringPoll.epoch || 0) !== dispatchEpoch && (roomDuringPoll.holds || {})[memberKey]) { + return null + } + + let state: GroupSessionSnapshot | null = null + + try { + state = (await requestForBot(member, 'session.resume', { + session_id: stored || liveRuntime, + profile: member.name + })) as GroupSessionSnapshot + } catch { + continue + } + + if (!binding.isLive()) { + return null + } + + if (state?.session_id) { + runtimeIds.add(state.session_id) + } + + const messages = Array.isArray(state?.messages) ? state.messages : [] + const busy = Boolean(state?.inflight || state?.running) + // A clarify blocking inside the member's session is a question for the + // HUMAN (#90694) — mirror it into the room store so a card renders, and + // hold the turn open: the member isn't stalling, it's waiting on us. + const awaitingUser = syncGroupClarify(context.group, member, state) + const done = !busy && !awaitingUser + + if (messages.length > before && done) { + const replyText = pickGroupTurnReply(messages, before) + + if (replyText !== null) { + recordGroupActivity(context.group, { + kind: isGroupPassText(replyText) ? 'passed' : 'replied', + member: member.name, + thread + }) + + return replyText + } + + recordGroupActivity(context.group, { + kind: 'passed', + member: member.name, + thread + }) + + return null + } + + // Still visibly working — or waiting on the user's answer to a clarify: + // extend the deadline (never past the hard cap). A pending question must + // outlive the base turn timeout or it dies unanswered at 3 minutes. + if (busy || awaitingUser) { + deadline = Math.min(started + GROUP_TURN_HARD_CAP_MS, Math.max(deadline, Date.now() + GROUP_TURN_TIMEOUT_MS)) + } + } + + if (!binding.isLive()) { + return null + } + + // Timeout — clear any still-mirrored question card (the server-side + // clarify timeout runs its own course) and read as a pass, but remember the baseline + thread + // (runtime-only) so the finished reply can be posted late into the RIGHT + // thread instead of vanishing. + recordGroupActivity(context.group, { + kind: 'timed-out', + member: member.name, + thread + }) + syncGroupClarify(context.group, member, null) + updateGroupChat(context.group, (r: GroupChatRoom) => { + r.stranded = { + ...(r.stranded || {}), + [groupMemberKey(member)]: { + before, + thread + } + } + + return r + }) + + return null +} + +async function prepareGroupTurnBaseline( + member: GroupMember, + runtime: string, + stored: GroupMemberSessionHandle['stored'] +) { + // Baseline: how many messages exist before our submit. + let before = 0 + // Every runtime id this turn has seen for the member's session. Terminal + // frames are keyed by runtime id, and a resume can hand back a fresh one. + const runtimeIds = new Set([runtime]) + + try { + const pre = (await requestForBot(member, 'session.resume', { + session_id: stored || runtime, + profile: member.name + })) as GroupSessionSnapshot + + before = Array.isArray(pre?.messages) ? pre.messages.length : pre?.message_count || 0 + + if (pre?.session_id) { + runtimeIds.add(pre.session_id) + } + } catch { + /* lazy session — zero messages */ + } + + return { before, runtimeIds } +} + async function runGroupChatMemberTurnLeased( group: string, member: GroupMember, @@ -682,7 +901,9 @@ async function runGroupChatMemberTurnLeased( thread: string, images?: Attachment[] ): Promise { - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) try { const { runtime, stored } = await ensureGroupChatSession(group, member) @@ -694,78 +915,15 @@ async function runGroupChatMemberTurnLeased( // #91868/#94569: remember the epoch this turn was dispatched under so the // poll loop below can tell an explicit stop from ordinary room churn. const dispatchEpoch = ($groupChats.get()[group] || {}).epoch || 0 - const memberKey = groupMemberKey(member) recordGroupActivity(group, { kind: 'working', member: member.name, thread }) - // Baseline: how many messages exist before our submit. - let before = 0 - // Every runtime id this turn has seen for the member's session. Terminal - // frames are keyed by runtime id, and a resume can hand back a fresh one. - const runtimeIds = new Set([runtime]) + const { before, runtimeIds } = await prepareGroupTurnBaseline(member, runtime, stored) - try { - const pre = (await requestForBot(member, 'session.resume', { - session_id: stored || runtime, - profile: member.name - })) as GroupSessionSnapshot - - before = Array.isArray(pre?.messages) ? pre.messages.length : pre?.message_count || 0 - - if (pre?.session_id) { - runtimeIds.add(pre.session_id) - } - } catch { - /* lazy session — zero messages */ - } - - // Stage this delta's attachments into the member's session so the model - // receives the actual payload with the prompt — the same attach RPCs the - // 1:1 chat uses (they also work cross-connection, where the member's - // gateway can't see this machine's files). Images queue as vision tiles, - // PDFs render per-page via pdf.attach, and other files materialize in the - // session workspace (their @file: refs are appended to the prompt so the - // member's file tools can read them). A failed attach degrades that - // member to text-only; the transcript line still names the attachment so - // the member knows something was shared. - const fileRefs: string[] = [] - - for (const img of Array.isArray(images) ? images : []) { - if (!img || typeof img.data !== 'string' || !img.data) { - continue - } - - try { - if (img.kind === 'pdf') { - await requestForBot(member, 'pdf.attach', { - session_id: runtime, - content_base64: img.data, - filename: img.name || 'attachment.pdf' - }) - } else if (img.kind === 'file') { - const res = (await requestForBot(member, 'file.attach', { - session_id: runtime, - data_url: img.data, - name: img.name || 'attachment' - })) as { ref_text?: string } - - if (res?.ref_text) { - fileRefs.push(`${img.name || 'attachment'} → ${res.ref_text}`) - } - } else { - await requestForBot(member, 'image.attach_bytes', { - session_id: runtime, - content_base64: img.data, - filename: img.name || 'attachment.png' - }) - } - } catch { - /* text-only fallback for this member */ - } - } + const fileRefs = await stageGroupTurnAttachments(member, runtime, images) if (!binding.isLive()) { return null @@ -785,120 +943,20 @@ async function runGroupChatMemberTurnLeased( } runtimeIds.add(liveRuntime) - const started = Date.now() - let deadline = started + GROUP_TURN_TIMEOUT_MS - // After the terminal frame fires, the gateway still has to flip - // session.running off in its turn `finally` — re-check quickly for a few - // beats instead of falling back to the slow backstop cadence. - let quickRechecks = 0 - while (Date.now() < deadline) { - const signalled = quickRechecks - ? await waitForTurnSignal([], GROUP_TURN_SETTLE_RECHECK_MS) - : await waitForTurnSignal([...runtimeIds], GROUP_TURN_POLL_MS) - - quickRechecks = signalled ? GROUP_TURN_SETTLE_RECHECKS : Math.max(0, quickRechecks - 1) - - // #91868/#94569: an explicit stop (stopGroupThread) bumped the epoch AND - // held this member — the member's session was interrupted, so nothing is - // coming; abandon the poll instead of grinding until the deadline. Both - // conditions on purpose: an ordinary newer send bumps the epoch WITHOUT - // a hold, and that turn must keep polling so finished work can still be - // delivered (the #93127 commit check decides its fate, not this loop). - if (!binding.isLive()) { - return null - } - - const roomDuringPoll = $groupChats.get()[group] || {} - - if ((roomDuringPoll.epoch || 0) !== dispatchEpoch && (roomDuringPoll.holds || {})[memberKey]) { - return null - } - - let state: GroupSessionSnapshot | null = null - - try { - state = (await requestForBot(member, 'session.resume', { - session_id: stored || liveRuntime, - profile: member.name - })) as GroupSessionSnapshot - } catch { - continue - } - - if (!binding.isLive()) { - return null - } - - if (state?.session_id) { - runtimeIds.add(state.session_id) - } - - const messages = Array.isArray(state?.messages) ? state.messages : [] - const busy = Boolean(state?.inflight || state?.running) - // A clarify blocking inside the member's session is a question for the - // HUMAN (#90694) — mirror it into the room store so a card renders, and - // hold the turn open: the member isn't stalling, it's waiting on us. - const awaitingUser = syncGroupClarify(group, member, state) - const done = !busy && !awaitingUser - - if (messages.length > before && done) { - const replyText = pickGroupTurnReply(messages, before) - - if (replyText !== null) { - recordGroupActivity(group, { - kind: isGroupPassText(replyText) ? 'passed' : 'replied', - member: member.name, - thread - }) - - return replyText - } - - recordGroupActivity(group, { - kind: 'passed', - member: member.name, - thread - }) - - return null - } - - // Still visibly working — or waiting on the user's answer to a clarify: - // extend the deadline (never past the hard cap). A pending question must - // outlive the base turn timeout or it dies unanswered at 3 minutes. - if (busy || awaitingUser) { - deadline = Math.min(started + GROUP_TURN_HARD_CAP_MS, Math.max(deadline, Date.now() + GROUP_TURN_TIMEOUT_MS)) - } - } - - if (!binding.isLive()) { - return null - } - - // Timeout — clear any still-mirrored question card (the server-side - // clarify timeout runs its own course) and read as a pass, but remember the baseline + thread - // (runtime-only) so the finished reply can be posted late into the RIGHT - // thread instead of vanishing. - recordGroupActivity(group, { - kind: 'timed-out', - member: member.name, - thread + return await pollGroupMemberTurn({ + get group() { + return group + }, + member, + thread, + dispatchEpoch, + stored, + liveRuntime, + runtimeIds, + before, + binding }) - syncGroupClarify(group, member, null) - updateGroupChat(group, (r: GroupChatRoom) => { - r.stranded = { - ...(r.stranded || {}), - [groupMemberKey(member)]: { - before, - thread - } - } - - return r - }) - - return null } finally { binding.dispose() } @@ -908,7 +966,9 @@ async function runGroupChatMemberTurnLeased( * after we stopped waiting. Called at the member's next turn boundary and * on user sends, so long-running work is delivered late rather than lost. */ export async function harvestStrandedGroupReply(group: string, member: GroupMember) { - const binding = followGroupChat(group, name => { group = name }) + const binding = followGroupChat(group, name => { + group = name + }) try { const memberKey = groupMemberKey(member) diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-content.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane-content.tsx new file mode 100644 index 0000000000..69ff4580e1 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-content.tsx @@ -0,0 +1,178 @@ +import { Button, Codicon, DisclosureCaret, GlyphSpinner, PanelEmpty, RowButton } from '@hermes/plugin-sdk' +import type { ReactNode, RefObject } from 'react' + +import type { useRoster } from './data' +import { $showHiddenBots } from './hidden-bots' +import type { useBots } from './i18n' +import type { deriveRosterPresentation, deriveRosterRows } from './roster-pane-derivation' +import type { rosterSectionRenderers } from './roster-pane-sections' +import type { rosterGatewayOptions } from './roster-sections' +import type { RosterRow } from './types' + +interface RosterContentProps { + b: ReturnType + staleNotice: string | number | null + isLoading: boolean + initialRosterLoading: boolean + roster: RosterRow[] + error: ReturnType['error'] + gatewayUp: boolean + refetch: ReturnType['refetch'] + allBotsHidden: boolean + hiddenExpanded: boolean + rosterRows: ReturnType['rosterRows'] + matchingHiddenBots: RosterRow[] + query: string + selectedGateway: ReturnType[number] | undefined + showGatewaySections: boolean + sortedGroupRows: ReturnType['sortedGroupRows'] + gatewaySections: ReturnType['gatewaySections'] + showHiddenSection: boolean + hiddenSectionRef: RefObject + hasRosterConstraint: boolean + hiddenBots: RosterRow[] + showHiddenRows: boolean + hiddenGatewaySections: ReturnType['hiddenGatewaySections'] + renderBotRow: (bot: RosterRow, keyPrefix?: string) => ReactNode + renderGroupChatSection: ReturnType['renderGroupChatSection'] + renderGatewaySection: ReturnType['renderGatewaySection'] + renderUserSections: ReturnType['renderUserSections'] + renderHiddenGatewaySection: ReturnType['renderHiddenGatewaySection'] +} + +export function renderRosterContent({ + b, + staleNotice, + isLoading, + initialRosterLoading, + roster, + error, + gatewayUp, + refetch, + allBotsHidden, + hiddenExpanded, + rosterRows, + matchingHiddenBots, + query, + selectedGateway, + showGatewaySections, + sortedGroupRows, + gatewaySections, + showHiddenSection, + hiddenSectionRef, + hasRosterConstraint, + hiddenBots, + showHiddenRows, + hiddenGatewaySections, + renderBotRow, + renderGroupChatSection, + renderGatewaySection, + renderUserSections, + renderHiddenGatewaySection +}: RosterContentProps) { + return ( + <> + {staleNotice ? ( +
+ {staleNotice} +
+ ) : null} + {(isLoading || initialRosterLoading) && !roster.length ? ( +
+ +
+ ) : error && !roster.length ? ( +
+
+ {gatewayUp + ? b.roster.rosterUnavailable(error instanceof Error ? error.message : 'gateway error') + : b.roster.waitingForGateway} +
+ +
+ ) : roster.length === 0 ? ( + + ) : allBotsHidden && !hiddenExpanded ? ( +
+
+ + {b.roster.allHidden} +
+

{b.roster.allHiddenDesc}

+ +
+ ) : rosterRows.length === 0 && matchingHiddenBots.length === 0 ? ( +
+ +
+ ) : ( +
+
+ {showGatewaySections + ? [ + sortedGroupRows.length ? renderGroupChatSection() : null, + ...gatewaySections.sections.map(renderGatewaySection) + ].filter(Boolean) + : renderUserSections(rosterRows)} + {showHiddenSection ? ( +
+ {hasRosterConstraint ? ( +
+ + Hidden + {matchingHiddenBots.length} +
+ ) : ( + $showHiddenBots.set(!hiddenExpanded)} + > + + Hidden + {hiddenBots.length} + + )} + {showHiddenRows ? ( + matchingHiddenBots.length ? ( + hiddenGatewaySections.sectioned ? ( + hiddenGatewaySections.sections.map(renderHiddenGatewaySection) + ) : ( + matchingHiddenBots.map((bot: RosterRow) => renderBotRow(bot, 'hidden:')) + ) + ) : ( +
{b.roster.noHiddenMatch}
+ ) + ) : null} +
+ ) : null} +
+
+ )} + + ) +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-derivation.ts b/apps/desktop/src/plugins/hermes-bots/roster-pane-derivation.ts new file mode 100644 index 0000000000..d83f1677de --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-derivation.ts @@ -0,0 +1,248 @@ +import { botActivitySession } from './data' +import { botRosterKey, filterBots, preferReachableSameNameRows } from './data' +import type { $groupChats } from './group-chat' +import { groupChatMemberBots, groupChatNames, groupLastActivity } from './group-membership' +import { isBotPinned } from './hidden-bots' +import { isBotHidden } from './hidden-bots' +import { filterBotsByGateway, groupMatchesRosterFilters, rosterGatewaySections } from './roster-sections' +import type { rosterGatewayOptions } from './roster-sections' +import { botRosterMeta } from './routing' +import { BOT_ROSTER_SEARCH_THRESHOLD } from './row-helpers' +import { ACTIVE_WINDOW_S, rosterActivityMatches } from './row-helpers' +import type { BotMeta, GroupMember, RosterActivityFilter, RosterKindFilter, RosterRow } from './types' +/** The two row shapes the roster sorts together — `kind` is the discriminant. */ +interface RosterBotRow { + active: boolean + activity: number + bot: RosterRow + kind: 'bot' + pinned: boolean +} +export interface RosterGroupRow { + active: boolean + activity: number + kind: 'group' + members: GroupMember[] + name: string + pinned: boolean +} + +interface RosterRowsInput { + roster: RosterRow[] + allMeta: Record + gatewayFilter: string + query: string + activityFilter: RosterActivityFilter + rowKindFilter: RosterKindFilter + groupRooms: ReturnType + activeRosterKeys: Set + gatewayOptions: ReturnType + activityOf: (bot: RosterRow) => number + isPinned: (bot: RosterRow) => boolean +} + +export function deriveRosterRows({ + roster, + allMeta, + gatewayFilter, + query, + activityFilter, + rowKindFilter, + groupRooms, + activeRosterKeys, + gatewayOptions, + activityOf, + isPinned +}: RosterRowsInput) { + const activeSourceRoster = roster.filter(bot => !bot.remoteSource) + // Hidden rows remain fully alive and recoverable at the bottom. Every + // non-display consumer continues to receive the complete roster. + const hiddenBots = roster.filter(bot => isBotHidden(bot, allMeta)) + const visibleRoster = roster.filter(bot => !isBotHidden(bot, allMeta)) + const gatewayRoster = filterBotsByGateway(visibleRoster, gatewayFilter) + + const filteredRoster = filterBots(gatewayRoster, allMeta, query).filter((bot: RosterRow) => + rosterActivityMatches( + { + activity: activityOf(bot), + active: activeRosterKeys.has(botRosterKey(bot)) + }, + activityFilter + ) + ) + + const filteredHiddenBots = filterBots(filterBotsByGateway(hiddenBots, gatewayFilter), allMeta, query).filter( + (bot: RosterRow) => + rosterActivityMatches( + { + activity: activityOf(bot), + active: activeRosterKeys.has(botRosterKey(bot)) + }, + activityFilter + ) + ) + + const groupNames = groupChatNames(allMeta, groupRooms) + + const groupRows = groupNames + .map(name => ({ + name, + members: groupChatMemberBots(name, roster, allMeta) + })) + .filter(row => groupMatchesRosterFilters(row.name, row.members, allMeta, query, gatewayFilter)) + .map((row): RosterGroupRow => ({ + kind: 'group', + name: row.name, + members: row.members, + pinned: Boolean(groupRooms[row.name]?.pinned), + activity: groupLastActivity(groupRooms[row.name]), + active: + Boolean( + groupLastActivity(groupRooms[row.name]) && + Date.now() - groupLastActivity(groupRooms[row.name]) <= ACTIVE_WINDOW_S * 1000 + ) || row.members.some(member => activeRosterKeys.has(botRosterKey(member))) + })) + .filter(row => rowKindFilter !== 'bots' && rosterActivityMatches(row, activityFilter)) + + const botRows = + rowKindFilter === 'groups' + ? [] + : preferReachableSameNameRows(filteredRoster).map((bot): RosterBotRow => ({ + kind: 'bot', + bot, + pinned: isPinned(bot), + activity: activityOf(bot), + active: activeRosterKeys.has(botRosterKey(bot)) + })) + + const sortRosterRows = (rows: T[]): T[] => + rows.slice().sort((a, b) => { + const pa = a.pinned ? 1 : 0 + const pb = b.pinned ? 1 : 0 + + if (pa !== pb) { + return pb - pa + } + + return b.activity - a.activity + }) + + const rosterRows = sortRosterRows([...botRows, ...groupRows]) + const sortedGroupRows = sortRosterRows(groupRows) + const gatewaySections = rosterGatewaySections(botRows, gatewayOptions, gatewayFilter) + const showGatewaySections = gatewaySections.sectioned && botRows.length > 0 + + return { + activeSourceRoster, + hiddenBots, + visibleRoster, + filteredHiddenBots, + groupNames, + rosterRows, + sortedGroupRows, + gatewaySections, + showGatewaySections + } +} + +interface RosterPresentationInput { + rowKindFilter: RosterKindFilter + activityFilter: RosterActivityFilter + gatewayFilter: string + query: string + filteredHiddenBots: RosterRow[] + hiddenBots: RosterRow[] + hiddenExpanded: boolean + roster: RosterRow[] + groupNames: string[] + visibleRoster: RosterRow[] + gatewayOptions: ReturnType +} + +export function deriveRosterPresentation({ + rowKindFilter, + activityFilter, + gatewayFilter, + query, + filteredHiddenBots, + hiddenBots, + hiddenExpanded, + roster, + groupNames, + visibleRoster, + gatewayOptions +}: RosterPresentationInput) { + const activeFilterCount = + (rowKindFilter === 'all' ? 0 : 1) + (activityFilter === 'all' ? 0 : 1) + (gatewayFilter === 'all' ? 0 : 1) + + const hasRosterConstraint = Boolean(query.trim()) || activeFilterCount > 0 + const matchingHiddenBots = rowKindFilter === 'groups' ? [] : filteredHiddenBots + const showHiddenSection = hiddenBots.length > 0 && (!hasRosterConstraint || matchingHiddenBots.length > 0) + const showHiddenRows = hiddenExpanded || hasRosterConstraint + const rosterItemCount = roster.length + groupNames.length + + const allBotsHidden = + !hasRosterConstraint && visibleRoster.length === 0 && groupNames.length === 0 && hiddenBots.length > 0 + + const showRosterSearch = + gatewayOptions.length > 1 || rosterItemCount >= BOT_ROSTER_SEARCH_THRESHOLD || Boolean(query.trim()) + + const showRosterFilters = + gatewayOptions.length > 1 || + groupNames.length > 0 || + rosterItemCount >= BOT_ROSTER_SEARCH_THRESHOLD || + activeFilterCount > 0 + + const showRosterTools = showRosterSearch || showRosterFilters + + const hiddenGatewaySections = rosterGatewaySections( + matchingHiddenBots.map((bot: RosterRow) => ({ + kind: 'bot', + bot + })), + gatewayOptions, + gatewayFilter + ) + + return { + activeFilterCount, + hasRosterConstraint, + matchingHiddenBots, + showHiddenSection, + showHiddenRows, + allBotsHidden, + showRosterSearch, + showRosterFilters, + showRosterTools, + hiddenGatewaySections + } +} + +export function sortRosterBots(sourceWithSelectedOwner: RosterRow[], allMeta: Record) { + // Messaging-app order: most recent activity first, where "activity" is + // the newest of (bot created, last message in any of its sessions). A + // freshly created bot tops the list until another bot gets a message. + // No special slot for the primary bot — it competes on recency too. + const activityOf = (bot: RosterRow): number => { + const created = botRosterMeta(bot, allMeta)?.created || bot.ui_meta?.['hermes-bots']?.created || 0 + const lastMsg = (botActivitySession(bot)?.last_active || 0) * 1000 + + return Math.max(created, lastMsg) + } + + // Pin is a source-qualified Desktop preference, not gateway profile state. + const isPinned = (bot: RosterRow): boolean => isBotPinned(bot, allMeta) + + const roster = sourceWithSelectedOwner.slice().sort((a, b) => { + const pa = isPinned(a) ? 1 : 0 + const pb = isPinned(b) ? 1 : 0 + + if (pa !== pb) { + return pb - pa + } + + return activityOf(b) - activityOf(a) + }) + + return { roster, activityOf, isPinned } +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-dialogs.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane-dialogs.tsx new file mode 100644 index 0000000000..8e1f25b161 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-dialogs.tsx @@ -0,0 +1,161 @@ +import { ConfirmDialog, host } from '@hermes/plugin-sdk' +import type { useI18n } from '@hermes/plugin-sdk' + +import { CreateAgentDialog, CreateGroupChatDialog, GroupDialog } from './create-dialog' +import type { useRoster } from './data' +import { EditProfileDialog } from './edit-profile-dialog' +import { disbandGroupChat, openGroupChat } from './group-chat-view' +import type { useBots } from './i18n' +import { deleteBot } from './profile-ops' +import type { GroupMember, RosterRow } from './types' +import { createBotSection, renameBotSection } from './user-sections' +import { SectionNameDialog } from './user-sections-ui' + +interface renderRosterDialogsProps { + b: ReturnType + t: ReturnType['t'] + createOpen: boolean + setCreateOpen: (value: boolean) => void + groupCreateOpen: boolean + setGroupCreateOpen: (value: boolean) => void + editing: RosterRow | null + setEditing: (value: RosterRow | null) => void + deleting: (RosterRow & { path?: string }) | null + setDeleting: (value: (RosterRow & { path?: string }) | null) => void + deletingGroup: { members: GroupMember[]; name: string } | null + setDeletingGroup: (value: { members: GroupMember[]; name: string } | null) => void + grouping: RosterRow | null + setGrouping: (value: RosterRow | null) => void + sectionDialog: null | { bot?: RosterRow; mode: 'create' } | { id: string; mode: 'rename'; name: string } + setSectionDialog: ( + value: null | { bot?: RosterRow; mode: 'create' } | { id: string; mode: 'rename'; name: string } + ) => void + roster: RosterRow[] + activeSourceRoster: RosterRow[] + refetch: ReturnType['refetch'] +} + +export function renderRosterDialogs({ + b, + t, + createOpen, + setCreateOpen, + groupCreateOpen, + setGroupCreateOpen, + editing, + setEditing, + deleting, + setDeleting, + deletingGroup, + setDeletingGroup, + grouping, + setGrouping, + sectionDialog, + setSectionDialog, + roster, + activeSourceRoster, + refetch +}: renderRosterDialogsProps) { + return ( + <> + { + setCreateOpen(false) + void refetch() + }} + open={createOpen} + roster={activeSourceRoster} + /> + setGroupCreateOpen(false)} + onCreated={groupName => openGroupChat(groupName)} + open={groupCreateOpen} // Full multi-source roster: group chats can seat bots from other + // registered connections — their turns route to their own machines. + roster={roster} + /> + { + if (!open) { + setSectionDialog(null) + } + }} + onSubmit={name => { + if (sectionDialog?.mode === 'rename') { + renameBotSection(sectionDialog.id, name) + } else { + createBotSection(name, sectionDialog?.bot ? [sectionDialog.bot] : []) + } + }} + open={Boolean(sectionDialog)} + /> + { + setEditing(null) + void refetch() + }} + open={Boolean(editing)} + /> + {grouping ? setGrouping(null)} /> : null} + + {'This will permanently delete the bot '} + {deleting.name} + {' and its associated Hermes profile at '} + {deleting.path}. This cannot be undone. + + ) : null + } + destructive + doneLabel="Deleted" + onClose={() => setDeleting(null)} + onConfirm={async () => { + if (!deleting) { + return + } + + const name = deleting.name + await deleteBot(deleting) + await refetch() + host.notify({ + kind: 'success', + message: `Deleted profile ${name}` + }) + }} + open={Boolean(deleting)} + title={b.bot.deleteTitle} + /> + setDeletingGroup(null)} + onConfirm={async () => { + if (!deletingGroup) { + return + } + + await disbandGroupChat(deletingGroup.name, deletingGroup.members) + host.notify({ + kind: 'success', + message: `Deleted group “${deletingGroup.name}”` + }) + }} + open={Boolean(deletingGroup)} + title={b.group.deleteTitle} + /> + + ) +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-lifecycle.ts b/apps/desktop/src/plugins/hermes-bots/roster-pane-lifecycle.ts new file mode 100644 index 0000000000..680cd76b68 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-lifecycle.ts @@ -0,0 +1,54 @@ +import { atom, host } from '@hermes/plugin-sdk' +import { useEffect } from 'react' + +import { $lastRoster } from './data' +import type { useRoster } from './data' +import { displayName } from './labels' +import { mergeServerMeta, pullServerAvatars } from './profile-ops' +import { trackInboundActivity } from './roster-actions' +import { botRosterMeta, botWorkspaceOwnerKey } from './routing' +import { backfillMessagingProtocol } from './soul' +import type { GatewaySource } from './types' +import type { BotMeta, RosterRow } from './types' + +/** Last source inventory returned by the desktop-wide agent roster. */ +export const $lastSources = atom([]) + +interface RosterSnapshotInput { + data: ReturnType['data'] + live: RosterRow[] | null + roster: RosterRow[] + allMeta: Record + activeSourceRoster: RosterRow[] +} + +export function usePublishRosterSnapshot({ data, live, roster, allMeta, activeSourceRoster }: RosterSnapshotInput) { + useEffect(() => { + if (!live) { + return + } + + // Offline-owner ghosts belong only to this render. Shared roster state + // feeds merge caching, group membership, creation, and durable sync. These + // writes must settle after render: other subscribers of the same atoms + // would otherwise be updated while BotsPane was still rendering. + $lastRoster.set(roster.filter(row => !row?.ghost)) + // Tabs caption a bot chat by its bot (#99152); republished with the + // roster so a rename follows and tiles restored at boot resolve. + roster.forEach(bot => { + host.setWorkspaceOwnerLabel?.(botWorkspaceOwnerKey(bot), displayName(bot, botRosterMeta(bot, allMeta))) + }) + + if (Array.isArray(data?.sources)) { + $lastSources.set(data.sources) + } + + mergeServerMeta(activeSourceRoster, data?.fetchedAt || 0) + pullServerAvatars(activeSourceRoster) + trackInboundActivity(roster) + backfillMessagingProtocol(activeSourceRoster) + // React Query owns the stable server snapshot; derived arrays intentionally + // follow that snapshot rather than retriggering on their own atom writes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [data]) +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-sections.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane-sections.tsx new file mode 100644 index 0000000000..3c63207871 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-sections.tsx @@ -0,0 +1,195 @@ +import { host } from '@hermes/plugin-sdk' +import type { ReactNode } from 'react' + +import { botRosterKey } from './data' +import type { useBots } from './i18n' +import type { RosterGroupRow } from './roster-pane-derivation' +import { GatewayKindGlyph, GatewaySectionHeading, RosterSectionHeader } from './roster-sections' +import type { ResolvedRosterGatewaySection } from './roster-sections' +import type { BotMeta, GroupMember, RosterRow } from './types' +import type { $botSections } from './user-sections' +import { + deleteBotSection, + groupRowsBySection, + moveBotSection, + moveBotsToSection, + UNASSIGNED_SECTION_KEY +} from './user-sections' +import { SectionDropZone, UserSectionHeader } from './user-sections-ui' + +interface RosterSectionRenderersProps { + b: ReturnType + userSections: ReturnType + roster: RosterRow[] + allMeta: Record + dragging: string | null + rosterSectionCollapsed: (id: string) => boolean + toggleRosterSection: (id: string) => void + setSectionDialog: ( + value: null | { bot?: RosterRow; mode: 'create' } | { id: string; mode: 'rename'; name: string } + ) => void + renderBotRow: (bot: RosterRow, keyPrefix?: string) => ReactNode + renderGroupRow: (row: { members: GroupMember[]; name: string }) => ReactNode + sortedGroupRows: RosterGroupRow[] +} + +export function rosterSectionRenderers({ + b, + userSections, + roster, + allMeta, + dragging, + rosterSectionCollapsed, + toggleRosterSection, + setSectionDialog, + renderBotRow, + renderGroupRow, + sortedGroupRows +}: RosterSectionRenderersProps) { + const removeSection = (id: string) => { + const name = userSections.find(section => section.id === id)?.name || '' + const { members, undo } = deleteBotSection(id, roster) + + // No confirmation: nothing is lost (the bots fall back to Unassigned) and + // the toast's Undo puts the section and its members back. + host.notify({ + action: { label: b.sections.undo, onClick: undo }, + durationMs: 8_000, + kind: 'info', + message: b.sections.deleted(name, members.length) + }) + } + + // USER SECTIONS — composed with the gateway sections, not instead of them. + // The gateway headings own the top level whenever the roster shows more + // than one connection (that axis answers "where does this run", which no + // folder name can, and a bot's membership lives in its profile on THAT + // gateway); user sections group the rows INSIDE each connection bucket, + // indented under it, and group the flat list when there is only one. + // `keyPrefix` keeps row keys unique across the gateway buckets. + type UserSectionRow = { bot: RosterRow; kind?: 'bot' } | RosterGroupRow + + const renderUserSections = (rows: UserSectionRow[], keyPrefix = '') => { + // No sections made: the plain list, exactly as before this feature. + if (!userSections.length) { + return rows.map(row => (row.kind === 'group' ? renderGroupRow(row) : renderBotRow(row.bot, keyPrefix))) + } + + const nested = Boolean(keyPrefix) + const blocks = groupRowsBySection(rows, userSections, allMeta) + + return ( + blocks + // An empty Unassigned is not worth a heading; an empty NAMED section + // is, because it is somewhere the user made and is about to drop into. + // Inside a gateway bucket the same empty section would repeat under + // every connection, so there it only appears while a drag is in flight + // (as the drop target it exists for); the row menu files into it + // regardless. + .filter(block => block.rows.length || (block.id && (!nested || dragging))) + .map(block => { + const key = `${keyPrefix}${block.id ? `user-section:${block.id}` : UNASSIGNED_SECTION_KEY}` + const collapsed = rosterSectionCollapsed(key) + const order = userSections.findIndex(section => section.id === block.id) + + return ( + row.kind !== 'group' && botRosterKey(row.bot) === dragging) + } + key={key} + nested={nested} + onDropBot={rosterKey => { + const bot = roster.find(row => botRosterKey(row) === rosterKey) + + // `block.id` is null for Unassigned, which is exactly the value + // moveBotsToSection wants for "clear the assignment". + if (bot) { + void moveBotsToSection([bot], block.id) + } + }} + > + = 0 && order < userSections.length - 1} + canMoveUp={order > 0} + collapsed={collapsed} + count={block.rows.length} + id={block.id} + name={block.name} + onDelete={() => block.id && removeSection(block.id)} + onMove={delta => block.id && moveBotSection(block.id, delta)} + onRename={() => block.id && setSectionDialog({ id: block.id, mode: 'rename', name: block.name })} + onToggle={() => toggleRosterSection(key)} + /> + {collapsed ? null : block.rows.length ? ( +
+ {block.rows.map(row => + row.kind === 'group' ? renderGroupRow(row) : renderBotRow(row.bot, `${key}:`) + )} +
+ ) : ( + // Empty section: a quiet dashed slot that says what it is for, + // and doubles as a roomy drop target. +
+ {b.sections.emptyHint} +
+ )} + + ) + }) + ) + } + + const renderGatewaySection = (section: ResolvedRosterGatewaySection) => { + const sectionId = `gateway:${section.id}` + const collapsed = rosterSectionCollapsed(sectionId) + + return ( +
+ toggleRosterSection(sectionId)} + option={section.option} + /> + {collapsed ? null : ( +
{renderUserSections(section.rows, `${section.id}:`)}
+ )} +
+ ) + } + + const renderGroupChatSection = () => { + const sectionId = 'group-chats' + const collapsed = rosterSectionCollapsed(sectionId) + + return ( +
+ toggleRosterSection(sectionId)} + tip={`${sortedGroupRows.length} global group chat${sortedGroupRows.length === 1 ? '' : 's'}`} + /> + {collapsed ? null :
{sortedGroupRows.map(renderGroupRow)}
} +
+ ) + } + + const renderHiddenGatewaySection = (section: ResolvedRosterGatewaySection) => ( +
+
+ + + {section.option?.label || section.option?.connectionId || 'Current gateway'} + + {section.rows.length} +
+ {section.rows.map(row => renderBotRow(row.bot, `hidden:${section.id}:`))} +
+ ) + + return { renderUserSections, renderGatewaySection, renderGroupChatSection, renderHiddenGatewaySection } +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane-toolbar.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane-toolbar.tsx new file mode 100644 index 0000000000..ca36666486 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane-toolbar.tsx @@ -0,0 +1,227 @@ +import { + Button, + cn, + Codicon, + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, + SearchField, + Tip +} from '@hermes/plugin-sdk' + +import { botSourceStatus } from './data' +import type { useBots } from './i18n' +import { setActivityToasts } from './roster-actions' +import { GatewayKindGlyph } from './roster-sections' +import type { rosterGatewayOptions } from './roster-sections' +import type { RosterActivityFilter, RosterKindFilter, RosterRow } from './types' + +interface renderRosterToolbarProps { + b: ReturnType + activityToasts: boolean + activeSourceRoster: RosterRow[] + setCreateOpen: (value: boolean) => void + setGroupCreateOpen: (value: boolean) => void + setSectionDialog: ( + value: null | { bot?: RosterRow; mode: 'create' } | { id: string; mode: 'rename'; name: string } + ) => void + showRosterTools: boolean + showRosterSearch: boolean + showRosterFilters: boolean + query: string + setQuery: (value: string) => void + activeFilterCount: number + gatewayOptions: ReturnType + rowKindFilter: RosterKindFilter + setRowKindFilter: (value: RosterKindFilter) => void + activityFilter: RosterActivityFilter + setActivityFilter: (value: RosterActivityFilter) => void + gatewayFilter: string + setGatewayFilter: (value: string) => void +} + +export function renderRosterToolbar({ + b, + activityToasts, + activeSourceRoster, + setCreateOpen, + setGroupCreateOpen, + setSectionDialog, + showRosterTools, + showRosterSearch, + showRosterFilters, + query, + setQuery, + activeFilterCount, + gatewayOptions, + rowKindFilter, + setRowKindFilter, + activityFilter, + setActivityFilter, + gatewayFilter, + setGatewayFilter +}: renderRosterToolbarProps) { + return ( + <> +
+ + Bots + +
+ + + + + + + + + + + setCreateOpen(true)}> + + {b.bot.newTitle} + + setGroupCreateOpen(true)}> + + {b.group.newTitle} + + + setSectionDialog({ mode: 'create' })}> + + {b.sections.newSection} + + + +
+
+ {showRosterTools ? ( +
+ {showRosterSearch ? ( + + ) : ( + + )} + {showRosterFilters ? ( + + + + + + + + {( + [ + ['all', b.roster.botsAndGroups], + ['bots', b.roster.botsOnly], + ['groups', b.roster.groupsOnly] + ] as [RosterKindFilter, string][] + ).map(([value, label]) => ( + setRowKindFilter(value)}> + {label} + {rowKindFilter === value ? : null} + + ))} + + {( + [ + ['all', b.roster.anyActivity], + ['active', b.roster.activeNow], + ['recent', b.roster.recentlyActive], + ['older', b.roster.older] + ] as [RosterActivityFilter, string][] + ).map(([value, label]) => ( + setActivityFilter(value)}> + {label} + {activityFilter === value ? : null} + + ))} + {gatewayOptions.length > 1 ? : null} + {gatewayOptions.length > 1 ? ( + setGatewayFilter('all')}> + + All gateways + {gatewayFilter === 'all' ? : null} + + ) : null} + {gatewayOptions.length > 1 + ? gatewayOptions.map(option => { + const status = botSourceStatus({ + sourceError: option.error, + sourceReachable: option.reachable + }) + + return ( + setGatewayFilter(option.connectionId)} + > + + {option.label || option.connectionId} + + {option.count} + + {gatewayFilter === option.connectionId ? : null} + + ) + }) + : []} + {activeFilterCount ? : null} + {activeFilterCount ? ( + { + setRowKindFilter('all') + setActivityFilter('all') + setGatewayFilter('all') + }} + > + {b.roster.clearFilters} + + ) : null} + + + ) : null} +
+ ) : null} + + ) +} diff --git a/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx b/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx index cd94ee134a..1401c35042 100644 --- a/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx +++ b/apps/desktop/src/plugins/hermes-bots/roster-pane.tsx @@ -1,33 +1,4 @@ -/** - * The Bots pane itself: the roster's selection reconciliation, the - * workspace-ownership reads its lifecycle keys off, and the pane that lists - * every bot and group chat. - * - * The top of the roster stack. It composes the rows, the section headings and - * the dialogs; nothing in Bot Mode imports it except the plugin entry point. - */ - -import { - atom, - Button, - cn, - Codicon, - ConfirmDialog, - DisclosureCaret, - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuSeparator, - DropdownMenuTrigger, - GlyphSpinner, - host, - PanelEmpty, - RowButton, - SearchField, - Tip, - useI18n, - useValue -} from '@hermes/plugin-sdk' +import { host, useI18n, useValue } from '@hermes/plugin-sdk' import { useEffect, useRef, useState } from 'react' import { BotRow, GroupRow } from './bot-row' @@ -42,60 +13,43 @@ import { parseRosterKey, saveSelectedRosterBot } from './bot-state' -import { CreateAgentDialog, CreateGroupChatDialog, GroupDialog } from './create-dialog' +/** + * The Bots pane itself: the roster's selection reconciliation, the + * workspace-ownership reads its lifecycle keys off, and the pane that lists + * every bot and group chat. + * + * The top of the roster stack. It composes the rows, the section headings and + * the dialogs; nothing in Bot Mode imports it except the plugin entry point. + */ import { $botMeta, $lastRoster, annotateBotSource, - botActivitySession, botRosterKey, botSourceStatus, - filterBots, - preferReachableSameNameRows, sourceByConnection, useRoster } from './data' -import { EditProfileDialog } from './edit-profile-dialog' import { $groupChats, $groupChatWorkspace, $groupClarify, $groupNeedsYou } from './group-chat' -import { disbandGroupChat, GroupChatWorkspace, openGroupChat } from './group-chat-view' -import { groupChatMemberBots, groupChatNames, groupLastActivity } from './group-membership' +import { GroupChatWorkspace, openGroupChat } from './group-chat-view' +import { groupChatMemberBots } from './group-membership' import { $groupMainTabsRev, shouldRenderGroupChatInPane } from './group-panes' import { groupHasPendingClarify } from './group-turns' -import { $showHiddenBots, isBotHidden, isBotPinned } from './hidden-bots' +import { $showHiddenBots, isBotHidden } from './hidden-bots' import { useBots } from './i18n' -import { displayName } from './labels' -import { deleteBot, mergeServerMeta, pullServerAvatars } from './profile-ops' -import { $activityToasts, setActivityToasts, trackInboundActivity } from './roster-actions' -import { - botNeedsHandleLabel, - filterBotsByGateway, - GatewayKindGlyph, - GatewaySectionHeading, - groupMatchesRosterFilters, - rosterGatewayOptions, - rosterGatewaySections, - RosterSectionHeader -} from './roster-sections' -import type { ResolvedRosterGatewaySection } from './roster-sections' -import { botRosterMeta, botWorkspaceOwnerKey, setBotsWorkspaceOwner } from './routing' -import { ACTIVE_WINDOW_S, activeBots, BOT_ROSTER_SEARCH_THRESHOLD, rosterActivityMatches } from './row-helpers' -import { backfillMessagingProtocol } from './soul' +import { $activityToasts } from './roster-actions' +import { renderRosterContent } from './roster-pane-content' +import { deriveRosterPresentation, deriveRosterRows, sortRosterBots } from './roster-pane-derivation' +import { renderRosterDialogs } from './roster-pane-dialogs' +import { $lastSources, usePublishRosterSnapshot } from './roster-pane-lifecycle' +import { rosterSectionRenderers } from './roster-pane-sections' +import { renderRosterToolbar } from './roster-pane-toolbar' +import { botNeedsHandleLabel, rosterGatewayOptions } from './roster-sections' +import { botWorkspaceOwnerKey, setBotsWorkspaceOwner } from './routing' +import { activeBots } from './row-helpers' import type { BotMeta, GatewaySource, GroupMember, RosterActivityFilter, RosterKindFilter, RosterRow } from './types' -import { - $botSections, - $draggingBot, - createBotSection, - deleteBotSection, - groupRowsBySection, - moveBotSection, - moveBotsToSection, - renameBotSection, - UNASSIGNED_SECTION_KEY -} from './user-sections' -import { SectionDropZone, SectionNameDialog, useEscapeCancelsBotDrag, UserSectionHeader } from './user-sections-ui' - -/** Last source inventory returned by the desktop-wide agent roster. */ -const $lastSources = atom([]) +import { $botSections, $draggingBot } from './user-sections' +import { useEscapeCancelsBotDrag } from './user-sections-ui' // ── roster pane ────────────────────────────────────────────────────────────── @@ -234,21 +188,35 @@ export function releaseStaleOpenBotChat(focusedStoredId: null | string | undefin } } -/** The two row shapes the roster sorts together — `kind` is the discriminant. */ -interface RosterBotRow { - active: boolean - activity: number - bot: RosterRow - kind: 'bot' - pinned: boolean -} -interface RosterGroupRow { - active: boolean - activity: number - kind: 'group' - members: GroupMember[] - name: string - pinned: boolean +function useReconcileRosterOwner( + data: ReturnType['data'], + error: ReturnType['error'], + selectionHydrated: boolean, + roster: RosterRow[], + sourceSnapshot: GatewaySource[], + allMeta: Record +) { + // The roster has ANSWERED once data or a terminal error exists — that, not + // row count, is what lets this pane stop showing its loading state (an empty + // answer is a real answer; a pending one must not flash "No bots"). Keep the + // persisted-selection writes out of render: React may replay a render, but + // an abandoned render must never become a storage mutation. + useEffect(() => { + if (!data && !error) { + return + } + + $rosterHydrated.set(true) + + if (selectionHydrated) { + reconcileRosterSelection(roster, sourceSnapshot, allMeta) + const selected = selectedRosterBot(roster, $selectedRosterKey.get()) + + if ($botsPaneVisible.get() && !$groupChatWorkspace.get() && selected) { + setBotsWorkspaceOwner(botWorkspaceOwnerKey(selected), selected) + } + } + }, [data, error, selectionHydrated, roster, sourceSnapshot, allMeta]) } export function BotsPane() { @@ -306,19 +274,6 @@ export function BotsPane() { }, [gatewayUp, refetch]) const allMeta = useValue($botMeta) - // Messaging-app order: most recent activity first, where "activity" is - // the newest of (bot created, last message in any of its sessions). A - // freshly created bot tops the list until another bot gets a message. - // No special slot for the primary bot — it competes on recency too. - const activityOf = (bot: RosterRow): number => { - const created = botRosterMeta(bot, allMeta)?.created || bot.ui_meta?.['hermes-bots']?.created || 0 - const lastMsg = (botActivitySession(bot)?.last_active || 0) * 1000 - - return Math.max(created, lastMsg) - } - - // Pin is a source-qualified Desktop preference, not gateway profile state. - const isPinned = (bot: RosterRow): boolean => isBotPinned(bot, allMeta) // Resilience (@wesleysimplicio, #13): a failed refresh must not erase a // roster the user already had — mixed local+cloud gateways and remotes // waking from sleep fail transiently. Render the last good snapshot with @@ -330,16 +285,7 @@ export function BotsPane() { const sourceWithSelectedOwner = selectionHydrated && rosterHydrated ? rosterWithSelectedOwner(source, sourceSnapshot, selectedRosterKey) : source - const roster = sourceWithSelectedOwner.slice().sort((a, b) => { - const pa = isPinned(a) ? 1 : 0 - const pb = isPinned(b) ? 1 : 0 - - if (pa !== pb) { - return pb - pa - } - - return activityOf(b) - activityOf(a) - }) + const { roster, activityOf, isPinned } = sortRosterBots(sourceWithSelectedOwner, allMeta) // React Query can briefly report neither loading nor data while the plugin // and the persisted connection registry hydrate. Keep that transition in a @@ -354,117 +300,58 @@ export function BotsPane() { setGatewayFilter('all') } }, [gatewayFilterExists]) - const activeSourceRoster = roster.filter(bot => !bot.remoteSource) - // Hidden rows remain fully alive and recoverable at the bottom. Every - // non-display consumer continues to receive the complete roster. const hiddenExpanded = useValue($showHiddenBots) - const hiddenBots = roster.filter(bot => isBotHidden(bot, allMeta)) - const visibleRoster = roster.filter(bot => !isBotHidden(bot, allMeta)) - const gatewayRoster = filterBotsByGateway(visibleRoster, gatewayFilter) - const filteredRoster = filterBots(gatewayRoster, allMeta, query).filter((bot: RosterRow) => - rosterActivityMatches( - { - activity: activityOf(bot), - active: activeRosterKeys.has(botRosterKey(bot)) - }, - activityFilter - ) - ) - - const filteredHiddenBots = filterBots(filterBotsByGateway(hiddenBots, gatewayFilter), allMeta, query).filter( - (bot: RosterRow) => - rosterActivityMatches( - { - activity: activityOf(bot), - active: activeRosterKeys.has(botRosterKey(bot)) - }, - activityFilter - ) - ) - - const groupNames = groupChatNames(allMeta, groupRooms) - - const groupRows = groupNames - .map(name => ({ - name, - members: groupChatMemberBots(name, roster, allMeta) - })) - .filter(row => groupMatchesRosterFilters(row.name, row.members, allMeta, query, gatewayFilter)) - .map((row): RosterGroupRow => ({ - kind: 'group', - name: row.name, - members: row.members, - pinned: Boolean(groupRooms[row.name]?.pinned), - activity: groupLastActivity(groupRooms[row.name]), - active: - Boolean( - groupLastActivity(groupRooms[row.name]) && - Date.now() - groupLastActivity(groupRooms[row.name]) <= ACTIVE_WINDOW_S * 1000 - ) || row.members.some(member => activeRosterKeys.has(botRosterKey(member))) - })) - .filter(row => rowKindFilter !== 'bots' && rosterActivityMatches(row, activityFilter)) - - const botRows = - rowKindFilter === 'groups' - ? [] - : preferReachableSameNameRows(filteredRoster).map((bot): RosterBotRow => ({ - kind: 'bot', - bot, - pinned: isPinned(bot), - activity: activityOf(bot), - active: activeRosterKeys.has(botRosterKey(bot)) - })) - - const sortRosterRows = (rows: T[]): T[] => - rows.slice().sort((a, b) => { - const pa = a.pinned ? 1 : 0 - const pb = b.pinned ? 1 : 0 - - if (pa !== pb) { - return pb - pa - } - - return b.activity - a.activity - }) - - const rosterRows = sortRosterRows([...botRows, ...groupRows]) - const sortedGroupRows = sortRosterRows(groupRows) - const gatewaySections = rosterGatewaySections(botRows, gatewayOptions, gatewayFilter) - const showGatewaySections = gatewaySections.sectioned && botRows.length > 0 - - const activeFilterCount = - (rowKindFilter === 'all' ? 0 : 1) + (activityFilter === 'all' ? 0 : 1) + (gatewayFilter === 'all' ? 0 : 1) - - const hasRosterConstraint = Boolean(query.trim()) || activeFilterCount > 0 - const matchingHiddenBots = rowKindFilter === 'groups' ? [] : filteredHiddenBots - const showHiddenSection = hiddenBots.length > 0 && (!hasRosterConstraint || matchingHiddenBots.length > 0) - const showHiddenRows = hiddenExpanded || hasRosterConstraint - const rosterItemCount = roster.length + groupNames.length - - const allBotsHidden = - !hasRosterConstraint && visibleRoster.length === 0 && groupNames.length === 0 && hiddenBots.length > 0 - - const showRosterSearch = - gatewayOptions.length > 1 || rosterItemCount >= BOT_ROSTER_SEARCH_THRESHOLD || Boolean(query.trim()) - - const showRosterFilters = - gatewayOptions.length > 1 || - groupNames.length > 0 || - rosterItemCount >= BOT_ROSTER_SEARCH_THRESHOLD || - activeFilterCount > 0 - - const showRosterTools = showRosterSearch || showRosterFilters - const rosterSectionCollapsed = (id: string): boolean => !hasRosterConstraint && collapsedRosterSections.has(id) - - const hiddenGatewaySections = rosterGatewaySections( - matchingHiddenBots.map((bot: RosterRow) => ({ - kind: 'bot', - bot - })), + const { + activeSourceRoster, + hiddenBots, + visibleRoster, + filteredHiddenBots, + groupNames, + rosterRows, + sortedGroupRows, + gatewaySections, + showGatewaySections + } = deriveRosterRows({ + roster, + allMeta, + gatewayFilter, + query, + activityFilter, + rowKindFilter, + groupRooms, + activeRosterKeys, gatewayOptions, - gatewayFilter - ) + activityOf, + isPinned + }) + + const { + activeFilterCount, + hasRosterConstraint, + matchingHiddenBots, + showHiddenSection, + showHiddenRows, + allBotsHidden, + showRosterSearch, + showRosterFilters, + showRosterTools, + hiddenGatewaySections + } = deriveRosterPresentation({ + rowKindFilter, + activityFilter, + gatewayFilter, + query, + filteredHiddenBots, + hiddenBots, + hiddenExpanded, + roster, + groupNames, + visibleRoster, + gatewayOptions + }) + + const rosterSectionCollapsed = (id: string): boolean => !hasRosterConstraint && collapsedRosterSections.has(id) const toggleRosterSection = (id: string): void => { setCollapsedRosterSections(previous => { @@ -493,56 +380,9 @@ export function BotsPane() { return () => cancelAnimationFrame(frame) }, [hiddenExpanded, hasRosterConstraint]) - useEffect(() => { - if (!live) { - return - } + usePublishRosterSnapshot({ data, live, roster, allMeta, activeSourceRoster }) - // Offline-owner ghosts belong only to this render. Shared roster state - // feeds merge caching, group membership, creation, and durable sync. These - // writes must settle after render: other subscribers of the same atoms - // would otherwise be updated while BotsPane was still rendering. - $lastRoster.set(roster.filter(row => !row?.ghost)) - // Tabs caption a bot chat by its bot (#99152); republished with the - // roster so a rename follows and tiles restored at boot resolve. - roster.forEach(bot => { - host.setWorkspaceOwnerLabel?.(botWorkspaceOwnerKey(bot), displayName(bot, botRosterMeta(bot, allMeta))) - }) - - if (Array.isArray(data?.sources)) { - $lastSources.set(data.sources) - } - - mergeServerMeta(activeSourceRoster, data?.fetchedAt || 0) - pullServerAvatars(activeSourceRoster) - trackInboundActivity(roster) - backfillMessagingProtocol(activeSourceRoster) - // React Query owns the stable server snapshot; derived arrays intentionally - // follow that snapshot rather than retriggering on their own atom writes. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [data]) - - // The roster has ANSWERED once data or a terminal error exists — that, not - // row count, is what lets this pane stop showing its loading state (an empty - // answer is a real answer; a pending one must not flash "No bots"). Keep the - // persisted-selection writes out of render: React may replay a render, but - // an abandoned render must never become a storage mutation. - useEffect(() => { - if (!data && !error) { - return - } - - $rosterHydrated.set(true) - - if (selectionHydrated) { - reconcileRosterSelection(roster, sourceSnapshot, allMeta) - const selected = selectedRosterBot(roster, $selectedRosterKey.get()) - - if ($botsPaneVisible.get() && !$groupChatWorkspace.get() && selected) { - setBotsWorkspaceOwner(botWorkspaceOwnerKey(selected), selected) - } - } - }, [data, error, selectionHydrated, roster, sourceSnapshot, allMeta]) + useReconcileRosterOwner(data, error, selectionHydrated, roster, sourceSnapshot, allMeta) const staleNotice = error && !live && roster.length @@ -580,509 +420,95 @@ export function BotsPane() { /> ) - const removeSection = (id: string) => { - const name = userSections.find(section => section.id === id)?.name || '' - const { members, undo } = deleteBotSection(id, roster) - - // No confirmation: nothing is lost (the bots fall back to Unassigned) and - // the toast's Undo puts the section and its members back. - host.notify({ - action: { label: b.sections.undo, onClick: undo }, - durationMs: 8_000, - kind: 'info', - message: b.sections.deleted(name, members.length) + const { renderUserSections, renderGatewaySection, renderGroupChatSection, renderHiddenGatewaySection } = + rosterSectionRenderers({ + b, + userSections, + roster, + allMeta, + dragging, + rosterSectionCollapsed, + toggleRosterSection, + setSectionDialog, + renderBotRow, + renderGroupRow, + sortedGroupRows }) - } - - // USER SECTIONS — composed with the gateway sections, not instead of them. - // The gateway headings own the top level whenever the roster shows more - // than one connection (that axis answers "where does this run", which no - // folder name can, and a bot's membership lives in its profile on THAT - // gateway); user sections group the rows INSIDE each connection bucket, - // indented under it, and group the flat list when there is only one. - // `keyPrefix` keeps row keys unique across the gateway buckets. - type UserSectionRow = { bot: RosterRow; kind?: 'bot' } | RosterGroupRow - - const renderUserSections = (rows: UserSectionRow[], keyPrefix = '') => { - // No sections made: the plain list, exactly as before this feature. - if (!userSections.length) { - return rows.map(row => (row.kind === 'group' ? renderGroupRow(row) : renderBotRow(row.bot, keyPrefix))) - } - - const nested = Boolean(keyPrefix) - const blocks = groupRowsBySection(rows, userSections, allMeta) - - return ( - blocks - // An empty Unassigned is not worth a heading; an empty NAMED section - // is, because it is somewhere the user made and is about to drop into. - // Inside a gateway bucket the same empty section would repeat under - // every connection, so there it only appears while a drag is in flight - // (as the drop target it exists for); the row menu files into it - // regardless. - .filter(block => block.rows.length || (block.id && (!nested || dragging))) - .map(block => { - const key = `${keyPrefix}${block.id ? `user-section:${block.id}` : UNASSIGNED_SECTION_KEY}` - const collapsed = rosterSectionCollapsed(key) - const order = userSections.findIndex(section => section.id === block.id) - - return ( - row.kind !== 'group' && botRosterKey(row.bot) === dragging) - } - key={key} - nested={nested} - onDropBot={rosterKey => { - const bot = roster.find(row => botRosterKey(row) === rosterKey) - - // `block.id` is null for Unassigned, which is exactly the value - // moveBotsToSection wants for "clear the assignment". - if (bot) { - void moveBotsToSection([bot], block.id) - } - }} - > - = 0 && order < userSections.length - 1} - canMoveUp={order > 0} - collapsed={collapsed} - count={block.rows.length} - id={block.id} - name={block.name} - onDelete={() => block.id && removeSection(block.id)} - onMove={delta => block.id && moveBotSection(block.id, delta)} - onRename={() => block.id && setSectionDialog({ id: block.id, mode: 'rename', name: block.name })} - onToggle={() => toggleRosterSection(key)} - /> - {collapsed ? null : block.rows.length ? ( -
- {block.rows.map(row => - row.kind === 'group' ? renderGroupRow(row) : renderBotRow(row.bot, `${key}:`) - )} -
- ) : ( - // Empty section: a quiet dashed slot that says what it is for, - // and doubles as a roomy drop target. -
- {b.sections.emptyHint} -
- )} - - ) - }) - ) - } - - const renderGatewaySection = (section: ResolvedRosterGatewaySection) => { - const sectionId = `gateway:${section.id}` - const collapsed = rosterSectionCollapsed(sectionId) - - return ( -
- toggleRosterSection(sectionId)} - option={section.option} - /> - {collapsed ? null : ( -
{renderUserSections(section.rows, `${section.id}:`)}
- )} -
- ) - } - - const renderGroupChatSection = () => { - const sectionId = 'group-chats' - const collapsed = rosterSectionCollapsed(sectionId) - - return ( -
- toggleRosterSection(sectionId)} - tip={`${sortedGroupRows.length} global group chat${sortedGroupRows.length === 1 ? '' : 's'}`} - /> - {collapsed ? null :
{sortedGroupRows.map(renderGroupRow)}
} -
- ) - } - - const renderHiddenGatewaySection = (section: ResolvedRosterGatewaySection) => ( -
-
- - - {section.option?.label || section.option?.connectionId || 'Current gateway'} - - {section.rows.length} -
- {section.rows.map(row => renderBotRow(row.bot, `hidden:${section.id}:`))} -
- ) return (
-
- - Bots - -
- - - - - - - - - - - setCreateOpen(true)}> - - {b.bot.newTitle} - - setGroupCreateOpen(true)}> - - {b.group.newTitle} - - - setSectionDialog({ mode: 'create' })}> - - {b.sections.newSection} - - - -
-
- {showRosterTools ? ( -
- {showRosterSearch ? ( - - ) : ( - - )} - {showRosterFilters ? ( - - - - - - - - {( - [ - ['all', b.roster.botsAndGroups], - ['bots', b.roster.botsOnly], - ['groups', b.roster.groupsOnly] - ] as [RosterKindFilter, string][] - ).map(([value, label]) => ( - setRowKindFilter(value)}> - {label} - {rowKindFilter === value ? : null} - - ))} - - {( - [ - ['all', b.roster.anyActivity], - ['active', b.roster.activeNow], - ['recent', b.roster.recentlyActive], - ['older', b.roster.older] - ] as [RosterActivityFilter, string][] - ).map(([value, label]) => ( - setActivityFilter(value)}> - {label} - {activityFilter === value ? : null} - - ))} - {gatewayOptions.length > 1 ? : null} - {gatewayOptions.length > 1 ? ( - setGatewayFilter('all')}> - - All gateways - {gatewayFilter === 'all' ? : null} - - ) : null} - {gatewayOptions.length > 1 - ? gatewayOptions.map(option => { - const status = botSourceStatus({ - sourceError: option.error, - sourceReachable: option.reachable - }) - - return ( - setGatewayFilter(option.connectionId)} - > - - {option.label || option.connectionId} - - {option.count} - - {gatewayFilter === option.connectionId ? : null} - - ) - }) - : []} - {activeFilterCount ? : null} - {activeFilterCount ? ( - { - setRowKindFilter('all') - setActivityFilter('all') - setGatewayFilter('all') - }} - > - {b.roster.clearFilters} - - ) : null} - - - ) : null} -
- ) : null} - {staleNotice ? ( -
- {staleNotice} -
- ) : null} - {(isLoading || initialRosterLoading) && !roster.length ? ( -
- -
- ) : error && !roster.length ? ( -
-
- {gatewayUp - ? b.roster.rosterUnavailable(error instanceof Error ? error.message : 'gateway error') - : b.roster.waitingForGateway} -
- -
- ) : roster.length === 0 ? ( - - ) : allBotsHidden && !hiddenExpanded ? ( -
-
- - {b.roster.allHidden} -
-

{b.roster.allHiddenDesc}

- -
- ) : rosterRows.length === 0 && matchingHiddenBots.length === 0 ? ( -
- -
- ) : ( -
-
- {showGatewaySections - ? [ - sortedGroupRows.length ? renderGroupChatSection() : null, - ...gatewaySections.sections.map(renderGatewaySection) - ].filter(Boolean) - : renderUserSections(rosterRows)} - {showHiddenSection ? ( -
- {hasRosterConstraint ? ( -
- - Hidden - {matchingHiddenBots.length} -
- ) : ( - $showHiddenBots.set(!hiddenExpanded)} - > - - Hidden - {hiddenBots.length} - - )} - {showHiddenRows ? ( - matchingHiddenBots.length ? ( - hiddenGatewaySections.sectioned ? ( - hiddenGatewaySections.sections.map(renderHiddenGatewaySection) - ) : ( - matchingHiddenBots.map((bot: RosterRow) => renderBotRow(bot, 'hidden:')) - ) - ) : ( -
{b.roster.noHiddenMatch}
- ) - ) : null} -
- ) : null} -
-
- )} - { - setCreateOpen(false) - void refetch() - }} - open={createOpen} - roster={activeSourceRoster} - /> - setGroupCreateOpen(false)} - onCreated={groupName => openGroupChat(groupName)} - open={groupCreateOpen} // Full multi-source roster: group chats can seat bots from other - // registered connections — their turns route to their own machines. - roster={roster} - /> - { - if (!open) { - setSectionDialog(null) - } - }} - onSubmit={name => { - if (sectionDialog?.mode === 'rename') { - renameBotSection(sectionDialog.id, name) - } else { - createBotSection(name, sectionDialog?.bot ? [sectionDialog.bot] : []) - } - }} - open={Boolean(sectionDialog)} - /> - { - setEditing(null) - void refetch() - }} - open={Boolean(editing)} - /> - {grouping ? setGrouping(null)} /> : null} - - {'This will permanently delete the bot '} - {deleting.name} - {' and its associated Hermes profile at '} - {deleting.path}. This cannot be undone. - - ) : null - } - destructive - doneLabel="Deleted" - onClose={() => setDeleting(null)} - onConfirm={async () => { - if (!deleting) { - return - } - - const name = deleting.name - await deleteBot(deleting) - await refetch() - host.notify({ - kind: 'success', - message: `Deleted profile ${name}` - }) - }} - open={Boolean(deleting)} - title={b.bot.deleteTitle} - /> - setDeletingGroup(null)} - onConfirm={async () => { - if (!deletingGroup) { - return - } - - await disbandGroupChat(deletingGroup.name, deletingGroup.members) - host.notify({ - kind: 'success', - message: `Deleted group “${deletingGroup.name}”` - }) - }} - open={Boolean(deletingGroup)} - title={b.group.deleteTitle} - /> + {renderRosterToolbar({ + b, + activityToasts, + activeSourceRoster, + setCreateOpen, + setGroupCreateOpen, + setSectionDialog, + showRosterTools, + showRosterSearch, + showRosterFilters, + query, + setQuery, + activeFilterCount, + gatewayOptions, + rowKindFilter, + setRowKindFilter, + activityFilter, + setActivityFilter, + gatewayFilter, + setGatewayFilter + })} + {renderRosterContent({ + b, + staleNotice, + isLoading, + initialRosterLoading, + roster, + error, + gatewayUp, + refetch, + allBotsHidden, + hiddenExpanded, + rosterRows, + matchingHiddenBots, + query, + selectedGateway, + showGatewaySections, + sortedGroupRows, + gatewaySections, + showHiddenSection, + hiddenSectionRef, + hasRosterConstraint, + hiddenBots, + showHiddenRows, + hiddenGatewaySections, + renderBotRow, + renderGroupChatSection, + renderGatewaySection, + renderUserSections, + renderHiddenGatewaySection + })} + {renderRosterDialogs({ + b, + t, + createOpen, + setCreateOpen, + groupCreateOpen, + setGroupCreateOpen, + editing, + setEditing, + deleting, + setDeleting, + deletingGroup, + setDeletingGroup, + grouping, + setGrouping, + sectionDialog, + setSectionDialog, + roster, + activeSourceRoster, + refetch + })}
) }