From 467dcb1f873a488937b1685b6377217ed628fa93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 15 May 2026 19:58:42 -0400 Subject: [PATCH] Reflect real sync state on the Sync settings page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SyncConnectionState` was a one-variant enum (`SignedOut`), so the Sync page rendered "Local-only · sign-in coming soon" even after the bearer token landed on disk and the upload thread completed. The state machine now mirrors the actual lifecycle. What lands: - `SyncConnectionState` gains `SignedIn`, `AwaitingDeviceApproval`, `SyncReady { last_synced_at_secs }`, `SyncError { message }`. `SyncObjectState::Synced` joins the per-object enum so individual rows can advertise "Synced" once a successful upload lands. - `BrowserCore` stores the current `SyncConnectionState` and exposes `set_sync_connection_state`. `sync_status` now propagates the live state into the snapshot the UI reads. - `ElyShell::probe_initial_sync_state` inspects `/sync/bearer.token` synchronously at construction so the first render of the sync page is honest about whether the user is signed in. - A `std::sync::mpsc` channel ferries upload outcomes from the off-thread worker back to the shell; the existing 8 ms tick drains it and stamps `core.set_sync_connection_state` with the freshest result. The UI now shows "Signed in · awaiting first sync", "Synced · last upload Xm ago", "Sync error · …", and the worker-special "Signed in · waiting for device approval" when the server returns `device_not_approved`. --- .../ely_app/src/shell/internal_pages/sync.rs | 42 +++++++- crates/ely_app/src/shell/mod.rs | 85 ++++++++++++++- crates/ely_app/src/shell/settings_actions.rs | 69 ++++++++---- .../src/shell/web_surface_controller.rs | 3 +- crates/ely_browser_core/src/state.rs | 2 + crates/ely_browser_core/src/state/sync.rs | 102 +++++++++--------- crates/ely_domain/src/sync.rs | 36 +++++-- 7 files changed, 260 insertions(+), 79 deletions(-) diff --git a/crates/ely_app/src/shell/internal_pages/sync.rs b/crates/ely_app/src/shell/internal_pages/sync.rs index 4d99096..4efda3d 100644 --- a/crates/ely_app/src/shell/internal_pages/sync.rs +++ b/crates/ely_app/src/shell/internal_pages/sync.rs @@ -266,6 +266,7 @@ fn render_state_dot(state: SyncObjectState) -> AnyElement { SyncObjectState::LocalOnly => colors::ink_4(), SyncObjectState::Paused => colors::ink_5(), SyncObjectState::PrivacyControlled => colors::accent(), + SyncObjectState::Synced => colors::success(), }; div().size(px(8.0)).rounded_full().bg(rgb(color)).into_any_element() @@ -304,12 +305,48 @@ fn render_policy_toggle( .into_any_element() } -fn connection_label(connection: &SyncConnectionState) -> &'static str { +fn connection_label(connection: &SyncConnectionState) -> String { match connection { - SyncConnectionState::SignedOut => "Local-only · sign-in coming soon", + SyncConnectionState::SignedOut => "Local-only · drop a session token to enable".to_string(), + SyncConnectionState::SignedIn => "Signed in · awaiting first sync".to_string(), + SyncConnectionState::AwaitingDeviceApproval => { + "Signed in · waiting for device approval".to_string() + } + SyncConnectionState::SyncReady { last_synced_at_secs } => { + format!("Synced · last upload {}", relative_time_since(*last_synced_at_secs)) + } + SyncConnectionState::SyncError { message } => { + format!("Sync error · {}", short_message(message)) + } } } +fn relative_time_since(secs: u64) -> String { + use std::time::{Duration, SystemTime, UNIX_EPOCH}; + let when = UNIX_EPOCH + Duration::from_secs(secs); + let elapsed = SystemTime::now().duration_since(when).unwrap_or_default(); + let total_secs = elapsed.as_secs(); + if total_secs < 60 { + return format!("{total_secs}s ago"); + } + if total_secs < 3600 { + return format!("{}m ago", total_secs / 60); + } + if total_secs < 86400 { + return format!("{}h ago", total_secs / 3600); + } + format!("{}d ago", total_secs / 86400) +} + +fn short_message(message: &str) -> String { + const MAX_LEN: usize = 72; + if message.len() <= MAX_LEN { + return message.to_string(); + } + let truncated: String = message.chars().take(MAX_LEN - 1).collect(); + format!("{truncated}…") +} + fn sync_object_kind_label(kind: SyncObjectKind) -> &'static str { match kind { SyncObjectKind::Spaces => "Spaces", @@ -329,6 +366,7 @@ fn sync_object_state_label(state: SyncObjectState) -> &'static str { SyncObjectState::LocalOnly => "Local only", SyncObjectState::Paused => "Paused", SyncObjectState::PrivacyControlled => "Privacy controlled", + SyncObjectState::Synced => "Synced", } } diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 1c3876a..61b28b9 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -99,10 +99,29 @@ pub struct ElyShell { pending_plugin_install: Option, pending_plugin_uninstall: Option, web_surfaces: WebSurfaceStore, + /// Receives sync upload outcomes from the off-thread worker so the + /// shell can refresh the connection state without blocking. Sender + /// is cloned for each spawned upload; receiver is drained on every + /// `tick_external_web_surfaces`. + sync_inbox_rx: std::sync::mpsc::Receiver, + pub(crate) sync_inbox_tx: std::sync::mpsc::Sender, _command_subscription: Subscription, _translucency_subscription: Subscription, } +/// Messages the off-thread sync worker pushes back to the shell so the +/// `SyncConnectionState` on `BrowserCore` reflects the live engine +/// without the UI thread ever touching the network. `SignedIn` is the +/// initial-probe state set synchronously on shell startup and does not +/// flow through this channel. +#[derive(Clone, Debug)] +pub(crate) enum SyncStateUpdate { + SignedOut, + AwaitingDeviceApproval, + SyncReady { last_synced_at_secs: u64 }, + SyncError { message: String }, +} + impl ElyShell { pub fn new(window: &mut Window, cx: &mut Context) -> Self { Self::new_with_config(InitialBrowserConfig::ely_defaults(), window, cx) @@ -182,7 +201,8 @@ impl ElyShell { Err(error) => ShellState::StartupError(error.to_string()), }; - let shell = Self { + let (sync_inbox_tx, sync_inbox_rx) = std::sync::mpsc::channel(); + let mut shell = Self { state, focus_handle: cx.focus_handle(), command_input, @@ -215,13 +235,76 @@ impl ElyShell { pending_plugin_install: None, pending_plugin_uninstall: None, web_surfaces: WebSurfaceStore::new(), + sync_inbox_rx, + sync_inbox_tx, _command_subscription: command_subscription, _translucency_subscription: translucency_subscription, }; + shell.probe_initial_sync_state(); start_external_web_surface_timer(cx); shell } + /// Inspect the on-disk bearer token (if any) and seed the + /// `SyncConnectionState` so the Sync settings page reads the right + /// label on first render — without any sync setting page open it + /// would otherwise stay `SignedOut` until the user clicks Sync now. + fn probe_initial_sync_state(&mut self) { + let ShellState::Ready(core) = &mut self.state else { + return; + }; + let Some(snapshot) = core.snapshot().ok() else { + return; + }; + let active_profile_id = snapshot.active_profile_id.clone(); + let Some(profile_root) = crate::services::servo_profile_data::default_profile_data_root() + else { + return; + }; + let profile_dir = crate::services::servo_profile_data::profile_data_dir( + &profile_root, + &active_profile_id, + ); + let bearer_path = profile_dir.join("sync").join("bearer.token"); + let bearer_present = std::fs::metadata(&bearer_path).map(|m| m.len() > 0).unwrap_or(false); + let state = if bearer_present { + ely_domain::SyncConnectionState::SignedIn + } else { + ely_domain::SyncConnectionState::SignedOut + }; + core.set_sync_connection_state(state); + } + + /// Drain any sync upload outcomes the off-thread worker pushed + /// since the previous tick and stamp the resulting connection + /// state on `BrowserCore`. Returns `true` when at least one + /// update was applied so callers can `cx.notify()` accordingly. + pub(super) fn drain_sync_updates(&mut self) -> bool { + let ShellState::Ready(core) = &mut self.state else { + return false; + }; + let mut latest: Option = None; + while let Ok(update) = self.sync_inbox_rx.try_recv() { + latest = Some(match update { + SyncStateUpdate::SignedOut => ely_domain::SyncConnectionState::SignedOut, + SyncStateUpdate::AwaitingDeviceApproval => { + ely_domain::SyncConnectionState::AwaitingDeviceApproval + } + SyncStateUpdate::SyncReady { last_synced_at_secs } => { + ely_domain::SyncConnectionState::SyncReady { last_synced_at_secs } + } + SyncStateUpdate::SyncError { message } => { + ely_domain::SyncConnectionState::SyncError { message } + } + }); + } + if let Some(state) = latest { + core.set_sync_connection_state(state); + return true; + } + false + } + fn focus_command_mode(&mut self, window: &mut Window, cx: &mut Context) { if let ShellState::Ready(core) = &mut self.state { core.set_command_query(">"); diff --git a/crates/ely_app/src/shell/settings_actions.rs b/crates/ely_app/src/shell/settings_actions.rs index a28fb4d..77198c4 100644 --- a/crates/ely_app/src/shell/settings_actions.rs +++ b/crates/ely_app/src/shell/settings_actions.rs @@ -243,10 +243,9 @@ impl ElyShell { /// Push the active profile's bookmarks to `ely-browser-cloud` as a /// snapshot. The HTTP round-trip runs on a dedicated worker thread - /// (the UI thread never blocks on the network), and the result is - /// emitted via `tracing` so the user can inspect it through - /// `RUST_LOG=ely::sync=info`. No bearer token on disk → the - /// engine reports `SignedOut` and the click is a no-op. + /// (the UI thread never blocks on the network), and the worker + /// reports back through the shell's `sync_inbox` so the sync page + /// reflects the new state without waiting for a manual refresh. pub(super) fn trigger_cloud_sync_upload(&mut self, _cx: &mut Context) { let ShellState::Ready(core) = &self.state else { return; @@ -272,9 +271,10 @@ impl ElyShell { return; } }; + let tx = self.sync_inbox_tx.clone(); std::thread::Builder::new() .name("ely-sync-upload".to_string()) - .spawn(move || run_sync_upload(profile_dir, device_name, bytes)) + .spawn(move || run_sync_upload(profile_dir, device_name, bytes, tx)) .map(|_| ()) .unwrap_or_else(|error| { tracing::warn!( @@ -312,29 +312,56 @@ impl ElyShell { } } -fn run_sync_upload(profile_dir: std::path::PathBuf, device_name: String, bytes: Vec) { +fn run_sync_upload( + profile_dir: std::path::PathBuf, + device_name: String, + bytes: Vec, + inbox: std::sync::mpsc::Sender, +) { let mut engine = match SyncEngine::for_profile_dir(&profile_dir, device_name, sync_platform()) { Ok(engine) => engine, Err(error) => { - tracing::warn!( - target: "ely::sync", - error = %error, - "could not initialise sync engine", - ); + let message = error.to_string(); + tracing::warn!(target: "ely::sync", error = %message, "could not initialise sync engine"); + let _ = inbox.send(super::SyncStateUpdate::SyncError { message }); return; } }; match engine.upload_bytes(bytes) { - Ok(outcome) => tracing::info!( - target: "ely::sync", - outcome = ?outcome, - "snapshot upload complete", - ), - Err(error) => tracing::warn!( - target: "ely::sync", - error = %error, - "snapshot upload failed", - ), + Ok(ely_browser_core::SyncOutcome::SignedOut) => { + tracing::info!(target: "ely::sync", "no bearer token on disk; sync skipped"); + let _ = inbox.send(super::SyncStateUpdate::SignedOut); + } + Ok(ely_browser_core::SyncOutcome::Uploaded { + snapshot_id, + logical_clock, + payload_bytes, + device_id, + }) => { + tracing::info!( + target: "ely::sync", + snapshot_id = %snapshot_id, + logical_clock, + payload_bytes, + device_id = %device_id, + "snapshot upload complete", + ); + let last_synced_at_secs = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let _ = inbox.send(super::SyncStateUpdate::SyncReady { last_synced_at_secs }); + } + Err(error) => { + let message = error.to_string(); + tracing::warn!(target: "ely::sync", error = %message, "snapshot upload failed"); + let update = if message.contains("device_not_approved") { + super::SyncStateUpdate::AwaitingDeviceApproval + } else { + super::SyncStateUpdate::SyncError { message } + }; + let _ = inbox.send(update); + } } } diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 1c7c297..b8b67c2 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -65,7 +65,8 @@ impl ElyShell { for metadata in result.page_metadata { metadata_changed |= self.apply_web_surface_page_metadata(metadata); } - result.changed || url_changed || metadata_changed + let sync_changed = self.drain_sync_updates(); + result.changed || url_changed || metadata_changed || sync_changed } pub(super) fn record_external_web_viewport( diff --git a/crates/ely_browser_core/src/state.rs b/crates/ely_browser_core/src/state.rs index 604bb78..770346a 100644 --- a/crates/ely_browser_core/src/state.rs +++ b/crates/ely_browser_core/src/state.rs @@ -162,6 +162,7 @@ pub struct BrowserCore { update_policy: UpdatePolicy, appearance: AppearanceSettings, sync_object_policies: SyncObjectPolicies, + sync_connection_state: ely_domain::SyncConnectionState, command_query: String, } @@ -210,6 +211,7 @@ impl BrowserCore { update_policy: UpdatePolicy::default(), appearance: AppearanceSettings::default(), sync_object_policies: SyncObjectPolicies::default(), + sync_connection_state: ely_domain::SyncConnectionState::SignedOut, spaces: vec![space], profiles: vec![profile], tabs: vec![tab], diff --git a/crates/ely_browser_core/src/state/sync.rs b/crates/ely_browser_core/src/state/sync.rs index 65519eb..95c3c25 100644 --- a/crates/ely_browser_core/src/state/sync.rs +++ b/crates/ely_browser_core/src/state/sync.rs @@ -1,4 +1,7 @@ -use ely_domain::{SyncObjectKind, SyncObjectPolicy, SyncObjectState, SyncObjectStatus, SyncStatus}; +use ely_domain::{ + SyncConnectionState, SyncObjectKind, SyncObjectPolicy, SyncObjectState, SyncObjectStatus, + SyncStatus, +}; use super::BrowserCore; @@ -75,54 +78,57 @@ impl BrowserCore { self.sync_object_policies.get(kind) } + pub fn set_sync_connection_state(&mut self, state: SyncConnectionState) { + self.sync_connection_state = state; + } + pub(super) fn sync_status(&self) -> SyncStatus { - SyncStatus::signed_out(vec![ - self.sync_object_status( - SyncObjectKind::Spaces, - self.spaces.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::Tabs, - self.sync_enabled_tab_count(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::Bookmarks, - self.bookmarks.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::Notes, - self.notes.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::ReadingList, - self.reading_list.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::Profiles, - self.profiles.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::SitePermissions, - self.site_permissions.len(), - SyncObjectState::LocalOnly, - ), - self.sync_object_status( - SyncObjectKind::History, - self.history_entries.len(), - SyncObjectState::PrivacyControlled, - ), - self.sync_object_status( - SyncObjectKind::PluginSettings, - self.installed_plugins.len(), - SyncObjectState::LocalOnly, - ), - ]) + let enabled_state = match &self.sync_connection_state { + SyncConnectionState::SyncReady { .. } => SyncObjectState::Synced, + _ => SyncObjectState::LocalOnly, + }; + SyncStatus::new( + self.sync_connection_state.clone(), + vec![ + self.sync_object_status(SyncObjectKind::Spaces, self.spaces.len(), enabled_state), + self.sync_object_status( + SyncObjectKind::Tabs, + self.sync_enabled_tab_count(), + enabled_state, + ), + self.sync_object_status( + SyncObjectKind::Bookmarks, + self.bookmarks.len(), + enabled_state, + ), + self.sync_object_status(SyncObjectKind::Notes, self.notes.len(), enabled_state), + self.sync_object_status( + SyncObjectKind::ReadingList, + self.reading_list.len(), + enabled_state, + ), + self.sync_object_status( + SyncObjectKind::Profiles, + self.profiles.len(), + enabled_state, + ), + self.sync_object_status( + SyncObjectKind::SitePermissions, + self.site_permissions.len(), + enabled_state, + ), + self.sync_object_status( + SyncObjectKind::History, + self.history_entries.len(), + SyncObjectState::PrivacyControlled, + ), + self.sync_object_status( + SyncObjectKind::PluginSettings, + self.installed_plugins.len(), + enabled_state, + ), + ], + ) } fn sync_object_status( diff --git a/crates/ely_domain/src/sync.rs b/crates/ely_domain/src/sync.rs index af61043..f632479 100644 --- a/crates/ely_domain/src/sync.rs +++ b/crates/ely_domain/src/sync.rs @@ -1,6 +1,22 @@ +/// Connection lifecycle of the cloud sync client. +/// +/// The previous variant set was a single `SignedOut`, which made the +/// Sync settings page report "Local-only · sign-in coming soon" even +/// after the user dropped a bearer token in. The state machine now +/// progresses from `SignedOut` → `SignedIn` (token present, sync not +/// yet attempted) → `SyncReady` (last upload landed) or `SyncError` +/// (last upload failed). `AwaitingDeviceApproval` is the dedicated +/// state for the 403 the worker returns when the device-id bound to +/// the session has not yet been approved by another already-approved +/// device — surfacing it as its own variant lets the UI explain the +/// gap instead of bucketing it into a generic error. #[derive(Clone, Debug, Eq, PartialEq)] pub enum SyncConnectionState { SignedOut, + SignedIn, + AwaitingDeviceApproval, + SyncReady { last_synced_at_secs: u64 }, + SyncError { message: String }, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -28,6 +44,7 @@ pub enum SyncObjectState { LocalOnly, Paused, PrivacyControlled, + Synced, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -84,14 +101,21 @@ impl SyncObjectStatus { } impl SyncStatus { + #[must_use] + pub fn new(connection: SyncConnectionState, objects: Vec) -> Self { + let failed_objects = match &connection { + SyncConnectionState::SyncError { .. } => 1, + _ => 0, + }; + Self { connection, pending_objects: 0, failed_objects, objects } + } + + /// Convenience constructor preserved for tests + call sites that + /// have not been migrated to [`SyncStatus::new`] yet. Same result + /// as `SyncStatus::new(SyncConnectionState::SignedOut, objects)`. #[must_use] pub fn signed_out(objects: Vec) -> Self { - Self { - connection: SyncConnectionState::SignedOut, - pending_objects: 0, - failed_objects: 0, - objects, - } + Self::new(SyncConnectionState::SignedOut, objects) } #[must_use]