R5 + R6 + zombie fix + bug-hunt e2e + Copilot fixes (re-opened #50)#51
Open
adyz wants to merge 6 commits into
Open
R5 + R6 + zombie fix + bug-hunt e2e + Copilot fixes (re-opened #50)#51adyz wants to merge 6 commits into
adyz wants to merge 6 commits into
Conversation
…te, title) From the post-review refactor plan (plan.md, R5) — the battery-drain finding. Old e2e untouched and green (37/37); 79/79 unit. radioMachine: the error state's wait-for-recovery loop moves into substates (backoff -> offlineRecheck). Entry fx and the sound supervisor run ONCE per error cycle; an offline recheck only re-arms its own child timer (reenter on the atomic child leaves the parent alone). Before, every 10s tick re-entered 'error' and re-ran the whole entry pipeline — new MediaMetadata (lock-screen artwork refetch + widget flicker), 6 action handlers re-registered, poster img.src reassigned, supervisor interval torn down and recreated — all night, for as long as the phone sat offline. The offlineRecheck context flag and its two actions are gone; the cadence and backoff values are unchanged (the old cadence unit tests pass as-is). mediaSession: updateMediaSession memoizes the presentation (MediaMetadata, poster img.src, document.title, loadingMsg) on (state, station title) — identical consecutive renders are skipped and cloudinaryImageUrl is computed once instead of twice. Deliberately NOT memoized: action-handler re-registration and playbackState — iOS resets those out from under us (d798cc9), so they re-assert on every call, exactly as before. New unit test pins the phase's point: 50 offline rechecks produce zero new side-effect calls (mediaSession/sounds/buttons/supervisor), and the loop still recovers on the first online check. NEEDS DEVICE SMOKE before merge: phone offline in error 5+ min, locked — error tone keeps playing, Now Playing widget no longer flickers every 10s; wifi back on -> recovers to playing by itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ter stop Device-observed on master (R4b): sometimes the loading tone played UNDER the live radio — the exact overlapping-sounds class the state machine exists to forbid. Root cause: carrySound()'s never-trade-audible-for- silent revert ran in a .catch with NO generation guard. When the swap's play() settled late (iOS latency) and the machine had meanwhile recovered to 'playing' (stop already ran: paused, src cleared, isPlaying=false), the catch reverted the src and RESTARTED the element — and nothing could stop it again, because isPlaying was already false so every later stop() skipped it. The revert now runs only while still wanted: same generation and not an AbortError (our own stop/reclaim interrupting the pending play). This was the only unguarded resurrect path in the file — every other catch already checks the generation. New unit test pins the repro: stop lands while the swap's play() is in flight; the late rejection must leave the element dead (no revert, no restart). 80/80 unit, 37/37 e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-blocking SW writes Same PR as R5 + the zombie fix, per Adrian (one PR for everything left). - main.ts: the ~22 status/station image fetches no longer compete with the stream connection and sound-blob preloads at startup — deferred to requestIdleCallback (setTimeout fallback for Safari). The precache LIST stays complete, deliberately: the untouched e2e (and the product) pin all-station posters working offline; only the timing moves. - sw.js: the app-shell cache.put leaves the response path (event.waitUntil, same pattern as the cloudinary branch) — pages get first bytes without waiting for a body download + disk write; /downloads/ (the 2.4MB APK) is streamed straight through instead of landing in the app cache; APP_CACHE bumped to v4 so existing installs evict the cached APK. - plan.md: R5/R6 statuses + the deliberate deviations from the initial R6 sketch (full list kept, page-level put kept — first-load SW claim race). 80/80 unit, 37/37 e2e (image tests untouched and green), typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The e2e that catches Adrian's device repro (loading tone audible under the live radio): three stress tests in the new describe 'Only the radio — no tone may survive under the stream'. The sharp one reproduces the race deterministically — the tone swap's play() is denied LATE (one-shot, 600ms, like locked iOS) and the network recovers inside that window, so the rejection lands after the playing-state cleanup ran. Against the unguarded carrySound it fails with "loadingNoise became audible 517ms after the stream settled"; with the generation guard (c5e6bc8) it passes. All three hold a 3s observation window after the stream settles, because the zombie resurrects after things look clean. Copilot review fixes (PR #50): - unexpectedOfflinePause -> retrying now runs stopPlayer, symmetric with STALLED/PLAYER_ERROR — the dead stream no longer stays attached through RETRY_DELAY. (+ unit test) - PLAYER_ERROR in paused is deliberately unhandled: a deliberate pause stays silent even if the still-attached stream errors later. The pre-R3 test pinning the opposite (retry from paused) is replaced by one pinning the product rule. (+ unit test) - PLAY/STOP consume a pending user-pause intent — a stale intent can no longer explain away the next unexpected offline pause as a user pause. (+ unit test) - warmUp() respects an active carry like ensure() does: the gesture steadies the carrier element instead of starting a second one under it. - README: stale unit-test count replaced with the two-file description. 82/82 unit, 40/40 e2e, typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CI Summary
|
…the spot Adrian still heard radio + an unidentifiable background tone on device — proof that guarding resurrection paths one by one is whack-a-mole. The machine can only COMMAND the tone elements; late async callbacks can disobey after the bookkeeping already says "stopped". Enforce the invariant at the element boundary instead, by construction: on every 'playing' event of a tone element, if the machine is in a state where no feedback tone may sound (idle/playing/paused — !isFeedbackAudible), pause the element immediately. Catches every resurrection path, known or not. Inert in the tone states, where swap/carry behavior is legitimate. Test-first: the new e2e resurrects a tone out of nowhere while the radio plays and asserts it is silenced the moment it becomes audible (and stays silent through the 3s watch window). Fails without the enforcer, passes with it. 82/82 unit, 41/41 e2e, typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s e2e - The 'playing'-event enforcer only caught fresh (re)starts; a tone that never obeyed its stop fires no new 'playing' event and sailed under it. Also listen on 'timeupdate' (fires ~4x/s during playback): any tone audible in a no-tone state now dies within ~250ms no matter how it got there. Inert in tone states, as before. - New e2e from Adrian's exact device script: offline mid-playback, next+ prev while the error tone swaps, back online with slow connects, then rapid back-and-forward flapping between stations — at the end, only the radio (3s watch window). Passes 3x consecutively with the enforcer. 82/82 unit, 42/42 e2e, typecheck clean. 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.
Re-opened #50 — its content briefly landed on master by mistake (a commit made while the local branch pointed at master); master was immediately restored to
64fe6b7and GitHub auto-marked #50 as merged in that window. This PR is the real thing; #50's description applies, plus two additions since:New: the failing test for the tone-under-stream bug
Only the radio — no tone may survive under the stream(3 e2e stress tests). The sharp one reproduces Adrian's device repro deterministically: the tone swap'splay()is denied late (one-shot, 600ms, like locked iOS) and the network recovers inside that window. Against the unguardedcarrySound(master) it fails with"loadingNoise became audible 517ms after the stream settled"; with the generation guard in this PR it passes. All three tests hold a 3s observation window after the stream settles, because the zombie resurrects after things look clean.New: Copilot review fixes
unexpectedOfflinePause→ retrying now runsstopPlayer, symmetric with STALLED/PLAYER_ERROR (+ unit test).PLAYER_ERRORinpausedis deliberately unhandled — a deliberate pause stays silent even if the still-attached stream errors later; the pre-R3 test pinning the opposite is replaced (+ unit test).warmUp()respects an active carry likeensure()— the gesture steadies the carrier instead of starting a second element under it.From #50 (unchanged)
R5 — offline rechecks re-run no effects (error substates; fx + supervisor once per cycle) +
updateMediaSessionmemoized on (state, title), iOS handler re-registration untouched. Zombie fix — generation guard incarrySound's revert. R6 — image precache deferred to idle, SW cache writes off the response path, APK out of the app cache (v4).Verification
82/82 unit, 40/40 e2e (37 old untouched + 3 bug-hunt), typecheck clean.
Device smoke (same as #50): locked offline in error 5+ min → tone keeps playing, no widget flicker, recovers on wifi; wifi off/on cycles during playback → no tone left under the stream; lock-screen prev/next sanity.
🤖 Generated with Claude Code