From 06498836bb5de4a67557476d1c6e722671a8e63e Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Tue, 5 May 2026 00:20:51 +0200 Subject: [PATCH] fix(federation): gate registry PUT on successful initial GET MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/systems/client-federation.md | 15 +- .../stores/instanceStore.registrySync.test.ts | 141 ++++++++++++++++++ packages/web/src/stores/instanceStore.ts | 54 ++++++- 3 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 packages/web/src/stores/instanceStore.registrySync.test.ts diff --git a/docs/systems/client-federation.md b/docs/systems/client-federation.md index 08a50082..b8c25233 100644 --- a/docs/systems/client-federation.md +++ b/docs/systems/client-federation.md @@ -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 diff --git a/packages/web/src/stores/instanceStore.registrySync.test.ts b/packages/web/src/stores/instanceStore.registrySync.test.ts new file mode 100644 index 00000000..7af40561 --- /dev/null +++ b/packages/web/src/stores/instanceStore.registrySync.test.ts @@ -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); + }); +}); diff --git a/packages/web/src/stores/instanceStore.ts b/packages/web/src/stores/instanceStore.ts index 78264fcd..5ecd3a19 100644 --- a/packages/web/src/stores/instanceStore.ts +++ b/packages/web/src/stores/instanceStore.ts @@ -173,6 +173,10 @@ interface InstanceState { pendingSyncOrigins: string[]; registry: Map; 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; deleteIdentity: (origins: string[], mode?: 'leave' | 'soft' | 'full') => Promise>; forceRemoveEntry: (origin: string) => void; @@ -200,6 +204,7 @@ export const useInstanceStore = create((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((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((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((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((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((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 }, }));