fix(sync): recover from a locked keyring without restarting the app

src/core/scheduler.rs

@@ -34,13 +34,18 @@ pub const DEBOUNCE_MS: u64 = 2000;
 /// Cooldown after a sync finishes before the next one may start (s).
 pub const COOLDOWN_SECONDS: u64 = 4;
 
-/// Issue #170: budget of back-to-back keyring retries before the folder parks
-/// in the keyring-locked state. A transiently-unavailable Secret Service (bus
-/// coming up after login) recovers within these, but a genuinely locked
-/// collection must not retry forever.
+/// Issue #170: budget of back-to-back keyring retries before the folder falls
+/// back to the slow keyring watch (issue #214). A transiently-unavailable
+/// Secret Service (bus coming up after login) recovers within these, but a
+/// genuinely locked collection must not retry at a fast pace forever.
 pub const KEYRING_RETRY_MAX: u32 = 5;
 /// Starting retry delay for the keyring backoff (ms); each attempt doubles it.
 pub const KEYRING_RETRY_BASE_MS: u64 = 2000;
+/// Interval (ms) between keyring re-checks once the fast retry budget is
+/// exhausted (issue #214). Mirrors the server probe (issue #179): uncapped,
+/// so the folder recovers without a restart whenever the keyring is unlocked
+/// later, by any means.
+pub const KEYRING_WATCH_INTERVAL_MS: u64 = 60_000;
 /// Interval (ms) between server health probes while a folder is parked as
 /// server-unreachable (issue #179). Kept short so recovery is noticed quickly
 /// without hammering the network.
@@ -665,26 +670,53 @@ impl SchedulerInner {
     }
 
     /// Schedule a keyring retry with a capped exponential backoff (issue
-    /// #170). Once the retry budget is exhausted the folder stays in the
-    /// keyring-locked state and waits for a manual action instead of retrying
-    /// forever. The retry re-enters `start` on a timer WITHOUT changing the
+    /// #170). The retry re-enters `start` on a timer WITHOUT changing the
     /// visible state, so the informative keyring-locked label is preserved
-    /// while it re-attempts.
+    /// while it re-attempts. Once the fast budget is exhausted the folder
+    /// falls back to a slow periodic watch (issue #214) instead of parking
+    /// until a restart: a locked keyring is often unlocked later in the
+    /// session (by the user, the session itself, or the engine's own unlock
+    /// attempt), and the folder must notice on its own.
     fn schedule_keyring_retry(&mut self) {
+        if self.stopped || self.start_source.is_some() || self.preparing || self.running {
+            return;
+        }
+        if self.keyring_retry_count >= KEYRING_RETRY_MAX {
+            self.schedule_keyring_watch();
+            return;
+        }
+        let attempt = self.keyring_retry_count;
+        self.keyring_retry_count += 1;
+        let delay = Duration::from_millis(KEYRING_RETRY_BASE_MS << attempt.min(4));
+        let weak = self.self_ref.clone();
+        let id = self.source.borrow_mut().add_timeout(
+            delay,
+            Box::new(move || {
+                if let Some(inner) = weak.upgrade() {
+                    inner.borrow_mut().start();
+                }
+            }),
+        );
+        self.start_source = Some(id);
+    }
+
+    /// Slow periodic re-check while the keyring stays locked (issue #214),
+    /// mirroring the server probe (issue #179): each tick re-enters `start`
+    /// WITHOUT changing the visible state; a run that still finds the keyring
+    /// locked re-arms the watch from `finished`, and the first run that
+    /// resolves credentials clears the gate.
+    fn schedule_keyring_watch(&mut self) {
         if self.stopped
             || self.start_source.is_some()
             || self.preparing
             || self.running
-            || self.keyring_retry_count >= KEYRING_RETRY_MAX
+            || !self.keyring_locked
         {
             return;
         }
-        let attempt = self.keyring_retry_count;
-        self.keyring_retry_count += 1;
-        let delay = Duration::from_millis(KEYRING_RETRY_BASE_MS << attempt.min(4));
         let weak = self.self_ref.clone();
         let id = self.source.borrow_mut().add_timeout(
-            delay,
+            Duration::from_millis(KEYRING_WATCH_INTERVAL_MS),
             Box::new(move || {
                 if let Some(inner) = weak.upgrade() {
                     inner.borrow_mut().start();
@@ -1727,28 +1759,62 @@ mod tests {
         assert_eq!(scheduler.state().snapshot().state, AppState::IdleOk);
     }
 
-    /// Issue #170: the keyring retry has a capped budget, so a genuinely
-    /// locked collection does not retry forever.
+    /// Issue #170: the fast keyring retry has a capped budget, so a genuinely
+    /// locked collection does not retry back-to-back forever. Issue #214:
+    /// once the budget is exhausted the folder falls back to a slow periodic
+    /// watch instead of parking until a restart.
     #[test]
-    fn keyring_retry_budget_is_capped() {
+    fn keyring_retry_budget_is_capped_and_falls_back_to_a_watch() {
         let (scheduler, source, runner) = make_scheduler(None);
         scheduler.request(Trigger::Startup);
         run_idle(&source);
-        // Burn the whole retry budget with back-to-back locked outcomes.
-        let mut calls = 1;
-        loop {
+        // Burn the whole fast retry budget with back-to-back locked outcomes.
+        for _ in 0..KEYRING_RETRY_MAX {
             finish(&runner, SyncOutcome::KeyringLocked);
-            if source.borrow().pending() == 0 {
-                break;
-            }
             let retry_id = source.borrow().only_id();
             fire_timer(&source, retry_id);
-            calls += 1;
         }
-        assert_eq!(calls, 1 + KEYRING_RETRY_MAX as usize);
-        // No more retries are scheduled; the folder is parked keyring-locked.
-        assert_eq!(source.borrow().pending(), 0);
+        assert_eq!(
+            runner.0.borrow().start_calls,
+            1 + KEYRING_RETRY_MAX as usize
+        );
+        // The next locked outcome exhausts the budget: no further fast retry,
+        // but a slow watch keeps re-checking the keyring.
+        finish(&runner, SyncOutcome::KeyringLocked);
+        assert!(scheduler.keyring_locked());
+        assert_eq!(
+            source.borrow().pending(),
+            1,
+            "a keyring watch must keep re-checking after the retry budget"
+        );
+    }
+
+    /// Issue #214: after the fast retry budget is exhausted, the slow keyring
+    /// watch keeps re-entering the engine on its own, so the folder recovers
+    /// without restarting the app as soon as the keyring is unlocked - by the
+    /// user, the session, or the app's own unlock attempt.
+    #[test]
+    fn keyring_watch_recovers_without_restart_once_unlocked() {
+        let (scheduler, source, runner) = make_scheduler(None);
+        scheduler.request(Trigger::Startup);
+        run_idle(&source);
+        for _ in 0..KEYRING_RETRY_MAX {
+            finish(&runner, SyncOutcome::KeyringLocked);
+            let retry_id = source.borrow().only_id();
+            fire_timer(&source, retry_id);
+        }
+        // The budget is exhausted: the folder sits keyring-locked with only
+        // the slow watch pending, no external trigger in sight.
+        finish(&runner, SyncOutcome::KeyringLocked);
+        let calls = runner.0.borrow().start_calls;
         assert!(scheduler.keyring_locked());
+        // The keyring gets unlocked; the next watch tick reconciles.
+        let watch_id = source.borrow().only_id();
+        fire_timer(&source, watch_id);
+        assert_eq!(runner.0.borrow().start_calls, calls + 1);
+        finish(&runner, SyncOutcome::Success);
+        assert!(!scheduler.keyring_locked());
+        assert_eq!(scheduler.state().snapshot().state, AppState::IdleOk);
     }
 
     /// Issue #179: a NetworkError outcome arms the server-unreachable gate:

src/nextcloud/credentials.rs

@@ -184,6 +184,27 @@ impl CredentialsStore {
         Ok(Some(password))
     }
 
+    /// Try to unlock every locked collection (issue #214).
+    ///
+    /// GNOME collections created by the desktop session usually share the
+    /// login password: the Secret Service then completes the unlock prompt
+    /// on its own as long as the client connection stays alive through it,
+    /// so the common case returns without any user interaction. Returns
+    /// `Ok(true)` when at least one collection was locked and got unlocked.
+    /// Per-collection failures are skipped so one stubborn collection does
+    /// not block the rest.
+    pub fn unlock_locked_collections() -> Result<bool, CredentialError> {
+        let service = SecretService::connect(EncryptionType::Dh)?;
+        let mut unlocked_any = false;
+        for collection in service.get_all_collections()? {
+            let was_locked = collection.is_locked().unwrap_or(false);
+            if was_locked && collection.unlock().is_ok() {
+                unlocked_any = true;
+            }
+        }
+        Ok(unlocked_any)
+    }
+
     /// Drop the cached password for an account (issue #178).
     ///
     /// Called when a sync run proves the credential wrong (authentication

src/nextcloud/sync_engine.rs

@@ -80,6 +80,18 @@ impl CredentialLookup {
 pub trait CredentialSource: Send + Sync + 'static {
     /// Resolve the password for the given account.
     fn lookup(&self, account: &AccountConfig) -> CredentialLookup;
+
+    /// Issue #214: best-effort unlock of the keyring collections. Returns
+    /// `true` when at least one locked collection was unlocked, so the
+    /// caller re-runs [`CredentialSource::lookup`] once. GNOME collections
+    /// created by the desktop session usually share the login password: the
+    /// Secret Service then completes the unlock prompt on its own as long as
+    /// the client connection stays alive through it, so the common case
+    /// needs no user interaction. Sources without an unlock facility keep
+    /// the default: no unlock, no retry.
+    fn unlock_keyring(&self) -> bool {
+        false
+    }
 }
 
 /// [`CredentialSource`] backed by the desktop Secret Service.
@@ -100,6 +112,10 @@ impl CredentialSource for KeyringCredentialSource {
             Err(_) => CredentialLookup::Unavailable,
         }
     }
+
+    fn unlock_keyring(&self) -> bool {
+        CredentialsStore::unlock_locked_collections().unwrap_or(false)
+    }
 }
 
 /// Outcome of a finished `nextcloudcmd` run, mirroring `SyncResult`.
@@ -458,7 +474,16 @@ fn engine_thread(
     process: Arc<Mutex<Option<Child>>>,
 ) -> EngineRun {
     let started = Instant::now();
-    let password = match credentials.lookup(&inputs.account) {
+    let mut lookup = credentials.lookup(&inputs.account);
+    // Issue #214: a locked collection may share the session password, in
+    // which case the Secret Service unlocks it without prompting while the
+    // client connection stays alive through the prompt. When the unlock
+    // succeeds, resolve the password once more instead of reporting the
+    // keyring as locked.
+    if matches!(lookup, CredentialLookup::Locked) && credentials.unlock_keyring() {
+        lookup = credentials.lookup(&inputs.account);
+    }
+    let password = match lookup {
         CredentialLookup::Found(password) => password,
         // Transient secret-service trouble (bus not ready at startup, locked
         // collection, agent hiccup) must not arm the credential gate
@@ -729,6 +754,44 @@ mod tests {
         }
     }
 
+    /// Issue #214: a source whose lookup is locked once but whose keyring can
+    /// be unlocked on demand (the collections share the session password, so
+    /// the Secret Service unlocks them without prompting).
+    struct UnlockingCredentials {
+        locked: std::sync::atomic::AtomicBool,
+        lookups: std::sync::atomic::AtomicUsize,
+    }
+
+    impl CredentialSource for UnlockingCredentials {
+        fn lookup(&self, _account: &AccountConfig) -> CredentialLookup {
+            self.lookups
+                .fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+            if self.locked.swap(false, std::sync::atomic::Ordering::SeqCst) {
+                CredentialLookup::Locked
+            } else {
+                CredentialLookup::Found("secret".to_string())
+            }
+        }
+
+        fn unlock_keyring(&self) -> bool {
+            true
+        }
+    }
+
+    /// Issue #214: a source that stays locked and cannot unlock anything must
+    /// be asked exactly once per run (no unlock retry loop).
+    struct LockedCredentials {
+        lookups: std::sync::atomic::AtomicUsize,
+    }
+
+    impl CredentialSource for LockedCredentials {
+        fn lookup(&self, _account: &AccountConfig) -> CredentialLookup {
+            self.lookups
+                .fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+            CredentialLookup::Locked
+        }
+    }
+
     fn account() -> AccountConfig {
         AccountConfig {
             id: "test-account".to_string(),
@@ -953,6 +1016,65 @@ mod tests {
         assert!(events.is_empty());
     }
 
+    /// Issue #214: when the unlock attempt succeeds, the lookup is retried
+    /// once and the run proceeds with the resolved password (here surfacing
+    /// as EngineMissing, which is only reachable after a successful lookup).
+    #[test]
+    fn locked_lookup_retries_once_after_a_successful_unlock() {
+        let (progress_tx, progress_rx) = async_channel::unbounded();
+        let credentials = Arc::new(UnlockingCredentials {
+            locked: std::sync::atomic::AtomicBool::new(true),
+            lookups: std::sync::atomic::AtomicUsize::new(0),
+        });
+        let engine = SyncEngine::new(
+            account(),
+            folder(),
+            NetworkConfig::default(),
+            None,
+            Some("/nonexistent/nextcloudcmd".into()),
+            progress_tx,
+        )
+        .with_credentials(credentials.clone());
+        let (outcome, _events) = run_engine(engine, &progress_rx);
+        assert_eq!(outcome, SyncOutcome::EngineMissing);
+        assert_eq!(
+            credentials
+                .lookups
+                .load(std::sync::atomic::Ordering::SeqCst),
+            2,
+            "the lookup must be retried once after a successful unlock"
+        );
+    }
+
+    /// Issue #214: a lookup that stays locked and an unlock that cannot help
+    /// must not loop: one lookup per run, keyring-locked outcome.
+    #[test]
+    fn locked_lookup_without_unlock_is_not_retried() {
+        let (progress_tx, progress_rx) = async_channel::unbounded();
+        let credentials = Arc::new(LockedCredentials {
+            lookups: std::sync::atomic::AtomicUsize::new(0),
+        });
+        let engine = SyncEngine::new(
+            account(),
+            folder(),
+            NetworkConfig::default(),
+            None,
+            None,
+            progress_tx,
+        )
+        .with_credentials(credentials.clone());
+        let (outcome, events) = run_engine(engine, &progress_rx);
+        assert_eq!(outcome, SyncOutcome::KeyringLocked);
+        assert!(events.is_empty());
+        assert_eq!(
+            credentials
+                .lookups
+                .load(std::sync::atomic::Ordering::SeqCst),
+            1,
+            "a failed unlock must not re-run the lookup"
+        );
+    }
+
     #[test]
     fn unreachable_secret_service_maps_to_keyring_locked() {
         // Issue #85: at startup the session bus may not be ready yet; that