ewilliams28/turnstone
1
0
Fork 0
mirror of https://github.com/turnstonelabs/turnstone.git synced 2026-08-12 23:12:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(#900): close two vacuity holes the round-2 scenarios left open

Browse Source 
Round-3 review, unprimed. Two of its majors were the new scenarios
asserting things they did not prove — the false-detector class this
campaign keeps returning to.

E8 never checked that the held /history was still OUTSTANDING when the
redial completed. The disconnect/send/wait_turn/redial sequence is
unbounded (wait_turn alone allows 45s), so on a slow box the payload
resolves while evtSource is still null, the PRESENCE term declines it, and
the run stamps dupes1-healed1 without ever evaluating the generation term.
It now fails loudly with the counter values instead. E7 gained the same
positive proof its siblings already carried: sse_opens == 0 only means
"nothing connected in 8s", which is not the same as "the held load
settled and its .finally chose not to reconnect".

E5's stated control was simply wrong, in three places. A hide nulls
evtSource, and connectSSE early-returns while hidden, so the scenario
cannot produce the non-null-but-not-OPEN source that readyState === OPEN
exists for — it exercises the presence term only. The earlier control
removed both terms at once, which is what disguised it. The readyState
half is covered by reasoning plus coord parity, and its correctness twin
IS covered through the render-time gate by E6/E8; that scope is now
written down rather than overclaimed. Coord's G5 has the same shape.

The retry floor becomes a shared export beside its jitter: four sites must
move together (both clients' arms, both non-occurrence windows) and it was
the only one of them with no single source of truth. Interactive's use of
the expression had no pin at all — reverting it to a bare 2000 would have
broken cross-client parity with the suite green. Coord's re-anchor still
raised ValueError rather than failing on a named assertion, and its first
replacement used a fixed window that truncated mid-expression.
This commit is contained in:
Patrick Buckley
2026-07-24 17:52:54 -07:00
parent 7ed5d90a98
commit 4007fab855
7 changed files with 89 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/recovery_e2e.py
+51 -12
View File
@@ -113,8 +113,10 @@ repaints every turn the hidden render just committed (only ``system_turn``
and compaction markers carry id dedup; content and tool rows do not). The
detector is a NON-OCCURRENCE counted at the fault layer: ``history_requests``
UNCHANGED across the hidden window (``hidden0``), which regresses to
``hidden1`` the moment the ``evtSource.readyState === OPEN`` term is
removed. A replay_ok reconnect carries no synthetic ``state_change``, so
``hidden1`` the moment the guard's transport clause is removed. Note the
scope: a hide nulls ``evtSource``, so this exercises the PRESENCE term only —
the ``readyState`` half needs a redial in progress, which no fault primitive
produces deterministically (see the runner's docstring). A replay_ok reconnect carries no synthetic ``state_change``, so
the latch survives ``__show`` — the accepted liveness lag — and the heal
rides a plain send's ORGANIC settle into the transport-free backstop, hence
exactly ONE new SSE open across show + heal. Stamps
@@ -605,8 +607,6 @@ PAGE_HTML = r"""<!doctype html>
if (ctl) ctl.connect();
else pane._loadHistoryThenConnect(wsId);
// Shared by the rewind scenarios (E2/E3): click the REAL rewind
// button on the idx-th user row. Depends only on `pane`.
// Count a sentinel's OCCURRENCES in the transcript text. Every other
// detector in this harness counts `.msg.user` rows, and user rows are
// never emitted on the SSE stream — so none of them can observe a turn
@@ -633,6 +633,8 @@ PAGE_HTML = r"""<!doctype html>
pane.connectSSE(pane.wsId);
};
// Shared by the rewind scenarios (E2/E3): click the REAL rewind
// button on the idx-th user row. Depends only on `pane`.
window.__clickRewind = function (idx) {
const rows = pane.messagesEl.querySelectorAll(".msg.user");
const row = rows[idx];
@@ -923,7 +925,9 @@ PAGE_HTML = r"""<!doctype html>
// and tool rows carry no id dedup). The fire guard's
// ``evtSource.readyState === OPEN`` term skips the hidden firing
// instead: hiddenDelta 0 is the NON-OCCURRENCE detector, and it
// regresses to 1 the moment the term is removed.
// regresses to 1 the moment the transport clause is removed. Scope:
// a hide nulls evtSource, so this reaches the PRESENCE term only —
// not the readyState half, which needs a redial in progress.
//
// A replay_ok reconnect carries no synthetic state_change (only
// fresh/truncated replays do), so the latch stays closed across
@@ -2821,8 +2825,19 @@ def run_hidden_retry(chrome: str) -> str:
THE DETECTOR is a NON-OCCURRENCE, counted at the fault layer:
``history_requests`` must be UNCHANGED across the whole hidden window.
Remove the ``evtSource.readyState === OPEN`` term and the hidden fetch
lands, stamping hidden1 — the scenario's negative control.
Remove the transport clause from the fire guard and the hidden fetch lands,
stamping hidden1 — the scenario's negative control.
SCOPE, stated precisely (do not overclaim it): a hide nulls ``evtSource``
outright, and ``connectSSE`` early-returns while ``document.hidden``, so
this scenario can only ever exercise the guard's PRESENCE term
(``this.evtSource &&``). It structurally cannot produce the non-null,
not-OPEN source the ``readyState === OPEN`` term exists for — that state
needs a native redial in progress, which no fault primitive here produces
deterministically. The readyState half is therefore covered by REASONING
plus coord parity, not by this scenario; its correctness twin IS covered,
by E6/E8 through ``_refetchHistory``'s render-time gate. Same true of
coord's G5.
A replay_ok reconnect carries NO synthetic ``state_change`` (only
fresh/truncated replays do), so the latch stays closed across ``__show``:
@@ -2865,9 +2880,10 @@ def run_hidden_retry(chrome: str) -> str:
if not _poll_until(lambda: cdp.evaluate("window.__pane.evtSource === null"), 5, 0.05):
raise AssertionError("hidden-retry: close-on-hide never dropped the transport")
hidden_baseline = node.history_requests
# NON-occurrence window: the retry fires at ~2s post-failure, so give
# it 3.5s. Without the guard this poll returns True (the hidden fetch
# lands) and hidden_delta stamps 1.
# NON-occurrence window: the retry fires at STALE_RETRY_BASE_MS plus up
# to STALE_RETRY_JITTER_MS, so the window must outlast floor+ceiling.
# Without the guard this poll returns True (the hidden fetch lands) and
# hidden_delta stamps 1.
# Window = the 2000 ms floor + the jitter ceiling + slack. The
# retry's delay is `2000 + rand*STALE_RETRY_JITTER_MS` (#900), so a
# window sized on the floor alone would close BEFORE a
@@ -3125,6 +3141,18 @@ def run_reconnect_in_await(chrome: str) -> str:
0.05,
):
raise AssertionError("reconnect-in-await: the redial never reached OPEN")
# NON-VACUITY, the leg this scenario turns on: the held fetch must
# still be OUTSTANDING right now. If it already resolved — a slow box
# can push the disconnect/send/wait_turn/redial sequence past the hold
# — the payload landed while evtSource was still null, the PRESENCE
# term declined it, and a green verdict would never have touched the
# generation term at all.
if node.history_ok != ok_baseline:
raise AssertionError(
"reconnect-in-await: the held /history resolved BEFORE the "
f"redial (history_ok={node.history_ok}, baseline={ok_baseline}) "
"— the generation term was never exercised; raise the hold"
)
# Release the knob for later arrivals; the held fetch serves out its
# own 6000 ms regardless, which is what the polls below outlast.
node.delay_history(0)
@@ -3186,6 +3214,7 @@ def run_destroy_invalidation(chrome: str) -> str:
# Hold the FIRST /history — the one connect() dispatches — so the
# teardown lands with the load genuinely outstanding.
node.delay_history(4000)
ok_baseline = node.history_ok
url = f"{node.base_url}/recovery?ws_id={ws_id}&scenario=destroy-invalidation"
_set_cookie_and_navigate(cdp, node.base_url, node.token, url)
# history_requests counts on ARRIVAL, before the hold sleeps.
@@ -3210,6 +3239,15 @@ def run_destroy_invalidation(chrome: str) -> str:
# raising that deadline would make this detector vacuous.
node.delay_history(0)
_poll_until(lambda: node.events_requests != 0, 8, 0.1)
# sse_opens == 0 only means "nothing connected in 8s". Prove the held
# load actually SETTLED inside that window, or the .finally under test
# never ran and the non-occurrence is measuring nothing.
if node.history_ok < ok_baseline + 1:
raise AssertionError(
"destroy-invalidation: the held /history never resolved inside "
f"the observation window (history_ok={node.history_ok}) — the "
".finally under test never ran"
)
sse_opens = node.events_requests
vis_null = cdp.evaluate("window.__pane._visHandler === null")
print(f" destroy-invalidation sse_opens={sse_opens} vis_null={vis_null}")
@@ -3725,8 +3763,9 @@ def run_coord_hidden_retry(chrome: str) -> str:
# transport down but deliberately leaves the retry timer armed.
cdp.evaluate("window.__hide && window.__hide()")
hidden_baseline = node.history_requests
# NON-occurrence window: the retry fires at ~2s post-failure; give
# it 3.5s. Without the evtSource guard this poll returns True
# NON-occurrence window: the retry fires at STALE_RETRY_BASE_MS plus up
# to STALE_RETRY_JITTER_MS, so the window must outlast floor+ceiling.
# Without the evtSource guard this poll returns True
# (the hidden fetch lands) and hiddenDelta stamps 1.
# Window = the 2000 ms floor + the jitter ceiling + slack. The
# retry's delay is `2000 + rand*STALE_RETRY_JITTER_MS` (#900), so a
tests/test_coordinator_page.py
+14 -4
View File
@@ -716,10 +716,20 @@ def test_coordinator_history_stale_latch_contract():
"exists to kill."
)
retry_arm = body.index("staleRetryTimer = setTimeout")
# 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.
# #900 made the delay an expression over two SHARED constants, so the old
# literal `}, 2000);` anchor is gone. Assert the new anchor EXISTS before
# slicing on it: `.index` would raise ValueError and the suite would ERROR
# with an anonymous traceback rather than fail on a named assertion, which
# is the exact failure mode this re-anchor exists to remove. Coord and
# interactive must carry the identical expression or the herd-spread
# invariant silently forks.
# Unwindowed on purpose: the expression occurs exactly once in coord, so a
# fixed slice buys nothing and can truncate mid-expression (it did).
# Locality is established by the retry_fire slice below, which starts at
# the arm — this assertion only has to prove the anchor exists at all.
assert "STALE_RETRY_BASE_MS + Math.random() * STALE_RETRY_JITTER_MS" in body, (
"the coord retry must keep the shared floor + jitter (#900)"
)
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 "
tests/test_interactive_pane_js.py
+8
View File
@@ -555,6 +555,14 @@ def test_interactive_refetch_failure_preserves_the_pane() -> None:
assert "this.evtSource.readyState === EventSource.OPEN" in cl_seg[retry:], (
"the clear_ui retry must require a live stream at fire time (#900)"
)
# The delay is floor + spread, both from the SHARED module: one clear_ui
# reaches every listener on the ws, so an un-spread retry re-fetches in
# lockstep across tabs, and the floor is what the e2e non-occurrence
# windows size themselves on. Coord carries the identical expression —
# this pin is what keeps the two from drifting.
assert "STALE_RETRY_BASE_MS + Math.random() * STALE_RETRY_JITTER_MS" in cl_seg[retry:], (
"the clear_ui retry must keep the shared floor + jitter (#900)"
)
# Terminal teardown must invalidate in-flight loads AND cancel the
# failure retry. The token bump (#900) is the chokepoint: without it
tests/test_sse_overflow_js.py
+2 -1
View File
@@ -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 seven tuning constants and the two
"""The single source of truth exports the eight 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")
@@ -46,6 +46,7 @@ def test_module_exports_constants_and_pure_helpers() -> None:
# flight. Both e2e non-occurrence detectors size their windows on
# the floor plus this value.
("STALE_RETRY_JITTER_MS", "500"),
("STALE_RETRY_BASE_MS", "2000"),
):
assert f"export const {const} = {value};" in body, f"missing export const {const}"
assert "export function overflowWindowTripped(" in body
turnstone/console/static/coordinator/coordinator.js
+2 -1
View File
@@ -58,6 +58,7 @@ import {
DEGRADED_COOLDOWN_RESET_MS,
TRUNCATED_RESYNC_JITTER_MS,
STALE_RETRY_JITTER_MS,
STALE_RETRY_BASE_MS,
overflowWindowTripped,
degradedCooldownStep,
} from "/shared/sse_overflow.js";
@@ -3806,7 +3807,7 @@ function createCoordinatorPane(root, wsId, opts) {
// 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,
STALE_RETRY_BASE_MS + Math.random() * STALE_RETRY_JITTER_MS,
);
}
if (!_pendingEditSend) return;
turnstone/shared_static/interactive.js
+2 -1
View File
@@ -64,6 +64,7 @@ import {
DEGRADED_COOLDOWN_RESET_MS,
TRUNCATED_RESYNC_JITTER_MS,
STALE_RETRY_JITTER_MS,
STALE_RETRY_BASE_MS,
overflowWindowTripped,
degradedCooldownStep,
} from "./sse_overflow.js";
@@ -2579,7 +2580,7 @@ class Pane {
// 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,
STALE_RETRY_BASE_MS + Math.random() * STALE_RETRY_JITTER_MS,
);
}
if (token !== this._historyLoadToken && this.wsId !== editWs) {
turnstone/shared_static/sse_overflow.js
+10 -4
View File
@@ -45,8 +45,8 @@ 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
// Spread for the clear_ui staleness retry, ADDITIVE over STALE_RETRY_BASE_MS
// (`STALE_RETRY_BASE_MS + 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
@@ -57,10 +57,16 @@ export const TRUNCATED_RESYNC_JITTER_MS = 10000;
// 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.
// The floor is load-bearing for the e2e non-occurrence detectors (they size
// their windows on floor + this); raising either requires widening those.
export const STALE_RETRY_JITTER_MS = 500;
// The floor the jitter is additive OVER. Exported rather than left as a
// literal in each client because FOUR sites must move together: both panes'
// retry arms and both e2e non-occurrence windows, which size themselves on
// floor + jitter and go vacuous if the retry can fire before they open.
export const STALE_RETRY_BASE_MS = 2000;
// 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