fix: coherent waiting message and self-healing keyring retry

src/core/scheduler.rs

@@ -34,6 +34,14 @@ 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.
+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;
+
 /// How a finished reconciliation turned out.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum SyncOutcome {
@@ -118,6 +126,11 @@ struct SchedulerInner {
     inotify_during_sync: bool,
     feedback_followup_pending: bool,
     keyring_locked: bool,
+    /// Issue #170: how many times the keyring retry has run back-to-back.
+    /// A transient locked service recovers on its own, but a genuinely
+    /// locked collection must not retry forever: once the budget is spent the
+    /// folder parks in the keyring-locked state until a manual action.
+    keyring_retry_count: u32,
     delete_alert: Option<DeleteAlert>,
     delete_bypass_once: bool,
     /// Whether a synchronization has ever completed successfully. Drives the
@@ -186,6 +199,7 @@ impl Scheduler {
             inotify_during_sync: false,
             feedback_followup_pending: false,
             keyring_locked: false,
+            keyring_retry_count: 0,
             delete_alert: None,
             delete_bypass_once: false,
             ever_synced: false,
@@ -515,6 +529,36 @@ impl SchedulerInner {
         self.start_source = Some(id);
     }
 
+    /// 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
+    /// visible state, so the informative keyring-locked label is preserved
+    /// while it re-attempts.
+    fn schedule_keyring_retry(&mut self) {
+        if self.stopped
+            || self.start_source.is_some()
+            || self.preparing
+            || self.running
+            || self.keyring_retry_count >= KEYRING_RETRY_MAX
+        {
+            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);
+    }
+
     fn start(&mut self) {
         self.start_source = None;
         if self.stopped || self.preparing || self.running || self.queue.is_empty() || !self.online {
@@ -611,13 +655,20 @@ impl SchedulerInner {
             };
             if !acquired {
                 self.queue.extend(reasons.iter().copied());
-                // Issue #165: name the folder the queue is waiting on (when it
-                // is a tracked holder), instead of the generic "another
-                // account" wording. Falls back to the generic message when no
-                // tracked folder is available.
-                let message = match permit.active_folder_name() {
-                    Some(name) => t("Waiting for {folder} to finish…").replace("{folder}", &name),
-                    None => t("Waiting for another folder to finish…").to_string(),
+                // Issue #165/#169: name the folder the queue is waiting on only
+                // when this scheduler is the next in line (no waiters ahead of
+                // it). Behind it the message stays generic, so a run of many
+                // queued folders does not repeat the same folder name on every
+                // row and does not name one that may already be done.
+                let message = if permit.waiter_count() == 0 {
+                    match permit.active_folder_name() {
+                        Some(name) => {
+                            t("Waiting for {folder} to finish…").replace("{folder}", &name)
+                        }
+                        None => t("Waiting for another folder to finish…").to_string(),
+                    }
+                } else {
+                    t("Waiting for another folder to finish…").to_string()
                 };
                 self.state.set(AppState::SyncQueued, message);
                 let source = self.source.clone();
@@ -660,13 +711,15 @@ impl SchedulerInner {
         let (ran, conflicted) = match outcome {
             SyncOutcome::Success => {
                 self.keyring_locked = false;
+                self.keyring_retry_count = 0;
                 self.auth_required = false;
                 self.ever_synced = true;
                 self.set_idle_state();
                 (true, false)
             }
             SyncOutcome::Conflict => {
                 self.keyring_locked = false;
+                self.keyring_retry_count = 0;
                 self.auth_required = false;
                 self.ever_synced = true;
                 self.state.set(
@@ -778,6 +831,17 @@ impl SchedulerInner {
             if let Some(permit) = &self.permit {
                 permit.release();
             }
+            // Issue #170: a keyring-locked outcome is often transient — the
+            // Secret Service may not be up right after login. Rather than
+            // waiting for the next external trigger (which may never come
+            // within the session), schedule a capped backoff retry so the next
+            // run re-reads the credentials and clears the flag once the
+            // service is ready. The retry does not touch the visible
+            // keyring-locked state.
+            if matches!(outcome, SyncOutcome::KeyringLocked) {
+                self.queue.add(Trigger::Retry);
+                self.schedule_keyring_retry();
+            }
         }
     }
 
@@ -1278,18 +1342,70 @@ mod tests {
         assert!(scheduler.keyring_locked());
         assert_eq!(scheduler.state().snapshot().state, AppState::KeyringLocked);
 
-        // Issue #85: an automatic trigger retries the run instead of parking
-        // the folder in needs-attention; the keyring may have come up in the
-        // meantime and the lookup is cheap.
+        // An automatic trigger queues up (the keyring retry is already
+        // pending from the locked outcome), so it does not start a second,
+        // parallel run; the existing retry still re-enters the engine and
+        // recovers once the service is ready.
         scheduler.request(Trigger::LocalInotify);
-        assert_eq!(scheduler.queue_len(), 1);
+        // The keyring retry (queued from the locked outcome) plus this trigger.
+        assert_eq!(scheduler.queue_len(), 2);
+        let retry_id = source.borrow().only_id();
+        fire_timer(&source, retry_id);
+        assert_eq!(runner.0.borrow().start_calls, 2);
+        finish(&runner, SyncOutcome::Success);
+        assert!(!scheduler.keyring_locked());
+        assert_eq!(scheduler.state().snapshot().state, AppState::IdleOk);
+    }
+
+    /// Issue #170: a keyring-locked outcome schedules its own backoff retry,
+    /// so the folder recovers as soon as the Secret Service is ready even if
+    /// no external trigger arrives later in the session.
+    #[test]
+    fn keyring_locked_schedules_its_own_retry() {
+        let (scheduler, source, runner) = make_scheduler(None);
+        scheduler.request(Trigger::Startup);
         run_idle(&source);
+        assert_eq!(runner.0.borrow().start_calls, 1);
+        finish(&runner, SyncOutcome::KeyringLocked);
+        assert!(scheduler.keyring_locked());
+        assert_eq!(scheduler.state().snapshot().state, AppState::KeyringLocked);
+
+        // A backoff retry is already pending without any new external trigger:
+        // firing it re-enters the engine (the visible state stays keyring
+        // locked while it re-attempts).
+        assert_eq!(source.borrow().pending(), 1);
+        let retry_id = source.borrow().only_id();
+        fire_timer(&source, retry_id);
         assert_eq!(runner.0.borrow().start_calls, 2);
         finish(&runner, SyncOutcome::Success);
         assert!(!scheduler.keyring_locked());
         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.
+    #[test]
+    fn keyring_retry_budget_is_capped() {
+        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 {
+            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!(scheduler.keyring_locked());
+    }
+
     #[test]
     fn shared_permit_queues_a_second_account_until_release() {
         let permit = SyncPermit::try_new(1).unwrap();
@@ -1312,6 +1428,45 @@ mod tests {
         assert_eq!(runner_second.0.borrow().start_calls, 1);
     }
 
+    /// Issue #169: only the head of the wait queue names the folder it is
+    /// waiting on; the schedulers behind it use the generic wording instead
+    /// of repeating the same folder name / naming one that may already finish.
+    #[test]
+    fn queued_schedulers_only_name_the_holder_at_the_head() {
+        let permit = SyncPermit::try_new(1).unwrap();
+        let root = tempfile::tempdir().expect("tempdir");
+        let (first, source_first, runner_first) = make_scheduler(Some(permit.clone()));
+        first.set_local_root(Some(std::path::PathBuf::from(root.path())));
+        let (second, source_second, _runner_second) = make_scheduler(Some(permit.clone()));
+        second.set_local_root(Some(root.path().join("a")));
+        let (third, source_third, _runner_third) = make_scheduler(Some(permit.clone()));
+        third.set_local_root(Some(root.path().join("b")));
+
+        // The holder runs; two schedulers queue behind it.
+        first.request(Trigger::Manual);
+        run_idle(&source_first);
+        assert_eq!(runner_first.0.borrow().start_calls, 1);
+        assert!(permit.active_folder_name().is_some());
+
+        second.request(Trigger::Manual);
+        run_idle(&source_second);
+        // Only one waiter so far: it is the head and names the folder.
+        let expected = format!(
+            "Waiting for {} to finish…",
+            root.path().file_name().unwrap().to_string_lossy()
+        );
+        assert_eq!(second.state().snapshot().message, expected);
+
+        third.request(Trigger::Manual);
+        run_idle(&source_third);
+        // The third is behind the second: it uses the generic wording, not the
+        // same folder name as the head.
+        assert_eq!(
+            third.state().snapshot().message,
+            "Waiting for another folder to finish…"
+        );
+    }
+
     /// Issue #154: the startup backlog runs strictly one folder at a time in
     /// the exact order the startup triggers were requested. Every scheduler
     /// defers its start through one idle hop (dispatched FIFO, like the GLib

src/core/sync_permit.rs

@@ -155,6 +155,14 @@ impl SyncPermit {
             .map(str::to_owned)
     }
 
+    /// How many schedulers are currently waiting for a release (issue #169).
+    /// A scheduler with zero waiters ahead of it is the head of the queue and
+    /// can name the folder it is waiting on; anyone behind it should use the
+    /// generic wording instead of repeating the same folder name.
+    pub fn waiter_count(&self) -> usize {
+        self.inner.borrow().waiters.len()
+    }
+
     /// Release one slot and wake the oldest waiter, if any.
     ///
     /// The woken callback runs synchronously (Python semantics); it must not