Fix presence detection for multiple windows/tabs/devices

frontend/src/app.ts

@@ -11,9 +11,8 @@ import './components/participant_view/participant_view';
 import './components/settings/settings';
 
 import {MobxLitElement} from '@adobe/lit-mobx';
-import {CSSResultGroup, html, nothing, TemplateResult} from 'lit';
+import {CSSResultGroup, html, nothing} from 'lit';
 import {customElement} from 'lit/decorators.js';
-import {classMap} from 'lit/directives/class-map.js';
 
 import {core} from './core/core';
 import {AnalyticsService} from './services/analytics.service';
@@ -23,15 +22,19 @@ import {Pages, RouterService} from './services/router.service';
 import {SettingsService} from './services/settings.service';
 import {PresenceService} from './services/presence.service';
 
-import {ColorMode} from './shared/types';
-
 import {styles} from './app.scss';
 
 /** App main component. */
 @customElement('deliberation-lab')
 export class App extends MobxLitElement {
   static override styles: CSSResultGroup = [styles];
 
+  // Unique per-tab connection ID for presence tracking
+  private readonly connectionId: string =
+    typeof crypto !== 'undefined' && crypto.randomUUID
+      ? crypto.randomUUID()
+      : Math.random().toString(36).slice(2) + Date.now();
+
   private readonly analyticsService = core.getService(AnalyticsService);
   private readonly authService = core.getService(AuthService);
   private readonly homeService = core.getService(HomeService);
@@ -92,6 +95,7 @@ export class App extends MobxLitElement {
         this.presenceService.setupPresence(
           this.routerService.activeRoute.params['experiment'],
           this.routerService.activeRoute.params['participant'],
+          this.connectionId,
         );
 
         return html`

frontend/src/services/presence.service.ts

@@ -22,8 +22,15 @@ export class PresenceService extends Service {
     makeObservable(this);
   }
 
-  setupPresence(experimentId: string, participantPrivateId: string) {
-    const statusRef = ref(this.sp.firebaseService.rtdb, `/status/${experimentId}/${participantPrivateId}`);
+  setupPresence(
+    experimentId: string,
+    participantPrivateId: string,
+    connectionId: string,
+  ) {
+    const statusRef = ref(
+      this.sp.firebaseService.rtdb,
+      `/status/${experimentId}/${participantPrivateId}/${connectionId}`,
+    );
 
     const isOffline = {
       connected: false,
@@ -35,17 +42,22 @@ export class PresenceService extends Service {
       last_changed: serverTimestamp(),
     };
 
-    onValue(ref(this.sp.firebaseService.rtdb, '.info/connected'), (snapshot) => {
-      const isConnected = snapshot.val();
-      if (!isConnected) {
-        return;
-      }
-
-      // Set the user's status in rtdb. The callback will reset it to online
-      // if the user reconnects.
-      onDisconnect(statusRef).set(isOffline).then(() => {
-        set(statusRef, isOnline);
-      });
-    });
+    onValue(
+      ref(this.sp.firebaseService.rtdb, '.info/connected'),
+      (snapshot) => {
+        const isConnected = snapshot.val();
+        if (!isConnected) {
+          return;
+        }
+
+        // Set the user's status in rtdb. The callback will reset it to online
+        // if the user reconnects.
+        onDisconnect(statusRef)
+          .set(isOffline)
+          .then(() => {
+            set(statusRef, isOnline);
+          });
+      },
+    );
   }
 }

functions/src/participant.endpoints.ts

@@ -53,9 +53,6 @@ export const createParticipant = onCall(async (request) => {
     prolificId: data.prolificId,
   });
 
-  // Temporarily always mark participants as connected (PR #537)
-  participantConfig.connected = true; // TODO: Remove this line
-
   // If agent config is specified, add to participant config
   if (data.agentConfig) {
     participantConfig.agentConfig = data.agentConfig;

functions/src/presence.triggers.test.ts

@@ -3,9 +3,9 @@ import firebaseFunctionsTest from 'firebase-functions-test';
 // Mock './app' before importing function under test
 jest.mock('./app', () => {
   const setMock = jest.fn();
-  const getMock = jest.fn().mockResolvedValue({ exists: true });
-  const docMock = jest.fn().mockReturnValue({ get: getMock, set: setMock });
-  const firestoreMock = jest.fn().mockReturnValue({ doc: docMock });
+  const getMock = jest.fn().mockResolvedValue({exists: true});
+  const docMock = jest.fn().mockReturnValue({get: getMock, set: setMock});
+  const firestoreMock = jest.fn().mockReturnValue({doc: docMock});
 
   // Expose mocks via module-scoped object for assertions
   return {
@@ -22,73 +22,106 @@ jest.mock('./app', () => {
   };
 });
 
-import { mirrorPresenceToFirestore } from './presence.triggers';
-// @ts-ignore
-import { __mocks__ } from './app';
+import {mirrorPresenceToFirestore} from './presence.triggers';
+// @ts-expect-error __mocks__ doesn't exist on the real app module, it's just for accessing mocks
+import {__mocks__} from './app';
+import {DataSnapshot} from 'firebase-admin/database';
+import {Change} from 'firebase-functions/v1';
 
-const testEnv = firebaseFunctionsTest({ projectId: 'deliberate-lab-test' });
+const testEnv = firebaseFunctionsTest({projectId: 'deliberate-lab-test'});
 
 describe('mirrorPresenceToFirestore', () => {
   afterEach(() => {
     jest.clearAllMocks();
   });
 
-  let wrapped: any;
-  let beforeSnap: any;
-  let afterSnap: any;
-  let change: any;
+  interface MockParent {
+    once: jest.Mock<
+      Promise<{
+        val: () => Record<string, {connected: boolean; last_changed: number}>;
+      }>
+    >;
+    child: jest.Mock<{set: jest.Mock}>;
+  }
+
+  let beforeSnap: DataSnapshot;
+  let afterSnap: DataSnapshot;
+  let change: {before: typeof beforeSnap; after: typeof afterSnap};
 
   beforeEach(() => {
-    wrapped = testEnv.wrap(mirrorPresenceToFirestore);
+    const parentVal = {
+      conn789: {connected: true, last_changed: 1234567890},
+      conn456: {connected: false, last_changed: 1234560000},
+      _aggregate: {state: 'offline', ts: 1234560000},
+    };
+    const mockParent: MockParent = {
+      once: jest.fn().mockResolvedValue({val: () => parentVal}),
+      child: jest.fn().mockReturnValue({set: jest.fn()}),
+    };
 
-    beforeSnap = testEnv.database.makeDataSnapshot(null, '/status/exp123/user456');
-    afterSnap = testEnv.database.makeDataSnapshot(
-      { connected: true, last_changed: 1234567890 },
-      '/status/exp123/user456'
-    );
+    // @ts-expect-error Type is not really compatible with DataSnapshot, but this is just a mock
+    afterSnap = {
+      ref: {parent: mockParent},
+      val: () => ({connected: true, last_changed: 1234567890}),
+    } as DataSnapshot;
 
-    change = { before: beforeSnap, after: afterSnap };
+    beforeSnap = afterSnap as DataSnapshot; // before is not actually used
+
+    change = {before: beforeSnap, after: afterSnap} as Change<DataSnapshot>;
   });
 
   it('updates firestore with the connected status from rtdb', async () => {
-    const { getMock, docMock, setMock } = __mocks__;
+    const {getMock, docMock, setMock} = __mocks__;
+    const wrapped = testEnv.wrap(mirrorPresenceToFirestore);
 
     getMock.mockResolvedValueOnce({
       exists: true,
-      data: jest.fn(() => ({ connected: true })),
+      data: jest.fn(() => ({connected: true})),
     });
 
+    // @ts-expect-error mocks don't completely duplicate the real types
     await wrapped(change, {
       params: {
         experimentId: 'exp123',
         participantPrivateId: 'user456',
+        connectionId: 'conn789',
       },
     });
 
-    expect(docMock).toHaveBeenCalledWith('experiments/exp123/participants/user456');
-    expect(setMock).toHaveBeenCalledWith({ connected: true }, { merge: true });
+    expect(docMock).toHaveBeenCalledWith(
+      'experiments/exp123/participants/user456',
+    );
+    expect(setMock).toHaveBeenCalledWith(
+      expect.objectContaining({connected: true}),
+      {merge: true},
+    );
   });
 
   it('does not mirror presence data for agent participants', async () => {
-    const { getMock, docMock, setMock } = __mocks__;
+    const {getMock, docMock, setMock} = __mocks__;
+    const wrapped = testEnv.wrap(mirrorPresenceToFirestore);
 
     const mockFirestoreProfile = {
-      agentConfig: { role: 'observer' },
+      agentConfig: {role: 'observer'},
     };
 
     getMock.mockResolvedValueOnce({
       exists: true,
       data: jest.fn(() => mockFirestoreProfile),
     });
 
+    // @ts-expect-error mocks don't completely duplicate the real types
     await wrapped(change, {
       params: {
         experimentId: 'exp123',
         participantPrivateId: 'user456',
+        connectionId: 'conn789',
       },
     });
 
-    expect(docMock).toHaveBeenCalledWith('experiments/exp123/participants/user456');
+    expect(docMock).toHaveBeenCalledWith(
+      'experiments/exp123/participants/user456',
+    );
     expect(setMock).not.toHaveBeenCalled();
   });
 });

functions/src/presence.triggers.ts

@@ -1,50 +1,99 @@
-import {database} from 'firebase-functions';
+import * as admin from 'firebase-admin';
+
+import {database, pubsub} from 'firebase-functions';
 import {app} from './app';
 
+const dbInstance = database.instance(
+  `${process.env.GCLOUD_PROJECT}-default-rtdb`,
+);
+
 /**
- * Mirror the presence status from RTDB to Firestore.
+ * Mirror presence from RTDB → Firestore and maintain an aggregate node. Use
+ * separate connection IDs to track individual connections, to properly support
+ * multiple browser tabs or devices.
  *
- * This function is triggered when the presence status of a participant changes in the RTDB.
- * It updates the corresponding participant document in Firestore with the new status.
+ * RTDB write path:
+ *   /status/{experimentId}/{participantPrivateId}/{connectionId}
  *
- * Currently, rtdb is only used in Deliberate Lab for presence tracking (using the rtdb websocket).
+ * Firestore doc path:
+ *   experiments/{experimentId}/participants/{participantPrivateId}
  */
-export const mirrorPresenceToFirestore = database
-  .instance(`${process.env.GCLOUD_PROJECT}-default-rtdb`) // other parts of firebase use the -default-rtdb suffix, so stay consistent
-  .ref('/status/{experimentId}/{participantPrivateId}') // rtdb path, not firestore path
+export const mirrorPresenceToFirestore = dbInstance
+  .ref('/status/{experimentId}/{participantPrivateId}/{connectionId}')
   .onWrite(async (change, context) => {
-    const {experimentId, participantPrivateId} = context.params;
-    console.log(
-      `mirrorPresenceToFirestore triggered for experimentId=${experimentId} participantPrivateId=${participantPrivateId}`,
-    );
-    const status = change.after.val();
+    const {experimentId, participantPrivateId, connectionId} = context.params;
 
-    if (!status) return null; // status was deleted
+    if (connectionId.startsWith('_')) return null;
 
-    // Find the matching participant doc
-    const participantRef = app
+    const parentRef = change.after.ref.parent; // participantPrivateId
+    const aggRef = parentRef!.child('_aggregate');
+    const fsRef = app
       .firestore()
       .doc(`experiments/${experimentId}/participants/${participantPrivateId}`);
-    const participantSnapshot = await participantRef.get();
 
-    if (!participantSnapshot.exists) {
+    const siblingsSnapshot = await parentRef!.once('value');
+
+    let online = false;
+    for (const key in siblingsSnapshot.val()) {
+      if (key.startsWith('_')) {
+        // ignore _aggregate, future meta-nodes
+        continue;
+      }
+
+      if (siblingsSnapshot.val()[key].connected) {
+        online = true;
+        break;
+      }
+    }
+
+    await aggRef.set({
+      state: online ? 'online' : 'offline',
+      ts: admin.database.ServerValue.TIMESTAMP,
+    });
+
+    const snapshot = await fsRef.get();
+    if (!snapshot.exists) {
       console.warn(
-        `No participant found with id=${participantPrivateId} in experiment=${experimentId}`,
+        `No participant ${participantPrivateId} in experiment ${experimentId}`,
       );
       return null;
     }
-    const participant = participantSnapshot.data();
-
-    if (participant && participant.agentConfig) {
-      return null; // Don't update presence for agent participants
-    }
-
-    // Temporarily prevent participants from switching from connected
-    // to disconnected (PR #537)
-    // TODO: Remove this logic to resume actual presence detection
-    if (!status.connected) {
+    if (snapshot.data()?.agentConfig) {
+      // Skip bot/agent participants
       return null;
     }
 
-    return participantRef.set({connected: status.connected}, {merge: true});
+    return fsRef.set(
+      {
+        connected: online,
+        last_changed: admin.firestore.FieldValue.serverTimestamp(),
+      },
+      {merge: true},
+    );
+  });
+
+export const scrubStalePresence = pubsub
+  .schedule('every 24 hours')
+  .onRun(async () => {
+    const cutoff = Date.now() - 72 * 60 * 60 * 1000; // 72 hours
+    const root = admin.app().database().ref('status');
+    const usersSnapshot = await root.get();
+    const userSnapshots: admin.database.DataSnapshot[] = [];
+    for (const userSnapshot of Object.values(usersSnapshot.val() || {})) {
+      userSnapshots.push(userSnapshot as admin.database.DataSnapshot);
+    }
+    for (const userSnapshot of userSnapshots) {
+      const connSnapshots: admin.database.DataSnapshot[] = [];
+      userSnapshot.forEach((connSnapshot) => {
+        connSnapshots.push(connSnapshot);
+      });
+      for (const connSnapshot of connSnapshots) {
+        if (
+          !connSnapshot.key!.startsWith('_') &&
+          connSnapshot.child('ts').val() < cutoff
+        ) {
+          connSnapshot.ref.remove();
+        }
+      }
+    }
   });