From efbf7db4c2738e28fa6ab0bb9121cd703e64f249 Mon Sep 17 00:00:00 2001 From: Eva Date: Sat, 30 May 2026 05:48:00 +0700 Subject: [PATCH] fix(openworlds): DM-narration in-flight turn survives nav + free-text recovery (Closes #340, #341, #342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three #324-v2 adversarial findings cluster on the DM-narration pending state + screen navigation. The pending indicator, the /chat tail (its cursor + accumulated beats), and the optimistic player echo were ALL local to ScreenTable, so navigating away (Table→Party→Map) unmounted them mid-turn. #340 (data-integrity): a DM beat that landed while the player was on another screen was never ingested, and `pending` reset to null on return (the bar re-opened as if the turn had finished) — a silent story hole. FIX: lift the in-flight-turn state to the App (new `useLiveSession` hook in app.jsx), so the /chat poll keeps running regardless of which screen is mounted, the beat always lands in the chronicle, the player echo persists, and the narrating indicator clears on the turn that actually resolved it. ScreenTable reads/writes it via a `liveSession` prop. #341 (nav blocked during narration): the nav rail / tab bar live at the app level and were never pending-gated — verified (not covered, not disabled, stable bbox, click changes screen). The one measurable continuous-motion hazard was the "narrating" dots animating via `transform: scale` (87 unstable frames/3s), which keeps the chronicle in perpetual layout motion an automated / assistive "is it stable yet?" actionability wait can trip on during a long narration. FIX: the dots pulse is now OPACITY-ONLY (0 layout churn), and the lifted pending state touches ONLY the action bar — nav stays fully interactive. #342 (markup freeze): free-text was POSTed to the DM verbatim (it stalled 35s+) and rode into the echo as raw markup; the only escape was the 12-min backstop. FIX (viewer-side): (a) `neutralizeMarkup()` strips angle-bracket tags + defangs `{{ }}` and caps length BEFORE the move is sent or echoed (React already escapes on display); (b) a 90s recovery timeout flags a stalled turn `stuck` — the bar re-enables with a "The DM seems stuck — try again" affordance — so a stalled turn never hard-freezes the session (the 12-min backstop remains). Viewer-only; engine stays the sole writer (no /move or wire-contract change). qa/ui_audit_health.sh --quick --axe: axe 0 across all 17 screens. --- viewer/openworlds/app.jsx | 156 +++++++++++++++++++++++++++- viewer/openworlds/screen-table.jsx | 160 +++++++++++++++-------------- 2 files changed, 238 insertions(+), 78 deletions(-) diff --git a/viewer/openworlds/app.jsx b/viewer/openworlds/app.jsx index 5d29c5e7..6894141a 100644 --- a/viewer/openworlds/app.jsx +++ b/viewer/openworlds/app.jsx @@ -42,6 +42,151 @@ window.OpenWorldsA11y = window.OpenWorldsA11y || { }, }; +// #342: neutralize markup in player free-text BEFORE it is sent to the engine or echoed into the +// chronicle. The adversarial run (#324 v2) found that submitting "", "{{ }}", or +// "…" sent the raw markup straight to the DM (it stalled 35s+) and rode along in the local +// echo. React already escapes on *display* (it never renders raw HTML), so this is NOT an XSS fix — +// it is a robustness fix: a hostile/odd free-text turn must not be able to wedge the DM or the loop. +// We strip angle-bracket tags and defang template-style "{{ … }}" / "}}" runs to plain text, collapse +// whitespace, and cap absurd length — keeping ordinary apostrophes, quotes, punctuation, and emoji +// intact so a normal in-character line is untouched. Viewer-side only; the engine stays sole writer. +window.neutralizeMarkup = window.neutralizeMarkup || function neutralizeMarkup(raw) { + if (typeof raw !== "string") return ""; + let t = raw; + // Drop anything that looks like an HTML/XML tag (incl. bodies are kept as text + // once their tags are removed). Do it twice so "<>" style nesting can't leave a stray bracket. + t = t.replace(/<\/?[a-zA-Z][^>]*>/g, " ").replace(/<\/?[a-zA-Z][^>]*>/g, " "); + // Defang stray angle brackets that weren't part of a full tag. + t = t.replace(/[<>]/g, " "); + // Defang template/handlebars-style delimiters so they can't be interpreted downstream. + t = t.replace(/\{\{+/g, "(").replace(/\}\}+/g, ")"); + // Collapse whitespace runs (a pasted wall of newlines/tabs shouldn't reach the DM as-is). + t = t.replace(/\s+/g, " ").trim(); + // Hard cap — a 1000+ char dump is an attack-class input, not a turn. + if (t.length > 2000) t = t.slice(0, 2000).trim(); + return t; +}; + +// #340 + #342: the live-session "in-flight turn" state — the /chat tail, its cursor, the +// accumulated DM/player beats, the local player echo, AND the "DM is narrating…" pending +// indicator — lifted from ScreenTable to the App so it SURVIVES screen navigation. Previously +// all of this was local to ScreenTable, so navigating away (Table→Map→Party) unmounted it: the +// in-flight DM beat that landed while away was never ingested (a silent "story hole", #340) and +// the pending indicator reset to null on return (the bar re-opened as if the turn had finished). +// Owning it at the app level means the /chat poll keeps running regardless of which screen is +// mounted, the beat always lands in the log, and the narrating state clears correctly on the +// turn that actually resolved it — no matter where the player wandered. +// +// The poll is best-effort and a no-op unless a LIVE campaign is bound (mirrors the server's +// /chat gating: empty items when no chat is configured / the view isn't the live run). +const PENDING_RECOVERY_MS = 90 * 1000; // #342: re-enable the bar if the DM stalls this long… +const PENDING_BACKSTOP_MS = 12 * 60 * 1000; // …with the original hard backstop as a final net. +function useLiveSession(state) { + const campaigns = Array.isArray(state?.campaigns) ? state.campaigns : []; + const activeCampaign = + campaigns.find((c) => c.id === state?.activeCampaign) || + campaigns[0] || + {}; + const campaignId = activeCampaign.campaign_id || state?.activeCampaign || activeCampaign.id || ""; + const source = activeCampaign.source || ""; + const runId = activeCampaign.runId || ""; + + const [chatBeats, setChatBeats] = React.useState([]); + const [log, setLog] = React.useState([]); // local optimistic player echoes + const [pending, setPending] = React.useState(null); // { text, since, stuck? } | null + const chatCursor = React.useRef(0); + const dmBeatCountRef = React.useRef(0); + const recoveryTimer = React.useRef(null); + const backstopTimer = React.useRef(null); + + // sanitizeNarration lives in screen-table.jsx (loaded first); fall back to identity if absent. + const sanitize = (txt) => (typeof window.sanitizeNarration === "function" ? window.sanitizeNarration(txt) : (txt || "")); + + const clearTimers = React.useCallback(() => { + if (recoveryTimer.current) { window.clearTimeout(recoveryTimer.current); recoveryTimer.current = null; } + if (backstopTimer.current) { window.clearTimeout(backstopTimer.current); backstopTimer.current = null; } + }, []); + + const clearPending = React.useCallback(() => { clearTimers(); setPending(null); }, [clearTimers]); + + // #342: arm the narrating indicator + a SHORT recovery timeout. If a DM beat doesn't arrive within + // PENDING_RECOVERY_MS the turn is flagged `stuck` (the bar re-enables with a "try again" hint) + // instead of staying frozen until the 12-minute backstop. A real beat (below) clears it outright. + const armPending = React.useCallback((text) => { + clearTimers(); + setPending({ text, since: Date.now(), stuck: false }); + recoveryTimer.current = window.setTimeout(() => { + setPending((p) => (p ? { ...p, stuck: true } : p)); + }, PENDING_RECOVERY_MS); + backstopTimer.current = window.setTimeout(() => setPending(null), PENDING_BACKSTOP_MS); + }, [clearTimers]); + + const recordPlayerEcho = React.useCallback((who, text) => { + setLog((l) => [...l, { kind: "action", who, text }]); + }, []); + + React.useEffect(() => clearTimers, [clearTimers]); + + // When the bound live campaign changes, reset the tail so we don't bleed one run's beats into + // another (the cursor is per-file; a new run starts at 0). + React.useEffect(() => { + chatCursor.current = 0; + dmBeatCountRef.current = 0; + setChatBeats([]); + setLog([]); + clearPending(); + }, [campaignId, source, runId, clearPending]); + + // The app-level /chat poll. Visibility-aware (pauses when the tab is hidden) and best-effort. + React.useEffect(() => { + if (!campaignId) return undefined; + let cancelled = false; + let timer = null; + const pollOnce = async () => { + if (cancelled) return; + try { + const params = new URLSearchParams(); + params.set("campaign", campaignId); + if (source) params.set("source", source); + if (runId) params.set("run", runId); + params.set("since", String(chatCursor.current)); + const resp = await fetch(`/chat?${params.toString()}`, { cache: "no-store" }); + if (!resp.ok) return; + const payload = await resp.json(); + const items = Array.isArray(payload.items) ? payload.items : []; + if (!cancelled && items.length) { + const beats = items + .map((it) => { + if (it.role === "player") return { kind: "dialog", who: "You", text: it.text }; + const clean = sanitize(it.text); + return clean ? { kind: "narration", text: clean } : null; + }) + .filter(Boolean); + if (beats.length) setChatBeats((prev) => [...prev, ...beats]); + // A fresh DM narration beat means the turn resolved → clear the narrating indicator + // (and its timers). Player echoes / wholly-internal beats don't count. + if (beats.some((b) => b.kind === "narration")) { + dmBeatCountRef.current += beats.filter((b) => b.kind === "narration").length; + clearPending(); + } + } + if (!cancelled && typeof payload.next === "number") chatCursor.current = payload.next; + } catch (_e) { /* chat tail is non-critical; keep last good */ } + }; + const stop = () => { if (timer !== null) { window.clearInterval(timer); timer = null; } }; + const start = () => { if (timer === null) timer = window.setInterval(pollOnce, 4000); }; + const onVisibility = () => { + if (document.visibilityState === "visible") { pollOnce(); start(); } else { stop(); } + }; + document.addEventListener("visibilitychange", onVisibility); + onVisibility(); + return () => { cancelled = true; stop(); document.removeEventListener("visibilitychange", onVisibility); }; + }, [campaignId, source, runId, clearPending]); + + return { chatBeats, log, pending, armPending, clearPending, recordPlayerEcho }; +} +window.useLiveSession = useLiveSession; + function App() { const [state, setState] = React.useState(window.INITIAL_STATE || {}); const [screen, setScreen] = React.useState("launcher"); @@ -57,6 +202,12 @@ function App() { ? window.useTweaks(TWEAK_DEFAULTS) : [TWEAK_DEFAULTS, () => {}]); + // #340: the in-flight-turn / live-narration state lives HERE (above the screen router) so it + // survives navigation — the DM beat lands and the narrating indicator clears no matter which + // screen is mounted when the turn resolves. ScreenTable reads/writes it via the `liveSession` + // prop; the nav rail and every other screen are intentionally untouched by it. + const liveSession = useLiveSession(state); + React.useEffect(() => { document.documentElement.setAttribute("data-palette", t.palette || "warm"); }, [t.palette]); @@ -281,6 +432,7 @@ function App() { setCampMode={setCampMode} nativeState={nativeState} refreshNative={refreshNative} + liveSession={liveSession} /> @@ -358,11 +510,11 @@ function capabilityForScreen(screen, nativeState) { return null; } -function ScreenRouter({ screen, state, setState, onNavigate, campMode, setCampMode, nativeState, refreshNative }) { +function ScreenRouter({ screen, state, setState, onNavigate, campMode, setCampMode, nativeState, refreshNative, liveSession }) { switch (screen) { case "launcher": return ; case "roster": return ; - case "table": return ; + case "table": return ; case "combat": return ; case "character": return ; case "create": return ; diff --git a/viewer/openworlds/screen-table.jsx b/viewer/openworlds/screen-table.jsx index 79171ff3..dcc44e17 100644 --- a/viewer/openworlds/screen-table.jsx +++ b/viewer/openworlds/screen-table.jsx @@ -79,7 +79,7 @@ const ACTION_HINTS = { const DICE_HINT = (sides) => `Roll a d${sides} — ask the Dungeon Master to resolve a d${sides} check.`; const DECLARE_HINT = "Type what your hero does in your own words, then Declare to take the turn."; -function ScreenTable({ onNavigate, state, setState }) { +function ScreenTable({ onNavigate, state, setState, liveSession }) { const campaigns = Array.isArray(state?.campaigns) ? state.campaigns : []; const activeCampaign = campaigns.find((c) => c.id === state?.activeCampaign) || @@ -90,18 +90,15 @@ function ScreenTable({ onNavigate, state, setState }) { const [advisory, setAdvisory] = React.useState(null); const [surfaceStatus, setSurfaceStatus] = React.useState("loading"); const demoLog = []; - const [log, setLog] = React.useState([]); - const [chatBeats, setChatBeats] = React.useState([]); - const chatCursor = React.useRef(0); const [input, setInput] = React.useState(""); - // #327: a visible "DM is narrating…" affordance. The unchanged DM's opening / resolving turn - // can take many minutes; without this the play loop reads as frozen the moment a move is sent. - // `pending` holds the submitted line + when it was sent; it is set the instant /move succeeds - // and cleared when a NEW DM narration beat lands via /chat (tracked by dmBeatCount below) — or - // by a long safety timeout so a dropped beat can never wedge the bar shut forever. - const [pending, setPending] = React.useState(null); - const dmBeatCountRef = React.useRef(0); - const pendingTimer = React.useRef(null); + // #340: the in-flight-turn state (the /chat tail + accumulated beats, the local player echo, and + // the "DM is narrating…" pending indicator) is owned by the APP (useLiveSession) so it survives + // screen navigation — a DM beat that lands while the player is on another screen still gets + // ingested, and the narrating state clears on the turn that actually resolved it. ScreenTable + // reads/writes that lifted state through the `liveSession` prop. A no-op fallback keeps the + // screen renderable in isolation (e.g. a direct deep-link before the hook has bound a campaign). + const session = liveSession || { chatBeats: [], log: [], pending: null, armPending: () => {}, clearPending: () => {}, recordPlayerEcho: () => {} }; + const { chatBeats, log, pending } = session; const logRef = React.useRef(null); const inputRef = React.useRef(null); const toast = window.useToast ? window.useToast() : (() => {}); @@ -158,42 +155,11 @@ function ScreenTable({ onNavigate, state, setState }) { if (!isCancelled()) setAdvisory(advPayload?.directorAdvisory || null); } } catch (error) { /* advisory is non-critical; keep last good */ } - // Live DM narration (#242 EPIC C): tail /chat so the player sees the DM's prose beats + - // their own lines during a LIVE session — not just engine recentEvents. Best-effort; - // empty/no-op when no chat is configured (read-only view). Mirrors dashboard.html's poll. - try { - const cParams = new URLSearchParams(params); - cParams.set("since", String(chatCursor.current)); - const cResp = await fetch(`/chat?${cParams.toString()}`, { cache: "no-store" }); - if (cResp.ok) { - const cPayload = await cResp.json(); - const items = Array.isArray(cPayload.items) ? cPayload.items : []; - if (!isCancelled() && items.length) { - // #335: player dialog passes through untouched; DM narration is run through - // sanitizeNarration so a GM-advisory directive / bare engine-tool line that - // bled into the /chat stream never reaches the player's chronicle. A beat that - // sanitizes to empty (it was *entirely* internal) is dropped. - const beats = items - .map((it) => { - if (it.role === "player") return { kind: "dialog", who: "You", text: it.text }; - const clean = sanitizeNarration(it.text); - return clean ? { kind: "narration", text: clean } : null; - }) - .filter(Boolean); - if (beats.length) setChatBeats((prev) => [...prev, ...beats]); - // #327: a fresh DM narration beat means the turn resolved — clear the pending indicator - // so the action bar re-opens and the spinner stops. (Player echoes don't count; and a - // beat that was wholly internal advisory — now dropped — must not count either, else a - // leak-only turn would silently re-open the bar with no visible narration. #335) - const dmArrived = beats.some((b) => b.kind === "narration"); - if (dmArrived) { - dmBeatCountRef.current += beats.filter((b) => b.kind === "narration").length; - setPending(null); - } - } - if (!isCancelled() && typeof cPayload.next === "number") chatCursor.current = cPayload.next; - } - } catch (error) { /* chat tail is non-critical; keep last good */ } + // NOTE (#340): the live DM-narration /chat tail used to be polled HERE, but it's now owned by + // the app-level useLiveSession hook (app.jsx) so a beat that lands while the player is on + // another screen still gets ingested and the narrating indicator clears correctly. ScreenTable + // only loads its own surface + advisory; the chronicle's chat beats arrive via the `liveSession` + // prop. (Engine stays sole writer — this is purely where the read-poll lives.) }, [campaignId, activeCampaign.source, activeCampaign.runId]); React.useEffect(() => { @@ -241,34 +207,51 @@ function ScreenTable({ onNavigate, state, setState }) { if (logRef.current) logRef.current.scrollTop = logRef.current.scrollHeight; }, [visibleLog]); - // #327: arm the "DM is narrating…" pending state when a move is accepted, and a safety - // auto-clear so a missed /chat beat can't lock the bar forever (the DM turn is long but - // bounded). The /chat poll clears `pending` the instant a new DM narration beat arrives. - const armPending = (text) => { - setPending({ text, since: Date.now() }); - if (pendingTimer.current) window.clearTimeout(pendingTimer.current); - pendingTimer.current = window.setTimeout(() => setPending(null), 12 * 60 * 1000); - }; - React.useEffect(() => () => { if (pendingTimer.current) window.clearTimeout(pendingTimer.current); }, []); + // #340 + #342: arming / clearing the "DM is narrating…" pending state now lives in the app-level + // useLiveSession hook (so it survives navigation, and carries the 90s recovery + 12-min backstop). + // ScreenTable just calls into it. `pendingActive` is the gate for the action bar — a turn that the + // recovery timeout flagged `stuck` is NO LONGER pending (the bar re-opens so the player can retry). + const armPending = session.armPending; + const recordPlayerEcho = session.recordPlayerEcho; + const pendingActive = Boolean(pending && !pending.stuck); + const pendingStuck = Boolean(pending && pending.stuck); + + // #342: surface the recovery exactly once when a turn goes `stuck` so the player knows the bar + // re-opened on purpose (the DM stalled) rather than silently. + const stuckNotified = React.useRef(false); + React.useEffect(() => { + if (pendingStuck && !stuckNotified.current) { + stuckNotified.current = true; + toast({ kind: "danger", title: "The Dungeon Master seems stuck", body: "No reply came back in time — your input is re-enabled. Try again or rephrase." }); + } + if (!pending) stuckNotified.current = false; + }, [pendingStuck, pending, toast]); const postMove = async (move, label, actionId) => { const enabledAction = actionId ? enabledActionById(actionId) : null; - if (!move || !canAct || pending || (actionId && !enabledAction)) { - toast({ kind: "danger", title: "Action unavailable", body: pending ? "The Dungeon Master is still narrating — one move at a time." : readOnlyReason }); + if (!move || !canAct || pendingActive || (actionId && !enabledAction)) { + toast({ kind: "danger", title: "Action unavailable", body: pendingActive ? "The Dungeon Master is still narrating — one move at a time." : readOnlyReason }); return; } - const text = label || move.text || move.name || "declares an action"; + // #342: neutralize any markup in a free-text move (kind "do"/"say"/etc. carry the player's words + // in move.text) BEFORE it is sent to the engine OR echoed — so an injection-y turn can't choke + // the DM or ride along in the chronicle as raw markup. Structured moves (no free text) pass through. + const cleanMove = (typeof move.text === "string" && move.text) + ? { ...move, text: window.neutralizeMarkup(move.text) } + : move; + const rawLabel = label || cleanMove.text || cleanMove.name || "declares an action"; + const text = window.neutralizeMarkup(String(rawLabel)) || "declares an action"; try { const response = await fetch(writeLane.endpoint || "/move", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ ...move, campaign: surface?.campaign_id || campaignId }), + body: JSON.stringify({ ...cleanMove, campaign: surface?.campaign_id || campaignId }), }); const payload = await response.json().catch(() => ({})); if (!response.ok || payload.ok === false) { throw new Error(payload.reason || `move ${response.status}`); } - setLog((l) => [...l, { kind: "action", who: hero.name, text }]); + recordPlayerEcho(hero.name, text); armPending(text); loadSurface(); } catch (error) { @@ -277,7 +260,7 @@ function ScreenTable({ onNavigate, state, setState }) { }; const sendAction = async () => { - if (pending) return; + if (pendingActive) return; const text = input.trim(); if (!text) return; const action = actionById("do"); @@ -299,7 +282,7 @@ function ScreenTable({ onNavigate, state, setState }) { }; const invokeAction = (action) => { - if (pending) { + if (pendingActive) { toast({ kind: "danger", title: "Action unavailable", body: "The Dungeon Master is still narrating — one move at a time." }); return; } @@ -396,7 +379,8 @@ function ScreenTable({ onNavigate, state, setState }) { {visibleLog.length ? visibleLog.map((entry, i) => ( )) :
No moves yet
} - {pending && } + {pendingActive && } + {pendingStuck && } {/* Action bar */} @@ -408,10 +392,10 @@ function ScreenTable({ onNavigate, state, setState }) {
{/* #337: dice buttons explain themselves on hover — a newbie didn't know d20/d12/d8/d6 ask the DM for a check. */} - - - - + + + +
{/* #337: one-line hint under the action bar so a first-timer knows free-text + Declare is the core loop, distinct from the quick-action buttons. */}
@@ -423,12 +407,12 @@ function ScreenTable({ onNavigate, state, setState }) { value={input} onChange={(e) => setInput(e.target.value)} onKeyDown={(e) => e.key === "Enter" && sendAction()} - disabled={Boolean(pending)} + disabled={pendingActive} title={DECLARE_HINT} - placeholder={pending ? "The Dungeon Master is narrating…" : (canAct ? "Describe what your hero does..." : `Read-only: ${readOnlyReason}`)} - style={{ ...inkInput, fontFamily: "var(--f-body)", fontSize: 16, opacity: pending ? 0.6 : 1 }} + placeholder={pendingActive ? "The Dungeon Master is narrating…" : pendingStuck ? "The DM seemed stuck — try again." : (canAct ? "Describe what your hero does..." : `Read-only: ${readOnlyReason}`)} + style={{ ...inkInput, fontFamily: "var(--f-body)", fontSize: 16, opacity: pendingActive ? 0.6 : 1 }} /> - {pending ? "Narrating…" : "Declare"} + {pendingActive ? "Narrating…" : pendingStuck ? "Try again" : "Declare"}
@@ -513,7 +497,7 @@ function ScreenTable({ onNavigate, state, setState }) { detail={a.available ? a.groupLabel : a.disabled_reason} hint={ACTION_HINTS[a.id]} tone={a.available ? (a.group === "combat" ? "royal" : "") : "crimson"} - disabled={!a.available || Boolean(pending)} + disabled={!a.available || pendingActive} onClick={() => invokeAction(a)} /> ))} @@ -724,12 +708,19 @@ function DmNarratingBeat({ since }) { // so a tiny self-contained