rover server restart desync fix hopefully

server/src/services/assignmentService/index.js

@@ -97,8 +97,20 @@ roverManager.managerEvents.on('private', ({ roverId, open }) => {
   }
 });
 
-roverManager.managerEvents.on('rover', ({ action }) => {
+roverManager.managerEvents.on('rover', ({ roverId, action }) => {
-  if (action === 'removed' || action === 'upsert') {
+  if (action === 'removed') {
+    /*
+      The physical rover record is the authority for current driver ownership.
+      Once it disappears, every assignment that names it must be released and
+      run through ordinary placement again. Leaving those map entries intact
+      lets the same id become visible after reconnect without recreating its
+      driver membership, which is the exact stale-UI/video-auth split this
+      lifecycle boundary must prevent.
+    */
+    reassignFromRover(roverId);
+    return;
+  }
+  if (action === 'upsert') {
     reassignWaiting();
   }
 });

server/src/services/roverManager/index.js

@@ -72,6 +72,7 @@ const {
 } = privateAccess;
 
 const roverLifecycle = createRoverLifecycle({
+  io,
   rovers,
   socketToRovers,
   managerEvents,
@@ -86,6 +87,7 @@ const roverLifecycle = createRoverLifecycle({
 const {
   requestControl,
   releaseControl,
+  removeRoverDrivers,
   isDriver,
   canDrive,
   getRoversForSocket,
@@ -119,6 +121,7 @@ const rosterLifecycle = createRosterLifecycle({
   normalizePrivateSafety,
   stopDockGuard: (...args) => stopDockGuard(...args),
   getControlDenialReason,
+  removeRoverDrivers,
 });
 
 const {

server/src/services/roverManager/rosterLifecycle.js

@@ -24,6 +24,7 @@ function createRosterLifecycle(deps) {
     isRoverVisibleToSocket,
     normalizePrivateSafety,
     stopDockGuard,
+    removeRoverDrivers,
   } = deps;
 
   function ensureRecord(id) {
@@ -106,7 +107,14 @@ function createRosterLifecycle(deps) {
   function removeRover(id) {
     const record = rovers.get(id);
     if (!record) return;
+    /*
+      Remove the public record before emitting driver-removal events. Any
+      session sync caused by those events must already see this rover as
+      offline, while removeRoverDrivers still receives the captured record so
+      it can clean the reverse membership index and Socket.IO room membership.
+    */
     rovers.delete(id);
+    removeRoverDrivers(id, record);
     stopDockGuard(id);
     privateButtonStates.delete(id);
     privateNoUsersSince.delete(id);

server/src/services/roverManager/roverLifecycle.js

@@ -3,6 +3,7 @@
 // Scope: Keeps runtime behavior unchanged by reusing rover-manager state maps and injected policy helpers.
 function createRoverLifecycle(deps) {
   const {
+    io,
     rovers,
     socketToRovers,
     managerEvents,
@@ -85,6 +86,40 @@ function createRoverLifecycle(deps) {
     managerEvents.emit('driver', { socketId: socket.id, roverId, action: 'remove' });
   }
 
+  function removeRoverDrivers(roverId, removedRecord = null) {
+    /*
+      A rover connection owns the record that contains its driver set, but the
+      reverse socket-to-rover index outlives that record. Disconnect cleanup
+      must therefore remove both halves before a reconnect creates a fresh
+      record with the same id. Otherwise session assignment can name the rover
+      while video/control authorization correctly sees no driver membership.
+
+      removedRecord is accepted because rosterLifecycle deliberately deletes
+      the public rover record first. Session syncs triggered by the driver
+      events below will consequently hide the unavailable rover immediately,
+      even before assignmentService finishes normal reassignment.
+    */
+    const record = removedRecord || rovers.get(roverId);
+    if (!record) return [];
+    const driverIds = Array.from(record.drivers || []);
+
+    driverIds.forEach((socketId) => {
+      const joined = socketToRovers.get(socketId);
+      if (joined) {
+        joined.delete(roverId);
+        if (joined.size === 0) socketToRovers.delete(socketId);
+      }
+
+      const socket = io.sockets.sockets.get(socketId);
+      socket?.leave(record.room);
+      record.drivers.delete(socketId);
+      turnService.driverRemoved(roverId, socketId);
+      managerEvents.emit('driver', { socketId, roverId, action: 'remove' });
+    });
+
+    return driverIds;
+  }
+
   function isDriver(roverId, socket) {
     const record = rovers.get(roverId);
     if (!record) return false;
@@ -175,6 +210,7 @@ function createRoverLifecycle(deps) {
     removeSocket,
     requestControl,
     releaseControl,
+    removeRoverDrivers,
     isDriver,
     canDrive,
     getRoversForSocket,

server/src/services/roverManager/roverLifecycle.test.js

@@ -0,0 +1,59 @@
+// Rover Manager Lifecycle Tests
+// Purpose: Verifies that physical rover removal clears every ownership index before a same-id reconnect.
+// Scope: Covers driver membership cleanup only; assignment placement policy remains in assignmentService.
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const EventEmitter = require('node:events');
+const { createRoverLifecycle } = require('./roverLifecycle');
+
+test('removing a rover clears driver sets, reverse membership, rooms, and turns', () => {
+  const roverId = 'rover-one';
+  const socketId = 'driver-one';
+  const leftRooms = [];
+  const removedTurns = [];
+  const driverEvents = [];
+  const socket = {
+    id: socketId,
+    leave: (room) => leftRooms.push(room),
+  };
+  const record = {
+    id: roverId,
+    room: `rover:${roverId}`,
+    drivers: new Set([socketId]),
+  };
+  const rovers = new Map([[roverId, record]]);
+  const socketToRovers = new Map([[socketId, new Set([roverId])]]);
+  const managerEvents = new EventEmitter();
+  managerEvents.on('driver', (event) => driverEvents.push(event));
+
+  const lifecycle = createRoverLifecycle({
+    io: { sockets: { sockets: new Map([[socketId, socket]]) } },
+    rovers,
+    socketToRovers,
+    managerEvents,
+    turnService: {
+      driverRemoved: (removedRoverId, removedSocketId) => {
+        removedTurns.push([removedRoverId, removedSocketId]);
+      },
+    },
+    isAdmin: () => false,
+    sendAlert: () => {},
+    ALERT_COLOR: '#000000',
+    getMode: () => 'public',
+    getControlDenialReason: () => null,
+  });
+
+  /* Mirror rosterLifecycle's ordering: the public record is gone before the
+     captured record is supplied for complete membership cleanup. */
+  rovers.delete(roverId);
+  const removedDriverIds = lifecycle.removeRoverDrivers(roverId, record);
+
+  assert.deepEqual(removedDriverIds, [socketId]);
+  assert.equal(record.drivers.size, 0);
+  assert.equal(socketToRovers.has(socketId), false);
+  assert.deepEqual(leftRooms, [`rover:${roverId}`]);
+  assert.deepEqual(removedTurns, [[roverId, socketId]]);
+  assert.deepEqual(driverEvents, [
+    { socketId, roverId, action: 'remove' },
+  ]);
+});

server/src/services/sessionService/index.js

@@ -135,6 +135,20 @@ function buildUserEntry(socket) {
   const role = getRole(socket);
   const assignment = assignmentService.describeAssignment(socket.id);
   const primaryRover = roverManager.getPrimaryRoverForSocket(socket.id);
+  /*
+    assignmentService owns automatic placement policy, while roverManager owns
+    actual control membership. Validate both candidate indexes before exposing
+    presence because neither cached direction is authoritative without the
+    physical rover record agreeing that this socket is one of its drivers.
+  */
+  const verifiedPrimaryRover = primaryRover
+    && roverManager.isDriver(primaryRover, socket)
+    ? primaryRover
+    : null;
+  const verifiedAssignmentRover = assignment?.roverId
+    && roverManager.isDriver(assignment.roverId, socket)
+    ? assignment.roverId
+    : null;
   const ptzChatTarget = getPtzChatTargetForSocket(socket.id);
   return {
     socketId: socket.id,
@@ -147,7 +161,7 @@ function buildUserEntry(socket) {
       the PTZ chat target while the socket is queued or operating so presence,
       queue lookup, and chat identity all agree.
     */
-    roverId: ptzChatTarget?.roverId || primaryRover || assignment?.roverId || null,
+    roverId: ptzChatTarget?.roverId || verifiedPrimaryRover || verifiedAssignmentRover || null,
   };
 }
 
@@ -172,7 +186,17 @@ function buildSession(socket) {
   }));
   const roster = roverManager.getRosterForSocket(socket);
   const assignment = assignmentService.describeAssignment(socket?.id || '');
-  const assignmentRoverId = filterVisibleRoverId(socket, assignment?.roverId);
+  /*
+    Visibility alone is insufficient here: a reconnected rover can be visible
+    before a stale assignment map has recreated actual driver membership. The
+    session contract consumed by every UI surface must require both visibility
+    and roverManager's authoritative membership check.
+  */
+  const verifiedAssignmentRover = assignment?.roverId
+    && roverManager.isDriver(assignment.roverId, socket)
+    ? assignment.roverId
+    : null;
+  const assignmentRoverId = filterVisibleRoverId(socket, verifiedAssignmentRover);
   const activeDrivers = filterActiveDriversForSocket(getActiveDrivers(), socket);
   const turnQueues = filterTurnQueuesForSocket(getTurnQueues(), socket);
   const socials = features.socials && configuredSocials?.length ? configuredSocials : [];