fix(federation): gate registry PUT on successful initial GET

Without a sync-ready gate, a transient GET failure during autoConnectAll left the local registry Map empty/incomplete while `set()` still computed `registryUpdatedAt = Date.now()`. The trailing `syncRegistry()` would then PUT the empty payload with a fresh timestamp; the server's LWW guard accepted it and legitimate registry rows were wiped — including remote-instance entries the user never explicitly removed. Add `_registrySyncReady` flag, set true only after a successful initial GET. `syncRegistry()` short-circuits while false, so the degraded mode (GET failed) is display-only: the Map is still populated locally from \`replicatedInstances\` synthesis (status \`auth_expired\`) so the Connections UI shows the user's known remotes, but mutations don't push. On the next session where GET succeeds, localStorage cached tokens reseed the registry and \`syncRegistry()\` pushes the merged authoritative state — no data lost, sync deferred until we have a complete picture. \`reset()\` clears the flag alongside the registry. Spec updated with the sync-ready gate and degraded-mode behavior. Unit tests cover the gate on initial fail, mutations during degraded mode, recovery on next successful GET, and the synthesis fallback for empty replicatedInstances.
2026-05-05 00:20:51 +02:00
parent c05c04181d
commit 06498836bb
3 changed files with 204 additions and 6 deletions
@@ -347,7 +347,20 @@ Client-driven LWW whole-registry push (same pattern as `profileSync.ts`):
 1. User mutates registry → `registryUpdatedAt = Date.now()`
 2. Client calls `PUT /api/users/@me/federation-registry` on all connected instances
 3. Server rejects if `updatedAt <= stored` (409 Conflict)
-4. On startup, client fetches registry from home via `GET`, merges with localStorage tokens
+4. On startup, client fetches registry from home via `GET`, merges with localStorage tokens, and seeds any `replicatedInstances` entries that aren't yet in the registry (with status `auth_expired`) so users with pre-feature data — or whose initial GET failed — still see their connections in the UI
+
+### Sync-Ready Gate
+
+`PUT` is gated behind an in-memory `_registrySyncReady` flag that is set true **only after a successful initial GET** in `autoConnectAll`. Until that flag flips, `syncRegistry()` is a no-op (and `autoConnectAll` does not call it).
+
+**Why:** without this gate, a transient GET failure would leave the local Map empty/incomplete, but `set()` would still compute `registryUpdatedAt = Date.now()` (since `serverRegistryUpdatedAt = 0`). The trailing `syncRegistry()` would PUT the empty payload with a fresh-now timestamp; the server's LWW guard (`updatedAt > stored`) accepts it, and legitimate registry rows are wiped — including remote-instance entries the user never explicitly removed.
+
+**Degraded mode (GET failed):**
+- Registry Map is populated locally from `replicatedInstances` synthesis (display-only) so the UI still shows the user's known remotes as `auth_expired`.
+- Mutations (`connectToRemote`, `disconnectInstance`, `reconnectInstance`, etc.) still update the local Map but **do not push** to home — `syncRegistry()` short-circuits.
+- On the next session where GET succeeds, `localStorage` cached tokens reseed the registry and `syncRegistry()` pushes the merged authoritative state. No data is lost; sync is just deferred until we have a complete picture to merge against.
+
+`reset()` (logout/account switch) clears `_registrySyncReady` along with the registry Map.

 ### API

@@ -0,0 +1,141 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+
+const getFederationRegistry = vi.fn();
+const putFederationRegistry = vi.fn(async () => ({ ok: true, updatedAt: 1 }));
+const ensurePeered = vi.fn(async () => ({ peeringStatus: 'active' }));
+
+vi.mock('../api/client', () => ({
+  api: {
+    users: {
+      getFederationRegistry: () => getFederationRegistry(),
+      putFederationRegistry: (data: unknown) => putFederationRegistry(data),
+      me: vi.fn(),
+    },
+    federation: {
+      ensurePeered: (data: unknown) => ensurePeered(data),
+    },
+  },
+  createApiClient: () => ({}),
+}));
+
+vi.mock('../hooks/useWebSocket', () => ({
+  connectInstance: vi.fn(),
+  disconnectInstance: vi.fn(),
+  disconnectAllRemote: vi.fn(),
+}));
+vi.mock('../utils/dmOriginFailover', () => ({
+  failoverDmOriginsFromDisconnected: vi.fn(),
+}));
+vi.mock('../utils/federationOps', () => ({ clearPasswordSyncTimers: vi.fn() }));
+vi.mock('../audio/AudioManager', () => ({
+  AudioManager: { getInstance: vi.fn().mockReturnValue({ setOutputDevice: vi.fn(), setVolume: vi.fn() }) },
+}));
+
+const mockUser = {
+  id: 'user-1',
+  username: 'youruser',
+  homeInstance: null,
+  homeUserId: 'user-1',
+  replicatedInstances: [
+    { origin: 'https://orbit.example', username: 'youruser@nova.example' },
+  ],
+};
+
+vi.mock('./authStore', () => ({
+  useAuthStore: Object.assign(
+    (selector: (s: unknown) => unknown) => selector({ user: mockUser, token: 'tok' }),
+    { getState: () => ({ user: mockUser, token: 'tok' }), setState: vi.fn(), subscribe: vi.fn() }
+  ),
+}));
+
+import { useInstanceStore } from './instanceStore';
+
+beforeEach(() => {
+  getFederationRegistry.mockReset();
+  putFederationRegistry.mockClear();
+  ensurePeered.mockClear();
+  localStorage.clear();
+  useInstanceStore.setState({
+    instances: [],
+    registry: new Map(),
+    registryUpdatedAt: 0,
+    _autoConnectDone: false,
+    _registrySyncReady: false,
+    pendingSyncOrigins: [],
+  });
+});
+
+describe('instanceStore registry sync gating', () => {
+  it('does NOT PUT registry when initial GET fails (prevents empty-clobber)', async () => {
+    getFederationRegistry.mockRejectedValueOnce(new Error('network'));
+
+    await useInstanceStore.getState().autoConnectAll();
+
+    expect(putFederationRegistry).not.toHaveBeenCalled();
+    expect(useInstanceStore.getState()._registrySyncReady).toBe(false);
+  });
+
+  it('synthesizes replicatedInstances entries when GET fails so UI is not empty', async () => {
+    getFederationRegistry.mockRejectedValueOnce(new Error('network'));
+
+    await useInstanceStore.getState().autoConnectAll();
+
+    const reg = useInstanceStore.getState().registry;
+    expect(reg.has('https://orbit.example')).toBe(true);
+    expect(reg.get('https://orbit.example')?.status).toBe('auth_expired');
+  });
+
+  it('PUTs registry after a successful initial GET and flips _registrySyncReady', async () => {
+    getFederationRegistry.mockResolvedValueOnce({
+      registry: [{
+        origin: 'https://orbit.example',
+        label: 'Orbit',
+        username: 'youruser@nova.example',
+        remoteUserId: 'remote-1',
+        status: 'auth_expired',
+        addedAt: 1,
+        lastConnectedAt: 1,
+        disconnectedAt: null,
+        errorMessage: null,
+      }],
+      updatedAt: 100,
+    });
+
+    await useInstanceStore.getState().autoConnectAll();
+
+    expect(useInstanceStore.getState()._registrySyncReady).toBe(true);
+    expect(putFederationRegistry).toHaveBeenCalledTimes(1);
+    const payload = putFederationRegistry.mock.calls[0]![0] as { registry: unknown[] };
+    expect(payload.registry).toHaveLength(1);
+  });
+
+  it('syncRegistry no-ops while _registrySyncReady is false (post-failure mutation)', async () => {
+    getFederationRegistry.mockRejectedValueOnce(new Error('network'));
+    await useInstanceStore.getState().autoConnectAll();
+    putFederationRegistry.mockClear();
+
+    useInstanceStore.setState({
+      registry: new Map([['https://other.example', {
+        origin: 'https://other.example',
+        label: 'Other', username: 'u', remoteUserId: '',
+        status: 'connected' as const,
+        addedAt: 1, lastConnectedAt: 1, disconnectedAt: null, errorMessage: null,
+      }]]),
+      registryUpdatedAt: Date.now(),
+    });
+
+    await useInstanceStore.getState().syncRegistry();
+
+    expect(putFederationRegistry).not.toHaveBeenCalled();
+  });
+
+  it('reset clears _registrySyncReady', async () => {
+    getFederationRegistry.mockResolvedValueOnce({ registry: [], updatedAt: 1 });
+    await useInstanceStore.getState().autoConnectAll();
+    expect(useInstanceStore.getState()._registrySyncReady).toBe(true);
+
+    useInstanceStore.getState().reset();
+
+    expect(useInstanceStore.getState()._registrySyncReady).toBe(false);
+  });
+});
@@ -173,6 +173,10 @@ interface InstanceState {
  pendingSyncOrigins: string[];
  registry: Map<string, FederationRegistryEntry>;
  registryUpdatedAt: number;
+  // True once we've successfully fetched the authoritative registry from the
+  // home server at least once this session. Until then, syncRegistry() must
+  // not PUT — our local view is incomplete and would clobber server state.
+  _registrySyncReady: boolean;
  syncRegistry: () => Promise<void>;
  deleteIdentity: (origins: string[], mode?: 'leave' | 'soft' | 'full') => Promise<Record<string, { success: boolean; error?: string; ownedSpaces?: { id: string; name: string }[] }>>;
  forceRemoveEntry: (origin: string) => void;
@@ -200,6 +204,7 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({
  pendingSyncOrigins: [],
  registry: new Map(),
  registryUpdatedAt: 0,
+  _registrySyncReady: false,

  probeInstance: async (url: string) => {
    const origin = normalizeOrigin(url);
@@ -706,6 +711,11 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({

  syncRegistry: async () => {
    if (!get()._autoConnectDone) return;
+    // Block PUT until we've successfully read the authoritative server registry
+    // at least once. Otherwise a transient GET failure during autoConnectAll
+    // would let us push an empty/incomplete registry with a fresh timestamp,
+    // wiping legitimate server-side entries via LWW.
+    if (!get()._registrySyncReady) return;

    const { registry, registryUpdatedAt, instances } = get();
    const currentUser = useAuthStore.getState().user;
@@ -784,10 +794,12 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({
    // Fetch server-side registry (source of truth for entry list)
    let serverRegistry: FederationRegistryEntry[] = [];
    let serverRegistryUpdatedAt = 0;
+    let serverRegistryFetched = false;
    try {
      const res = await api.users.getFederationRegistry();
      serverRegistry = res.registry;
      serverRegistryUpdatedAt = res.updatedAt;
+      serverRegistryFetched = true;
    } catch (err) {
      console.warn('Failed to fetch federation registry from home:', err);
    }
@@ -798,6 +810,29 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({
      registry.set(entry.origin, entry);
    }

+    // Seed any replicatedInstances that aren't yet in the registry. This covers
+    // (a) accounts whose remotes were added before the federation registry table
+    // existed, and (b) the GET-failed degraded mode where we still want the user
+    // to see their known connections (as auth_expired) instead of an empty list.
+    // These synthesized entries are display-only until the next successful GET
+    // — we never PUT while _registrySyncReady is false.
+    for (const ri of currentUser.replicatedInstances) {
+      const origin = ri.origin || `https://${ri.domain}`;
+      if (isSelfOrigin(origin)) continue;
+      if (registry.has(origin)) continue;
+      registry.set(origin, {
+        origin,
+        label: new URL(origin).host,
+        username: ri.username || '',
+        remoteUserId: '',
+        status: 'auth_expired',
+        addedAt: Date.now(),
+        lastConnectedAt: null,
+        disconnectedAt: null,
+        errorMessage: 'Re-authenticate to connect',
+      });
+    }
+
    // Migration: promote localStorage-only entries to registry
    for (const [origin] of Object.entries(cached)) {
      if (origin === window.location.origin) continue;
@@ -1066,12 +1101,21 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({
      .filter(([, v]) => v.pendingPasswordSync)
      .map(([origin]) => origin);

-    // Persist reconciled registry
+    // Persist reconciled registry. _registrySyncReady gates outbound PUTs:
+    // only flip true when we've authoritatively read from the home server.
    const registryUpdatedAt = serverRegistryUpdatedAt > 0 ? Math.max(serverRegistryUpdatedAt, Date.now()) : Date.now();
-    set({ _autoConnectDone: true, pendingSyncOrigins: pendingOrigins, registry, registryUpdatedAt });
+    set({
+      _autoConnectDone: true,
+      pendingSyncOrigins: pendingOrigins,
+      registry,
+      registryUpdatedAt,
+      _registrySyncReady: serverRegistryFetched,
+    });

-    // Sync reconciled registry to all instances
-    get().syncRegistry().catch(() => {});
+    // Sync reconciled registry to all instances (no-ops if fetch failed)
+    if (serverRegistryFetched) {
+      get().syncRegistry().catch(() => {});
+    }
  },

  reset: () => {
@@ -1086,7 +1130,7 @@ export const useInstanceStore = create<InstanceState>((set, get) => ({

    clearPasswordSyncTimers();

-    set({ instances: [], isLoading: false, error: null, _autoConnectDone: false, pendingSyncOrigins: [], registry: new Map(), registryUpdatedAt: 0 });
+    set({ instances: [], isLoading: false, error: null, _autoConnectDone: false, pendingSyncOrigins: [], registry: new Map(), registryUpdatedAt: 0, _registrySyncReady: false });
    // Token cache preserved — scoped per user, survives logout for seamless reconnect
  },
 }));