Skip to content

fix(bot-mode): give each group thread its own member session (#90420, salvage #93580) - #111003

Merged
OutThisLife merged 3 commits into
mainfrom
fix/93580-thread-scoped-member-sessions
Sep 15, 2026
Merged

OutThisLife merged 3 commits into
mainfrom
fix/93580-thread-scoped-member-sessions

Conversation

@austinpickett

Copy link
Copy Markdown
Collaborator

What this is

Bot Mode group threads each get their own hidden member session, so a bot answering thread B no longer replies carrying thread A's context.

Supersedes #93580 (@Wenfengcheng) — same premise and same core design, rebuilt against the decomposed Bot Mode modules. That branch targeted apps/desktop/src/plugins/hermes-bots/plugin.js and two tests/*.test.mjs, all three of which no longer exist on main; cherry-picking it gives modify/delete on every file (its merge base is ~9,000 commits back). The idea was right, so it is carried forward here with credit rather than reimplemented from scratch.

The defect

ensureGroupChatSession(group, member) resolved a member's hidden plumbing session with no thread component anywhere:

  • key = groupMemberKey(member) — bare name or source-qualified key
  • resume title = Group: <roomId>
  • durable pointer written to room.sessions[memberKey]

Every other layer of the room engine is already thread-aware. The delta is narrowed with groupThreadOf(e) === thread and the watermark is ${thread}::${memberKey} (group-round-members.ts:44-48). So thread B built a correctly thread-scoped prompt and then submitted it into thread A's backend transcript — the model saw A's history and answered out of it.

The fix

  1. Session identity gains the thread. groupSessionKey(thread, member) → thread:<id>::<memberKey>, mirroring the composite the watermarks already use. Titles become Group: <roomId> · <thread>. The member half stays source-qualified, so a remote research and a local research still never share a session.
  2. Pre-thread rooms migrate instead of orphaning. A room persisted before this holds one bare member pointer. The first thread to speak adopts it — resuming the old session and re-seating it under that thread's key — and once any thread-scoped key exists the bare pointer is no longer adoptable, so later threads mint their own. Existing conversations continue in a real thread rather than sitting behind an unreferenced sid.
  3. Co-keyed readers follow. The stop interrupt targets the session of the thread the stop came from; the stranded-reply harvest resumes the session named by its own marker (which already carried {before, thread}); the clarify mirror is keyed by thread too, since one member can now be blocked in two threads at once and a room-and-member key let thread B's question replace thread A's card; the hide sweep's owner lookup reads the member half of the key and keeps its :: source check on that half alone.

Verification

  • New room-level regression proven red on the unfixed code: both threads resolved to sid-research-1 (expected 'sid-research-1' not to be 'sid-research-1'). Green after.
  • Bot Mode suite: 601/601 pass (64 files) on main@345cd2b057.
  • npm run typecheck --workspace apps/desktop: pass.
  • npm run build --workspace apps/desktop: pass, assert-dist-built ok.
  • Full desktop vitest: 9867 pass / 3 fail — all 3 (voice-prefs ×2, managed-ssh-update) reproduce on clean main with this branch stashed, so they are pre-existing and unrelated.

Tests

Four behavior contracts, no snapshots: distinct sessions per thread with same-thread reuse; the room-level no-bleed check on the actual transcripts; pre-thread adoption (first thread continues, later threads do not); source-qualified remote keys.

Supersedes #93580
Closes #90420
Closes #106460

austinpickett and others added 3 commits September 14, 2026 12:08
A group member's hidden plumbing session was keyed by member alone, so
every thread in a room collapsed onto one backend transcript: thread B
resumed thread A's session and answered carrying A's context.

Everything else in the room engine is already thread-aware — the delta is
filtered by `groupThreadOf(e) === thread` and the watermark is
`${thread}::${memberKey}` — so only session identity was missing. Key and
title it per thread, and adopt a room's pre-thread pointer into the first
thread that speaks so an upgraded room's history is continued rather than
orphaned.

Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
The co-keyed readers of `room.sessions` had to follow session identity or
they would split from it: the stop interrupt targeted a member's only
session regardless of which thread issued the stop, and the stranded-reply
harvest resumed by bare member key even though its marker already carries
`{before, thread}`.

The clarify mirror moves too — with per-thread sessions one member can be
blocked in two threads at once, and a room-and-member key let thread B's
question silently replace thread A's card. The sweep's owner lookup stays
a MEMBER question, so it reads the member half of the key and keeps the
`::` source-qualifier check on that half alone.

Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
The room-level test is the reported defect: two composer sends, two
threads, and neither backend transcript may contain the other thread's
prompt. Proven red on the unfixed code — both threads resolved to
sid-research-1.

Also pins same-thread continuity, the pre-thread adoption (first thread
continues the old conversation, later threads do not), and that the
member half of the key stays source-qualified so a remote `research` and
a local `research` never share a session.

Co-authored-by: Wenfengcheng <30426178+Wenfengcheng@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on c0a8d6d — test(bot-mode): pin thread-scoped session identity

⚠️ Warnings

OSV vulnerability scan · View job

76 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.


debug info

CI timings

CI timings · View report · View job

Wall time 2m56s vs 5m16s (-44.3%). 4 job(s) slower, 3 faster, 1 unchanged.

  • Detect affected areas: -15.0s
  • JS & TS checks / JS & TS checks: +13.0s
  • Check no committed infographics / check-no-committed-infographics: +9.0s
  • OSV scan / Emit review status: +7.0s
  • Check no case-colliding filenames / check-case-collisions: -5.0s

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 14, 2026

@OutThisLife OutThisLife left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified in a review worktree on current main: the room-level no-bleed regression is red with main's group-rounds.ts/group-turns.ts and green with this branch; hermes-bots suite 610/610 with #110969 applied alongside, no overlap. Same design as #93580 carried forward on the decomposed modules plus the pre-thread pointer adoption. Approving.

@OutThisLife
OutThisLife merged commit 5871d75 into main Sep 15, 2026
34 checks passed
@OutThisLife
OutThisLife deleted the fix/93580-thread-scoped-member-sessions branch September 15, 2026 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

3 participants