diff --git a/tests/test_app_js.py b/tests/test_app_js.py index 549bd25c..1c7745b5 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -2241,12 +2241,18 @@ def test_sse_cursor_captured_from_message_event_never_the_source_object() -> Non every cursor guard keeps a future editor from "simplifying" the numeric ones to match a terser string form. - The GLOBAL stream (app.js) is the deliberate exception: its manual - reconnects are pinned CURSORLESS — the global ring's counter reboots - at 0 on restart (KNOWN GAP #881), so a stale cursor draws - ``replay_ok``-empty with no node_snapshot and the roster ghosts; - cursorless always draws the fresh snapshot. Revisit when #881 - lands. + The GLOBAL stream (app.js) was pinned CURSORLESS here until #881 + landed its boot-epoch signal: global ids are now + ``"{boot_epoch}-{counter}"``, so a cursor from a prior boot + mismatches the live epoch and draws ``replay_truncated`` + a fresh + node_snapshot instead of the old ``replay_ok``-empty ghost-roster + shape — the reason for cursorlessness is gone, and app.js now + captures/presents the cursor like the per-ws clients. Its cursor + stays an OPAQUE STRING end to end (never ``parseInt``/``Number`` + — the epoch prefix would NaN any numeric read), presented via + ``?last_event_id=`` (EventSource cannot set the header manually), + and cleared where the record dies: the ``replay_truncated`` + handler and ``onLogout``. Comments are stripped before the scan (module-level, string-aware stripper) so documentation may name the anti-pattern verbatim — the @@ -2262,6 +2268,11 @@ def test_sse_cursor_captured_from_message_event_never_the_source_object() -> Non _COORD_JS, r'if \(event\.lastEventId != null && event\.lastEventId !== ""\) \{', ), + ( + _APP_JS, + r'if \(e\.lastEventId != null && e\.lastEventId !== ""\) \{\s*' + r"globalLastEventId = e\.lastEventId;", + ), ): body = path.read_text(encoding="utf-8") code = _strip_js_comments(body) @@ -2275,14 +2286,22 @@ def test_sse_cursor_captured_from_message_event_never_the_source_object() -> Non '``!= null && !== ""`` guard) not found' ) app_code = _strip_js_comments(_APP_JS.read_text(encoding="utf-8")) - assert not dead_form.search(app_code), ( - "app.js: dead EventSource-object cursor read must not return" + # Presentation: manual reconnects carry the stored cursor as a query + # param, behind the same string-guard idiom (see docstring above for + # why the explicit form is pinned). + assert re.search( + r'if \(globalLastEventId != null && globalLastEventId !== ""\) \{\s*' + r'globalUrl \+= "\?last_event_id=" \+ encodeURIComponent\(globalLastEventId\);', + app_code, + ), ( + "app.js: manual global reconnect must present the stored cursor " + "via ?last_event_id= behind the house string guard" ) - assert "lastEventId" not in app_code and "last_event_id" not in app_code, ( - "app.js global stream must stay CURSORLESS on manual reconnects " - "until #881's boot-epoch staleness signal lands — a stale cursor " - "on the reborn global ring draws replay_ok-empty with no " - "node_snapshot (ghost roster)." + # The cursor is opaque — any numeric interpretation of it is a bug + # (the epoch prefix turns Number()/parseInt() into NaN silently). + assert not re.search(r"(?:Number|parseInt)\(\s*globalLastEventId", app_code), ( + "app.js: the global cursor is an opaque epoch-tagged string — " + "never interpret it numerically" ) diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index d718718b..2d1e4950 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -16,6 +16,14 @@ let workstreams = {}; let currentWsId = null; let globalEvtSource = null; let globalRetryDelay = 1000; +// Resume cursor for the global stream — the last SSE id observed, an +// OPAQUE ``"{boot_epoch}-{counter}"`` string (#881). Captured from the +// MessageEvent in the global onmessage, presented as ``?last_event_id=`` +// on manual reconnects, and cleared when the record is dead: on a +// ``replay_truncated`` envelope (the gap is being healed by snapshot + +// resync) and on logout (roster identity reset). Never parsed here — +// only the server can interpret it. +let globalLastEventId = null; let dashboardVisible = false; let _historyNavigation = false; let _lastHealth = null; @@ -115,6 +123,10 @@ window.onLogout = function () { globalEvtSource.close(); globalEvtSource = null; } + // Roster identity reset — the next principal starts cursorless + // (fresh snapshot) rather than resuming from this session's stream + // position. + globalLastEventId = null; fireRender(); }; @@ -1752,29 +1764,42 @@ function connectGlobalSSE() { globalEvtSource.close(); globalEvtSource = null; } - // Manual reconnects on the GLOBAL stream are DELIBERATELY cursorless - // (no ``?last_event_id=`` — do not add a MessageEvent lastEventId - // capture here like the per-ws streams'): the global ring cannot - // report truncation for a stale cursor after a node restart (its - // counter reboots at 0 — KNOWN GAP #881), so presenting one draws - // ``replay_ok``-empty with NO node_snapshot and the roster ghosts - // exactly when a full rebuild is most needed. Cursorless manual - // reconnects always draw the fresh node_snapshot — lossless for - // roster STATE (unlike per-ws append-only history, where the - // storage-seeded counter makes a stale cursor report ``truncated`` - // and the cursor is therefore safe to track). Native auto-reconnect - // keeps its browser-internal header either way. Revisit when #881's - // boot-epoch staleness signal lands. - globalEvtSource = new EventSource("/v1/api/events/global"); + // Manual reconnects present the stored cursor (an opaque + // ``"{boot_epoch}-{counter}"`` string) via the query param — a + // ``new EventSource(url)`` cannot set the ``Last-Event-ID`` header. + // Safe since #881: a cursor from another boot (or another node, or a + // pre-epoch client) mismatches the server's live epoch and draws + // ``replay_truncated`` + a fresh node_snapshot — the ghost-roster + // shape that once forced these reconnects cursorless (a stale cursor + // drew ``replay_ok``-empty with NO snapshot against the reborn ring) + // can no longer occur. A live cursor skips the wholesale snapshot + // rebuild and replays just the missed deltas. Native auto-reconnect + // keeps its browser-internal header either way; the string guard is + // the house cursor-guard idiom (see test_app_js.py). + let globalUrl = "/v1/api/events/global"; + if (globalLastEventId != null && globalLastEventId !== "") { + globalUrl += "?last_event_id=" + encodeURIComponent(globalLastEventId); + } + globalEvtSource = new EventSource(globalUrl); globalEvtSource.onopen = function () { globalRetryDelay = 1000; }; globalEvtSource.onmessage = function (e) { - // Guarded parse: native auto-reconnect's header cursor has already - // advanced past this frame, so a parse failure is a permanently-lost - // roster mutation — resync the roster from REST instead of silently - // drifting (a dropped ws_created renders as a conversation that - // never appears; a dropped ws_closed as a ghost row forever). + // Cursor capture BEFORE the parse, mirroring the browser: native + // reconnect's header cursor advances past every id-bearing frame + // whether or not its JSON parses, and the manual cursor must agree + // with it (both recover a lost frame the same way — the resync + // below). MessageEvent property, never the EventSource object + // (which has no such property — see the test_app_js.py tripwire). + if (e.lastEventId != null && e.lastEventId !== "") { + globalLastEventId = e.lastEventId; + } + // Guarded parse: the cursor (native header and stored copy alike) + // has already advanced past this frame, so a parse failure is a + // permanently-lost roster mutation — resync the roster from REST + // instead of silently drifting (a dropped ws_created renders as a + // conversation that never appears; a dropped ws_closed as a ghost + // row forever). let data = null; try { data = JSON.parse(e.data); @@ -1793,9 +1818,15 @@ function connectGlobalSSE() { // stream itself. applyRosterSnapshot(data.workstreams || [], { evict: true }); } else if (data.type === "replay_truncated") { - // Events between our cursor and the buffer head are gone for good. - // The node_snapshot that follows rebuilds the roster; refetch too so - // recovery doesn't depend on event ordering. + // Events between our cursor and the buffer head (reason + // "ring_evicted"), or everything since another boot minted the + // cursor (reason "boot_epoch"), are gone for good. The cursor is + // spent — clear it so a manual reconnect racing in before the + // next id-bearing frame draws a fresh snapshot instead of + // re-presenting the dead cursor for a redundant truncated round. + // The node_snapshot that follows rebuilds the roster; refetch too + // so recovery doesn't depend on event ordering. + globalLastEventId = null; resyncRoster(); } else if (data.type === "ws_state") { updateTabIndicator(data.ws_id, data.state, {