Retry a native direct-path negotiation that throws, not only one that never opens

[dormouse-bot]

untilOpen in lib/src/host/remote/native-direct-peer.test.ts never spent its
retry budget on the failure it was written for. await start() sat outside the
try, so a negotiation that threw propagated on the first attempt with three
of four attempts unused — only a negotiation that opened no channel was ever
retried. This retries both, and moves the cleanup that implies into untilOpen
itself.

Why the run went red

Run 34994870240
failed one case out of 3,162:

FAIL src/host/remote/native-direct-peer.test.ts > the direct path over the native addon
     > does not carry a channel's reliability across to the answerer
Error: libdatachannel error while adding remote description: Got a remote candidate without ICE transport
 ❯ RTCPeerConnection.setRemoteDescription node-datachannel/src/polyfill/RTCPeerConnection.ts:506:25
 ❯ startReliabilityProbe src/host/remote/native-direct-peer.test.ts:179:17
 ❯ untilOpen src/host/remote/native-direct-peer.test.ts:100:17

Line 179 is the offerer taking the answer, inside startReliabilityProbe —
which, as its own comment says, has to complete the whole negotiation inside
start, because this stack raises datachannel when the association carries
the channel, not when the offer describes it. So for that case nearly every
ICE-level failure landed in the un-retried region, and the addon's error became
the case's error.

That is the outcome the file explicitly rules out: "a lost attempt is retried
on a fresh session rather than reported as a broken addon", backed by a
measured ~2% loss rate. The retry machinery was already there and already
correct in intent; its scope was one line too narrow.

The shape of the fix

Retrying a start that throws creates an obligation the old shape never had:
the peers it built before throwing are unreachable — there is no Negotiation
to abandon — so they would stay open, gathering candidates beside the next
attempt. That is precisely the interference Negotiation.abandon exists to
prevent ("so a retry does not run beside them"), on cases whose point is
measuring what one real association carries.

Rather than give each start its own try/cleanup/rethrow, untilOpen hands
it an Attempt with a single method — keep(peer), which registers a peer and
returns it. A lost attempt closes everything registered, whether the loss was a
throw or a channel that never opened. One place owns the retry and the cleanup
that follows from it.

This matters because the convention was already easy to miss: all three
negotiations here exchange descriptions inline, including the anonymous one in
"carries a full-size Noise transport message in one frame, intact". That third
one fails differently — DirectPeer.offer()/.answer() swallow the addon's
error, #fail closes only the end that saw it, and the throw is then
expect(offer).not.toBeNull() firing with the other peer still built. A
per-call-site convention would have had to be remembered there too.

The one change outside that contract is the trickle forwarding in
startReliabilityProbe, which was void-ing addIceCandidate with no catch.
This polyfill passes a candidate straight to libdatachannel with no buffering
(addIceCandidate → native addRemoteCandidate), so a candidate arriving
before the far end has its remote description throws — as an unhandled
rejection, which no retry can absorb. It is dropped now. This is the adjacent
hazard on the same lines, not the error this run hit; that one came from
setRemoteDescription itself.

Verification

Not run locally: the sandbox has no route to the npm registry, so dependencies
could not be installed, and this test needs the native node-datachannel addon
and real host candidates. CI on this branch is the check that matters.

Nothing about what the tests assert changes — every expectation in all three
cases is untouched.

Why this is intermittent, not the node-datachannel 0.33.4 bump

node-datachannel went to 0.33.4 in #642 (merged 2026-09-14 22:31). This test
passed on every main run after it and before the failure — 34904560730,
34904675804, 34904689401, 34919507596, 34919589579, 34919733499, 34919792817 —
and on 34995199739,
the next commit on main after the red one, which is green across all five jobs.
So main is not currently broken; the defect is that the harness turned a ~2%
per-attempt ICE loss into a red default branch instead of a second attempt.

[cloudflare-workers-and-pages]

Deploying mouseterm with    Cloudflare Pages

Latest commit:	0266dc9
Status:	✅  Deploy successful!
Preview URL:	https://3df1241f.mouseterm.pages.dev
Branch Preview URL:	https://fix-ci-34994870240.mouseterm.pages.dev

View logs

[dormouse-bot]

Feedback on work in progress — not a merge verdict. Mark the PR ready when you
want the full review.

One gap, and a note on where the fix sits.

The third start has no abandon-on-throw, and it negotiates inline too. The
anonymous callback in "carries a full-size Noise transport message in one
frame, intact" runs offerer.offer(), answerer.answer(), and
acceptAnswer() inside start — the same shape as startReliabilityProbe, and
the same setRemoteDescription path that turned run 34994870240 red. It is the
one start this PR did not give a try/abandon/rethrow.

It doesn't throw the addon's error, because DirectPeer.offer() and .answer()
swallow it — each catch calls #fail(...) and returns null, and #fail
closes its own peer only. The throw is then the guard on the next line
(expect(offer).not.toBeNull()), which fires with the other peer still built
and gathering. untilOpen cannot clean that up: started is still undefined
on this path, so started?.abandon() is a no-op.

Before this PR that cost one leaked peer and the case failed. Now the attempt is
retried, so a run that loses ICE on three of four attempts leaves three stale
RTCPeerConnections gathering candidates beside the live one — the interference
the Negotiation.abandon doc calls out ("so a retry does not run beside them"),
on the case whose whole point is measuring what one real association carries.
afterAll's cleanup() still frees the native threads, so nothing hangs the
worker; the cost is confined to the retries.

The contract would be harder to miss if untilOpen owned it. Each start
now repeats the same build-abandon-up-front, try, catch, abandon,
rethrow shape, and the doc comment asserts all of them do it — which is what
made the third one invisible. Having untilOpen hand start something to
register peers on (or take a cleanup alongside the promise) would put the
guarantee in the one place that already owns the retry, instead of in a
convention three call sites have to each remember.

[dormouse-bot]

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The third negotiation is covered: Attempt.keep now reaches the peer DirectPeer.#fail leaves behind, which is the end that had nothing left to abandon it. One leftover from the move.

startConnected's abandon is now dead, and the Negotiation doc describes a job Attempt.keep took over. untilOpen's catch was that abandon's only caller — the two cases built on connectedDirect() ("negotiates a channel and carries protocol-v1 on it" and "ends the session at both ends when the Burrow closes its peer") take the run and never call it — so its body, closing [...clientPeers, ...burrowPeers], is unreachable after this commit, kept compiling only by the Negotiation constraint it has to satisfy. The other two implementations are still called, but from the finally of a case that finished, not from a retry: abandon's doc, "Drop this attempt's peers, so a retry does not run beside them", is the one claim in the file that now points at the wrong mechanism — which is the same way the third negotiation stayed invisible.

Narrowing Negotiation to open() alone settles both. untilOpen returns T, so the two starts that declare their own abandon still type-check at run.abandon(); startConnected drops the member with nothing left to satisfy, and the stale doc line goes with it. What is left is one contract per phase — the attempt closes what a lost negotiation built, the case closes what a finished one returns — instead of one member serving both and neither well.