Background work outlives its owner's teardown — five confirmed sites, one missing primitive

[MastaP]

Found by an adversarial sweep (6 lenses, 13 candidates, 8 confirmed after independent refutation attempts; deduplicated to 5 distinct defects). None are in the #767 diff — all are pre-existing, in files that change touches only incidentally or not at all.

The shape

Seven defects fixed during #767 were all one class: work that outlives the teardown meant to stop it. Every one was fixed locally at its site. The sweep asked whether that shape is systemic, and all six lenses independently concluded it is, with the same diagnosis:

The repo defines a teardown verb per component and hand-rolls a boolean latch for it. There is no shared abstraction linking a spawned background task to its owner's lifetime, so each latch is checked only where someone happened to look. Seven fixes hardened one latch — TokenRegistry.dispose — and the same shape sits unguarded elsewhere.

Notably, where a component owns its timer outright the discipline is correct (e.g. ConvergenceHeartbeat.schedule() re-checks running after clearTimer, so a late settle() cannot re-arm). The failures cluster where work is spawned from an async continuation and the owner's teardown has already passed its check.

1. A failed address switch orphans a whole NostrClient — transport/NostrTransportProvider.ts:495-534

setIdentity overwrites this.nostrClient at :500, and the only disposal of the previous client is the success tail oldClient.disconnect() at :534. No try/finally.

Reach: sphere.switchToAddress(n) → core/Sphere.ts:2306 await this._transport.setIdentity(...) → the :495 branch is taken on any running wallet (nothing in setIdentity ever changes status) → one slow relay makes NostrClient.connect's Promise.all reject, and the provider's 10s race (TIMEOUTS.WEBSOCKET_CONNECT) fires before the SDK's own 30s per-relay timeout → throw skips :534 and propagates uncaught out of switchToAddress.

The orphan stays fully live: per-relay ping setInterval, an exponential auto-reconnect chain guarded only by this.closed (which nothing will now set), open sockets, and relay subscriptions whose onEvent closures still call provider.handleEvent(...) — including writing LAST_WALLET_EVENT_TS through a storage provider destroy() has already disconnected.

Teardown cannot reach it: disconnect() (:335-347) closes the replacement. grep -rn "oldClient" returns exactly :497 and :534. The Mux is no backstop — it deliberately doesn't close a shared client (MultiAddressTransportMux.ts:423-425), and rebindToSharedClient() only runs after setIdentity resolves. Each retry leaks another, because status is still 'connected'.

2. The mailbox drain runs past facade.stop() — modules/payments-v2/receive/Receive.ts:118-133

Found independently by three lenses. Receive's own poll interval launches void this.drainOnce(), which is never registered in PaymentsFacade.pendingOps — the documented quiescence gate. So facade.stop() clears the interval and returns while a drain is still doing wallet-api I/O, engine.verify, scoped-KV writes and transfer:incoming emission.

That work continues into the rest of Sphere.destroy(), against providers being disconnected underneath it. This one is money-adjacent: a token can be verified and acked while its owner is being torn down.

3. switchToAddress racing destroy() re-arms a complete live wallet — core/Sphere.ts:2186

An unawaited sphere.switchToAddress(1) concurrent with await sphere.destroy() can, after destroy() has returned, construct a fresh MultiAddressTransportMux with its own NostrClient and sockets, fresh per-address modules holding transport subscriptions, a newly started PaymentsFacade (wallet-api session, wake WebSocket with reconnect + heartbeat timers, per-stream pull intervals, receive poll), and re-identify/reconnect the transport.

A destroyed Sphere ends up with a full live wallet attached and no owner.

4. engine.dispose() mid-verify hangs quiescence forever — core/Sphere.ts:1463-1471 (breaks-function)

With the documented opt-in worker pool (Sphere.init({ verification: { createWorker } })), disposing the engine terminates its workers without settling the in-flight batch. The awaiting drain promise never resolves or rejects, so the facade's quiescence loop waits on it forever. Not a leak — a hang, on the documented teardown path.

5. TokenRegistry's static teardown doesn't use the instance one — registry/TokenRegistry.ts:260-272

resetInstance() / static destroy() call stopAutoRefresh() only. They don't set the disposed flag, don't bump the generation, and don't abort an in-flight fetch — so an initial load already in its continuation re-arms the interval on an instance getInstance() can no longer return, and its abort timer survives. The instance dispose() added in #767 does all of this correctly; the static path simply doesn't call it.

This is the cheapest fix of the five and the closest to #767's scope.

Recommendation

Fixing these five individually repeats what #767 did seven times. The lenses agree the answer is one primitive: a way to register spawned background work against its owner's lifetime, so that teardown cancels what it started rather than each site remembering to re-check a latch after every await. PaymentsFacade.pendingOps is the closest thing that exists — #2 is a defect precisely because one spawn site forgot to use it.

Suggested order: 5 (trivial, in-file), then 4 (a hang, on a documented path), then 1 and 2, then 3 with the primitive.

---

Sweep method: 6 parallel lenses over timers, sockets/aborts, floating promises, partial teardown, the payments vertical, and the registry; every candidate then handed to an independent agent instructed to refute it and to reject anything already fixed in #767. 13 candidates → 8 confirmed → 5 distinct. Evidence gathered 2026-09-02 against refactor/per-instance-registry.

[MastaP]

Two more of the same class, found while fixing item 1 and verified against
transport/NostrTransportProvider.ts on fix/766-sphere-lifecycle. Neither is
touched by the item-1 fix — that one deliberately changed only client ownership.

6. A failed setIdentity leaves the identity partially applied

setIdentity writes the new identity before it touches any client:

• :494 this.identity = identity;
• :499-502 clears processedEventIds / lastEventTs / lastDmEventTs / fallbackDmSince
• :505 this.keyManager = NostrKeyManager.fromPrivateKey(secretKey);

only then reaching the client swap at :513. So when the swap throws — which,
after the item-1 fix, correctly leaves the provider running the old socket and
the old subscriptions — the provider is running them with the new key
material and a wiped dedup window.

The item-1 fix makes the client side of this consistent (the provider keeps a
working client on every failure path); it deliberately did not reorder these
assignments, which is pre-existing. The consequence is that a retry loop is the
only correct caller behaviour: a caller that swallows the error is left in a mixed
state that nothing detects or repairs.

Fix shape: either stage identity/keyManager into locals and commit them alongside
this.nostrClient = nextClient, or restore the previous values in the catch. The
first is preferable — same "the field moves last" rule item 1 applies to the client.

7. Connection listeners are never detached from a disposed client

MultiAddressTransportMux removes its listener before dropping a client
(:387, :422, :554). NostrTransportProvider never calls
removeConnectionListener at all — grep finds it only in the mux.

Benign for the clients setIdentity builds: their listeners only logger.debug
(:539-543). Not benign for the one connect() builds — its onReconnected
(:296-303) calls this.subscribeToEvents() on the provider:

onReconnected: (url) => {
  ...
  // Re-establish subscriptions — the relay drops them on disconnect.
  this.subscribeToEvents().catch(...)
},

That client is exactly the one the first setIdentity swap disposes. It is
constructed with autoReconnect: this.config.autoReconnect, so if a disconnected
client ever fires onReconnected, a zombie re-subscribes on the live provider —
using whatever keyManager the provider holds by then, which after a swap is a
different identity.

Fix shape: keep the listener object and removeConnectionListener it before
disconnect(), exactly as the mux does.

[MastaP]

Items 1, 2, 3, 4 and 6 are fixed on fix/766-sphere-lifecycle (#772); item 5 was already fixed there. Item 7 (connection listeners never detached from a disposed client) is still open.

Item 6 landed sooner than planned because the Copilot review found it independently from the diff, while the file was already open for item 1. The fix stages identity, keyManager and the dedup window and commits them together with the client, so a failed swap applies neither — rather than restoring them in the catch. Reason: every mid-flight reader of keyManager (getNostrPubkey, the gift-wrap send path, subscribeToEvents, publishIdentityBinding) then reads the key the still-installed old client is actually subscribed with.

A third defect of the same class turned up in the same review and is fixed with it: after item 1 moved the field assignment to after the connect, a disconnect() landing during that connect would null the field and return, and the resolving swap would then install a live client plus subscriptions owned by nobody. There is now an ownership re-check before any field moves.

Every guard was falsified — reverted, observed failing, restored — and each has a mutation probe. One note for whoever picks up item 7: the else if (this.isConnected()) branch in setIdentity is unreachable (isConnected() is exactly the first branch's condition). Left in place rather than deleted as out-of-scope dead code, but it is a fossil.