Skip to content

fix(desktop): require session_id for all session-scoped gateway events - #52025

Closed
Sahil-SS9 wants to merge 1 commit into
NousResearch:mainfrom
Sahil-SS9:fix/issue-49106-session-leak
Closed

fix(desktop): require session_id for all session-scoped gateway events#52025
Sahil-SS9 wants to merge 1 commit into
NousResearch:mainfrom
Sahil-SS9:fix/issue-49106-session-leak

Conversation

@Sahil-SS9

Copy link
Copy Markdown
Contributor

Fixes #49106
Fixes #47709

Description

Previously only subagent.* events required an explicit session_id.
All other events (message.delta, message.complete, tool.start,
tool.complete, reasoning.delta, session.info, clarify.request,
approval.request, etc.) fell through to activeSessionIdRef.current
when unscoped, causing background session output to bleed into whichever
session the user was currently viewing.

This was the root cause of:

Fix

Expanded gatewayEventRequiresSessionId to require session_id for all
session-scoped events. Only truly global broadcasts (gateway.ready,
skin.changed) are exempt. Unknown event types are also required to
have a session_id (defensive).

The tui_gateway already stamps every session-scoped event with the
correct session_id, so this change is safe in normal operation.
If the gateway ever fails to stamp one (race, background-process
delivery, subagent mirror, stale transport), dropping is safer than
silently attributing to the focused session.

Verification

  • Updated tests pass (gateway-events.test.ts)
  • Quality gates: S1 (secrets), S2 (personal refs), B1 (branch current),
    C1 (conventional commits), C2 (no em-dashes), F1 (focused diff) all pass
  • Only 2 files changed, no unrelated edits

Previously only subagent.* events required an explicit session_id.
All other events (message, tool, reasoning, session.info, etc.) fell
through to activeSessionIdRef.current when unscoped, causing background
session output to bleed into the focused chat.

Expand gatewayEventRequiresSessionId to require session_id for all
session-scoped events. Only truly global broadcasts (gateway.ready,
skin.changed) are exempt.

Fixes NousResearch#49106 (Web/WeChat session history leak)
Fixes NousResearch#47709 (Desktop stream-bleed: newer session output in older session)
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 24, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #49659 — verified by comparing diffs: both PRs make the identical change to apps/desktop/src/lib/gateway-events.ts, widening gatewayEventRequiresSessionId() to require session_id for every session-scoped event (exempting only gateway.ready / skin.changed) and dropping unscoped events rather than attributing them to the focused chat. #49659 is the earlier-open PR for #49106/#47709.

Related competing approaches that take a different mechanism (pin/resolve unscoped events to their originating session instead of dropping): #48281 and #47743. Maintainer should pick one mechanism across this cluster.

@Sahil-SS9

Sahil-SS9 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @alt-glitch for flagging. You're right — #49659 made the same fix, and mine is a duplicate. I'll close this PR. Appreciate the pointer to #48281 and #47743 as well — I'll review those approaches.

@Sahil-SS9

Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #49659 — same fix authored independently. No further action needed on this branch.

@Sahil-SS9 Sahil-SS9 closed this Jun 24, 2026
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/*) duplicate This issue or pull request already exists 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

Development

Successfully merging this pull request may close these issues.

Web/WeChat sessions leak history: turns from one Hermes session appear in another [Bug]: Desktop shows newer session output in an older session

2 participants