fix: re-register forked/replaced sessions with the broker#45
Open
iRonin wants to merge 1 commit into
Open
Conversation
The intercom runtime only initialized on session_start. When a session's id changed in-process without a fresh session_start (forking/branching a session, or a resume path that adopts an id), it never re-registered and stayed unreachable until a full process restart. getLiveContext() rejects any context whose live session id diverges from currentSessionId, and currentSessionId is only advanced inside handlers that are themselves gated by getLiveContext() — so once the id diverges, no handler can ever update it and the broker registration is stranded. Extract the session_start init into startSessionRuntime() and add adoptSessionContext(), called from turn_start, which re-initializes when the runtime never started or the live session id diverged. A turn_start only fires for a live session, so the disposed/shuttingDown flags are not used to gate adoption. Adds regression tests: a forked session that starts on its first turn registers, and an in-process fork re-registers under the new identity. Fixes nicobailon#44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Forked/replaced sessions never re-register with the broker. The runtime is initialized only in
session_start; when a session's id changes in-process without a freshsession_start(forking/branching a session, or a resume path that adopts an id), it never (re-)registers and stays unreachable over intercom until a full process restart. No error surfaces — the session is alive and running turns locally but invisible to peers.Fixes #44.
Root cause
getLiveContext()returnsnullwheneverctx.sessionManager.getSessionId() !== currentSessionId.getLiveContext()and bails first.currentSessionIdis only advanced inside those gated handlers (e.g.turn_start).So once the live id diverges from
currentSessionId, the guard rejects every handler,currentSessionIdcan never catch up, and the broker registration is stranded under the old identity. A related case: if nosession_startever fires, the extension stays inert even while the session runs turns.Fix
session_startinitialization intostartSessionRuntime(ctx)(also drops any client still registered under the previous identity).adoptSessionContext(ctx), called fromturn_start, which re-initializes when the runtime never started or the live session id diverged from the registered one. Aturn_startonly fires for a live session, so thedisposed/shuttingDownflags (true both before the firstsession_startand after a real shutdown) are intentionally not used to gate adoption —startSessionRuntimeresets them.Minimal and protocol-neutral; no broker changes.
Tests
Adds two regression tests to
intercom.integration.test.ts:session_start) registers with the broker — fails before the fix (session never appears); passes after./name, the presence name is derived from the session id; before the fix the broker keeps the stale pre-fork name, after the fix it switches to the new identity and drops the old one.Verified fail-then-pass against the unpatched tree. Full suite green: