diff --git a/sdk/typescript/src/events.ts b/sdk/typescript/src/events.ts index abb60797..dbb65b89 100644 --- a/sdk/typescript/src/events.ts +++ b/sdk/typescript/src/events.ts @@ -26,7 +26,7 @@ export interface HistoryEvent { * (e.g. `watch_triggered`'s `{watch_name, command, poll_count, max_polls, * is_final}`) so the renderer can rebuild per-kind UI (the watch-result * card); absent for kinds with no structured data - * - `attachments`: per-attachment metadata `{kind, filename, mime_type, size_bytes}` + * - `attachments`: per-attachment metadata `{kind, filename, mime_type}` * - `reasoning`: concatenated reasoning text for assistant turns that * round-tripped a thinking-block lane (Anthropic-with-thinking today; * OpenAI Responses + Gemini in later phases). Present only when the diff --git a/tests/test_app_js.py b/tests/test_app_js.py index b3a82537..928997fb 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -279,6 +279,34 @@ def test_replay_renders_system_turn_via_add_system_context() -> None: ) +def test_retry_walk_skips_operator_context_cards() -> None: + """Interactive twin of the coord retry-skip guard. + ``_attachRetryToLastAssistant`` walks back past ``.operator-context`` rows + before testing for ``.ts-approval`` — so a watch-result / guard-finding + card (or a plain system bubble) trailing a tool-only turn doesn't make retry + attach to a stale earlier assistant turn. Pin the walk predicate (scoped to + the method) AND the shared marker on every operator row that can trail a + tool batch, so adding a card kind without the marker fails loudly here.""" + body = _APP_JS.read_text(encoding="utf-8") + start = _pane_method_offset(body, "_attachRetryToLastAssistant") + end = _pane_method_offset(body, "announceToolBlock") + fn = body[start:end] + assert 'classList.contains("operator-context")' in fn, ( + "_attachRetryToLastAssistant must walk back past .operator-context " + "rows so the tool-only retry skip fires even when a card trails." + ) + # Every operator row that can trail a tool batch carries the shared marker. + for cls in ( + '"msg system-context operator-context"', + '"msg watch-result operator-context"', + '"msg guard-finding operator-context"', + ): + assert cls in body, ( + f"operator row className {cls} must carry the operator-context " + "marker or the retry walk won't skip it." + ) + + # --------------------------------------------------------------------------- # Phase 8 — Chunk D: MCP error embed + settings panel UX # --------------------------------------------------------------------------- diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index aaf2b315..8c0ee7f6 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -335,13 +335,46 @@ def test_coord_history_renders_system_turn_via_msg_variants(): assert 'role === "system"' in body, ( "coord history loop must have a system-role branch for first-class operator-context turns." ) - # The ``system`` _MSG_VARIANTS entry gives the bubble operator styling. - assert 'system: "system-context"' in body, ( - "coordinator.js must map the system role to the .msg.system-context " - "variant so operator-context turns get the operator styling." + # The ``system`` _MSG_VARIANTS entry gives the bubble operator styling and + # tags it with the shared ``operator-context`` marker (so the retry-skip + # walk steps over it — see test_coord_retry_walk_skips_operator_context_cards). + assert 'system: "system-context operator-context"' in body, ( + "coordinator.js must map the system role to the " + "'system-context operator-context' variant so operator-context turns " + "get the operator styling AND carry the retry-skip marker." ) +def test_coord_retry_walk_skips_operator_context_cards(): + """Retry must NOT regenerate a stale assistant turn when the last DOM row is + a tool batch trailed by an operator-context row. ``_refreshRetryButton`` + walks back past ``.operator-context`` rows before testing for + ``.coord-tool-batch`` — which only works if EVERY operator row carries the + shared marker. Pin the walk predicate AND the marker on each structured + card so a new card kind (or a walk keyed on a single class) can't silently + re-introduce the wrong-turn retry regression.""" + from pathlib import Path + + coord_js = Path(__file__).resolve().parent.parent / ( + "turnstone/console/static/coordinator/coordinator.js" + ) + body = coord_js.read_text(encoding="utf-8") + + assert 'classList.contains("operator-context")' in body, ( + "_refreshRetryButton must walk back past .operator-context rows so the " + "tool-only retry skip fires even when a card trails the tool batch." + ) + for builder, cls in ( + ("appendWatchResult", '"msg watch-result operator-context"'), + ("appendGuardFinding", '"msg guard-finding operator-context"'), + ("appendIdleChildren", '"msg idle-children operator-context"'), + ): + assert cls in body, ( + f"{builder} must tag its card with the shared operator-context " + f"marker ({cls}) or the retry walk won't skip it." + ) + + def test_coordinator_js_seeds_resume_cursor_only_on_initial_connect(): """coordinator.js must consume the /history resume cursor the same way ui/static/app.js does: the shared make_history_handler trims the diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index 77d20cd7..7a7e5b09 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -356,7 +356,10 @@ // `.msg.system-context` rule (turnstone/shared_static/chat.css). The // generic history-replay branch already renders unknown roles via // appendText("system", …); this variant gives it the operator styling. - system: "system-context", + // The `operator-context` marker is shared by every operator row (this + // bubble + the watch-result / guard-finding / idle-children cards) so the + // retry-skip walk in _refreshRetryButton can skip them all uniformly. + system: "system-context operator-context", }; function appendMsg(role, html, opts) { @@ -402,7 +405,7 @@ // _buildWatchResultBubble. function appendWatchResult(meta, content) { const el = document.createElement("div"); - el.className = "msg watch-result"; + el.className = "msg watch-result operator-context"; el.setAttribute("role", "article"); el.setAttribute("data-ts-role", "watch"); el.setAttribute("aria-label", "watch"); @@ -452,7 +455,7 @@ // _buildGuardFindingBubble. function appendGuardFinding(meta) { const el = document.createElement("div"); - el.className = "msg guard-finding"; + el.className = "msg guard-finding operator-context"; el.setAttribute("role", "article"); el.setAttribute("data-ts-role", "output_guard"); el.setAttribute("aria-label", "output guard"); @@ -494,7 +497,7 @@ // producer); rendered via textContent so a hostile workstream name is inert. function appendIdleChildren(meta) { const el = document.createElement("div"); - el.className = "msg idle-children"; + el.className = "msg idle-children operator-context"; el.setAttribute("role", "article"); el.setAttribute("data-ts-role", "idle_children"); el.setAttribute("aria-label", "idle children"); @@ -535,7 +538,7 @@ // "queued message" bubble for a ``user_interjection`` system turn — shows the // user's raw words (``meta.message``) rather than the model-directed framing // baked into ``content``, with brighter emphasis for ``!!!``-important - // interjections. Reuses ``appendText`` (→ ``.msg.system-context``) + a class. + // interjections. Reuses ``appendText`` and adds ``.important`` for ``!!!`` ones. function appendInterjection(meta, content) { const important = meta && meta.priority === "important"; const text = @@ -543,7 +546,6 @@ const el = appendText("system", text, { label: important ? "queued message · important" : "queued message", }); - el.classList.add("interjection"); if (important) el.classList.add("important"); return el; } @@ -4439,13 +4441,16 @@ const old = messagesEl.querySelectorAll(".msg.assistant .msg-actions"); for (let i = 0; i < old.length; i++) old[i].parentNode.removeChild(old[i]); // Skip retry when the most recent semantic turn is tool-only (last DOM - // child is a .coord-tool-batch construct); walk back past .system-context - // operator bubbles first so the guard still fires when the tool turn - // carried a nudge / guard finding. (Coord's batch class is + // child is a .coord-tool-batch construct); walk back past operator-context + // rows first — the plain system bubble AND the structured watch-result / + // guard-finding / idle-children cards all carry .operator-context — so the + // guard still fires when the tool turn carried a nudge / guard finding. + // Keying on the shared marker (not any single card class) keeps the skip + // correct as new card kinds are added. (Coord's batch class is // .coord-tool-batch — the interactive pane uses .ts-approval, which does // not exist in coord's DOM.) let lastChild = messagesEl.lastElementChild; - while (lastChild && lastChild.classList.contains("system-context")) { + while (lastChild && lastChild.classList.contains("operator-context")) { lastChild = lastChild.previousElementSibling; } if (lastChild && lastChild.classList.contains("coord-tool-batch")) { diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index df391398..983ae18c 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -3731,6 +3731,8 @@ def make_attachment_handlers(cfg: SessionEndpointConfig) -> AttachmentHandlers: return JSONResponse({"attachments": rows}) async def get_content(request: Request) -> Response: + import asyncio + from starlette.responses import Response as _Response from turnstone.core.attachment_buffer import get_attachment_buffer @@ -3762,8 +3764,14 @@ def make_attachment_handlers(cfg: SessionEndpointConfig) -> AttachmentHandlers: stored_mime = staged.mime_type or "application/octet-stream" filename = staged.filename or "attachment" else: - row = get_attachment(attachment_id) - if not row or not attachment_referenced_in_ws(attachment_id, ws_id): + # Both committed-blob gates are sync DB I/O — the ref check is an + # unbounded ws-scoped LIKE scan (O(turns-in-ws)) run on every + # committed-image request, so keep it off the event loop. Matches + # the asyncio.to_thread convention used throughout this module. + row = await asyncio.to_thread(get_attachment, attachment_id) + if not row or not await asyncio.to_thread( + attachment_referenced_in_ws, attachment_id, ws_id + ): return JSONResponse({"error": "Not found"}, status_code=404) body = row.get("content") or b"" kind = row.get("kind") or "" diff --git a/turnstone/sdk/events.py b/turnstone/sdk/events.py index ee78c7fd..ed6e5640 100644 --- a/turnstone/sdk/events.py +++ b/turnstone/sdk/events.py @@ -67,6 +67,8 @@ class HistoryEvent(ServerEvent): poll_count, max_polls, is_final}``) so the renderer can rebuild per-kind UI (the watch-result card); absent when the kind carries no structured data + * ``attachments`` — per-attachment metadata ``{kind, filename, + mime_type}`` (turns that carried image / document refs) * ``reasoning`` — concatenated reasoning text for assistant turns that round-tripped a thinking-block lane (Anthropic-with-thinking today; OpenAI Responses + Gemini in later phases). Present only diff --git a/turnstone/shared_static/chat.css b/turnstone/shared_static/chat.css index a2f2dea8..c4fafea9 100644 --- a/turnstone/shared_static/chat.css +++ b/turnstone/shared_static/chat.css @@ -1005,7 +1005,9 @@ trajectory) — the consolidation of the metacognition reminder / user-interjection / output-guard bubbles into one turn type that FOLLOWS the turn it advises. Shared so the interactive UI and the console coord - viewer render it identically. Yellow accent reads as "operator metadata" + viewer render the bubble identically (the visible kind label is + interactive-only — see ``.msg-system-context-label`` below). Yellow accent + reads as "operator metadata" against the amber user colour and the cyan tool cards; deliberately quieter so it doesn't compete for attention. The ``watch_triggered`` kind carries structured per-kind meta and branches into the richer ``.msg.watch-result`` @@ -1017,6 +1019,12 @@ padding: 6px 10px; white-space: pre-wrap; } +/* The visible kind label is built ONLY by the interactive pane + (app.js ``addSystemContext`` → a ``.msg-system-context-label`` span). The + coord viewer (coordinator.js ``appendMsg``) renders the kind via + border-colour + ``data-ts-role``/``aria-label`` with no visible label for + ANY role, so this rule (and its ``.important`` variant below) is inert on the + coord pane by design — not dead CSS. */ .msg.system-context .msg-system-context-label { color: var(--yellow); font-weight: 600; diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 9fa4b032..a5ac5e05 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -273,7 +273,8 @@ class Pane { source === "user_interjection" && meta && typeof meta === "object"; const important = isInterjection && meta.priority === "important"; const el = document.createElement("div"); - el.className = "msg system-context" + (important ? " important" : ""); + el.className = + "msg system-context operator-context" + (important ? " important" : ""); const body = document.createElement("div"); body.className = "msg-body"; const labelEl = document.createElement("span"); @@ -2211,14 +2212,16 @@ class Pane { // .msg.assistant selector already excludes them — no extra guard needed. // // Skip retry attachment when the most recent semantic turn is - // tool-only — last DOM child is a .ts-approval block. Walk back - // past .system-context bubbles (operator-context system turns that - // FOLLOW the tool batch they advise) so the guard fires correctly - // even when the tool turn carried a nudge / guard finding. Without - // this skip, retry lands on a stale prior assistant content bubble - // belonging to an earlier turn. + // tool-only — last DOM child is a .ts-approval block. Walk back past + // operator-context rows (the plain system bubble AND the structured + // watch-result / guard-finding cards — every operator row carries + // .operator-context) which FOLLOW the tool batch they advise, so the + // guard fires even when the tool turn carried a nudge / guard finding. + // Keying on the shared marker (not any single card class) keeps the skip + // correct as new card kinds are added. Without it, retry lands on a + // stale prior assistant content bubble belonging to an earlier turn. let lastChild = this.messagesEl.lastElementChild; - while (lastChild && lastChild.classList.contains("system-context")) { + while (lastChild && lastChild.classList.contains("operator-context")) { lastChild = lastChild.previousElementSibling; } if (lastChild && lastChild.classList.contains("ts-approval")) { @@ -2758,7 +2761,7 @@ class Pane { // buildWatchResultBubble. function _buildWatchResultBubble(meta, content) { const el = document.createElement("div"); - el.className = "msg watch-result"; + el.className = "msg watch-result operator-context"; el.setAttribute("role", "article"); el.setAttribute("data-ts-role", "watch"); el.setAttribute("aria-label", "watch"); @@ -2805,7 +2808,7 @@ function _buildWatchResultBubble(meta, content) { // tool chip omits to stay terse. All text via textContent. function _buildGuardFindingBubble(meta) { const el = document.createElement("div"); - el.className = "msg guard-finding"; + el.className = "msg guard-finding operator-context"; el.setAttribute("role", "article"); el.setAttribute("data-ts-role", "output_guard"); el.setAttribute("aria-label", "output guard");