sync: migrate lifecycle handling with Makepad runtime alignment#160
Merged
Conversation
Specifies how robrix2's app lifecycle should behave across mobile and
desktop: persist state and pause Matrix sync on background, resume sync
on foreground, save state on quit-requested and window-close, and dedup
identical consecutive writes via a serialized-bytes fingerprint.
Documents the supporting work this depends on:
- aligning robrix2's Makepad runtime + cargo-makepad CLI with the
Robius ecosystem default (makepad/dev), which provides the
QuitRequested event, request_quit, MAKEPAD_BUNDLE_NAME, and the
more elaborate Android NDK / iOS build-tool setup
- splitting save_app_state into serialize + bytes-save helpers so the
in-process fingerprint dedup is possible without disturbing the
public save_app_state signature or any of its existing call sites
Enumerates the robrix2-local enhancements that the design explicitly
preserves and refuses to regress:
- load_app_state stays Result<AppState>
- should_restore_loaded_app_state's broader restore criteria
- skip_app_state_restore_once / take_skip_app_state_restore_once
explicit-logout flow
- rs.robius.robrix bundle identifier
- .cargo/config.toml [env] { value, force = true } syntax
- Box-wrapped RestoreAppStateFromPersistentState payload
Plans seven independently buildable commits so the highest-risk piece
(the Makepad runtime alignment) is auditable on its own.
Switches makepad-widgets and makepad-code-editor from the kevinaboos
fork branch to the canonical makepad/dev branch used by the rest of
the Robius ecosystem. This brings in the QuitRequested event,
Cx::request_quit, the MAKEPAD_BUNDLE_NAME env var, and the broader
Android NDK / iOS build-tool surface.
For Android and iOS builds, the matching cargo-makepad CLI install
command is:
cargo install --force --git https://github.com/makepad/makepad.git --branch dev cargo-makepad
API compat fix: CheckBox::set_active and RadioButton::set_active gained
an `animate: Animate` parameter in makepad/dev. Updated 10 call sites
in robrix2 to pass Animate::No (programmatic state restoration, no
visual transition needed).
Without this, the macOS application menu next to the Apple logo shows
'MakepadStdinLoop' instead of 'Robrix' when launched via cargo run.
Uses the { value, force = true } syntax already established in this
file for bundle/icon overrides.
Splits the in-process save path into three layered functions without changing the public save_app_state signature: - serialize_app_state(&AppState) -> Result<Vec<u8>> - save_app_state_bytes(&[u8], &UserId) -> Result<()> - save_app_state(AppState, OwnedUserId) -> Result<()> (now a one-line delegation; existing call sites unaffected) Lets the in-process lifecycle handler serialize the app state once, fingerprint the bytes, and skip the disk write when the bytes match the previous save. The disk path also gains a create_dir_all so a fresh user state directory does not fail the first write. load_app_state's signature is intentionally unchanged.
Introduces a desired/assumed running pair of atomics plus a tokio lifecycle lock so the Matrix sync service can be reconciled to a declared lifecycle state without racing the existing direct start/stop paths. Public API additions: - sync_service_desired_running() -> bool - set_sync_service_desired_running(bool, &'static str) - stop_sync_service_for_shutdown(Duration) -> Result<(), Elapsed> Internal: - apply_sync_service_desired_state(&'static str) reconciliation loop - initial-sync and account-switch placement sites now route through apply_sync_service_desired_state instead of calling start().await directly before the Arc lands in SYNC_SERVICE - sync error restart honors sync_service_desired_running() before re-issuing start; uses apply_sync_service_desired_state for restart - account-switch cleanup and clear_app_state clear ASSUMED_RUNNING handle_load_app_state is intentionally not modified.
Adds an AppLifecycle struct and an AppStateSaveFingerprint type that together capture transient lifecycle position (foreground / active / last save fingerprint / shutdown latch) and let the app dedup identical consecutive persistence writes. Replaces the previous inline Event::Shutdown block in handle_event with a single dispatch call into handle_lifecycle_event, which routes: - Pause / Background: persist state, mark inactive / send sync stop - Resume / Foreground: mark active, send sync start - WindowCloseRequested: persist state if it's the main window - QuitRequested: persist state - Shutdown: once-only handle_shutdown — persist, stop sync with 3s timeout, then save the TSP wallet under #[cfg(feature = "tsp")] persist_runtime_state is the single chokepoint for save-now: serialize once, hash + length + user_id into a fingerprint, compare to the last saved fingerprint, skip the disk write when identical.
Replaces Makepad's default stock menu (File / Edit / View / ...) with a minimal Robrix app menu containing just a 'Quit Robrix' item bound to Cmd+Q. The Quit item dispatches through the runtime's quit-request path, which surfaces as Event::QuitRequested in handle_lifecycle_event so the app gets a chance to persist state before exit. The 'Robrix' menu label next to the Apple logo comes from MAKEPAD_BUNDLE_NAME (set in .cargo/config.toml in an earlier commit). The items inside the menu come from this WindowMenu DSL block.
When the logout modal hits the unrecoverable point-of-no-return path and the user opts for immediate restart, route the quit through cx.request_quit(QuitReason::App) instead of cx.quit(). This goes through the lifecycle handler's Event::QuitRequested arm and gives persist_runtime_state a chance to save before the process exits, matching the design's intent that every user-initiated quit goes through the same save-then-exit flow.
…elogin The relogin reset path direct-calls sync_service.stop() and leaves SYNC_SERVICE_ASSUMED_RUNNING with whatever value it had before. Under token-expiry, the session-change handler running this reset can race ahead of the sync-state subscriber's Error(invalid_token) arm that would otherwise clear ASSUMED_RUNNING. If the race fires, ASSUMED_RUNNING stays true while the actual service is gone. The next login then calls apply_sync_service_desired_state which reads DESIRED=true / ASSUMED=true, treats them as already converged, and breaks without starting the new service — leaving the re-authenticated session with no Matrix sync. Fix: clear ASSUMED_RUNNING right after stop() in this function, so the state machine's view of "actually running" stays consistent with the real world regardless of which task races to the relogin first.
adf1488 to
3c1b880
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds first-class app lifecycle handling to robrix2 on every platform: save state and pause Matrix sync when the app moves to background on mobile, resume sync on foreground, save state on the main window's close request, save state when the OS asks the app to quit (menu Quit, Cmd+Q, terminal Ctrl+C), and dedup identical consecutive disk writes. Also brings the Makepad runtime in line with the Robius ecosystem default (
makepad/dev) and adds the macOS app menu identity that ships with it.What Changed
makepad-widgetsandmakepad-code-editortomakepad/dev. Brings inEvent::QuitRequested,Cx::request_quit, theMAKEPAD_BUNDLE_NAMEenv var, ctrlc-based SIGINT handling, a more elaborate Android NDK / iOS build-tool surface, and the newAnimateparameter onCheckBox::set_active/RadioButton::set_active(10 mechanical call-site fixes included).MAKEPAD_BUNDLE_NAME = "Robrix"env entry in.cargo/config.toml(using the established{ value, force = true }syntax) and aWindowMenuDSL block inscript_mod!with a "Quit Robrix" item bound to Cmd+Q.save_app_stateis now a 2-line delegation to two new public helpers —serialize_app_state(&AppState) -> Result<Vec<u8>>andsave_app_state_bytes(&[u8], &UserId) -> Result<()>. The disk path also gainscreate_dir_allfor first-write robustness. Publicsave_app_statesignature is unchanged; existing call sites are unaffected.SYNC_SERVICE_DESIRED_RUNNING,SYNC_SERVICE_ASSUMED_RUNNING,SYNC_SERVICE_LIFECYCLE_LOCK) and four new functions (sync_service_desired_running,set_sync_service_desired_running,apply_sync_service_desired_state,stop_sync_service_for_shutdown) reconcile the running sync state to a declared lifecycle desire. Initial-sync and account-switch placement sites are rewired through the state machine; sync error restart honors the lifecycle-desired flag.AppLifecyclestruct onApp(foreground / active / last-save-fingerprint / shutdown-latch), single dispatcherhandle_lifecycle_eventinhandle_event, and three helpers —persist_runtime_state(single save-now chokepoint with fingerprint dedup),handle_shutdown(once-only, persists then stops sync with a 3-second timeout then saves the TSP wallet under#[cfg(feature = "tsp")]). The previous inlineEvent::Shutdownblock inhandle_eventis removed.cx.quit()in the logout modal's unrecoverable-error restart path is nowcx.request_quit(QuitReason::App)so the lifecycle handler can persist before exit.reset_runtime_state_for_reloginnow clearsSYNC_SERVICE_ASSUMED_RUNNINGafter its directsync_service.stop().await. Without this, a token-expiry reauth could race with the sync-error subscriber and leave the state machine convinced the service is running when it is gone — causing the re-authenticated session to silently skip the initial sync start.Robrix2-Local Behavior Preserved
Every robrix2-local enhancement in the touched files is preserved verbatim — this work is explicitly additive:
load_app_statestaysResult<AppState>(NOTResult<Option<AppState>>). Its body and doc comment are unchanged.should_restore_loaded_app_state(broader restore criteria across dock + bot settings + app language + translation) is unchanged.skip_app_state_restore_once/take_skip_app_state_restore_onceexplicit-logout flow is unchanged.handle_load_app_statebody insrc/sliding_sync.rsis unchanged.RestoreAppStateFromPersistentState(Box<AppState>)boxed-payload shape is unchanged.MAKEPAD_BUNDLE_IDENTIFIER = "rs.robius.robrix"(robrix2-canonical) is unchanged. The newMAKEPAD_BUNDLE_NAMEentry follows the same{ value, force = true }syntax.Validation
cargo buildpasses on desktop.cargo test --libshows 4 pre-existing failures inroom::room_input_barandhome::room_screen(bot mention parsing). Confirmed by running the same tests againstorigin/mainbefore this branch — same 4 fail there. These are pre-existing and untouched by this PR; tracked as separate work.Notes
cargo-makepadCLI install command is:sync_service.start()/stop()callsites that were not rewired through the new state machine are intentionally left in place to match the pattern this work is ported from. Onlyreset_runtime_state_for_relogingained anASSUMED_RUNNING.store(false)line as a follow-up fix to a real race.docs/superpowers/specs/anddocs/superpowers/plans/capture the full reasoning, including the 3-layer no-regression filter and the per-commit risk isolation strategy.