doDisconnect fired the courtesy `disconnect` message without awaiting it, then
tore the relay connection down on the next line:
conn.client.relay(disconnectMsg).catch(() => {}); // fire and forget
...
conn.cleanup(); // closes the pool underneath it
relay() resolves only after `Promise.allSettled(pool.publish(...))` — a real
round trip to every configured relay. cleanup() closed the pool while that
publish was still in flight, so the message usually never left and the dapp went
on believing the wallet was connected until its own liveness timeout fired.
Downstream wallets were patching this out of the published package.
Teardown now splits into two halves with opposite timing requirements.
Registry removal stays synchronous. getConnections() is what a UI renders, and
connect() returns an existing connection for a URI, so leaving this one in the
map while its teardown is pending would hand a caller a dying connection. This
is the one place this differs from !30 and from the downstream patches, which
defer the registry removal along with the teardown.
Relay teardown is deferred until the publish settles, bounded by
DISCONNECT_PUBLISH_TIMEOUT_MS (5s). The bound matters: "the publish never
settles" is exactly the case where a relay is unreachable, and a socket that is
never closed is worse than a courtesy message that is never delivered.
disconnect() keeps its synchronous void signature — not a breaking change.
Why it shipped broken: disconnect.test.ts only covered dapp → wallet. Nothing
exercised wallet → dapp, and the failure is invisible from the wallet's side —
its own state is correct either way, and only the peer notices.
disconnect-delivery.test.ts covers that direction over a live relay, including
the two invariants the deferral must not break (registry cleared immediately,
URI reusable afterwards).
Also fixes a latent hang in the integration harness that the new file exposed.
setupConnection gated both the dapp_ready send and the message handler inside
the keyexchangecomplete callback, so the handshake hung on receiving the wallet's
single wallet_ready for the cycle. Miss it — the dapp's subscription can come up
after the wallet has already published — and key exchange never resolves, the
handler never registers, no dapp_ready is ever sent, and the wallet, guarded by
walletReadySentThisCycle, has nothing prompting it to retry. It now re-announces
dapp_ready(wallet_discovered=false) every 2s until key exchange completes, which
resets that guard and earns another wallet_ready: the recovery path mutual
discovery already specifies, which the harness was not using. Plus retry: 2 on
the integration config, since these tests talk to live relays and a dropped
connection is an environment failure rather than a regression.
docs/wallet.md gains a "Sending disconnect" section for the synchronous/deferred
split and what a caller may rely on. docs/protocol.md gains the sender-side half
of the courtesy-disconnect semantics, which previously read as though "no
acknowledgement" licensed fire-and-forget. That reading is what produced the bug.
The race was diagnosed and first fixed by hantyrram (Ronaldo Ramano) in !30,
which this supersedes — the deferral is their fix; this changes only how it is
scoped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dcda4fce6a