Repository navigation
fix(webapp): stop playback on sign-out whatever the replaygain mode - #329
Conversation
Signing out unmounts PlayerProvider. Its cleanup paused and dropped the audio element only when a ReplayGain graph existed, so with ReplayGain off, the default, the music carried on over the sign-in page. The element is now paused, emptied and dropped on every unmount; the AudioContext cleanup and the graphMounted guard are unchanged. The Playwright test plays a real WAV with ReplayGain off, signs out and asserts that no element that was asked to play is still playing. It fails without the fix. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
github-actions
Bot
added
type: fix
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Nettoyage et test à la déconnexion webapp/src/player/player.tsx, webapp/e2e/studio-nocturne.spec.ts |
Le nettoyage ferme le contexte et réinitialise le gain si le graphe existe. Dans tous les cas, il met l’élément audio en pause, retire sa source et le recharge. Le test utilise un WAV silencieux et vérifie que la lecture s’arrête après la déconnexion. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related PRs
- InstaZDLL/waveflow-server#326: Cette PR ajoute ReplayGain et le nettoyage du contexte au démontage de
PlayerProvider.
Merge Risk: 
49c12
-
Playback is stopped on sign-out with ReplayGain off, and no actionable merge risk remains after normal checks.
Security architecture risk: 49c12
-
The change stops browser playback during sign-out regardless of ReplayGain mode, without expanding access or privileges. No introduced or materially worsened security architecture concern was identified in the inspected change.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The effective change is confined to playback resources owned by the browser's PlayerProvider. It reduces continued playback after leaving the authenticated shell; it does not establish server-side stream-ticket revocation or a broader credential-lifecycle guarantee.
Trust Boundaries and Controls
- observed — The authenticated route retains its session check. The sign-out control awaits logout and then navigates to the separate login route, removing the shell that contains PlayerProvider. The changed cleanup strengthens resource termination at that existing boundary rather than altering the authentication decision.
Resilience and Maintainability Implications
- observed — The new desktop regression test requires actual playback progress with no AudioContext construction, signs out, and then checks that every recorded playback element is paused. Its stream responses are mocked; the reported passing and inversion runs were not independently executed during this review.
-
Pre-merge checks: 
-
Check name Status Explanation Title check Le titre décrit clairement le correctif principal : arrêter la lecture lors de la déconnexion, quel que soit le mode ReplayGain. Description check La description est complète. Elle explique le problème, le changement, le test de régression et les résultats de validation. Les sections utilisent des intitulés différents du modèle, mais couvrent so… Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
La description est complète. Elle explique le problème, le changement, le test de régression et les résultats de validation. Les sections utilisent des intitulés différents du modèle, mais couvrent son contenu essentiel. La section Notes est optionnelle.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Biome (2.5.15)
-
webapp/e2e/studio-nocturne.spec.ts
-
Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
webapp/src/player/player.tsx
-
Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.
-
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 2
- Charged: $0.50
- View usage details
Note
This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
Signing out unmounts
PlayerProvider. The unmount cleanup added in #326 paused and dropped the<audio>element only on its way out of a ReplayGain graph, after an earlyreturnwhen no graph existed. With ReplayGain off, which is the default, nothing paused the element, which is never in the document, so the music carried on over the sign-in page. The bug was older than #326.The cleanup now closes the candidate and the graph as before, then always pauses the element, removes its
src, callsload()and drops the ref. The mount effect creates a fresh element, as it already did after the graph path, so StrictMode and Fast Refresh replays keep working. ThegraphMountedguard is unchanged.Test
stops the music when the listener signs out with ReplayGain offinwebapp/e2e/studio-nocturne.spec.ts:pausedandcurrentTimemean something;HTMLMediaElement.prototype.play, and counts AudioContexts;currentTime > 0and checks that no context was made (so this is the plain path);Inversion: run against a fresh
webapp/distbuilt without the fix, it fails at the last poll (Expected: 0, Received: 1). With the fix it passes.Checks
The bundle was rebuilt before every Playwright run. The runs used port 4199 through an uncommitted local config, not 4173 or 4533.
bun --cwd=webapp run build: okbiome check .: no errors (the 10 warnings were already there, in CSS andbiome.json)vitest: 29 files, 235 tests passedSummary by CodeRabbit