From 7daf3b782d3a29e38667d81a57e62bbfd7511994 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Fri, 24 Jul 2026 17:16:35 -0700 Subject: [PATCH] fix(#900): stream generation closes the reconnect-inside-the-await render; jitter both retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review. The render-time cursor-safety gate was point-in-time: a transport that dropped AND finished re-establishing inside the /history await reads back OPEN and is indistinguishable from one that never moved. It is not — the redial re-presented the frozen cursor, the server answered replay_ok, and the quiesce buffered that slice, so the render commits rows the flush then repaints on top. Object identity cannot see it either, since a native reconnect reuses the same EventSource; only a counter can. _connectEpoch is bumped in onopen and nowhere else. Native auto-reconnect calls neither connectSSE nor disconnectSSE, so those two are blind to the exact case this exists for; connectSSE would also false-bump on its document.hidden early return, which establishes no stream; and a closed source can never fire a late open. Captured at dispatch, required unchanged before a seedless render commits. This is original-strata residual, not a regression this branch introduced: before #900 the render was ungated entirely. The branch closed the fire-time half and the still-down cases; these are the drop-and-recover ones that were always open. Two rulings written in at the gate, since neither is closed: a fresh/truncated reconnect inside the await declines a render that would have been safe (one wasted /history, self-healing via the flushed synthetic state_change), and a refetch dispatched between onopen and the replay slice arriving still renders past the frozen cursor — replay_ok emits no end-of-replay marker, so no client-side signal exists (#903). Coord's half of the same gate is #904; its exposure is a race rather than this determinism, so it is not ported blind. Also corrected: the claim that the idle-edge backstop's stream is live by construction. It isn't — handleEvent also runs from the quiesce flush, so a queued idle edge reaches the backstop with the transport down. The render-time gate is what covers it. The clear_ui retry gains additive jitter in BOTH clients from one shared constant: a declined render now leaves the latch set, so a successful fetch can arm the retry, and the decline trigger is herd-shaped. Kept small deliberately — the spread works against #884's single-flight, which coalesces a lockstep herd. test_coordinator_page.py anchored the fire guard on a literal `}, 2000);` and on exact indentation; both would have ERRORED rather than failed once the delay became an expression. --- tests/test_coordinator_page.py | 18 +- tests/test_interactive_pane_js.py | 63 +++++- tests/test_sse_overflow_js.py | 9 +- .../console/static/coordinator/coordinator.js | 68 +++--- turnstone/shared_static/interactive.js | 194 +++++++++++++----- turnstone/shared_static/sse_overflow.js | 16 ++ 6 files changed, 278 insertions(+), 90 deletions(-) diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index 599b4c5d..75443823 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -716,7 +716,11 @@ def test_coordinator_history_stale_latch_contract(): "exists to kill." ) retry_arm = body.index("staleRetryTimer = setTimeout") - retry_fire = _strip_comments(body[retry_arm : body.index("}, 2000);", retry_arm)]) + # Anchor on the delay EXPRESSION's opening, not on a literal `}, 2000);` + # — #900 made the delay `2000 + Math.random() * STALE_RETRY_JITTER_MS`, + # and a literal anchor raises ValueError (the suite ERRORS instead of + # failing) the moment the spread changes. + retry_fire = _strip_comments(body[retry_arm : body.index("STALE_RETRY_JITTER_MS", retry_arm)]) assert "!refetchesInFlight" in retry_fire, ( "the retry's fire guard must yield to an in-flight refetch " "(mirrors interactive's !_replayQueue pin)." @@ -732,11 +736,13 @@ def test_coordinator_history_stale_latch_contract(): "render-time gate would discard (the chokepoint carries the " "correctness; these are the efficiency layer — keep them)." ) - assert ( - "visHandler &&\n evtSource &&\n" - " evtSource.readyState === EventSource.OPEN\n" - " ) {" in body - ), ( + # Whitespace-normalised, not indentation-exact: #900 made the delay an + # expression, which reflowed setTimeout to prettier's multi-line call form + # and re-indented this whole callback body. Still an ORDERED-tail pin, so + # a mere comment mention cannot satisfy it. + cond_start = body.index("if (", retry_arm) + guard = " ".join(body[cond_start : body.index(") {", cond_start)].split()) + assert guard.endswith("visHandler && evtSource && evtSource.readyState === EventSource.OPEN"), ( "the retry's fire guard must require an OPEN stream — not handle " "existence: CONNECTING keeps the handle with a frozen cursor and " "a pending replay, and a seedless heal then double-renders when " diff --git a/tests/test_interactive_pane_js.py b/tests/test_interactive_pane_js.py index 711c0d2f..547201f8 100644 --- a/tests/test_interactive_pane_js.py +++ b/tests/test_interactive_pane_js.py @@ -387,13 +387,72 @@ def test_interactive_refetch_failure_preserves_the_pane() -> None: ref = body.index("async _refetchHistory(wsId, token, seedCursor = false) {") ref_seg = body[ref : body.index("\n _beginReplayQuiesce(token) {", ref)] gate = ref_seg.index("const cursorSafe =") - assert "seedCursor ||" in ref_seg[gate : gate + 200], ( + gate_seg = ref_seg[gate : ref_seg.index(";", gate)] + assert "seedCursor ||" in gate_seg, ( "the render-time gate must exempt seeded (disconnect-first) loads (#900)" ) - assert "this.evtSource.readyState === EventSource.OPEN" in ref_seg[gate : gate + 200] + assert "this.evtSource.readyState === EventSource.OPEN" in gate_seg + # Connection-generation term (#900 r2): readyState alone cannot see a + # transport that DROPPED and finished RE-ESTABLISHING inside the await — + # it reads OPEN either way, while the redial re-presented the frozen + # cursor and the quiesce buffered the replay the render would duplicate. + # Object identity is not a substitute: a NATIVE reconnect reuses the same + # EventSource, so only a counter can see it. + assert "this._connectEpoch === epoch" in gate_seg, ( + "the seedless render must require an unchanged stream generation (#900)" + ) assert "if (data && cursorSafe) {" in ref_seg, ( "the seedless render must be gated on a live cursor (#900)" ) + # Captured at DISPATCH, not read live at render time — a live read would + # compare the epoch against itself and the term would be vacuous. + assert "const epoch = this._connectEpoch;" in ref_seg, ( + "the gate must compare against the generation captured at dispatch (#900)" + ) + assert ref_seg.index("const epoch = this._connectEpoch;") < ref_seg.index("await authFetch("), ( + "the generation must be captured BEFORE the fetch await (#900)" + ) + + # The bump belongs to onopen and NOWHERE else. A native auto-reconnect + # calls neither connectSSE nor disconnectSSE, so those two are blind to + # the very case the term exists for; connectSSE would additionally + # FALSE-bump on its document.hidden early return, which establishes no + # stream. Absence from both is therefore load-bearing, not incidental. + assert "this._connectEpoch = 0;" in body, ( + "the generation must be initialised — undefined makes every compare " + "NaN-false and silently declines every seedless render (#900)" + ) + onopen = body.index("this.evtSource.onopen = () => {") + onopen_seg = body[onopen : body.index("\n };", onopen)] + assert "this._connectEpoch += 1;" in onopen_seg, ( + "the stream generation must be bumped in onopen — the only site that " + "fires for a NATIVE auto-reconnect (#900)" + ) + conn = body.index("connectSSE(wsId) {") + assert "_connectEpoch" not in _strip_comments(body[conn:onopen]), ( + "connectSSE must NOT bump the generation — it would false-bump on the " + "document.hidden early return, which establishes no stream (#900)" + ) + dis = re.search(r"\n disconnectSSE\(\) \{(.*?)\n \}\n", body, re.S) + assert dis is not None, "disconnectSSE not found" + # Comment-stripped: the method's inventory comment NAMES both fields as + # deliberately-not-cleared, which is the ruling — assert on code only. + dis_code = _strip_comments(dis.group(1)) + assert "_connectEpoch" not in dis_code, ( + "disconnectSSE must NOT bump the generation — a teardown is decided " + "by the presence term, and a re-establish by the next onopen (#900)" + ) + # THE load-bearing negative: disconnectSSE deliberately does NOT cancel + # the clear_ui retry (transport-only redials keep the pending heal + # intent). That is exactly why the retry can fire against a dead + # transport, which is what its OPEN fire-guard term exists to handle — + # so a "symmetry" cleanup adding the cancel here would silently make + # that guard unreachable and E5's detector vacuous, with nothing else + # failing. Coord pins the same invariant (test_coordinator_page.py). + assert "_staleRetryTimer" not in dis_code, ( + "disconnectSSE must NOT cancel the clear_ui failure retry — " + "transport-only reconnects keep the pending heal intent (#890/#900)" + ) # Failure branch: quiesce release only. fail = body.index("Failed fetch = DOM + ref + repair-intent no-op") diff --git a/tests/test_sse_overflow_js.py b/tests/test_sse_overflow_js.py index ec093bd2..099540e5 100644 --- a/tests/test_sse_overflow_js.py +++ b/tests/test_sse_overflow_js.py @@ -29,7 +29,7 @@ _SSE_OVERFLOW = _ROOT / "turnstone/shared_static/sse_overflow.js" def test_module_exports_constants_and_pure_helpers() -> None: - """The single source of truth exports the five tuning constants and the two + """The single source of truth exports the seven tuning constants and the two pure helpers. Both panes import these by name (pinned in their own suites), so a rename here is a breaking change that must surface loudly.""" body = _SSE_OVERFLOW.read_text(encoding="utf-8") @@ -39,6 +39,13 @@ def test_module_exports_constants_and_pure_helpers() -> None: ("DEGRADED_COOLDOWN_BASE_MS", "15000"), ("DEGRADED_COOLDOWN_MAX_MS", "120000"), ("DEGRADED_COOLDOWN_RESET_MS", "300000"), + ("TRUNCATED_RESYNC_JITTER_MS", "10000"), + # ADDITIVE over the retry's 2000 floor, and deliberately NOT the + # truncated-resync spread: this one works against #884's /history + # single-flight, so it stays small enough to sit inside a typical + # flight. Both e2e non-occurrence detectors size their windows on + # the floor plus this value. + ("STALE_RETRY_JITTER_MS", "500"), ): assert f"export const {const} = {value};" in body, f"missing export const {const}" assert "export function overflowWindowTripped(" in body diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index acee1b55..a56d884d 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -57,6 +57,7 @@ import { DEGRADED_COOLDOWN_MAX_MS, DEGRADED_COOLDOWN_RESET_MS, TRUNCATED_RESYNC_JITTER_MS, + STALE_RETRY_JITTER_MS, overflowWindowTripped, degradedCooldownStep, } from "/shared/sse_overflow.js"; @@ -3770,35 +3771,43 @@ function createCoordinatorPane(root, wsId, opts) { // pane UI, so the user's committed edit still delivers. if (historyStale && visHandler) { if (staleRetryTimer) clearTimeout(staleRetryTimer); - staleRetryTimer = setTimeout(() => { - staleRetryTimer = null; - if ( - historyStale && - !refetchesInFlight && - !busy && - !currentAssistantEl && - !currentReasoningEl && - visHandler && - evtSource && - evtSource.readyState === EventSource.OPEN - ) { - // Stream must be OPEN, not merely present: close-on-hide - // keeps this timer armed by design (the fire can land - // with the transport down), and a CONNECTING source has - // a frozen cursor with a pending replay — a seedless - // fetch then would render past it (double-render when - // the replay lands). Skip instead; the latch survives - // and the next organic settle re-fires the backstop. - // The backstop itself needs no such term: it runs - // inside SSE dispatch, so its stream is live by - // construction. refetchHistory's render-time gate - // re-checks every invariant across the await window. - // Fire-and-forget, seedless (live stream — lastEventId - // must not rewind); a render throw stays loud, as on the - // backstop. - refetchHistory(); - } - }, 2000); + staleRetryTimer = setTimeout( + () => { + staleRetryTimer = null; + if ( + historyStale && + !refetchesInFlight && + !busy && + !currentAssistantEl && + !currentReasoningEl && + visHandler && + evtSource && + evtSource.readyState === EventSource.OPEN + ) { + // Stream must be OPEN, not merely present: close-on-hide + // keeps this timer armed by design (the fire can land + // with the transport down), and a CONNECTING source has + // a frozen cursor with a pending replay — a seedless + // fetch then would render past it (double-render when + // the replay lands). Skip instead; the latch survives + // and the next organic settle re-fires the backstop. + // The backstop itself needs no such term: it runs + // inside SSE dispatch, so its stream is live by + // construction. refetchHistory's render-time gate + // re-checks every invariant across the await window. + // Fire-and-forget, seedless (live stream — lastEventId + // must not rewind); a render throw stays loud, as on the + // backstop. + refetchHistory(); + } + // ADDITIVE spread over the 2000 floor: one clear_ui reaches + // every listener on the ws, so an un-spread retry re-fetches + // in lockstep across tabs. The floor is load-bearing (the + // e2e non-occurrence windows size on it) — jitter up, never + // down. Mirrored in interactive.js; the constant is shared. + }, + 2000 + Math.random() * STALE_RETRY_JITTER_MS, + ); } if (!_pendingEditSend) return; const editText = _pendingEditSend; @@ -5998,7 +6007,6 @@ function createCoordinatorPane(root, wsId, opts) { if (staleRetryTimer) { clearTimeout(staleRetryTimer); staleRetryTimer = null; - } toolRows.clear(); activeBatch = null; diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js index cdcb5ec6..95686947 100644 --- a/turnstone/shared_static/interactive.js +++ b/turnstone/shared_static/interactive.js @@ -63,6 +63,7 @@ import { DEGRADED_COOLDOWN_MAX_MS, DEGRADED_COOLDOWN_RESET_MS, TRUNCATED_RESYNC_JITTER_MS, + STALE_RETRY_JITTER_MS, overflowWindowTripped, degradedCooldownStep, } from "./sse_overflow.js"; @@ -262,6 +263,14 @@ class Pane { this.projectName = ""; this._lastStatusEvt = null; this._historyLoadToken = 0; + // Monotonic STREAM-generation counter (#900), bumped only in + // evtSource.onopen. _refetchHistory captures it at dispatch and its + // render-time gate requires it unchanged, so a transport that dropped + // and re-established across a seedless fetch's await cannot be mistaken + // for one that never moved — readyState reads OPEN in both cases. + // Must stay initialised: undefined would make every compare NaN-false + // and silently decline every seedless render for the life of the pane. + this._connectEpoch = 0; // Event backlog while a clear_ui rebuild is in flight — see // _beginReplayQuiesce. {token, events[]} or null. (The truncated // resync no longer quiesces: it tears the stream down first, so no @@ -484,8 +493,16 @@ class Pane { this.evtSource.close(); this.evtSource = null; } - // Deliberately NOT cleared here: _agentCards/_agentOrphans and any armed - // _replayQueue. disconnectSSE also runs for transport-only reconnects + // Deliberately NOT cleared here: _agentCards/_agentOrphans, any armed + // _replayQueue, and _staleRetryTimer. The retry's survival is + // load-bearing, not an oversight: it is a REST heal, not transport + // state, so a transport-only redial must keep the pending repair intent + // — and it is precisely why that timer can fire against a dead stream, + // which is what its OPEN fire-guard term exists to handle. Cancelling + // it here would silently make that guard unreachable. Terminal-only + // cancel (destroy/giveUp); pinned negatively in + // tests/test_interactive_pane_js.py. disconnectSSE also runs for + // transport-only reconnects // (connectSSE's first line, the host's 5s recovery beat) where the DOM // survives — wiping the card map there made the next child event build a // DUPLICATE agent card beside the still-attached one, and cancelling @@ -1426,6 +1443,20 @@ class Pane { this.evtSource = new EventSource(evtUrl); this.evtSource.onopen = () => { + // Connection generation (#900) — bumped HERE and nowhere else, because + // "a stream generation began" is the only thing a cursor can be + // anchored to. onopen is the sole site that means that: per spec the + // announce-the-connection step fires `open` for EVERY established + // connection, including the ones the browser's own reestablish loop + // produces — and a NATIVE auto-reconnect calls neither connectSSE nor + // disconnectSSE (see onerror below), so those two are blind to the + // case this exists for. connectSSE would also FALSE-bump on the + // document.hidden early return above, which establishes nothing. And + // a close()d source can never fire a late open (the spec's queued task + // early-returns on readyState CLOSED), so a discarded stream cannot + // bump a generation the pane has moved past. Read once, by + // _refetchHistory's render-time gate. + this._connectEpoch += 1; this.retryDelay = 1000; this.statusBarEl.classList.remove("ws-sb-disconnected"); if (this._lastStatusEvt) this.updateStatus(this._lastStatusEvt); @@ -1778,14 +1809,27 @@ class Pane { // transient reconnect, or — on a re-render that trims an orphan // with no reconnect — strand the omitted turn). const id = wsId || this.wsId; + // Stream generation at DISPATCH (#900), captured at entry rather than + // beside the await — it mirrors coordinator.js refetchHistory's + // `const seq = ++refetchSeq;`, and entry-capture can only ever decline + // MORE if a future edit inserts transport-touching work ahead of the + // fetch, which is the safe direction. + const epoch = this._connectEpoch; let data = null; // Deliberately unbounded and unabortable, deferred not overlooked - // (#900): coordinator.js carries an AbortController set plus a 15s - // bound so destroy() can cut a slow /history loose. Here the load - // token already makes a post-teardown settle render-inert, so the - // residual is a detached closure pinned for the request's life — a - // resource cost, not a correctness defect. It belongs with the - // shared recovery core rather than a third hand-port. + // (#900, tracked as #905): coordinator.js carries an AbortController + // set plus a 15s bound so destroy() can cut a slow /history loose. + // Here the load token already makes a post-teardown settle + // render-inert, so the residual is a resource cost, not a correctness + // defect. The honest bound is wider than "one request": authFetch + // retries up to three attempts, sleeping Retry-After on 429 and doing + // a refresh round-trip on 401, so the detached pane's closure stays + // reachable for all of it — and that same unbounded await is what + // makes the slow transport-bounce cases (recover beat ~5s, degraded + // timer 15-120s) reachable by the epoch gate below. REOPEN when a + // /history blocks long enough for the pin to matter (large-session + // resume), or with the shared recovery core, which should own one + // implementation rather than a third hand-port. try { const r = await authFetch( this._base + @@ -1837,9 +1881,40 @@ class Pane { // and a DOM short one user row can only UNDER-count a later rewind — // non-destructive, the same ruling the Route-L reload carries at // _historyStale's declaration. + // The generation term is what makes this gate total rather than + // point-in-time (#900 r2). A transport that DROPPED and finished + // RE-ESTABLISHING inside the await — natively, or via a hide/show, the + // recover beat, or the degraded timer — reads back `OPEN` and is + // indistinguishable from one that never moved. It is not: the redial + // presented the frozen _lastEventId, the server answered replay_ok, and + // the quiesce BUFFERED that slice, so rendering here would commit the + // same turns the flush is about to repaint on top. Only a counter can + // see it — object identity cannot, since a native reconnect reuses the + // same EventSource. (Do NOT cite coordinator.js's REMOVED r8 epoch as + // precedent: that one was a clear_ui/truncation-generation stamp made + // redundant by its seq gate. This is a connection generation — a + // different value with a different matrix.) + // + // KNOWN FALSE-DECLINE, accepted: if the reconnect answered fresh or + // truncated rather than replay_ok, its slice carries no ring content and + // the render would have been safe. Cost is one wasted /history, and it + // self-heals — the flushed synthetic state_change lands in the idle-edge + // backstop, or the truncated handler schedules a seeded resync. + // + // (Coord's half of this gate is tracked as #904 — its exposure is a + // race rather than this determinism, so it is not ported blind.) + // + // KNOWN RESIDUAL, server-side-blocked (#903): a refetch DISPATCHED in + // the gap between onopen and the replay slice actually arriving captures + // an already-bumped generation and still renders past the frozen cursor. + // replay_ok emits no end-of-replay marker, so no client-side signal + // exists to gate on; closing it needs a server change. Narrow (the + // replay almost always beats a fresh HTTP round-trip) but real. const cursorSafe = seedCursor || - (!!this.evtSource && this.evtSource.readyState === EventSource.OPEN); + (!!this.evtSource && + this.evtSource.readyState === EventSource.OPEN && + this._connectEpoch === epoch); if (data && cursorSafe) { // Fresh-connect fast-forward: when the trailing turn is an // executing in-flight tool batch the server can replay, /history @@ -2447,48 +2522,65 @@ class Pane { // case). if (this._historyStale && token === this._historyLoadToken) { if (this._staleRetryTimer) clearTimeout(this._staleRetryTimer); - this._staleRetryTimer = setTimeout(() => { - this._staleRetryTimer = null; - if ( - token === this._historyLoadToken && - this._historyStale && - !this._replayQueue && - !this.busy && - !this.currentAssistantEl && - !this.currentReasoningEl && - this.evtSource && - this.evtSource.readyState === EventSource.OPEN - ) { - // !_replayQueue: yield to an in-flight quiesced - // fetch (the idle-edge backstop shares this token) - // instead of stomping its queue for a same-token - // double-render. Fire-and-forget like the backstop: - // no composer state to strand here, so a render throw is - // left loud/uncaught (see the backstop's note). - // - // The stream must be OPEN, not merely present (#900): - // disconnectSSE deliberately keeps this timer armed, so - // the fire can land with the transport down — hidden tab - // (close-on-hide), degraded catch-up cooldown, or a - // CLOSED source — and a CONNECTING one has a frozen - // cursor with a replay pending. A seedless refetch then - // paints rows the frozen _lastEventId still sits BELOW, - // and the next connect replays that slice on top (see - // _refetchHistory's render-time gate for the full - // trace). Skipping keeps the latch, so the idle-edge - // backstop heals at the next settle — the accepted - // liveness lag already ruled there. NOTE the polarity - // is deliberately the OPPOSITE of the host recover - // beat's `readyState !== CLOSED`: that one asks "is - // native reconnect still working the problem" (don't - // stomp it), this one asks "is the cursor live" (a - // reconnecting stream's is not). The backstop needs no - // such term — it runs inside SSE dispatch, so its - // stream is live by construction. - this._beginReplayQuiesce(token); - this._refetchHistory(this.wsId, token); - } - }, 2000); + this._staleRetryTimer = setTimeout( + () => { + this._staleRetryTimer = null; + if ( + token === this._historyLoadToken && + this._historyStale && + !this._replayQueue && + !this.busy && + !this.currentAssistantEl && + !this.currentReasoningEl && + this.evtSource && + this.evtSource.readyState === EventSource.OPEN + ) { + // !_replayQueue: yield to an in-flight quiesced + // fetch (the idle-edge backstop shares this token) + // instead of stomping its queue for a same-token + // double-render. Fire-and-forget like the backstop: + // no composer state to strand here, so a render throw is + // left loud/uncaught (see the backstop's note). + // + // The stream must be OPEN, not merely present (#900): + // disconnectSSE deliberately keeps this timer armed, so + // the fire can land with the transport down — hidden tab + // (close-on-hide), degraded catch-up cooldown, or a + // CLOSED source — and a CONNECTING one has a frozen + // cursor with a replay pending. A seedless refetch then + // paints rows the frozen _lastEventId still sits BELOW, + // and the next connect replays that slice on top (see + // _refetchHistory's render-time gate for the full + // trace). Skipping keeps the latch, so the idle-edge + // backstop heals at the next settle — the accepted + // liveness lag already ruled there. NOTE the polarity + // is deliberately the OPPOSITE of the host recover + // beat's `readyState !== CLOSED`: that one asks "is + // native reconnect still working the problem" (don't + // stomp it), this one asks "is the cursor live" (a + // reconnecting stream's is not). The idle-edge backstop + // needs no such term, but NOT because its stream is live + // by construction — it isn't. handleEvent also runs from + // _endReplayQuiesce's flush, which fires AFTER a seedless + // fetch settles, so a state_change:idle queued during that + // fetch reaches the backstop with the transport already + // down. What covers it is _refetchHistory's render-time + // gate: the backstop's refetch is seedless, so the gate + // declines it and the cost is one wasted /history, never a + // double-render. Add a fire-time term there only if a + // backstop refetch is ever dispatched somewhere the + // render-time gate does not cover. + this._beginReplayQuiesce(token); + this._refetchHistory(this.wsId, token); + } + // ADDITIVE spread over the 2000 floor: one clear_ui reaches + // every listener on the ws, so an un-spread retry re-fetches + // in lockstep across tabs. The floor is load-bearing (the + // e2e non-occurrence windows size on it) — jitter up, never + // down. Mirrored in coordinator.js; the constant is shared. + }, + 2000 + Math.random() * STALE_RETRY_JITTER_MS, + ); } if (token !== this._historyLoadToken && this.wsId !== editWs) { // Cross-ws supersession: drop the pending edit + release diff --git a/turnstone/shared_static/sse_overflow.js b/turnstone/shared_static/sse_overflow.js index 58c6b021..7cf09ab6 100644 --- a/turnstone/shared_static/sse_overflow.js +++ b/turnstone/shared_static/sse_overflow.js @@ -45,6 +45,22 @@ export const DEGRADED_COOLDOWN_RESET_MS = 300000; // total per-restart fetch count. export const TRUNCATED_RESYNC_JITTER_MS = 10000; +// Spread for the clear_ui staleness retry, ADDITIVE over a 2000 ms floor +// (`2000 + Math.random() * this`). One clear_ui fans out to every listener +// on the workstream, so an un-spread retry makes N tabs re-fetch /history in +// near-lockstep — and #900 widened that arm: a render the cursor-safety gate +// declines now leaves the latch set, so a SUCCESSFUL fetch can arm the retry +// too, and the decline trigger (transport down) is itself herd-shaped. +// +// Deliberately small, and deliberately NOT TRUNCATED_RESYNC_JITTER_MS: this +// spread works AGAINST #884's `(ws_id, limit, generation)` single-flight, +// which coalesces a lockstep herd into one reconstruction. Spreading past a +// typical flight duration de-coalesces it — lower peak, higher total. 500 ms +// keeps the window comparable to a flight while still breaking lockstep. +// The 2000 floor is load-bearing for the e2e non-occurrence detectors (they +// size their windows on it); raising this constant requires widening those. +export const STALE_RETRY_JITTER_MS = 500; + // Rolling-window trip check. Prunes `times` in place (entries older than // windowMs against nowMs) and reports whether count-or-more remain. A // standalone pure function so the trip logic can be lifted verbatim into a