fix(openworlds): DM-narration in-flight turn survives nav + free-text recovery (Closes #340, #341, #342) - #343
Merged
Merged
Conversation
… recovery (Closes #340, #341, #342) 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.
📝 WalkthroughWalkthroughThis PR refactors the live-narration state out of ChangesLive Session State & UI Resilience
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@viewer/openworlds/app.jsx`:
- Around line 145-183: pollOnce can run concurrently causing duplicate fetches
for the same chatCursor; add an in-flight guard to prevent overlapping polls. In
the pollOnce closure (and where it's invoked from onVisibility), introduce a
boolean like isPolling/inFlight that returns early if true, set it true
immediately before the fetch begins and reset it in a finally block after
processing (ensure chatCursor.current is updated while the guard is held).
Update references: pollOnce, start, stop, onVisibility, timer,
chatCursor.current, dmBeatCountRef.current and clearPending so no overlapping
requests can race on the same `since` value.
In `@viewer/openworlds/screen-table.jsx`:
- Around line 239-255: The sanitized free-text in cleanMove/rawLabel/text can
become empty (e.g., "<b></b>" or "{{}}"), but the code still posts and records
an echo; update the logic in the block around cleanMove/rawLabel/text (before
the fetch call that uses writeLane.endpoint) to detect when
window.neutralizeMarkup(String(rawLabel)) is empty or only whitespace and reject
the turn early (e.g., return or throw and do not call fetch, recordPlayerEcho,
or armPending); ensure the check references cleanMove, rawLabel, and text so
malformed/no-op moves are not sent to the server or recorded locally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7108db95-a3ef-4975-a47a-c8c8e09d9ca8
📒 Files selected for processing (2)
viewer/openworlds/app.jsxviewer/openworlds/screen-table.jsx
Comment on lines
+145
to
+183
| 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); }; |
There was a problem hiding this comment.
Prevent overlapping /chat polls from duplicating beats.
This loop can start a second pollOnce before the first fetch updates chatCursor.current. When that happens both requests query the same since value, so the chronicle can append duplicate beats and clear pending on stale narration.
Suggested fix
React.useEffect(() => {
if (!campaignId) return undefined;
let cancelled = false;
let timer = null;
+ let inFlight = false;
const pollOnce = async () => {
- if (cancelled) return;
+ if (cancelled || inFlight) return;
+ inFlight = true;
try {
const params = new URLSearchParams();
params.set("campaign", campaignId);
if (source) params.set("source", source);
if (runId) params.set("run", runId);
@@
- } catch (_e) { /* chat tail is non-critical; keep last good */ }
+ } catch (_e) { /* chat tail is non-critical; keep last good */ }
+ finally { inFlight = false; }
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/openworlds/app.jsx` around lines 145 - 183, pollOnce can run
concurrently causing duplicate fetches for the same chatCursor; add an in-flight
guard to prevent overlapping polls. In the pollOnce closure (and where it's
invoked from onVisibility), introduce a boolean like isPolling/inFlight that
returns early if true, set it true immediately before the fetch begins and reset
it in a finally block after processing (ensure chatCursor.current is updated
while the guard is held). Update references: pollOnce, start, stop,
onVisibility, timer, chatCursor.current, dmBeatCountRef.current and clearPending
so no overlapping requests can race on the same `since` value.
Comment on lines
+239
to
255
| 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); |
There was a problem hiding this comment.
Reject free-text turns that sanitize down to empty.
Inputs like <b></b> or {{}} become empty here, but the code still posts the move and records a fallback echo. That leaves the hardening path still sending malformed/no-op turns to the engine.
Suggested fix
const cleanMove = (typeof move.text === "string" && move.text)
? { ...move, text: window.neutralizeMarkup(move.text) }
: move;
+ if (typeof move.text === "string" && !cleanMove.text) {
+ toast({
+ kind: "danger",
+ title: "Nothing to send",
+ body: "Please enter plain text for your action.",
+ });
+ return;
+ }
const rawLabel = label || cleanMove.text || cleanMove.name || "declares an action";
const text = window.neutralizeMarkup(String(rawLabel)) || "declares an action";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@viewer/openworlds/screen-table.jsx` around lines 239 - 255, The sanitized
free-text in cleanMove/rawLabel/text can become empty (e.g., "<b></b>" or
"{{}}"), but the code still posts and records an echo; update the logic in the
block around cleanMove/rawLabel/text (before the fetch call that uses
writeLane.endpoint) to detect when window.neutralizeMarkup(String(rawLabel)) is
empty or only whitespace and reject the turn early (e.g., return or throw and do
not call fetch, recordPlayerEcho, or armPending); ensure the check references
cleanMove, rawLabel, and text so malformed/no-op moves are not sent to the
server or recorded locally.
100yenadmin
added a commit
that referenced
this pull request
May 30, 2026
Closes #344) (#346) #343 added a 90s 'stuck' recovery that re-enables the action bar and relabels Declare to 'Try again' when the DM stalls. But the button was wired to sendAction, which reads input.trim() — and the first submit clears the input box (setInput('')). So by the time the bar re-opens stuck, the box is empty, sendAction early-returns, and the click is a silent no-op. The #324-v2 veteran (vet1) hit exactly this: DM timed out on turn 1, 'Try again' did nothing. Fix (viewer-only; engine stays sole writer; no /move or wire-contract change): - Capture the in-flight move (already-neutralized move object + label + actionId) in a lastMoveRef on every postMove. - New retryStuck() handler: if the player typed NEW text into the re-opened box, send that (the 'or rephrase' path, which already worked); otherwise re-POST the exact stalled move. Either way postMove → armPending re-arms the narrating state + the 90s recovery / 12-min backstop timers. - The Declare button + Enter key route through onDeclareClick (pendingStuck ? retryStuck() : sendAction()), so non-stuck behavior is unchanged. The other #344 finding — Table nav still blocked from Map/Journal during narration — did NOT reproduce: headless CDP repro against the real JSX shows the nav rail fully interactive during an active pending turn (all 6 buttons enabled, pointer-events:auto, hit-test uncovered) and a clean Table→Map→Table and Table→Journal→Table round-trip with the in-flight turn preserved. #341's app-level/pending-agnostic nav holds. qa/ui_audit_health.sh --quick --axe: axe total 0 across all 17 screens. Co-authored-by: Eva <eva@100yen.org>
This was referenced May 30, 2026
fix(dm,viewer): stop DM story-craft scaffolding leaking into the player Chronicle (Closes #347)
#349
Merged
fix(openworlds): adaptive 'stuck' recovery so a slow Act-opening isn't pre-empted (Closes #348)
#351
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #340
Closes #341
Closes #342
Fixes the three #324-v2 adversarial-playtest findings (adv1). They all cluster on the DM-narration "pending" state + screen navigation. Lineage: #328 added the pending affordance + disabled action bar; #338 animated it (elapsed timer + 12-min backstop); the
/chatpoll renders DM beats into the chronicle inscreen-table.jsx.Root cause shared by all three: the pending indicator, the
/chattail (cursor + accumulated beats), and the optimistic player echo were all local toScreenTable, so navigating away unmounted them mid-turn. All fixes are viewer-side; the engine stays the sole writer — no/moveor wire-contract change.#340 — DM narration silently dropped on mid-turn nav (P1, data-integrity)
Root:
pending+ the/chatpoll + its cursor + the player echo lived inScreenTable. Navigating away (Table→Party→Map) unmounted the screen → the in-flight DM beat that landed while away was never ingested, andpendingreset tonullon return so the bar re-opened as if the turn had finished — a silent story hole.Fix: lifted the in-flight-turn state to the App via a new
useLiveSessionhook (app.jsx), passed toScreenTableas aliveSessionprop. The/chatpoll now runs app-wide regardless of which screen is mounted, so the beat always lands in the chronicle, the player echo persists across nav, and the narrating indicator clears on the turn that actually resolved it.Evidence (CDP repro of the real JSX): submit on Table → nav to Party → push a DM beat while away → return to Table. Before: player echo lost on remount, bar re-opened prematurely. After:
beatPresent=true,echoPresent=true,stillNarrating=false,inputDisabled=false— the turn completes cleanly across navigation.#341 — Nav buttons time out / unclickable during narration (P1)
Root / investigation: the nav rail + tab bar live at the app level and were never pending-gated. Exhaustive headless probing (CDP, real JSX) shows the nav buttons during pending and from the Map are: not covered (hit-test), not disabled,
pointer-events:auto, accessible-name clean+unambiguous, stable bounding box (0 unstable frames/3s), and a real click changes the screen. The one measurable continuous-motion hazard was the "narrating" dots animating viatransform: scale(87 unstable frames/3s), which keeps the chronicle in perpetual layout motion — exactly the never-settles churn an automated/assistive "is this element stable yet?" actionability wait can trip on during a long (35–60s) narration.Fix (defensive, per the issue's prescription "keep the nav rail fully interactive; only the action bar pending-gated"):
pendingstate touches only the action bar (input / Declare / dice / encounter quick-actions). Nav rail + tab bar remain entirely pending-agnostic (verified before+after).#342 — Markup input freezes the session 35s+ (P1)
Root: free-text was POSTed to the DM verbatim (
<script>…</script>,{{ }},<b>) — the DM stalled — and the raw markup rode into the chronicle echo. The only escape was the 12-min backstop, so Declare/Enter went dead and the log froze.Fix (viewer-side):
neutralizeMarkup()strips angle-bracket tags, defangs{{ … }}, collapses whitespace, and caps length before the move is sent and before the optimistic echo. (React already escapes on display — confirmed 0 raw HTML tags injected — so this is a robustness fix, not XSS.) Ordinary apostrophes/quotes/punctuation/emoji pass through untouched. Scoped to the Table's free-text box — combat & dialogue have no player-free-text→/movepath (engine-provided/structured text only), so they're intentionally not touched.stuck: the bar re-enables with a "The DM seems stuck — try again" beat + toast (the Declare button reads "Try again"), so a stalled turn never hard-freezes the session. The original 12-min hard backstop remains as a final net.Evidence:
/movebody before =<script>alert(1)</script> {{ 7*7 }} <b>bold</b>→ after =alert(1) ( 7*7 ) bold. After 92s with no DM reply: stuck beat shown,inputDisabled=false, Declare="Try again", and a retry move posts successfully.Validation (host-aware, light)
qa/ui_audit_health.sh --quick --axe→ axe total: 0 violations across all 17 screens; full health summary PASS (Chrome 148 + browser-driver-manager).qa/ui_playtest.shwas intentionally not run (the loop re-runs it; avoids the 5-hour rate-limit) — targeted repro per the task.Files changed:
viewer/openworlds/app.jsx,viewer/openworlds/screen-table.jsx(2 files).Please admin-merge on green — do not auto-merge.
Summary by CodeRabbit
Bug Fixes
New Features