From 53157acf8659d4677e6034762e2b7f88ec46eb85 Mon Sep 17 00:00:00 2001 From: gnacho Date: Sun, 23 Aug 2026 11:26:33 +0200 Subject: [PATCH 1/3] fix(ui): explain why a folder is waiting and stop conflict loops A folder row queued behind the shared permit showed only the generic 'Waiting to synchronize' label, hiding why it was stuck (the scheduler does attach a specific reason like 'Waiting for another account to finish'). Surface that message on the row (issue #165). A run that ends with conflicted copies also re-queued the local feedback, bouncing a folder with unresolved conflicts (a large tree reconciling tens of thousands of files) into a sync loop while smaller folders waited. On a conflicted run, drop the local feedback and leave the message that the user must review the log, letting only the cadence of interval/remote triggers retry. Tests for both, gate 663 passed + 1 ignored. Closes #165 --- src/core/scheduler.rs | 61 +++++++++++++++++++++++++++++++++++------ src/ui/folder_status.rs | 61 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 10 deletions(-) diff --git a/src/core/scheduler.rs b/src/core/scheduler.rs index b91633e..40abaab 100644 --- a/src/core/scheduler.rs +++ b/src/core/scheduler.rs @@ -648,13 +648,17 @@ impl SchedulerInner { if self.stopped { return; } - let ran = match outcome { + // `conflicted` drives the post-run feedback handling: a run that ended + // with conflicted copies must not queue another reconciliation in a + // loop (issue #165); the row already carries the "review the log" + // message and the sync stays parked until the user addresses it. + let (ran, conflicted) = match outcome { SyncOutcome::Success => { self.keyring_locked = false; self.auth_required = false; self.ever_synced = true; self.set_idle_state(); - true + (true, false) } SyncOutcome::Conflict => { self.keyring_locked = false; @@ -664,7 +668,7 @@ impl SchedulerInner { AppState::IdleOk, t("Synchronized with conflicts — review the log"), ); - true + (true, true) } SyncOutcome::AuthFailed => { self.keyring_locked = false; @@ -675,7 +679,7 @@ impl SchedulerInner { AppState::AuthRequired, t("Credentials rejected. Sign in again in Account settings."), ); - false + (false, false) } SyncOutcome::NoCredentials => { self.keyring_locked = false; @@ -686,19 +690,19 @@ impl SchedulerInner { AppState::AuthRequired, t("No saved credentials. Use Sign in again in Account settings."), ); - false + (false, false) } SyncOutcome::KeyringLocked => { self.keyring_locked = true; self.state .set(AppState::KeyringLocked, t("Password keyring is locked")); - false + (false, false) } SyncOutcome::Failed => { self.keyring_locked = false; self.state .set(AppState::Error, t("Synchronization failed — view the log")); - true + (true, false) } SyncOutcome::NetworkError => { // Issue #162: the server itself is unreachable even though the @@ -711,7 +715,7 @@ impl SchedulerInner { self.keyring_locked = false; self.state .set(AppState::Offline, t("Waiting for a network connection")); - false + (false, false) } }; // Fase 3: after a successful run the guard baseline is refreshed so @@ -735,7 +739,15 @@ impl SchedulerInner { flag.set(false); } if ran { - if self.inotify_during_sync { + if conflicted { + // Issue #165: a conflicted run must not trigger a follow-up + // reconciliation from local feedback (that is what keeps a + // folder with unresolved conflicts bouncing). Drop any local + // feedback queued during the run and let the interval/remote + // triggers retry on their own cadence. + self.queue.discard(Trigger::LocalInotify); + self.feedback_followup_pending = false; + } else if self.inotify_during_sync { if feedback_followup { // Suppress only the local feedback from the reconciliation // itself; manual/remote triggers stay queued. @@ -1205,6 +1217,37 @@ mod tests { assert_eq!(scheduler.queue_len(), 1); } + /// Issue #165: a run that ends with conflicted copies must not re-queue + /// the local feedback in a loop. The queue is left empty (only the + /// interval/remote triggers will retry later) and the folder stays in the + /// 'review the log' state until the user addresses the conflict. + #[test] + fn conflicted_run_does_not_requeue_local_feedback() { + let (scheduler, source, runner) = make_scheduler(None); + scheduler.request(Trigger::Manual); + run_idle(&source); + assert_eq!(runner.0.borrow().start_calls, 1); + // A local change arrives during the run, exactly like the success case. + scheduler.request(Trigger::LocalInotify); + finish(&runner, SyncOutcome::Conflict); + assert_eq!( + scheduler.state().snapshot().state, + AppState::IdleOk, + "the folder stays IdleOk with the conflict message" + ); + assert_eq!( + scheduler.state().snapshot().message, + "Synchronized with conflicts — review the log" + ); + // Unlike the success case the local feedback is NOT re-enqueued, so the + // folder does not bounce back into a sync loop. + assert_eq!( + scheduler.queue_len(), + 0, + "no feedback re-queued on conflict" + ); + } + #[test] fn stop_removes_pending_sources_and_cancels_process() { let (scheduler, source, runner) = make_scheduler(None); diff --git a/src/ui/folder_status.rs b/src/ui/folder_status.rs index 7451cbd..6ea91b8 100644 --- a/src/ui/folder_status.rs +++ b/src/ui/folder_status.rs @@ -553,6 +553,16 @@ fn render( ) && !snapshot.message.is_empty() { parts.push(snapshot.message.clone()); + } else if snapshot.state == AppState::SyncQueued && !snapshot.message.is_empty() { + // Issue #165: surface the specific reason the run is queued (e.g. + // waiting behind the shared permit) instead of the generic + // "Waiting to synchronize" label. When the message is exactly the + // generic label the row keeps the label and does not repeat itself. + if snapshot.message != status { + parts.push(snapshot.message.clone()); + } else { + parts.push(status.to_string()); + } } else { parts.push(status.to_string()); } @@ -752,11 +762,21 @@ mod tests { assert!(!row.local_size.is_visible()); // Outside the synchronized state the synced-in-local segment // disappears: queued and syncing rows show only the status. - state.set(AppState::SyncQueued, "queued"); + state.set(AppState::SyncQueued, "Waiting to synchronize"); assert_eq!( row.row.subtitle().as_deref(), Some("Waiting to synchronize") ); + // A specific blocking reason replaces the generic label (issue + // #165). + state.set( + AppState::SyncQueued, + "Waiting for another account to finish…", + ); + assert_eq!( + row.row.subtitle().as_deref(), + Some("Waiting for another account to finish…") + ); state.set(AppState::IdleOk, "ok"); assert_eq!(row.row.subtitle().as_deref(), Some("Synced in local docs")); // Live progress (issue #86): a progress event shows the line, @@ -774,6 +794,45 @@ mod tests { }); } + #[test] + fn queued_row_surfaces_the_blocking_reason() { + crate::ui::test_helpers::gtk_smoke(|| { + set_locale(Locale::English); + let folder = FolderConfig { + id: "f1".to_string(), + local_root: "/tmp/a".to_string(), + remote_path: "/docs".to_string(), + space_id: None, + size_confirmed: false, + }; + let state = StateController::new(AppState::SyncQueued); + let row = FolderStatusRow::new( + folder, + Some(state.clone()), + FolderRowCallbacks::default(), + None, + None, + ); + // The generic wait shows the generic label when there is no + // more specific reason (no message). + assert_eq!( + row.row.subtitle().as_deref(), + Some("Waiting to synchronize") + ); + // Issue #165: with a specific blocking reason present, the row + // shows it instead of the generic label. + state.set( + AppState::SyncQueued, + "Waiting for another account to finish…", + ); + assert_eq!( + row.row.subtitle().as_deref(), + Some("Waiting for another account to finish…") + ); + reset_locale(); + }); + } + #[test] fn local_tree_size_sums_files_and_skips_symlinks() { let dir = tempfile::tempdir().unwrap(); From b1555977288159b92135a850f559101c04aad43a Mon Sep 17 00:00:00 2001 From: gnacho Date: Sun, 23 Aug 2026 11:35:20 +0200 Subject: [PATCH 2/3] fix(ui): name the folder a sync waits on and align the summary text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The queued message said 'another account' and was generic (issue #165 follow-up). Name the folder that holds the shared permit instead, so the row reads 'Waiting for to finish…' (falls back to 'another folder'). The account summary card could read 'Connected' next to a red light: the connection text only distinguished Offline while the light used a severity map. Align the text with the severity so problem states (error, credentials rejected, keyring locked, delete review) say what the light conveys. Also use the same severity mapping for the initial icon render so it does not flip between globes and status icons. w/ #165 --- po/es.po | 12 +++++-- src/core/scheduler.rs | 13 +++++--- src/core/sync_permit.rs | 14 ++++++++ src/ui/folder_status.rs | 8 ++--- src/ui/main_window.rs | 66 ++++++++++++++++++++++++++++--------- src/util/translations/es.rs | 3 +- 6 files changed, 89 insertions(+), 27 deletions(-) diff --git a/po/es.po b/po/es.po index 570be9f..1e6139b 100644 --- a/po/es.po +++ b/po/es.po @@ -2541,6 +2541,10 @@ msgstr "Proveedor de sincronización" msgid "Synchronization blocked: password keyring is locked" msgstr "Sincronización bloqueada: el almacén de contraseñas está bloqueado" +#: src/core/account_runtime.rs +msgid "Synchronization blocked: the server is unreachable" +msgstr "Sincronización bloqueada: el servidor no está disponible" + #: src/core/scheduler.rs msgid "Waiting for local changes to settle" msgstr "Esperando a que los cambios locales se asienten" @@ -2583,8 +2587,12 @@ msgid "Waiting for a network connection" msgstr "Esperando una conexión de red" #: src/core/scheduler.rs -msgid "Waiting for another account to finish…" -msgstr "Esperando a que otra cuenta termine…" +msgid "Waiting for another folder to finish…" +msgstr "Esperando a que termine otra carpeta…" + +#: src/core/scheduler.rs +msgid "Waiting for {folder} to finish…" +msgstr "Esperando a que termine {folder}…" #: src/core/scheduler.rs msgid "Will pause after the current synchronization" diff --git a/src/core/scheduler.rs b/src/core/scheduler.rs index 40abaab..118e433 100644 --- a/src/core/scheduler.rs +++ b/src/core/scheduler.rs @@ -611,10 +611,15 @@ impl SchedulerInner { }; if !acquired { self.queue.extend(reasons.iter().copied()); - self.state.set( - AppState::SyncQueued, - t("Waiting for another account to finish…"), - ); + // 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(), + }; + self.state.set(AppState::SyncQueued, message); let source = self.source.clone(); let weak = self.self_ref.clone(); permit.wait_for_release(Box::new(move || { diff --git a/src/core/sync_permit.rs b/src/core/sync_permit.rs index 80c2557..acc1168 100644 --- a/src/core/sync_permit.rs +++ b/src/core/sync_permit.rs @@ -141,6 +141,20 @@ impl SyncPermit { .any(|active| paths_overlap(active, &canonical)) } + /// The display name of the oldest tracked holder currently running, if + /// any. Returns the folder name (the last path component) so a queued + /// scheduler can say which folder it is waiting on (issue #165). `None` + /// when no holder is tracked (a plain `try_acquire` holder, or idle). + pub fn active_folder_name(&self) -> Option { + let inner = self.inner.borrow(); + inner + .roots + .first() + .and_then(|root| root.file_name()) + .and_then(|name| name.to_str()) + .map(str::to_owned) + } + /// Release one slot and wake the oldest waiter, if any. /// /// The woken callback runs synchronously (Python semantics); it must not diff --git a/src/ui/folder_status.rs b/src/ui/folder_status.rs index 6ea91b8..8e86809 100644 --- a/src/ui/folder_status.rs +++ b/src/ui/folder_status.rs @@ -771,11 +771,11 @@ mod tests { // #165). state.set( AppState::SyncQueued, - "Waiting for another account to finish…", + "Waiting for another folder to finish…", ); assert_eq!( row.row.subtitle().as_deref(), - Some("Waiting for another account to finish…") + Some("Waiting for another folder to finish…") ); state.set(AppState::IdleOk, "ok"); assert_eq!(row.row.subtitle().as_deref(), Some("Synced in local docs")); @@ -823,11 +823,11 @@ mod tests { // shows it instead of the generic label. state.set( AppState::SyncQueued, - "Waiting for another account to finish…", + "Waiting for another folder to finish…", ); assert_eq!( row.row.subtitle().as_deref(), - Some("Waiting for another account to finish…") + Some("Waiting for another folder to finish…") ); reset_locale(); }); diff --git a/src/ui/main_window.rs b/src/ui/main_window.rs index 810fa4c..2ecf602 100644 --- a/src/ui/main_window.rs +++ b/src/ui/main_window.rs @@ -194,16 +194,11 @@ impl AccountView { .orientation(gtk4::Orientation::Horizontal) .spacing(8) .build(); - let connected = !matches!( - runtime.state().snapshot().state, - crate::state::AppState::Offline | crate::state::AppState::Error - ); let light = gtk4::Image::builder().pixel_size(22).build(); - light.set_icon_name(Some(if connected { - "nextsync-state-globe" - } else { - "nextsync-state-globe-off" - })); + // Issue #165: use the same severity mapping the live subscription + // applies (summary_light_for), so the initial render and the updates + // agree instead of flipping between globes and status icons. + light.set_icon_name(Some(summary_light_for(runtime.state().snapshot().state))); line_one.append(&light); // The status label doubles as the anti-race guard for the // background quota fetch (detached rows keep their text). @@ -1906,15 +1901,26 @@ pub fn summary_light_for(state: crate::state::AppState) -> &'static str { } } -/// The connection text for the account summary card. Only `Offline` reads -/// "Not connected"; every other state implies the server is reachable (the -/// light already carries the severity, issue #129). +/// The connection text for the account summary card, mirroring the severity +/// the light already conveys (issue #129): not just "Connected", but a text +/// that matches the state so the card is not contradictory (issue #165). +/// +/// - Healthy/paused/queued/syncing states read "Connected" (or a specific +/// positive/neutral status) — the server is reachable. +/// - `Offline` reads "Not connected". +/// - Problem states (error/auth/keyring/delete review) do not read +/// "Connected": they surface a clear attention message instead of a lying +/// green-ish label next to a red light. pub fn summary_connection_text(state: crate::state::AppState) -> &'static str { use crate::state::AppState; - if state == AppState::Offline { - t("Not connected") - } else { - t("Connected") + match state { + AppState::Offline => t("Not connected"), + AppState::Error => t("Synchronization failed"), + AppState::AuthRequired => t("Credentials rejected"), + AppState::KeyringLocked => t("Password keyring is locked"), + AppState::DeleteReview => t("Review deletions"), + AppState::Unconfigured => t("Not connected"), + _ => t("Connected"), } } @@ -2239,6 +2245,34 @@ mod tests { ); } + #[test] + fn summary_connection_text_matches_the_light() { + // Issue #165: the text must not contradict the light. Problem states + // read a clear message (the red light), not a lying "Connected". + use crate::state::AppState; + set_locale(Locale::English); + assert_eq!(summary_connection_text(AppState::IdleOk), "Connected"); + assert_eq!(summary_connection_text(AppState::Syncing), "Connected"); + assert_eq!(summary_connection_text(AppState::Offline), "Not connected"); + assert_eq!( + summary_connection_text(AppState::Error), + "Synchronization failed" + ); + assert_eq!( + summary_connection_text(AppState::AuthRequired), + "Credentials rejected" + ); + assert_eq!( + summary_connection_text(AppState::KeyringLocked), + "Password keyring is locked" + ); + assert_eq!( + summary_connection_text(AppState::DeleteReview), + "Review deletions" + ); + reset_locale(); + } + #[test] fn main_window_construction_smoke() { // Must run through the shared GTK test worker: a second `gtk4::init()` diff --git a/src/util/translations/es.rs b/src/util/translations/es.rs index f4fd824..9347ff3 100644 --- a/src/util/translations/es.rs +++ b/src/util/translations/es.rs @@ -471,9 +471,10 @@ pub static CATALOG: &[(&str, &str)] = &[ ("Waiting for a network connection", "Esperando una conexión de red"), ("Waiting for an allowed Wi-Fi network", "Esperando una red Wi-Fi permitida"), ("Waiting for an unmetered network connection", "Esperando una conexión de red sin límite de datos"), - ("Waiting for another account to finish…", "Esperando a que otra cuenta termine…"), + ("Waiting for another folder to finish…", "Esperando a que termine otra carpeta…"), ("Waiting for authorization in your browser…", "Esperando autorización en el navegador…"), ("Waiting for local changes to settle", "Esperando a que los cambios locales se asienten"), + ("Waiting for {folder} to finish…", "Esperando a que termine {folder}…"), ("Waiting to synchronize", "A la espera"), ("What's New", "Novedades"), ("When a remote folder is configured, the --path argument is passed to nextcloudcmd; leaving the field as / keeps the previous root-to-root behaviour.", "Cuando se configura una carpeta remota, se pasa el argumento --path a nextcloudcmd; dejar el campo como / mantiene el comportamiento anterior de raíz a raíz."), From b5dafe3aaa5bed606edf1bfc2d5c2954c0491b75 Mon Sep 17 00:00:00 2001 From: gnacho Date: Sun, 23 Aug 2026 11:39:23 +0200 Subject: [PATCH 3/3] docs(i18n): use 'borrados masivos' for the deletion-guard wording The deletion-guard strings translated 'deletions' as 'eliminaciones', but the flow is about reviewing a mass local deletion. Use 'borrados masivos' across the guard strings for clearer Spanish (Review deletions, approve once, none pending). --- po/es.po | 8 ++++---- src/util/translations/es.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/po/es.po b/po/es.po index 1e6139b..6457911 100644 --- a/po/es.po +++ b/po/es.po @@ -97,7 +97,7 @@ msgstr "Restaurar desde Nextcloud" #: src/nextsync/application.py:634 msgid "Approve These Deletions Once" -msgstr "Aprobar estas eliminaciones una vez" +msgstr "Aprobar estos borrados masivos una vez" #: src/nextsync/application.py:656 msgid "Finishing synchronization" @@ -950,11 +950,11 @@ msgstr "Llavero de contraseñas bloqueado" #: src/nextsync/ui/folder_status.py:29 src/nextsync/ui/main_window.py:148 msgid "Review Deletions" -msgstr "Revisar eliminaciones" +msgstr "Revisar borrados masivos" #: src/ui/folder_status.rs msgid "Review deletions" -msgstr "Revisar eliminaciones" +msgstr "Revisar borrados masivos" #: src/nextsync/ui/folder_status.py:97 msgid "Folder options" @@ -2640,7 +2640,7 @@ msgstr "Estos borrados se propagarán al servidor al sincronizar." #: src/ui/main_window.rs msgid "No deletions are pending review." -msgstr "No hay eliminaciones pendientes de revisión." +msgstr "No hay borrados masivos pendientes de revisión." #: src/ui/conflict_resolver.rs msgid "Deletions" diff --git a/src/util/translations/es.rs b/src/util/translations/es.rs index 9347ff3..2502471 100644 --- a/src/util/translations/es.rs +++ b/src/util/translations/es.rs @@ -41,7 +41,7 @@ pub static CATALOG: &[(&str, &str)] = &[ ("Allow invalid or self-signed certificates", "Permitir certificados no válidos o autofirmados"), ("Another synchronization engine ({name}) is already running on this folder. Close it and try again.", "Otro motor de sincronización ({name}) ya está funcionando en esta carpeta. Ciérralo e inténtalo de nuevo."), ("App Token", "Token de aplicación"), - ("Approve These Deletions Once", "Aprobar estas eliminaciones una vez"), + ("Approve These Deletions Once", "Aprobar estos borrados masivos una vez"), ("Ask before syncing folders larger than", "Preguntar antes de sincronizar carpetas mayores de"), ("Authentication", "Autenticación"), ("Authentication and synchronization failures", "Fallo de autenticación y sincronización"), @@ -233,7 +233,7 @@ pub static CATALOG: &[(&str, &str)] = &[ ("No conflicted copies found in {folder}.", "No se encontraron copias en conflicto en {folder}."), ("No credentials are saved for this account.", "No hay credenciales guardadas para esta cuenta."), ("No deleted files to resolve", "No hay archivos borrados que resolver"), - ("No deletions are pending review.", "No hay eliminaciones pendientes de revisión."), + ("No deletions are pending review.", "No hay borrados masivos pendientes de revisión."), ("No files are deleted: neither the local synchronized folders nor anything on the server. Only this app forgets the account, its credentials and its sync configuration.", "No se elimina ningún archivo: ni las carpetas locales sincronizadas ni nada del servidor. Solo esta aplicación olvida la cuenta, sus credenciales y su configuración de sincronización."), ("No pending local changes since the last synchronization.", "No hay cambios locales pendientes desde la última sincronización."), ("No restorable files were found in the server trash.", "No se encontraron archivos recuperables en la papelera del servidor."), @@ -335,11 +335,11 @@ pub static CATALOG: &[(&str, &str)] = &[ ("Resume Sync", "Reanudar sincronización"), ("Resume sync", "Reanudar sincronización"), ("Retained the previous last-known-good safety baseline after interrupted runs.", "Se conservó la última base de seguridad válida tras ejecuciones interrumpidas."), - ("Review Deletions", "Revisar eliminaciones"), + ("Review Deletions", "Revisar borrados masivos"), ("Review Setup", "Revisar configuración"), ("Review after this many missing files", "Revisar tras este número de archivos desaparecidos"), ("Review after this percentage is missing", "Revisar tras este porcentaje de desaparición"), - ("Review deletions", "Revisar eliminaciones"), + ("Review deletions", "Revisar borrados masivos"), ("Run a local interval", "Ejecutar un intervalo local"), ("Run a remote interval", "Ejecutar un intervalo remoto"), ("Run one synchronization without changing the power preference?", "¿Ejecutar una sincronización sin cambiar la preferencia de energía?"),