Builds on the previous commit, which correctly identified the race and the shape
of the answer. Three things needed to change before it could ship, and the reason
each matters is easy to miss.
DEFER THE TEARDOWN, NOT THE REGISTRY
The previous commit moved everything into clean(), including
this.connections.delete() and the connectionsChanged emit. That leaves the
connection in the map until the publish settles, so after disconnect() returns:
- getConnections() still lists a connection the user just disconnected;
- connect() matches by URI at the top of the function, so reconnecting to the
same URI hands back the dying connection — which clean() then deletes,
leaving the caller holding an id for something already gone.
Registry removal is now synchronous and only conn.cleanup() — the transport
teardown, the part that was killing the in-flight publish — is deferred.
BOUND THE WAIT
There was no timeout: if relay() never settles, clean() never runs and the
interval, the socket and the map entry leak permanently. That is not a corner
case. relay() enqueues rather than publishes when the client is not ready, so
disconnecting while the relay is unreachable — precisely when a user reaches for
disconnect — can hang forever. DISCONNECT_PUBLISH_TIMEOUT_MS bounds it: an
undelivered courtesy message is a smaller problem than a socket that never
closes.
RESTORE THE TYPES
doDisconnect had lost its parameter annotations and its `private` modifier, and
the message literal its DisconnectMessage type. Under "strict": true that is
TS7006 on both parameters — the package does not compile — and dropping `private`
widened the public API by accident. Formatting is back to the repo's prettier
config.
TESTS
None of the above is visible from the wallet's side: its own state is correct
either way and only the peer notices, which is how the original bug shipped.
disconnect.test.ts only ever covered dapp -> wallet.
disconnect-delivery.test.ts covers wallet -> dapp over a live relay. Verified by
reverting the fix: the two delivery tests time out after 20s, and the two
invariant tests — immediate registry removal, and reconnecting on the same URI —
pass either way, which is precisely why they are there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
a919735874