diff --git a/tests/test_app_js.py b/tests/test_app_js.py index f7eee1b3..d32eab8f 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -1735,3 +1735,25 @@ def test_early_paint_screen_reader_announce() -> None: assert "toolAnnounce(_toolAnnounceText(list))" in body assert 'block.setAttribute("aria-busy", "true")' in body assert 'block.removeAttribute("aria-busy")' in body + + +def test_global_stream_recovery_floor_and_render_coalescing() -> None: + """Perf-audit P0/P1 for the Tier-1 global stream. The server's recovery + events for a truncated reconnect gap (``node_snapshot`` as the floor, + ``replay_truncated`` as the marker) used to fall through the handler + silently — workstreams created during a long hidden-tab gap never + rendered again, and missed ``ws_closed`` left ghost rows forever. A + malformed frame is the same permanent drift (the cursor advances before + the parse), so it resyncs too. ``fireRender`` is rAF-coalesced: every + ``ws_state`` (≥2 per tool round per workstream) used to trigger a + synchronous full rail rebuild.""" + body = _APP_JS.read_text(encoding="utf-8") + assert 'data.type === "node_snapshot"' in body + assert 'data.type === "replay_truncated"' in body + assert "function applyRosterSnapshot(" in body + assert "function resyncRoster(" in body + assert "malformed frame" in body + fire = body.index("function fireRender()") + assert "requestAnimationFrame(" in body[fire : fire + 700], ( + "fireRender must coalesce subscriber repaints to one per frame" + ) diff --git a/tests/test_conversation_js.py b/tests/test_conversation_js.py index 623109f4..a2d5a04a 100644 --- a/tests/test_conversation_js.py +++ b/tests/test_conversation_js.py @@ -150,3 +150,21 @@ def test_warning_and_verdict_normalize_risk() -> None: assert "normalizeRiskLevel(a.risk_level)" in body, "warning must normalize" assert '"conv-warning conv-warning--" + risk' in body assert 'badge.classList.add("conv-verdict--" + risk)' in body + + +def test_unbounded_render_inputs_are_capped() -> None: + """Perf-audit P0: the two builders that used to render unbounded input. + The diff preview caps rendered lines and appends incrementally — the old + single ``diff.append(...nodes)`` spread threw RangeError past engine + spread-arity limits, killing the tool card (and the approval gate) for + the batch. The raw result body clamps at RAW_CAP so one multi-MB tool + output can't become a multi-MB pre-wrap text node rebuilt on every + re-render.""" + body = _body() + assert "MAX_PREVIEW_LINES" in body + assert "diff.append(...nodes)" not in body, ( + "preview nodes must append incrementally, not via one spread call" + ) + assert "more preview lines not shown" in body + assert "RAW_CAP" in body + assert "truncated for display" in body diff --git a/tests/test_interactive_pane_js.py b/tests/test_interactive_pane_js.py index 568092a0..0608167f 100644 --- a/tests/test_interactive_pane_js.py +++ b/tests/test_interactive_pane_js.py @@ -245,3 +245,103 @@ def test_controller_terminal_dead_state() -> None: assert "base: base," in body, "the controller must expose its transport base" # Dead controllers don't reconnect on re-auth. assert "if (connected && !dead) pane._loadHistoryThenConnect(wsId);" in body + + +def test_stream_pipeline_is_wedge_proof() -> None: + """Long-session hardening (perf audit P0): the SSE pipeline must not be + able to permanently wedge the pane. ``onmessage`` guards BOTH the + ``JSON.parse`` and the ``handleEvent`` dispatch (an exception escaping it + doesn't close the EventSource, so an unguarded throw left the streaming + refs poisoned for the rest of the session), and ``stream_end`` resets the + segment refs BEFORE the finalize render, with a plain-text fallback — + with the old order a finalize throw skipped the clears and every later + delta painted into the dead segment.""" + body = _INTERACTIVE.read_text(encoding="utf-8") + assert "dropping malformed SSE frame" in body + assert "handleEvent failed for" in body + case = body.index('case "stream_end"') + seg = body[case : body.index("break;", case)] + clears = seg.index("this.currentAssistantBodyEl = null;") + finalize = seg.index("streamingRenderFinalize(") + assert clears < finalize, ( + "stream_end must clear segment refs BEFORE finalize — the old " + "finalize-first order wedged all later assistant output on a throw." + ) + assert "doneBodyEl.textContent = doneBuffer;" in seg + + +def test_rebuild_quiesces_live_events_and_releases_agent_tracking() -> None: + """clear_ui / replay_truncated re-render race (perf audit P0): live SSE + events painted between the history snapshot and ``replaceChildren()`` + were wiped with no redelivery, and streaming refs kept pointing at + detached nodes. Pinned: the quiesce queue sits on the handleEvent hot + path, both re-render triggers arm it, ``replayHistory`` resets the + streaming refs and clears the agent-card/orphan maps (the detached-DOM + retention leak), and the mid-stream guard covers the reasoning bubble.""" + body = _INTERACTIVE.read_text(encoding="utf-8") + assert "this._replayQueue.events.push(evt);" in body + assert body.count("this._beginReplayQuiesce(") >= 2, ( + "both clear_ui and replay_truncated must arm the quiesce" + ) + assert "!this.currentAssistantEl && !this.currentReasoningEl" in body + replay = body.index("replayHistory(messages) {") + seg = body[replay : replay + 1600] + for line in ( + "this._resetStreamingRefs();", + "this._clearAgentTracking();", + ): + assert line in seg, f"replayHistory must reset: {line!r}" + assert "this._agentCards.clear();" in body + # Review-hardened lifecycle: the card entry SURVIVES the terminal + # tool_result (a late child event finding no Map entry would rebuild a + # duplicate empty card beside the finished one), and transport-only + # reconnects preserve the maps + any armed quiesce queue — clearing them + # in disconnectSSE duplicated cards and dropped buffered orphan steps on + # every transient stream blip. Full-reload cleanup lives in + # _loadHistoryThenConnect; terminal cleanup in the factory's destroy(). + assert "this._agentCards.delete(callId);" not in body + disc = body.index("disconnectSSE() {") + disc_seg = body[disc : body.index("_loadHistoryThenConnect(wsId) {", disc)] + assert "this._clearAgentTracking();" not in disc_seg + assert "this._replayQueue = null;" not in disc_seg + load = body.index("_loadHistoryThenConnect(wsId) {") + load_seg = body[load : load + 2200] + assert "this._clearAgentTracking();" in load_seg + assert "this._replayQueue = null;" in load_seg + # A mid-stream replay_truncated DEFERS the re-sync (flag consumed on the + # idle edge) instead of dropping it — skipping left the lost-event gap + # unrepaired for the rest of the session. + assert "this._pendingTruncatedResync = true;" in body + # The refetch FAILURE branch resets streaming refs too — it never reaches + # replayHistory, and stale refs there streamed the retried generation's + # first segment into a detached bubble. + fail = body.index("Failure path never reaches replayHistory") + assert "this._resetStreamingRefs();" in body[fail : fail + 400], ( + "the refetch failure branch must reset streaming refs" + ) + + +def test_per_token_hot_path_avoids_container_scans() -> None: + """P1 (perf audit): per-token work must stay O(1) in transcript length. + The thinking indicator is an instance ref (the class-selector miss walked + the whole transcript on EVERY content/reasoning delta); near-bottom state + comes from the passive scroll listener instead of a forced-layout + geometry read per event; the scroll pin is rAF-coalesced; per-tool + row/stream lookups resolve through the self-healing caches.""" + body = _INTERACTIVE.read_text(encoding="utf-8") + stripped = _strip_comments(body) + assert 'querySelector(".thinking-indicator")' not in stripped, ( + "thinking indicator must use the instance ref, not a container scan" + ) + assert "this._thinkingEl" in body + near = body.index("isNearBottom() {") + assert "return this._nearBottom;" in body[near : near + 700] + assert "passive: true" in body + # The rAF pin re-checks the flag AT FIRE TIME (a user scroll landing in + # the schedule→rAF window must win over a stale pin), with force + # requests latched across the coalescing window; resizes re-derive the + # flag via ResizeObserver since they move the bottom without a scroll. + assert "this._scrollPinForce = false;" in body + assert "ResizeObserver" in body + for helper in ("_toolRow(callId) {", "_streamEl(callId) {"): + assert helper in body, f"missing lookup-cache helper: {helper!r}" diff --git a/tests/test_renderer_js.py b/tests/test_renderer_js.py index 0d2294b6..280d6b2c 100644 --- a/tests/test_renderer_js.py +++ b/tests/test_renderer_js.py @@ -1511,3 +1511,41 @@ def test_link_url_with_ampersand_not_double_escaped() -> None: "Expected single & encoding for `&`; got:\n" + out ) assert "&amp;" not in out + + +def test_render_markdown_depth_capped_and_throw_safe() -> None: + """Perf-audit P0: ``renderMarkdown`` recurses for blockquote/callout + bodies, and a few KB of nested ``"> "`` used to overflow the call stack + mid-render. The exported wrapper depth-caps the recursion (bailing to + escaped text) and keeps the ``_fnDepth`` accounting in a try/finally so a + body throw can't strand it elevated (which froze ``_fnScopeId`` and + collided footnote ids for every later message).""" + body = _RENDERER_JS.read_text(encoding="utf-8") + assert "var _MD_MAX_DEPTH" in body + assert "_fnDepth >= _MD_MAX_DEPTH" in body + wrapper = body.index("export function renderMarkdown(text)") + seg = body[wrapper : body.index("function _renderMarkdownBody(text)")] + assert "try {" in seg and "finally {" in seg and "_fnDepth--;" in seg, ( + "depth accounting must ride a try/finally in the wrapper" + ) + + +def test_streaming_apply_marks_buffer_only_on_success() -> None: + """Perf-audit P0: ``_streamingRenderApply`` must set + ``el._lastRenderedBuffer`` only AFTER a successful render, with a + plain-text fallback on throw. Marking before the render made an errored + frame look done — the finalize short-circuit then pinned the broken DOM + forever. The mermaid chain must also be rejection-proof (a sync throw in + a settle handler used to leave every later diagram stuck at 'Loading + diagram…').""" + body = _RENDERER_JS.read_text(encoding="utf-8") + apply_at = body.index("function _streamingRenderApply") + seg = body[apply_at : apply_at + 2000] + render_at = seg.index("renderMarkdown(buffer)") + mark_at = seg.index("el._lastRenderedBuffer = buffer;") + assert render_at < mark_at, "buffer must be marked rendered only on success" + assert "el.textContent = buffer;" in seg + chain_at = body.index("_mermaidRenderChain = _mermaidRenderChain") + assert ".catch(function (e) {" in body[chain_at : chain_at + 3500], ( + "every mermaid chain link must settle back to fulfilled" + ) diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js index f1c5bd84..55ef7fa1 100644 --- a/turnstone/console/static/app.js +++ b/turnstone/console/static/app.js @@ -21,6 +21,10 @@ window.onLoginSuccess = function () { } }; window.onLogout = function () { + if (sseReconnectTimer) { + clearTimeout(sseReconnectTimer); + sseReconnectTimer = null; + } if (evtSource) { evtSource.close(); evtSource = null; @@ -67,6 +71,10 @@ let currentView = "home"; // "home" | "overview" | "filtered" | "admin" let currentFilter = { state: null, node: null, page: 1, per_page: 50 }; let evtSource = null; let retryDelay = 1000; +// Pending reconnect handle — tracked so logout (and a fresh connectSSE) can +// cancel it; an untracked timer fired post-logout and opened a new +// EventSource that 401s and re-probes in a loop. +let sseReconnectTimer = null; let clusterState = null; let _navigatingFromPopstate = false; @@ -346,6 +354,10 @@ function _fireRenderSubs() { // --- SSE Connection --- function connectSSE() { + if (sseReconnectTimer) { + clearTimeout(sseReconnectTimer); + sseReconnectTimer = null; + } if (evtSource) { evtSource.close(); evtSource = null; @@ -380,11 +392,11 @@ function connectSSE() { showLogin(); return; } - setTimeout(connectSSE, retryDelay); + sseReconnectTimer = setTimeout(connectSSE, retryDelay); retryDelay = Math.min(retryDelay * 2, 30000); }) .catch(function () { - setTimeout(connectSSE, retryDelay); + sseReconnectTimer = setTimeout(connectSSE, retryDelay); retryDelay = Math.min(retryDelay * 2, 30000); }); }; diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index dbc89f17..83569aea 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -2207,31 +2207,64 @@ function createCoordinatorPane(root, wsId, opts) { // Transient errors (network blips, intermediary timeouts) just // let native reconnect run — no scheduleReconnect needed // because the source isn't dead. - var probe = typeof authFetch === "function" ? authFetch : fetch; - probe("/v1/api/workstreams/" + encodeURIComponent(wsId)).then( - function (r) { - if (r.status === 401 && typeof showLogin === "function") { - try { - if (evtSource) evtSource.close(); - } catch (_) { - /* noop */ - } - evtSource = null; - // Cancel the pending CLOSED-state recovery timer (set - // below). Without this, 5 s later the timer would - // observe ``!evtSource`` and call ``scheduleReconnect``, - // which would open a new EventSource that gets 401 again - // → infinite reconnect loop while the login overlay is - // up. The login flow re-arms ``connectSSE`` after a - // successful sign-in via its own callback path. - if (reconnectTimer) { - clearTimeout(reconnectTimer); - reconnectTimer = null; - } - showLogin("Session expired. Please sign in to reconnect."); - } - }, - ); + // Raw fetch (not authFetch) — need to inspect status before throwing. + // authFetch never RESOLVES with a 401 (it calls showLogin() itself and + // throws Error("auth")), so probing through it made this branch dead + // code: the close/cancel-timer handling below never ran and the + // CLOSED-state recovery kept cycling scheduleReconnect behind the + // login overlay — exactly the loop this branch exists to prevent. + // Mirrors the app.js dashboard probe. ``.catch``: a network-dead + // probe is the transient case; native/manual reconnect owns it. + // + // The 401 body is inspected BEFORE the generic-expiry handling: a + // code=version_mismatch body must take auth.js's upgrade path + // (reload-after-re-login flag + "upgrade" overlay). The old authFetch + // probe did that as a side effect of authFetch's own 401 handling; a + // raw fetch must do it explicitly or a server upgrade leaves stale + // pre-upgrade JS running after sign-in. NOTE the positive-form guard + // (r.status === 401) directly above the close(): the reconnect- + // contract pin (test_app_js._onerror_preserves_native_reconnect) keys + // on that marker within a short window to allow a terminal close. + fetch("/v1/api/workstreams/" + encodeURIComponent(wsId)) + .then(function (r) { + if (!(r.status === 401 && typeof showLogin === "function")) return; + return r + .json() + .catch(function () { + return null; + }) + .then(function (body) { + try { + if (evtSource) evtSource.close(); + } catch (_) { + /* noop */ + } + evtSource = null; + // Cancel the pending CLOSED-state recovery timer (set + // below). Without this, 5 s later the timer would + // observe ``!evtSource`` and call ``scheduleReconnect``, + // which would open a new EventSource that gets 401 again + // → infinite reconnect loop while the login overlay is + // up. The login flow re-arms ``connectSSE`` after a + // successful sign-in via its own callback path. + if (reconnectTimer) { + clearTimeout(reconnectTimer); + reconnectTimer = null; + } + if ( + body && + body.code === "version_mismatch" && + typeof noteVersionMismatch === "function" + ) { + noteVersionMismatch(); + } else { + showLogin("Session expired. Please sign in to reconnect."); + } + }); + }) + .catch(function () { + /* transient network failure — reconnect machinery handles it */ + }); // CLOSED-state recovery: native auto-reconnect covers the // transient case (source stays in CONNECTING and eventually // re-opens). But if the browser gives up — hard 4xx after @@ -3535,13 +3568,7 @@ function createCoordinatorPane(root, wsId, opts) { // pending count is maintained incrementally on cache mutations // (see ``pendingApprovalIds`` near the cache definition) so this // is O(1) per render rather than an O(N) walk over the cache. - const pending = pendingApprovalIds.size; - childrenCountEl.textContent = rows.length - ? "(" + - rows.length + - (pending > 0 ? " · " + pending + " pending" : "") + - ")" - : ""; + _refreshChildrenCount(); _restoreRowFocus(childrenTreeEl, focusKey); } @@ -3562,13 +3589,32 @@ function createCoordinatorPane(root, wsId, opts) { const replacement = renderChildRow(entry); row.replaceWith(replacement); const obs = _getChildObserver(); - if (obs) obs.observe(replacement); + if (obs) { + // Release the detached row from the persistent observer — this is + // now the hot path (every child_ws_state tick), and observed-but- + // detached rows are strong refs that would accumulate without bound + // between full renders (which reset targets via disconnect()). + obs.unobserve(row); + obs.observe(replacement); + } _restoreRowFocus(replacement, focusKey); + // Keep the "(N · x pending)" annotation live on the targeted path — + // approval edges arrive as state ticks now that child_ws_state no + // longer takes the full render. + _refreshChildrenCount(); } else { renderChildren(); } } + function _refreshChildrenCount() { + const total = childrenState.size; + const pending = pendingApprovalIds.size; + childrenCountEl.textContent = total + ? "(" + total + (pending > 0 ? " · " + pending + " pending" : "") + ")" + : ""; + } + function renderTaskRow(task) { const row = document.createElement("div"); row.className = "task-row"; @@ -3855,6 +3901,12 @@ function createCoordinatorPane(root, wsId, opts) { ws_id: childId, name: "", }; + // Terminal-bucket membership BEFORE the mutation: the tree sort keys on + // it (non-terminal first), so a state tick that crosses the boundary + // needs the full re-sorting render; everything else takes the targeted + // single-row path below. + const wasTerminal = + existing.state === "closed" || existing.state === "deleted"; existing.state = ev.state || existing.state; existing.activity_state = typeof ev.activity_state === "string" @@ -3924,7 +3976,18 @@ function createCoordinatorPane(root, wsId, opts) { ? cached.sseUpdatedAt || 0 : 0, }); - renderChildren(); + // child_ws_state is the HIGHEST-frequency child event (a tick per state/ + // activity change of every child) — route it through the targeted + // single-row update instead of the full-tree rebuild. The full render + // (sort + replaceChildren + observer re-observe of every row) is + // reserved for membership/sort-order changes: a terminal-bucket + // crossing here, and created/closed/rename in their own handlers. + // _updateChildRow falls back to renderChildren() itself when the row + // isn't painted yet (a brand-new child). + const isTerminal = + existing.state === "closed" || existing.state === "deleted"; + if (wasTerminal !== isTerminal) renderChildren(); + else _updateChildRow(childId); // Do NOT invalidateLiveBadge on routine state ticks — that // defeats the 5s TTL cache and devolves rate-limiting to the // 250ms debouncer. The TTL check in scheduleLiveFetch handles diff --git a/turnstone/shared_static/auth.js b/turnstone/shared_static/auth.js index b8bc34a0..5dad93ae 100644 --- a/turnstone/shared_static/auth.js +++ b/turnstone/shared_static/auth.js @@ -47,6 +47,15 @@ if (_authChannel) { }; } +// Raw-fetch callers (the SSE-error probes, which must inspect a 401's status +// without authFetch's throw-on-401 contract) route a version_mismatch body +// here so the post-re-login reload still picks up the new assets — the same +// flag+overlay path authFetch takes below. +export function noteVersionMismatch() { + _authUpgradeReload = true; + showLogin("upgrade"); +} + export async function authFetch(url, opts) { const maxRetries = 2; for (let attempt = 0; attempt <= maxRetries; attempt++) { @@ -813,4 +822,5 @@ Object.assign(window, { hideLogin, logout, initLogin, + noteVersionMismatch, }); diff --git a/turnstone/shared_static/composer_queue.js b/turnstone/shared_static/composer_queue.js index 556e4328..0701f768 100644 --- a/turnstone/shared_static/composer_queue.js +++ b/turnstone/shared_static/composer_queue.js @@ -95,6 +95,9 @@ export function createQueueController(opts) { typeof opts.onAfterDequeue === "function" ? opts.onAfterDequeue : null; var onIdle = typeof opts.onIdle === "function" ? opts.onIdle : null; var onNotice = typeof opts.onNotice === "function" ? opts.onNotice : null; + // Live queued bubbles — the idle sweep iterates this instead of querying + // the whole messages container (see onIdleEdge). + var _liveQueued = new Set(); // Upper bound on the dequeue DELETE so a wedged proxied node (the exact // case this flow targets) can't leave a card stuck "dismissing" forever. var DELETE_TIMEOUT_MS = 15000; @@ -199,6 +202,7 @@ export function createQueueController(opts) { host.appendChild(dismiss); messagesEl.appendChild(el); + _liveQueued.add(el); _scrollIntoView(); return el; } @@ -319,6 +323,7 @@ export function createQueueController(opts) { } function remove(el) { + _liveQueued.delete(el); if (el && el.parentNode) el.remove(); } @@ -329,6 +334,7 @@ export function createQueueController(opts) { // cancelled, so present it as sent. If the user had clicked × first // (dismissAttempted), tell them it was too late. function _promote(el) { + _liveQueued.delete(el); var attempted = el.dataset.dismissAttempted; el.classList.remove("msg-queued", "msg-queued-important"); delete el.dataset.msgId; @@ -351,8 +357,19 @@ export function createQueueController(opts) { // onIdle hook so the consumer can run edge-only cleanup (e.g. clearing // cancel/force-stop timers). function onIdleEdge() { - var queued = messagesEl.querySelectorAll(".msg-queued:not([aria-busy])"); - queued.forEach(_promote); + // Sweep the controller-local live set, not the DOM: the old + // ".msg-queued:not([aria-busy])" query walked every element under the + // messages container (O(transcript) per busy→idle edge) to find the + // handful of queued bubbles that always sit in the tail. Bubbles wiped + // by a full re-render prune lazily via the isConnected check. + _liveQueued.forEach(function (el) { + if (!el.isConnected) { + _liveQueued.delete(el); + return; + } + if (el.hasAttribute("aria-busy")) return; // mid-dequeue — let it settle + _promote(el); + }); if (onIdle) onIdle(); } diff --git a/turnstone/shared_static/conversation.css b/turnstone/shared_static/conversation.css index e2eedd84..40a02ec6 100644 --- a/turnstone/shared_static/conversation.css +++ b/turnstone/shared_static/conversation.css @@ -293,6 +293,16 @@ .conv-diff-warn { color: var(--warn); } +/* Preview-omission notice — rendered as a SIBLING below the .conv-row-diff + scroll box (never inside it, where the 240px fold hides it). Neutral ink, + not --warn: informational omission, and AA-safe on both themes. */ +.conv-diff-omit { + margin-top: 2px; + padding: 2px 10px; + font-family: var(--font-mono); + font-size: 11px; + color: var(--ink-3); +} /* Verdict badge — interactive's rich shape (risk + rec + conf, expandable detail) in the coordinator's neutral idiom. Risk drives the left-stripe diff --git a/turnstone/shared_static/conversation.js b/turnstone/shared_static/conversation.js index 4c29a578..e4390614 100644 --- a/turnstone/shared_static/conversation.js +++ b/turnstone/shared_static/conversation.js @@ -273,10 +273,19 @@ export function buildConvCmd(item) { if (item.preview) { const diff = document.createElement("div"); diff.className = "conv-row-diff"; - const lines = stripAnsi(item.preview).split("\n"); - const nodes = []; + // The preview is uncapped upstream (a whole multiline command / one line + // per edited line) — cap what we RENDER: past ~400 lines the preview + // carries no decision value, the DOM cost is ~2 nodes/line in every + // transcript row, and an argument-spread append of an unbounded node + // list can throw RangeError mid-paint (engines cap spread arity around + // 65k args), killing the tool card — and the approval gate — for the + // batch. Appended incrementally for the same reason. + const MAX_PREVIEW_LINES = 400; + let lines = stripAnsi(item.preview).split("\n"); + const omitted = lines.length - MAX_PREVIEW_LINES; + if (omitted > 0) lines = lines.slice(0, MAX_PREVIEW_LINES); lines.forEach((line, i) => { - if (i > 0) nodes.push("\n"); + if (i > 0) diff.appendChild(document.createTextNode("\n")); const trimmed = line.trim(); let cls = null; if (trimmed.startsWith("-")) cls = "conv-diff-del"; @@ -286,13 +295,25 @@ export function buildConvCmd(item) { const span = document.createElement("span"); span.className = cls; span.textContent = line; - nodes.push(span); + diff.appendChild(span); } else { - nodes.push(line); + diff.appendChild(document.createTextNode(line)); } }); - diff.append(...nodes); frag.appendChild(diff); + // The omission notice sits BELOW the scroll box as a sibling, not as the + // diff's last child: .conv-row-diff is a 240px inner scroller, so an + // inline marker would sit thousands of pixels below its fold — invisible + // exactly at the approval moment, where the operator must know the + // preview is partial. Its own neutral class (not .conv-diff-warn): + // an omission is informational, not a command warning, and raw --warn + // fails AA on the light panel background. + if (omitted > 0) { + const more = document.createElement("div"); + more.className = "conv-diff-omit"; + more.textContent = "… " + omitted + " more preview lines not shown"; + frag.appendChild(more); + } } return frag; } @@ -596,6 +617,20 @@ export function buildConvResult(output, opts) { } } } + // Clamp the rendered body — same rationale as the JSON pretty-print cap + // above: the server ships tool output verbatim, and a single multi-MB + // result (an agent cat-ing a large file) becomes a multi-MB pre-wrap text + // node that stalls layout on insert and is rebuilt on every full + // re-render. The transcript shows the head; the full output stays in + // history/storage. + const RAW_CAP = 64 * 1024; + if (pretty.length > RAW_CAP) { + pretty = + pretty.slice(0, RAW_CAP) + + "\n… (" + + pretty.length.toLocaleString() + + " chars total — truncated for display)"; + } const body = document.createElement("span"); body.textContent = pretty; block.appendChild(body); diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js index 8291b7fe..835767af 100644 --- a/turnstone/shared_static/interactive.js +++ b/turnstone/shared_static/interactive.js @@ -205,6 +205,24 @@ class Pane { this.projectName = ""; this._lastStatusEvt = null; this._historyLoadToken = 0; + // Event backlog while a clear_ui / replay_truncated rebuild is in + // flight — see _beginReplayQuiesce. {token, events[]} or null. + this._replayQueue = null; + // Hot-path caches — all invalidated by _clearAgentTracking/replayHistory. + // _nearBottom mirrors the scroller position via a passive scroll listener + // (no per-token geometry reads); the two Maps make per-event row/stream + // lookups O(1) instead of whole-transcript attribute-selector scans. + this._nearBottom = true; + this._scrollPinPending = false; + this._scrollPinForce = false; + this._thinkingEl = null; + this._retryHolderEl = null; + this._toolRowIndex = new Map(); + this._streamElIndex = new Map(); + this._resizeObs = null; + // Set when replay_truncated arrives mid-stream (refetching then would + // detach the live bubble); consumed on the next idle edge. + this._pendingTruncatedResync = false; this._cancelTimeout = null; this._forceTimeout = null; this._pendingEditSend = null; @@ -254,6 +272,14 @@ 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 + // (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 + // orphan grace timers silently dropped buffered steps. Ws-switch and + // full-reload cleanup happens in _loadHistoryThenConnect; terminal + // cleanup in the factory's destroy(). this._stopRecording(true); this._stopTTS(); } @@ -298,17 +324,22 @@ class Pane { } addThinkingIndicator() { - if (this.messagesEl.querySelector(".thinking-indicator")) return; + // Instance ref, not a container query: removeThinkingIndicator runs on + // EVERY content/reasoning delta, and a class-selector miss walks the + // whole transcript subtree — O(N) per streamed token at 5000 messages. + if (this._thinkingEl) return; const el = document.createElement("div"); el.className = "thinking-indicator"; el.textContent = "Thinking"; + this._thinkingEl = el; this.messagesEl.appendChild(el); this.scrollToBottom(); } removeThinkingIndicator() { - const el = this.messagesEl.querySelector(".thinking-indicator"); - if (el) el.remove(); + if (!this._thinkingEl) return; + this._thinkingEl.remove(); + this._thinkingEl = null; } addSystemNudgeMarker() { @@ -440,18 +471,9 @@ class Pane { // announceToolBlock. const stick = this.isNearBottom(); - const escapedId = callId ? CSS.escape(callId) : ""; - let el = escapedId - ? this.messagesEl.querySelector( - '.tool-output-stream[data-call-id="' + escapedId + '"]', - ) - : null; + let el = this._streamEl(callId); if (!el) { - let target = escapedId - ? this.messagesEl.querySelector( - '.conv-row[data-call-id="' + escapedId + '"]', - ) - : null; + let target = this._toolRow(callId); if (!target) { // A namespaced sub-agent child id ("::") whose row hasn't // nested yet must NOT graft its stream onto the last top-level batch — @@ -479,19 +501,26 @@ class Pane { el.setAttribute("aria-live", "off"); el.textContent = ""; target.after(el); + if (callId) this._streamElIndex.set(callId, el); } el.appendChild(document.createTextNode(stripped)); - el.scrollTop = el.scrollHeight; + // rAF-coalesced inner pin: the eager scrollTop=scrollHeight after every + // text append forced one whole-page reflow per chunk (geometry read on a + // just-dirtied layout). One pin per frame is visually identical. + if (!el._pinPending) { + el._pinPending = true; + requestAnimationFrame(() => { + el._pinPending = false; + el.scrollTop = el.scrollHeight; + }); + } this.scrollToBottom(stick); } showOutputWarning(evt) { if (!evt.call_id || evt.risk_level === "none") return; - const escapedId = CSS.escape(evt.call_id); - const toolDiv = this.messagesEl.querySelector( - '.conv-row[data-call-id="' + escapedId + '"]', - ); + const toolDiv = this._toolRow(evt.call_id); if (!toolDiv) return; // Shared DOM-builder with replayHistory \u2014 single source of truth for // role / class / escape semantics. Argument shape mirrors the @@ -520,7 +549,15 @@ class Pane { updateVerdictBadge(verdict) { if (!verdict || !verdict.call_id) return; const escapedId = CSS.escape(verdict.call_id); - const badge = this.messagesEl.querySelector( + // Badges anchor either inside the row (solo verdicts, replay) or at the + // batch-block level (judge-pending panels) — scope the query to the + // row's batch, which covers both, instead of scanning the whole + // transcript per verdict event. Row-less lookups (row already replaced + // by output) fall back to the container scan so the late-verdict toast + // path keeps working. + const vRow = this._toolRow(verdict.call_id); + const vScope = (vRow && vRow.closest(".conv-batch")) || this.messagesEl; + const badge = vScope.querySelector( '.conv-verdict[data-call-id="' + escapedId + '"]', ); if (!badge) { @@ -629,18 +666,40 @@ class Pane { } isNearBottom() { - return ( - this.messagesEl.scrollHeight - - this.messagesEl.scrollTop - - this.messagesEl.clientHeight < - 80 - ); + // Cached from the passive scroll listener (_createDOM) instead of read + // from geometry: the old scrollHeight/scrollTop/clientHeight triplet + // forced a synchronous layout of the whole transcript, and this runs on + // every streamed token and every tool chunk. Content growth without a + // scroll leaves the cache untouched — which is the DESIRED semantics: + // "pinned" is a statement about where the user last scrolled to, not + // about the current pixel distance (the old post-append measurement is + // exactly what used to silently disengage auto-follow at tool time). + return this._nearBottom; } scrollToBottom(force) { - if (force || this.isNearBottom()) { - this.messagesEl.scrollTop = this.messagesEl.scrollHeight; - } + if (force) this._scrollPinForce = true; + else if (!this._nearBottom) return; + // rAF-coalesced pin: at most one scrollHeight read + scrollTop write per + // frame no matter how many deltas arrived. The pin re-checks + // _nearBottom AT FIRE TIME: a user wheel-scroll can land between the + // schedule (when the cached flag was still true) and the rAF — pinning + // anyway would yank them back to the bottom, and the programmatic + // scroll's own event would re-mark the flag true, trapping them there + // for the rest of the stream. Scroll events fire before rAF callbacks + // within a frame, so the re-check sees the user's disengage. Force + // requests latch across the coalescing window (a forced pin must win + // even if a non-forced schedule got there first). + if (this._scrollPinPending) return; + this._scrollPinPending = true; + requestAnimationFrame(() => { + this._scrollPinPending = false; + const forced = this._scrollPinForce; + this._scrollPinForce = false; + if (forced || this._nearBottom) { + this.messagesEl.scrollTop = this.messagesEl.scrollHeight; + } + }); } _createDOM() { @@ -717,6 +776,35 @@ class Pane { this.messagesEl.setAttribute("role", "log"); this.messagesEl.setAttribute("aria-live", "polite"); this.messagesEl.setAttribute("aria-label", "Chat messages"); + // Track "pinned to bottom" from actual scrolls (user or programmatic) + // instead of reading scroller geometry per event — see isNearBottom(). + // Passive: never blocks the compositor thread. + this.messagesEl.addEventListener( + "scroll", + () => { + this._nearBottom = + this.messagesEl.scrollHeight - + this.messagesEl.scrollTop - + this.messagesEl.clientHeight < + 80; + }, + { passive: true }, + ); + // Layout changes that move the bottom WITHOUT a scroll event (window + // resize, split-drag, orientation change) would leave the cached flag + // stale — a user visually back at the bottom after growing the pane + // stayed disengaged until they nudged the scroller. Resizes are rare, + // so the geometry read here is off the hot path by construction. + if (typeof ResizeObserver === "function") { + this._resizeObs = new ResizeObserver(() => { + this._nearBottom = + this.messagesEl.scrollHeight - + this.messagesEl.scrollTop - + this.messagesEl.clientHeight < + 80; + }); + this._resizeObs.observe(this.messagesEl); + } this.el.appendChild(this.messagesEl); // Per-workstream status bar (above input) @@ -885,13 +973,32 @@ class Pane { if (this.evtSource && this.evtSource.lastEventId) { this._lastEventId = this.evtSource.lastEventId; } - const data = JSON.parse(e.data); + // Guarded parse + dispatch. onmessage is the pane's whole event + // pipeline: an exception escaping it doesn't close the EventSource, so + // pre-guard a single malformed frame (or one throwing handler case) + // left the streaming refs (currentAssistantEl / contentBuffer) stale + // and every later turn painted into the poisoned segment — the + // "output stops rendering while the backend is healthy" wedge. + let data = null; + try { + data = JSON.parse(e.data); + } catch (err) { + console.warn("interactive: dropping malformed SSE frame", err); + return; + } // Tag the event with its own SSE id so the system_turn handler can // dedup a turn already painted from /history against the same turn // redelivered by an SSE replay. e.lastEventId is this event's id; // buffered events (system_turn included) always carry one. if (e.lastEventId) data._event_id = e.lastEventId; - this.handleEvent(data); + try { + this.handleEvent(data); + } catch (err) { + console.error( + "interactive: handleEvent failed for " + (data && data.type), + err, + ); + } }; this.evtSource.onerror = () => { @@ -933,6 +1040,14 @@ class Pane { // below gets the new ws's full initial state instead. this._lastEventId = null; this._lastStatusEvt = null; + // Full-reload cleanup (NOT in disconnectSSE — transport-only reconnects + // must preserve these): a stale quiesce queue would wedge the new load's + // events behind a flush that never comes, stale agent tracking points at + // the DOM this load is about to replace, and a pending truncated-resync + // is superseded by the full refetch below. + this._replayQueue = null; + this._clearAgentTracking(); + this._pendingTruncatedResync = false; // Generation token — a slow refetch (e.g. a large resumed session) must // not render its history, reconnect its stream, or fire its resend after // the pane has switched to another ws. Newest load wins; older ones drop. @@ -977,7 +1092,10 @@ class Pane { // Drop a superseded load: a newer _loadHistoryThenConnect (ws switch) // bumped the token while this fetch was in flight, so rendering now would // paint the wrong ws's history into the pane. - if (token !== undefined && token !== this._historyLoadToken) return; + if (token !== undefined && token !== this._historyLoadToken) { + this._endReplayQuiesce(token); + return; + } if (data) { // Fresh-connect fast-forward: when the trailing turn is an // executing in-flight tool batch the server can replay, /history @@ -995,17 +1113,113 @@ class Pane { // shape (server-side projection in make_history_handler: // flat tool_calls, top-level source/reminders/attachments, collapsed // content, derived denied/is_error/pending) — feed it straight to - // replayHistory. No client-side normalisation. - this.replayHistory(data.messages || []); + // replayHistory. No client-side normalisation. The quiesce release + // rides a finally so a loud replay throw (deliberately uncaught, see + // above) can't strand the event queue and wedge the pane. + try { + this.replayHistory(data.messages || []); + } finally { + this._endReplayQuiesce(token); + } } else { + // Failure path never reaches replayHistory — reset the streaming refs + // here too, or the flushed backlog and resumed live events would paint + // into the subtree clear_ui already wiped. + this._resetStreamingRefs(); this.showEmptyState(); + this._endReplayQuiesce(token); } } + _beginReplayQuiesce(token) { + // Arm the handleEvent queue for a full re-render (clear_ui / + // replay_truncated). Token-owned: a newer load's quiesce replaces this + // one wholesale — events queued before the newer snapshot was fetched + // are covered by that snapshot, so dropping them is lossless. + this._replayQueue = { token: token, events: [] }; + } + + _endReplayQuiesce(token) { + const q = this._replayQueue; + if (!q || q.token !== token) return; + this._replayQueue = null; + // Replay the backlog in arrival order. A queued clear_ui re-arms the + // quiesce mid-flush and the remainder queues behind ITS rebuild. Each + // dispatch is guarded like onmessage: one bad event must not drop the + // rest of the backlog. + for (const evt of q.events) { + try { + this.handleEvent(evt); + } catch (err) { + console.error("interactive: queued event replay failed", err); + } + } + } + + _clearAgentTracking() { + // Release task-agent bookkeeping ahead of (or after) a full rebuild. + // Entries left in _agentCards would pin every replaced card subtree as + // reachable detached DOM — unbounded growth across an hours-long + // session's rewinds/compaction re-syncs — and a stale _agentOrphans + // grace timer would escape buffered steps into the rebuilt pane. + if (this._agentCards) this._agentCards.clear(); + if (this._agentOrphans) { + for (const entry of this._agentOrphans.values()) { + if (entry.timer != null) clearTimeout(entry.timer); + } + this._agentOrphans.clear(); + } + // The row/stream lookup caches share this exact lifecycle (entries are + // DOM refs into the subtree being replaced) — drop them together. + if (this._toolRowIndex) this._toolRowIndex.clear(); + if (this._streamElIndex) this._streamElIndex.clear(); + } + + _toolRow(callId) { + // O(1) call_id → .conv-row resolution with a self-healing cache: a hit + // is validated for liveness (isConnected + id match) so a row replaced + // by the pending→resolved upgrade or a batch rebuild falls back to one + // scoped query and re-caches. The old per-event attribute-selector + // scan walked the whole transcript — O(N) per tool event. + if (!callId) return null; + let row = this._toolRowIndex.get(callId); + if (row && row.isConnected && row.dataset.callId === callId) return row; + row = this.messagesEl.querySelector( + '.conv-row[data-call-id="' + CSS.escape(callId) + '"]', + ); + if (row) this._toolRowIndex.set(callId, row); + else this._toolRowIndex.delete(callId); + return row; + } + + _streamEl(callId) { + // Same cache discipline as _toolRow for the per-tool streaming
 —
+    // resolved on every tool_output_chunk, the chattiest event in an agent
+    // session.
+    if (!callId) return null;
+    let el = this._streamElIndex.get(callId);
+    if (el && el.isConnected && el.dataset.callId === callId) return el;
+    el = this.messagesEl.querySelector(
+      '.tool-output-stream[data-call-id="' + CSS.escape(callId) + '"]',
+    );
+    if (el) this._streamElIndex.set(callId, el);
+    else this._streamElIndex.delete(callId);
+    return el;
+  }
+
   handleEvent(evt) {
     // Guard: drop events that belong to a different workstream.
     // This prevents cross-contamination during tab switches and reconnects.
     if (evt.ws_id && evt.ws_id !== this.wsId) return;
+    // While a clear_ui / replay_truncated rebuild is in flight, live events
+    // must not paint into a DOM the imminent replaceChildren() will wipe —
+    // anything painted in the [snapshot-fetch → rebuild] window is lost with
+    // no redelivery (the re-render callers never rewind _lastEventId).  Queue
+    // them; _endReplayQuiesce replays the backlog once the rebuild lands.
+    if (this._replayQueue) {
+      this._replayQueue.events.push(evt);
+      return;
+    }
     switch (evt.type) {
       case "thinking_start":
         this.isThinking = true;
@@ -1048,7 +1262,7 @@ class Pane {
         this.scrollToBottom();
         break;
 
-      case "stream_end":
+      case "stream_end": {
         if (this._cancelTimeout) {
           clearTimeout(this._cancelTimeout);
           this._cancelTimeout = null;
@@ -1057,21 +1271,32 @@ class Pane {
           clearTimeout(this._forceTimeout);
           this._forceTimeout = null;
         }
-        // Finalize the current streaming segment's markdown.  This fires
-        // per-segment (between tool calls), NOT per-turn.  Busy state is
-        // managed by state_change events instead.
-        if (this.currentAssistantBodyEl && this.contentBuffer) {
-          streamingRenderFinalize(
-            this.currentAssistantBodyEl,
-            this.contentBuffer,
-          );
-        }
+        // Reset the segment state BEFORE the finalize render, and guard the
+        // render with a plain-text fallback (mirrors coordinator.js).  With
+        // the old order a finalize throw skipped these clears, so every
+        // later content delta appended into the poisoned buffer/bubble and
+        // no new assistant segment ever painted — the permanent-wedge shape
+        // of "output stops rendering while the backend stays healthy".
+        const doneBodyEl = this.currentAssistantBodyEl;
+        const doneBuffer = this.contentBuffer;
         this.currentAssistantBodyEl = null;
         this.currentAssistantEl = null;
         this.currentReasoningEl = null;
         this.contentBuffer = "";
+        // Finalize the completed streaming segment's markdown.  This fires
+        // per-segment (between tool calls), NOT per-turn.  Busy state is
+        // managed by state_change events instead.
+        if (doneBodyEl && doneBuffer) {
+          try {
+            streamingRenderFinalize(doneBodyEl, doneBuffer);
+          } catch (err) {
+            console.warn("interactive: streamingRenderFinalize failed", err);
+            doneBodyEl.textContent = doneBuffer;
+          }
+        }
         this.scrollToBottom(true);
         break;
+      }
 
       case "in_progress_snapshot":
         // One-shot replay of the in-progress turn's reasoning + content
@@ -1120,6 +1345,16 @@ class Pane {
         if (evt.state === "idle" || evt.state === "error") {
           this.setBusy(false);
           this._attachRetryToLastAssistant();
+          // Deferred replay_truncated re-sync: the truncation arrived while
+          // a segment was streaming (refetching then would have detached the
+          // live bubble), so repair the lost-event gap now that the turn is
+          // settled and /history is complete.
+          if (this._pendingTruncatedResync) {
+            this._pendingTruncatedResync = false;
+            const rsToken = this._historyLoadToken;
+            this._beginReplayQuiesce(rsToken);
+            this._refetchHistory(this.wsId, rsToken);
+          }
           // Only steal focus if this is the active pane and no approval pending.
           if (this._host.isFocused(this) && !this.pendingApproval) {
             this.inputEl.focus();
@@ -1315,7 +1550,9 @@ class Pane {
         // the load token so a ws switch mid-flight discards both the
         // re-render and the resend (no cross-ws send).
         const token = this._historyLoadToken;
+        this._beginReplayQuiesce(token);
         this.messagesEl.replaceChildren();
+        this._resetStreamingRefs();
         this._refetchHistory(this.wsId, token)
           .then(() => {
             if (token !== this._historyLoadToken) return;
@@ -1357,9 +1594,19 @@ class Pane {
         // floor's in_progress_snapshot already paints it, and an async
         // refetch's replaceChildren() would detach the live bubble so
         // content deltas render nowhere. Re-syncs on the next clean
-        // (re)connect.
-        if (!this.currentAssistantEl)
-          this._refetchHistory(this.wsId, this._historyLoadToken);
+        // (re)connect.  The guard covers BOTH streaming targets — a
+        // reasoning-only segment (currentReasoningEl without a content
+        // bubble yet) is just as detachable as a content one.  Mid-stream
+        // the resync is DEFERRED, not dropped: skipping outright left the
+        // lost-event gap unrepaired for the rest of the session (no clean
+        // reconnect may come for hours); the idle edge consumes the flag.
+        if (!this.currentAssistantEl && !this.currentReasoningEl) {
+          const rtToken = this._historyLoadToken;
+          this._beginReplayQuiesce(rtToken);
+          this._refetchHistory(this.wsId, rtToken);
+        } else {
+          this._pendingTruncatedResync = true;
+        }
         break;
     }
   }
@@ -1980,8 +2227,30 @@ class Pane {
       });
   }
 
+  _resetStreamingRefs() {
+    // Null every ref that can point into a wiped subtree, so the next event
+    // creates fresh targets instead of writing invisibly into detached
+    // nodes.  Called wherever the transcript DOM is (or is about to be)
+    // replaced — replayHistory, the clear_ui immediate wipe, and the
+    // refetch-FAILURE path (which shows the empty state without ever
+    // reaching replayHistory; leaving refs stale there made the retried
+    // generation's whole first segment stream into a detached bubble).
+    this.currentAssistantEl = null;
+    this.currentAssistantBodyEl = null;
+    this.currentReasoningEl = null;
+    this.contentBuffer = "";
+    this.announcedBlockEl = null;
+    this._thinkingEl = null;
+    this._retryHolderEl = null;
+  }
+
   replayHistory(messages) {
     this.messagesEl.replaceChildren();
+    // The rebuild just orphaned any in-flight streaming targets — reset them,
+    // and release the agent-card/orphan maps whose entries now point at
+    // replaced subtrees (detached-DOM retention).
+    this._resetStreamingRefs();
+    this._clearAgentTracking();
     // Reset the per-pane dedup set: ids of operator-context system turns
     // already painted from /history.  A later SSE replay that redelivers one
     // (resume-cursor overlap) is skipped by the system_turn handler.
@@ -2252,9 +2521,15 @@ class Pane {
   }
 
   _attachRetryToLastAssistant() {
-    // Remove any previous retry buttons
-    const old = this.messagesEl.querySelectorAll(".msg.assistant .msg-actions");
-    for (let i = 0; i < old.length; i++) old[i].parentNode.removeChild(old[i]);
+    // Remove the previous holder's action bar via the tracked ref — the old
+    // whole-transcript ".msg.assistant .msg-actions" sweep was O(N) per
+    // busy→idle edge.  At most one assistant bar exists (this method is its
+    // only writer); a holder detached by a rebuild no-ops harmlessly.
+    if (this._retryHolderEl) {
+      const oldBar = this._retryHolderEl.querySelector(".msg-actions");
+      if (oldBar) oldBar.remove();
+      this._retryHolderEl = null;
+    }
     // Find the last assistant message with content and add retry.
     // Reasoning blocks emit as .msg.reasoning (distinct modifier) so the
     // .msg.assistant selector already excludes them — no extra guard needed.
@@ -2275,13 +2550,25 @@ class Pane {
     if (lastChild && lastChild.classList.contains("conv-batch")) {
       return;
     }
-    const assistants = this.messagesEl.querySelectorAll(".msg.assistant");
-    if (assistants.length) {
-      const lastAssistant = assistants[assistants.length - 1];
+    // Walk backwards from the tail for the last assistant bubble — the
+    // match is at (or near) the end of the transcript, so this touches a
+    // handful of siblings instead of collecting all N assistant rows.
+    let lastAssistant = this.messagesEl.lastElementChild;
+    while (
+      lastAssistant &&
+      !(
+        lastAssistant.classList.contains("msg") &&
+        lastAssistant.classList.contains("assistant")
+      )
+    ) {
+      lastAssistant = lastAssistant.previousElementSibling;
+    }
+    if (lastAssistant) {
       this._addRetryAction(lastAssistant);
       if (this._voiceRoles && this._voiceRoles.tts) {
         this._addTtsAction(lastAssistant);
       }
+      this._retryHolderEl = lastAssistant;
     }
   }
 
@@ -2565,10 +2852,9 @@ class Pane {
   }
 
   _ensureAgentCard(parentCallId) {
-    const escId = parentCallId ? CSS.escape(parentCallId) : "";
-    const parentRow = escId
-      ? this.messagesEl.querySelector('.conv-row[data-call-id="' + escId + '"]')
-      : null;
+    // _toolRow cache: a busy task agent resolves its parent row once per
+    // child event — the uncached scan was O(transcript) per step.
+    const parentRow = this._toolRow(parentCallId);
     if (!parentRow) return null;
     if (!this._agentCards) this._agentCards = new Map();
     let card = this._agentCards.get(parentCallId);
@@ -2700,17 +2986,17 @@ class Pane {
     const stick = this.isNearBottom();
     // A task_agent's OWN result completing flips its card running -> done/error
     // (child sub-tool results carry namespaced ids, never keys of _agentCards).
+    // The entry deliberately SURVIVES the result: a late child event (SSE
+    // replay overlap) re-entering _ensureAgentCard with no Map entry would
+    // build a duplicate empty card beside the finished one.  Entries hold
+    // attached DOM (not a leak); the detached-retention hazard is rebuilds,
+    // which _clearAgentTracking covers in replayHistory.
     if (this._agentCards && this._agentCards.has(callId)) {
       this._agentCards.get(callId).wrap.dataset.state = isError
         ? "error"
         : "done";
     }
-    const escapedId = callId ? CSS.escape(callId) : "";
-    let target = escapedId
-      ? this.messagesEl.querySelector(
-          '.conv-row[data-call-id="' + escapedId + '"]',
-        )
-      : null;
+    let target = this._toolRow(callId);
     if (!target) {
       // A namespaced sub-agent child id ("::") whose row hasn't
       // nested yet must NOT graft its output onto the last top-level batch row
@@ -2732,18 +3018,17 @@ class Pane {
     if (!target) return;
 
     // Remove the streaming output element for this tool
-    let streamEl = null;
-    if (escapedId) {
-      streamEl = this.messagesEl.querySelector(
-        '.tool-output-stream[data-call-id="' + escapedId + '"]',
-      );
-    } else {
+    let streamEl = this._streamEl(callId);
+    if (!streamEl) {
       const next = target.nextElementSibling;
       if (next && next.classList.contains("tool-output-stream")) {
         streamEl = next;
       }
     }
-    if (streamEl) streamEl.remove();
+    if (streamEl) {
+      streamEl.remove();
+      if (callId) this._streamElIndex.delete(callId);
+    }
 
     const stripped = stripAnsi(output || "").trim();
     if (!stripped) return;
@@ -3885,6 +4170,16 @@ function createInteractivePane(root, wsId, opts) {
         recoverTimer = null;
       }
       pane.disconnectSSE();
+      // Terminal cleanup that transport-only reconnects must NOT do (see
+      // disconnectSSE): cancel orphan grace timers so a post-destroy escape
+      // can't paint into the detached pane / shared announcer, release the
+      // card maps, and stop observing the detached scroller.
+      pane._clearAgentTracking();
+      pane._replayQueue = null;
+      if (pane._resizeObs) {
+        pane._resizeObs.disconnect();
+        pane._resizeObs = null;
+      }
       if (pane.el && pane.el.parentNode) {
         pane.el.parentNode.removeChild(pane.el);
       }
diff --git a/turnstone/shared_static/renderer.js b/turnstone/shared_static/renderer.js
index 9e45cf5a..06e28f1e 100644
--- a/turnstone/shared_static/renderer.js
+++ b/turnstone/shared_static/renderer.js
@@ -261,11 +261,31 @@ function _langToCssClass(lang) {
 // ---------------------------------------------------------------------------
 //  Main markdown renderer
 // ---------------------------------------------------------------------------
+// Hard cap on renderMarkdown re-entrancy.  Blockquote/callout/list bodies
+// recurse through renderMarkdown; a pathological input (a few KB of nested
+// "> " prefixes) would otherwise overflow the call stack mid-render — an
+// exception the streaming callers can only partially recover from.  Beyond
+// the cap the nested body renders as escaped plain text: degraded, visible.
+var _MD_MAX_DEPTH = 100;
+
 export function renderMarkdown(text) {
-  // Scope footnote IDs per top-level render call (prevents collisions across messages)
+  if (_fnDepth >= _MD_MAX_DEPTH) {
+    return "

" + escapeHtml(String(text == null ? "" : text)) + "

"; + } + // Scope footnote IDs per top-level render call (prevents collisions across + // messages). Depth accounting rides a try/finally: a throw anywhere in the + // body used to strand _fnDepth elevated, freezing _fnScopeId so footnote + // anchor ids collided across every later message. if (_fnDepth === 0) _fnScopeId++; _fnDepth++; + try { + return _renderMarkdownBody(text); + } finally { + _fnDepth--; + } +} +function _renderMarkdownBody(text) { // Pre-pass: extract blockquote blocks and recursively render. // Must run FIRST (before code/math protection) so the recursive call // processes raw markdown, not text with outer-scope placeholders. @@ -742,7 +762,6 @@ export function renderMarkdown(text) { return inlineMaths[parseInt(idx)]; }); - _fnDepth--; return result; } @@ -1119,37 +1138,72 @@ function _renderMermaidBlock(container, callback) { return; } _mermaidPending.set(source, [container]); - _mermaidRenderChain = _mermaidRenderChain.then(function () { - var pending = _mermaidPending.get(source) || []; - _mermaidPending.delete(source); - var id = "mermaid-" + ++_mermaidIdCounter; - return mermaid.render(id, source).then( - function (result) { - _cacheFifoEntry( - _mermaidSvgCache, - source, - { svg: result.svg, bindFunctions: result.bindFunctions }, - _MERMAID_CACHE_MAX, - ); - for (var i = 0; i < pending.length; i++) { - var c = pending[i]; - if (c.isConnected) { - _applyMermaidSvg(c, result.svg, result.bindFunctions); + // ``linkPending`` is hoisted to the link's closure so the rejection-proof + // .catch below can paint the error on the containers THIS link captured — + // by the time it runs, the link already removed them from _mermaidPending, + // so without the hoist they'd sit at "Loading diagram…" forever. + var linkPending = null; + _mermaidRenderChain = _mermaidRenderChain + .then(function () { + var pending = _mermaidPending.get(source) || []; + linkPending = pending; + _mermaidPending.delete(source); + var id = "mermaid-" + ++_mermaidIdCounter; + return mermaid.render(id, source).then( + function (result) { + _cacheFifoEntry( + _mermaidSvgCache, + source, + { svg: result.svg, bindFunctions: result.bindFunctions }, + _MERMAID_CACHE_MAX, + ); + for (var i = 0; i < pending.length; i++) { + var c = pending[i]; + if (c.isConnected) { + // Per-container guard: one bad apply (a bindFunctions throw) + // must not skip the remaining containers for this source. + try { + _applyMermaidSvg(c, result.svg, result.bindFunctions); + } catch (e) { + _applyMermaidError(c, source, "diagram apply failed"); + } + } } + }, + function (err) { + var orphan = document.getElementById(id); + if (orphan) orphan.remove(); + var msg = err && err.message ? err.message : "Diagram error"; + _cacheFifoEntry(_mermaidErrorCache, source, msg, _MERMAID_CACHE_MAX); + for (var i = 0; i < pending.length; i++) { + var c = pending[i]; + if (c.isConnected) _applyMermaidError(c, source, msg); + } + }, + ); + }) + .catch(function (e) { + // Rejection-proof every link: a sync throw escaping the link body + // (e.g. mermaid.render throwing on malformed input before returning a + // promise) would otherwise reject the shared chain, and every later + // diagram would silently sit at "Loading diagram…" forever. Settle + // back to fulfilled and paint the error on the containers this link + // had already claimed. Deliberately NO _mermaidPending.delete(source) + // here: the link deleted its own entry up top, and any entry present + // NOW belongs to a newer re-entry for the same source — deleting it + // would orphan THAT link's containers. + var msg = (e && e.message) || "Diagram error"; + _cacheFifoEntry(_mermaidErrorCache, source, msg, _MERMAID_CACHE_MAX); + (linkPending || []).forEach(function (c) { + if (!c.isConnected) return; + try { + _applyMermaidError(c, source, msg); + } catch (_) { + /* container-level failure — nothing left to degrade to */ } - }, - function (err) { - var orphan = document.getElementById(id); - if (orphan) orphan.remove(); - var msg = err && err.message ? err.message : "Diagram error"; - _cacheFifoEntry(_mermaidErrorCache, source, msg, _MERMAID_CACHE_MAX); - for (var i = 0; i < pending.length; i++) { - var c = pending[i]; - if (c.isConnected) _applyMermaidError(c, source, msg); - } - }, - ); - }); + }); + console.warn("renderer: mermaid render chain error", e); + }); if (callback) callback(); } @@ -1253,19 +1307,36 @@ export function reRenderAllMermaid() { // --------------------------------------------------------------------------- function _streamingRenderApply(el, buffer) { if (el._lastRenderedBuffer === buffer) return; + try { + el.innerHTML = renderMarkdown(buffer); + } catch (e) { + // A render failure must not wedge the stream: show THIS frame as plain + // text and leave the buffer UN-marked, so the next delta / the finalize + // pass re-attempts a full render (partial-input throws heal themselves + // once the closing tokens arrive). Marking before the render used to + // make an errored frame look done — the finalize short-circuit then + // pinned the stale DOM forever. + console.warn("renderer: streaming render failed; plain-text frame", e); + el.textContent = buffer; + return; + } el._lastRenderedBuffer = buffer; - var html = renderMarkdown(buffer); - el.innerHTML = html; // Progressive hljs + mermaid render — see comment above. Both are // no-ops when the element has no matching code blocks, and their // source-keyed caches avoid re-tokenizing / re-rendering for // sources we've already processed. Subsequent rAF ticks that // re-extract the same closed fence hit the cache synchronously. - if (typeof postRenderHljs === "function") { - postRenderHljs(el); - } - if (typeof postRenderMermaid === "function") { - postRenderMermaid(el); + // Guarded: decoration failures degrade to undecorated markup, never to + // a broken segment state upstream. + try { + if (typeof postRenderHljs === "function") { + postRenderHljs(el); + } + if (typeof postRenderMermaid === "function") { + postRenderMermaid(el); + } + } catch (e) { + console.warn("renderer: post-render decoration failed", e); } } diff --git a/turnstone/shared_static/toast.js b/turnstone/shared_static/toast.js index 56ed2202..d00bb521 100644 --- a/turnstone/shared_static/toast.js +++ b/turnstone/shared_static/toast.js @@ -11,6 +11,15 @@ export function showToast(message, type) { const el = document.getElementById("toast"); if (!el) return; if (_toastShowing) { + // Coalesce + cap: the queue drains at one toast per ~3.3s, so any + // sustained source (verdict toasts during an auto-approved tool storm) + // would otherwise grow it for the rest of the session and keep + // surfacing hours-stale notices. Identical consecutive messages + // collapse; beyond the cap the OLDEST queued toast drops (newest wins — + // it reflects current state). + const last = _toastQueue[_toastQueue.length - 1]; + if (last && last.message === message && last.type === type) return; + if (_toastQueue.length >= 5) _toastQueue.shift(); _toastQueue.push({ message: message, type: type }); return; } diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index c015ab9c..26fce67d 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -1527,8 +1527,34 @@ function connectGlobalSSE() { if (globalEvtSource && globalEvtSource.lastEventId) { globalLastEventId = globalEvtSource.lastEventId; } - const data = JSON.parse(e.data); - if (data.type === "ws_state") { + // Guarded parse: the cursor above 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); + } catch (err) { + console.warn("global SSE: malformed frame — resyncing roster", err); + resyncRoster(); + return; + } + if (data.type === "node_snapshot") { + // Recovery floor: the server emits this when our resume cursor + // predates its ring buffer (fresh connect, or a truncated gap after + // hidden-tab/sleep). The snapshot carries the FULL workstream + // inventory — rebuild the roster wholesale; per-ws panes re-sync + // through their own Tier-2 streams. Eviction is safe here (and only + // here): the snapshot is serialized with ws_created/ws_closed on the + // 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. + resyncRoster(); + } else if (data.type === "ws_state") { updateTabIndicator(data.ws_id, data.state, { tokens: data.tokens, context_ratio: data.context_ratio, @@ -2065,6 +2091,87 @@ document.addEventListener("keydown", function (e) { // 16. Init // =========================================================================== +// Rebuild the roster from a node_snapshot payload (workstream items keyed by +// ``id`` — the snapshot mirrors the console-collector projection, not the +// REST list's ``ws_id``). ``opts.evict``: remove roster entries missing +// from the list and close their panes. Eviction is ONLY safe for the +// in-stream node_snapshot — it is serialized with ws_created/ws_closed on +// the SSE stream, so it can't race a roster mutation. An out-of-band REST +// snapshot (resyncRoster) can be built server-side BEFORE a create whose +// ws_created the client already consumed; evicting from it would close a +// live, freshly-opened conversation. REST resyncs therefore merge only; +// missed-ws_closed ghosts heal on the next in-stream snapshot. +function applyRosterSnapshot(list, opts) { + const evict = !!(opts && opts.evict); + const seen = {}; + (list || []).forEach(function (ws) { + if (!ws || !ws.id) return; + seen[ws.id] = true; + const cur = workstreams[ws.id] || {}; + cur.name = ws.name || cur.name || ws.id.slice(0, 6); + cur.state = ws.state || cur.state || "idle"; + cur.parent_ws_id = ws.parent_ws_id || null; + cur.project_id = ws.project_id || null; + workstreams[ws.id] = cur; + }); + if (evict) { + const pm = window.TS_SHELL && window.TS_SHELL.panes; + for (const id in workstreams) { + if (!seen[id]) { + // Gap recovery can retire a session the user is LOOKING at — the + // live ws_closed (and its eviction toast) is exactly what was missed + // during the gap — so closing the pane wordlessly would yank it + // mid-read. Toast only when an open pane goes away; mass ghost-row + // cleanup in the rail stays quiet. + const wasOpen = !!(pm && pm.hasPane("interactive", id)); + const name = + (workstreams[id] && workstreams[id].name) || id.slice(0, 6); + delete workstreams[id]; + closeSessionPane(id); + if (wasOpen) showToast("Session ended: " + name); + } + } + } + fireRender(); +} + +// REST fallback for the same recovery (replay_truncated / a malformed frame +// whose cursor already advanced). MERGE-ONLY (see applyRosterSnapshot) and +// gated on r.ok — a 503 during a node restart parses as a JSON error body +// with no ``workstreams``, which must not read as an authoritative empty +// roster. In-flight latch: one resync at a time — repeated triggers during +// an outage must not stack fetches. +let _rosterResyncInflight = null; +function resyncRoster() { + if (_rosterResyncInflight) return _rosterResyncInflight; + _rosterResyncInflight = authFetch("/v1/api/workstreams") + .then(function (r) { + if (!r.ok) return null; + return r.json(); + }) + .then(function (data) { + if (!data || !data.workstreams) return; + applyRosterSnapshot( + data.workstreams.map(function (ws) { + return { + id: ws.ws_id, + name: ws.name, + state: ws.state, + parent_ws_id: ws.parent_ws_id, + project_id: ws.project_id, + }; + }), + ); + }) + .catch(function () { + /* transient — the next snapshot or reconnect heals the roster */ + }) + .finally(function () { + _rosterResyncInflight = null; + }); + return _rosterResyncInflight; +} + function initWorkstreams() { return authFetch("/v1/api/workstreams") .then(function (r) { @@ -2230,15 +2337,27 @@ window.addEventListener("popstate", function (e) { // Rail re-render fan-out — the rail subscribes via TS_APP.onRender; every // roster mutation calls fireRender() so the Workspaces section stays live. +// rAF-coalesced: the server emits ws_state at least twice per tool round for +// EVERY workstream on the node, and each subscriber repaint rebuilds the +// whole rail (replaceChildren + a listener per row) — uncoalesced, a busy +// session drove thousands of full rebuilds per hour, O(#workstreams) each. +// All subscribers are snapshot-driven repaints, so batching to one repaint +// per frame is lossless. const _renderSubs = []; +let _renderScheduled = false; function fireRender() { - for (const cb of _renderSubs) { - try { - cb(); - } catch (e) { - console.error("TS_APP render subscriber failed", e); + if (_renderScheduled) return; + _renderScheduled = true; + requestAnimationFrame(function () { + _renderScheduled = false; + for (const cb of _renderSubs) { + try { + cb(); + } catch (e) { + console.error("TS_APP render subscriber failed", e); + } } - } + }); } // Open / focus an interactive session as a pane (base="" local transport — the