Skip to content
Open
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
57 changes: 54 additions & 3 deletions apps/desktop/src/plugins/hermes-bots/group-round-members.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
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 { isGroupPassText, isSessionGoneError, runGroupChatMemberTurn } from './group-turns'
import type { Attachment, GroupMember, GroupMessage } from './types'

export interface GroupRoundMemberContext {
Expand All @@ -24,8 +24,20 @@ export interface GroupRoundMemberContext {
binding: { isLive(): boolean }
isCurrent(): boolean
failedMembers?: Set<string>
// Per-drive count of transient session-reap (4001-class) turn failures per
// member key. A thrown turn of that class (e.g. a ws_orphan_reap 4001 the
// turn-level retry couldn't recover) must NOT silently consume the user's
// delta and go stale: leave the watermark unadvanced so the next round
// re-drives the member, up to MAX_FAILED_TURN_RETRIES, after which the
// room settles against a genuinely-down member.
failedRetries?: Map<string, number>
}

/** How many extra times a member whose turn THREW is re-driven before the
* round loop gives up and lets the room settle. Timeouts (reply===null
* without a throw) keep their existing stranded-harvest path untouched. */
export const MAX_FAILED_TURN_RETRIES = 1

/** #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). */
Expand Down Expand Up @@ -128,7 +140,7 @@ async function runVisibleMemberTurn(
export async function runGroupRoundMember(
context: GroupRoundMemberContext,
member: GroupMember
): Promise<boolean | null> {
): Promise<boolean | null | 'retry'> {
const { thread, startEpoch, binding } = context

if (context.failedMembers?.has(groupMemberKey(member))) {
Expand All @@ -145,6 +157,8 @@ export async function runGroupRoundMember(
const anchorId = room.log.at(-1)?.id ?? null
let reply: null | string = null
let accepted = false
let turnFailed = false
let turnError: any = null

try {
reply = await runVisibleMemberTurn(context, member, prompt, deltaImages)
Expand Down Expand Up @@ -174,8 +188,20 @@ export async function runGroupRoundMember(
: {})
})
noteBotAttention(groupMemberKey(member), reason || error?.message || error)
context.failedMembers?.add(groupMemberKey(member))

// Parking in failedMembers is deferred to the retry decision below: a
// transient session-reap throw gets one re-drive (MAX_FAILED_TURN_RETRIES)
// before the member is parked for the rest of this drive. Any OTHER
// failure class is ambiguous — it may have double-delivered — so it parks
// immediately (the no-retry contract "does not retry ambiguous member
// admission / ambiguous submit" pins).
if (!context.failedRetries || !isSessionGoneError(error)) {
context.failedMembers?.add(groupMemberKey(member))
}

reply = null // a failed turn is a pass, never a room error
turnFailed = true
turnError = error
}

// #93127: the turn may have finished AFTER a newer user send bumped
Expand Down Expand Up @@ -220,6 +246,31 @@ export async function runGroupRoundMember(
return null
}

// A THROWN turn of the transient session-reap class that still has retries
// left must NOT advance the watermark: leaving the user's delta unseen lets
// the NEXT round re-drive this member instead of silently consuming the
// mention. Only 4001-class throws (isSessionGoneError — unambiguously
// recoverable, nothing was delivered) are retried (timeouts keep the
// stranded-harvest path), and only up to MAX_FAILED_TURN_RETRIES so the
// room still settles against a genuinely-down member — at which point the
// member is parked in failedMembers (skipped until the user acts again).
// Return 'retry' so spokeThisRound stays > 0 and the round loop runs
// another round (the room would otherwise settle after a silent round
// before the re-drive) — no entry is appended, so this only keeps the
// loop alive; the actual reply lands on the retry round.
if (turnFailed && context.failedRetries && isSessionGoneError(turnError)) {
const failedKey = groupMemberKey(member)
const priorRetries = context.failedRetries.get(failedKey) || 0

if (priorRetries < MAX_FAILED_TURN_RETRIES) {
context.failedRetries.set(failedKey, priorRetries + 1)

return 'retry'
}

context.failedMembers?.add(failedKey)
}

// Resolve the frozen submit boundary against the retained log. If it was
// trimmed away, every surviving entry is still unseen. Throws do not
// acknowledge input, and a timed-out turn keeps its submitted boundary.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'

import type * as groupChat from './group-chat'
import type * as groupRounds from './group-rounds'
import { createGroupGateway, drain, runTimersInline, scriptedStorage } from './group-test-utils'
import type { GatewayOptions, ScriptedGateway } from './group-test-utils'
import type * as groupTurns from './group-turns'
import type { GroupMember } from './types'

// Reproduction for the "member never answers, message goes stale" report.
//
// Source path proven before writing this (group-rounds.ts):
// - a failed member turn is swallowed to `reply = null` (catch, ~L665),
// - the loop STILL advances that member's watermark to log.length (~L706),
// - the only re-drive for a silent-but-owed member,
// `unaddressedGroupMentions`, counts ONLY member->member handoffs
// (`entry.from.kind !== 'member'` is skipped, ~L361).
//
// So when the USER directly @-mentions a member and that member's turn fails
// once on a transient reap, the user's message is consumed (watermark past it)
// and never re-driven — the bot stays silent forever. This test pins the
// CORRECT behavior (the member eventually answers), so it is RED on today's
// code and GREEN once the re-drive covers a user-cited member.

const { host } = vi.hoisted(() => ({ host: {} as Record<string, unknown> }))

vi.mock('@hermes/plugin-sdk', async () => {
const { pluginSdkMock } = await import('./group-test-utils')

return pluginSdkMock(host)
})

interface Room {
chat: typeof groupChat
gateway: ScriptedGateway
rounds: typeof groupRounds
turns: typeof groupTurns
}

async function loadRoom(options: GatewayOptions = {}): Promise<Room> {
vi.resetModules()
const gateway = createGroupGateway(options)

for (const key of Object.keys(host)) {
delete host[key]
}

Object.assign(host, gateway.host)

const [chat, rounds, turns, shared] = await Promise.all([
import('./group-chat'),
import('./group-rounds'),
import('./group-turns'),
import('./shared')
])

shared.setPluginCtx(scriptedStorage(gateway.storage))

return { chat, gateway, rounds, turns }
}

const MEMBERS: GroupMember[] = [
{ name: 'research', title: '' },
{ name: 'builder', title: '' }
]

const log = (room: Room, group: string) => room.chat.$groupChats.get()[group]?.log || []

async function settle(room: Room, group: string) {
await drain(() => Boolean(room.chat.$groupChats.get()[group]?.running))
}

beforeEach(() => {
runTimersInline()
})

describe('user-mentioned member whose first turn fails transiently', () => {
it('still answers the user (the mention is not permanently consumed)', async () => {
// builder throws a 4001-class "runtime session was reaped" error on its
// FIRST submit AND again on the turn-level submit retry's resubmit — a
// reap the turn-level retry could not recover — then answers on the
// room-level re-drive. A generic (ambiguous) error must NOT be retried —
// that contract is pinned by group-rounds.test.ts "does not retry
// ambiguous member admission / ambiguous submit".
let builderAttempts = 0

const reapError: any = new Error('session-scoped RPC rejected: rt-builder-1 not in memory')
reapError.code = 4001

const room = await loadRoom({
turn: ({ profile }) => {
if (profile === 'builder') {
builderAttempts += 1

if (builderAttempts <= 2) {
throw reapError
}

return 'builder here — on it.'
}

return '(pass)'
}
})

// The user addresses builder directly.
room.rounds.sendToGroupChat('Council', MEMBERS, '@builder can you take a look?')
await settle(room, 'Council')

const memberLines = log(room, 'Council')
.filter(entry => entry.from.kind === 'member')
.map(entry => `${entry.from.name}: ${entry.text}`)

// CORRECT behavior: builder eventually posts its reply. On today's code the
// failed first turn is swallowed, the watermark is advanced past the user
// message, and the user-cited member is never re-driven — so builder stays
// silent and this assertion fails (the bug).
expect(memberLines.some(line => line.startsWith('builder:'))).toBe(true)
})
})
11 changes: 9 additions & 2 deletions apps/desktop/src/plugins/hermes-bots/group-rounds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,9 @@ export async function runGroupChatRounds(group: string, members: GroupMember[],
startEpoch,
failedMembers,
binding,
isCurrent
isCurrent,
// Per-drive transient-failure retry counters (see GroupRoundMemberContext).
failedRetries: new Map<string, number>()
}

let posted = 0
Expand Down Expand Up @@ -591,7 +593,12 @@ export async function runGroupChatRounds(group: string, members: GroupMember[],
return
}

if (result) {
// 'retry' = a transient failure that must be re-driven next round:
// keep the round alive (so the loop doesn't settle before the retry)
// without counting it as a posted message.
if (result === 'retry') {
spokeThisRound += 1
} else if (result) {
posted += 1
spokeThisRound += 1
}
Expand Down