fix(bot-mode): fail closed on transient group-session resume failures

[pierrenode]

Summary

Sibling-gap: findExistingCanonicalChat was fixed for exactly this bug class hours earlier in this same file (87b645f) — "FAIL CLOSED. A failed registry lookup MUST NOT read as 'no Bot Chat exists' — that is the one remaining way to fork a bot's forever chat." ensureGroupChatSession (group-member sessions, not the 1:1 canonical chat) still has the pre-fix shape: its resume loop (stored sid, then title lookup) catches any session.resume error identically and falls through to session.create.

for (const target of [known, title]) {
  ...
  try {
    const res = await requestForBot(member, 'session.resume', { session_id: target, ... })
    ...
  } catch {
    /* fall through to create */   // <- any error, not just "doesn't exist"
  }
}

A transient failure (the backend still warming up after a restart — the exact window findExistingCanonicalChat's fix calls out — or a network blip on a cross-connection lookup) reads as "no session, mint a new one." That mints a fresh hidden session AND silently overwrites room.sessions[key] with the new id, orphaning the member's real prior history from the room. ensureGroupChatSession is actually more exposed than the 1:1 case it mirrors: it runs on every group turn (runGroupChatMemberTurn), with two independent swallow points instead of one.

Fix

Distinguish "genuinely doesn't exist" from "transient failure" the same way the gateway itself does. session.resume's own handler (tui_gateway/methods_session.py) returns JSON-RPC error code 4007 only when the target truly isn't found (checked by id, then by title); every other failure — including 4130 ("session too large to resume", SessionResumeTooLargeError — a session that genuinely does exist) — now surfaces as a thrown error instead of being silently read as "doesn't exist."

The call site already has a safety net for this: runGroupChatMemberTurn's caller wraps it in try { ... } catch { recordGroupActivity(group, { kind: 'failed', ... }); reply = null /* a failed turn is a pass, never a room error */ }. So a transient hiccup now costs the member one skipped round (it retries from scratch next round, since nothing was persisted to room.sessions[key] on failure) instead of permanently forking the session.

Testing

• apps/desktop/src/plugins/hermes-bots/tests/group-chat.test.mjs: two new tests —
  • a transient resume failure fails closed instead of forking the member session: a .code = 5000 (non-4007) resume error rejects ensureGroupChatSession and leaves room.sessions and the session count untouched (mutation-verified: fails on pre-fix code with "Missing expected rejection" — it silently created a second session instead).
  • a genuinely nonexistent stored session (4007) still falls through to the title lookup, then create: confirms the legitimate "nothing to resume yet" path still works.
• Updated the session.resume "not found" mocks in this test file plus the two sibling harnesses that independently mock the same RPC (group-activity.test.mjs, group-chat-attachments.test.mjs) to shape their thrown error like the real gateway's JsonRpcGatewayError (a .code property, per apps/shared/src/json-rpc-gateway.ts) — their old plain-Error, code-less mocks no longer matched what production now branches on, which surfaced as 3 failures across those two files until fixed the same way.
• Full apps/desktop/src/plugins/hermes-bots/tests/*.test.mjs suite: 413/413 passing.
• node --check clean on all four changed files. eslint could not run (pre-existing, unrelated to this change — workspace-scoped apps/desktop install is missing the root-only globals peer dependency this repo's eslint.config.mjs requires).

Competing PRs

Checked gh pr list --search across several keyword combinations. #90343 ("bot sessions always follow the profile's current config") touches ensureGroupChatSession but only adds new session.create parameters (room_plumbing, follow_profile_config) for a different bug (stale provider pin after a profile switch) — it doesn't touch the resume try/catch this PR changes. No other open PR addresses this gap.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Overall: correct and well-tested fail-closed fix — the fork-on-transient-failure consequence (silently overwriting room.sessions[key], orphaning real history) fully justifies surfacing instead of minting, the 4007 carve-out is narrowly drawn, and both new tests pin the two directions (transient rejects; genuine-not-found still falls through to title lookup then create). Two small points:

1. Numeric-code strictness — plugin.js:5825: error?.code !== 4007 assumes the gateway always emits a number code. If any transport path serializes JSON-RPC errors with string codes ("4007") or a subclass sets it via a getter that's absent after re-wrapping, a genuine not-found gets re-thrown as a hard failure — the opposite failure mode of the bug being fixed. A cheap String(error?.code) !== '4007' (with a comment) would make the comparison shape-agnostic while staying equally narrow.

2. Triplicated test stub — the identical err.code = 4007 mock-shaping block is copied into three test files (group-activity.test.mjs, group-chat-attachments.test.mjs, group-chat.test.mjs). These suites already share loading patterns; a tiny exported helper (or fixture) for "throw shaped like JsonRpcGatewayError" would keep the three in sync if the error shape ever grows fields.

The error message including the member name and the explicit "not starting a new one" clause is exactly the right operator-facing wording.

[teknium1]

Merged via ring-2 consolidated PR #93430 — your commit cherry-picked with authorship preserved (group-session resume fails closed on transient errors; only 4007 means absent, mirroring your earlier findExistingCanonicalChat fix). Thanks @pierrenode — that's two solid halves of the bot-mode reliability train from you.