fix(bot-mode): isolate group member sessions by thread

[Wenfengcheng]

What does this PR do?

Bot Mode group threads now keep separate hidden model sessions for each room + thread + member identity.

The room log and watermarks were already thread-aware, but ensureGroupChatSession(group, member) still stored one session pointer per member. Starting thread B therefore resumed the same backend transcript used by thread A, allowing a visually new thread to inherit the previous topic and model context.

This change scopes the session pointer, title, transcript bootstrap, stranded replies, clarification/approval state, and background harvest to the thread. The same member still reuses one session within a thread, while a different thread gets a different hidden session.

Related Issue

Fixes #90420

Changes Made

• Key hidden member sessions by stable room scope, thread, and source-qualified member identity.
• Reuse the same session within one thread while bootstrapping a new session from only that thread's retained history.
• Keep legacy member pointers for compatibility without using them as cross-thread owners.
• Persist immutable session ownership for local and remote members so hide/sweep behavior stays source-safe.
• Scope stranded replies, clarification/approval mirrors, retry polling, and watermarks to the originating thread.
• Keep session scope stable and length-safe across room renames and long room ids.
• Fail closed when a late session ensure races with group disband, preventing a deleted/tombstoned room from being recreated.

How to Test

1. Open Desktop → Bots and create or open a group containing a bot.
2. Start thread A and discuss a unique topic.
3. Start thread B with a different topic and ask a question that would expose prior context.
4. Thread B should not inherit thread A's topic; returning to thread A should retain its own context.

Validation on exact SHA 770878181e60a80d66bb9973987cc93bc0dc0fe2:

node --test apps/desktop/src/plugins/hermes-bots/tests/group-chat.test.mjs
# 98 pass, 0 fail

node --test apps/desktop/src/plugins/hermes-bots/tests/*.test.mjs
# 539 pass, 0 fail

npm run typecheck --workspace apps/desktop
# PASS

npm run build --workspace apps/desktop
# PASS

git diff --check origin/main...HEAD
# PASS

A real Windows Electron E2E created two visible group threads and inspected the actual state.db: the threads produced two distinct session ids, and neither backend transcript contained the other thread's unique prompt (1 pass, 0 fail). The temporary environment-specific harness was not included in this diff.

Independent exact-SHA review: PASS, zero concrete #90420 defects.

Scope and overlap

PR #92041 addresses the separate room-drive scheduling race: serializing live follow-ups and preserving the pre-submit watermark boundary. This PR does not duplicate that queue. It closes #90420's remaining model-context leak by making hidden member-session identity thread-scoped.

Type of Change

• Bug fix
• Tests

Checklist

• Rebased onto current main
• Regression tests prove different threads use different sessions
• Same-thread continuity remains covered
• Local and source-qualified remote ownership remains covered
• Disband, stranded reply, clarification, and approval boundaries covered
• Tested on Windows 11

[Enough1122]

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

Strong design and unusually thorough testing — per-thread session isolation with stable scopes across renames (sessionScopeId), single-flight deduping of concurrent ensures, tombstone rejection for disbanded rooms, legacy-contract preservation for callers without a thread, and behavioral tests covering each of those plus clarify isolation. Points:

1. apps/desktop/src/plugins/hermes-bots/plugin.js:~7560 — bootstrapDelta feeds the entire retained history of the thread into a brand-new session's first prompt, uncapped. A long-running topic that predates session isolation (the migration case this explicitly targets) produces one oversized turn-1 prompt — potentially blowing context limits or cost expectations. Cap the bootstrap slice (e.g., last N entries, mirroring whatever bound the room log trimmer uses) and note the truncation in the prompt so the member knows history was elided.

2. apps/desktop/src/plugins/hermes-bots/plugin.js:~6800 — the title-length guard throws when scope + thread exceeds 100 chars. Scope is capped (≤40 roomId or minted id), but the thread segment is platform-supplied; a verbose thread identifier turns every subsequent round into a caught failure + attention badge instead of a working session. Truncating/hashing an over-long thread segment for the title (while keeping the full id as the storage key) would degrade gracefully instead.

3. plugin.js:~6931 — normalizedThread maps falsy-but-not-null threads to the literal string 'legacy', which then becomes part of storage keys and titles. A platform whose real thread id is "legacy" collides with the sentinel; a prefixed form (__legacy__) costs nothing now and avoids a weird future bug.

Minor: hideOwnedBotSessions's key.endsWith('::' + persistedKey) matching is correct given the :: boundary, but one comment stating why plain includes would be wrong would help the next reader. The stranded-marker shape migration (number → object) is handled carefully.

[Wenfengcheng]

Thanks for the review. I checked all three points against exact head 770878181e60a80d66bb9973987cc93bc0dc0fe2; none requires a code change:

1. The bootstrap source is the retained thread, but the actual prompt is already bounded at the consumption boundary: deltaLines: delta.slice(-GROUP_CHAT_HISTORY_LIMIT) (plugin.js:7571-7576), where GROUP_CHAT_HISTORY_LIMIT = 24 (plugin.js:6202). The room log itself is also bounded by trimGroupChatLog(..., GROUP_CHAT_HISTORY_LIMIT * 4) (plugin.js:6417-6431). A new per-thread session therefore receives at most the last 24 retained entries, not the whole retained room history.
2. Group thread ids are not platform-supplied. They are minted internally by mintGroupThreadId() as t${Date.now().toString(36)}-${...} (plugin.js:11851-11852), and legacy hydration assigns plugin-owned synthetic ids. The 100-character guard is fail-closed defense for corrupted/foreign persisted state, not a reachable verbose-platform-id path.
3. legacy is deliberately the pre-thread sentinel used by groupThreadOf(entry) (plugin.js:11847-11849). Live ids always use the t... mint above, so the sentinel cannot collide with a real generated thread id.

The endsWith('::' + persistedKey) boundary is intentional as noted; no behavior change is needed. I am handling the now-reported main-branch conflict separately and will re-run exact-SHA verification after rebasing.

[Wenfengcheng]

Rebased onto current main@41447a6d7063b2772b0c2f26a5b22d9bd444fb43 and force-with-lease updated the PR to exact head 213d7a7539360f37ac211e9faba443a5ec59c42f.

The conflict overlapped main's per-turn retained route lease (#93602) with this PR's per-thread session ensure. The merged contract now retains the member route before thread-session ensure/create/resume and keeps it through attachment staging, submit, 4001 stored-id recovery, and polling. Direct helper callers still retain automatically; the production round path passes an explicit already-retained flag, so it does not double-retain. Every continue/return/error path releases through finally.

A new production-path regression calls runGroupChatRounds itself and verifies the lease covers session creation and prompt submission while the durable pointer remains thread-scoped (t1::helper).

Validation on exact head:

• full Hermes Bots plugin suite: 566/566 passed
• focused thread + lease integration: 106/106 passed
• Desktop TypeScript typecheck: passed
• production build + dist assertion: passed
• git diff --check: passed
• independent exact-SHA review: PASS

GitHub now reports the PR mergeable with no conflict. Fork workflows are action_required (awaiting maintainer approval), not failed.

[maxmilian]

@Wenfengcheng — two things you may not have seen, since neither of them shows up as a notification on this PR.

1. It has gone CONFLICTING again. Head is 24eeafa, and mergeable currently reports CONFLICTING / mergeStateStatus: DIRTY. This is the second time main has moved under it — you rebased through the #93602 retained-route-lease overlap back on Aug 25, and the Bot Mode modules have been decomposed since.

2. Your PR was named as the designated fix on Sep 9. @teknium1 filed #106460 from the weekly user-pain miner — two Discord support threads this week, support-confirmed repro, classified P2 with severity raised because cross-thread bleed is also an information-leak surface when the threads belong to different clients. The issue says, verbatim:

SALVAGE CANDIDATE: PR #93580 (@Wenfengcheng) — fix(bot-mode): isolate group member sessions by thread. Currently CONFLICTING against main and needs adapting to the decomposed Bot Mode modules, but it targets exactly this defect. Recommend cherry-pick salvage onto origin/main with contributor credit rather than a fresh implementation.

The proposed direction there — key member-session resolution on (group, thread) and migrate existing group-keyed sessions to the group's default thread so history is not orphaned — is what this PR already does, plus the migration step.

So this is not a stale PR any more; it is the one an open P2 points at. Worth a rebase whenever you have the time, and linking it to #106460 so the two do not drift apart.

If you would rather not carry it further, say so and I am happy to do the rebase-and-adapt work against the decomposed modules with your commit kept as the base and credit to you — but it is your patch and your call, and I would rather you land it.

[austinpickett]

Superseded by #111003.

Your premise and your design were both right, and #111003 carries them forward with credit (Co-authored-by). The only reason this one couldn't be rebased is mechanical: this branch targets apps/desktop/src/plugins/hermes-bots/plugin.js and the two tests/*.test.mjs files, and all three were deleted when Bot Mode was decomposed into per-topic modules. A cherry-pick onto current main gives modify/delete on every file, so there was no diff left to salvage — only the idea, which is the part that mattered.

What carried over unchanged: keying member-session identity on (thread, member), keeping the member half source-qualified, and scoping the stranded-reply and clarify/approval state to the originating thread. #111003 adds the migration step #106460 asked for — a room persisted before this adopts its bare member pointer into the first thread that speaks, so existing history continues in a real thread instead of being orphaned by the upgrade.

Thanks for the original work and for the careful exact-SHA reviews on this thread.

[Wenfengcheng]

@maxmilian @austinpickett Sorry I missed this message and did not follow up sooner. Thank you both for following up and carrying the fix forward in #111003. I really appreciate the work to adapt it to the decomposed Bot Mode modules, address the migration, and preserve the contributor credit.