mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
refactor(ui): L-shell step 5e.1a — shared conversation.js for the dup'd helpers
Stand up shared_static/conversation.js as the deduplicated conversational-pane substrate both panes import (interactive via ./, the coordinator via /shared/ — both ES modules since 5e.0). First tenants are the byte-identical duplicates the in-file comments flagged for the step-5e lift: stripAnsi, the watch-result card builder, and the system-nudge marker. No visible change — the builders return the same DOM; each caller still appends + scrolls. - conversation.js: stripAnsi (null-safe variant), buildWatchResultCard, buildSystemNudgeMarker. - interactive.js / coordinator.js: import the three, drop their local copies, delegate appendWatchResult + the nudge marker through the shared builders. - stripAnsi unified on the coordinator's null-safe form (interactive's threw on a non-string arg); identical output for string inputs. - tests: new test_conversation_js.py pins the module; the two retry-walk guards now check the watch-result marker in conversation.js (it moved there). - refreshes interactive.js's header comment, stale since 5e.0 made the coordinator an ES module too.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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"'),
|
||||
):
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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
|
||||
// `<script type="module">`. It also publishes `window.InteractivePane` /
|
||||
// `window.createInteractivePane` so the still-classic standalone `app.js`
|
||||
// shell can read the class — safe because app.js builds panes only AFTER the
|
||||
@@ -23,6 +23,12 @@
|
||||
// sanctioned exception); pane code root-scopes to its own element.
|
||||
// ===========================================================================
|
||||
|
||||
import {
|
||||
stripAnsi,
|
||||
buildWatchResultCard,
|
||||
buildSystemNudgeMarker,
|
||||
} from "./conversation.js";
|
||||
|
||||
let _paneCounter = 0;
|
||||
|
||||
// Voice-role availability comes from /v1/api/models (stt_default_alias /
|
||||
@@ -288,17 +294,10 @@ class Pane {
|
||||
}
|
||||
|
||||
addSystemNudgeMarker() {
|
||||
// Thin .msg.user.system-nudge marker rendered as the anchor for
|
||||
// wake-driven reminder bubbles. Replaces the previously-invisible
|
||||
// synthetic empty user turn with a visible-but-subtle DOM element so
|
||||
// the bubble below it lands in the right place even when the wake
|
||||
// fires long after the user's last real message.
|
||||
// The marker DOM is shared (conversation.buildSystemNudgeMarker); the Pane
|
||||
// owns empty-state removal + placement.
|
||||
this.removeEmptyState();
|
||||
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();
|
||||
this.messagesEl.appendChild(el);
|
||||
return el;
|
||||
}
|
||||
@@ -314,7 +313,7 @@ class Pane {
|
||||
// richer `.msg.watch-result` card instead of the plain operator bubble.
|
||||
this.removeEmptyState();
|
||||
if (source === "watch_triggered" && meta && typeof meta === "object") {
|
||||
const card = _buildWatchResultBubble(meta, content || "");
|
||||
const card = buildWatchResultCard(meta, content || "");
|
||||
this.messagesEl.appendChild(card);
|
||||
this.scrollToBottom(true);
|
||||
return card;
|
||||
@@ -2787,55 +2786,6 @@ class Pane {
|
||||
}
|
||||
}
|
||||
|
||||
// Build a 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 watch body (the system
|
||||
// turn's content); ``meta`` carries the structured fields (``watch_name`` /
|
||||
// ``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 coordinator pane's
|
||||
// buildWatchResultBubble.
|
||||
function _buildWatchResultBubble(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 — 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;
|
||||
}
|
||||
|
||||
// Build a structured ``.msg.guard-finding`` card for an ``output_guard``
|
||||
// operator-context system turn. ``meta`` carries the structured finding
|
||||
// ``{flags, risk_level, annotations, redacted}``. Reuses the tool-result
|
||||
@@ -3159,16 +3109,11 @@ function _mcpErrorTitle(err) {
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Conversational rendering helpers — tool output, media embeds, MCP-error
|
||||
// cards, verdict badges, ANSI stripping. Moved here with the Pane (they are
|
||||
// used only by it); the coordinator pane carries its own copies, which the
|
||||
// step-5e base lift will reconcile.
|
||||
// cards, verdict badges. Used only by the Pane. stripAnsi + the watch-result
|
||||
// card + the system-nudge marker were lifted to the shared conversation.js
|
||||
// (step 5e.1, imported at the top); the rest reconciles with the CSS-vocabulary
|
||||
// unify (5e.2), since the coordinator carries .coord-tool-* variants of them.
|
||||
// ---------------------------------------------------------------------------
|
||||
function stripAnsi(s) {
|
||||
return s.replace(
|
||||
/\x1b(?:\[[0-9;?]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\)?|[()#][A-Za-z0-9]|.)/g,
|
||||
"",
|
||||
);
|
||||
}
|
||||
|
||||
function buildToolDiv(item) {
|
||||
const div = document.createElement("div");
|
||||
|
||||
Reference in New Issue
Block a user