amadan
client: a diagnosis that runs out releases its target
S2-17 (P2): the budget was threaded correctly and then leaked out of the one exit nobody watched. When an aborted probe or a transient refusal exhausted waitWithin, recoverFromNotFound returned quietly: the queue was still there, the pump's finally restarted it, its expired head called unreachable, and that opened a reset with a fresh thirty seconds. Codex measured it — zero stale-target calls, 31000 for the offer and 61000 for the reset — so "the target was dropped on time" was true of a client that had already restarted the clock behind it. Both loops now round back to a single check at the top, which is the one place a spent budget is answered, and it answers by releasing the target: queue dropped before the pump is released, connection closed, mark set, and the next roster settles it. resetLoop gains that check too — it looked at the participation before every attempt but never at the clock, so one POST could go out past the deadline that was supposed to cover it. createPC clears the stale mark, because building a connection to a peer means the server has just named it; otherwise a pair the hub reset while this tab was giving up on it would be rebuilt and never spoken to. state() exposes the stale set, which is what lets a gate tell "dropped and waiting for a roster" from "dropped and quietly trying again". negotiation.test.mjs gains a pure test of the exhaustion path itself: the recovery lifted out of the shipped source, given fifty milliseconds and a probe that never answers, must release its target exactly once and ask for nothing.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
2 files changed,
+74
−2
internal/meet/static/room.js+30 −2test/browser/negotiation.test.mjs+44 −0
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js| index ac8006e..d3ffce3 100644 |
| --- a/internal/meet/static/room.js |
| +++ b/internal/meet/static/room.js |
| @@ -762,6 +762,12 @@ |
| terminal('nowebrtc'); |
| return null; |
| } |
| + // Building a connection to a peer means the server has just named it |
| + // — a roster, a `peer`, or a `pair` — so whatever a 404 marked stale |
| + // about it is out of date. Without this, a pair the hub reset while |
| + // this tab was giving up on it would be rebuilt and then never |
| + // spoken to, because `send` drops for a stale target. |
| + stale.delete(id); |
| const entry = { |
| pc, |
| // §5.1: the peer with the lower id is polite and rolls back on a |
| @@ -1215,6 +1221,14 @@ |
| let wait = BACKOFF_MIN; |
| for (;;) { |
| if (state.stopped || state.peer !== peer) return; |
| + if (Date.now() >= deadline) { |
| + // The budget this reset inherited is spent. One more POST would |
| + // be a request made outside the deadline that was supposed to |
| + // cover it, and the target has to be released either way |
| + // (Codex S2-17): the roster is what settles it now. |
| + staleTarget(to, true, deadline); |
| + return; |
| + } |
| const res = await postSignal({ peer, type: 'reset', to }, { deadline }); |
| if (res.signedOut) { |
| terminal('signedout'); |
| @@ -1236,7 +1250,9 @@ |
| } |
| const ms = res.status === 429 ? retryAfter(res.retryAfter) : wait; |
| wait = Math.min(wait * 2, BACKOFF_MAX); |
| - if (!(await waitWithin(ms, deadline))) return; |
| + // As in the diagnosis: round the loop, and let the check at the |
| + // top be the one place a spent budget is answered. |
| + await waitWithin(ms, deadline); |
| } |
| } |
| @@ -1311,7 +1327,16 @@ |
| // name: try the diagnosis again rather than acting on it. |
| const ms = res.status === 429 ? retryAfter(res.retryAfter) : wait; |
| wait = Math.min(wait * 2, STREAM_BACKOFF_MAX); |
| - if (!(await waitWithin(ms, deadline))) return; |
| + // Deliberately not a `return` when the budget runs out here: the |
| + // loop goes round and the check at the top is the one exit for an |
| + // exhausted diagnosis, so it cannot be reached without releasing |
| + // the target (Codex S2-17). Returning straight from this line left |
| + // the queue intact, the pump's `finally` restarted it, its expired |
| + // head called `unreachable`, and that opened a reset with a fresh |
| + // thirty seconds — the bounded recovery this whole path exists to |
| + // be. `waitWithin` does not sleep when there is nothing left, so |
| + // this is not a spin. |
| + await waitWithin(ms, deadline); |
| } |
| } |
| @@ -1787,6 +1812,9 @@ |
| // Rosters applied, so a gate can tell "this tab closed a |
| // connection" from "the server told this tab who is here". |
| rosters, |
| + // Targets a 404 has marked stale: nothing is sent to them and |
| + // no reset is asked for until a roster settles it. |
| + stale: Array.from(stale), |
| peers: Array.from(pcs.keys()), |
| frames: new Map(frames), |
| signaling, |
diff --git a/test/browser/negotiation.test.mjs b/test/browser/negotiation.test.mjs| index cc84c54..1d25b1c 100644 |
| --- a/test/browser/negotiation.test.mjs |
| +++ b/test/browser/negotiation.test.mjs |
| @@ -162,6 +162,50 @@ test('an offer arriving while an answer is being applied is answered, not discar |
| assert.equal(e.isSettingRemoteAnswerPending, false, 'the flag must be cleared once the answer is applied'); |
| }); |
| +// The recovery ladder, lifted the same way. This one is not about |
| +// negotiation at all — it lives here because this file is where room.js |
| +// can be run without a browser, and because a browser scenario cannot |
| +// see which internal path a client took, only what it did next. |
| +// |
| +// What it pins is §4.6's budget: a diagnosis that runs out of time must |
| +// release its target rather than returning quietly, because a quiet |
| +// return leaves the queue in place, the pump restarts it, its expired |
| +// head calls `unreachable`, and that opens a reset with a fresh thirty |
| +// seconds — recovery restarting the clock it was supposed to be bounded |
| +// by (Codex S2-17). |
| +const recovery = new Function( |
| + 'state', 'probe', 'refused', 'terminal', 'enterAfresh', 'staleTarget', |
| + 'sqid', 'SEND_DEADLINE', 'STREAM_BACKOFF_MAX', 'BACKOFF_MIN', |
| + [lift(src, 'recoverFromNotFound'), lift(src, 'waitWithin'), lift(src, 'sleep'), lift(src, 'retryAfter')].join('\n') + |
| + '\nreturn { recoverFromNotFound };', |
| +); |
| + |
| +test('a diagnosis that runs out of time releases its target and asks for nothing', async () => { |
| + const released = []; |
| + const enteredAfresh = []; |
| + const terminals = []; |
| + const { recoverFromNotFound } = recovery( |
| + { stopped: false, peer: 'mypeer', boot: '' }, |
| + // A probe that never answers, exactly as the AbortController leaves |
| + // it: a rejected promise. |
| + async () => { throw new Error('aborted'); }, |
| + () => false, |
| + (reason) => terminals.push(reason), |
| + () => enteredAfresh.push(true), |
| + (to, fromReset) => released.push([to, fromReset]), |
| + 'room', 30000, 10000, 500, |
| + ); |
| + |
| + // A budget of fifty milliseconds, so the test costs fifty |
| + // milliseconds. The path is the same one thirty seconds takes. |
| + await recoverFromNotFound('', 'target', { deadline: Date.now() + 50 }); |
| + |
| + assert.deepEqual(released, [['target', true]], |
| + 'the target must be released exactly once, and never with a reset behind it'); |
| + assert.deepEqual(enteredAfresh, [], 'nothing here says this participation ended'); |
| + assert.deepEqual(terminals, [], 'nor that the call did'); |
| +}); |
| + |
| // The case above cannot prove the answer handler clears the flag on its |
| // own, because onOffer clears it too (round-3 note). So: an answer with |
| // no offer behind it. |