diff --git a/crates/uffs-cli/src/commands/update/mod.rs b/crates/uffs-cli/src/commands/update/mod.rs index 00dbc945a..9d22dcca6 100644 --- a/crates/uffs-cli/src/commands/update/mod.rs +++ b/crates/uffs-cli/src/commands/update/mod.rs @@ -173,6 +173,9 @@ enum UpdatePlan { fn assess(report: &DetectionReport) -> UpdatePlan { let installed = report::distinct_versions(report); let skewed = installed.len() > 1; + // A core binary missing from a real install root makes it *incomplete* — + // an update reconciles the full core set back into place. + let incomplete = has_missing_core(report); let Some(latest) = acquire::latest_version() else { return UpdatePlan::Offline; }; @@ -182,13 +185,29 @@ fn assess(report: &DetectionReport) -> UpdatePlan { // Zero or mixed versions → an update realigns the install. _ => true, }; - if skewed || newer { + if skewed || newer || incomplete { UpdatePlan::Available { latest } } else { UpdatePlan::UpToDate { latest } } } +/// True when any **unmanaged** install root is missing a core binary — i.e. +/// the install is incomplete relative to the canonical set +/// (`binaries::KNOWN_BINARIES`, the single source of truth). `WinGet` roots are +/// delegated to `winget upgrade`, so they are not reconciled here. +fn has_missing_core(report: &DetectionReport) -> bool { + report + .roots + .iter() + .filter(|root| root.channel.label() == "unmanaged") + .any(|root| { + binaries::KNOWN_BINARIES + .iter() + .any(|stem| !root.binaries.iter().any(|bin| bin.name == *stem)) + }) +} + /// Run the full end-to-end update when one is needed; otherwise report the /// install is current. Journaled + auto-rollback (delegated to `apply`). fn run_automatic_update(report: &DetectionReport, verbose: bool) -> Result<()> { diff --git a/crates/uffs-cli/src/commands/update/snapshot.rs b/crates/uffs-cli/src/commands/update/snapshot.rs index 49d6b7aaa..f4ed39bb9 100644 --- a/crates/uffs-cli/src/commands/update/snapshot.rs +++ b/crates/uffs-cli/src/commands/update/snapshot.rs @@ -14,7 +14,7 @@ use std::path::PathBuf; use serde_json::{Value, json}; use super::model::DetectionReport; -use super::procinfo; +use super::{binaries, procinfo}; /// Directory that holds update snapshots + staging /// (`/update`). @@ -79,19 +79,35 @@ fn build_snapshot_value( captured_unix: u64, to_version: Option<&str>, ) -> Value { + // An apply snapshot (to_version set) reconciles each unmanaged root to the + // full core set: a missing core binary is appended with a null version so + // `acquire` downloads it and `apply` *adds* it. A doctor/non-apply snapshot + // (to_version = None) reports the install exactly as found. + let complete = to_version.is_some(); let targets: Vec = report .roots .iter() .map(|root| { + let mut bins: Vec = root + .binaries + .iter() + .map(|binary| { + json!({ "name": binary.name, "on_disk_version": binary.version }) + }) + .collect(); + if complete && root.channel.label() == "unmanaged" { + for stem in binaries::KNOWN_BINARIES { + if !root.binaries.iter().any(|binary| binary.name == stem) { + bins.push(json!({ "name": stem, "on_disk_version": Value::Null })); + } + } + } json!({ "root": root.dir.display().to_string(), "channel": root.channel.label(), "scope": root.scope.label(), "anchored_by": root.anchored_by.iter().map(|anchor| anchor.label()).collect::>(), - "binaries": root.binaries.iter().map(|binary| json!({ - "name": binary.name, - "on_disk_version": binary.version, - })).collect::>(), + "binaries": bins, }) }) .collect(); @@ -169,6 +185,11 @@ mod tests { probe("/targets/0/channel", json!("unmanaged")); probe("/targets/0/anchored_by/1", json!("daemon")); probe("/targets/0/binaries/0/on_disk_version", json!("0.6.2")); + // Apply snapshot (to_version set) → the unmanaged root is completed to + // the full core set: missing core binaries are appended after the + // present ones, each with a null on-disk version (= "add it"). + probe("/targets/0/binaries/1/name", json!("uffs")); + probe("/targets/0/binaries/1/on_disk_version", json!(null)); probe("/running/0/component", json!("daemon")); probe("/running/0/pid", json!(4242_u32)); probe("/running/0/command_line", json!("uffsd --no-retire")); diff --git a/crates/uffs-update/src/apply.rs b/crates/uffs-update/src/apply.rs index 4484bf360..62cd03c8e 100644 --- a/crates/uffs-update/src/apply.rs +++ b/crates/uffs-update/src/apply.rs @@ -143,20 +143,43 @@ pub(crate) fn prune_backup(binary: &Path) -> Result<()> { Ok(()) } -/// Backup-then-swap a single binary from `staged` into `target`, leaving -/// a `.bak` for rollback. Fails (without swapping) if the staged image is -/// missing. +/// Place the staged image at `target`, atomically. +/// +/// - **Replace** (target exists): back the old image aside to `.bak`, then swap +/// the new one in — returns `Some(bak)`. Rollback = restore the `.bak`. +/// - **Add** (target absent): no prior image to back up, so just place the new +/// one — returns `None`. This is the completeness path; rollback of an added +/// binary is a *delete* (the caller records `BinaryEntry::added`). /// /// # Errors /// -/// Propagates backup/swap failures; bails if `staged` is absent. -pub(crate) fn backup_and_swap(staged: &Path, target: &Path) -> Result { +/// The staged image is missing, or a backup/rename fails. +pub(crate) fn backup_and_swap(staged: &Path, target: &Path) -> Result> { if !staged.is_file() { bail!("staged image missing: {}", staged.display()); } - let bak = backup(target)?; - swap_in(staged, target)?; - Ok(bak) + if target.exists() { + let bak = backup(target)?; + swap_in(staged, target)?; + Ok(Some(bak)) + } else { + swap_in(staged, target)?; + Ok(None) + } +} + +/// Roll back an **added** binary (no `.bak` exists): delete the image we +/// placed. Idempotent — an already-absent target is success. +/// +/// # Errors +/// +/// The file exists but cannot be removed. +pub(crate) fn remove_added(target: &Path) -> Result<()> { + if target.exists() { + std::fs::remove_file(target) + .with_context(|| format!("removing added binary {}", target.display()))?; + } + Ok(()) } #[cfg(test)] @@ -164,7 +187,8 @@ mod tests { use std::path::{Path, PathBuf}; use super::{ - backup, backup_and_swap, backup_path, prune_backup, restore, swap_in, sweep_stale_backups, + backup, backup_and_swap, backup_path, prune_backup, remove_added, restore, swap_in, + sweep_stale_backups, }; fn scratch(tag: &str) -> PathBuf { @@ -189,7 +213,9 @@ mod tests { write(&target, "OLD"); write(&staged, "NEW"); - let bak = backup_and_swap(&staged, &target).expect("apply"); + let bak = backup_and_swap(&staged, &target) + .expect("apply") + .expect("replace returns a backup"); assert_eq!(read(&target), "NEW", "target holds the new image"); assert_eq!(read(&bak), "OLD", "backup holds the old image"); assert!(!staged.exists(), "staged consumed by the rename"); @@ -200,6 +226,27 @@ mod tests { assert!(!bak.exists(), "backup consumed by the restore"); } + #[test] + fn add_then_rollback_deletes_the_added_binary() { + // Completeness "add": the target does not exist yet. + let dir = scratch("add"); + let target = dir.join("uffs-mft"); + let staged = dir.join("uffs-mft.new"); + write(&staged, "NEW"); + + // Add → no backup, target placed, return is None (signals "added"). + let backup = backup_and_swap(&staged, &target).expect("add"); + assert!(backup.is_none(), "an add has no backup"); + assert_eq!(read(&target), "NEW", "added image is in place"); + assert!(!backup_path(&target).exists(), "no .bak for an add"); + + // Rollback of an add = delete the placed image. + remove_added(&target).expect("remove_added"); + assert!(!target.exists(), "rollback removed the added binary"); + // Idempotent on an already-absent target. + remove_added(&target).expect("remove_added is idempotent"); + } + #[test] fn backup_is_idempotent() { let dir = scratch("idem"); diff --git a/crates/uffs-update/src/journal.rs b/crates/uffs-update/src/journal.rs index e2c1e8ec1..6bd6f04e0 100644 --- a/crates/uffs-update/src/journal.rs +++ b/crates/uffs-update/src/journal.rs @@ -88,6 +88,11 @@ pub(crate) struct BinaryEntry { /// Backup file name (in `backup_dir`) once backed up. #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) backup: Option, + /// True when this binary was newly **added** (no prior file existed) by a + /// completeness reconcile. There is no `.bak`, so rollback **deletes** the + /// placed image rather than restoring a backup. + #[serde(default)] + pub(crate) added: bool, } /// One install root being updated. @@ -262,6 +267,20 @@ impl Journal { } } + /// Mark a binary as newly added (no prior file) so rollback deletes it + /// rather than restoring a non-existent `.bak`. + pub(crate) fn set_binary_added(&mut self, root: &Path, name: &str) { + for target in &mut self.targets { + if target.root == root { + for binary in &mut target.binaries { + if binary.name == name { + binary.added = true; + } + } + } + } + } + /// `true` when this journal describes an interrupted run that Phase H /// should finish or undo: not terminal **and** its owner is gone. pub(crate) const fn needs_recovery(&self, owner_alive: bool) -> bool { @@ -295,6 +314,7 @@ mod tests { name: "uffsd".to_owned(), status: BinaryStatus::Pending, backup: None, + added: false, }], }]; journal diff --git a/crates/uffs-update/src/orchestrate.rs b/crates/uffs-update/src/orchestrate.rs index f3cce4655..6fd706cf9 100644 --- a/crates/uffs-update/src/orchestrate.rs +++ b/crates/uffs-update/src/orchestrate.rs @@ -68,6 +68,7 @@ pub(crate) fn journal_from_snapshot( name: binary.name.clone(), status: BinaryStatus::Pending, backup: None, + added: false, }) .collect(), }) @@ -146,15 +147,24 @@ where journal.transition(UpdateState::Swapping, "apply.begin")?; let plan = collect_plan(journal); - // backup + swap each + // backup + swap each (or, for a missing core binary, *add* it) for (root, stem, target) in &plan { let staged = staged_dir.join(exe_name(stem)); - if let Err(err) = apply::backup_and_swap(&staged, target) { - rollback_all(journal)?; - return Err(err); + match apply::backup_and_swap(&staged, target) { + // `None` ⇒ the target did not exist: a completeness add. Record it + // so rollback deletes the placed image instead of hunting a `.bak`. + Ok(backup) => { + if backup.is_none() { + journal.set_binary_added(root, stem); + } + journal.set_binary_status(root, stem, BinaryStatus::Swapped); + journal.save()?; + } + Err(err) => { + rollback_all(journal)?; + return Err(err); + } } - journal.set_binary_status(root, stem, BinaryStatus::Swapped); - journal.save()?; } journal.transition(UpdateState::Swapped, "apply.all_swapped")?; @@ -171,15 +181,34 @@ where Ok(()) } -/// Restore every backed-up binary (pre-commit rollback, INV-3). +/// Roll back every touched binary (pre-commit rollback, INV-3): restore a +/// **replaced** binary from its `.bak`, and **delete** an **added** binary +/// (no `.bak` exists). Both primitives are idempotent on untouched targets. /// /// # Errors /// -/// Propagates a restore failure. +/// Propagates a restore / remove failure. pub(crate) fn rollback_all(journal: &mut Journal) -> Result<()> { journal.transition(UpdateState::RollingBack, "rollback.begin")?; - for (root, stem, target) in collect_plan(journal) { - apply::restore(&target)?; + // Snapshot (root, stem, target, added) first — the `set_binary_status` + // below needs `&mut journal`, so we cannot hold an iterator into it. + let items: Vec<(PathBuf, String, PathBuf, bool)> = journal + .targets + .iter() + .flat_map(|target| { + let root = target.root.clone(); + target.binaries.iter().map(move |binary| { + let path = root.join(exe_name(&binary.name)); + (root.clone(), binary.name.clone(), path, binary.added) + }) + }) + .collect(); + for (root, stem, target, added) in items { + if added { + apply::remove_added(&target)?; + } else { + apply::restore(&target)?; + } journal.set_binary_status(&root, &stem, BinaryStatus::RolledBack); } journal.transition(UpdateState::RolledBack, "rollback.done")?;