fix: stable, caller-supplied broker session id (survives reconnect)#40
Open
iRonin wants to merge 1 commit into
Open
fix: stable, caller-supplied broker session id (survives reconnect)#40iRonin wants to merge 1 commit into
iRonin wants to merge 1 commit into
Conversation
…tity The broker assigned a fresh randomUUID() on every register, so the routing identity changed on every reconnect. A peer that resolved a name to an id (or sent by id) would hit "Session not found" once the target reconnected with a new id \u2014 even though the target was alive and named. Let the client supply a stable id (the pi session id) in the register message; the broker uses it as the routing identity when present and falls back to randomUUID for older clients. Reconnects keep the same id, so previously resolved ids stay valid. As a side effect the list/parens id, any status surface, and the subagent-chat-* fallback alias all converge on one stable id. Also harden the socket-close handler: only evict a registry entry if it still points at the closing socket, so a reconnect that rebinds the id before the old socket's close fires is not torn down by that late close. Test: a worker connects with a stable id, a peer resolves it, the worker reconnects (new socket, same id), and the peer's send to the SAME id still delivers. Verified the test fails with per-connect random ids.
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
1 participant
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
Makes the broker routing identity stable across reconnects by letting the client supply its own id (the pi session id) at register time. Relates to #39; complementary to #35/#37.
Problem
broker.tsassignsconst id = randomUUID()on everyregister, so the routing identity changes on every reconnect. A peer that resolved a name → id (the normalsend/askpath doesresolveSessionTarget→client.send(<id>)) hitsdelivery_failed: "Session not found"the moment the target reconnects with a new id — even though the target is alive and correctly named. Sending "by name" is affected too, because the sender resolves the name to an id first and then routes by id.Fix
registeraccepts an optional caller-suppliedsessionId. When present the broker uses it as the routing identity; otherwise it falls back torandomUUID()(older clients unaffected).closehandler: only evict a registry entry when it still points at the closing socket, so a reconnect that rebinds the id before the old socket'sclosefires isn't torn down by that late close.Side benefit: the list/parens id, any status surface, and the
subagent-chat-*fallback alias converge on one stable id instead of three different values.Compatibility
Protocol-additive (
sessionId?onregister). A mixed fleet works: clients that don't send it still get a random id. No behavior change for clients that never reconnect.Test
A worker connects with a stable id; a peer resolves it; the worker reconnects (new socket, same id); the peer's send to the same id still delivers. Verified the test fails with per-connect random ids and passes with the fix. Full suite 37/37.