Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apps/desktop/src/plugins/hermes-bots/group-chat-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,11 @@ export function GroupChatWorkspace({ group, members, onBack, visible = true }: G
</div>
]}
{roomClarifies.map(entry => (
<GroupClarifyCard entry={entry} key={`clarify:${entry.memberKey}:${entry.requestId}`} members={members} />
<GroupClarifyCard
entry={entry}
key={`clarify:${entry.thread || 'legacy'}:${entry.memberKey}:${entry.requestId}`}
members={members}
/>
))}
{room.running ? (
<div className="px-2 py-1 text-[0.7rem] italic text-(--ui-text-quaternary)" key={'working'}>
Expand Down
36 changes: 36 additions & 0 deletions apps/desktop/src/plugins/hermes-bots/group-membership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,42 @@ export function groupMemberKey(member: GroupMember): string {
return member?.sourceScoped || member?.remoteSource ? botRosterKey(member) : member?.name
}

/** Marks a session key as thread-scoped. Pre-thread rooms stored ONE session
* per member under the bare `groupMemberKey`, and a source-qualified member
* key is itself `<connectionId>::<name>` — so the thread segment needs its
* own literal prefix to stay distinguishable from both. */
const GROUP_SESSION_THREAD_PREFIX = 'thread:'

/** Identity of a member's hidden plumbing session INSIDE one thread. Threads
* are separate conversations (own log slice, own watermarks — see
* `${thread}::${memberKey}` in group-round-members), so they must not share
* one backend transcript: thread B resuming thread A's session is how a
* member answered B carrying A's context (#90420, #106460). */
export function groupSessionKey(thread: string, member: GroupMember): string {
return `${GROUP_SESSION_THREAD_PREFIX}${thread || 'legacy'}::${groupMemberKey(member)}`
}

/** The member half of a session key. Bare keys are pre-thread rooms and are
* their own member key; the sweep reads owners by member, not by thread. */
export function groupSessionMemberKey(key: string): string {
if (!key.startsWith(GROUP_SESSION_THREAD_PREFIX)) {
return key
}

const rest = key.slice(GROUP_SESSION_THREAD_PREFIX.length)
const boundary = rest.indexOf('::')

return boundary === -1 ? rest : rest.slice(boundary + 2)
}

/** Whether this member already owns a thread-scoped session anywhere in the
* room — i.e. the room has been through the pre-thread migration. */
export function hasThreadScopedGroupSession(sessions: Record<string, unknown>, memberKey: string): boolean {
return Object.keys(sessions || {}).some(
key => key.startsWith(GROUP_SESSION_THREAD_PREFIX) && groupSessionMemberKey(key) === memberKey
)
}

/** Serializable immutable owner captured beside every group plumbing session. */
export function groupSessionOwner(member: RosterRow) {
const route = botConnectionRoute(member)
Expand Down
54 changes: 43 additions & 11 deletions apps/desktop/src/plugins/hermes-bots/group-rounds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,15 @@ describe('per-member delta', () => {
const shared: GroupMember[] = [{ name: 'research', title: '' }]
const quiet = () => !room.chat.$groupChats.get().Alpha?.running && !room.chat.$groupChats.get().Beta?.running

// Start both rooms without waiting for either drive to finish.
room.rounds.sendToGroupChat('Alpha', shared, 'ALPHA_ONLY_1')
room.rounds.sendToGroupChat('Beta', shared, 'BETA_ONLY_1')
// Start both rooms without waiting for either drive to finish. Each send
// stays in ONE thread per room, so the isolation under test is
// cross-ROOM: two rooms sharing a member must not share its session.
const alphaThread = room.rounds.sendToGroupChat('Alpha', shared, 'ALPHA_ONLY_1')
const betaThread = room.rounds.sendToGroupChat('Beta', shared, 'BETA_ONLY_1')
await drain(() => !quiet())

const alphaFirst = room.gateway.calls.find(call => call.title === 'Group: Alpha')
const betaFirst = room.gateway.calls.find(call => call.title === 'Group: Beta')
const alphaFirst = room.gateway.calls.find(call => call.title?.startsWith('Group: Alpha'))
const betaFirst = room.gateway.calls.find(call => call.title?.startsWith('Group: Beta'))

expect(alphaFirst && betaFirst).toBeTruthy()
expect(alphaFirst?.stored).not.toBe(betaFirst?.stored)
Expand All @@ -328,19 +330,20 @@ describe('per-member delta', () => {
expect(alphaFirst?.prompt).not.toContain('BETA_ONLY_1')
expect(betaFirst?.prompt).toContain('BETA_ONLY_1')
expect(betaFirst?.prompt).not.toContain('ALPHA_ONLY_1')
expect(room.chat.$groupChats.get().Alpha.sessions?.research).toBe(alphaFirst?.stored)
expect(room.chat.$groupChats.get().Beta.sessions?.research).toBe(betaFirst?.stored)
// Sessions are keyed per thread; each room minted exactly one.
expect(Object.values(room.chat.$groupChats.get().Alpha.sessions || {})).toEqual([alphaFirst?.stored])
expect(Object.values(room.chat.$groupChats.get().Beta.sessions || {})).toEqual([betaFirst?.stored])

// Interleave a second pair. Each room resumes its own session and receives
// only its unseen room delta, never the sibling room's messages.
const firstCallCount = room.gateway.calls.length
room.rounds.sendToGroupChat('Alpha', shared, 'ALPHA_ONLY_2')
room.rounds.sendToGroupChat('Beta', shared, 'BETA_ONLY_2')
room.rounds.sendToGroupChat('Alpha', shared, 'ALPHA_ONLY_2', alphaThread)
room.rounds.sendToGroupChat('Beta', shared, 'BETA_ONLY_2', betaThread)
await drain(() => !quiet())

const second = room.gateway.calls.slice(firstCallCount)
const alphaSecond = second.find(call => call.title === 'Group: Alpha')
const betaSecond = second.find(call => call.title === 'Group: Beta')
const alphaSecond = second.find(call => call.title?.startsWith('Group: Alpha'))
const betaSecond = second.find(call => call.title?.startsWith('Group: Beta'))

expect(alphaSecond?.stored).toBe(alphaFirst?.stored)
expect(betaSecond?.stored).toBe(betaFirst?.stored)
Expand Down Expand Up @@ -399,6 +402,35 @@ describe('threads', () => {
expect(replies.length).toBeGreaterThanOrEqual(1)
expect(replies.every(entry => entry.thread === billing)).toBe(true)
})

it('never lets one thread answer out of another thread’s session (#90420, #106460)', async () => {
// The user-visible defect: two visible threads shared ONE hidden member
// session, so the model answering thread B had thread A's transcript in
// its context and replied with A's topic.
const room = await loadRoom()
const member: GroupMember[] = [{ name: 'research', title: '' }]

const alpha = room.rounds.sendToGroupChat('Bleed', member, 'ALPHA_TOPIC secret')
await settle(room, 'Bleed')
const beta = room.rounds.sendToGroupChat('Bleed', member, 'BETA_TOPIC unrelated')
await settle(room, 'Bleed')

const alphaCall = room.gateway.calls.find(call => call.prompt.includes('ALPHA_TOPIC'))
const betaCall = room.gateway.calls.find(call => call.prompt.includes('BETA_TOPIC'))

expect(alpha).not.toBe(beta)
// Two threads, two distinct hidden sessions — the room stores one per thread.
expect(alphaCall?.stored).toBeTruthy()
expect(alphaCall?.stored).not.toBe(betaCall?.stored)
expect(Object.keys(room.chat.$groupChats.get().Bleed.sessions || {})).toHaveLength(2)

// And neither backend transcript ever saw the other thread's prompt.
const alphaMessages = room.gateway.sessions.get(String(alphaCall?.stored))?.messages || []
const betaMessages = room.gateway.sessions.get(String(betaCall?.stored))?.messages || []

expect(alphaMessages.some(message => message.content.includes('BETA_TOPIC'))).toBe(false)
expect(betaMessages.some(message => message.content.includes('ALPHA_TOPIC'))).toBe(false)
})
})

describe('turn prompt', () => {
Expand Down
18 changes: 16 additions & 2 deletions apps/desktop/src/plugins/hermes-bots/group-rounds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ import {
updateGroupChat
} from './group-chat'
import type { GroupChatRoom, GroupHoldStamp } from './group-chat'
import { durableGroupChatMembers, followGroupChat, groupMemberKey } from './group-membership'
import {
durableGroupChatMembers,
followGroupChat,
groupMemberKey,
groupSessionKey,
hasThreadScopedGroupSession
} from './group-membership'
import { runGroupContinuationMembers, runGroupRoundMember } from './group-round-members'
import { rejectGroupSlashCommand } from './group-slash'
import { harvestStrandedGroupReply } from './group-turns'
Expand Down Expand Up @@ -398,7 +404,15 @@ export async function stopGroupThread(group: string, thread: null | string, memb
})

// The captured descriptor owns routing even if the roster has changed.
const sessionId = onTurn ? (room.sessions || {})[groupMemberKey(onTurn)] : null
// Sessions are per thread, so a stop targets the session of the thread it
// was issued from; an unmigrated room still answers on its bare pointer.
const sessions = room.sessions || {}
const onTurnKey = onTurn ? groupMemberKey(onTurn) : ''

const sessionId = onTurn
? sessions[groupSessionKey(thread || 'legacy', onTurn)] ||
(hasThreadScopedGroupSession(sessions, onTurnKey) ? null : sessions[onTurnKey])
: null

if (onTurn && sessionId) {
try {
Expand Down
Loading
Loading