diff --git a/desktop/src/shared/theme/CommunityThemeController.tsx b/desktop/src/shared/theme/CommunityThemeController.tsx index c002227564..bc76f8bdb9 100644 --- a/desktop/src/shared/theme/CommunityThemeController.tsx +++ b/desktop/src/shared/theme/CommunityThemeController.tsx @@ -14,6 +14,7 @@ import { markCommunityThemeMigrated, readCommunityThemeOutbox, readCommunityThemePreference, + refreshCommunityThemeAppearanceSnapshot, sameCommunityThemePreference, writeCommunityThemeOutbox, writeCommunityThemePreference, @@ -289,6 +290,25 @@ export function CommunityThemeController() { communityThemeAppearanceFallback(appearanceSnapshotRef.current), ); if (stored && sameCommunityThemePreference(stored, preference)) return; + // A genuine user edit that changes glass/opacity/prominent-tab is the + // user's current profile-wide appearance intent, so refresh the snapshot + // that seeds no-record communities. Without this the snapshot stays frozen + // at the pre-migration value and a later empty community resurrects it over + // the user's current choice. Only a changed appearance refreshes it, so an + // accent- or theme-only edit never leaks this community's glass into the + // profile-wide seed. + const priorScoped = scopedPreferenceRef.current; + if ( + !priorScoped || + priorScoped.glassBackground !== preference.glassBackground || + priorScoped.glassOpacity !== preference.glassOpacity || + priorScoped.prominentActiveTab !== preference.prominentActiveTab + ) { + appearanceSnapshotRef.current = refreshCommunityThemeAppearanceSnapshot( + pubkey, + preference, + ); + } scopedPreferenceRef.current = preference; if (!writeCommunityThemePreference(pubkey, relayUrl, preference)) return; if (!writeCommunityThemeOutbox(pubkey, relayUrl, preference)) return; diff --git a/desktop/src/shared/theme/communityThemePreference.test.mjs b/desktop/src/shared/theme/communityThemePreference.test.mjs index 2cb852082d..e565f3888b 100644 --- a/desktop/src/shared/theme/communityThemePreference.test.mjs +++ b/desktop/src/shared/theme/communityThemePreference.test.mjs @@ -16,6 +16,7 @@ import { readCommunityThemeAppearanceSnapshot, readCommunityThemeOutbox, readCommunityThemePreference, + refreshCommunityThemeAppearanceSnapshot, sameCommunityThemePreference, writeCommunityThemeOutbox, writeCommunityThemePreference, @@ -237,6 +238,93 @@ test("a full store still yields the correct in-session appearance fallback", () }); }); +test("a full store pins the snapshot across a controller remount", () => { + // The controller is remounted per community under a keyed provider, so a + // ref cannot carry the snapshot between mounts. When the store is full the + // capture must still return the profile's first-seen value on the next + // mount, rather than re-capturing the current (previous community's) + // appearance. This pins that the in-memory fallback has profile lifetime. + globalThis.window = { + localStorage: { + getItem: () => null, + setItem: () => { + throw new Error("quota exceeded"); + }, + removeItem: () => {}, + }, + }; + const firstMount = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }; + const first = captureCommunityThemeAppearanceSnapshot( + "remount-pk", + firstMount, + ); + assert.deepEqual(first, { + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }); + + // The next mount is a different community whose applied appearance differs. + // Capture must return the pinned first value, not this community's. + const secondMount = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: false, + glassOpacity: 30, + prominentActiveTab: false, + }; + assert.deepEqual( + captureCommunityThemeAppearanceSnapshot("remount-pk", secondMount), + first, + ); +}); + +test("a user glass edit refreshes the snapshot for no-record communities", () => { + // The initial snapshot is frozen at the pre-migration appearance. Once the + // user changes glass, that former value is stale: a no-record community must + // inherit the new choice, not resurrect the old one. refresh overwrites the + // durable snapshot so the empty-scope fallback tracks the current choice. + globalThis.window = { localStorage: localStorageStub() }; + const preMigration = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }; + const pinned = captureCommunityThemeAppearanceSnapshot( + "edit-pk", + preMigration, + ); + assert.deepEqual(pinned, { + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }); + + const edited = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: false, + glassOpacity: 40, + prominentActiveTab: false, + }; + const refreshed = refreshCommunityThemeAppearanceSnapshot("edit-pk", edited); + assert.deepEqual(refreshed, { + glassBackground: false, + glassOpacity: 40, + prominentActiveTab: false, + }); + // A subsequent capture (a later community mount) sees the refreshed value, + // so the no-record fallback carries the user's current choice. + assert.deepEqual( + captureCommunityThemeAppearanceSnapshot("edit-pk", preMigration), + refreshed, + ); +}); + test("desktop appearance limits match the shared wire contract", () => { const contract = JSON.parse( readFileSync( diff --git a/desktop/src/shared/theme/communityThemePreference.ts b/desktop/src/shared/theme/communityThemePreference.ts index 0b13b50402..ae3cb52bc8 100644 --- a/desktop/src/shared/theme/communityThemePreference.ts +++ b/desktop/src/shared/theme/communityThemePreference.ts @@ -228,14 +228,18 @@ export function readCommunityThemeAppearanceSnapshot( } } +const inMemoryAppearanceSnapshots = new Map(); + /** * Persist the profile's pre-migration appearance the first time it is seen and * return the durable snapshot. Writing once fixes the value while the live * global appearance keys are still authoritative, so later community switches — * which now rewrite those keys per community — cannot corrupt the seed the * per-record migration inherits from. Subsequent calls return the stored - * snapshot unchanged; a full storage falls back to the live appearance for the - * current session without pinning a wrong value. + * snapshot unchanged; a full storage falls back to a module-level in-memory + * cache so the profile's first-seen value still survives the per-community + * controller remount, rather than being re-captured from a later community's + * already-rewritten appearance. */ export function captureCommunityThemeAppearanceSnapshot( pubkey: string, @@ -243,6 +247,8 @@ export function captureCommunityThemeAppearanceSnapshot( ): CommunityThemeAppearance { const existing = readCommunityThemeAppearanceSnapshot(pubkey); if (existing) return existing; + const cached = inMemoryAppearanceSnapshots.get(pubkey); + if (cached) return cached; const snapshot: CommunityThemeAppearance = { glassBackground: appearance.glassBackground, glassOpacity: appearance.glassOpacity, @@ -254,7 +260,43 @@ export function captureCommunityThemeAppearanceSnapshot( JSON.stringify(snapshot), ); } catch { - // The live appearance still seeds this session even when storage is full. + // A full store cannot durably pin the snapshot, so retain it in memory for + // the life of this process. Without this the next community remount finds + // neither a stored snapshot nor this value and re-captures the current + // (previous community's) appearance as the seed. + inMemoryAppearanceSnapshots.set(pubkey, snapshot); + } + return snapshot; +} + +/** + * Overwrite the profile's appearance snapshot with a later explicit user glass + * choice. The initial snapshot is frozen at the pre-migration appearance, but + * once the user changes glass/opacity/prominent-tab that former value is stale: + * a no-record community would otherwise resurrect it and republish it over the + * user's current choice. Callers refresh only on a genuine user edit, never on + * a programmatic per-community apply, so a community's own record can never + * leak into the profile-wide seed. + */ +export function refreshCommunityThemeAppearanceSnapshot( + pubkey: string, + appearance: CommunityThemeAppearance, +): CommunityThemeAppearance { + const snapshot: CommunityThemeAppearance = { + glassBackground: appearance.glassBackground, + glassOpacity: appearance.glassOpacity, + prominentActiveTab: appearance.prominentActiveTab, + }; + try { + window.localStorage.setItem( + communityThemeAppearanceSnapshotKey(pubkey), + JSON.stringify(snapshot), + ); + inMemoryAppearanceSnapshots.delete(pubkey); + } catch { + // Retain the refreshed value in memory when the store is full, mirroring + // the capture path so the current choice still survives a remount. + inMemoryAppearanceSnapshots.set(pubkey, snapshot); } return snapshot; } diff --git a/mobile/lib/shared/theme/community_theme_preference.dart b/mobile/lib/shared/theme/community_theme_preference.dart index 52cd87619c..a88990813d 100644 --- a/mobile/lib/shared/theme/community_theme_preference.dart +++ b/mobile/lib/shared/theme/community_theme_preference.dart @@ -138,15 +138,16 @@ class CommunityThemePreference { includesGlassOpacity && includesProminentActiveTab; - /// Fold [source]'s desktop-only appearance into this preference. Used when - /// republishing a mobile edit that carries no desktop opinion so the relay - /// coordinate keeps the glass and prominent-tab values a desktop client - /// authored. A preference that already includes the fields is returned - /// unchanged; only the fields [source] actually carries are adopted. + /// Adopt [source]'s desktop-only appearance in place of this preference's. + /// Mobile has no UI that authors glass or prominent-tab, so those fields are + /// never a local opinion — they are only ever a copy of what was hydrated. + /// A cached "full" preference therefore carries stale desktop fields once + /// another client changes them, so republishing a mobile edit must take the + /// desktop-only fields from the latest observed coordinate rather than trust + /// its own cache; otherwise it silently replays stale glass over the relay. CommunityThemePreference mergeDesktopAppearanceFrom( CommunityThemePreference source, ) { - if (includesDesktopAppearance) return this; return CommunityThemePreference( version: version, theme: theme, diff --git a/mobile/lib/shared/theme/community_theme_sync.dart b/mobile/lib/shared/theme/community_theme_sync.dart index 9e4cb0aad2..944e62efb7 100644 --- a/mobile/lib/shared/theme/community_theme_sync.dart +++ b/mobile/lib/shared/theme/community_theme_sync.dart @@ -49,6 +49,7 @@ class CommunityThemeSyncManager { Timer? _publishTimer; Timer? _subscriptionRetryTimer; + Timer? _hydrationRecoveryTimer; void Function()? _unsubscribe; CommunityThemePreference? _pending; CommunityThemePreference? _lastPublished; @@ -57,6 +58,7 @@ class CommunityThemeSyncManager { RemoteCommunityTheme? _lastRemote; int _subscriptionEpoch = 0; int _subscriptionRetryAttempt = 0; + int _hydrationRecoveryAttempt = 0; int _publishRetryAttempt = 0; bool _publishInFlight = false; bool _publishRequestedWhileInFlight = false; @@ -179,6 +181,48 @@ class CommunityThemeSyncManager { }); } + /// Retry the history query while an edit is held by the pre-hydration gate + /// and the coordinate is still unobserved. Without this, an initial + /// `fetchRemote` that returns `unavailable` (a transient history failure) + /// while the live subscription stays quiet would strand the edit forever: + /// neither `initialize`'s absence path nor `_accept` ever runs, so + /// `_hydrationObserved` never flips and every later [flush] returns early. + void _scheduleHydrationRecovery() { + if (_disposed || + _hydrationObserved || + _hydrationRecoveryTimer != null || + _pending == null) { + return; + } + final multiplier = 1 << min(_hydrationRecoveryAttempt, 5); + _hydrationRecoveryAttempt++; + _hydrationRecoveryTimer = Timer(subscriptionRetryBase * multiplier, () { + _hydrationRecoveryTimer = null; + unawaited(_recoverHydration()); + }); + } + + Future _recoverHydration() async { + if (_disposed || _hydrationObserved || _pending == null) return; + final result = await fetchRemote(); + if (_disposed || _hydrationObserved) return; + if (result.status == CommunityThemeRemoteStatus.valid) { + _accept(result.remote!); + } else if (result.status == CommunityThemeRemoteStatus.absent) { + _observeHydration(); + } else { + // Still invalid or unavailable: keep retrying with backoff until the + // coordinate is observed or the edit is superseded/cancelled. + _scheduleHydrationRecovery(); + } + } + + void _cancelHydrationRecovery() { + _hydrationRecoveryTimer?.cancel(); + _hydrationRecoveryTimer = null; + _hydrationRecoveryAttempt = 0; + } + Future _recoverLiveSubscription() async { if (_disposed) return; if (!await _startLiveSubscription()) return; @@ -216,16 +260,15 @@ class CommunityThemeSyncManager { /// Record that the relay coordinate has been observed at least once (a valid /// record or a confirmed absence) and release an edit held by the - /// pre-hydration gate in [flush]. Only an incomplete edit is ever gated, so - /// a complete edit's own scheduled publish is left untouched. + /// pre-hydration gate in [flush]. Mobile authors no desktop-only fields, so + /// every edit is gated until the coordinate is known and cannot be merged + /// before then. void _observeHydration() { if (_hydrationObserved) return; _hydrationObserved = true; + _cancelHydrationRecovery(); final pending = _pending; - if (!_disposed && - pending != null && - !pending.includesDesktopAppearance && - !_publishInFlight) { + if (!_disposed && pending != null && !_publishInFlight) { // Accelerate the held edit now that the coordinate is known. Any pending // debounce timer is cancelled by _schedulePublish. A publish in flight is // left to its own finally-block, which reschedules the pending edit. @@ -255,6 +298,7 @@ class CommunityThemeSyncManager { void cancelPending() { _publishTimer?.cancel(); _publishTimer = null; + _cancelHydrationRecovery(); _pending = null; } @@ -267,11 +311,14 @@ class CommunityThemeSyncManager { } final preference = _pending; if (_disposed || preference == null) return; - // Hold an edit that carries no desktop appearance opinion until the relay - // coordinate has been observed, so a pre-hydration edit cannot replace a - // desktop-authored record before we have seen it. _observeHydration - // releases it. A complete edit is never gated. - if (!preference.includesDesktopAppearance && !_hydrationObserved) { + // Hold every edit until the relay coordinate has been observed, so a + // pre-hydration edit cannot replace a desktop-authored record before we + // have seen it. Mobile never authors the desktop-only fields, so even an + // edit that carries them (inherited from a stale full cache) must wait: + // without an observed coordinate there is nothing to merge the current + // desktop values from. _observeHydration releases it. + if (!_hydrationObserved) { + _scheduleHydrationRecovery(); return; } if (preference == _lastPublished) { diff --git a/mobile/test/shared/theme/community_theme_sync_test.dart b/mobile/test/shared/theme/community_theme_sync_test.dart index 6ae83ea808..709857c108 100644 --- a/mobile/test/shared/theme/community_theme_sync_test.dart +++ b/mobile/test/shared/theme/community_theme_sync_test.dart @@ -353,41 +353,50 @@ void main() { }, ); - test( - 'published coordinate rejects delayed same-second initialization result', - () async { - const stale = CommunityThemePreference( - theme: 'dracula', - accent: '#ef4444', - followSystem: false, - ); - final history = Completer>(); - final session = _FakeSession(historyFuture: history.future); - final relay = _FakeSignedRelay(eventId: 'published-a'); - final applied = []; - final manager = _manager( - session, - relay, - onRemote: (remote) => applied.add(remote.preference), - ); + test('a delayed initialization result never overrides a held edit', () async { + // The gate holds the edit until initialize resolves, so a publish can no + // longer race ahead of hydration. When the delayed history arrives it is + // observed (not applied over the pending edit), the gate releases, and + // the held edit publishes above the observed coordinate. + const stale = CommunityThemePreference( + theme: 'dracula', + accent: '#ef4444', + followSystem: false, + ); + final history = Completer>(); + final session = _FakeSession(historyFuture: history.future); + final relay = _FakeSignedRelay(eventId: 'published-a'); + final applied = []; + final manager = _manager( + session, + relay, + onRemote: (remote) => applied.add(remote.preference), + ); - final initializing = manager.initialize(); - manager.publish(local); - await manager.flush(); - final createdAt = relay.submittedEvents.single.createdAt; - history.complete([ - _event( - id: 'published-z', - createdAt: createdAt, - content: jsonEncode(stale.toJson()), - ), - ]); - await initializing; + final initializing = manager.initialize(); + manager.publish(local); + await manager.flush(); + // Held: initialize has not resolved, so the coordinate is unobserved. + expect(relay.submissions, isEmpty); + expect(manager.pending, local); - expect(applied, isEmpty); - expect(manager.pending, isNull); - }, - ); + history.complete([ + _event( + id: 'published-z', + createdAt: 100, + content: jsonEncode(stale.toJson()), + ), + ]); + await initializing; + await _waitUntil(() => relay.submissions.isNotEmpty); + + // The delayed stale result was observed but never applied over the + // pending edit, and the released edit still publishes exactly once. + expect(applied, isEmpty); + expect(manager.pending, isNull); + expect(relay.submissions, hasLength(1)); + expect(jsonDecode(relay.submissions.single.content)['theme'], 'buzz'); + }); test( 'gated incomplete edit holds until hydration then merges remote glass', @@ -475,6 +484,95 @@ void main() { expect(published.containsKey('glassOpacity'), isFalse); expect(published.containsKey('prominentActiveTab'), isFalse); }); + + test( + 'a full cache edit republishes the observed remote glass, not the cache', + () async { + // Regression: a mobile cache decoded from a desktop-authored record + // carries all three includes* flags, so it looks "complete". Mobile has + // no UI to author glass, so those fields are stale once another client + // changes them. The published coordinate must carry the current remote's + // glass, never the cache's. + const staleFullCacheEdit = CommunityThemePreference( + theme: 'dracula', + accent: '#ef4444', + followSystem: false, + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + ); + const currentRemote = CommunityThemePreference( + theme: 'buzz', + accent: '#3b82f6', + followSystem: true, + glassBackground: false, + glassOpacity: 40, + prominentActiveTab: false, + ); + final session = _FakeSession( + history: [ + _event( + id: 'current', + createdAt: 100, + content: jsonEncode(currentRemote.toJson()), + ), + ], + ); + final relay = _FakeSignedRelay(); + final manager = _manager(session, relay); + + await manager.initialize(); + manager.publish(staleFullCacheEdit); + await manager.flush(); + + final published = + jsonDecode(relay.submissions.single.content) as Map; + expect(published['theme'], 'dracula'); + expect(published['accent'], '#ef4444'); + expect(published['glassBackground'], false); + expect(published['glassOpacity'], 40); + expect(published['prominentActiveTab'], false); + }, + ); + + test( + 'a transient history failure does not strand a gated edit forever', + () async { + // Regression: the pre-hydration gate must not deadlock when the initial + // fetch fails (unavailable, not absent) while the live subscription stays + // quiet. A bounded history-retry releases the gate once the relay answers. + const incompleteEdit = CommunityThemePreference( + theme: 'dracula', + accent: '#ef4444', + followSystem: false, + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, + ); + // First fetch throws (unavailable); the retry succeeds with an absence. + final session = _FakeSession(fetchFailuresRemaining: 1); + final relay = _FakeSignedRelay(); + final manager = _manager(session, relay); + + final result = await manager.initialize(); + expect(result.status, CommunityThemeRemoteStatus.unavailable); + + manager.publish(incompleteEdit); + await manager.flush(); + // Held: the coordinate has not been observed yet. + expect(relay.submissions, isEmpty); + expect(manager.pending, incompleteEdit); + + // The scheduled recovery re-queries history, observes the absence, and + // releases the gate so the edit finally publishes. + await _waitUntil(() => relay.submissions.isNotEmpty); + expect(session.fetchCalls, greaterThanOrEqualTo(2)); + final published = + jsonDecode(relay.submissions.single.content) as Map; + expect(published['theme'], 'dracula'); + expect(manager.pending, isNull); + }, + ); } CommunityThemeSyncManager _manager( @@ -518,13 +616,16 @@ class _FakeSession extends RelaySessionNotifier { List history = const [], this.historyFuture, this.error, + this.fetchFailuresRemaining = 0, this.onFetchHistory, }) : history = List.of(history); List history; final Future>? historyFuture; final Object? error; + int fetchFailuresRemaining; final void Function()? onFetchHistory; int subscribeCalls = 0; + int fetchCalls = 0; final List _listeners = []; final List _closedCallbacks = []; @@ -537,7 +638,12 @@ class _FakeSession extends RelaySessionNotifier { NostrFilter filter, { Duration timeout = const Duration(seconds: 8), }) async { + fetchCalls++; if (error != null) throw error!; + if (fetchFailuresRemaining > 0) { + fetchFailuresRemaining--; + throw StateError('history unavailable'); + } onFetchHistory?.call(); if (historyFuture != null) return historyFuture!; return history;