Skip to content

Commit 823cf6e

Browse files
committed
Source the packages-list pause reason from fetched data, not a frontend cache
The Installed Packages List cached each package's autosync pause reason in a page-scoped frontend map reconciled from Tauri events plus a get_autosync_snapshot seed. That reconciliation caused three P1 races (snapshot-vs-events, async listener-registration window, manual-clear staleness) because not every backend pause-clear path emits a UI event. Make it data-driven instead: the list-data command reads the autosync watcher's paused map (the single source of truth, the same source get_autosync_snapshot uses) and stamps each row's new pausedReason field with the Other-reason message. The UI derives the red state and hint purely from that fetched data and refetches on autosync events, so the durable red state always reflects authoritative backend data. Removes the frontend paused_map, resolved_since_mount, the snapshot-seeding spawn_local, and the now-unused listen_with_ready helper (folded back into listen). Adds byte-identical pausedReason wire-form tests on both sides.
1 parent 1ab155d commit 823cf6e

4 files changed

Lines changed: 197 additions & 148 deletions

File tree

quilt-sync/src-tauri/src/commands/package_list.rs

Lines changed: 108 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
//! Installed-packages list (light phase) and per-package status refresh
22
//! (heavy phase) for the Leptos UI.
33
4+
use std::collections::HashMap;
5+
46
use serde::Serialize;
57

68
use crate::Error;
9+
use crate::autopull::Watcher;
710
use crate::model;
811
use crate::quilt;
912

@@ -26,16 +29,25 @@ pub struct InstalledPackageListItem {
2629
/// the UI can still surface a misconfigured remote when origin
2730
/// resolution fails (status: "error" branch).
2831
pub remote_display: Option<String>,
32+
/// The autosync watcher's `Other` pause message for this namespace,
33+
/// if it is currently paused for a reason the status string cannot
34+
/// carry (workflow refusal, hash mismatch, etc.); `None` otherwise.
35+
///
36+
/// Read straight from the watcher's paused map (the single source of
37+
/// truth) at fetch time, so the UI derives the red/hint state from
38+
/// authoritative data instead of a reconciled frontend cache.
39+
pub paused_reason: Option<String>,
2940
}
3041

3142
async fn get_installed_packages_list_data_from_model(
3243
m: &impl model::QuiltModel,
3344
tracing: &crate::telemetry::Telemetry,
45+
paused_reasons: &HashMap<String, String>,
3446
) -> Result<InstalledPackagesListData, Error> {
3547
let list = m.get_installed_packages_list().await?;
3648
let mut packages = Vec::new();
3749
for installed_package in list {
38-
match load_package_item(m, tracing, &installed_package).await {
50+
match load_package_item(m, tracing, &installed_package, paused_reasons).await {
3951
Ok(item) => packages.push(item),
4052
Err(err) => {
4153
tracing::warn!(
@@ -52,28 +64,33 @@ async fn load_package_item(
5264
m: &impl model::QuiltModel,
5365
tracing: &crate::telemetry::Telemetry,
5466
installed_package: &quilt::InstalledPackage,
67+
paused_reasons: &HashMap<String, String>,
5568
) -> Result<InstalledPackageListItem, Error> {
69+
let namespace = installed_package.namespace.to_string();
70+
let paused_reason = paused_reasons.get(&namespace).cloned();
5671
let lineage = m.get_installed_package_lineage(installed_package).await?;
5772

5873
let Some(remote_uri) = lineage.remote_uri.as_ref() else {
5974
return Ok(InstalledPackageListItem {
60-
namespace: installed_package.namespace.to_string(),
75+
namespace,
6176
status: "local".to_string(),
6277
has_changes: false,
6378
uri: None,
6479
remote_display: None,
80+
paused_reason,
6581
});
6682
};
6783

6884
let typed_uri = quilt_uri::S3PackageUri::from(remote_uri);
6985

7086
if remote_uri.origin.is_none() {
7187
return Ok(InstalledPackageListItem {
72-
namespace: installed_package.namespace.to_string(),
88+
namespace,
7389
status: "error".to_string(),
7490
has_changes: false,
7591
uri: Some(typed_uri),
7692
remote_display: Some(remote_uri.to_string()),
93+
paused_reason,
7794
});
7895
}
7996

@@ -85,20 +102,34 @@ async fn load_package_item(
85102
let has_changes = false; // Refined by refresh_package_status
86103

87104
Ok(InstalledPackageListItem {
88-
namespace: installed_package.namespace.to_string(),
105+
namespace,
89106
status: upstream_state.to_string(),
90107
has_changes,
91108
uri: Some(typed_uri),
92109
remote_display: Some(remote_display),
110+
paused_reason,
93111
})
94112
}
95113

96114
#[tauri::command]
97115
pub async fn get_installed_packages_list_data(
98116
m: tauri::State<'_, model::Model>,
99117
tracing: tauri::State<'_, crate::telemetry::Telemetry>,
118+
watcher: tauri::State<'_, Watcher>,
100119
) -> Result<InstalledPackagesListData, String> {
101-
get_installed_packages_list_data_from_model(&*m, &tracing)
120+
// Read the watcher's paused map — the single source of truth — the
121+
// same way `get_autosync_snapshot` does. Only `Other`-reason pauses
122+
// carry a `message`; those are the reasons the status string cannot
123+
// convey, so they are the only ones surfaced on each row.
124+
let paused_reasons: HashMap<String, String> = watcher
125+
.snapshot()
126+
.await
127+
.paused
128+
.into_iter()
129+
.filter_map(|entry| entry.message.map(|message| (entry.namespace, message)))
130+
.collect();
131+
132+
get_installed_packages_list_data_from_model(&*m, &tracing, &paused_reasons)
102133
.await
103134
.map_err(|e| e.to_frontend_string())
104135
}
@@ -203,7 +234,7 @@ mod tests {
203234
mocks::mock_installed_packages_list(&mut model);
204235
let tracing = crate::telemetry::Telemetry::default();
205236

206-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
237+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
207238
.await
208239
.map_err(|e| e.to_string())?;
209240

@@ -266,7 +297,7 @@ mod tests {
266297
});
267298

268299
let tracing = crate::telemetry::Telemetry::default();
269-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
300+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
270301
.await
271302
.map_err(|e| e.to_string())?;
272303

@@ -316,7 +347,7 @@ mod tests {
316347
});
317348

318349
let tracing = crate::telemetry::Telemetry::default();
319-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
350+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
320351
.await
321352
.map_err(|e| e.to_string())?;
322353

@@ -352,7 +383,7 @@ mod tests {
352383
});
353384

354385
let tracing = crate::telemetry::Telemetry::default();
355-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
386+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
356387
.await
357388
.map_err(|e| e.to_string())?;
358389

@@ -386,7 +417,7 @@ mod tests {
386417
.returning(|_| Ok(quilt::lineage::PackageLineage::default()));
387418

388419
let tracing = crate::telemetry::Telemetry::default();
389-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
420+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
390421
.await
391422
.map_err(|e| e.to_string())?;
392423

@@ -426,7 +457,7 @@ mod tests {
426457
});
427458

428459
let tracing = crate::telemetry::Telemetry::default();
429-
let data = get_installed_packages_list_data_from_model(&model, &tracing)
460+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &HashMap::new())
430461
.await
431462
.map_err(|e| e.to_string())?;
432463

@@ -643,4 +674,70 @@ mod tests {
643674
assert!(!result.has_changes);
644675
Ok(())
645676
}
677+
678+
// ── paused_reason population (data-driven red state) ──
679+
680+
#[tokio::test]
681+
async fn test_installed_packages_list_data_populates_paused_reason() -> Result<(), String> {
682+
let mut model = mocks::create();
683+
684+
let pkgs = vec![
685+
make_installed_package(("test", "paused")),
686+
make_installed_package(("test", "clean")),
687+
];
688+
model
689+
.expect_get_installed_packages_list()
690+
.return_once(move || Ok(pkgs));
691+
model
692+
.expect_get_installed_package_lineage()
693+
.returning(|pkg| {
694+
let uri = make_manifest_uri(&pkg.namespace.to_string());
695+
Ok(quilt::lineage::PackageLineage::from_remote(
696+
uri,
697+
"abcdef".to_string(),
698+
))
699+
});
700+
701+
// Stand in for the watcher's paused map: only the paused namespace
702+
// has an `Other` message.
703+
let mut paused_reasons = HashMap::new();
704+
paused_reasons.insert(
705+
"test/paused".to_string(),
706+
"workflow rejected metadata".to_string(),
707+
);
708+
709+
let tracing = crate::telemetry::Telemetry::default();
710+
let data = get_installed_packages_list_data_from_model(&model, &tracing, &paused_reasons)
711+
.await
712+
.map_err(|e| e.to_string())?;
713+
714+
let find = |ns: &str| data.packages.iter().find(|p| p.namespace == ns).unwrap();
715+
assert_eq!(
716+
find("test/paused").paused_reason.as_deref(),
717+
Some("workflow rejected metadata"),
718+
);
719+
assert!(find("test/clean").paused_reason.is_none());
720+
Ok(())
721+
}
722+
723+
/// The serialized row must be byte-identical to what the UI mirror
724+
/// (`quilt_sync_ui::commands::PackageItemData`) deserializes in its
725+
/// `package_item_data_wire_form_is_verbatim`. If the two drift, the
726+
/// list silently drops the pause reason (or a whole field) at the
727+
/// Tauri boundary.
728+
#[test]
729+
fn package_item_data_wire_form_is_verbatim() {
730+
let item = InstalledPackageListItem {
731+
namespace: "acme/data".to_string(),
732+
status: "paused".to_string(),
733+
has_changes: false,
734+
uri: None,
735+
remote_display: None,
736+
paused_reason: Some("workflow rejected metadata".to_string()),
737+
};
738+
assert_eq!(
739+
serde_json::to_string(&item).unwrap(),
740+
r#"{"namespace":"acme/data","status":"paused","hasChanges":false,"uri":null,"remoteDisplay":null,"pausedReason":"workflow rejected metadata"}"#
741+
);
742+
}
646743
}

quilt-sync/ui/src/commands.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,13 @@ pub struct PackageItemData {
261261
pub has_changes: bool,
262262
pub uri: Option<S3PackageUri>,
263263
pub remote_display: Option<String>,
264+
/// The autosync watcher's `Other` pause message for this namespace,
265+
/// fetched with the list data. `Some` means the row is autosync-paused
266+
/// for a reason the status string cannot carry, so it renders red with
267+
/// this reason as its third line; `None` means no such pause. Sourced
268+
/// directly from the backend's authoritative paused map — there is no
269+
/// frontend cache to go stale.
270+
pub paused_reason: Option<String>,
264271
}
265272

266273
#[derive(Clone, Debug, Deserialize)]
@@ -878,7 +885,34 @@ pub async fn send_crash_report(zip_path: String) -> Result<String, String> {
878885

879886
#[cfg(test)]
880887
mod tests {
881-
use super::{CommitViolation, CommitWorkflows, ViolationField, WorkflowInfo, WorkflowIntent};
888+
use super::{
889+
CommitViolation, CommitWorkflows, PackageItemData, ViolationField, WorkflowInfo,
890+
WorkflowIntent,
891+
};
892+
893+
/// The mirror struct must deserialize the exact JSON the backend
894+
/// (`quilt_sync::commands::package_list::InstalledPackageListItem`)
895+
/// serializes. This literal is anchored identically in the backend's
896+
/// `package_item_data_wire_form_is_verbatim`; if the two drift, the
897+
/// list silently drops the pause reason (or a whole field) at the
898+
/// Tauri boundary — the exact class of bug this data-driven design
899+
/// exists to prevent.
900+
#[test]
901+
fn package_item_data_wire_form_is_verbatim() {
902+
let item = serde_json::from_str::<PackageItemData>(
903+
r#"{"namespace":"acme/data","status":"paused","hasChanges":false,"uri":null,"remoteDisplay":null,"pausedReason":"workflow rejected metadata"}"#,
904+
)
905+
.unwrap();
906+
assert_eq!(item.namespace, "acme/data");
907+
assert_eq!(item.status, "paused");
908+
assert!(!item.has_changes);
909+
assert!(item.uri.is_none());
910+
assert!(item.remote_display.is_none());
911+
assert_eq!(
912+
item.paused_reason.as_deref(),
913+
Some("workflow rejected metadata")
914+
);
915+
}
882916

883917
/// The mirror types must deserialize the exact tagged JSON the backend
884918
/// (`quilt_sync::commands::commit_data::CommitViolation`) serializes. These

0 commit comments

Comments
 (0)