From 7329b201e28f787f4901351685705deddd8d252f Mon Sep 17 00:00:00 2001 From: Eva Date: Sat, 30 May 2026 04:29:43 +0700 Subject: [PATCH] fix(openworlds): strip DM-internal leaks + animate narrating state + label action bar (Closes #335, #336, #337) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three play-surface bugs surfaced by the #324 AI playtester (run play2, newbie) on the now-reachable Table screen. All viewer-side (engine stays sole writer); no wire-contract / engine change. #335 [P1, data-integrity] DM-internal "GM Advisory" directives + bare engine tool names (e.g. "record their first memory with `remember`") leaked into the player-facing story scroll via the /chat render — the AI-DM equivalent of a system-prompt leak. Root cause: the /chat tail maps every non-player item to a narration beat with no filtering, so when the DM agent "thinks out loud" the housekeeping prose lands in the player's chronicle. - Hard guard (viewer): new sanitizeNarration() strips GM-advisory headers, the Director debt-nudge family (mirrors engine/director.py::_nudge) and bare/underscored engine-tool lines. Applied at BOTH the /chat ingestion map AND the LogEntry narration render (render-path-complete: also covers engine recentEvents). Line-oriented so a stray advisory line is removed without nuking the real prose around it; a wholly-internal beat is dropped (and no longer falsely clears the #327 pending state). - Soft guard (DM skill): a nudge in dungeon-master/SKILL.md — the advisory is for the DM, never echoed into player narration; pay the debt as fiction, then remember/add_quest silently. #336 [P1] The "DM is narrating…" state (#328) was static — 35–60s turns read as frozen. Added a LIVE elapsed-time readout (0:07 → 0:52 …, the strongest "alive" cue, works under reduced-motion since it's information), a subtle label shimmer, and a "this can take up to a minute" hint. role=status + aria-live polite announce once; the per-second counter + dots are aria-hidden so a screen reader isn't spammed. Reduced-motion stills the dots/shimmer (global token + belt-and-suspenders CSS) while keeping the timer + hint. #337 [P2] The action-bar buttons (Continue/Say/Do/Check/Save), dice (d20/d12/d8/d6), and free-text Declare were unexplained — the newbie ignored them all. Added native title= tooltips (hover + SR description, no new DOM, visible label stays the accessible name) keyed by engine action id, plus a one-line inline hint under the action bar. BrassButton now forwards an optional title. Validation: in-browser Babel transpile OK (vendored 7.29.0); sanitizeNarration logic 22/22 cases incl. the issue's verbatim leak + adversarial prose; rendered DOM confirms 0 directive/tool strings in the player chronicle while real narration survives, the narrating beat shows a ticking 0:NN timer, and all action affordances render; qa/ui_audit_health.sh --quick --axe = axe 0 across all 18 screens; no JS console errors on the real table page. --- skills/dungeon-master/SKILL.md | 2 +- viewer/openworlds/chrome.jsx | 6 +- viewer/openworlds/screen-table.jsx | 196 ++++++++++++++++++++++++----- 3 files changed, 170 insertions(+), 34 deletions(-) diff --git a/skills/dungeon-master/SKILL.md b/skills/dungeon-master/SKILL.md index f96fcab4..06a85d37 100644 --- a/skills/dungeon-master/SKILL.md +++ b/skills/dungeon-master/SKILL.md @@ -27,7 +27,7 @@ This is the heart of the experience. A "beat" is one exchange of the story. Run *(Opening a BRAND-NEW campaign? Don't drop the player mid-scene. Open with the guaranteed 4-beat **cold open** first — `get_prelude` (Arrival → Meeting → Inciting Incident → Threshold), woven in your own prose — then enter this cycle. See `reference/quest-generation.md`.)* -1. **Re-ground** — `clawdnd-engine` `get_state` (especially after a gap/compaction). When the moment touches the past ("haven't we met this NPC?", "what did we decide about the cult?"), `recall`/`recall_npc`/`recall_decisions` first so the world stays consistent — recall is fuzzy and works *within* a session too, not just across sessions. The adventure's **companion and named NPCs/villains already exist** from `start_adventure`; `get_state` to find their ids and use them — never `create_character` a second copy of a companion (the engine will reject a duplicate). **Re-check the player's voice preference** here too: `/voice-toggle` is a conversation-only setting, so after a compaction don't assume it — if they'd turned voice off, stay text-only rather than resuming `speak`. (Follow-up: persist as a `Campaign.voice_enabled` field.) **Then consult `get_campaign_director`** — it names what the campaign OWES right now: a *hook the party committed to but you never tracked* → `add_quest` it; an NPC *introduced but still silent* → give them a line; a *due consequence* → land it; a *stalled quest* → push it. Honor the **top** debt by weaving it into THIS beat — don't recite the list. (It's advisory: the engine surfaces the debt, you decide how to pay it.) **In the same breath, check the two living-story surfaces:** `present_events` for any stumble-into decisional whose moment has arrived (a devil's bargain offered, a faction's offer come due) — stage it IN-CHARACTER this beat and resolve the pick with `resolve_event`; and `check_companion_arc` for a bond that just turned or a `betrayal_warning` telling you a companion's loyalty is fracturing — **foreshadow** the break, don't spring it. These three (Director, events, arcs) are how the world keeps *moving* under the scene — see `reference/living-arcs.md`. +1. **Re-ground** — `clawdnd-engine` `get_state` (especially after a gap/compaction). When the moment touches the past ("haven't we met this NPC?", "what did we decide about the cult?"), `recall`/`recall_npc`/`recall_decisions` first so the world stays consistent — recall is fuzzy and works *within* a session too, not just across sessions. The adventure's **companion and named NPCs/villains already exist** from `start_adventure`; `get_state` to find their ids and use them — never `create_character` a second copy of a companion (the engine will reject a duplicate). **Re-check the player's voice preference** here too: `/voice-toggle` is a conversation-only setting, so after a compaction don't assume it — if they'd turned voice off, stay text-only rather than resuming `speak`. (Follow-up: persist as a `Campaign.voice_enabled` field.) **Then consult `get_campaign_director`** — it names what the campaign OWES right now: a *hook the party committed to but you never tracked* → `add_quest` it; an NPC *introduced but still silent* → give them a line; a *due consequence* → land it; a *stalled quest* → push it. Honor the **top** debt by weaving it into THIS beat — don't recite the list. (It's advisory: the engine surfaces the debt, you decide how to pay it.) **The advisory is for YOU, never the player: never echo a Director nudge, a "GM Advisory" line, or an engine tool name (`remember`, `add_quest`, …) into the player-facing narration — that's a system-prompt-style leak. Pay the debt as fiction (give the silent NPC a line; let a consequence land as an event), then `remember`/`add_quest` it silently behind the scene.** **In the same breath, check the two living-story surfaces:** `present_events` for any stumble-into decisional whose moment has arrived (a devil's bargain offered, a faction's offer come due) — stage it IN-CHARACTER this beat and resolve the pick with `resolve_event`; and `check_companion_arc` for a bond that just turned or a `betrayal_warning` telling you a companion's loyalty is fracturing — **foreshadow** the break, don't spring it. These three (Director, events, arcs) are how the world keeps *moving* under the scene — see `reference/living-arcs.md`. 2. **Narrate** — describe the scene vividly and voice it; voice each NPC in their own `voice_id`. **Honor `pacing_mode` from `get_state`:** `downtime` = let scenes breathe (social, shopping, recovery, character beats); `adventure` (default) = tension and momentum. On **first arrival at a location**, call `look_around` *before* narrating, and `get_scene` to pull any **authored beat** (read_aloud, dm_notes, check DCs) — run the author's intent in your own words: play the staged villain beat, the heartbreak line, the felt threat they wrote, rather than improvising past them. (When you're *generating* the world yourself, there's no authored scene — you are the author; see `reference/living-world.md`.) **Light up the visual layer in EVERY mode** (authored or generated): on first arrival at a location, `generate_image(kind="scene", scope=, prompt="")`; on a character's first on-screen appearance, `generate_image(kind="portrait", scope="portrait-"+, prompt="")`. The dashboard fetches `/image?scope=` (scene) and `/image?scope=portrait-` (faces). Degrades gracefully — the default `null` provider is a no-op that shows a placeholder and never blocks the beat. 3. **Companion reacts + advises — EVERY beat (the default, not a garnish).** Call `clawdnd-engine` `companion_advise(companion_id, situation=)`; it returns the companion's voice + personality + memory callbacks + a prompt. **Voice the companion's reaction and honest opinion** in their own voice — banter, worry, push-back, a plan. A companion that goes quiet is the #1 way this stops feeling like an adventure. They have goals and a past; let them show. 4. **Deliberate together** — when the party faces a real choice, let it be a *conversation*: the player weighs the companion's take, they may argue, then the player decides. Record the outcome with `record_decision(summary, options, chosen, rationale, actor_ids)` so it can be called back to later ("last time we trusted Grett…"). Big choices echo: schedule fallout with `add_consequence`. diff --git a/viewer/openworlds/chrome.jsx b/viewer/openworlds/chrome.jsx index 68f1ffb2..f79e871f 100644 --- a/viewer/openworlds/chrome.jsx +++ b/viewer/openworlds/chrome.jsx @@ -289,10 +289,12 @@ function IconPlate({ size = 56, label, framed = true, glyph, tone, children, onC ); } -function BrassButton({ children, onClick, tone, size, disabled, style, type = "button" }) { +function BrassButton({ children, onClick, tone, size, disabled, style, type = "button", title }) { const cls = ["btn", tone, size, disabled ? "disabled" : ""].filter(Boolean).join(" "); + // `title` is optional — forwarded so callers can attach a hover/affordance tooltip + // (e.g. the #337 action-bar hints) without giving every BrassButton one. return ( - ); diff --git a/viewer/openworlds/screen-table.jsx b/viewer/openworlds/screen-table.jsx index bf3ef184..79171ff3 100644 --- a/viewer/openworlds/screen-table.jsx +++ b/viewer/openworlds/screen-table.jsx @@ -1,5 +1,84 @@ /* Screen: Campaign Table — live session: scene art + party + GM narration + actions */ +// #335: hard viewer-side guard against DM-INTERNAL housekeeping leaking into the +// player-facing story scroll. The /chat tail can carry the DM agent "thinking out +// loud" — a GM-Advisory directive ("NPC introduced but hasn't spoken — record their +// first memory with `remember`") or a bare engine-tool mention. That text is the +// AI-DM equivalent of a leaked system prompt and must NEVER render in the chronicle. +// The engine stays the sole writer; this is a read/projection filter only. +// +// `sanitizeNarration(text)` returns a cleaned narration string, or "" when the WHOLE +// beat was internal (caller drops it). It is line-oriented so a single stray advisory +// line inside an otherwise-real beat is removed without nuking the prose around it. +const DM_ENGINE_TOOLS = [ + "remember", "recall", "recall_decisions", "log_event", "add_quest", + "update_decision", "record_decision", "add_consequence", "check_consequences", + "world_tick", "travel_to", "advance_time", "long_rest", "downtime", "award_xp", + "adjust_reputation", "social_check", "companion_advise", "check_companion_arc", + "lookup_lore", "recall_lore", "resolve_scene_debt", "end_combat", "start_combat", + "end_session", "begin_session", "create_character", "level_up", "set_scene", +]; +// A line whose ENTIRE content is a GM-advisory directive or a bare tool reference. +const _TOOLS_ALT = DM_ENGINE_TOOLS.join("|"); +// Header line of the right-panel Director advisory if it ever bleeds into prose. +const _GM_ADVISORY_HEADER = /^\s*(?:#{1,6}\s*)?(?:\**\s*)?GM\s+Advisory\b/i; +const _ADVISORY_SUBTITLE = /^\s*what the campaign owes the story\b/i; +// The debt-nudge family (mirrors servers/engine/director.py::_nudge) — DM-facing +// imperatives that name an engine tool / structural-debt action. +const _ADVISORY_DIRECTIVE = new RegExp( + "(?:" + + "\\b(?:has been introduced but hasn'?t spoken)\\b|" + + "\\b(?:untracked hook)\\b.*\\bcall\\b|" + + "\\bquest\\b.*\\bhas stalled\\b|" + + "\\bwas offered but never resolved\\b|" + + "\\bconsequence\\b.*\\b(?:is due|overdue)\\b|" + + "\\bstanding thread\\b.*\\b(?:overdue|world-?beat)\\b|" + + "\\b(?:record|give) (?:their|them) (?:a line|first memory)\\b|" + + // generic "…with/via/using/call " imperative naming an engine tool + "\\b(?:call|use|via|with|using)\\b[^.]{0,40}\\b(?:" + _TOOLS_ALT + ")\\b" + + ")", "i", +); +// A line that is ESSENTIALLY just an engine-tool token (optionally back-ticked, +// optionally with a trivial call signature) — e.g. "`remember`", "remember(...)". +const _BARE_TOOL_LINE = new RegExp( + "^\\s*[`'\"(]*\\s*(?:" + _TOOLS_ALT + ")\\s*(?:\\([^)]*\\))?\\s*[`'\")]*\\s*[.;:]?\\s*$", + "i", +); +function _isInternalLine(line) { + const t = (line || "").trim(); + if (!t) return false; // keep blank lines for caller's join (they're harmless) + return _GM_ADVISORY_HEADER.test(t) + || _ADVISORY_SUBTITLE.test(t) + || _ADVISORY_DIRECTIVE.test(t) + || _BARE_TOOL_LINE.test(t); +} +function sanitizeNarration(text) { + if (typeof text !== "string" || !text) return ""; + const kept = text + .split(/\r?\n/) + .filter((line) => !_isInternalLine(line)); + // Collapse the blank-line runs an excised directive may leave behind. + return kept.join("\n").replace(/\n{3,}/g, "\n\n").trim(); +} + +// #337: the quick-action buttons (Continue / Say / Do / Check / Save) and the dice buttons are +// icon+label only — a first-timer can't tell how they differ from typing free-text + Declare, so +// the #324 newbie ignored all of them. These short hints surface as native `title=` tooltips +// (hover + most screen readers expose them) so the affordance is discoverable with zero new DOM. +// Keyed by the engine action `id` (server.py build_action_model); viewer-only copy, no wire change. +const ACTION_HINTS = { + continue: "Continue — advance the scene without adding a new action of your own.", + say: "Say — speak in-character; formats your input as spoken dialogue.", + do: "Do — attempt a physical action; formats your input as something your hero does.", + check: "Check — roll a skill or ability check (Perception, Persuasion, …).", + save: "Save — make a saving throw to resist or avoid an effect.", + attack: "Attack — strike a foe; uses your action this turn.", + "bonus-action": "Bonus — take a bonus action this turn (a quick second move).", + reaction: "Reaction — respond out of turn (e.g. an opportunity attack or parry).", +}; +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 }) { const campaigns = Array.isArray(state?.campaigns) ? state.campaigns : []; const activeCampaign = @@ -90,12 +169,22 @@ function ScreenTable({ onNavigate, state, setState }) { const cPayload = await cResp.json(); const items = Array.isArray(cPayload.items) ? cPayload.items : []; if (!isCancelled() && items.length) { - const beats = items.map((it) => it.role === "player" - ? { kind: "dialog", who: "You", text: it.text } - : { kind: "narration", text: it.text }); - setChatBeats((prev) => [...prev, ...beats]); + // #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.) + // 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; @@ -307,7 +396,7 @@ function ScreenTable({ onNavigate, state, setState }) { {visibleLog.length ? visibleLog.map((entry, i) => ( )) :
No moves yet
} - {pending && } + {pending && } {/* Action bar */} @@ -318,10 +407,15 @@ function ScreenTable({ onNavigate, state, setState }) { {hero.name}
- - - - + {/* #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. */} +
+ Type freely and press Declare, or use the quick actions on the right (hover each for what it does).
setInput(e.target.value)} onKeyDown={(e) => e.key === "Enter" && sendAction()} disabled={Boolean(pending)} + 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 }} /> - {pending ? "Narrating…" : "Declare"} + {pending ? "Narrating…" : "Declare"}
@@ -416,6 +511,7 @@ function ScreenTable({ onNavigate, state, setState }) { icon={a.available ? (a.icon || "quest.scroll") : "inventory.locked"} label={a.label} 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)} onClick={() => invokeAction(a)} @@ -516,6 +612,12 @@ function ConditionRow({ icon, name, who, detail, tone }) { function LogEntry({ entry }) { const kind = entry.kind || "narration"; if (entry.kind === "narration") { + // #335: render-path-complete guard. Every narration source (the /chat tail AND + // engine recentEvents) flows through this one branch, so sanitizing here means a + // GM-advisory/tool-name leak can't reach the player regardless of which projection + // produced it. A beat that is wholly internal renders nothing. + const text = sanitizeNarration(entry.text); + if (!text) return null; return (
Chronicle - {entry.text} + {text}
); @@ -570,25 +672,49 @@ function LogEntry({ entry }) { ); } -// #327: the persistent "DM is narrating…" beat shown in the chronicle while a submitted move is -// being resolved. The DM's turn is long (minutes), so this is the difference between "the world -// is thinking" and "the app froze". Mirrors the narration LogEntry's gilt-rule styling, with a -// gentle pulsing trio of dots (honored only when reduced-motion is off). aria-live so a screen -// reader announces the wait too. -function DmNarratingBeat() { +// #327 + #336: the persistent "DM is narrating…" beat shown in the chronicle while a submitted +// move is being resolved. The DM's turn is long (35–60+s), so this is the difference between "the +// world is thinking" and "the app froze". #328 added the label + pulsing dots; #336 adds the +// missing *progress* signal so a 35–60s wait no longer reads as a freeze: +// • a LIVE elapsed-time readout (0:07 → 0:42 …) — the strongest "still alive" cue, and it keeps +// ticking even under reduced-motion (it's information, not decoration); +// • a one-line "this can take up to a minute" hint so a first-timer knows the wait is expected. +// a11y: role="status" + aria-live="polite" announces the wait ONCE on mount. The per-second +// elapsed counter and the dots are aria-hidden so a screen reader isn't spammed every tick. +// Reduced-motion: the pulsing dots + shimmer are disabled (CSS below + the global token); the +// elapsed text and hint remain, so the "is it busy?" question is still answered without motion. +function DmNarratingBeat({ since }) { + const start = typeof since === "number" ? since : Date.now(); + const [now, setNow] = React.useState(() => Date.now()); + React.useEffect(() => { + const id = window.setInterval(() => setNow(Date.now()), 1000); + return () => window.clearInterval(id); + }, []); + const secs = Math.max(0, Math.floor((now - start) / 1000)); + const mm = Math.floor(secs / 60); + const ss = String(secs % 60).padStart(2, "0"); + const elapsedLabel = `${mm}:${ss}`; return (
-
- The Dungeon Master is narrating -
); @@ -596,24 +722,32 @@ function DmNarratingBeat() { // Keyframes + reduced-motion fallback, injected once (the OpenWorlds bundle is in-browser Babel, // so a tiny self-contained