fix(reliability): recover a stalled-mid-stream DM beat — never lock the player out (#623 deeper layer)

[100yenadmin]

The bug (the lone v1.0.4-rc2 RRI holdout @c92a393)

In the 5-persona sweep, the newbie persona gave up (the only one). A DM beat streamed partial prose via /events ("You give the sergeant your own name… The charcoal touches the paper… That's the arithmetic o—") then froze mid-generation; the action bar stayed disabled for 15+ min with no recovery path (no retry / refresh / cancel) → give-up. This flips both no_give_up and zero_critical for newbie.

Root cause (proven, file:line)

(a) Client — primary. viewer/openworlds/app.jsx notePendingProgress re-arms the full position recovery window (180s later / 240s first) and clears stuck on every streamed /events paragraph. A beat that streams several partial paragraphs and then freezes keeps pushing the stuck deadline forward with each partial — so with partials closer than the window, stuck never fires and recovery is deferred to the 12-min PENDING_BACKSTOP_MS, which clears pending to null (a plain re-enabled bar — no "Try again" affordance, the partial narration stranded). That is the ~12–15-min lockout.

#623/#743 interaction — confirmed implicated. The #623/#743 heartbeat + live-progress stream is exactly what feeds notePendingProgress; the perceived-latency fix masked the genuine-stall detector (per-progress reset).

(b) Backend — compounding. qa/ui_playtest.sh's dm_turn (the GUI-sweep driver the newbie ran) invoked claude -p with no timeout, no retry, no fallback — unlike scripts/play.sh's dm_turn. A frozen process hangs forever → the turn never resolves on /chat → the client stall ceiling is the only recovery.

Note: timeout(1) IS a wall-clock from process launch, so it does cover a mid-stream stall — but only where it was wired (play.sh), not in the GUI driver. That asymmetry is (b). The client recovery action already exists (the stuck "Try again" path) — the gap is that stuck never fired in time, not that there's no action.

The fix (additive; engine stays sole writer; #720 dedup / #623 heartbeat / #735 unchanged)

• Client (the layer the root cause is in): a hard stuck-backstop PENDING_STUCK_BACKSTOP_MS (5 min), armed once in armPending, anchored to submit, not reset by progress, flipping stuck=true — the recoverable "Try again" affordance, not a silent null clear. Ordering is now: position recovery (resettable — preserves perf(engine): non-blocking generate_image — off the synchronous DM-turn path #399/[reliability] DM beats silently drop / hang >10min with no recovery — sabotages story personas (crit) #623 live-progress) < stuck-backstop (hard, recoverable) < 12-min null-backstop. notePendingProgress is otherwise unchanged, so a healthy streaming turn is never falsely stuck. (A per-progress 90s clamp was tried and rejected — it re-introduces the perf(engine): non-blocking generate_image — off the synchronous DM-turn path #399 false-stuck on a heartbeat-only turn whose real prose lands >90s later.)
• Backend: qa/ui_playtest.sh's dm_turn now wraps claude -p in timeout (shared clawdnd_dm_timeout) and routes the result through clawdnd_dm_narration_or_fallback (same call shape as play.sh), so a killed beat still resolves on /chat. Bash 3.2-clean.

Tests (real red→green guards)

• viewer/tests/test_recovery_timing.py::MidStreamStallTests (4): trickle-then-freeze recovers to stuck (not null) within ~5 min; the ceiling is not reset by progress; a healthy resolving turn never trips it; the pre-stream slow open keeps its full window. RED on origin/main.
• servers/engine/tests/test_dm_session_remint.py (3): ui_playtest.sh dm_turn wraps the beat in timeout, uses the narration fallback, and clawdnd_dm_timeout is 3.2-clean. 2/3 RED on origin/main.

Verify

• viewer/tests/test_recovery_timing.py: 32 passed
• viewer/tests (full): 483 passed, 6 skipped
• servers/engine/tests/test_dm_session_remint.py: 24 passed
• qa/fast_gate.sh: 188 passed
• bash -n clean under macOS system bash 3.2 on both modified scripts

Confidence & residual risk

~85%. The node-harness tests prove the timing contract, not end-to-end browser behavior. Only a real 5-persona GUI sweep @ this SHA can confirm the holdout flips (no_give_up + zero_critical). The 5-min ceiling is a deliberate UX judgement (generous enough to never false-stuck a healthy slow cold-open; the player still waits up to 5 min before "Try again" — far better than 12-15 min, tunable). Do not merge before the GUI sweep + adversarial review.

Summary by CodeRabbit

• Bug Fixes
  • Fixed indefinite hangs during DM streaming by implementing timeout-based recovery mechanisms.
  • Enhanced in-flight turn recovery with a new hard-stop timeout for stuck or stalled turns.
  • Improved fallback logic to ensure turns recover gracefully when streaming is interrupted or stalls mid-progress.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes #745 by hardening both the backend DM turn driver and the frontend turn-recovery logic against mid-stream stalls. The DM turn now times out per beat and falls back to engine-logged narration; the viewer's live-session hook now arms a hard stuck-backstop ceiling that marks pending as stuck when no progress arrives, independent of retry logic.

Changes

Mid-stream stall recovery: DM timeout + viewer stuck backstop

Layer / File(s)	Summary
DM turn bounded timeout implementation qa/ui_playtest.sh	dm_turn wraps claude -p in a beat-specific timeout computed via clawdnd_dm_timeout, captures streamed JSONL, and returns via clawdnd_dm_narration_or_fallback to ensure the turn-ending /chat line is always produced even on timeout or error.
DM timeout regression tests servers/engine/tests/test_dm_session_remint.py	Three tests verify that the DM turn uses a bounded timeout, recovers stalled beats via narration fallback, and that the timeout helper returns positive integers for both beat tiers while remaining bash 3.2 compatible.
Viewer stuck backstop constant and contract viewer/openworlds/app.jsx	Introduces PENDING_STUCK_BACKSTOP_MS (5 minutes) documenting its ordering between adaptive per-position recovery and null backstop; exports stuckBackstopMs in window.__PENDING_TIMING__ for test/devtools visibility.
Viewer stuck backstop timer lifecycle viewer/openworlds/app.jsx	Manages the stuck-backstop timer: adds stuckBackstopTimer ref, arms it at pending creation so pending transitions to stuck after the ceiling independent of streamed progress, clears it with other timers on resolution, exposes notePendingProgress hook return, and clarifies timer behavior in comments.
Viewer stuck backstop regression tests viewer/tests/test_recovery_timing.py	Extends test harness with note() helper to simulate progress, adds MidStreamStallTests class validating constant export/ordering, mid-stream freeze recovery as a non-null stuck state, progress does not reset the ceiling, healthy streams do not false-positive, and pre-stream slow opens preserve the first-beat recovery window.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

• electricsheephq/WorldOS#745: The primary issue this PR addresses; both DM timeout and viewer stuck-backstop changes directly target the mid-stream stall hang failure mode.
• electricsheephq/WorldOS#406: Overlaps on the useLiveSession pending recovery logic—the PR's notePendingProgress and stuckBackstopMs additions are related to existing backstop/pending contract.
• electricsheephq/WorldOS#648: The useLiveSession changes (adding notePendingProgress and stuck-backstop) address the pending-state contract that the reported Enter-submit spinner bug touches.

Possibly related PRs

• electricsheephq/WorldOS#666: Modifies window.__PENDING_TIMING__ logic in viewer/openworlds/app.jsx by adding armGraceMs guard in clearPending; this PR adds a new stuckBackstopMs timing behavior in the same export contract.
• electricsheephq/WorldOS#360: Introduces the clawdnd_dm_narration_or_fallback helper via qa/lib_beat_driver.sh; this PR rewires qa/ui_playtest.sh's dm_turn to use that helper for turn-ending narration on timeout/error.
• electricsheephq/WorldOS#567: Adds the clawdnd_dm_timeout helper that this PR uses to bound claude -p per beat in qa/ui_playtest.sh.

Poem

🐰 A timeout wraps the chatty Claude,
And up above, a backstop nods—
When streaming stalls mid-beat, behold,
No more hangs; the turn's controlled! 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding mid-stream stall recovery for DM beats to prevent player lockout, with clear reference to the related issue context.
Description check	✅ Passed	The description is comprehensive and well-structured, covering the bug, root cause, fix, tests, and verification against the template requirements (summary present, licensing checkboxes included, validation checks listed).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

qa/ui_playtest.sh (1)

139-150: 💤 Low value

Shellcheck SC2034 is a false positive — consider silencing it.

The CLAWDND_DM_MODEL variable is consumed by clawdnd_dm_timeout (sourced from lib_beat_driver.sh at line 28), which uses ${CLAWDND_DM_MODEL:-} to select the cold-open tier. Shellcheck doesn't trace into sourced functions, so it reports the assignment as unused.

Silencing the warning and adding a brief usage note would prevent future confusion:

Proposed clarification

-CLAWDND_DM_MODEL="$DM_MODEL"
+# shellcheck disable=SC2034  # read by clawdnd_dm_timeout (sourced from lib_beat_driver.sh)
+CLAWDND_DM_MODEL="$DM_MODEL"

🤖 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 `@qa/ui_playtest.sh` around lines 139 - 150, The CLAWDND_DM_MODEL assignment is
flagged by ShellCheck SC2034 but is actually used indirectly by
clawdnd_dm_timeout (sourced from lib_beat_driver.sh); silence the false positive
by adding a ShellCheck directive on the CLAWDND_DM_MODEL line (disable SC2034)
and add a one-line comment noting that clawdnd_dm_timeout in lib_beat_driver.sh
consumes this variable so future readers/tools don’t remove it; reference
clawdnd_dm_timeout and lib_beat_driver.sh in that comment.

Source: Linters/SAST tools

🤖 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.

Nitpick comments:
In `@qa/ui_playtest.sh`:
- Around line 139-150: The CLAWDND_DM_MODEL assignment is flagged by ShellCheck
SC2034 but is actually used indirectly by clawdnd_dm_timeout (sourced from
lib_beat_driver.sh); silence the false positive by adding a ShellCheck directive
on the CLAWDND_DM_MODEL line (disable SC2034) and add a one-line comment noting
that clawdnd_dm_timeout in lib_beat_driver.sh consumes this variable so future
readers/tools don’t remove it; reference clawdnd_dm_timeout and
lib_beat_driver.sh in that comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 21943c51-518f-45d2-9d69-b3df011e90ce

📥 Commits

Reviewing files that changed from the base of the PR and between c92a393 and d7f481f.

📒 Files selected for processing (4)

• qa/ui_playtest.sh
• servers/engine/tests/test_dm_session_remint.py
• viewer/openworlds/app.jsx
• viewer/tests/test_recovery_timing.py