Fix device takeover leaving both devices on the session with no working controls #23

Merged

kyuno merged 1 commit from fix/device-takeover-stale-connection into master

2026-09-18 11:33:25 +02:00

kyuno commented

2026-09-18 10:23:49 +02:00

Owner

Summary

Two gaps around an already-correct takeover mechanism (SocketGateway's "join" handler), not the mechanism itself.

The old device never reacted to being kicked. Both Player.ts and MusicBar.ts wired onClose: () => {} — a complete no-op — to the connection's "disconnect" event. So the moment a takeover force-disconnected the old device's socket, its video/hls.js instance (or the persistent music bar) just kept running forever on a dead connection — both devices looked fully active, since nothing ever told the old one to stop or navigate away.
Reacting to the kick had no safe way to do that. The obvious fix — call the same leave path "Quitter" already uses — ran straight into a worse bug: POST /api/parties/:partyId/leave removes a member by userId alone, with no equivalent of the socket handlers' own staleness guard. Calling it from the old, superseded device would have ended the new device's membership too, silently undoing the takeover it was supposed to be cleaning up after.

Fixed both: onClose now tears down local playback and leaves whenever this route/module's own explicit teardown hasn't already run (leftIntentionally in Player.ts, activePartyId already null in MusicBar.ts); the leave route (and the client, which now sends its own deviceId) gained the same staleness guard the socket handlers already had, via a new PartyService.isStaleDevice(member, deviceId) — only ever blocks on positive evidence of a mismatch.

Test plan

npm run typecheck/npm run build — clean on both projects
npm test (server) — 153/153, including new coverage for isStaleDevice (matching deviceId, mismatched, unset member deviceId, unset caller deviceId, unknown member — each checked against why it should/shouldn't block)
Checked the client fix against the exact reported scenario, not just reasoned about: a fake io() (real .on/.emit/a scriptable .disconnect() that fires real listeners) let joinMusicParty/leaveMusicParty run for real. Firing disconnect right after a real leaveMusicParty() produced no toast, bar stayed hidden (no spurious message on an intentional leave); firing it with no prior leave — the takeover-kick shape — produced the "Connexion à la party interrompue" toast and correctly tore the bar down.
Player.ts's leftIntentionally fix mirrors this exact shape (confirmed by reading, not re-verified with a second live harness — hls.js/a full player DOM would need mocking for no more signal than the MusicBar case already gives).

🤖 Generated with Claude Code

## Summary Two gaps around an already-correct takeover mechanism (`SocketGateway`'s "join" handler), not the mechanism itself. 1. **The old device never reacted to being kicked.** Both `Player.ts` and `MusicBar.ts` wired `onClose: () => {}` — a complete no-op — to the connection's "disconnect" event. So the moment a takeover force-disconnected the old device's socket, its video/hls.js instance (or the persistent music bar) just kept running forever on a dead connection — both devices looked fully active, since nothing ever told the old one to stop or navigate away. 2. **Reacting to the kick had no safe way to do that.** The obvious fix — call the same leave path "Quitter" already uses — ran straight into a worse bug: `POST /api/parties/:partyId/leave` removes a member by `userId` alone, with no equivalent of the socket handlers' own staleness guard. Calling it from the *old*, superseded device would have ended the *new* device's membership too, silently undoing the takeover it was supposed to be cleaning up after. Fixed both: `onClose` now tears down local playback and leaves whenever this route/module's own explicit teardown hasn't already run (`leftIntentionally` in Player.ts, `activePartyId` already null in MusicBar.ts); the leave route (and the client, which now sends its own `deviceId`) gained the same staleness guard the socket handlers already had, via a new `PartyService.isStaleDevice(member, deviceId)` — only ever blocks on positive evidence of a mismatch. ## Test plan - [x] `npm run typecheck`/`npm run build` — clean on both projects - [x] `npm test` (server) — 153/153, including new coverage for `isStaleDevice` (matching deviceId, mismatched, unset member deviceId, unset caller deviceId, unknown member — each checked against *why* it should/shouldn't block) - [x] Checked the client fix against the exact reported scenario, not just reasoned about: a fake `io()` (real `.on`/`.emit`/a scriptable `.disconnect()` that fires real listeners) let `joinMusicParty`/`leaveMusicParty` run for real. Firing `disconnect` right after a real `leaveMusicParty()` produced no toast, bar stayed hidden (no spurious message on an intentional leave); firing it with no prior leave — the takeover-kick shape — produced the "Connexion à la party interrompue" toast and correctly tore the bar down. - Player.ts's `leftIntentionally` fix mirrors this exact shape (confirmed by reading, not re-verified with a second live harness — hls.js/a full player DOM would need mocking for no more signal than the MusicBar case already gives). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

kyuno added 1 commit 2026-09-18 10:23:50 +02:00

Fix device takeover leaving both devices on the session with no working controls

Publier l'image Docker / test (pull_request) Failing after 4s

Details

Publier l'image Docker / publish (pull_request) Has been skipped

Details

3ae1fce806

Two gaps around an already-correct takeover mechanism, not the
mechanism itself. First: Player.ts/MusicBar.ts both wired onClose to a
no-op, so the moment a takeover force-disconnected the old device its
video/hls.js instance (or the persistent music bar) just kept running
forever on a dead connection - both devices looked fully active, since
nothing ever told the old one to stop or navigate away. Second: the
obvious fix (call the same leave path Quitter uses) ran into a worse
bug - POST /leave removes a member by userId alone with no staleness
guard, so the old devices own leave call would have ended the NEW
devices membership too. Fixed both: onClose now tears down local
playback and leaves whenever this routes own explicit teardown has not
already run, and the leave route (plus the client, which now sends its
own deviceId) gained the same staleness guard the socket handlers
already had, via a new PartyService.isStaleDevice.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

kyuno merged commit b31377c1e9 into master

2026-09-18 11:33:25 +02:00

kyuno deleted branch fix/device-takeover-stale-connection

2026-09-18 11:33:25 +02:00

kyuno referenced this pull request from a commit

2026-09-18 11:33:27 +02:00

Merge pull request 'Fix device takeover leaving both devices on the session with no working controls' (#23) from fix/device-takeover-stale-connection into master

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: kyuno/watchparty#23

No description provided.