fix(admin): live-refresh map annotations in monitor

[gelbh]

Summary

• Gate useSessionSync on authReady so admin permanent auth completes before Firestore subscriptions
• Clear annotation store when resubscribing to a new session
• Emulator test for delayed syncEnabled

Test plan

• npx vitest related --run src/hooks/session/useSessionSync.ts
• npm run typecheck
• Admin embedded monitor shows new annotations without re-selecting session

Summary by CodeRabbit

• Bug Fixes
  • Session synchronization now waits for authentication readiness before applying remote updates.
  • Added the ability to disable session synchronization and correctly stop/resume remote annotation syncing.
  • Remote annotations now start from a clean baseline to prevent stale or partial data.
• New Features
  • useSessionSync now supports a syncEnabled option to control synchronization.
• Tests
  • Added coverage for the sync-enabled gate and ensured remote annotations remain unavailable while disabled, then appear when enabled.
  • Improved emulator test cleanup and reduced reliance on fixed timing.

[coderabbitai]

Walkthrough

Adds authentication-gated session synchronization, updates annotation and offline-queue effects for enablement changes and cleanup, and adds tests covering synchronization toggling and session readiness transitions.

Changes

Session synchronization gating

Layer / File(s)	Summary
Configurable synchronization effects src/hooks/session/useSessionSync.ts, src/hooks/session/useSessionSync.emulator.test.ts	useSessionSync accepts optional syncEnabled, gates subscriptions and queue flushing, resets annotations during baseline setup, protects async cleanup, and tests enablement transitions.
Authentication-readiness integration src/hooks/session/useSharedSessionScreen.ts, src/hooks/session/useSharedSessionScreen.test.ts	Shared-session synchronization is enabled from authentication readiness, with tests verifying changes when the active session changes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• gelbh/jetlag#9: Also gates shared-session synchronization based on authentication readiness.
• gelbh/jetlag#24: Also changes remote synchronization and offline queue flushing in useSessionSync.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/admin-annotation-refresh

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/hooks/session/useSessionSync.ts (1)

68-79: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Reset annotation state on session changes. useSessionSync returns before replaceAnnotations([]) while syncEnabled is false, so the previous session’s annotations can remain visible until auth completes. setAnnotations([]) also leaves redoAnnotationIds, selectedAnnotationId, geometryEditAnnotationId, and pulsingAnnotationIds intact.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/session/useSessionSync.ts` around lines 68 - 79, Reset all
annotation-related state whenever useSessionSync detects a session change or
disabled sync, before the early return; use the complete annotation reset path
rather than only replaceAnnotations([]), clearing redoAnnotationIds,
selectedAnnotationId, geometryEditAnnotationId, and pulsingAnnotationIds as
well.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hooks/session/useSessionSync.emulator.test.ts`:
- Around line 97-125: Replace the fixed 200 ms delay in the “waits for
syncEnabled before subscribing to annotations” test with a deterministic
behavior-focused check at the subscription boundary, such as spying on or
controlling the annotation subscription invoked by useSessionSync. Assert no
subscription occurs while enabled is false, then rerender with enabled true and
assert the subscription is created and the annotation is received, covering both
disabled and enabled states without timing-based synchronization.
- Around line 97-126: Extend the “waits for syncEnabled before subscribing to
annotations” test to seed stale annotations and verify they are cleared when
subscribing to a new session, then rerender from enabled=true to enabled=false
and confirm subsequent remote annotation changes no longer reach the annotation
store. Preserve the existing false-to-true assertion while covering both the
new-session baseline reset and unsubscribe transition.

In `@src/hooks/session/useSessionSync.ts`:
- Around line 151-153: Update the queue-flush useEffect around flushQueue so
each effect instance has an epoch or abort guard that invalidates on cleanup.
Ensure in-flight flushQueue work checks that guard before calling
flushOfflineQueue or updating pendingWrites and lastSyncError, preventing
disposed effects from writing or mutating shared sync state after syncEnabled or
session changes.

In `@src/hooks/session/useSharedSessionScreen.ts`:
- Around line 118-119: Update the session/admin-auth transition handling in the
hook containing useSessionSync so permanentAuthReady is reset whenever the
relevant session or admin-permanent state changes. Track the active
waitForPermanentAuthReady invocation and ignore completions from stale promises,
ensuring authReady remains false until the current transition’s readiness check
resolves before useSessionSync({ syncEnabled: authReady }) can enable syncing.

---

Outside diff comments:
In `@src/hooks/session/useSessionSync.ts`:
- Around line 68-79: Reset all annotation-related state whenever useSessionSync
detects a session change or disabled sync, before the early return; use the
complete annotation reset path rather than only replaceAnnotations([]), clearing
redoAnnotationIds, selectedAnnotationId, geometryEditAnnotationId, and
pulsingAnnotationIds as well.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b842ad07-cb4b-4091-bfb0-d2588c74a9fa

📥 Commits

Reviewing files that changed from the base of the PR and between a0d3cac and 8d1c124.

📒 Files selected for processing (3)

• src/hooks/session/useSessionSync.emulator.test.ts
• src/hooks/session/useSessionSync.ts
• src/hooks/session/useSharedSessionScreen.ts

[gelbh]

@coderabbitai approve

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/hooks/session/useSharedSessionScreen.ts (1)

67-85: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Async auth-readiness chain still lacks a cancellation/epoch guard.

Switching to permanentAuthSessionId === session?.id fixes the case where an older session's readiness resolves before the new one (correctly stays false). It does not fix the reverse ordering: if the current session's promise resolves first (enabling sync) and a stale, still-in-flight promise from a previous session later resolves, it overwrites permanentAuthSessionId back to the old id — flipping authReady/syncEnabled back to false for the already-ready, current session. Since useSessionSync now clears the annotation store and unsubscribes on every syncEnabled transition, this would cause visible annotation flicker/loss for an admin monitor that was already correctly synced. The effect also reruns on every session object change (not just id change), increasing the number of overlapping, uncancelled waitForPermanentAuthReady() calls.

Track an epoch/generation counter (or an AbortController-style flag) per effect run, and ignore .then() completions from stale invocations:

🔧 Suggested guard

   useEffect(() => {
     if (authMode === "admin-permanent") {
       if (
         !session ||
         session.id === LOCAL_SESSION_ID ||
         !isFirebaseConfigured()
       ) {
         return;
       }

+      let cancelled = false;
       void waitForPermanentAuthReady().then(() => {
+        if (cancelled) return;
         const currentUser = getFirebaseAuth().currentUser;
         if (myUid && currentUser && currentUser.uid !== myUid) {
           setLastSyncError("No access to this session.");
         }
         setPermanentAuthSessionId(session.id);
       });
-      return;
+      return () => {
+        cancelled = true;
+      };
     }

This is the same root cause previously flagged ("Reset permanentAuthReady on admin-auth/session transitions") and appears not to have been marked resolved.

#!/bin/bash
# Inspect waitForPermanentAuthReady to assess whether repeated calls can have
# out-of-order/variable-latency resolution (vs. a cached, already-resolved promise).
rg -n "function waitForPermanentAuthReady|export.*waitForPermanentAuthReady" -A 20

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/session/useSharedSessionScreen.ts` around lines 67 - 85, The
permanent-auth readiness effect can apply stale promise completions from
previous runs. Update the effect containing waitForPermanentAuthReady to track
an epoch or cancellation flag per invocation, invalidate it during cleanup, and
only perform setLastSyncError and setPermanentAuthSessionId when the completion
belongs to the latest active effect run; avoid overlapping work from session
object changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hooks/session/useSharedSessionScreen.test.ts`:
- Around line 116-142: Add a test alongside “resets admin sync gating when
monitored session changes” that uses separate deferred promises for each
session’s waitForPermanentAuthReady call, resolves the newer session before the
older one, and verifies the final useSessionSyncMock call keeps syncEnabled set
for the current session after the stale promise resolves. Ensure the test
rerenders after changing mockSession and covers the out-of-order resolution
guard in useSharedSessionScreen.

---

Outside diff comments:
In `@src/hooks/session/useSharedSessionScreen.ts`:
- Around line 67-85: The permanent-auth readiness effect can apply stale promise
completions from previous runs. Update the effect containing
waitForPermanentAuthReady to track an epoch or cancellation flag per invocation,
invalidate it during cleanup, and only perform setLastSyncError and
setPermanentAuthSessionId when the completion belongs to the latest active
effect run; avoid overlapping work from session object changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6ddbd410-3cee-4727-83bf-6eaa9e4c837d

📥 Commits

Reviewing files that changed from the base of the PR and between 8d1c124 and 079e103.

📒 Files selected for processing (4)

• src/hooks/session/useSessionSync.emulator.test.ts
• src/hooks/session/useSessionSync.ts
• src/hooks/session/useSharedSessionScreen.test.ts
• src/hooks/session/useSharedSessionScreen.ts

[coderabbitai]

✅ Action performed

Comments resolved and changes approved.