diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 69fb09df9..2e71246f3 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -754,6 +754,9 @@ worse, lets a warm cache silently serve unpatched bytes): (parity with rollback/repair/`scan --prune`; GC errors warn and continue, repair's posture). Package archives (`.socket/packages/`) are legacy in v5.0: nothing writes or reads them, so every GC sweep removes the whole directory. + Like `rollback`, `remove` retains the original blobs for every patch left in the manifest + and for removed-but-not-installed patches, so removing one patch preserves offline rollback + of other active patches. Only blobs no longer referenced by that keep set are collected. * **remove restores hosted pins (v5.0)**: an identifier matching hosted pins in the lockfiles (purl or patch uuid; v5 keeps no hosted ledger) restores each matched pin to its default upstream registry entry — the same restore as `rollback` (see "Hosted unwind coverage"), for every @@ -830,7 +833,7 @@ A bare `rollback` (or a scoped one, for its scope) restores the SYSTEM to unpatc 3. **Vendored leg** — each in-scope ledger entry (embedded-record entries included) is reverted through the vendor backends: lockfile wiring restored, artifact dir deleted (and its emptied `.socket/vendor//` husk pruned, v5.0), ledger entry dropped + persisted per purl (crash-consistent, like `vendor --revert`). A **drift-keep** (the backend refused a drifted lock) keeps the entry, the artifact, AND the manifest record (`vendoredKept`, exit 1 — the system is still patched); a failure is recorded and other entries proceed. 4. **Hosted leg** — each in-scope hosted pin is restored to its default upstream registry entry; see "Hosted unwind coverage" below. After a hosted leg with no failure, a wet run deletes a pre-v5 `redirect-state.json` once no lockfile pins a hosted patch any more (a failed delete is the `legacy_redirect_ledger_kept` warning). 5. **Manifest cleanup** — entries are removed ONLY for in-scope purls whose legs fully succeeded, were not-installed, or were release-variant siblings narrowed away by an attempted variant that succeeded (half a variant group never lingers — `remove` parity); drift-kept and failed purls keep their records, and a failed variant holds its whole group. No-op removals never rewrite the file. A failed write surfaces as `manifest_write_failed` (warning + `partial_failure` exit 1; GC still runs against the unchanged manifest). -6. **GC** — `cleanup_unused_blobs` + diff/package-archive sweeps against the post-removal manifest, with beforeHash blobs pinned (synthetic afterHash-slot records) for (a) removed-but-not-installed entries (a crawler miss must not destroy the only local revert data — `remove` parity) and (b) EVERY entry remaining in the post-removal manifest — still-active patches (failed, drift-kept, eco-/path-excluded) keep their revert data, so a scoped or failed run never destroys the blobs a later rollback needs; only blobs referenced solely by genuinely-removed entries are swept. GC errors warn (`cleanup_failed`) and continue — they never affect the exit (repair's posture). +6. **GC** — blob, diff and legacy package-archive sweeps against the post-removal manifest, using the same artifact-reference policy as `remove`, retaining beforeHash blobs for (a) removed-but-not-installed entries (a crawler miss must not destroy the only local revert data — `remove` parity) and (b) EVERY entry remaining in the post-removal manifest — still-active patches (failed, drift-kept, eco-/path-excluded) keep their revert data, so a scoped or failed run never destroys the blobs a later rollback needs; only blobs referenced solely by genuinely-removed entries are swept. GC errors warn (`cleanup_failed`) and continue — they never affect the exit (repair's posture). **Confirmation prompt.** A wet, non-preserve run with work prompts once, remove-style, composing only the clauses that apply into one English list (`a and b`, `a, b, and c`) with counted nouns: `Roll back N patches`, `remove them from the local manifest`, `delete M vendored artifacts and their ledger records`, `restore H hosted packages to the upstream registry` (e.g. `Roll back 1 patch, remove it from the local manifest, and restore 1 hosted package to the upstream registry?`) — default yes, auto-accepted under `--yes`/`--json`/non-TTY (the shared `confirm` semantics; CI unaffected). Decline prints `Rollback cancelled.` and exits 0. `--dry-run` and `--preserve-state` runs are prompt-free (they delete no local state). diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 0d4be4bb2..84f87a7c9 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -1,6 +1,6 @@ use clap::Args; use socket_patch_core::api::client::get_api_client_with_overrides; -use socket_patch_core::manifest::cleanup_blobs::format_bytes; +use socket_patch_core::manifest::cleanup_blobs::{format_bytes, ArtifactReferences}; use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::patch::redirect::upstream::HostedPin; @@ -14,8 +14,7 @@ use std::time::Duration; use super::get::short_uuid; use super::rollback::{ - pin_before_hash_blobs, rollback_patches_inner, run_hosted_leg, sweep_failure, - sweep_unused_artifacts, HostedLegOutcome, InnerSelection, + rollback_patches_inner, run_hosted_leg, sweep_failure, HostedLegOutcome, InnerSelection, }; use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend}; use crate::args::{apply_env_toggles, GlobalArgs}; @@ -935,24 +934,15 @@ pub async fn run(args: RemoveArgs) -> i32 { } // ── GC ────────────────────────────────────────────────────────────── - // Clean up unused blobs (previewed, not deleted, on --dry-run). The - // reference manifest is the post-removal manifest PLUS one synthetic - // keep record per retained entry above: `cleanup_unused_blobs` keeps - // only afterHash blobs (beforeHash blobs are normally re-downloadable - // on demand), so each pinned before-hash is listed in an afterHash - // slot. Scoped to REVERT data only — the retained entries' real - // afterHash blobs stay sweepable like any other orphan. - let mut cleanup_reference = updated_manifest; - let pinned_purls: Vec = retained_not_installed - .iter() - .map(|p| (*p).to_string()) - .collect(); - pin_before_hash_blobs(&mut cleanup_reference, &manifest, pinned_purls.iter()); + let references = ArtifactReferences::after_removal( + &manifest, + &updated_manifest, + retained_not_installed.iter().copied(), + ); let mut blobs_removed = 0; let mut archives_removed = 0; if !args.preserve_state { - let sweep = - sweep_unused_artifacts(&cleanup_reference, &socket_dir, args.common.dry_run).await; + let sweep = references.sweep(&socket_dir, args.common.dry_run).await; // repair's posture: a failed pass (or a pass that could not unlink // every orphan) warns and continues, never fatal; its partial // counts still stand. diff --git a/crates/socket-patch-cli/src/commands/repair.rs b/crates/socket-patch-cli/src/commands/repair.rs index 323b447b8..de049be4d 100644 --- a/crates/socket-patch-cli/src/commands/repair.rs +++ b/crates/socket-patch-cli/src/commands/repair.rs @@ -5,7 +5,7 @@ use socket_patch_core::api::blob_fetcher::{ }; use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient}; use socket_patch_core::manifest::cleanup_blobs::{ - format_all_in_use, format_cleanup_result_for, CleanupResult, + format_all_in_use, format_cleanup_result_for, ArtifactReferences, CleanupResult, }; use socket_patch_core::manifest::operations::read_manifest; use socket_patch_core::patch::apply::PatchSources; @@ -16,7 +16,7 @@ use std::time::Duration; use crate::args::{apply_env_toggles, parse_bool_flag, GlobalArgs}; use crate::commands::fetch_stage::files_diffs_cannot_cover; use crate::commands::lock_cli::{acquire_or_emit, error_envelope}; -use crate::commands::rollback::{sweep_failure, sweep_unused_artifacts}; +use crate::commands::rollback::sweep_failure; use crate::json_envelope::{Command, Envelope, PatchAction, PatchEvent, Status}; #[derive(Args)] @@ -627,7 +627,9 @@ async fn repair_inner( // summary prints once all three passes are in, so "nothing to clean // up" is only said when all three really are empty. if let (false, Some(manifest)) = (args.download_only, manifest.as_ref()) { - let sweep = sweep_unused_artifacts(manifest, &socket_dir, args.common.dry_run).await; + let sweep = ArtifactReferences::for_apply(manifest) + .sweep(&socket_dir, args.common.dry_run) + .await; let passes = [ ("blob", BLOB, sweep.blobs), ("diff", DIFF_ARCHIVE, sweep.diffs), diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 5f4191038..e9583fa8a 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2,9 +2,7 @@ use clap::Args; use socket_patch_core::api::blob_fetcher::{fetch_blobs_by_hash, format_fetch_result}; use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient}; use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem}; -use socket_patch_core::manifest::cleanup_blobs::{ - cleanup_unused_archives, cleanup_unused_blobs, CleanupResult, -}; +use socket_patch_core::manifest::cleanup_blobs::{ArtifactReferences, CleanupResult}; use socket_patch_core::manifest::operations::{ get_before_hash_blobs, read_manifest, write_manifest, }; @@ -31,54 +29,6 @@ use crate::json_envelope::Command as EnvelopeCommand; use crate::looks_like_uuid; use crate::ui::{plural, StatusLine}; -/// Pin the beforeHash blobs of `purls` into `reference` as synthetic keep -/// records: `cleanup_unused_blobs` keeps only afterHash blobs (beforeHash -/// blobs are normally re-downloadable on demand), so each pinned -/// before-hash is listed in an afterHash slot. Scoped to REVERT data only -/// — the pinned entries' real afterHash blobs stay sweepable like any -/// other orphan. Shared by rollback's default GC and `remove`'s -/// crawler-miss guard. -pub(crate) fn pin_before_hash_blobs<'a>( - reference: &mut PatchManifest, - source: &PatchManifest, - purls: impl IntoIterator, -) { - for purl in purls { - let Some(record) = source.patches.get(purl) else { - continue; - }; - let pinned: HashMap = record - .files - .iter() - .filter(|(_, info)| !info.before_hash.is_empty()) - .map(|(file, info)| { - // A synthetic key so pins never clobber the real file rows - // of an entry that REMAINS in the reference (whose afterHash - // blobs must stay kept). The sweep reads only the hash - // VALUES, never the keys, and this reference manifest is - // in-memory only. - ( - format!("{file}#beforeHash-pin"), - PatchFileInfo { - before_hash: String::new(), - after_hash: info.before_hash.clone(), - }, - ) - }) - .collect(); - if pinned.is_empty() { - continue; // every file was created-by-patch: no revert blobs - } - if let Some(existing) = reference.patches.get_mut(purl) { - existing.files.extend(pinned); - } else { - let mut keep_record = record.clone(); - keep_record.files = pinned; - reference.patches.insert(purl.clone(), keep_record); - } - } -} - #[derive(Args)] pub struct RollbackArgs { /// What to roll back: a package PURL, a patch UUID, or a path glob @@ -812,38 +762,6 @@ pub(crate) struct HostedLegOutcome { pub(crate) edited_files: std::collections::BTreeSet, } -/// One GC pass over `.socket/blobs`, `diffs` and `packages` against -/// `reference` (the post-removal manifest with the revert blobs a later -/// rollback needs pinned in). Each directory reports separately: callers -/// own the warn-and-continue posture and the wording. The core sweep -/// removes an emptied directory, so a fully reverted project keeps none -/// of the three. Shared by rollback's GC, remove's post-removal sweep and -/// repair's cleanup phase. -pub(crate) struct ArtifactSweep { - pub(crate) blobs: std::io::Result, - pub(crate) diffs: std::io::Result, - pub(crate) packages: std::io::Result, -} - -pub(crate) async fn sweep_unused_artifacts( - reference: &PatchManifest, - socket_dir: &Path, - dry_run: bool, -) -> ArtifactSweep { - ArtifactSweep { - blobs: cleanup_unused_blobs(reference, &socket_dir.join("blobs"), dry_run).await, - diffs: cleanup_unused_archives(reference, &socket_dir.join("diffs"), dry_run).await, - // Nothing writes or reads `.socket/packages/` any more; sweep the - // leftover directory whole. - packages: cleanup_unused_archives( - &PatchManifest::default(), - &socket_dir.join("packages"), - dry_run, - ) - .await, - } -} - /// The `cleanup_failed` detail for one sweep pass labelled `label`: the /// directory-level error that stopped the pass, or — after a pass that /// kept sweeping past unlink failures — the files it could not remove @@ -1661,35 +1579,19 @@ pub async fn run(args: RollbackArgs) -> i32 { } // ── GC ─────────────────────────────────────────────────────── - // Sweep against the post-removal manifest, with beforeHash - // blobs pinned (synthetic afterHash-slot records — the sweep - // keeps only afterHash blobs) for (a) removed-but-not-installed - // entries — a crawler miss must not destroy the only local - // revert data — and (b) in-scope entries that FAILED this run: - // their entries stay, and the blobs the gate just downloaded - // must survive for an offline retry. + // Removal retains originals for active patches and crawler misses. let mut gc_json: serde_json::Value = serde_json::json!({ "skipped": true }); let mut gc_bytes_freed: u64 = 0; if cleanup_allowed { - // Pin the beforeHash blobs of EVERY entry remaining in the - // manifest (still-active patches keep their revert data — - // an eco-scoped or failed run must never destroy the blobs - // a later rollback needs) plus removed-but-not-installed - // entries (remove's crawler-miss guard). Blobs referenced - // only by genuinely-removed entries are what gets swept. - let pinned_purls: Vec = removed - .iter() - .filter(|p| not_installed.contains(p)) - .chain(updated_manifest.patches.keys()) - .cloned() - .collect(); - // The post-removal manifest is not needed past the sweep: - // it becomes the (pin-augmented) reference in place. - let mut cleanup_reference = updated_manifest; - pin_before_hash_blobs(&mut cleanup_reference, &manifest, pinned_purls.iter()); - let sweep = - sweep_unused_artifacts(&cleanup_reference, &socket_dir, args.common.dry_run) - .await; + let references = ArtifactReferences::after_removal( + &manifest, + &updated_manifest, + removed + .iter() + .filter(|p| not_installed.contains(p)) + .map(String::as_str), + ); + let sweep = references.sweep(&socket_dir, args.common.dry_run).await; let mut removed_counts = [0usize; 3]; for (slot, (label, result)) in removed_counts.iter_mut().zip([ ("blob", sweep.blobs), @@ -4418,60 +4320,6 @@ mod tests { ); } - /// A purl absent from the source manifest contributes nothing to the - /// GC reference: the pin loop skips it (the lookup-miss `continue`) - /// rather than inserting an empty synthetic keep record, and present - /// purls around it still pin normally. - #[test] - fn pin_before_hash_blobs_skips_purls_absent_from_source() { - let mut present = make_record("uuid-present"); - present.files.insert( - "package/index.js".to_string(), - PatchFileInfo { - before_hash: "beefbeef".to_string(), - after_hash: "cafecafe".to_string(), - }, - ); - let mut source = PatchManifest { - patches: HashMap::new(), - setup: None, - }; - source - .patches - .insert("pkg:npm/present@1.0.0".to_string(), present); - - let mut reference = PatchManifest { - patches: HashMap::new(), - setup: None, - }; - let purls = [ - "pkg:npm/ghost@9.9.9".to_string(), - "pkg:npm/present@1.0.0".to_string(), - ]; - pin_before_hash_blobs(&mut reference, &source, purls.iter()); - - assert!( - !reference.patches.contains_key("pkg:npm/ghost@9.9.9"), - "a purl the source manifest does not hold must not grow a \ - synthetic record, got {:?}", - reference.patches.keys().collect::>() - ); - let pinned = reference - .patches - .get("pkg:npm/present@1.0.0") - .expect("the present purl must still pin"); - assert_eq!(pinned.files.len(), 1, "got {:?}", pinned.files); - assert_eq!( - pinned - .files - .get("package/index.js#beforeHash-pin") - .expect("synthetic pin key") - .after_hash, - "beefbeef", - "the beforeHash must be pinned in an afterHash slot" - ); - } - /// The vendored leg tolerates a key with no ledger entry: the scope /// resolver guarantees keys exist, but a divergent ledger must skip /// the key silently (the lookup-miss `continue`) rather than panic or diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 14cd5486f..cd0bf9db5 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -2,7 +2,7 @@ //! orphan blob/diff/package-archive sweeps, in both mutating (apply) and //! read-only (preview) forms. -use socket_patch_core::manifest::cleanup_blobs::CleanupResult; +use socket_patch_core::manifest::cleanup_blobs::{ArtifactReferences, CleanupResult}; use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::utils::composer_version::purl_identity_key; @@ -14,7 +14,7 @@ use std::time::Duration; use crate::args::GlobalArgs; use crate::commands::lock_cli::lock_failure; -use crate::commands::rollback::{sweep_failure, sweep_unused_artifacts}; +use crate::commands::rollback::sweep_failure; use crate::commands::vendor::{run_vendor_gc, VendorGcSummary}; /// Aggregated outcome of a GC pass (or preview). Serialized into the @@ -145,7 +145,7 @@ impl GcSummary { /// The orphan blob/diff/package sweep against the (post-prune) manifest. /// `dry_run = true` for the preview path; `dry_run = false` for the apply -/// path — the shared `sweep_unused_artifacts` natively supports dry-run, so +/// path — `ArtifactReferences::sweep` natively supports dry-run, so /// the same function works for both. A pass that failed outright counts /// as empty; that failure (or a wet pass's unremovable orphans) rides /// `warnings` as `cleanup_failed` so the output never reads as a clean @@ -156,7 +156,9 @@ async fn run_gc( socket_dir: &Path, dry_run: bool, ) -> GcSummary { - let sweep = sweep_unused_artifacts(manifest, socket_dir, dry_run).await; + let sweep = ArtifactReferences::for_apply(manifest) + .sweep(socket_dir, dry_run) + .await; let mut warnings = Vec::new(); let mut take = |label: &str, pass: std::io::Result| { if let Some(detail) = sweep_failure(label, &pass) { diff --git a/crates/socket-patch-cli/tests/remove/remove_duality_invariants.rs b/crates/socket-patch-cli/tests/remove/remove_duality_invariants.rs index 8c147f1cd..b5910cb11 100644 --- a/crates/socket-patch-cli/tests/remove/remove_duality_invariants.rs +++ b/crates/socket-patch-cli/tests/remove/remove_duality_invariants.rs @@ -107,6 +107,75 @@ fn make_preserve_fixture(root: &Path) -> (PathBuf, String, String) { (socket, before_hash, after_hash) } +/// Removing one patch must not collect another active patch's only local +/// rollback data (#559). Exercise both commands against the same lifecycle. +#[test] +fn scoped_removal_preserves_other_patches_for_offline_rollback() { + for (command, skip_rollback) in [("remove", false), ("remove", true), ("rollback", false)] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let (socket, before_hash, after_hash) = make_preserve_fixture(root); + let other_purl = "pkg:npm/other-patch@1.0.0"; + let original = b"other original\n"; + let patched = b"other patched\n"; + let other_before = common::git_sha256(original); + let other_after = common::git_sha256(patched); + let mut manifest = read_manifest(&socket); + let mut record = manifest["patches"][PRESERVE_PURL].clone(); + record["uuid"] = serde_json::json!("88888888-8888-4888-8888-888888888888"); + record["files"]["package/a.js"] = serde_json::json!({ + "beforeHash": other_before, "afterHash": other_after, + }); + manifest["patches"][other_purl] = record.clone(); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_vec(&manifest).unwrap(), + ) + .unwrap(); + let package = root.join("node_modules/other-patch"); + std::fs::create_dir_all(&package).unwrap(); + std::fs::write( + package.join("package.json"), + r#"{"name":"other-patch","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(package.join("a.js"), patched).unwrap(); + std::fs::write(socket.join("blobs").join(&other_before), original).unwrap(); + std::fs::write(socket.join("blobs").join(&other_after), patched).unwrap(); + + let mut args = vec![command, PRESERVE_PURL, "--json", "--yes", "--offline"]; + if skip_rollback { + args.push("--skip-rollback"); + } + let (code, stdout, stderr) = common::run_with_env(root, &args, &[]); + assert_eq!(code, 0, "{command}: {stdout}\n{stderr}"); + let remaining = read_manifest(&socket); + assert_eq!(remaining["patches"].as_object().unwrap().len(), 1); + assert_eq!(remaining["patches"][other_purl], record); + assert_eq!(std::fs::read(package.join("a.js")).unwrap(), patched); + assert!(!socket.join("blobs").join(&before_hash).exists()); + assert!(!socket.join("blobs").join(&after_hash).exists()); + assert!(socket.join("blobs").join(&other_after).exists()); + assert!( + socket.join("blobs").join(&other_before).exists(), + "{command} swept the remaining patch's rollback data" + ); + + let (code, stdout, stderr) = + common::run_with_env(root, &["rollback", other_purl, "--json", "--offline"], &[]); + assert_eq!( + code, 0, + "offline rollback after {command}: {stdout}\n{stderr}" + ); + assert_eq!(std::fs::read(package.join("a.js")).unwrap(), original); + assert!(read_manifest(&socket)["patches"] + .as_object() + .unwrap() + .is_empty()); + assert!(!socket.join("blobs").exists()); + } +} + /// `remove --preserve-state` on an installed, patched package must restore /// the file to its ORIGINAL bytes (the rollback half still runs) while /// keeping ALL local state: the manifest entry survives byte-for-byte, both diff --git a/crates/socket-patch-core/src/manifest/cleanup_blobs.rs b/crates/socket-patch-core/src/manifest/cleanup_blobs.rs index de3a3e745..1f003c937 100644 --- a/crates/socket-patch-core/src/manifest/cleanup_blobs.rs +++ b/crates/socket-patch-core/src/manifest/cleanup_blobs.rs @@ -19,6 +19,73 @@ pub struct CleanupResult { pub failed: Vec, } +/// The blob hashes and patch archives a cleanup pass must preserve. +/// These are references, not synthetic patch records: filenames and patch +/// metadata cannot change which original or patched bytes remain reachable. +pub struct ArtifactReferences { + blobs: HashSet, + patch_uuids: HashSet, +} + +impl ArtifactReferences { + /// Repair and pruning retain the bytes needed to apply active patches. + pub fn for_apply(manifest: &PatchManifest) -> Self { + Self { + blobs: get_after_hash_blobs(manifest), + patch_uuids: manifest.patches.values().map(|r| r.uuid.clone()).collect(), + } + } + + /// Remove and rollback also retain originals for every remaining patch + /// and removed-but-not-installed patch. A crawler miss must not destroy + /// the only local restore data. Other removed patches become collectible. + pub fn after_removal<'a>( + previous: &PatchManifest, + remaining: &PatchManifest, + removed_not_installed: impl IntoIterator, + ) -> Self { + let mut references = Self::for_apply(remaining); + for record in remaining.patches.values().chain( + removed_not_installed + .into_iter() + .filter_map(|purl| previous.patches.get(purl)), + ) { + let mut has_original = false; + for file in record.files.values() { + if !file.before_hash.is_empty() { + references.blobs.insert(file.before_hash.clone()); + has_original = true; + } + } + if has_original { + references.patch_uuids.insert(record.uuid.clone()); + } + } + references + } + + /// Sweep each artifact directory independently so a failed pass does not + /// stop another. Callers report partial counts and cleanup warnings. + pub async fn sweep(&self, socket_dir: &Path, dry_run: bool) -> ArtifactSweep { + ArtifactSweep { + blobs: cleanup_dir(&socket_dir.join("blobs"), dry_run, |name| { + self.blobs.contains(name) + }) + .await, + diffs: cleanup_archives(&self.patch_uuids, &socket_dir.join("diffs"), dry_run).await, + // Nothing writes or reads legacy package archives any more. + packages: cleanup_dir(&socket_dir.join("packages"), dry_run, |_| false).await, + } + } +} + +/// Results from the independent blob, diff and legacy package sweeps. +pub struct ArtifactSweep { + pub blobs: std::io::Result, + pub diffs: std::io::Result, + pub packages: std::io::Result, +} + /// Shared core for `cleanup_unused_blobs` / `cleanup_unused_archives`. /// /// Walks `dir`, treats it as authoritative socket-patch state (so any @@ -134,6 +201,14 @@ pub async fn cleanup_unused_archives( dry_run: bool, ) -> Result { let used_uuids: HashSet = manifest.patches.values().map(|r| r.uuid.clone()).collect(); + cleanup_archives(&used_uuids, archives_dir, dry_run).await +} + +async fn cleanup_archives( + used_uuids: &HashSet, + archives_dir: &Path, + dry_run: bool, +) -> Result { cleanup_dir(archives_dir, dry_run, |name| { // Strip the .tar.gz suffix to recover the UUID. A file that does // not end in .tar.gz is never a valid archive, so it is always an @@ -279,6 +354,104 @@ mod tests { } } + #[tokio::test] + async fn artifact_retention_covers_active_removed_and_uninstalled_patches() { + for policy in [ + "apply", + "remaining", + "not-installed", + "created-only", + "removed", + ] { + let dir = tempfile::tempdir().unwrap(); + let mut manifest = create_test_manifest(); + let purl = "pkg:npm/pkg-a@1.0.0"; + let record = manifest.patches.get_mut(purl).unwrap(); + // This is a real filename. Synthetic beforeHash-pin records used + // to overwrite its afterHash, making its patched bytes collectible. + record.files.insert( + "package/index.js#beforeHash-pin".into(), + PatchFileInfo { + before_hash: String::new(), + after_hash: "created-file".into(), + }, + ); + if policy == "created-only" { + for file in record.files.values_mut() { + file.before_hash.clear(); + } + } + let empty = PatchManifest::default(); + let references = match policy { + "apply" => ArtifactReferences::for_apply(&manifest), + "remaining" => ArtifactReferences::after_removal(&manifest, &manifest, []), + "removed" => ArtifactReferences::after_removal(&manifest, &empty, []), + _ => ArtifactReferences::after_removal(&manifest, &empty, ["missing-purl", purl]), + }; + let blobs = dir.path().join("blobs"); + let diffs = dir.path().join("diffs"); + let packages = dir.path().join("packages"); + for path in [&blobs, &diffs, &packages] { + std::fs::create_dir(path).unwrap(); + } + let hashes = [ + BEFORE_HASH_1, + BEFORE_HASH_2, + AFTER_HASH_1, + AFTER_HASH_2, + "created-file", + ORPHAN_HASH, + ]; + for hash in hashes { + std::fs::write(blobs.join(hash), b"blob").unwrap(); + } + let archive = format!("{TEST_UUID}.tar.gz"); + std::fs::write(diffs.join(&archive), b"diff").unwrap(); + std::fs::write(diffs.join("orphan.tar.gz"), b"orphan").unwrap(); + std::fs::write(packages.join(&archive), b"legacy").unwrap(); + let keep_original = matches!(policy, "remaining" | "not-installed"); + let keep_patched = matches!(policy, "apply" | "remaining"); + let kept = 2 * usize::from(keep_original) + 3 * usize::from(keep_patched); + let keep_archive = keep_original || keep_patched; + + let preview = references.sweep(dir.path(), true).await; + assert_eq!( + preview.blobs.unwrap().blobs_removed, + hashes.len() - kept, + "{policy}" + ); + assert_eq!( + preview.diffs.unwrap().blobs_removed, + 2 - usize::from(keep_archive) + ); + assert_eq!(preview.packages.unwrap().blobs_removed, 1); + assert_eq!(std::fs::read_dir(&blobs).unwrap().count(), hashes.len()); + assert_eq!(std::fs::read_dir(&diffs).unwrap().count(), 2); + assert!(packages.join(&archive).exists()); + + let swept = references.sweep(dir.path(), false).await; + assert_eq!( + swept.blobs.unwrap().blobs_removed, + hashes.len() - kept, + "{policy}" + ); + assert_eq!( + swept.diffs.unwrap().blobs_removed, + 2 - usize::from(keep_archive) + ); + assert_eq!(swept.packages.unwrap().blobs_removed, 1); + for hash in [BEFORE_HASH_1, BEFORE_HASH_2] { + assert_eq!(blobs.join(hash).exists(), keep_original, "{policy}"); + } + for hash in [AFTER_HASH_1, AFTER_HASH_2, "created-file"] { + assert_eq!(blobs.join(hash).exists(), keep_patched, "{policy}"); + } + assert!(!blobs.join(ORPHAN_HASH).exists()); + assert_eq!(diffs.join(archive).exists(), keep_archive, "{policy}"); + assert!(!packages.exists()); + } + } + #[tokio::test] async fn test_cleanup_keeps_after_hash_removes_orphan() { let dir = tempfile::tempdir().unwrap();