R3: resume, toggle, and pause intent move into the machine#47
Merged
Conversation
From the post-review refactor plan (plan.md, R3). The adapter is now pure forwarding — every user gesture is one send(); what it MEANS per state is the machine's decision. Old e2e suite untouched and green (37/37). New events, replacing USER_PAUSE_INTENT and RESUME_FAILED: - RESUME — paused: in-machine play attempt (invoked attemptPlay in a paused.resuming substate; presentation stays 'paused', a failed play() re-applies the paused fx, leaving the substate discards stale resolve/reject). idle/error/recovering: raises PLAY with the selected station. Fixes the confirmed bug: lock-screen Play was a silent no-op in 'error' (the on-screen button worked) — both are now the same event. - TOGGLE — playing: pause with user intent marked; paused/idle/error/ recovering: same as RESUME; loading/retrying: full STOP. Fixes the confirmed bug: toggle during loading swallowed the pause (AbortError) and the loading timeout then restarted playback the user had stopped. - PAUSE_REQUESTED — marks intent + pauses; the feedback states override it (and TOGGLE) with a raised STOP, so the stop-vs-pause policy left mediaSession.ts and lives with the other policies. Also fixed (confirmed regression): restart-after-long-pause now honors the same offline fast-fail as PLAY — resuming on a dead network goes straight to the error state + recovery loop instead of ~9s of loading tone. Shared restartAfterLongPause ladder serves PLAYER_PLAY, RESUME and TOGGLE. Fallout: - playerIsPaused left RadioDeps — nothing reads the element's paused flag anymore; toggle is state-driven. - radioCore: handleResumeError/resumePlayer deleted; getState/log normalize the compound 'paused' state to the flat RadioState. - mediaSession 'pause' handler is a plain core.pauseRadio(). - 6 new unit tests spec the fixed behaviors; 2 resume tests adapted to the event API (one now asserts resume-while-playing is a no-op, previously it pinned the blind play() forward). NEEDS DEVICE SMOKE before merge (lock screen: play/pause/prev/next from paused AND from error; resume after long pause; offline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CI Summary
|
Play-then-immediate-lock -> wifi off: loading tone plays but the error tone never starts (fresh background start refused by iOS); once the app has been through an error cycle with the screen open, later lock-screen cycles work. The single tone channel (src swap on an already-audible element) fixes this by construction. 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.
Third phase of the post-review refactor plan (
plan.md, R3) — the core one. The adapter is now pure forwarding: every user gesture is onesend(), and what it means per state is the machine's decision. Old e2e suite untouched: 37/37 green; 69/69 unit (6 new); clean typecheck.Bugs closed (all from the review)
error— the'play'action handler blindly calledresumeRadio()(a bareplay()on a cleared src), while the on-screen button routed throughplayRadio. Both are now the sameRESUMEevent: inidle/error/recoveringit raisesPLAYwith the selected station.pauseRadio()'s AbortError was swallowed and the loading timeout later retried.TOGGLEinloading/retryingnow raises a fullSTOP, consistent with the lock-screen pause policy. New unit test locks it.restartAfterLongPauseladder (used byPLAYER_PLAY,RESUME,TOGGLE) now honorsisOnline: resuming on a dead network goes straight toerror+ recovery loop instead of ~9s of loading tone.Design
RESUMEinpaused→ invokedattemptPlayin apaused.resumingsubstate: presentation stays 'paused' (no newRadioState, noSTATE_FXentry), a failedplay()re-applies the paused fx exactly like the oldRESUME_FAILEDre-entry, and leaving the substate discards a stale resolve/reject like every otherattemptPlay.getState()/the transition log normalize the compound value, so logs read the same.PAUSE_REQUESTEDreplacesUSER_PAUSE_INTENT: marks intent + pauses at root; the four feedback states override it (andTOGGLE) with a raisedSTOP— the stop-vs-pause policy leftmediaSession.ts.USER_PAUSE_INTENTandRESUME_FAILEDevents are gone;handleResumeError/resumePlayerdeleted from the adapter;playerIsPausedleftRadioDeps(nothing reads the element's paused flag anymore).Deliberate behavior notes
resumeRadio()while alreadyplayingis now a no-op (was: a redundantplay()on the playing element). One old test pinned that blind forward; it now asserts the no-op.Per the plan, this touches the lock-screen paths: play/pause/prev/next from paused AND from error, resume after long pause, offline behavior — on iPhone (and ideally the macOS Now Playing widget).
🤖 Generated with Claude Code