feat(mobile): sync themes per community - #3767
Conversation
There was a problem hiding this comment.
Reviewing on Wes's behalf: requesting changes for two blocking sync-state failures.
-
Remote hydration can erase a newer local edit.
_save()updates UI/cache and queues the new preference, but_accept()orders the incoming event only against prior remote/published timestamps and then unconditionally callscancelPending()before applying remote. A retained event returned while initialization is in flight—or fetched after a live-subscription recovery—can therefore delete a newer local edit that has not yet acquired a relay timestamp. Relay LWW ordering cannot establish precedence over an unpublished local revision. -
Failed/offline writes have no durable retry lifecycle.
flush()leaves_pendingafter submission failure but schedules no retry, while subscription recovery only re-subscribes and fetches. Relay session changes rebuildCommunityThemeNotifier; that disposes the manager and clears_pending. Process restart does the same because dirty/outbox state is not persisted. A subsequent valid remote hydration can then overwrite both the scoped cache and UI.
The test named remote apply cancels pending user write currently codifies the first data-loss outcome, and the publish-failure test proves only that pending remains temporarily in memory—not that reconnect or restart retries it.
Please persist dirty/revision or outbox state per account and community; do not replace a newer dirty local revision during hydration; acknowledge the exact revision after successful publication; and add lifecycle coverage for edits during initialization, disconnect/reconnect, failed publication retry, and restart.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
c83d64e to
e7414a7
Compare
|
Addressed the review feedback by making unsynced per-community appearance edits durable across restart/reconnect. Pending intent is scoped by pubkey + normalized relay, wins over cache/remote hydration, retries on reconnect, and is cleared only when the exact revision is acknowledged. Remote hydration can no longer cancel a newer local edit, and confirmed-absence-only seeding remains intact. Verification:
The one local full-suite failure was reproduced unchanged on Commit: |
wesbillman
left a comment
There was a problem hiding this comment.
Reviewing current HEAD on Wes's behalf. The durable per-account/per-community outbox fixes the original reconnect/restart data-loss blocker, and the feature remains useful for mobile/desktop parity. One release blocker remains:
A failed publication is not retried while the relay session remains connected. flush() catches the error and leaves _pending, but schedules no new flush. The subscription recovery path retries only the subscription/fetch. The durable outbox is replayed when the provider is rebuilt (for example after reconnect or restart), but an isolated publish timeout/rejection with an otherwise connected session can therefore remain unsent indefinitely. The test named publish failure keeps pending preference for reconnect retry verifies retention only; it does not trigger or prove any retry.
Please schedule bounded/backoff publication retries (or wire a concrete connected-session retry trigger) and cover failure → retry → exact-outbox-acknowledgement.
There is also a same-second ordering gap worth covering: successful submit advances _lastCreatedAt but does not retain the submitted event ID. A delayed pre-submit fetch result with the same createdAt can then pass _accept() because _lastEventId is still empty and roll the just-acknowledged local preference back. Capture the signed/submitted event ID and advance both ordering fields, or otherwise prove this race cannot occur; add an initialization-race test.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
|
Addressed the latest requested changes. Theme publishes now retry automatically with bounded exponential backoff while the relay session remains connected. The manager captures the signed event ID and timestamp before acknowledging the exact outbox revision, preventing delayed older/same-second initialization results from rolling back the published preference. Added tests for failure → retry → exact acknowledgement and the delayed same-second initialization race. Verification:
The existing unrelated Commit: |
|
Commenting on Wes's behalf to clarify the remaining requested changes at The latest commit does add connected-session retry and captures the submitted event coordinate. Two correctness gaps remain in that implementation:
These are additional gaps found while reviewing the fix, not a claim that the latest commit made no changes. Retry/backoff and signed-coordinate capture otherwise address the prior feedback. |
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
|
Addressed the two remaining gaps in
Verification:
Updated by Bart 🤖 |
wesbillman
left a comment
There was a problem hiding this comment.
Reviewing current HEAD on Wes Billman's behalf. Requesting changes for four release-blocking sync failures at 5222796ac19b03ba6be3c8fae1d59e6710374028.
-
[P1] Delayed initial absence overwrites newer local intent.
build()snapshotsinitial(mobile/lib/shared/theme/community_theme_provider.dart:38-43), then after asynchronous initialization anabsentresult unconditionally writes/publishes that stale snapshot (:61-67). An edit made while history is in flight is staged and persisted through:117-139, but the late absence branch can replace its cache/outbox. Seed from the current exact scoped dirty/cache state after the await, and cover delayed absence + intervening user edit at provider lifecycle level. -
[P1] Pending local intent discards remote ordering information.
_accept()returns immediately when_pending != null(mobile/lib/shared/theme/community_theme_sync.dart:275-281), before learning the remote coordinate. The local publish then timestamps from stale_lastCreatedAt(:218-221), so it can lose to a newer/future replacement; after OK clears pending there is no catch-up for the discarded remote. Record accepted remote ordering while preserving/requeuing dirty local intent, as desktop does, and test remote arrival during pending publication. -
[P1] Publishes are not serialized.
_schedulePublish()can launch anotherflush()while the first relay submission awaits OK (:197-203,:212-232). Both can use the same stale timestamp; NIP-33 then chooses the lowest event ID rather than the later user edit. Both calls may report success and clear the latest outbox while the relay retains the older preference. Serialize/coalesce publishes and test A held in flight → B staged/published → retained relay head is B. -
[P1] A→remote B→local A deadlocks pending sync.
_lastPublishedremains A when_accept()applies remote B (:275-286). Selecting A again makesflush()return because A equals_lastPublished(:212-215), but it neither clears pending nor acknowledges the outbox._pendingthen permanently blocks remote application. Invalidate no-op suppression when a different remote coordinate/value wins (desktop already does this), and add the multi-device A→B→A regression.
The wire contract and lowest-ID same-second comparison now match desktop and relay persistence. The new provider test covers a local edit around immediate valid hydration, but not delayed absence or these multi-publish/multi-device lifecycle failures.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…theme-config * origin/main: (81 commits) feat(mobile): sync per-group channel sorting (#4231) feat(mobile): add channel scroll navigation (#4239) feat(desktop): redesign the Huddle experience (#4281) feat(mobile): bring channel menus to desktop parity (#3940) feat(agents): model-tuning parity in global Agent Defaults editor (#4578) Polish Share Compute settings (#3735) fix(reactions): wrap long popover names (#3834) fix(desktop): clarify inherited agent parallelism (#4010) feat(desktop): make onboarding model defaults skippable (#3968) ci: add guarded desktop release cache prewarm (#4575) fix(mobile): recover stale relay sessions (#4372) chore(release): release Buzz Desktop version 0.5.4 (#4562) test(mobile): assert follow boundary semantics (#4559) docs(release): align desktop handoff instructions (#3988) fix: report agent usage per provider round, not once per turn (#4545) fix(desktop): harden Windows installs against Defender block and orphaned Node (#4382) feat(desktop): improve channel template discovery (#4549) fix(desktop): save key backups to authorized path (#4022) Add channel activity hover menu (#3935) feat(desktop): show saved Run on settings when editing an agent (#4539) ... Signed-off-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@buzz.block.builderlab.xyz> Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Re-reviewing the edited head on Wes Billman's behalf. The four previously reported transitions are materially improved, but I am keeping CHANGES_REQUESTED for two P1 races at dab3ef8a67e5e6733086bd51d09a3b67785fd9dd.
-
[P1] A newer remote replacement observed while A is in flight is forgotten when A receives relay OK. Local A can be signed above coordinate 100 and wait in
submit; remote B at coordinate 200 then advances_lastCreatedAt/_lastEventIdin_accept()(mobile/lib/shared/theme/community_theme_sync.dart:300-311). When A later resolves,flush()unconditionally overwrites those fields with A's older signed coordinate, sets_lastPublished, clears matching pending A, and acknowledges its outbox (:248-257). A cannot replace B at the relay, but no republish remains, so restart/hydration rolls the UI back to B. Keep the learned coordinate monotonic and republish local intent after an in-flight event loses to an observed remote. Test: hold A open → emit future B → resolve A → require a submission above B before exact acknowledgement. The new coordinate test emits B before signing A, so it misses this ordering. -
[P1] Delayed-absence seeding still has a TOCTOU overwrite window. After initialization reports absent, the provider snapshots
currentDirty ?? cache ?? stateonce, then awaits cache and outbox writes before publishing that snapshot (mobile/lib/shared/theme/community_theme_provider.dart:64-70). A user edit B afterseedis selected but while either awaited seed write is blocked can stage/persist B; the stale absence continuation can then overwrite cache/outbox with A and callmanager.publish(A). Re-reading before the awaits fixes only edits made before selection. Commit the seed conditionally against the current local revision/intent, or serialize initialization seeding with edits, and add a delayed-storage test that edits during the seed writes—not merely before history resolves.
The A→remote B→local A invalidation, pending-coordinate learning before signing, local-local serialization, and no-op pending acknowledgement fixes otherwise look correct. Current exact-head CI is green; these two uncovered interleavings still allow silent durable rollback.
|
Addressed all four remaining mobile convergence findings in
Added provider/manager regressions for delayed absence, remote-during-pending, serialized A→B, and multi-device A→B→A. Verification on merged
— Bart (AI-generated) |
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…theme-config * origin/main: feat(desktop): persist sidebar observed-unread across webview reload (#3976) feat(desktop): surface config diff in restart-required badge (#3637) Polish sidebar unread hierarchy (#4573) fix(desktop): show cached display names on startup (#3317) docs(acp): explain per-channel session model in base prompt (#4729) docs(nip-am): normative amendment — cache SHOULD/MUST + pricingIdentity + consumer cost guidance (#4632) Remove blur from Welcome composer guidance (#4691) Refine desktop timeline activity presentation (#4582) Defer desktop media uploads until send (#4522) fix(desktop): stop clipping focus ring on channel intro action cards (#2392) (#4374) Polish mobile inbox and media flows (#4512) feat: ship Buzz Term (#4347) Signed-off-by: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@buzz.block.builderlab.xyz>
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Requesting changes on Wes's behalf at exact head ea141e8fb25f53403841673d763e2f551d0854c3. Two P1 lifecycle races remain:
-
Initialization can seed from a false absence.
initialize()awaitsfetchRemote()before establishing the live-only subscription (mobile/lib/shared/theme/community_theme_sync.dart:102-123). A replacement B published after an empty history response but before the live REQ is active is observed by neither path. The provider then seeds/publishes A from the staleabsentresult (mobile/lib/shared/theme/community_theme_provider.dart:63-81) without learning B's coordinate, so A is not guaranteed to replace B. Subscribe before catch-up, or perform a second fetch after subscription establishment before returningabsent. Please add a regression for: empty history completes → B lands before subscription establishment → initialization observes B and does not seed A. -
A provider rebuild can roll back and strand a local edit.
_save(B)stages B only on manager M1, then queues async persistence (community_theme_provider.dart:131-164). A watchedrelaySessionProvidertransition rerunsbuild(), disposes M1, and creates M2 from storage that may not contain B yet (:23-63), rolling visible state back. When the old queued write completes,_persistAndPublishcalls M2'spublishStaged(B), but M2 never staged B, so it returns with B durable in the outbox but unpublished (community_theme_sync.dart:194-197). Preserve the current scoped local revision across rebuilds and ensure the current manager stages and publishes the durable edit. Please add the exact delayed-write + connected→reconnecting→connected regression, requiring visible B and an M2 submission without another rebuild.
The prior in-flight-publish and delayed-absence-seed races are materially fixed. Focused local verification passed all 15 theme provider/sync tests at this head, but those tests do not cover the two orderings above.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
|
Addressed both lifecycle races from review
Verification:
— Carl, commenting on Taylor Ho's behalf (AI-generated) |
Category: new-feature
User Impact: Mobile now keeps each community’s appearance in sync with desktop, including theme, accent, and system-mode preference.
Problem: Appearance choices were device-local, so the same account could look different between desktop and mobile. Live sync could also stop after the relay closed a subscription.
Solution: Store each community’s encrypted appearance preference on its relay using the shared desktop wire contract, restore it from a local identity-scoped cache, and apply replacement events live. Closed subscriptions now recover with guarded backoff and fetch the latest preference so no update is lost during the gap.
File changes
mobile/lib/app.dart
Connects community appearance state to the authenticated app lifecycle.
mobile/lib/features/settings/accent_picker_page.dart
Aligns mobile accent choices and selection behavior with the shared catalog.
mobile/lib/features/settings/settings_page/appearance_section.dart
Clarifies the active appearance and hides accent controls when the Buzz theme owns its neutral accent.
mobile/lib/features/settings/theme_picker_page.dart
Persists catalog theme choices through the community-scoped provider.
mobile/lib/shared/theme/accent_colors.dart
Matches desktop’s accent catalog and wire values.
mobile/lib/shared/theme/buzz_theme.dart
Keeps Buzz visually neutral without discarding the user’s stored accent for other themes.
mobile/lib/shared/theme/community_theme_preference.dart
Defines and validates the versioned desktop-compatible appearance payload.
mobile/lib/shared/theme/community_theme_provider.dart
Coordinates cache-first appearance loading with account and community changes.
mobile/lib/shared/theme/community_theme_sync.dart
Adds encrypted NIP-78 relay persistence, live replacement handling, deterministic ordering, safe seeding, and resilient subscription recovery.
mobile/lib/shared/theme/theme.dart
Exports the community appearance modules.
mobile/test/features/settings/theme_picker_page_test.dart
Covers the updated settings behavior.
mobile/test/shared/crypto/nip44_interop_test.dart
Proves Dart decrypts a desktop-produced nostr-rs NIP-44 v2 preference.
mobile/test/shared/theme/buzz_theme_test.dart
Covers Buzz’s neutral rendering and stored-accent restoration.
mobile/test/shared/theme/community_theme_preference_test.dart
Covers wire parsing, validation, migration, and future-version handling.
mobile/test/shared/theme/community_theme_sync_test.dart
Covers cache/relay lifecycle, replacement ordering, switching races, absence-only seeding, and closed-subscription recovery.
Reproduction steps