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

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>

README.md

@@ -395,6 +395,10 @@ Then put it behind your existing reverse proxy like everything else
 - [x] Linked whatever's playing (video or music) to the OS's own media
       controls (Windows' SMTC, macOS' Now Playing, ...) — title/artwork,
       play/pause/seek/next — see docs/architecture.md
+- [x] Fixed switching a party between devices leaving both "on the
+      session" with no working controls — the old device never reacted
+      to being kicked, and the leave route had no way to tell a stale
+      device apart from the current one — see docs/architecture.md
 - [ ] Try a full party end-to-end with two real accounts (still the main gap)
 - [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is
       confirmed here — no real device/session)

client/src/api/party.ts

@@ -39,8 +39,14 @@ export function joinParty(partyId: string): Promise<PartyPublicState> {
   return apiFetch(`/api/parties/${encodeURIComponent(partyId)}/join`, { method: 'POST' });
 }
 
+/** Sends this device's own deviceId so the server can tell a stale
+ *  device (already replaced by a takeover on another one - see
+ *  getDeviceId's own comment) apart from the currently-active one, the
+ *  same way the socket "join"/"leave"/"disconnect" handlers already do
+ *  via the live connection itself - a REST call has no connection to
+ *  compare, so this is what lets the server apply that same guard here. */
 export function leaveParty(partyId: string): Promise<{ ok: true }> {
-  return apiFetch(`/api/parties/${partyId}/leave`, { method: 'POST' });
+  return apiFetch(`/api/parties/${partyId}/leave`, { method: 'POST', body: JSON.stringify({ deviceId: getDeviceId() }) });
 }
 
 export function getParty(partyId: string): Promise<PartyPublicState> {

client/src/musicBar/MusicBar.ts

@@ -266,7 +266,24 @@ export function joinMusicParty(partyId: string, party: PartyPublicState): void {
       else leaveMusicParty();
     },
     onError: (message) => showToast(message, { variant: 'error' }),
-    onClose: () => {}
+    // Fires for every disconnect, intentional or not - leaveMusicParty
+    // itself already clears activePartyId synchronously, well before its
+    // own conn.leave() actually closes the socket, so seeing it still set
+    // here means this device's connection died some other way (most
+    // often: replaced by a takeover on another device - see
+    // SocketGateway's "join" handler). Without this, the bar just sat
+    // there looking/sounding fully live forever on the old device, on a
+    // conn that was actually already dead underneath it - reported
+    // directly as "both devices end up on the session, controls never
+    // transfer." leaveMusicParty() is safe to call here even though the
+    // party already moved on: leaveParty's own deviceId check (see
+    // api/party.ts) makes its REST call a no-op for a stale device
+    // instead of ending the CURRENT device's membership by mistake.
+    onClose: () => {
+      if (!activePartyId) return;
+      showToast('Connexion à la party interrompue.', { variant: 'error' });
+      leaveMusicParty();
+    }
   });
 }
 

client/src/views/player/Player.ts

@@ -229,6 +229,14 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli
 
   let isHost = false;
   let hasNextQueued = false;
+  // Set at the top of cleanup() below, which every intentional way of
+  // leaving this route (the "Quitter" button, a sidebar link, browser
+  // back, ...) already runs synchronously before the socket actually
+  // closes - so by the time the async "disconnect" this connection's
+  // own onClose reacts to actually arrives, this is already true for
+  // every one of those. Still false there only means an unprompted
+  // close (most often: a device takeover elsewhere - see onClose below).
+  let leftIntentionally = false;
   let conn: PartyConnection | null = null;
   let hls: HlsInstance | null = null;
   let currentItemId: string | null = null;
@@ -1277,7 +1285,23 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli
         else navigate('/home');
       },
       onError: (message) => showToast(message, { variant: 'error' }),
-      onClose: () => {}
+      // Fires for every disconnect, intentional or not - leftIntentionally
+      // is already true by the time this runs for any of those (see its
+      // own comment), so still false here means an unprompted close, most
+      // often a device takeover elsewhere (SocketGateway's "join" handler)
+      // - without this, the video/hls.js instance just kept running
+      // forever on the old device, on a conn that was actually already
+      // dead underneath it, reported directly as "both devices end up on
+      // the session, controls never transfer." navigate() runs this same
+      // route's cleanup() below, which is now safe to do even though the
+      // party already moved on - leaveParty's own deviceId check (see
+      // api/party.ts) makes its REST call a no-op for a stale device
+      // instead of ending the CURRENT device's membership by mistake.
+      onClose: () => {
+        if (leftIntentionally) return;
+        showToast('Connexion à la party interrompue.', { variant: 'error' });
+        navigate('/home');
+      }
     });
   }
 
@@ -1291,6 +1315,7 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli
   // other way (sidebar link, browser back, ...). This is the fix for the
   // actual leak: previously, only the "Quitter" button ran any of this.
   return function cleanup() {
+    leftIntentionally = true;
     if (heartbeat) clearInterval(heartbeat);
     if (progressTicker) clearInterval(progressTicker);
     if (requestBannerTimer) clearTimeout(requestBannerTimer);

docs/architecture.md

@@ -4415,6 +4415,69 @@ call sites already gives), and the actual OS-level popup itself - the
 whole point - isn't something this sandbox can display or click
 regardless. Worth a real Windows/Chrome check once deployed.
 
+## Device takeover: both devices "on the session", neither one's controls working
+
+Reported directly: switching a watch/listen party from one device to
+another left both devices looking active, with no controls actually
+transferring. The device-takeover mechanism itself - `SocketGateway`'s
+"join" handler force-disconnecting the old device's socket once the new
+one confirms - was already correct and already guarded against the
+obvious failure mode (its own comment on the "leave"/"disconnect"
+handlers: a stale connection's own eventual disconnect must not be
+mistaken for a real leave). The actual bug was two gaps *around* that,
+neither in the takeover mechanism itself.
+
+**Gap 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, that device's own video/
+hls.js instance (or the persistent music bar) just kept running exactly
+as before: the socket was dead, but nothing told the local `<video>`/
+`<audio>` to stop, and nothing navigated away or reset `isHost`. From
+the old device's own screen, everything still looked (and for local-only
+playback, sounded) fully live - exactly "both devices on the session."
+Fixed: both now react to an *unprompted* close (one that didn't already
+run this route/module's own explicit teardown - `leftIntentionally` in
+Player.ts, `activePartyId` already being null in MusicBar.ts, both set
+synchronously by that explicit teardown well before the async
+"disconnect" this checks against ever arrives) by tearing down local
+playback and leaving, the same as if the user had clicked "Quitter"
+there themselves.
+
+**Gap 2 - reacting to the kick had no safe way to do that.** The obvious
+fix for gap 1 - call the same leave path "Quitter" already uses - ran
+straight into a second, worse bug: `POST /api/parties/:partyId/leave`
+removes a member by `userId` alone, with no equivalent of the socket
+handlers' own staleness guard (there's no live connection for a REST
+call to compare against). 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 by giving
+this route the same guard via `deviceId` instead of a socket reference:
+new `PartyService.isStaleDevice(member, deviceId)`, and `leaveParty()`
+(client) now sends its own `getDeviceId()` for the server to compare.
+Only ever blocks on positive evidence of a mismatch - a member with no
+recorded deviceId yet, or a caller that sent none, still leaves exactly
+as before this existed.
+
+Verified: full `typecheck`/`test`/`build` on both projects (153/153
+server tests - new coverage for `isStaleDevice` covering a matching
+deviceId, a mismatched one, an unset member deviceId, an unset caller
+deviceId, and an unknown member, each checked against the *reason* it
+should or shouldn't block, not just the outcome). The client-side
+onClose fix was checked against the exact reported scenario, not just
+reasoned about: a fake `io()` standing in for Socket.IO (real `.on`/
+`.emit`/a scriptable `.disconnect()` that fires real listeners) let
+`joinMusicParty`/`leaveMusicParty` run for real. Firing `disconnect`
+right after a real `leaveMusicParty()` call produced no toast and left
+the bar hidden (no spurious "connection lost" 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 own `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, given both share the same
+logic shape end to end).
+
 ## Known limitations
 
 - Playback always goes through `hls.js` against whatever Emby's

server/src/index.ts

@@ -615,9 +615,16 @@ app.post('/api/parties/:partyId/join', requireSession, (req, res) => {
 app.post('/api/parties/:partyId/leave', requireSession, (req, res) => {
   const party = PartyService.getById(param(req, 'partyId'));
   if (party) {
-    // Shared with the socket "leave"/"disconnect" handlers so the host-
-    // handoff notification and the lobby refresh only live in one place.
-    SocketGateway.applyLeave(io, party, req.embySession!.userId);
+    const member = party.members.get(req.embySession!.userId);
+    const deviceId = typeof req.body?.deviceId === 'string' ? req.body.deviceId : null;
+    // See PartyService.isStaleDevice's own comment for why this check
+    // exists at all - a REST call has no live connection to guard this
+    // with, unlike the socket "leave"/"disconnect" handlers.
+    if (!PartyService.isStaleDevice(member, deviceId)) {
+      // Shared with the socket "leave"/"disconnect" handlers so the host-
+      // handoff notification and the lobby refresh only live in one place.
+      SocketGateway.applyLeave(io, party, req.embySession!.userId);
+    }
   }
   res.json({ ok: true });
 });

server/src/services/PartyService.ts

@@ -144,6 +144,30 @@ class PartyServiceClass {
     return { itemId: party.itemId, positionTicks: host.positionTicks, isPaused: host.isPaused };
   }
 
+  /**
+   * Whether a request claiming to be `deviceId` is stale for `member` -
+   * already replaced by a takeover on a *different* device (see
+   * SocketGateway's "join" handler). Used by the REST /leave route
+   * (index.ts), which - unlike the socket "leave"/"disconnect" handlers,
+   * guarded by comparing `member.connection` against the live socket
+   * itself - has no connection to compare against instead. Without this,
+   * a device already superseded by a takeover elsewhere could still call
+   * /leave and end the CURRENT device's membership by mistake - confirmed
+   * as the real cause of a "both devices end up on the session, controls
+   * never transfer" report: the stale device's own client didn't react to
+   * being disconnected at all (see Player.ts/MusicBar.ts's onClose), so
+   * its still-rendered "Quitter" button stayed clickable and reached here.
+   *
+   * Only ever returns true on positive evidence of a mismatch (both sides
+   * known and different) - a member with no recorded deviceId yet, or a
+   * caller that didn't send one, is never treated as stale, mirroring the
+   * socket handlers' own "nothing to conflict with yet" reasoning for a
+   * null deviceId.
+   */
+  isStaleDevice(member: PartyMember | undefined, deviceId: string | null): boolean {
+    return !!(member?.deviceId && deviceId && member.deviceId !== deviceId);
+  }
+
   /**
    * Removes a member. If they were the host, promotes whoever's left
    * (arbitrary but deterministic-ish - good enough for v1); if nobody's

server/test/partyService.test.ts

@@ -155,6 +155,41 @@ describe('PartyService', () => {
     assert.equal(called, false);
   });
 
+  it('isStaleDevice(): false when the deviceId matches the member\'s own', () => {
+    const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
+    const alice = party.members.get('u-alice')!;
+    alice.deviceId = 'device-a';
+
+    assert.equal(PartyService.isStaleDevice(alice, 'device-a'), false);
+  });
+
+  it('isStaleDevice(): true when the deviceId differs from the member\'s own - the actual takeover case', () => {
+    const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
+    const alice = party.members.get('u-alice')!;
+    alice.deviceId = 'device-a'; // device B's takeover already moved this forward
+
+    assert.equal(PartyService.isStaleDevice(alice, 'device-b'), true);
+  });
+
+  it('isStaleDevice(): false when the member has no recorded deviceId yet - nothing to conflict with, same reasoning as the socket "join" handler', () => {
+    const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
+    const alice = party.members.get('u-alice')!; // deviceId still null - no socket "join" has succeeded yet
+
+    assert.equal(PartyService.isStaleDevice(alice, 'device-a'), false);
+  });
+
+  it('isStaleDevice(): false when the caller sent no deviceId at all - only ever blocks on positive evidence of a mismatch', () => {
+    const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
+    const alice = party.members.get('u-alice')!;
+    alice.deviceId = 'device-a';
+
+    assert.equal(PartyService.isStaleDevice(alice, null), false);
+  });
+
+  it('isStaleDevice(): false for an unknown member (already left some other way)', () => {
+    assert.equal(PartyService.isStaleDevice(undefined, 'device-a'), false);
+  });
+
   it('join() is idempotent: re-joining an existing member returns it as-is instead of resetting it', () => {
     const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
     const bob = PartyService.join(party, 'u-bob', 'Bob');