diff --git a/tests/test_app_js.py b/tests/test_app_js.py index ef8b8a67..2b56bf41 100644 --- a/tests/test_app_js.py +++ b/tests/test_app_js.py @@ -16,10 +16,7 @@ from pathlib import Path import pytest _APP_JS = Path(__file__).resolve().parent.parent / "turnstone/ui/static/app.js" -_INTERACTIVE_JS = ( - Path(__file__).resolve().parent.parent - / "turnstone/shared_static/interactive.js" -) +_INTERACTIVE_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/interactive.js" def _pane_method_offset(body: str, name: str) -> int: @@ -314,9 +311,14 @@ def test_retry_walk_skips_operator_context_cards() -> None: "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. + # The watch-result card moved to the shared conversation.js (step 5e.1); the + # plain system-context + guard-finding cards stay in the pane. + shared = (_INTERACTIVE_JS.parent / "conversation.js").read_text(encoding="utf-8") + assert '"msg watch-result operator-context"' in shared, ( + "buildWatchResultCard must carry the operator-context marker." + ) for cls in ( '"msg system-context operator-context"', - '"msg watch-result operator-context"', '"msg guard-finding operator-context"', ): assert cls in body, ( @@ -421,6 +423,7 @@ def test_phase8_mcp_error_helpers_defined() -> None: "STANDALONE_HOST must wire host.onConsentDetected -> _onConsentDetected" ) + def test_phase8_settings_panel_handlers_defined() -> None: """The settings modal exposes four entry points that the inline ``onclick`` attributes in index.html depend on. Renaming or diff --git a/tests/test_conversation_js.py b/tests/test_conversation_js.py new file mode 100644 index 00000000..13b73c46 --- /dev/null +++ b/tests/test_conversation_js.py @@ -0,0 +1,67 @@ +"""Guards for the shared conversational-pane module +(``turnstone/shared_static/conversation.js``). + +Born in step 5e.1: the deduplicated substrate BOTH the interactive pane +(shared_static/interactive.js) and the coordinator pane +(console/static/coordinator/coordinator.js) import. These pin the exports plus +the load-bearing invariants (operator-context marker, null-safe ANSI strip, no +innerHTML) so a regression in the shared module fails loudly here rather than +silently in one pane. +""" + +from __future__ import annotations + +from pathlib import Path + +_CONVERSATION_JS = ( + Path(__file__).resolve().parent.parent / "turnstone/shared_static/conversation.js" +) + + +def _body() -> str: + return _CONVERSATION_JS.read_text(encoding="utf-8") + + +def test_exports_the_shared_helpers() -> None: + """The three helpers both panes import must be exported — drop one and the + importing pane module fails to load entirely.""" + body = _body() + for name in ("stripAnsi", "buildWatchResultCard", "buildSystemNudgeMarker"): + assert f"export function {name}" in body, f"{name} must be exported" + + +def test_strip_ansi_is_null_safe() -> None: + """Unified on the coordinator's null-safe variant: a non-string argument + coerces to "" rather than throwing (interactive's old copy did not guard, + so this is a strict-superset behaviour for its call sites).""" + body = _body() + assert 'String(s == null ? "" : s).replace(' in body, ( + "stripAnsi must coerce its argument before .replace" + ) + + +def test_watch_card_carries_operator_context_marker() -> None: + """The watch-result card keeps the shared ``operator-context`` marker (the + retry-walk in both panes skips rows carrying it) and stays textContent-only.""" + body = _body() + assert '"msg watch-result operator-context"' in body + assert 'setAttribute("data-ts-role", "watch")' in body + for part in ( + "msg-watch-header", + "msg-watch-cmd", + "msg-watch-body", + "msg-watch-footer", + ): + assert part in body, f"watch card missing {part}" + + +def test_nudge_marker_shape() -> None: + body = _body() + assert '"msg user system-nudge"' in body + assert 'setAttribute("data-source", "system_nudge")' in body + + +def test_no_inner_html() -> None: + """House style: programmatic DOM only — no innerHTML *usage* in the shared + module (the header comment names it; guard the access pattern).""" + assert ".innerHTML" not in _body() diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py index 63a8e7b3..f13e1a43 100644 --- a/tests/test_coordinator_page.py +++ b/tests/test_coordinator_page.py @@ -390,8 +390,15 @@ def test_coord_retry_walk_skips_operator_context_cards(): "_refreshRetryButton must walk back past .operator-context rows so the " "tool-only retry skip fires even when a card trails the tool batch." ) + # The watch-result card moved to the shared conversation.js (step 5e.1); the + # guard-finding + idle-children cards stay in the coordinator pane. + shared = ( + Path(__file__).resolve().parent.parent / "turnstone/shared_static/conversation.js" + ).read_text(encoding="utf-8") + assert '"msg watch-result operator-context"' in shared, ( + "buildWatchResultCard must tag its card with the operator-context marker." + ) for builder, cls in ( - ("appendWatchResult", '"msg watch-result operator-context"'), ("appendGuardFinding", '"msg guard-finding operator-context"'), ("appendIdleChildren", '"msg idle-children operator-context"'), ): diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js index 15c3121d..c41a2daa 100644 --- a/turnstone/console/static/coordinator/coordinator.js +++ b/turnstone/console/static/coordinator/coordinator.js @@ -26,6 +26,12 @@ // bits a pane doesn't want: the "Console" back-link, the theme toggle, and the // shared #toast (the console shell already provides theme + toast). // --------------------------------------------------------------------------- +import { + stripAnsi, + buildWatchResultCard, + buildSystemNudgeMarker, +} from "/shared/conversation.js"; + function buildCoordChrome(root, opts) { opts = opts || {}; root.classList.add("coord-chrome-root"); @@ -484,18 +490,6 @@ function createCoordinatorPane(root, wsId, opts) { .replace(/'/g, "'"); } - // ANSI-escape stripper — mirrors ui/static/app.js so a tool that - // emits CSI sequences (rare on the coord tool surface, but bash - // through MCP / the underlying child node still can) lands as - // readable text inside the tool-batch result block. - function stripAnsi(s) { - return String(s == null ? "" : s).replace( - // eslint-disable-next-line no-control-regex - /\x1b(?:\[[0-9;?]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g, - "", - ); - } - // Post-process tool-output JSON: wrap known ws_id + node_id pairs in a // link pointing at /node/{node_id}/?ws_id={child_ws_id}. Only applies // when BOTH keys are present and look like valid hex ids. @@ -660,46 +654,10 @@ function createCoordinatorPane(root, wsId, opts) { // ``command`` / ``poll_count`` / ``max_polls`` / ``is_final``) delivered live // on the ``system_turn`` SSE event and on the ``/history`` projection. All // text goes through ``textContent`` so shell output containing angle brackets - // / scripts / steering bytes renders inertly. Mirrors the interactive pane's - // _buildWatchResultBubble. + // / scripts / steering bytes renders inertly. Delegates to the shared + // conversation.buildWatchResultCard; this wrapper appends + scrolls. function appendWatchResult(meta, content) { - const el = document.createElement("div"); - el.className = "msg watch-result operator-context"; - el.setAttribute("role", "article"); - el.setAttribute("data-ts-role", "watch"); - el.setAttribute("aria-label", "watch"); - const header = document.createElement("div"); - header.className = "msg-watch-header"; - header.textContent = - "watch" + (meta.watch_name ? " · " + String(meta.watch_name) : ""); - el.appendChild(header); - if (meta.command) { - const cmd = document.createElement("div"); - cmd.className = "msg-watch-cmd"; - cmd.textContent = "$ " + String(meta.command); - el.appendChild(cmd); - } - const body = document.createElement("pre"); - body.className = "msg-watch-body"; - // Prefer the structured ``output`` (raw shell output alone) so the body - // doesn't re-print the header / ``$ command`` lines the chrome already - // shows; fall back to the full turn ``content`` for legacy turns that - // predate the ``output`` meta field (migration 060 is additive). - body.textContent = - meta.output != null ? String(meta.output) : content || ""; - el.appendChild(body); - if (meta.poll_count != null && meta.max_polls != null) { - const footer = document.createElement("div"); - footer.className = "msg-watch-footer"; - const finalSuffix = meta.is_final ? " · final" : ""; - footer.textContent = - "poll " + - String(meta.poll_count) + - "/" + - String(meta.max_polls) + - finalSuffix; - el.appendChild(footer); - } + const el = buildWatchResultCard(meta, content); messagesEl.appendChild(el); _scheduleScroll(); return el; @@ -871,11 +829,7 @@ function createCoordinatorPane(root, wsId, opts) { // carried are now first-class operator-context ``system`` turns that // follow it and render via the ``system`` ``_MSG_VARIANTS`` styling. function appendSystemNudgeMarker() { - const el = document.createElement("div"); - el.className = "msg user system-nudge"; - el.setAttribute("data-source", "system_nudge"); - el.setAttribute("aria-label", "system nudge"); - el.textContent = "system nudge"; + const el = buildSystemNudgeMarker(); messagesEl.appendChild(el); return el; } diff --git a/turnstone/shared_static/conversation.js b/turnstone/shared_static/conversation.js new file mode 100644 index 00000000..922d8344 --- /dev/null +++ b/turnstone/shared_static/conversation.js @@ -0,0 +1,85 @@ +// conversation.js — shared conversational-pane substrate (step 5e). +// +// Deduplicated helpers used by BOTH the interactive pane +// (shared_static/interactive.js) and the coordinator pane +// (console/static/coordinator/coordinator.js). Both panes are ES modules +// (interactive since 5a, the coordinator since 5e.0), so this is a plain ESM +// module they import directly — interactive via `./conversation.js`, the +// coordinator via the absolute `/shared/conversation.js`. No window bridge: +// nothing classic consumes these (the standalone ui/static app.js drives the +// interactive Pane, which imports them on its behalf). +// +// House style holds: programmatic DOM (createElement / textContent / append), +// never innerHTML. Builders return a detached element and let the caller append +// + scroll, so they stay pane- and transport-agnostic. + +// ANSI / CSI escape stripper — a tool that emits control sequences (bash through +// MCP, or a child node) must land as readable text in the result block. +// Null-safe: a non-string argument coerces to "" rather than throwing. +export function stripAnsi(s) { + return String(s == null ? "" : s).replace( + // eslint-disable-next-line no-control-regex + /\x1b(?:\[[0-9;?]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g, + "", + ); +} + +// Structured `.msg.watch-result` card for a `watch_triggered` operator-context +// system turn — command-preview header + shell-output body + poll-counter footer. +// `content` is the formatted turn body; `meta` carries the structured fields +// (watch_name / command / output / poll_count / max_polls / is_final) delivered +// live on the `system_turn` SSE event and on the `/history` projection. All text +// goes through textContent so shell output containing angle brackets / scripts / +// steering bytes renders inertly. The caller appends + scrolls. +export function buildWatchResultCard(meta, content) { + const el = document.createElement("div"); + el.className = "msg watch-result operator-context"; + el.setAttribute("role", "article"); + el.setAttribute("data-ts-role", "watch"); + el.setAttribute("aria-label", "watch"); + const header = document.createElement("div"); + header.className = "msg-watch-header"; + header.textContent = + "watch" + (meta.watch_name ? " · " + String(meta.watch_name) : ""); + el.appendChild(header); + if (meta.command) { + const cmd = document.createElement("div"); + cmd.className = "msg-watch-cmd"; + cmd.textContent = "$ " + String(meta.command); + el.appendChild(cmd); + } + const body = document.createElement("pre"); + body.className = "msg-watch-body"; + // Prefer the structured `output` (raw shell output alone) so the body doesn't + // re-print the header / `$ command` lines the chrome already shows; fall back to + // the full turn `content` for legacy turns predating the `output` meta field + // (migration 060 is additive — no backfill). + body.textContent = meta.output != null ? String(meta.output) : content || ""; + el.appendChild(body); + if (meta.poll_count != null && meta.max_polls != null) { + const footer = document.createElement("div"); + footer.className = "msg-watch-footer"; + const finalSuffix = meta.is_final ? " · final" : ""; + footer.textContent = + "poll " + + String(meta.poll_count) + + "/" + + String(meta.max_polls) + + finalSuffix; + el.appendChild(footer); + } + return el; +} + +// Thin `.msg.user.system-nudge` marker — the visible-but-subtle anchor a +// wake-driven empty user turn renders, so the operator-context `system` turns +// that follow it land in the right place. The caller handles empty-state +// removal + append. +export function buildSystemNudgeMarker() { + const el = document.createElement("div"); + el.className = "msg user system-nudge"; + el.setAttribute("data-source", "system_nudge"); + el.setAttribute("aria-label", "system nudge"); + el.textContent = "system nudge"; + return el; +} diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js index 110e8fdc..a66b65ad 100644 --- a/turnstone/shared_static/interactive.js +++ b/turnstone/shared_static/interactive.js @@ -10,10 +10,10 @@ // `opts.base` URL prefix: "" locally, "/node/{id}" when the console proxies a // session living on a cluster node (the LOCALITY invariant). // -// ES module — the first legacy pane lifted into a real module (coordinator.js -// stays classic; the shared substrate it leans on — composer/renderer/etc. — -// does too, as those still have classic consumers). The console shell.js -// imports the factory directly; the standalone loads it as +// ES module — the first legacy pane lifted into a real module (step 5a; the +// coordinator pane followed in 5e.0). The shared substrate it leans on — +// composer/renderer/etc. — is still classic, consumed as globals. The console +// shell.js imports the factory directly; the standalone loads it as // `