From e6caeda11ac316dc3b46beee173917eddcf1621d Mon Sep 17 00:00:00 2001 From: Owen McGirr Date: Mon, 5 Oct 2026 14:24:07 +0100 Subject: [PATCH 1/5] Sync settings between computers through the account Add the settings sync engine. When signed in, the portable settings document is kept in the account's desktop_preferences row: - Every write is conditional on the revision last seen (POST to create, PATCH revision=eq.N to update, never upsert), so a lost race re-reads and merges instead of overwriting another computer's change. - The last document synced with each account is kept locally as the merge base. Changes on one side apply to the other; sections changed on both merge by section, with the account's copy winning a section changed on both sides. - The first sync on a computer whose settings differ from the account asks which to keep; an untouched install takes the account's settings. - A copy from a newer app is never overwritten; this install asks to be updated. An invalid copy is reported, not replaced. - Sync runs at start, after sign-in, when local settings have settled after a change, every five minutes for other computers' changes, and on demand, with backoff after failures. The Account tab shows sync status, Sync now, and the first-sync choice. Profiles and remote switches screens refresh when sync changes them. Refs #986 Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/build.rs | 3 + src-tauri/capabilities/main.json | 5 +- .../autogenerated/get_settings_sync.toml | 11 + .../autogenerated/resolve_settings_sync.toml | 11 + .../autogenerated/sync_settings_now.toml | 11 + src-tauri/src/account.rs | 10 +- src-tauri/src/lib.rs | 45 +- src-tauri/src/portable_settings.rs | 3 +- src-tauri/src/settings_sync.rs | 1360 +++++++++++++++++ src/App.test.tsx | 16 + src/App.tsx | 9 + src/api.ts | 15 +- src/scanning/useRemoteSwitches.test.tsx | 56 + src/scanning/useRemoteSwitches.ts | 13 + src/settings/AccountSection.test.tsx | 63 +- src/settings/AccountSection.tsx | 69 +- src/types.ts | 8 + 17 files changed, 1696 insertions(+), 12 deletions(-) create mode 100644 src-tauri/permissions/autogenerated/get_settings_sync.toml create mode 100644 src-tauri/permissions/autogenerated/resolve_settings_sync.toml create mode 100644 src-tauri/permissions/autogenerated/sync_settings_now.toml create mode 100644 src-tauri/src/settings_sync.rs create mode 100644 src/scanning/useRemoteSwitches.test.tsx diff --git a/src-tauri/build.rs b/src-tauri/build.rs index bc479eba..390b04be 100644 --- a/src-tauri/build.rs +++ b/src-tauri/build.rs @@ -46,6 +46,9 @@ fn main() { "cancel_sign_in", "sign_out", "delete_account", + "get_settings_sync", + "sync_settings_now", + "resolve_settings_sync", ]); let mut attributes = tauri_build::Attributes::new().app_manifest(app_manifest); if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") { diff --git a/src-tauri/capabilities/main.json b/src-tauri/capabilities/main.json index 757cc310..79b678ce 100644 --- a/src-tauri/capabilities/main.json +++ b/src-tauri/capabilities/main.json @@ -45,6 +45,9 @@ "allow-verify-sign-in-code", "allow-cancel-sign-in", "allow-sign-out", - "allow-delete-account" + "allow-delete-account", + "allow-get-settings-sync", + "allow-sync-settings-now", + "allow-resolve-settings-sync" ] } diff --git a/src-tauri/permissions/autogenerated/get_settings_sync.toml b/src-tauri/permissions/autogenerated/get_settings_sync.toml new file mode 100644 index 00000000..c67ed99d --- /dev/null +++ b/src-tauri/permissions/autogenerated/get_settings_sync.toml @@ -0,0 +1,11 @@ +# Automatically generated - DO NOT EDIT! + +[[permission]] +identifier = "allow-get-settings-sync" +description = "Enables the get_settings_sync command without any pre-configured scope." +commands.allow = ["get_settings_sync"] + +[[permission]] +identifier = "deny-get-settings-sync" +description = "Denies the get_settings_sync command without any pre-configured scope." +commands.deny = ["get_settings_sync"] diff --git a/src-tauri/permissions/autogenerated/resolve_settings_sync.toml b/src-tauri/permissions/autogenerated/resolve_settings_sync.toml new file mode 100644 index 00000000..35f682a2 --- /dev/null +++ b/src-tauri/permissions/autogenerated/resolve_settings_sync.toml @@ -0,0 +1,11 @@ +# Automatically generated - DO NOT EDIT! + +[[permission]] +identifier = "allow-resolve-settings-sync" +description = "Enables the resolve_settings_sync command without any pre-configured scope." +commands.allow = ["resolve_settings_sync"] + +[[permission]] +identifier = "deny-resolve-settings-sync" +description = "Denies the resolve_settings_sync command without any pre-configured scope." +commands.deny = ["resolve_settings_sync"] diff --git a/src-tauri/permissions/autogenerated/sync_settings_now.toml b/src-tauri/permissions/autogenerated/sync_settings_now.toml new file mode 100644 index 00000000..1f86f905 --- /dev/null +++ b/src-tauri/permissions/autogenerated/sync_settings_now.toml @@ -0,0 +1,11 @@ +# Automatically generated - DO NOT EDIT! + +[[permission]] +identifier = "allow-sync-settings-now" +description = "Enables the sync_settings_now command without any pre-configured scope." +commands.allow = ["sync_settings_now"] + +[[permission]] +identifier = "deny-sync-settings-now" +description = "Denies the sync_settings_now command without any pre-configured scope." +commands.deny = ["sync_settings_now"] diff --git a/src-tauri/src/account.rs b/src-tauri/src/account.rs index 1aa2842d..19178599 100644 --- a/src-tauri/src/account.rs +++ b/src-tauri/src/account.rs @@ -189,8 +189,6 @@ pub struct AccountView { /// A usable access token for the signed-in user. #[derive(Clone)] pub struct Authorized { - // Read by the sync engine (#986). - #[allow(dead_code)] pub user_id: String, pub access_token: String, } @@ -289,6 +287,14 @@ impl Account { let _ = self.store.delete(); } + /// The project URL and publishable key, for the database requests made by + /// settings sync with an `Authorized` token. + pub fn endpoint(&self) -> Option<(String, String)> { + self.config + .as_ref() + .map(|config| (config.url.clone(), config.key.clone())) + } + fn config(&self) -> Result<&Config, String> { self.config .as_ref() diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 2806889f..fe753049 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -37,6 +37,7 @@ mod scan_tile; mod scan_tree; mod scanning; mod scanning_runtime; +mod settings_sync; mod state; mod storage; mod switch_gestures; @@ -851,6 +852,11 @@ async fn verify_sign_in_code( let result = account.verify_code(&code).await; // Emitted on failure too: a rejected session signs out mid-action. account::emit(&app, &account.current_view().await); + if result.is_ok() { + if let Some(sync) = app.try_state::>() { + sync.wake(); + } + } result } @@ -872,6 +878,9 @@ async fn sign_out( let result = account.sign_out().await; // Emitted on failure too: a rejected session signs out mid-action. account::emit(&app, &account.current_view().await); + if let Some(sync) = app.try_state::>() { + sync.wake(); + } result } @@ -883,9 +892,39 @@ async fn delete_account( let result = account.delete_account().await; // Emitted on failure too: a rejected session signs out mid-action. account::emit(&app, &account.current_view().await); + if result.is_ok() { + if let Some(sync) = app.try_state::>() { + sync.forget(); + } + } result } +fn sync_engine(app: &AppHandle) -> Option> { + app.try_state::>() + .map(|engine| engine.inner().clone()) +} + +#[tauri::command] +fn get_settings_sync(app: AppHandle) -> settings_sync::SyncView { + sync_engine(&app).map_or_else(settings_sync::SyncView::unavailable, |engine| engine.view()) +} + +#[tauri::command] +async fn sync_settings_now(app: AppHandle) -> Result { + let engine = sync_engine(&app).ok_or("Settings sync is unavailable in this build.")?; + Ok(engine.sync(None).await) +} + +#[tauri::command] +async fn resolve_settings_sync( + app: AppHandle, + choice: settings_sync::Choice, +) -> Result { + let engine = sync_engine(&app).ok_or("Settings sync is unavailable in this build.")?; + Ok(engine.sync(Some(choice)).await) +} + fn publish_update(app: &AppHandle, model: &AppModel, update: UpdateView) -> AppState { model .shared @@ -1588,6 +1627,7 @@ pub fn run() { switch_runtime::install(app.handle()); remote_scan::install(app.handle()); point_scan_runtime::install(app.handle()); + settings_sync::install(app.handle()); install_tray(app)?; if updater_is_configured(app.config().plugins.0.get("updater")) { let model = app.state::(); @@ -1719,7 +1759,10 @@ pub fn run() { verify_sign_in_code, cancel_sign_in, sign_out, - delete_account + delete_account, + get_settings_sync, + sync_settings_now, + resolve_settings_sync ]) .run(tauri::generate_context!()) .expect("error while running Switchify PC"); diff --git a/src-tauri/src/portable_settings.rs b/src-tauri/src/portable_settings.rs index 02f573ab..4c32880f 100644 --- a/src-tauri/src/portable_settings.rs +++ b/src-tauri/src/portable_settings.rs @@ -5,8 +5,6 @@ //! registration never leave this install. Incoming documents are fully //! validated before anything is applied, and each section is applied through //! the same path as a local save so runtime state and overlays stay in step. -// Wired up by the sync engine (#986). -#![allow(dead_code)] use crate::{ point_scan, remote_scan, @@ -224,6 +222,7 @@ pub struct Plan { } impl Plan { + #[cfg(test)] pub fn is_empty(&self) -> bool { *self == Self::default() } diff --git a/src-tauri/src/settings_sync.rs b/src-tauri/src/settings_sync.rs new file mode 100644 index 00000000..8f1a5f9d --- /dev/null +++ b/src-tauri/src/settings_sync.rs @@ -0,0 +1,1360 @@ +//! Syncs the portable settings document with the signed-in account's +//! `desktop_preferences` row. +//! +//! The row's `revision` is owned by the server (1 on insert, +1 per update), +//! so every write is conditional on the revision last seen: a write that +//! loses a race matches nothing and the engine re-reads and merges. The +//! last document synced with each account is kept locally as the merge base. +//! A document from a newer app is never overwritten. + +use crate::account::{Account, Authorized}; +use crate::portable_settings::{self, Document, Local, ParseError, SCHEMA_VERSION}; +use serde::{Deserialize, Serialize}; +use serde_json::{json, Value}; +use std::future::Future; +use std::path::PathBuf; +use std::pin::Pin; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; +use tauri::{AppHandle, Emitter, Manager}; + +pub const SYNC_EVENT: &str = "settings-sync-changed"; +pub const PROFILES_EVENT: &str = "switch-profiles-changed"; +pub const REMOTE_SWITCHES_EVENT: &str = "remote-switches-changed"; +const STATE_FILE: &str = "settings-sync.json"; +/// How often local settings are compared with the last synced copy. +const POLL: Duration = Duration::from_secs(3); +/// How often the account's copy is checked for changes from other computers. +const REMOTE_CHECK: Duration = Duration::from_secs(300); +const MAX_BACKOFF: Duration = Duration::from_secs(300); +const MAX_ATTEMPTS: usize = 3; +/// Well under the server's 256 KiB limit. +const MAX_PAYLOAD_BYTES: usize = 200 * 1024; +const UPDATE_REQUIRED: &str = + "Your synced settings were saved by a newer Switchify PC. Update Switchify PC on this computer to keep syncing."; + +type BoxFuture<'a, T> = Pin + Send + 'a>>; + +/// The account's row as last read or written. +#[derive(Debug, Clone, PartialEq)] +pub struct Row { + pub revision: i64, + pub schema_version: u32, + pub payload: Value, +} + +/// The last document synced with an account: the merge base. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct Base { + user_id: String, + revision: i64, + document: Document, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum Choice { + /// Upload this computer's settings over the account's copy. + Local, + /// Replace this computer's settings with the account's copy. + Cloud, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub enum Status { + Off, + Syncing, + UpToDate, + /// This computer and the account both have different settings and no + /// previous sync to merge from; the user picks which to keep. + NeedsChoice, + /// The account's copy came from a newer Switchify PC. + UpdateRequired, + Error, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct SyncView { + pub status: Status, + /// Milliseconds since the Unix epoch. + pub last_synced_at: Option, + pub message: Option, +} + +impl SyncView { + /// For builds without accounts. + pub fn unavailable() -> Self { + Self::off() + } + + fn off() -> Self { + Self { + status: Status::Off, + last_synced_at: None, + message: None, + } + } +} + +/// The account's `desktop_preferences` row over PostgREST. +pub trait Remote: Send + Sync { + fn fetch<'a>(&'a self, auth: &'a Authorized) -> BoxFuture<'a, Result, String>>; + /// `None` when another computer created the row first. + fn create<'a>( + &'a self, + auth: &'a Authorized, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>>; + /// `None` when the row is no longer at `revision`. + fn update<'a>( + &'a self, + auth: &'a Authorized, + revision: i64, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>>; +} + +/// This install: its account, settings and sync state. +pub trait Host: Send + Sync { + /// `Ok(None)` when no one is signed in. + fn authorize(&self) -> BoxFuture<'_, Result, String>>; + fn local(&self) -> BoxFuture<'_, Result>; + /// Applies a document and returns the resulting local document, which + /// can differ (e.g. profile versions) and becomes the merge base. + fn apply(&self, document: Document) -> BoxFuture<'_, Result>; + /// The document of an install whose settings were never changed. + fn untouched(&self) -> Document; + fn load_state(&self) -> State; + fn save_state(&self, state: &State) -> Result<(), String>; + fn publish(&self, view: &SyncView); +} + +/// Persisted next to the other settings files. +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct State { + /// Random per install, sent as `updated_by`. Never the BLE desktop id. + #[serde(default)] + install_id: String, + #[serde(default)] + base: Option, + #[serde(default)] + last_synced_at: Option, +} + +/// What to do after comparing this install, the merge base and the account. +#[derive(Debug, PartialEq)] +enum Step { + InSync, + /// Local matches the account; record it as the base. + Adopt, + Create(Document), + Upload { + revision: i64, + document: Document, + }, + /// Apply `document` locally, then upload it if the account differs. + Apply { + document: Document, + upload_over: Option, + }, + NeedsChoice, +} + +fn decide( + local: &Document, + untouched: &Document, + base: Option<&Base>, + remote: Option<(&Row, &Document)>, + choice: Option, +) -> Step { + let Some((row, cloud)) = remote else { + return Step::Create(local.clone()); + }; + if let Some(choice) = choice { + return match choice { + Choice::Local if local == cloud => Step::Adopt, + Choice::Local => Step::Upload { + revision: row.revision, + document: local.clone(), + }, + Choice::Cloud => Step::Apply { + document: cloud.clone(), + upload_over: None, + }, + }; + } + match base { + Some(base) if base.revision == row.revision => { + if *local == base.document { + Step::InSync + } else { + Step::Upload { + revision: row.revision, + document: local.clone(), + } + } + } + Some(base) => { + let merged = merge(&base.document, local, cloud); + if merged == *local && merged == *cloud { + Step::Adopt + } else { + Step::Apply { + upload_over: (merged != *cloud).then_some(row.revision), + document: merged, + } + } + } + None if local == cloud => Step::Adopt, + // Nothing to lose on a computer that was never set up differently. + None if local == untouched => Step::Apply { + document: cloud.clone(), + upload_over: None, + }, + None => Step::NeedsChoice, + } +} + +/// Three-way merge by section. A section changed on only one side takes +/// that side; changed on both, the account's copy wins. +fn merge(base: &Document, local: &Document, cloud: &Document) -> Document { + fn pick(base: &T, local: &T, cloud: &T) -> T { + if local == base { + cloud.clone() + } else if cloud == base { + local.clone() + } else { + cloud.clone() + } + } + Document { + schema_version: SCHEMA_VERSION, + app: pick(&base.app, &local.app, &cloud.app), + profiles: pick(&base.profiles, &local.profiles, &cloud.profiles), + switches: pick(&base.switches, &local.switches, &cloud.switches), + point_scan: pick(&base.point_scan, &local.point_scan, &cloud.point_scan), + remote_switches: pick( + &base.remote_switches, + &local.remote_switches, + &cloud.remote_switches, + ), + } +} + +fn now_ms() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as i64) + .unwrap_or_default() +} + +enum Outcome { + Synced, + NeedsChoice, + UpdateRequired, +} + +pub struct Engine { + remote: Arc, + host: Arc, + // Only one sync runs at a time. + running: tokio::sync::Mutex<()>, + view: Mutex, + wake: tokio::sync::Notify, +} + +impl Engine { + pub fn new(remote: Arc, host: Arc) -> Self { + let last_synced_at = host.load_state().last_synced_at; + Self { + remote, + host, + running: tokio::sync::Mutex::new(()), + view: Mutex::new(SyncView { + last_synced_at, + ..SyncView::off() + }), + wake: tokio::sync::Notify::new(), + } + } + + pub fn view(&self) -> SyncView { + self.view.lock().unwrap_or_else(|p| p.into_inner()).clone() + } + + fn set(&self, status: Status, message: Option) { + let view = { + let mut view = self.view.lock().unwrap_or_else(|p| p.into_inner()); + view.status = status; + view.message = message; + if status == Status::UpToDate { + view.last_synced_at = Some(now_ms()); + } + if status == Status::Off { + view.last_synced_at = None; + } + view.clone() + }; + self.host.publish(&view); + } + + /// Asks the background loop to sync now (e.g. after signing in). + pub fn wake(&self) { + self.wake.notify_one(); + } + + /// Forgets the merge base, e.g. after the account is deleted. + pub fn forget(&self) { + let mut state = self.host.load_state(); + state.base = None; + state.last_synced_at = None; + let _ = self.host.save_state(&state); + self.set(Status::Off, None); + } + + /// Runs one sync. `choice` answers a `NeedsChoice`. + pub async fn sync(&self, choice: Option) -> SyncView { + let _running = self.running.lock().await; + match self.host.authorize().await { + Ok(None) => { + self.set(Status::Off, None); + return self.view(); + } + Ok(Some(auth)) => { + self.set(Status::Syncing, None); + match self.sync_as(&auth, choice).await { + Ok(Outcome::Synced) => self.set(Status::UpToDate, None), + Ok(Outcome::NeedsChoice) => self.set(Status::NeedsChoice, None), + Ok(Outcome::UpdateRequired) => { + self.set(Status::UpdateRequired, Some(UPDATE_REQUIRED.into())) + } + Err(error) => self.set(Status::Error, Some(error)), + } + } + Err(error) => self.set(Status::Error, Some(error)), + } + self.view() + } + + async fn sync_as(&self, auth: &Authorized, choice: Option) -> Result { + let mut state = self.host.load_state(); + if state.install_id.is_empty() { + state.install_id = uuid::Uuid::new_v4().to_string(); + self.host.save_state(&state)?; + } + for _ in 0..MAX_ATTEMPTS { + // A base from another account is never merged with this one. + let base = state + .base + .clone() + .filter(|base| base.user_id == auth.user_id); + let local = self.host.local().await?; + let row = self.remote.fetch(auth).await?; + let cloud = match &row { + None => None, + Some(row) => { + match portable_settings::parse(row.payload.clone(), row.schema_version) { + Ok(document) => Some(document), + // Never upload over a newer app's settings. + Err(ParseError::Newer(_)) => return Ok(Outcome::UpdateRequired), + Err(error @ ParseError::Invalid(_)) => return Err(error.to_string()), + } + } + }; + let remote = row.as_ref().zip(cloud.as_ref()); + let step = decide( + &local, + &self.host.untouched(), + base.as_ref(), + remote, + choice, + ); + let synced = match step { + Step::InSync => true, + Step::NeedsChoice => return Ok(Outcome::NeedsChoice), + Step::Adopt => { + let (row, _) = remote.expect("adopt needs a row"); + self.record(&mut state, auth, row.revision, local)?; + true + } + Step::Create(document) => { + match self + .remote + .create(auth, payload(&document)?, &state.install_id) + .await? + { + Some(row) => { + self.record(&mut state, auth, row.revision, document)?; + true + } + None => false, + } + } + Step::Upload { revision, document } => { + self.upload(&mut state, auth, revision, document).await? + } + Step::Apply { + document, + upload_over, + } => { + let applied = if document == local { + local + } else { + self.host.apply(document).await? + }; + match upload_over { + Some(revision) => self.upload(&mut state, auth, revision, applied).await?, + None => { + let (row, _) = remote.expect("apply needs a row"); + self.record(&mut state, auth, row.revision, applied)?; + true + } + } + } + }; + if synced { + return Ok(Outcome::Synced); + } + // Another computer wrote first: re-read and merge again. + } + Err("Settings changed on another computer at the same time. They will sync shortly.".into()) + } + + async fn upload( + &self, + state: &mut State, + auth: &Authorized, + revision: i64, + document: Document, + ) -> Result { + match self + .remote + .update(auth, revision, payload(&document)?, &state.install_id) + .await? + { + Some(row) => { + self.record(state, auth, row.revision, document)?; + Ok(true) + } + None => Ok(false), + } + } + + fn record( + &self, + state: &mut State, + auth: &Authorized, + revision: i64, + document: Document, + ) -> Result<(), String> { + state.base = Some(Base { + user_id: auth.user_id.clone(), + revision, + document, + }); + state.last_synced_at = Some(now_ms()); + self.host.save_state(state) + } + + /// Whether this install has changed since the last sync with `user_id`. + async fn changed_since_sync(&self) -> bool { + let Some(base) = self.host.load_state().base else { + return false; + }; + matches!(self.host.local().await, Ok(local) if local != base.document) + } + + /// Background loop: syncs at start, after `wake`, when local settings + /// have changed and settled, and periodically for other computers' + /// changes. Failures back off. + pub async fn run(self: Arc) { + let mut next_remote_check = Instant::now(); + let mut retry_at: Option = None; + let mut backoff = POLL; + let mut previous: Option = None; + loop { + let woken = tokio::select! { + () = self.wake.notified() => true, + () = tokio::time::sleep(POLL) => false, + }; + let now = Instant::now(); + let status = self.view().status; + let waiting_for_user = matches!(status, Status::NeedsChoice | Status::UpdateRequired); + let local = self.host.local().await.ok(); + // Upload once local changes have settled for one poll. + let settled_change = local.is_some() + && local == previous + && !waiting_for_user + && self.changed_since_sync().await; + previous = local; + let due = woken + || now >= next_remote_check + || (settled_change && retry_at.is_none_or(|at| now >= at)); + if !due { + continue; + } + next_remote_check = now + REMOTE_CHECK; + let view = self.sync(None).await; + if view.status == Status::Error { + retry_at = Some(Instant::now() + backoff); + backoff = (backoff * 2).min(MAX_BACKOFF); + } else { + retry_at = None; + backoff = POLL; + } + } + } +} + +fn payload(document: &Document) -> Result { + let value = serde_json::to_value(document).map_err(|error| error.to_string())?; + if value.to_string().len() > MAX_PAYLOAD_BYTES { + return Err("Settings are too large to sync. Remove some switch profiles.".into()); + } + Ok(value) +} + +/// PostgREST access with the signed-in user's token. +pub struct HttpRemote { + client: reqwest::Client, + url: String, + key: String, +} + +impl HttpRemote { + pub fn new(url: String, key: String) -> Self { + Self { + client: reqwest::Client::builder() + .timeout(Duration::from_secs(15)) + .build() + .unwrap_or_default(), + url, + key, + } + } + + fn request( + &self, + method: reqwest::Method, + query: &str, + auth: &Authorized, + ) -> reqwest::RequestBuilder { + self.client + .request( + method, + format!("{}/rest/v1/desktop_preferences?{query}", self.url), + ) + .header("apikey", &self.key) + .bearer_auth(&auth.access_token) + .header("Prefer", "return=representation") + } + + async fn rows(request: reqwest::RequestBuilder) -> Result<(u16, Vec), String> { + let unreachable = |_| "Could not reach Switchify to sync settings.".to_string(); + let response = request.send().await.map_err(unreachable)?; + let status = response.status().as_u16(); + let body: Value = response.json().await.unwrap_or(Value::Null); + let rows = body + .as_array() + .map(|rows| { + rows.iter() + .filter_map(|row| { + Some(Row { + revision: row["revision"].as_i64()?, + schema_version: u32::try_from(row["schema_version"].as_u64()?).ok()?, + payload: row["payload"].clone(), + }) + }) + .collect() + }) + .unwrap_or_default(); + Ok((status, rows)) + } +} + +const COLUMNS: &str = "select=revision,schema_version,payload"; + +fn http_error(status: u16) -> String { + match status { + 401 | 403 => "Sign in again to sync settings.".into(), + _ => format!("Settings sync failed (HTTP {status}). It will retry."), + } +} + +impl Remote for HttpRemote { + fn fetch<'a>(&'a self, auth: &'a Authorized) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + let query = format!("{COLUMNS}&user_id=eq.{}", auth.user_id); + let (status, mut rows) = + Self::rows(self.request(reqwest::Method::GET, &query, auth)).await?; + match status { + 200 => Ok(rows.pop()), + _ => Err(http_error(status)), + } + }) + } + + fn create<'a>( + &'a self, + auth: &'a Authorized, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + let body = json!({ + "user_id": auth.user_id, + "schema_version": SCHEMA_VERSION, + "payload": payload, + "updated_by": updated_by, + }); + let request = self + .request(reqwest::Method::POST, COLUMNS, auth) + .json(&body); + let (status, mut rows) = Self::rows(request).await?; + match status { + 200 | 201 => rows.pop().map(Some).ok_or_else(|| http_error(status)), + 409 => Ok(None), + _ => Err(http_error(status)), + } + }) + } + + fn update<'a>( + &'a self, + auth: &'a Authorized, + revision: i64, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + let query = format!( + "{COLUMNS}&user_id=eq.{}&revision=eq.{revision}", + auth.user_id + ); + let body = json!({ + "schema_version": SCHEMA_VERSION, + "payload": payload, + "updated_by": updated_by, + }); + let request = self + .request(reqwest::Method::PATCH, &query, auth) + .json(&body); + let (status, mut rows) = Self::rows(request).await?; + match status { + 200 => Ok(rows.pop()), + _ => Err(http_error(status)), + } + }) + } +} + +/// The running app as the sync host. +pub struct AppHost { + app: AppHandle, + path: Option, +} + +impl AppHost { + pub fn new(app: AppHandle) -> Self { + let path = app + .path() + .app_config_dir() + .ok() + .map(|dir| dir.join(STATE_FILE)); + Self { app, path } + } +} + +impl Host for AppHost { + fn authorize(&self) -> BoxFuture<'_, Result, String>> { + Box::pin(async move { + let account = self.app.state::(); + if !account.current_view().await.signed_in { + return Ok(None); + } + match account.authorize().await { + Ok(auth) => Ok(Some(auth)), + // A rejected session has just signed this install out. + Err(_) if !account.current_view().await.signed_in => { + crate::account::emit(&self.app, &account.current_view().await); + Ok(None) + } + Err(error) => Err(error), + } + }) + } + + fn local(&self) -> BoxFuture<'_, Result> { + Box::pin(async move { Ok(Local::read(&self.app).document()) }) + } + + fn apply(&self, document: Document) -> BoxFuture<'_, Result> { + Box::pin(async move { + let (tx, rx) = tokio::sync::oneshot::channel(); + let handle = self.app.clone(); + self.app + .run_on_main_thread(move || { + let _ = tx.send(portable_settings::apply(&handle, document)); + }) + .map_err(|error| error.to_string())?; + let applied = rx + .await + .map_err(|_| "Applying synced settings was cancelled.".to_string())??; + if applied.profiles { + let _ = self.app.emit(PROFILES_EVENT, ()); + } + if applied.remote { + let _ = self.app.emit(REMOTE_SWITCHES_EVENT, ()); + } + Ok(Local::read(&self.app).document()) + }) + } + + fn untouched(&self) -> Document { + Local { + settings: crate::state::AppSettings::default(), + profiles: Vec::new(), + switches: crate::switches::Settings::default(), + point_scan: crate::point_scan::Config::default(), + remote: crate::remote_scan::Config::default(), + } + .document() + } + + fn load_state(&self) -> State { + self.path + .as_ref() + .and_then(|path| std::fs::read(path).ok()) + .and_then(|bytes| serde_json::from_slice(&bytes).ok()) + .unwrap_or_default() + } + + fn save_state(&self, state: &State) -> Result<(), String> { + let path = self + .path + .as_ref() + .ok_or("Settings sync cannot find the settings folder.")?; + let failed = |_| "Settings sync could not save its progress.".to_string(); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(failed)?; + } + let bytes = serde_json::to_vec_pretty(state).map_err(|_| "Encoding failed".to_string())?; + let temp = path.with_extension("json.tmp"); + std::fs::write(&temp, bytes).map_err(failed)?; + std::fs::rename(&temp, path).map_err(failed) + } + + fn publish(&self, view: &SyncView) { + let _ = self.app.emit(SYNC_EVENT, view); + } +} + +/// Installs the engine when accounts are configured and starts its loop. +pub fn install(app: &AppHandle) { + let Some((url, key)) = app.state::().endpoint() else { + return; + }; + let engine = Arc::new(Engine::new( + Arc::new(HttpRemote::new(url, key)), + Arc::new(AppHost::new(app.clone())), + )); + app.manage(engine.clone()); + tauri::async_runtime::spawn(engine.run()); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::state::AppSettings; + use std::collections::VecDeque; + + fn untouched() -> Document { + Local { + settings: AppSettings::default(), + profiles: Vec::new(), + switches: crate::switches::Settings::default(), + point_scan: crate::point_scan::Config::default(), + remote: crate::remote_scan::Config::default(), + } + .document() + } + + fn with_dwell(mut document: Document, enabled: bool) -> Document { + document.app.dwell_click_enabled = enabled; + document + } + + fn with_speed(mut document: Document, speed: usize) -> Document { + document.point_scan.speed = speed; + document + } + + fn auth(user: &str) -> Authorized { + Authorized { + user_id: user.into(), + access_token: "token".into(), + } + } + + /// (kind, expected revision, payload, updated_by) + type Write = (&'static str, Option, Value, String); + + #[derive(Default)] + struct FakeRemote { + row: Mutex>, + writes: Mutex>, + /// Simulates another computer writing just before our next write. + interleave: Mutex>, + fail: Mutex, + } + + impl FakeRemote { + fn holding(document: &Document, revision: i64) -> Self { + let remote = Self::default(); + *remote.row.lock().unwrap() = Some(Row { + revision, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(document).unwrap(), + }); + remote + } + fn document(&self) -> Option { + self.row + .lock() + .unwrap() + .as_ref() + .map(|row| serde_json::from_value(row.payload.clone()).unwrap()) + } + fn revision(&self) -> Option { + self.row.lock().unwrap().as_ref().map(|row| row.revision) + } + fn interleave(&self) { + if let Some(payload) = self.interleave.lock().unwrap().pop_front() { + let mut row = self.row.lock().unwrap(); + let revision = row.as_ref().map_or(1, |row| row.revision + 1); + *row = Some(Row { + revision, + schema_version: SCHEMA_VERSION, + payload, + }); + } + } + } + + impl Remote for FakeRemote { + fn fetch<'a>( + &'a self, + _auth: &'a Authorized, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + if *self.fail.lock().unwrap() { + return Err("offline".into()); + } + Ok(self.row.lock().unwrap().clone()) + }) + } + fn create<'a>( + &'a self, + _auth: &'a Authorized, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + self.interleave(); + self.writes.lock().unwrap().push(( + "create", + None, + payload.clone(), + updated_by.into(), + )); + let mut row = self.row.lock().unwrap(); + if row.is_some() { + return Ok(None); + } + *row = Some(Row { + revision: 1, + schema_version: SCHEMA_VERSION, + payload, + }); + Ok(row.clone()) + }) + } + fn update<'a>( + &'a self, + _auth: &'a Authorized, + revision: i64, + payload: Value, + updated_by: &'a str, + ) -> BoxFuture<'a, Result, String>> { + Box::pin(async move { + self.interleave(); + self.writes.lock().unwrap().push(( + "update", + Some(revision), + payload.clone(), + updated_by.into(), + )); + let mut row = self.row.lock().unwrap(); + match row.as_mut() { + Some(current) if current.revision == revision => { + current.revision += 1; + current.payload = payload; + Ok(Some(current.clone())) + } + _ => Ok(None), + } + }) + } + } + + struct FakeHost { + user: Mutex>, + local: Mutex, + state: Mutex, + applied: Mutex>, + published: Mutex>, + /// Mimics apply normalizing what it stores (e.g. profile versions). + transform: fn(Document) -> Document, + } + + impl FakeHost { + fn new(local: Document) -> Self { + Self { + user: Mutex::new(Some("user-1".into())), + local: Mutex::new(local), + state: Mutex::default(), + applied: Mutex::default(), + published: Mutex::default(), + transform: |document| document, + } + } + fn local_now(&self) -> Document { + self.local.lock().unwrap().clone() + } + fn set_local(&self, document: Document) { + *self.local.lock().unwrap() = document; + } + fn base(&self) -> Option { + self.state.lock().unwrap().base.clone() + } + } + + impl Host for FakeHost { + fn authorize(&self) -> BoxFuture<'_, Result, String>> { + Box::pin(async move { Ok(self.user.lock().unwrap().as_deref().map(auth)) }) + } + fn local(&self) -> BoxFuture<'_, Result> { + Box::pin(async move { Ok(self.local_now()) }) + } + fn apply(&self, document: Document) -> BoxFuture<'_, Result> { + Box::pin(async move { + self.applied.lock().unwrap().push(document.clone()); + let stored = (self.transform)(document); + self.set_local(stored.clone()); + Ok(stored) + }) + } + fn untouched(&self) -> Document { + untouched() + } + fn load_state(&self) -> State { + self.state.lock().unwrap().clone() + } + fn save_state(&self, state: &State) -> Result<(), String> { + *self.state.lock().unwrap() = state.clone(); + Ok(()) + } + fn publish(&self, view: &SyncView) { + self.published.lock().unwrap().push(view.clone()); + } + } + + fn engine(remote: &Arc, host: &Arc) -> Engine { + Engine::new(remote.clone(), host.clone()) + } + + #[tokio::test] + async fn signed_out_is_off_and_touches_nothing() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + *host.user.lock().unwrap() = None; + let view = engine(&remote, &host).sync(None).await; + assert_eq!(view.status, Status::Off); + assert!(remote.writes.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn first_computer_creates_the_row() { + let remote = Arc::new(FakeRemote::default()); + let local = with_dwell(untouched(), true); + let host = Arc::new(FakeHost::new(local.clone())); + let view = engine(&remote, &host).sync(None).await; + assert_eq!(view.status, Status::UpToDate); + assert!(view.last_synced_at.is_some()); + assert_eq!(remote.document(), Some(local.clone())); + let base = host.base().unwrap(); + assert_eq!((base.user_id.as_str(), base.revision), ("user-1", 1)); + let writes = remote.writes.lock().unwrap(); + let install_id = &writes[0].3; + assert!( + uuid::Uuid::parse_str(install_id).is_ok(), + "random install id" + ); + } + + #[tokio::test] + async fn untouched_computer_takes_the_account_settings() { + let cloud = with_dwell(untouched(), true); + let remote = Arc::new(FakeRemote::holding(&cloud, 4)); + let host = Arc::new(FakeHost::new(untouched())); + let view = engine(&remote, &host).sync(None).await; + assert_eq!(view.status, Status::UpToDate); + assert_eq!(host.local_now(), cloud); + assert!(remote.writes.lock().unwrap().is_empty()); + assert_eq!(host.base().unwrap().revision, 4); + } + + #[tokio::test] + async fn customised_computer_asks_before_replacing_anything() { + let cloud = with_dwell(untouched(), true); + let local = with_speed(untouched(), 4); + let remote = Arc::new(FakeRemote::holding(&cloud, 2)); + let host = Arc::new(FakeHost::new(local.clone())); + let engine = engine(&remote, &host); + assert_eq!(engine.sync(None).await.status, Status::NeedsChoice); + assert_eq!(host.local_now(), local); + assert!(remote.writes.lock().unwrap().is_empty()); + assert!(host.base().is_none()); + + // Keep this computer's settings: uploaded over the account's copy. + assert_eq!( + engine.sync(Some(Choice::Local)).await.status, + Status::UpToDate + ); + assert_eq!(remote.document(), Some(local)); + assert_eq!(remote.revision(), Some(3)); + } + + #[tokio::test] + async fn choosing_cloud_replaces_this_computers_settings() { + let cloud = with_dwell(untouched(), true); + let remote = Arc::new(FakeRemote::holding(&cloud, 2)); + let host = Arc::new(FakeHost::new(with_speed(untouched(), 4))); + let engine = engine(&remote, &host); + engine.sync(None).await; + assert_eq!( + engine.sync(Some(Choice::Cloud)).await.status, + Status::UpToDate + ); + assert_eq!(host.local_now(), cloud); + assert!(remote.writes.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn identical_settings_need_no_choice() { + let local = with_speed(untouched(), 4); + let remote = Arc::new(FakeRemote::holding(&local, 2)); + let host = Arc::new(FakeHost::new(local)); + assert_eq!( + engine(&remote, &host).sync(None).await.status, + Status::UpToDate + ); + assert!(remote.writes.lock().unwrap().is_empty()); + assert_eq!(host.base().unwrap().revision, 2); + } + + #[tokio::test] + async fn local_change_uploads_on_the_last_seen_revision() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + host.set_local(with_dwell(untouched(), true)); + assert!(engine.changed_since_sync().await); + engine.sync(None).await; + let last = remote.writes.lock().unwrap().last().cloned().unwrap(); + assert_eq!((last.0, last.1), ("update", Some(1))); + assert_eq!(remote.revision(), Some(2)); + assert!(!engine.changed_since_sync().await); + } + + #[tokio::test] + async fn remote_change_is_applied_and_not_echoed() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + // Another computer changes the account's copy. + let other = with_dwell(untouched(), true); + *remote.row.lock().unwrap() = Some(Row { + revision: 2, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(&other).unwrap(), + }); + engine.sync(None).await; + assert_eq!(host.local_now(), other); + assert_eq!(remote.writes.lock().unwrap().len(), 1, "only the create"); + assert_eq!(host.base().unwrap().revision, 2); + } + + #[tokio::test] + async fn changes_on_both_sides_merge_by_section() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + *remote.row.lock().unwrap() = Some(Row { + revision: 2, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(with_dwell(untouched(), true)).unwrap(), + }); + host.set_local(with_speed(untouched(), 4)); + engine.sync(None).await; + let merged = with_speed(with_dwell(untouched(), true), 4); + assert_eq!(host.local_now(), merged); + assert_eq!(remote.document(), Some(merged)); + assert_eq!(remote.revision(), Some(3)); + } + + #[test] + fn the_account_wins_a_section_changed_on_both_sides() { + let base = untouched(); + let local = with_speed(base.clone(), 4); + let cloud = with_speed(base.clone(), 1); + assert_eq!(merge(&base, &local, &cloud).point_scan.speed, 1); + } + + #[tokio::test] + async fn losing_a_write_race_re_reads_and_merges() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + host.set_local(with_speed(untouched(), 4)); + // Another computer writes between our read and our write. + remote + .interleave + .lock() + .unwrap() + .push_back(serde_json::to_value(with_dwell(untouched(), true)).unwrap()); + assert_eq!(engine.sync(None).await.status, Status::UpToDate); + let merged = with_speed(with_dwell(untouched(), true), 4); + assert_eq!(remote.document(), Some(merged.clone())); + assert_eq!(host.local_now(), merged); + } + + #[tokio::test] + async fn create_race_falls_back_to_the_other_computers_row() { + let remote = Arc::new(FakeRemote::default()); + let other = with_dwell(untouched(), true); + remote + .interleave + .lock() + .unwrap() + .push_back(serde_json::to_value(&other).unwrap()); + let host = Arc::new(FakeHost::new(untouched())); + assert_eq!( + engine(&remote, &host).sync(None).await.status, + Status::UpToDate + ); + assert_eq!( + host.local_now(), + other, + "untouched install takes the winner's" + ); + } + + #[tokio::test] + async fn newer_app_settings_are_never_overwritten() { + let remote = Arc::new(FakeRemote::default()); + *remote.row.lock().unwrap() = Some(Row { + revision: 9, + schema_version: SCHEMA_VERSION + 1, + payload: json!({"futureShape": true}), + }); + let host = Arc::new(FakeHost::new(with_speed(untouched(), 4))); + let engine = engine(&remote, &host); + let view = engine.sync(Some(Choice::Local)).await; + assert_eq!(view.status, Status::UpdateRequired); + assert!(view.message.unwrap().contains("Update Switchify PC")); + assert!(remote.writes.lock().unwrap().is_empty()); + assert!(host.applied.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn invalid_account_copy_is_an_error_and_not_overwritten() { + let remote = Arc::new(FakeRemote::default()); + *remote.row.lock().unwrap() = Some(Row { + revision: 3, + schema_version: SCHEMA_VERSION, + payload: json!({"schemaVersion": 1}), + }); + let host = Arc::new(FakeHost::new(untouched())); + let view = engine(&remote, &host).sync(None).await; + assert_eq!(view.status, Status::Error); + assert!(remote.writes.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn another_accounts_base_is_never_merged() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + // A different account (e.g. deleted and recreated) with its own row. + *host.user.lock().unwrap() = Some("user-2".into()); + let cloud = with_dwell(untouched(), true); + *remote.row.lock().unwrap() = Some(Row { + revision: 1, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(&cloud).unwrap(), + }); + host.set_local(with_speed(untouched(), 4)); + // Same revision number as the old base, but it must not count. + assert_eq!(engine.sync(None).await.status, Status::NeedsChoice); + } + + #[tokio::test] + async fn normalized_apply_becomes_the_base_so_nothing_ping_pongs() { + let cloud = with_dwell(untouched(), true); + let remote = Arc::new(FakeRemote::holding(&cloud, 2)); + let mut host = FakeHost::new(untouched()); + // Applying stores something slightly different (like a profile + // version bump); that stored copy must become the base. + host.transform = |document| with_speed(document, 3); + let host = Arc::new(host); + let engine = engine(&remote, &host); + engine.sync(None).await; + assert_eq!(host.base().unwrap().document, host.local_now()); + assert!(!engine.changed_since_sync().await); + engine.sync(None).await; + assert!(remote.writes.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn network_failure_reports_an_error() { + let remote = Arc::new(FakeRemote::default()); + *remote.fail.lock().unwrap() = true; + let host = Arc::new(FakeHost::new(untouched())); + let view = engine(&remote, &host).sync(None).await; + assert_eq!(view.status, Status::Error); + assert_eq!(view.message.as_deref(), Some("offline")); + } + + #[tokio::test] + async fn forget_clears_the_base_and_turns_off() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + engine.forget(); + assert!(host.base().is_none()); + assert_eq!(engine.view().status, Status::Off); + assert!( + !host.state.lock().unwrap().install_id.is_empty(), + "install id kept" + ); + } + + #[tokio::test] + async fn oversized_documents_are_not_uploaded() { + let mut local = untouched(); + local.remote_switches[0].name = Some("x".repeat(MAX_PAYLOAD_BYTES)); + assert!(payload(&local).is_err()); + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(local)); + assert_eq!( + engine(&remote, &host).sync(None).await.status, + Status::Error + ); + assert!(remote.writes.lock().unwrap().is_empty()); + } + + /// Against a local `supabase start` stack (switchify-supabase). Run with: + /// SWITCHIFY_LOCAL_SUPABASE_URL=http://127.0.0.1:54321 + /// SWITCHIFY_LOCAL_SUPABASE_KEY= + /// cargo test settings_sync::tests::local_stack -- --ignored + #[tokio::test] + #[ignore = "needs a local Supabase stack"] + async fn local_stack_round_trips_the_document_and_enforces_revisions() { + let url = std::env::var("SWITCHIFY_LOCAL_SUPABASE_URL").unwrap(); + let key = std::env::var("SWITCHIFY_LOCAL_SUPABASE_KEY").unwrap(); + let client = reqwest::Client::new(); + let mut users = Vec::new(); + for _ in 0..2 { + let signup: Value = client + .post(format!("{url}/auth/v1/signup")) + .header("apikey", &key) + .json(&json!({ + "email": format!("sync-{}@example.test", uuid::Uuid::new_v4()), + "password": "local-only-password", + })) + .send() + .await + .unwrap() + .json() + .await + .unwrap(); + users.push(Authorized { + user_id: signup["user"]["id"].as_str().unwrap().into(), + access_token: signup["access_token"].as_str().unwrap().into(), + }); + } + let (me, other) = (&users[0], &users[1]); + let remote = HttpRemote::new(url, key); + + assert_eq!(remote.fetch(me).await.unwrap(), None); + // The untouched document has explicit nulls (e.g. unassigned remote + // slots); jsonb must keep them for the strict parser. + let document = untouched(); + let value = payload(&document).unwrap(); + assert!(value.to_string().contains("null")); + let created = remote + .create(me, value.clone(), "install") + .await + .unwrap() + .unwrap(); + assert_eq!(created.revision, 1); + let fetched = remote.fetch(me).await.unwrap().unwrap(); + assert_eq!( + portable_settings::parse(fetched.payload, fetched.schema_version).unwrap(), + document + ); + assert_eq!( + remote.create(me, value.clone(), "install").await.unwrap(), + None, + "409" + ); + + let changed = payload(&with_dwell(untouched(), true)).unwrap(); + let updated = remote + .update(me, 1, changed.clone(), "install") + .await + .unwrap() + .unwrap(); + assert_eq!(updated.revision, 2); + assert_eq!( + remote.update(me, 1, value, "install").await.unwrap(), + None, + "stale" + ); + + // Another account sees nothing of mine and cannot write to it. + assert_eq!(remote.fetch(other).await.unwrap(), None); + let forged = Authorized { + user_id: me.user_id.clone(), + access_token: other.access_token.clone(), + }; + assert_eq!( + remote.update(&forged, 2, json!({}), "x").await.unwrap(), + None + ); + assert_eq!(remote.fetch(me).await.unwrap().unwrap().payload, changed); + } +} diff --git a/src/App.test.tsx b/src/App.test.tsx index 56152c8a..8c0840ef 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -580,6 +580,22 @@ describe("Switchify PC shell", () => { await waitFor(() => expect(opener).toHaveFocus()); }); + it("refreshes Switch Forwarding profiles when settings sync changes them", async () => { + let changed: () => void = () => undefined; + vi.spyOn(api, "onProfilesChanged").mockImplementation(async (handler) => { changed = handler; return () => undefined; }); + const list = vi.spyOn(api, "listProfiles"); + render(); + fireEvent.click(await screen.findByRole("button", { name: "Mobile" })); + fireEvent.click(screen.getByRole("tab", { name: "Switch Forwarding" })); + await screen.findByRole("button", { name: "New profile" }); + const before = list.mock.calls.length; + list.mockResolvedValueOnce([{ id: "builtin.keyboard", version: 1, name: "Generic keyboard", provider: "mapped", builtIn: true, bindings: [] }, + { id: "6f1c1a52-6a0e-4c2b-9d0f-2f8a7b9e4c11", version: 2, name: "Synced desk", provider: "mapped", builtIn: false, bindings: [] }]); + act(() => changed()); + await waitFor(() => expect(list.mock.calls.length).toBe(before + 1)); + expect(await screen.findByRole("button", { name: /Synced desk/ })).toBeInTheDocument(); + }); + it("closes pristine new and duplicated profiles without a discard prompt", async () => { render(); fireEvent.click(await screen.findByRole("button", { name: "Mobile" })); diff --git a/src/App.tsx b/src/App.tsx index 0b0215c4..df242f55 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -733,6 +733,15 @@ export function App() { }, []); useEffect(() => { if (view === "profiles") void api.listProfiles().then(setProfiles).catch((reason) => setError(String(reason))); }, [view]); + // Settings sync can change profiles from another computer; refresh the list + // unless an edit is in progress, which a refresh would discard. + useEffect(() => { + if (view !== "profiles") return; + const stop = api.onProfilesChanged(() => { + if (!profileEditorDirty.current) void api.listProfiles().then(setProfiles).catch((reason) => setError(String(reason))); + }); + return () => { void stop.then((unlisten) => unlisten()); }; + }, [view]); const nav = useMemo(() => [ ["home", "Home", ], ["switches", "Switches", ], ["scanning", "Scanning", ], ["mouse", "Mouse", ], ["mobile", "Mobile", ], ["settings", "Settings", ], ["support", "Help", ], diff --git a/src/api.ts b/src/api.ts index 63e726a4..f2614c29 100644 --- a/src/api.ts +++ b/src/api.ts @@ -1,6 +1,6 @@ import { invoke } from "@tauri-apps/api/core"; import { listen, type UnlistenFn } from "@tauri-apps/api/event"; -import type { AccountView, AppSettings, AppState, SwitchProfile } from "./types"; +import type { AccountView, AppSettings, AppState, SwitchProfile, SyncView } from "./types"; export type ProfileExitAction = "hide" | "quit"; export type NavigationTarget = "home" | "settings" | "profiles"; @@ -120,6 +120,19 @@ export const api = { cancelSignIn: () => accountCall("cancel_sign_in"), signOut: () => accountCall("sign_out"), deleteAccount: () => accountCall("delete_account"), + settingsSync: () => "__TAURI_INTERNALS__" in window + ? invoke("get_settings_sync") + : Promise.resolve({ status: "off", lastSyncedAt: null, message: null }), + syncSettingsNow: () => invoke("sync_settings_now"), + resolveSettingsSync: (choice: "local" | "cloud") => invoke("resolve_settings_sync", { choice }), + onSettingsSync: async (handler: (view: SyncView) => void): Promise => { + if (!("__TAURI_INTERNALS__" in window)) return () => undefined; + return listen("settings-sync-changed", (event) => handler(event.payload)); + }, + onProfilesChanged: async (handler: () => void): Promise => { + if (!("__TAURI_INTERNALS__" in window)) return () => undefined; + return listen("switch-profiles-changed", () => handler()); + }, onAccount: async (handler: (account: AccountView) => void): Promise => { if (!("__TAURI_INTERNALS__" in window)) return () => undefined; return listen("account-changed", (event) => handler(event.payload)); diff --git a/src/scanning/useRemoteSwitches.test.tsx b/src/scanning/useRemoteSwitches.test.tsx new file mode 100644 index 00000000..18c95b8b --- /dev/null +++ b/src/scanning/useRemoteSwitches.test.tsx @@ -0,0 +1,56 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { useRemoteSwitches, type RemoteConfig } from "./useRemoteSwitches"; + +const mocks = vi.hoisted(() => ({ invoke: vi.fn(), listen: vi.fn() })); +vi.mock("@tauri-apps/api/core", () => ({ invoke: mocks.invoke })); +vi.mock("@tauri-apps/api/event", () => ({ listen: mocks.listen })); + +const slots = (first: string | null) => Array.from({ length: 8 }, (_, index) => ({ + pressAction: index === 0 ? first : null, + holdActions: [], +})); +const config = (first: string | null, revision = 1): RemoteConfig => ({ schemaVersion: 1, revision, slots: slots(first) as RemoteConfig["slots"] }); + +let stored: RemoteConfig; +const syncEvent = () => mocks.listen.mock.calls.find(([name]) => name === "remote-switches-changed")![1](); + +beforeEach(() => { + Object.defineProperty(window, "__TAURI_INTERNALS__", { value: {}, configurable: true }); + stored = config("select"); + mocks.listen.mockReset().mockResolvedValue(vi.fn()); + mocks.invoke.mockReset().mockImplementation(async (command: string, args?: { config: RemoteConfig }) => { + if (command === "get_remote_switches") return structuredClone(stored); + if (command === "save_remote_switches") { + stored = { ...args!.config, revision: stored.revision + 1 }; + return structuredClone(stored); + } + throw new Error(command); + }); +}); +afterEach(() => Reflect.deleteProperty(window, "__TAURI_INTERNALS__")); + +it("reloads slots that settings sync replaced", async () => { + const { result } = renderHook(() => useRemoteSwitches()); + await waitFor(() => expect(result.current.config?.slots[0].pressAction).toBe("select")); + stored = config("next", 5); + await act(async () => { syncEvent(); }); + await waitFor(() => expect(result.current.config?.slots[0].pressAction).toBe("next")); +}); + +it("keeps edits that are still saving instead of reloading over them", async () => { + let finishSave: () => void = () => undefined; + mocks.invoke.mockImplementation(async (command: string, args?: { config: RemoteConfig }) => { + if (command === "get_remote_switches") return structuredClone(stored); + await new Promise((resolve) => { finishSave = resolve; }); + stored = { ...args!.config, revision: stored.revision + 1 }; + return structuredClone(stored); + }); + const { result } = renderHook(() => useRemoteSwitches()); + await waitFor(() => expect(result.current.config).not.toBeNull()); + act(() => result.current.update(slots("back") as RemoteConfig["slots"])); + await act(async () => { syncEvent(); }); + expect(result.current.config?.slots[0].pressAction).toBe("back"); + expect(mocks.invoke.mock.calls.filter(([name]) => name === "get_remote_switches")).toHaveLength(1); + await act(async () => { finishSave(); }); +}); diff --git a/src/scanning/useRemoteSwitches.ts b/src/scanning/useRemoteSwitches.ts index c1a6eac8..23003d71 100644 --- a/src/scanning/useRemoteSwitches.ts +++ b/src/scanning/useRemoteSwitches.ts @@ -1,5 +1,6 @@ import { useEffect, useRef, useState } from "react"; import { invoke } from "@tauri-apps/api/core"; +import { listen } from "@tauri-apps/api/event"; import type { SwitchAction } from "./useSwitches"; // Switches forwarded from Switchify Remote. Slots are positional: slot N is @@ -40,8 +41,20 @@ export function useRemoteSwitches() { .catch((e) => { if (alive) setError(String(e)); }); + // Settings sync can replace the slots; reload them unless edits are + // still saving, in which case those edits win and sync on afterwards. + const stop = listen("remote-switches-changed", () => { + const m = model.current; + if (m.pending || m.revision !== m.saved) return; + void invoke("get_remote_switches").then((value) => { + if (!alive || m.pending || m.revision !== m.saved) return; + m.config = value; + setConfig(value); + }); + }); return () => { alive = false; + void stop.then((unlisten) => unlisten()); }; }, []); const save = (revision: number, next: RemoteConfig) => { diff --git a/src/settings/AccountSection.test.tsx b/src/settings/AccountSection.test.tsx index 3b1eadcd..ad11645b 100644 --- a/src/settings/AccountSection.test.tsx +++ b/src/settings/AccountSection.test.tsx @@ -1,22 +1,75 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { api } from "../api"; -import type { AccountView } from "../types"; +import type { AccountView, SyncView } from "../types"; import { AccountSection } from "./AccountSection"; const signedOut: AccountView = { available: true, signedIn: false, email: null, pendingEmail: null, keychainUnavailable: false }; const pending: AccountView = { ...signedOut, pendingEmail: "me@example.com" }; const signedIn: AccountView = { ...signedOut, signedIn: true, email: "me@example.com" }; -function start(view: AccountView) { +function start(view: AccountView, sync: SyncView = { status: "upToDate", lastSyncedAt: null, message: null }) { vi.spyOn(api, "account").mockResolvedValue(view); vi.spyOn(api, "onAccount").mockResolvedValue(() => undefined); + vi.spyOn(api, "settingsSync").mockResolvedValue(sync); + vi.spyOn(api, "onSettingsSync").mockResolvedValue(() => undefined); render(); } describe("AccountSection", () => { afterEach(() => vi.restoreAllMocks()); + describe("settings sync", () => { + it("shows when settings last synced and can sync now", async () => { + start(signedIn, { status: "upToDate", lastSyncedAt: Date.UTC(2026, 9, 5, 12, 0), message: null }); + expect(await screen.findByText(/Settings are up to date. Last synced/)).toBeInTheDocument(); + const now = vi.spyOn(api, "syncSettingsNow").mockResolvedValue({ status: "upToDate", lastSyncedAt: Date.now(), message: null }); + fireEvent.click(screen.getByRole("button", { name: "Sync now" })); + expect(now).toHaveBeenCalled(); + }); + + it("asks which settings to keep and resolves the choice", async () => { + start(signedIn, { status: "needsChoice", lastSyncedAt: null, message: null }); + expect(await screen.findByText(/different settings. Choose which to keep/)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Sync now" })).not.toBeInTheDocument(); + const resolve = vi.spyOn(api, "resolveSettingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: Date.now(), message: null }); + fireEvent.click(screen.getByRole("button", { name: "Keep this computer's settings" })); + expect(resolve).toHaveBeenCalledWith("local"); + await waitFor(() => expect(screen.getByRole("button", { name: "Sync now" })).toHaveFocus()); + }); + + it("can take the account settings instead", async () => { + start(signedIn, { status: "needsChoice", lastSyncedAt: null, message: null }); + const resolve = vi.spyOn(api, "resolveSettingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: Date.now(), message: null }); + fireEvent.click(await screen.findByRole("button", { name: "Use my account's settings" })); + expect(resolve).toHaveBeenCalledWith("cloud"); + }); + + it("shows the backend message for errors and newer app versions", async () => { + start(signedIn, { status: "updateRequired", lastSyncedAt: null, message: "Your synced settings were saved by a newer Switchify PC." }); + expect(await screen.findByText(/saved by a newer Switchify PC/)).toBeInTheDocument(); + }); + + it("follows sync changes from the backend", async () => { + let receive: (view: SyncView) => void = () => undefined; + vi.spyOn(api, "account").mockResolvedValue(signedIn); + vi.spyOn(api, "onAccount").mockResolvedValue(() => undefined); + vi.spyOn(api, "settingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: null, message: null }); + vi.spyOn(api, "onSettingsSync").mockImplementation(async (handler) => { receive = handler; return () => undefined; }); + render(); + await screen.findByText("Settings are up to date."); + receive({ status: "error", lastSyncedAt: null, message: "Could not reach Switchify to sync settings." }); + expect(await screen.findByText("Could not reach Switchify to sync settings.")).toBeInTheDocument(); + }); + + it("is hidden while confirming account deletion", async () => { + start(signedIn); + await screen.findByRole("button", { name: "Sync now" }); + fireEvent.click(screen.getByRole("button", { name: "Delete account…" })); + expect(screen.queryByRole("button", { name: "Sync now" })).not.toBeInTheDocument(); + }); + }); + it("says when accounts are unavailable in this build", async () => { start({ ...signedOut, available: false }); expect(await screen.findByText("Accounts are unavailable in this build.")).toBeInTheDocument(); @@ -41,7 +94,7 @@ describe("AccountSection", () => { fireEvent.click(screen.getByRole("button", { name: "Sign in" })); expect(verify).toHaveBeenCalledWith("123456"); await waitFor(() => expect(screen.getByRole("button", { name: "Sign out" })).toHaveFocus()); - expect(screen.getByRole("status")).toHaveTextContent("Signed in as me@example.com."); + expect(screen.getByText(/Signed in as/)).toHaveTextContent("Signed in as me@example.com."); }); it("shows a wrong-code error and keeps the code form", async () => { @@ -156,6 +209,8 @@ describe("AccountSection", () => { let receive: (view: AccountView) => void = () => undefined; vi.spyOn(api, "account").mockResolvedValue(signedOut); vi.spyOn(api, "onAccount").mockImplementation(async (handler) => { receive = handler; return () => undefined; }); + vi.spyOn(api, "settingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: null, message: null }); + vi.spyOn(api, "onSettingsSync").mockResolvedValue(() => undefined); render(<>); await screen.findByLabelText("Email"); const elsewhere = screen.getByRole("button", { name: "Elsewhere" }); @@ -181,6 +236,8 @@ describe("AccountSection", () => { let receive: (view: AccountView) => void = () => undefined; vi.spyOn(api, "account").mockResolvedValue(signedOut); vi.spyOn(api, "onAccount").mockImplementation(async (handler) => { receive = handler; return () => undefined; }); + vi.spyOn(api, "settingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: null, message: null }); + vi.spyOn(api, "onSettingsSync").mockResolvedValue(() => undefined); render(); await screen.findByLabelText("Email"); receive(signedIn); diff --git a/src/settings/AccountSection.tsx b/src/settings/AccountSection.tsx index 85970c75..813e9a3e 100644 --- a/src/settings/AccountSection.tsx +++ b/src/settings/AccountSection.tsx @@ -1,10 +1,10 @@ import { useEffect, useRef, useState, type FormEvent } from "react"; import { api } from "../api"; import { Button, Input } from "../ui/controls"; -import type { AccountView } from "../types"; +import type { AccountView, SyncView } from "../types"; import { SettingGroup } from "./controls"; -const description = "Sign in with the same account as Switchify on Android. We email you a code; there is no password."; +const description = "Sign in to bring your settings to your other computers. It is the same account as Switchify on Android. We email you a code; there is no password."; type FocusTarget = "email" | "code" | "signOut" | "deleteAccount" | "keepAccount"; type Screen = "loading" | "unavailable" | "keychain" | "signedIn" | "pending" | "signedOut"; @@ -125,6 +125,7 @@ export function AccountSection() { if (account.signedIn) { return

Signed in as {account.email}.

+ {!confirmDelete && } {confirmDelete ?

Deleting your account removes it and its saved settings from every device, including Switchify on Android. This cannot be undone.

@@ -171,3 +172,67 @@ export function AccountSection() { ; } + +const syncText: Record = { + off: "Settings sync is starting.", + syncing: "Syncing settings…", + upToDate: "Settings are up to date.", + needsChoice: "This computer and your account have different settings. Choose which to keep; the other is replaced.", + updateRequired: "", + error: "", +}; + +/** Sync status for the signed-in account, with the first-sync choice. */ +function SyncPanel() { + const [sync, setSync] = useState(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const syncNowRef = useRef(null); + const restoreFocus = useRef(false); + + useEffect(() => { + let active = true; + void api.settingsSync().then((view) => { if (active) setSync(view); }); + const unlisten = api.onSettingsSync((view) => setSync(view)); + return () => { active = false; void unlisten.then((stop) => stop()); }; + }, []); + + useEffect(() => { + if (restoreFocus.current && sync?.status !== "needsChoice") { + restoreFocus.current = false; + syncNowRef.current?.focus(); + } + }); + + const run = async (action: () => Promise, fromChoice = false) => { + if (busy) return; + setBusy(true); + setError(null); + try { + restoreFocus.current = fromChoice; + setSync(await action()); + } catch (failure) { + setError(failure instanceof Error ? failure.message : String(failure)); + } finally { + setBusy(false); + } + }; + + if (!sync) return null; + const last = sync.lastSyncedAt ? new Date(sync.lastSyncedAt).toLocaleString([], { dateStyle: "medium", timeStyle: "short" }) : null; + const text = sync.message ?? syncText[sync.status]; + return
+

+ {text}{sync.status === "upToDate" && last && <> Last synced {last}.} +

+ {sync.status === "needsChoice" + ?
+ + +
+ :
+ +
} + {error && {error}} +
; +} diff --git a/src/types.ts b/src/types.ts index 95cbbc9b..86e8813d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -88,3 +88,11 @@ export type AccountView = { pendingEmail: string | null; keychainUnavailable: boolean; }; + +export type SyncStatus = "off" | "syncing" | "upToDate" | "needsChoice" | "updateRequired" | "error"; + +export type SyncView = { + status: SyncStatus; + lastSyncedAt: number | null; + message: string | null; +}; From 61374f72aba961d6337a804faba41bb11c574af9 Mon Sep 17 00:00:00 2001 From: Owen McGirr Date: Mon, 5 Oct 2026 14:41:08 +0100 Subject: [PATCH 2/5] Merge sync changes without losing edits Address review on the settings sync engine: - Keep the account's copy and this computer's copy as separate merge bases, so a value normalized on apply (e.g. a profile version) never looks like an account change that silently overrides a later local edit. - Apply only if local settings still match what the merge was computed from, checked on the main thread; otherwise re-read and merge again, so an edit made during a sync is not reverted. - When an apply is followed by an upload, record an interim base first so a failed upload neither re-uploads the account's sections nor forgets this computer's changes. - Merge profiles by id: profiles added on two computers are both kept, an edit survives a deletion on the other side, and merged names stay unique. - Keep sync state in memory, stay idle while signed out or waiting for the user, and retry failed syncs on backoff even without a local change. - Report a deleted account (409 foreign key) instead of retrying a create. - forget() waits for a running sync; opening the Account tab wakes sync if a locked keychain hid the session at startup. - Sync panel moves focus to the choice when it replaces the focused button; the remote switches reload handles errors. Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/lib.rs | 20 +- src-tauri/src/settings_sync.rs | 507 ++++++++++++++++++++++----- src/scanning/useRemoteSwitches.ts | 2 + src/settings/AccountSection.test.tsx | 9 + src/settings/AccountSection.tsx | 15 +- 5 files changed, 454 insertions(+), 99 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index fe753049..e6fe8195 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -827,8 +827,20 @@ fn delete_switch_profile( } #[tauri::command] -async fn get_account(account: State<'_, account::Account>) -> Result { - Ok(account.view().await) +async fn get_account( + app: AppHandle, + account: State<'_, account::Account>, +) -> Result { + let view = account.view().await; + // Opening the tab can find a session a locked keychain hid at startup. + if view.signed_in { + if let Some(sync) = sync_engine(&app) { + if sync.view().status == settings_sync::Status::Off { + sync.wake(); + } + } + } + Ok(view) } #[tauri::command] @@ -893,8 +905,8 @@ async fn delete_account( // Emitted on failure too: a rejected session signs out mid-action. account::emit(&app, &account.current_view().await); if result.is_ok() { - if let Some(sync) = app.try_state::>() { - sync.forget(); + if let Some(sync) = sync_engine(&app) { + sync.forget().await; } } result diff --git a/src-tauri/src/settings_sync.rs b/src-tauri/src/settings_sync.rs index 8f1a5f9d..f53a7714 100644 --- a/src-tauri/src/settings_sync.rs +++ b/src-tauri/src/settings_sync.rs @@ -9,6 +9,7 @@ use crate::account::{Account, Authorized}; use crate::portable_settings::{self, Document, Local, ParseError, SCHEMA_VERSION}; +use crate::state::SwitchProfile; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use std::future::Future; @@ -43,13 +44,18 @@ pub struct Row { pub payload: Value, } -/// The last document synced with an account: the merge base. +/// The last sync with an account: the merge base. The account's copy and +/// this install's copy are kept apart because applying can normalize values +/// (e.g. profile versions), so the two need not be equal. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct Base { user_id: String, revision: i64, - document: Document, + /// The account's document at `revision`. + cloud: Document, + /// This install's document right after that sync. + local: Document, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -124,9 +130,15 @@ pub trait Host: Send + Sync { /// `Ok(None)` when no one is signed in. fn authorize(&self) -> BoxFuture<'_, Result, String>>; fn local(&self) -> BoxFuture<'_, Result>; - /// Applies a document and returns the resulting local document, which - /// can differ (e.g. profile versions) and becomes the merge base. - fn apply(&self, document: Document) -> BoxFuture<'_, Result>; + /// Applies `document` if this install still matches `expected` (the + /// settings the merge was computed from), returning the resulting local + /// document, which can differ (e.g. profile versions). `Ok(None)` means + /// the user changed something meanwhile; the sync re-reads and retries. + fn apply( + &self, + expected: Document, + document: Document, + ) -> BoxFuture<'_, Result, String>>; /// The document of an install whose settings were never changed. fn untouched(&self) -> Document; fn load_state(&self) -> State; @@ -158,10 +170,11 @@ enum Step { revision: i64, document: Document, }, - /// Apply `document` locally, then upload it if the account differs. + /// Apply `document` locally, then upload the result if it differs from + /// the account's copy. Apply { document: Document, - upload_over: Option, + upload: bool, }, NeedsChoice, } @@ -185,13 +198,13 @@ fn decide( }, Choice::Cloud => Step::Apply { document: cloud.clone(), - upload_over: None, + upload: false, }, }; } match base { Some(base) if base.revision == row.revision => { - if *local == base.document { + if *local == base.local { Step::InSync } else { Step::Upload { @@ -201,12 +214,12 @@ fn decide( } } Some(base) => { - let merged = merge(&base.document, local, cloud); + let merged = merge(base, local, cloud); if merged == *local && merged == *cloud { Step::Adopt } else { Step::Apply { - upload_over: (merged != *cloud).then_some(row.revision), + upload: merged != *cloud, document: merged, } } @@ -215,38 +228,100 @@ fn decide( // Nothing to lose on a computer that was never set up differently. None if local == untouched => Step::Apply { document: cloud.clone(), - upload_over: None, + upload: false, }, None => Step::NeedsChoice, } } -/// Three-way merge by section. A section changed on only one side takes -/// that side; changed on both, the account's copy wins. -fn merge(base: &Document, local: &Document, cloud: &Document) -> Document { - fn pick(base: &T, local: &T, cloud: &T) -> T { - if local == base { - cloud.clone() - } else if cloud == base { - local.clone() - } else { - cloud.clone() - } +/// Picks between this install's and the account's version of one value. +/// Local changes are measured against the local base and account changes +/// against the account base, so normalization on one side (e.g. a profile +/// version bump on apply) never looks like a change on the other. Changed +/// on both sides, the account's copy wins. +fn pick(base_local: &T, base_cloud: &T, local: &T, cloud: &T) -> T { + if local == base_local { + cloud.clone() + } else if cloud == base_cloud { + local.clone() + } else { + cloud.clone() } +} + +/// Three-way merge by section; profiles merge one by one. +fn merge(base: &Base, local: &Document, cloud: &Document) -> Document { + let (bl, bc) = (&base.local, &base.cloud); Document { schema_version: SCHEMA_VERSION, - app: pick(&base.app, &local.app, &cloud.app), - profiles: pick(&base.profiles, &local.profiles, &cloud.profiles), - switches: pick(&base.switches, &local.switches, &cloud.switches), - point_scan: pick(&base.point_scan, &local.point_scan, &cloud.point_scan), + app: pick(&bl.app, &bc.app, &local.app, &cloud.app), + profiles: merge_profiles(&bl.profiles, &bc.profiles, &local.profiles, &cloud.profiles), + switches: pick(&bl.switches, &bc.switches, &local.switches, &cloud.switches), + point_scan: pick( + &bl.point_scan, + &bc.point_scan, + &local.point_scan, + &cloud.point_scan, + ), remote_switches: pick( - &base.remote_switches, + &bl.remote_switches, + &bc.remote_switches, &local.remote_switches, &cloud.remote_switches, ), } } +/// Merges custom profiles by id, so profiles added or edited on different +/// computers at the same time are all kept. A profile deleted on one side +/// stays deleted unless the other side changed it since the last sync. +/// Names made equal by the merge get a numbered suffix, since names must +/// stay unique. +fn merge_profiles( + base_local: &[SwitchProfile], + base_cloud: &[SwitchProfile], + local: &[SwitchProfile], + cloud: &[SwitchProfile], +) -> Vec { + let find = |list: &[SwitchProfile], id: &str| list.iter().find(|p| p.id == id).cloned(); + let mut ids: Vec = cloud.iter().map(|p| p.id.clone()).collect(); + ids.extend( + local + .iter() + .filter(|p| !cloud.iter().any(|c| c.id == p.id)) + .map(|p| p.id.clone()), + ); + let mut merged: Vec = ids + .iter() + .filter_map(|id| { + let (bl, bc) = (find(base_local, id), find(base_cloud, id)); + let (l, c) = (find(local, id), find(cloud, id)); + let local_changed = l != bl; + let cloud_changed = c != bc; + match (local_changed, cloud_changed) { + (false, _) => c, + (true, false) => l, + // Both changed: the account wins, but an edit is never lost + // to a deletion on the other side. + (true, true) => c.or(l), + } + }) + .collect(); + let mut seen = std::collections::HashSet::new(); + for profile in &mut merged { + if !seen.insert(profile.name.to_ascii_lowercase()) { + let base: String = profile.name.chars().take(44).collect(); + let name = (2..) + .map(|n| format!("{base} ({n})")) + .find(|candidate| !seen.contains(&candidate.to_ascii_lowercase())) + .expect("an unused suffix always exists"); + seen.insert(name.to_ascii_lowercase()); + profile.name = name; + } + } + merged +} + fn now_ms() -> i64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -263,23 +338,26 @@ enum Outcome { pub struct Engine { remote: Arc, host: Arc, - // Only one sync runs at a time. + // Only one sync (or forget) runs at a time. running: tokio::sync::Mutex<()>, + // Mirrors the state file, so polling never re-reads it. + state: Mutex, view: Mutex, wake: tokio::sync::Notify, } impl Engine { pub fn new(remote: Arc, host: Arc) -> Self { - let last_synced_at = host.load_state().last_synced_at; + let state = host.load_state(); Self { remote, host, running: tokio::sync::Mutex::new(()), view: Mutex::new(SyncView { - last_synced_at, + last_synced_at: state.last_synced_at, ..SyncView::off() }), + state: Mutex::new(state), wake: tokio::sync::Notify::new(), } } @@ -288,9 +366,24 @@ impl Engine { self.view.lock().unwrap_or_else(|p| p.into_inner()).clone() } + fn state(&self) -> State { + self.state.lock().unwrap_or_else(|p| p.into_inner()).clone() + } + + fn save(&self, state: &State) -> Result<(), String> { + self.host.save_state(state)?; + *self.state.lock().unwrap_or_else(|p| p.into_inner()) = state.clone(); + Ok(()) + } + fn set(&self, status: Status, message: Option) { let view = { let mut view = self.view.lock().unwrap_or_else(|p| p.into_inner()); + // Repeated identical states (e.g. staying signed out) are not + // re-announced. + if view.status == status && view.message == message && status != Status::UpToDate { + return; + } view.status = status; view.message = message; if status == Status::UpToDate { @@ -309,12 +402,14 @@ impl Engine { self.wake.notify_one(); } - /// Forgets the merge base, e.g. after the account is deleted. - pub fn forget(&self) { - let mut state = self.host.load_state(); + /// Forgets the merge base, e.g. after the account is deleted. Waits for + /// any running sync so it cannot write the base back. + pub async fn forget(&self) { + let _running = self.running.lock().await; + let mut state = self.state(); state.base = None; state.last_synced_at = None; - let _ = self.host.save_state(&state); + let _ = self.save(&state); self.set(Status::Off, None); } @@ -322,10 +417,7 @@ impl Engine { pub async fn sync(&self, choice: Option) -> SyncView { let _running = self.running.lock().await; match self.host.authorize().await { - Ok(None) => { - self.set(Status::Off, None); - return self.view(); - } + Ok(None) => self.set(Status::Off, None), Ok(Some(auth)) => { self.set(Status::Syncing, None); match self.sync_as(&auth, choice).await { @@ -343,10 +435,10 @@ impl Engine { } async fn sync_as(&self, auth: &Authorized, choice: Option) -> Result { - let mut state = self.host.load_state(); + let mut state = self.state(); if state.install_id.is_empty() { state.install_id = uuid::Uuid::new_v4().to_string(); - self.host.save_state(&state)?; + self.save(&state)?; } for _ in 0..MAX_ATTEMPTS { // A base from another account is never merged with this one. @@ -379,8 +471,8 @@ impl Engine { Step::InSync => true, Step::NeedsChoice => return Ok(Outcome::NeedsChoice), Step::Adopt => { - let (row, _) = remote.expect("adopt needs a row"); - self.record(&mut state, auth, row.revision, local)?; + let (row, cloud) = remote.expect("adopt needs a row"); + self.record(&mut state, auth, row.revision, cloud.clone(), local)?; true } Step::Create(document) => { @@ -390,7 +482,13 @@ impl Engine { .await? { Some(row) => { - self.record(&mut state, auth, row.revision, document)?; + self.record( + &mut state, + auth, + row.revision, + document.clone(), + document, + )?; true } None => false, @@ -399,20 +497,33 @@ impl Engine { Step::Upload { revision, document } => { self.upload(&mut state, auth, revision, document).await? } - Step::Apply { - document, - upload_over, - } => { + Step::Apply { document, upload } => { + let (row, cloud) = remote.expect("apply needs a row"); let applied = if document == local { - local + Some(local.clone()) } else { - self.host.apply(document).await? + self.host.apply(local.clone(), document).await? }; - match upload_over { - Some(revision) => self.upload(&mut state, auth, revision, applied).await?, - None => { - let (row, _) = remote.expect("apply needs a row"); - self.record(&mut state, auth, row.revision, applied)?; + match applied { + // The user changed something meanwhile: start over. + None => false, + Some(applied) if upload && applied != *cloud => { + // Recorded before uploading with the account's copy + // on both sides: if the upload fails, sections taken + // from the account do not look like local changes, + // while this install's own changes still do and are + // uploaded on the next sync. + self.record( + &mut state, + auth, + row.revision, + cloud.clone(), + cloud.clone(), + )?; + self.upload(&mut state, auth, row.revision, applied).await? + } + Some(applied) => { + self.record(&mut state, auth, row.revision, cloud.clone(), applied)?; true } } @@ -421,7 +532,8 @@ impl Engine { if synced { return Ok(Outcome::Synced); } - // Another computer wrote first: re-read and merge again. + // Another computer wrote first, or the user changed something: + // re-read and merge again. } Err("Settings changed on another computer at the same time. They will sync shortly.".into()) } @@ -439,7 +551,7 @@ impl Engine { .await? { Some(row) => { - self.record(state, auth, row.revision, document)?; + self.record(state, auth, row.revision, document.clone(), document)?; Ok(true) } None => Ok(false), @@ -451,28 +563,30 @@ impl Engine { state: &mut State, auth: &Authorized, revision: i64, - document: Document, + cloud: Document, + local: Document, ) -> Result<(), String> { state.base = Some(Base { user_id: auth.user_id.clone(), revision, - document, + cloud, + local, }); state.last_synced_at = Some(now_ms()); - self.host.save_state(state) + self.save(state) } - /// Whether this install has changed since the last sync with `user_id`. + /// Whether this install has changed since its last sync. async fn changed_since_sync(&self) -> bool { - let Some(base) = self.host.load_state().base else { + let Some(base) = self.state().base else { return false; }; - matches!(self.host.local().await, Ok(local) if local != base.document) + matches!(self.host.local().await, Ok(local) if local != base.local) } /// Background loop: syncs at start, after `wake`, when local settings - /// have changed and settled, and periodically for other computers' - /// changes. Failures back off. + /// have changed and settled, when a failed sync is due a retry, and + /// periodically for other computers' changes. Failures back off. pub async fn run(self: Arc) { let mut next_remote_check = Instant::now(); let mut retry_at: Option = None; @@ -485,16 +599,25 @@ impl Engine { }; let now = Instant::now(); let status = self.view().status; - let waiting_for_user = matches!(status, Status::NeedsChoice | Status::UpdateRequired); - let local = self.host.local().await.ok(); + // Signed out, or waiting for the user: only a wake or the + // periodic check (which notices a sign-in) runs a sync. + let idle = matches!( + status, + Status::Off | Status::NeedsChoice | Status::UpdateRequired + ); + let local = if idle { + None + } else { + self.host.local().await.ok() + }; // Upload once local changes have settled for one poll. - let settled_change = local.is_some() - && local == previous - && !waiting_for_user - && self.changed_since_sync().await; + let settled_change = + local.is_some() && local == previous && self.changed_since_sync().await; previous = local; + let retry_due = status == Status::Error && retry_at.is_some_and(|at| now >= at); let due = woken || now >= next_remote_check + || retry_due || (settled_change && retry_at.is_none_or(|at| now >= at)); if !due { continue; @@ -555,7 +678,7 @@ impl HttpRemote { .header("Prefer", "return=representation") } - async fn rows(request: reqwest::RequestBuilder) -> Result<(u16, Vec), String> { + async fn rows(request: reqwest::RequestBuilder) -> Result<(u16, Vec, Value), String> { let unreachable = |_| "Could not reach Switchify to sync settings.".to_string(); let response = request.send().await.map_err(unreachable)?; let status = response.status().as_u16(); @@ -574,7 +697,7 @@ impl HttpRemote { .collect() }) .unwrap_or_default(); - Ok((status, rows)) + Ok((status, rows, body)) } } @@ -591,7 +714,7 @@ impl Remote for HttpRemote { fn fetch<'a>(&'a self, auth: &'a Authorized) -> BoxFuture<'a, Result, String>> { Box::pin(async move { let query = format!("{COLUMNS}&user_id=eq.{}", auth.user_id); - let (status, mut rows) = + let (status, mut rows, _) = Self::rows(self.request(reqwest::Method::GET, &query, auth)).await?; match status { 200 => Ok(rows.pop()), @@ -616,9 +739,13 @@ impl Remote for HttpRemote { let request = self .request(reqwest::Method::POST, COLUMNS, auth) .json(&body); - let (status, mut rows) = Self::rows(request).await?; + let (status, mut rows, body) = Self::rows(request).await?; match status { 200 | 201 => rows.pop().map(Some).ok_or_else(|| http_error(status)), + // 23503: the account no longer exists (deleted elsewhere). + 409 if body["code"] == "23503" => { + Err("This account no longer exists. Sign in again.".into()) + } 409 => Ok(None), _ => Err(http_error(status)), } @@ -645,7 +772,7 @@ impl Remote for HttpRemote { let request = self .request(reqwest::Method::PATCH, &query, auth) .json(&body); - let (status, mut rows) = Self::rows(request).await?; + let (status, mut rows, _) = Self::rows(request).await?; match status { 200 => Ok(rows.pop()), _ => Err(http_error(status)), @@ -694,25 +821,39 @@ impl Host for AppHost { Box::pin(async move { Ok(Local::read(&self.app).document()) }) } - fn apply(&self, document: Document) -> BoxFuture<'_, Result> { + fn apply( + &self, + expected: Document, + document: Document, + ) -> BoxFuture<'_, Result, String>> { Box::pin(async move { let (tx, rx) = tokio::sync::oneshot::channel(); let handle = self.app.clone(); self.app .run_on_main_thread(move || { - let _ = tx.send(portable_settings::apply(&handle, document)); + // Checked on the main thread, where settings commands run, + // so no edit can slip in between the check and the apply. + let result = if Local::read(&handle).document() == expected { + portable_settings::apply(&handle, document).map(Some) + } else { + Ok(None) + }; + let _ = tx.send(result); }) .map_err(|error| error.to_string())?; - let applied = rx + let Some(applied) = rx .await - .map_err(|_| "Applying synced settings was cancelled.".to_string())??; + .map_err(|_| "Applying synced settings was cancelled.".to_string())?? + else { + return Ok(None); + }; if applied.profiles { let _ = self.app.emit(PROFILES_EVENT, ()); } if applied.remote { let _ = self.app.emit(REMOTE_SWITCHES_EVENT, ()); } - Ok(Local::read(&self.app).document()) + Ok(Some(Local::read(&self.app).document())) }) } @@ -812,6 +953,7 @@ mod tests { /// Simulates another computer writing just before our next write. interleave: Mutex>, fail: Mutex, + fail_update: Mutex, } impl FakeRemote { @@ -893,6 +1035,9 @@ mod tests { updated_by: &'a str, ) -> BoxFuture<'a, Result, String>> { Box::pin(async move { + if *self.fail_update.lock().unwrap() { + return Err("offline".into()); + } self.interleave(); self.writes.lock().unwrap().push(( "update", @@ -913,14 +1058,20 @@ mod tests { } } + type Normalize = fn(Document) -> Document; + struct FakeHost { user: Mutex>, local: Mutex, state: Mutex, applied: Mutex>, published: Mutex>, - /// Mimics apply normalizing what it stores (e.g. profile versions). - transform: fn(Document) -> Document, + /// Mimics the next apply normalizing what it stores (e.g. profile + /// versions). Used once. + transform: Mutex>, + /// Edits the user makes while a sync is in flight, landing just + /// before the next apply. + edits: Mutex>, } impl FakeHost { @@ -931,7 +1082,8 @@ mod tests { state: Mutex::default(), applied: Mutex::default(), published: Mutex::default(), - transform: |document| document, + transform: Mutex::default(), + edits: Mutex::default(), } } fn local_now(&self) -> Document { @@ -952,12 +1104,23 @@ mod tests { fn local(&self) -> BoxFuture<'_, Result> { Box::pin(async move { Ok(self.local_now()) }) } - fn apply(&self, document: Document) -> BoxFuture<'_, Result> { + fn apply( + &self, + expected: Document, + document: Document, + ) -> BoxFuture<'_, Result, String>> { Box::pin(async move { + if let Some(edit) = self.edits.lock().unwrap().pop_front() { + self.set_local(edit); + } + if self.local_now() != expected { + return Ok(None); + } self.applied.lock().unwrap().push(document.clone()); - let stored = (self.transform)(document); + let transform = self.transform.lock().unwrap().take(); + let stored = transform.map_or(document.clone(), |t| t(document)); self.set_local(stored.clone()); - Ok(stored) + Ok(Some(stored)) }) } fn untouched(&self) -> Document { @@ -1122,12 +1285,169 @@ mod tests { assert_eq!(remote.revision(), Some(3)); } + fn base_of(document: &Document) -> Base { + Base { + user_id: "user-1".into(), + revision: 1, + cloud: document.clone(), + local: document.clone(), + } + } + #[test] fn the_account_wins_a_section_changed_on_both_sides() { let base = untouched(); let local = with_speed(base.clone(), 4); let cloud = with_speed(base.clone(), 1); - assert_eq!(merge(&base, &local, &cloud).point_scan.speed, 1); + assert_eq!(merge(&base_of(&base), &local, &cloud).point_scan.speed, 1); + } + + fn profile(id: &str, name: &str) -> SwitchProfile { + SwitchProfile { + id: id.into(), + version: 1, + name: name.into(), + provider: "mapped".into(), + built_in: false, + bindings: (1..=8) + .map(|switch_id| crate::state::SwitchBinding { + switch_id, + binding_type: "none".into(), + value: None, + keys: None, + click_count: None, + }) + .collect(), + } + } + const A: &str = "6f1c1a52-6a0e-4c2b-9d0f-2f8a7b9e4c11"; + const B: &str = "0b7e2d4a-1c3f-4e5a-8b9c-7d6e5f4a3b22"; + const C: &str = "1a2b3c4d-5e6f-4a7b-8c9d-0e1f2a3b4c5d"; + + fn with_profiles(mut document: Document, profiles: Vec) -> Document { + document.profiles = profiles; + document + } + + fn names(document: &Document) -> Vec { + document.profiles.iter().map(|p| p.name.clone()).collect() + } + + #[test] + fn profiles_added_on_two_computers_are_both_kept() { + let base = with_profiles(untouched(), vec![profile(A, "Desk")]); + let local = with_profiles(untouched(), vec![profile(A, "Desk"), profile(B, "Laptop")]); + let cloud = with_profiles(untouched(), vec![profile(A, "Desk"), profile(C, "Tablet")]); + let merged = merge(&base_of(&base), &local, &cloud); + assert_eq!(names(&merged), ["Desk", "Tablet", "Laptop"]); + } + + #[test] + fn profile_deletions_merge_with_edits() { + let base = with_profiles(untouched(), vec![profile(A, "Desk"), profile(B, "Laptop")]); + // The account deleted both. This computer renamed A and left B alone. + let local = with_profiles( + untouched(), + vec![profile(A, "Desk 2"), profile(B, "Laptop")], + ); + let cloud = with_profiles(untouched(), vec![]); + let merged = merge(&base_of(&base), &local, &cloud); + assert_eq!(names(&merged), ["Desk 2"], "an edit survives a deletion"); + // Deleted here and untouched in the account: stays deleted. + let local = with_profiles(untouched(), vec![profile(B, "Laptop")]); + let merged = merge(&base_of(&base), &local, &base); + assert_eq!(names(&merged), ["Laptop"]); + } + + #[test] + fn merged_profile_names_stay_unique() { + let base = untouched(); + let local = with_profiles(untouched(), vec![profile(B, "Desk")]); + let cloud = with_profiles(untouched(), vec![profile(A, "desk")]); + let merged = merge(&base_of(&base), &local, &cloud); + assert_eq!(names(&merged), ["desk", "Desk (2)"]); + } + + #[tokio::test] + async fn normalization_on_apply_never_hides_a_later_local_edit() { + // The review case: an apply stores a normalized value, another + // computer then changes a different section, and the user edits the + // normalized section here. The edit must survive. + let mut cloud = with_dwell(untouched(), true); + let remote = Arc::new(FakeRemote::holding(&cloud, 2)); + let host = Arc::new(FakeHost::new(untouched())); + *host.transform.lock().unwrap() = Some(|document| with_speed(document, 3)); + let engine = engine(&remote, &host); + engine.sync(None).await; + assert_eq!(host.local_now().point_scan.speed, 3); + // Another computer changes only the cursor. + cloud.app.cursor_crosshairs = true; + *remote.row.lock().unwrap() = Some(Row { + revision: 3, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(&cloud).unwrap(), + }); + // The user changes scan speed here. + host.set_local(with_speed(host.local_now(), 4)); + assert_eq!(engine.sync(None).await.status, Status::UpToDate); + let local = host.local_now(); + assert_eq!(local.point_scan.speed, 4, "local edit kept"); + assert!( + local.app.cursor_crosshairs, + "other computer's change applied" + ); + assert_eq!(remote.document().unwrap().point_scan.speed, 4); + } + + #[tokio::test] + async fn an_edit_made_during_a_sync_is_not_reverted() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + *remote.row.lock().unwrap() = Some(Row { + revision: 2, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(with_dwell(untouched(), true)).unwrap(), + }); + // While the account is being read, the user changes scan speed. + host.edits + .lock() + .unwrap() + .push_back(with_speed(untouched(), 4)); + assert_eq!(engine.sync(None).await.status, Status::UpToDate); + let merged = with_speed(with_dwell(untouched(), true), 4); + assert_eq!(host.local_now(), merged); + assert_eq!(remote.document(), Some(merged)); + } + + #[tokio::test] + async fn a_failed_upload_after_apply_keeps_local_changes_pending() { + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + *remote.row.lock().unwrap() = Some(Row { + revision: 2, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(with_dwell(untouched(), true)).unwrap(), + }); + host.set_local(with_speed(untouched(), 4)); + *remote.fail_update.lock().unwrap() = true; + assert_eq!(engine.sync(None).await.status, Status::Error); + assert_eq!( + host.local_now(), + with_speed(with_dwell(untouched(), true), 4) + ); + assert!( + engine.changed_since_sync().await, + "speed change still to upload" + ); + *remote.fail_update.lock().unwrap() = false; + assert_eq!(engine.sync(None).await.status, Status::UpToDate); + let uploaded = remote.document().unwrap(); + assert_eq!(uploaded.point_scan.speed, 4); + assert!(uploaded.app.dwell_click_enabled); } #[tokio::test] @@ -1224,14 +1544,13 @@ mod tests { async fn normalized_apply_becomes_the_base_so_nothing_ping_pongs() { let cloud = with_dwell(untouched(), true); let remote = Arc::new(FakeRemote::holding(&cloud, 2)); - let mut host = FakeHost::new(untouched()); + let host = Arc::new(FakeHost::new(untouched())); // Applying stores something slightly different (like a profile - // version bump); that stored copy must become the base. - host.transform = |document| with_speed(document, 3); - let host = Arc::new(host); + // version bump); that stored copy must become the local base. + *host.transform.lock().unwrap() = Some(|document| with_speed(document, 3)); let engine = engine(&remote, &host); engine.sync(None).await; - assert_eq!(host.base().unwrap().document, host.local_now()); + assert_eq!(host.base().unwrap().local, host.local_now()); assert!(!engine.changed_since_sync().await); engine.sync(None).await; assert!(remote.writes.lock().unwrap().is_empty()); @@ -1253,7 +1572,7 @@ mod tests { let host = Arc::new(FakeHost::new(untouched())); let engine = engine(&remote, &host); engine.sync(None).await; - engine.forget(); + engine.forget().await; assert!(host.base().is_none()); assert_eq!(engine.view().status, Status::Off); assert!( diff --git a/src/scanning/useRemoteSwitches.ts b/src/scanning/useRemoteSwitches.ts index 23003d71..1a56cbbc 100644 --- a/src/scanning/useRemoteSwitches.ts +++ b/src/scanning/useRemoteSwitches.ts @@ -50,6 +50,8 @@ export function useRemoteSwitches() { if (!alive || m.pending || m.revision !== m.saved) return; m.config = value; setConfig(value); + }).catch((e) => { + if (alive) setError(String(e)); }); }); return () => { diff --git a/src/settings/AccountSection.test.tsx b/src/settings/AccountSection.test.tsx index ad11645b..689fb958 100644 --- a/src/settings/AccountSection.test.tsx +++ b/src/settings/AccountSection.test.tsx @@ -38,6 +38,15 @@ describe("AccountSection", () => { await waitFor(() => expect(screen.getByRole("button", { name: "Sync now" })).toHaveFocus()); }); + it("moves focus to the choice when Sync now finds different settings", async () => { + start(signedIn); + vi.spyOn(api, "syncSettingsNow").mockResolvedValue({ status: "needsChoice", lastSyncedAt: null, message: null }); + const now = await screen.findByRole("button", { name: "Sync now" }); + now.focus(); + fireEvent.click(now); + await waitFor(() => expect(screen.getByRole("button", { name: "Use my account's settings" })).toHaveFocus()); + }); + it("can take the account settings instead", async () => { start(signedIn, { status: "needsChoice", lastSyncedAt: null, message: null }); const resolve = vi.spyOn(api, "resolveSettingsSync").mockResolvedValue({ status: "upToDate", lastSyncedAt: Date.now(), message: null }); diff --git a/src/settings/AccountSection.tsx b/src/settings/AccountSection.tsx index 813e9a3e..a34a1a0c 100644 --- a/src/settings/AccountSection.tsx +++ b/src/settings/AccountSection.tsx @@ -188,7 +188,9 @@ function SyncPanel() { const [busy, setBusy] = useState(false); const [error, setError] = useState(null); const syncNowRef = useRef(null); + const firstChoiceRef = useRef(null); const restoreFocus = useRef(false); + const previousStatus = useRef(sync?.status); useEffect(() => { let active = true; @@ -204,6 +206,17 @@ function SyncPanel() { } }); + // Entering or leaving the choice swaps the buttons; if the focused one + // went with them, focus the buttons that replaced it. + useEffect(() => { + const previous = previousStatus.current; + previousStatus.current = sync?.status; + if (!previous || previous === sync?.status) return; + const active = document.activeElement; + if (active && active !== document.body) return; + (sync?.status === "needsChoice" ? firstChoiceRef : syncNowRef).current?.focus(); + }, [sync?.status]); + const run = async (action: () => Promise, fromChoice = false) => { if (busy) return; setBusy(true); @@ -227,7 +240,7 @@ function SyncPanel() {

{sync.status === "needsChoice" ?
- +
:
From 188c603d97fcdc78ca427e027486e5e6532bd980 Mon Sep 17 00:00:00 2001 From: Owen McGirr Date: Mon, 5 Oct 2026 14:49:08 +0100 Subject: [PATCH 3/5] Compare synced profiles by content and cap merged profiles Address re-review on the settings sync engine: - Profile changes are judged on name, provider and bindings, not version, since versions drift between computers; a merged profile keeps the higher version. A version-only difference no longer beats a real edit or brings back a deleted profile. - A merge that would exceed 32 custom profiles stops with a clear message instead of failing every retry with a generic error. - The applied result is read on the main thread right after applying, so a settings change made just afterwards is not mistaken for synced. Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/settings_sync.rs | 106 +++++++++++++++++++++++++++++---- 1 file changed, 96 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/settings_sync.rs b/src-tauri/src/settings_sync.rs index f53a7714..51b70039 100644 --- a/src-tauri/src/settings_sync.rs +++ b/src-tauri/src/settings_sync.rs @@ -31,6 +31,9 @@ const MAX_BACKOFF: Duration = Duration::from_secs(300); const MAX_ATTEMPTS: usize = 3; /// Well under the server's 256 KiB limit. const MAX_PAYLOAD_BYTES: usize = 200 * 1024; +/// The custom profile limit enforced when settings are applied. +const MAX_SYNCED_PROFILES: usize = 32; +const TOO_MANY_PROFILES: &str = "Your computers have more than 32 switch profiles between them. Delete some on this computer, then sync again."; const UPDATE_REQUIRED: &str = "Your synced settings were saved by a newer Switchify PC. Update Switchify PC on this computer to keep syncing."; @@ -291,20 +294,34 @@ fn merge_profiles( .filter(|p| !cloud.iter().any(|c| c.id == p.id)) .map(|p| p.id.clone()), ); + // Versions drift between computers (applying keeps the local version + // when content matches), so only content counts as a change. + let content = |p: &Option| { + p.as_ref() + .map(|p| (p.name.clone(), p.provider.clone(), p.bindings.clone())) + }; let mut merged: Vec = ids .iter() .filter_map(|id| { let (bl, bc) = (find(base_local, id), find(base_cloud, id)); let (l, c) = (find(local, id), find(cloud, id)); - let local_changed = l != bl; - let cloud_changed = c != bc; - match (local_changed, cloud_changed) { - (false, _) => c, - (true, false) => l, + let local_changed = content(&l) != content(&bl); + let cloud_changed = content(&c) != content(&bc); + let mut chosen = match (local_changed, cloud_changed) { + (false, _) => c.clone(), + (true, false) => l.clone(), // Both changed: the account wins, but an edit is never lost // to a deletion on the other side. - (true, true) => c.or(l), - } + (true, true) => c.clone().or(l.clone()), + }?; + // Never move a version backwards, so connected phones notice. + chosen.version = [&l, &c] + .into_iter() + .flatten() + .map(|p| p.version) + .max() + .unwrap_or(chosen.version); + Some(chosen) }) .collect(); let mut seen = std::collections::HashSet::new(); @@ -499,6 +516,9 @@ impl Engine { } Step::Apply { document, upload } => { let (row, cloud) = remote.expect("apply needs a row"); + if document.profiles.len() > MAX_SYNCED_PROFILES { + return Err(TOO_MANY_PROFILES.into()); + } let applied = if document == local { Some(local.clone()) } else { @@ -833,15 +853,18 @@ impl Host for AppHost { .run_on_main_thread(move || { // Checked on the main thread, where settings commands run, // so no edit can slip in between the check and the apply. + // The result is read here too, before any later command + // can change settings and have its edit taken as synced. let result = if Local::read(&handle).document() == expected { - portable_settings::apply(&handle, document).map(Some) + portable_settings::apply(&handle, document) + .map(|applied| Some((applied, Local::read(&handle).document()))) } else { Ok(None) }; let _ = tx.send(result); }) .map_err(|error| error.to_string())?; - let Some(applied) = rx + let Some((applied, result)) = rx .await .map_err(|_| "Applying synced settings was cancelled.".to_string())?? else { @@ -853,7 +876,7 @@ impl Host for AppHost { if applied.remote { let _ = self.app.emit(REMOTE_SWITCHES_EVENT, ()); } - Ok(Some(Local::read(&self.app).document())) + Ok(Some(result)) }) } @@ -1368,6 +1391,69 @@ mod tests { assert_eq!(names(&merged), ["desk", "Desk (2)"]); } + #[test] + fn a_version_only_difference_is_not_an_edit() { + // The review case: the account's copy differs from the base only in + // version; the local copy was renamed. The rename must win. + let mut base_p = profile(A, "Desk"); + base_p.version = 3; + let mut local_p = profile(A, "Desk renamed"); + local_p.version = 4; + let mut cloud_p = profile(A, "Desk"); + cloud_p.version = 2; + let base = with_profiles(untouched(), vec![base_p]); + let merged = merge( + &base_of(&base), + &with_profiles(untouched(), vec![local_p]), + &with_profiles(untouched(), vec![cloud_p]), + ); + assert_eq!(names(&merged), ["Desk renamed"]); + assert_eq!(merged.profiles[0].version, 4, "versions never go backwards"); + } + + #[test] + fn a_version_only_difference_does_not_resurrect_a_deletion() { + let mut base_p = profile(A, "Desk"); + base_p.version = 3; + let mut local_p = base_p.clone(); + local_p.version = 5; + let base = with_profiles(untouched(), vec![base_p]); + let merged = merge( + &base_of(&base), + &with_profiles(untouched(), vec![local_p]), + &with_profiles(untouched(), vec![]), + ); + assert!(merged.profiles.is_empty()); + } + + #[tokio::test] + async fn too_many_profiles_after_a_merge_is_a_clear_error() { + let make = |prefix: u32| -> Vec { + (0..20) + .map(|n| { + profile( + &format!("{prefix:08x}-0000-4000-8000-{n:012x}"), + &format!("P{prefix}-{n}"), + ) + }) + .collect() + }; + let remote = Arc::new(FakeRemote::default()); + let host = Arc::new(FakeHost::new(untouched())); + let engine = engine(&remote, &host); + engine.sync(None).await; + *remote.row.lock().unwrap() = Some(Row { + revision: 2, + schema_version: SCHEMA_VERSION, + payload: serde_json::to_value(with_profiles(untouched(), make(1))).unwrap(), + }); + host.set_local(with_profiles(untouched(), make(2))); + let view = engine.sync(None).await; + assert_eq!(view.status, Status::Error); + assert_eq!(view.message.as_deref(), Some(TOO_MANY_PROFILES)); + assert!(host.applied.lock().unwrap().is_empty(), "nothing applied"); + } + #[tokio::test] async fn normalization_on_apply_never_hides_a_later_local_edit() { // The review case: an apply stores a normalized value, another From 79f76989f6c8157b7652d0965261ce7719e9d639 Mon Sep 17 00:00:00 2001 From: Owen McGirr Date: Mon, 5 Oct 2026 15:07:45 +0100 Subject: [PATCH 4/5] Converge synced profile versions so computers settle Address re-review: applying a profile with unchanged content kept the lower local version, so after lost same-profile races two computers could upload their own versions to each other forever. Same-content profiles now keep the higher version, so both converge. The reserved-name rename no longer bumps the version during a merge (only once at load, for connected phones). The sync tests' fake host now stores profiles with the real merge rules, and a two-computer test replays the review sequence and checks it settles. Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/portable_settings.rs | 10 ++++-- src-tauri/src/settings_sync.rs | 55 ++++++++++++++++++++++++++++++ src-tauri/src/state.rs | 15 +++++--- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src-tauri/src/portable_settings.rs b/src-tauri/src/portable_settings.rs index 4c32880f..ad190d6b 100644 --- a/src-tauri/src/portable_settings.rs +++ b/src-tauri/src/portable_settings.rs @@ -267,8 +267,9 @@ pub fn plan(local: &Local, incoming: Document) -> Result { /// Replaces the custom profiles with the incoming set, keeping built-ins. /// A changed profile gets a version above both copies so connected phones -/// notice the new bindings. Returns `None` when nothing would change. -fn merge_profiles( +/// notice the new bindings; an unchanged one keeps the higher of the two. +/// Returns `None` when nothing would change. +pub(crate) fn merge_profiles( local: &[SwitchProfile], mut incoming: Vec, ) -> Result>, String> { @@ -298,8 +299,11 @@ fn merge_profiles( .iter() .find(|candidate| candidate.id == profile.id && !candidate.built_in) { + // Same content: never lower the version, so computers that synced + // the same profile converge on one version instead of trading + // their own back and forth. profile.version = if same_profile(existing, &profile) { - existing.version + existing.version.max(profile.version) } else { profile.version.max(existing.version.saturating_add(1)) }; diff --git a/src-tauri/src/settings_sync.rs b/src-tauri/src/settings_sync.rs index 51b70039..579f9c86 100644 --- a/src-tauri/src/settings_sync.rs +++ b/src-tauri/src/settings_sync.rs @@ -1140,6 +1140,17 @@ mod tests { return Ok(None); } self.applied.lock().unwrap().push(document.clone()); + // Profiles are stored the way the app stores them (version + // rules included), which is what can make computers drift. + let mut document = document; + if let Some(profiles) = + portable_settings::merge_profiles(&expected.profiles, document.profiles.clone()) + .unwrap() + { + document.profiles = profiles; + } else { + document.profiles = expected.profiles.clone(); + } let transform = self.transform.lock().unwrap().take(); let stored = transform.map_or(document.clone(), |t| t(document)); self.set_local(stored.clone()); @@ -1426,6 +1437,50 @@ mod tests { assert!(merged.profiles.is_empty()); } + #[tokio::test] + async fn two_computers_settle_after_losing_profile_races() { + // The review sequence: each computer loses one same-profile race, + // which bumps its version for that profile. Afterwards any change + // must settle instead of the two uploading to each other forever. + let start = with_profiles(untouched(), vec![profile(A, "P"), profile(B, "Q")]); + let remote = Arc::new(FakeRemote::default()); + let host_a = Arc::new(FakeHost::new(start.clone())); + let host_b = Arc::new(FakeHost::new(start.clone())); + let (a, b) = (engine(&remote, &host_a), engine(&remote, &host_b)); + a.sync(None).await; + b.sync(None).await; + let rename = |host: &FakeHost, index: usize, name: &str| { + let mut document = host.local_now(); + document.profiles[index].name = name.into(); + document.profiles[index].version += 1; + host.set_local(document); + }; + // Both edit P; A uploads first and B loses. + rename(&host_a, 0, "P from A"); + rename(&host_b, 0, "P from B"); + a.sync(None).await; + b.sync(None).await; + a.sync(None).await; + // Both edit Q; B uploads first and A loses. + rename(&host_b, 1, "Q from B"); + rename(&host_a, 1, "Q from A"); + b.sync(None).await; + a.sync(None).await; + b.sync(None).await; + // One unrelated change, then many periodic checks on both. + host_a.set_local(with_dwell(host_a.local_now(), true)); + let writes_before = remote.writes.lock().unwrap().len(); + for _ in 0..10 { + a.sync(None).await; + b.sync(None).await; + } + let writes = remote.writes.lock().unwrap().len() - writes_before; + assert!(writes <= 2, "settled after {writes} uploads, not a loop"); + assert_eq!(host_a.local_now(), host_b.local_now()); + assert_eq!(remote.document(), Some(host_a.local_now())); + assert_eq!(names(&host_a.local_now()), ["P from A", "Q from B"]); + } + #[tokio::test] async fn too_many_profiles_after_a_merge_is_a_clear_error() { let make = |prefix: u32| -> Vec { diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 3f463be9..e0352766 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -266,7 +266,9 @@ fn is_reserved_profile_name(name: &str) -> bool { /// custom "Grid 3" made on a Mac) to the first free " (custom)", /// " (custom 2)", ... The result depends only on the list, so every /// install renames the same profiles the same way. -pub fn rename_reserved_profiles(profiles: &mut [SwitchProfile]) { +/// Returns the indexes renamed. +pub fn rename_reserved_profiles(profiles: &mut [SwitchProfile]) -> Vec { + let mut renamed = Vec::new(); for index in 0..profiles.len() { if profiles[index].built_in || !is_reserved_profile_name(&profiles[index].name) { continue; @@ -284,10 +286,10 @@ pub fn rename_reserved_profiles(profiles: &mut [SwitchProfile]) { .any(|profile| profile.name.trim().eq_ignore_ascii_case(candidate)) }) .expect("an unused suffix always exists"); - let profile = &mut profiles[index]; - profile.name = name; - profile.version = profile.version.saturating_add(1); + profiles[index].name = name; + renamed.push(index); } + renamed } pub fn built_in_profiles(include_grid3: bool) -> Vec { @@ -411,7 +413,10 @@ impl AppModel { .into_iter() .filter(|profile| !profile.built_in), ); - rename_reserved_profiles(&mut profiles); + // A renamed profile is a changed profile for connected phones. + for index in rename_reserved_profiles(&mut profiles) { + profiles[index].version = profiles[index].version.saturating_add(1); + } let mut engine = ProtocolEngine::new(desktop_id.clone()); let saved_pairing_count = saved.paired_devices.len(); let saved_paired_devices = saved.paired_devices; From 3ae535aa2beb415fd202d6926bdbd4bb04052384 Mon Sep 17 00:00:00 2001 From: Owen McGirr Date: Mon, 5 Oct 2026 15:12:51 +0100 Subject: [PATCH 5/5] Correct the profile version comment in sync merging Co-Authored-By: Claude Opus 5.5 (1M context) --- src-tauri/src/settings_sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/settings_sync.rs b/src-tauri/src/settings_sync.rs index 579f9c86..068254f5 100644 --- a/src-tauri/src/settings_sync.rs +++ b/src-tauri/src/settings_sync.rs @@ -294,8 +294,8 @@ fn merge_profiles( .filter(|p| !cloud.iter().any(|c| c.id == p.id)) .map(|p| p.id.clone()), ); - // Versions drift between computers (applying keeps the local version - // when content matches), so only content counts as a change. + // Versions can differ between computers (e.g. after a lost race bumps one + // copy), so only content counts as a change. let content = |p: &Option| { p.as_ref() .map(|p| (p.name.clone(), p.provider.clone(), p.bindings.clone()))