diff --git a/crates/spt-daemon/src/broker.rs b/crates/spt-daemon/src/broker.rs index 64799c0a..3428a5ea 100644 --- a/crates/spt-daemon/src/broker.rs +++ b/crates/spt-daemon/src/broker.rs @@ -10214,6 +10214,22 @@ impl Broker { crate::crc_swap::apply_crc_swap(&plan).map_err(|e| e.to_string()) })(); + // (3b) Heal a declared entry binary that arrived without its exec bit + // (releases#62) BEFORE the restart below spawns it. Loud; a no-op on + // Windows and on a manifest this daemon cannot read (the CLI's + // re-register reads the same manifest and heals again). + // [impl->REQ-ADAPTER-ENTRY-EXEC-BIT] + if swap_result.is_ok() { + if let Ok(m) = std::fs::read_to_string(install_dir.join("manifest.toml")) + .map_err(|e| e.to_string()) + .and_then(|t| { + spt_runtime::manifest::Manifest::from_toml_str(&t).map_err(|e| format!("{e:?}")) + }) + { + spt_runtime::entry_exec::force_entry_exec(&m, &install_dir, &req.adapter); + } + } + // (4)/(5) Restart each translation — from the NEW path on success, the OLD // path on a rolled-back failure. Build OFF-lock; re-insert under lock. let restart_path = match &swap_result { diff --git a/crates/spt-daemon/src/crc_swap.rs b/crates/spt-daemon/src/crc_swap.rs index 6ca2aef8..00637326 100644 --- a/crates/spt-daemon/src/crc_swap.rs +++ b/crates/spt-daemon/src/crc_swap.rs @@ -14,19 +14,49 @@ //! CRC here is plain content hashing (reuse [`crate::release::sha256_hex`], no new //! dep) — "replace only what changed", not cryptographic integrity (that is the //! archive signature, ADR-0024 / REQ-UPD-9). +//! +//! **Additive for binaries, a mirror for `strings/` (releases#278).** A file the +//! new version stopped shipping stays in place — EXCEPT under `strings/`, where it +//! is pruned after the swap commits. Additive is right for binaries: a running +//! translation child must never be yanked mid-update, and that is the Windows +//! exe-lock reason this module exists. It is wrong for `strings/`, which is pure +//! data a harness discovers by SCANNING the directory: a retired skill file left +//! there is not "unreferenced", it is found and keeps firing (the omp-spt field +//! case: stale flat skills beside their `/SKILL.md` replacements for two +//! months). use crate::release::sha256_hex; use std::path::{Path, PathBuf}; -/// One planned file replacement discovered by [`plan_crc_swap`]. +/// The install-root subtree that MIRRORS the archive on update — the only +/// subtree a [`SwapAction::Prune`] row may ever come from. +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] +pub const MIRRORED_SUBTREE: &str = "strings"; + +/// One planned row discovered by [`plan_crc_swap`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct PlannedSwap { /// Path relative to the tree root (for diagnostics / determinism). pub rel: PathBuf, - /// The staged source file (`/`). + /// The staged source file (`/`). For a [`SwapAction::Prune`] + /// row this path does not exist — its absence is the reason for the row. pub staged: PathBuf, /// The install-dir target (`/`). pub target: PathBuf, + /// What the apply does with this row. + pub action: SwapAction, +} + +/// The row classes of a [`plan_crc_swap`] plan. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SwapAction { + /// Place the staged file over (or at) the target — the content differs or the + /// target is absent. Committed by the never-strand protocol. + Place, + /// Remove the target: it sits under `strings/` and the staged archive no longer + /// ships it (releases#278). Removed only AFTER every `Place` row committed. + // [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] + Prune, } /// Walk `staging` and return the files whose content differs from their @@ -34,14 +64,79 @@ pub struct PlannedSwap { /// content is byte-identical are skipped, so a still-running unchanged binary is /// never disturbed. Read-only: does not mutate either tree. The returned plan is /// sorted by `rel` for deterministic apply + testing. +/// +/// It also emits a [`SwapAction::Prune`] row for every file under +/// `/strings/` with no counterpart under `/strings/` +/// (releases#278). Nothing outside `strings/` is ever a prune row, and the swap's +/// own `.new` / `.old[-N]` litter is left to the rules that already own it. // [impl->REQ-ADAPTER-LIVE-UPDATE] +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] pub fn plan_crc_swap(staging: &Path, install_dir: &Path) -> std::io::Result> { let mut plan = Vec::new(); collect_diffs(staging, staging, install_dir, &mut plan)?; + let mirrored = install_dir.join(MIRRORED_SUBTREE); + if mirrored.is_dir() { + collect_prunes(install_dir, &mirrored, staging, &mut plan)?; + } plan.sort_by(|a, b| a.rel.cmp(&b.rel)); Ok(plan) } +/// Recurse `dir` (a subtree of `/strings`), emitting a prune row for +/// every file the staged tree does not ship. +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] +fn collect_prunes( + install_dir: &Path, + dir: &Path, + staging: &Path, + plan: &mut Vec, +) -> std::io::Result<()> { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + collect_prunes(install_dir, &path, staging, plan)?; + continue; + } + if is_swap_litter(&path) { + continue; + } + let rel = path + .strip_prefix(install_dir) + .expect("path is under install_dir") + .to_path_buf(); + let staged = staging.join(&rel); + if !staged.exists() { + plan.push(PlannedSwap { + rel, + staged, + target: path, + action: SwapAction::Prune, + }); + } + } + Ok(()) +} + +/// `.new`, `.old`, `.old-` — the swap's own staging and displacement +/// names, owned by the Phase-0 GC and the commit loop, never by a prune. +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] +fn is_swap_litter(path: &Path) -> bool { + let Some(name) = path.file_name().and_then(|n| n.to_str()) else { + return false; + }; + if name.ends_with(".new") || name.ends_with(".old") { + return true; + } + match name.rfind(".old-") { + Some(i) => { + let n = &name[i + ".old-".len()..]; + !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit()) + } + None => false, + } +} + /// Recurse `dir` (a subtree of `root`), comparing each file against its /// `install_dir`-relative counterpart. fn collect_diffs( @@ -72,6 +167,7 @@ fn collect_diffs( rel, staged: path, target, + action: SwapAction::Place, }); } } @@ -87,6 +183,11 @@ fn collect_diffs( /// to a FRESH `.old[-N]` (if present) then rename `.new` → /// ``. /// 3. **Cleanup** — delete the displaced originals once the whole loop committed. +/// 4. **Prune** — only now, with every `Place` row committed, remove the +/// [`SwapAction::Prune`] targets (releases#278), then any dir under `strings/` +/// the prune left empty. A failed commit returns before this phase, so a +/// rolled-back update never loses a file; a prune that cannot remove its file +/// says so on stderr and the update still stands (the new bits are in). /// /// **C1 (REQ-CRC-SWAP-OLD-DISPLACE): displace, never replace.** The prior impl /// renamed the original to a FIXED `.old`. On a live box a surviving @@ -105,8 +206,59 @@ fn collect_diffs( /// restarts the OLD resident child and reports the update failed. // [impl->REQ-ADAPTER-LIVE-UPDATE] // [impl->REQ-CRC-SWAP-OLD-DISPLACE] +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] pub fn apply_crc_swap(plan: &[PlannedSwap]) -> std::io::Result<()> { - apply_crc_swap_with(plan, &|from, to| std::fs::rename(from, to)) + apply_then_prune(plan, &|from, to| std::fs::rename(from, to)) +} + +/// The commit, THEN the prune — split out so a unit can inject a failing rename +/// and prove a rolled-back commit prunes nothing. +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] +fn apply_then_prune( + plan: &[PlannedSwap], + rename: &dyn Fn(&Path, &Path) -> std::io::Result<()>, +) -> std::io::Result<()> { + apply_crc_swap_with(plan, rename)?; + apply_prunes(plan); + Ok(()) +} + +/// Phase 4 of [`apply_crc_swap`]: remove every prune row's target, then the +/// directories under `strings/` the removals emptied, deepest first. `strings/` +/// itself is never removed, and a dir that still holds anything stays +/// (`remove_dir` refuses a non-empty one). +// [impl->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] +fn apply_prunes(plan: &[PlannedSwap]) { + let mut emptied: Vec = Vec::new(); + for row in plan.iter().filter(|r| r.action == SwapAction::Prune) { + match std::fs::remove_file(&row.target) { + Ok(()) => { + // `` is `target` with `rel`'s components stripped. + let Some(root) = row.target.ancestors().nth(row.rel.components().count()) else { + continue; + }; + let mirrored = root.join(MIRRORED_SUBTREE); + let mut dir = row.target.parent(); + while let Some(d) = dir { + if d == mirrored || !d.starts_with(&mirrored) { + break; + } + emptied.push(d.to_path_buf()); + dir = d.parent(); + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => spt_proto::emit_line_err!( + "CRC_SWAP_PRUNE_FAILED: {}: {e} — the update stands; this stale file remains", + row.target.display() + ), + } + } + emptied.sort_by_key(|d| std::cmp::Reverse(d.components().count())); + emptied.dedup(); + for d in emptied { + let _ = std::fs::remove_dir(&d); + } } /// [`apply_crc_swap`] over an injectable `rename` so the never-strand rollback is @@ -129,6 +281,8 @@ fn apply_crc_swap_with( // safe; in the crash-recovery state the `.old` survives until a SUCCESSFUL commit // repopulates the target, and the NEXT run (target now present) sweeps it. // [impl->REQ-CRC-SWAP-OLD-DISPLACE] + let plan: Vec<&PlannedSwap> = plan.iter().filter(|s| s.action == SwapAction::Place).collect(); + let plan = plan.as_slice(); for swap in plan { if swap.target.exists() { gc_stale_old(&swap.target); @@ -268,8 +422,9 @@ fn rollback( } } -/// Remove any leftover `.new` staging files (failure cleanup). -fn cleanup_new(plan: &[PlannedSwap]) { +/// Remove any leftover `.new` staging files (failure cleanup). Takes the +/// `Place` rows only: a prune row stages nothing. +fn cleanup_new(plan: &[&PlannedSwap]) { for swap in plan { let _ = std::fs::remove_file(new_path(&swap.target)); } @@ -454,11 +609,13 @@ mod tests { rel: PathBuf::from("first"), staged: first_staged, target: first_target.clone(), + action: SwapAction::Place, }, PlannedSwap { rel: PathBuf::from("second"), staged: second_staged, target: second_target.clone(), + action: SwapAction::Place, }, ]; @@ -697,4 +854,93 @@ mod tests { assert!(msg.contains("bin.old"), "carries the displacement target: {msg}"); assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied, "preserves kind"); } + + /// The prune rows of a plan, by `rel`. + fn prunes(plan: &[PlannedSwap]) -> Vec { + plan.iter() + .filter(|s| s.action == SwapAction::Prune) + .map(|s| s.rel.clone()) + .collect() + } + + /// releases#278, the ruled unit: a dest holding a stale `strings/skills/old.md` + /// AND a stale root `foo.exe` yields EXACTLY ONE prune row — the `.md`. A + /// binary the new version dropped stays (additive), and the swap's own + /// `.new` / `.old[-N]` litter under `strings/` is not a prune row either. + // [unit->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] + #[test] + fn plan_prunes_only_stale_strings_never_binaries_or_litter() { + let staging = tempdir().unwrap(); + let install = tempdir().unwrap(); + let (s, i) = (staging.path(), install.path()); + write(&s.join("manifest.toml"), b"m"); + write(&s.join("strings/skills/new/SKILL.md"), b"new"); + write(&i.join("manifest.toml"), b"m"); + write(&i.join("strings/skills/new/SKILL.md"), b"new"); + write(&i.join("strings/skills/old.md"), b"stale"); + write(&i.join("foo.exe"), b"stale binary"); + write(&i.join("strings/skills/x.md.new"), b"litter"); + write(&i.join("strings/skills/x.md.old"), b"litter"); + write(&i.join("strings/skills/x.md.old-3"), b"litter"); + + let plan = plan_crc_swap(s, i).unwrap(); + assert_eq!(prunes(&plan), vec![PathBuf::from("strings/skills/old.md")]); + assert!( + plan.iter().all(|r| r.action == SwapAction::Prune), + "nothing else differs, so the prune is the whole plan: {plan:?}" + ); + } + + /// The field case end to end at the module seam: v1 shipped the flat + /// `strings/skills/a.md` + `strings/briefs/b.md`, v2 ships `a/SKILL.md` only. + /// After apply the install mirrors v2 under `strings/` — `a.md` gone, the + /// emptied `briefs/` dir gone, `strings/` itself kept — while the dropped + /// root binary stays, and no swap litter is left behind. + // [unit->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] + #[test] + fn apply_mirrors_strings_and_keeps_dropped_binaries() { + let staging = tempdir().unwrap(); + let install = tempdir().unwrap(); + let (s, i) = (staging.path(), install.path()); + write(&s.join("strings/skills/a/SKILL.md"), b"v2"); + write(&i.join("strings/skills/a.md"), b"v1"); + write(&i.join("strings/briefs/b.md"), b"v1"); + write(&i.join("old-helper.exe"), b"v1 binary"); + + let plan = plan_crc_swap(s, i).unwrap(); + apply_crc_swap(&plan).unwrap(); + + assert_eq!(fs::read(i.join("strings/skills/a/SKILL.md")).unwrap(), b"v2"); + assert!(!i.join("strings/skills/a.md").exists(), "the retired flat skill is pruned"); + assert!(!i.join("strings/briefs").exists(), "a dir the prune emptied is removed"); + assert!(i.join("strings").is_dir(), "strings/ itself is never removed"); + assert!(i.join("old-helper.exe").exists(), "binaries stay additive"); + assert!(!has_swap_litter(i), "no .new/.old litter"); + } + + /// A commit that FAILS and rolls back prunes nothing: the stale strings file + /// is still there, because the old version is what is installed. + // [unit->REQ-ADAPTER-UPDATE-PRUNES-STRINGS] + #[test] + fn a_rolled_back_commit_prunes_nothing() { + let staging = tempdir().unwrap(); + let install = tempdir().unwrap(); + let (s, i) = (staging.path(), install.path()); + write(&s.join("bin"), b"new bin"); + write(&i.join("bin"), b"old bin"); + write(&i.join("strings/stale.md"), b"v1"); + + let plan = plan_crc_swap(s, i).unwrap(); + assert_eq!(prunes(&plan), vec![PathBuf::from("strings/stale.md")]); + let fail_commit = |from: &Path, to: &Path| -> std::io::Result<()> { + if from.extension().and_then(|e| e.to_str()) == Some("new") { + Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "held")) + } else { + std::fs::rename(from, to) + } + }; + assert!(apply_then_prune(&plan, &fail_commit).is_err()); + assert_eq!(fs::read(i.join("bin")).unwrap(), b"old bin", "rolled back"); + assert!(i.join("strings/stale.md").exists(), "no prune on a failed commit"); + } }