R4: stop the zombie stream on stall/error; fix the ensure() preload race#48
Merged
Conversation
… a pending sound start From the post-review refactor plan (plan.md, R4). Two small behavior fixes, each with dedicated unit tests; old e2e untouched and green (37/37). - radioMachine: STALLED and PLAYER_ERROR from 'playing' now run stopPlayer on the way to 'retrying', symmetric with the LOADING_TIMEOUT path. The stalled stream stayed attached before, and a refilled buffer could resume audibly UNDER the loading tone during the 3s RETRY_DELAY — overlapping sounds, which the machine must never allow. (paused's PLAYER_ERROR keeps the old behavior: the element is already silent there, no overlap risk.) - soundEffects: minimal fix for the ensure() vs pending-preload race. On a slow first load, a supervisor tick landing while play() still waited for the blob would call element.play() on the empty src, the NotSupportedError flipped isPlaying to false, and the pending start bailed — one extra ~2.5s of silence exactly when the feedback sound mattered. ensure() now leaves the element alone while nothing was started yet (no src attribute = startPlayback hasn't run). The full reconcile() redesign stays in 4b. - soundEffects.test.ts: first unit tests for this module (fake <audio> element + controlled fetch): the race, OS-pause recovery, denied-play restart, and stop() semantics. - plan.md: R1-R3 marked done; 4b direction corrected after re-reading the PR #41 history with Adrian — NOT a single feedback element (no "never trade audible for silent" safety net without a partner element, and a0aec46 shows any real silence gap on a locked iPhone kills the session). 4b = the device-verified handoff/carry semantics, driven from the machine/reconciler instead of ad-hoc DOM coordination. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CI Summary
|
…omes R4b, next after R4 Adrian's explicit requirement: the error sound must be audible on the lock screen FROM THE FIRST TIME, not only after an error cycle with the app open. The play-then-lock repro is a requirement bug, not an accepted limitation. PR #41 proved the mechanism works on device; R4b rebuilds it machine-driven, ordered before R5/R6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adrian's device observations: after the silent-error state, unlocking shows the error screen with no sound; next/prev do NOT bring it back (the user's gesture is squandered — isPlaying intent is true while a denied programmatic attempt is in flight, so warmUp/play early-return and no element.play() runs inside the gesture stack); stop+play works because stop() force-resets the intent. New reconciler rule: every user gesture reconciles reality (element.paused → play() inside the gesture), plus four device acceptance criteria for R4b. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fourth phase of the post-review refactor plan (
plan.md, R4): two small behavior fixes, each with dedicated unit tests. Old e2e untouched: 37/37 green; 75/75 unit (6 new); clean typecheck.Fix 1 — zombie stream under the loading tone (PLAUSIBLE finding, philosophy violation)
STALLEDandPLAYER_ERRORfromplayingnow runstopPlayeron the way toretrying, symmetric with theLOADING_TIMEOUTpath. Before, the stalled stream stayed attached and un-paused: if the buffer refilled during the 3sRETRY_DELAY, the stream resumed audibly under the loading tone — overlapping sounds, which this project never allows.paused'sPLAYER_ERRORkeeps the old behavior (the element is already silent there).Fix 2 —
ensure()cancelling the sound it was supposed to protect (CONFIRMED finding)On a slow first load, a supervisor tick landing while
play()still waited for the blob preload would callelement.play()on the empty src; theNotSupportedErrorflippedisPlayingto false and the pending start bailed — one extra ~2.5s of silence exactly when the feedback sound mattered most.ensure()now leaves the element alone while nothing has been started yet (nosrcattribute ⇒startPlaybackhasn't run; the pending start is still in flight). Minimal fix only — the fullreconcile()redesign stays in 4b.Also in this PR
soundEffects.ts(the review flagged it as the most delicate untested async code): fake<audio>element + controlled fetch, covering the race, OS-pause recovery, denied-play restart, andstop()semantics.a0aec46shows any real silence gap on a locked iPhone kills the session and gets the pending play() denied. 4b = the device-verified handoff/carry semantics (deferred stop + carry as last resort + reclaim), driven from the machine/reconciler instead of ad-hoc DOM coordination.No lock-screen surface touched — no device smoke needed for this one (CI is enough).
🤖 Generated with Claude Code