diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 7bea75a7c..8b2fbf484 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1139,7 +1139,7 @@ A bare `rollback` (or a scoped one, for its scope) restores the SYSTEM to unpatc 1. **State discovery.** A missing manifest is no longer fatal when the vendor ledger or the lockfiles' hosted pins hold work (`rollback` runs manifest-less on hosted-only / vendored projects — every `scan`/`get --mode vendored` and v5 `scan --mode hosted` project is manifest-less). The **truly-empty** project — no manifest, no vendor ledger, no hosted pin — keeps the "Manifest not found" exit 1 (JSON: the error envelope with `error.code: "manifest_not_found"` plus a top-level `path`), with one v5.0 exception: when a pre-v5 `.socket/vendor/redirect-state.json` is the only thing left, nothing pins it any more, so a wet run deletes it and exits 0 (human `Removed the pre-v5 hosted ledger .socket/vendor/redirect-state.json: no lockfile pins a hosted patch.`, `Would remove …` on `--dry-run`, which deletes nothing; JSON: a `success` envelope with no events plus `legacyRedirectLedgerRemoved` (bool); a failed delete is the `legacy_redirect_ledger_kept` warning, still exit 0). A project whose lockfiles still reference `.socket/vendor/` artifacts but whose vendor ledger is missing errors asking for `.socket/vendor/state.json` to be restored from version control first (v5.0: `repair` no longer reconstructs the ledger). **Corrupt-ledger containment**: an unreadable vendor ledger fails ONLY the legs that need it — the vendored leg, manifest cleanup, and GC are skipped fail-closed (`vendor_state_unreadable` warning) while the agent and hosted legs still run; it drives `partialFailure` exit 1, and an emergency restore is never blocked by it. When the ONLY state on disk is an unreadable vendor ledger, the run fails closed naming the store. A pre-v5 redirect ledger is never read by rollback (v4's `redirect_state_unreadable` is no longer emitted). Under `--global`/`--global-prefix` the project's hosted pins and vendor ledger are not discovered at all, so the vendored and hosted legs below do not run (see "Global scope never touches the project's state"). 2. **Agent leg** — the existing in-place restore machinery, unchanged (v5.0 presentation: the human `No patches found in manifest` line prints only for an unscoped run with no work in ANY leg — a run whose work is all vendored/hosted stays quiet about the manifest): multi-copy restore, release-variant narrowing, the before-blob gate (+ on-demand download; a gate abort still exits 1 with per-package `missing_blob` failure results **and** skips manifest cleanup + GC entirely — nothing was restored, and the retry's revert data must survive), local-go redirect drop, and the `not_installed` exit-0 asymmetry verbatim. Vendor-owned purls are still excluded here (see the vendored-mode section) — they are handled by the next leg instead of being punted to other commands — with one exception: a vendored Cargo crate whose copy in the shared `$CARGO_HOME/registry/src` cache (or a `cargo vendor` dir) still carries an earlier agent-mode patch is restored here too (vendoring never touches that copy, and the manifest record holds the only blobs that can restore it); its record leaves the manifest only when both this leg and the vendored leg succeeded. `remove` restores it the same way. -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 (a `failed` `vendor_revert_kept` event, exit 1 — the system is still patched); a failure is recorded and other entries proceed. +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 (a `failed` `vendor_revert_kept` event, exit 1 — the system is still patched); a failure is recorded and other entries proceed. Then, on a run with work in any leg, uuid dirs under `.socket/vendor//` that no ledger entry owns (the losing side of a git merge of two vendor runs, an interrupted run) are swept as `vendor --revert` sweeps them: every one in the `--ecosystems` scope on an unscoped run, only those whose leaf names a targeted package (or whose uuid is a targeted uuid) on a scoped one; path-glob targets sweep none. The confirmation prompt counts them (`delete N orphaned vendored dirs with no ledger record`). A dir a lockfile still points into is kept with a `vendor_orphan_still_wired` warning, and one that cannot be deleted is a `cleanup_failed` warning. Skipped under `--preserve-state`, global scope, an unreadable ledger and a before-blob gate abort. `remove ` sweeps the orphans its target names the same way, once its vendored revert and manifest write have succeeded; they are `removed` `vendor_orphan_removed` events that `summary.removed` does not count, and the kept or undeletable ones `skipped` advisories (#1610). 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. A manifest record that a live hosted pin has superseded (the lockfile wires the same package release to a different patch uuid, e.g. an agent → hosted migration after the patch was replaced) and whose installed copy holds neither side of the record is not restored in place and does not fail the run: the hosted leg's lock restore and the next install unwind it, and the record leaves the manifest with the `rollback_record_superseded` warning (`remove` does the same instead of aborting before its hosted leg). A copy still holding the record's patched bytes is restored as usual. No-op removals never rewrite the file. A failed write surfaces as `manifest_write_failed` (warning + `partialFailure` exit 1; GC still runs against the unchanged manifest). 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). @@ -1185,11 +1185,12 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem | Vendored leg: reverted / preserved (`--preserve-state`) | `rolledBack` (`verified` on `--dry-run`), `details.mode: "vendored"`; preserved adds `details.preserved: true` | | Vendored leg: drift-keep | `failed` `vendor_revert_kept`, `details.mode: "vendored"` (exit 1: the system is still patched) | | Vendored leg: revert failed | `failed` `vendor_revert_failed` (`vendor_state_write_failed` when the ledger save failed), `details.mode: "vendored"` | +| Vendored leg: orphan uuid dir swept | `removed` `vendor_orphan_removed` (also on `--dry-run`), `details.mode: "vendored"`; `purl` is the package its leaf names, else `/` | | Hosted leg: pin restored | `rolledBack` (`verified` on `--dry-run`), `details.mode: "hosted"` | | Hosted leg: pin refused / write failed / contested wiring | `failed` `hosted_restore_refused` / artifact-level `hosted_write_failed` / artifact-level `hosted_wiring_contested`, `details.mode: "hosted"` | | Manifest entry dropped | `removed` (`verified` on `--dry-run`) with `details.manifest: true` | -Events carry the patch `uuid` when one is recorded. A package wired through two legs gets one event per leg. Cargo's `.cargo-checksum.json` resync rides the envelope's `sidecars[]`. Rollback's own top-level keys: `hosted: {editedFiles: N}` (distinct files the hosted leg rewrote) and `paths` (the path-glob targets verbatim, `[]` when none); `path` on `manifest_not_found` and `legacyRedirectLedgerRemoved` on the pre-v5-ledger exit. `gc` is the shared GC object (via `summary.bytesFreed` too); it is absent under `--preserve-state`, and absent with a `gc_skipped` warning after a before-blob gate abort or under a corrupt vendor ledger. `warnings[]` (omitted when empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `gc_skipped`, `legacy_redirect_ledger_kept`, `ownership_not_restored`, `rollback_record_superseded`, `gradle_m2_copy_not_restored`, `cargo_build_cache_stale` (see the apply warnings), the upstream-restore advisories (including `hosted_resolution_orphaned`) and the vendored/hosted leg advisories. +Events carry the patch `uuid` when one is recorded. A package wired through two legs gets one event per leg. Cargo's `.cargo-checksum.json` resync rides the envelope's `sidecars[]`. Rollback's own top-level keys: `hosted: {editedFiles: N}` (distinct files the hosted leg rewrote) and `paths` (the path-glob targets verbatim, `[]` when none); `path` on `manifest_not_found` and `legacyRedirectLedgerRemoved` on the pre-v5-ledger exit. `gc` is the shared GC object (via `summary.bytesFreed` too); it is absent under `--preserve-state`, and absent with a `gc_skipped` warning after a before-blob gate abort or under a corrupt vendor ledger. `warnings[]` (omitted when empty): `reinstall_required`, `hosted_state_not_preservable`, `out_of_scope_copies_restored`, `vendor_state_unreadable`, `cleanup_failed`, `manifest_write_failed`, `gc_skipped`, `legacy_redirect_ledger_kept`, `ownership_not_restored`, `rollback_record_superseded`, `gradle_m2_copy_not_restored`, `cargo_build_cache_stale` (see the apply warnings), `vendor_orphan_still_wired`, the upstream-restore advisories (including `hosted_resolution_orphaned`) and the vendored/hosted leg advisories. **Status and exit**: `success` (exit 0) unless something leaves the system still patched — an agent failure, a vendored drift-keep or failure, a hosted refusal, contested wiring under an unscoped run, a corrupt vendor ledger, a failed manifest write — which is `partialFailure` (exit 1). When something failed and nothing was rolled back, previewed, already original or not installed, the run failed as a whole: `status: "error"` with `error: {code: "rollback_failed"}` (exit 1; the `failed` events stay). Not-installed entries never flip the exit (the apply/rollback asymmetry). GC failures never affect the exit. A dry run's would-be refusals stay `failed` events (not `skipped`): a dry run that cannot roll something back exits 1, as before. Every early error (`manifest_not_found`, `manifest_invalid`, `manifest_unreadable`, `patch_not_found`, `ambiguous_target`, `path_glob_no_match`, `hosted_wiring_contested`, `vendor_ledger_missing`, `lock_held` / `lock_io`, the catch-all `rollback_failed`, and the exit-2 usage error `path_glob_invalid`) is a full envelope with empty `events`. diff --git a/crates/socket-patch-cli/src/commands/mod.rs b/crates/socket-patch-cli/src/commands/mod.rs index daf3d3454..96f4037cf 100644 --- a/crates/socket-patch-cli/src/commands/mod.rs +++ b/crates/socket-patch-cli/src/commands/mod.rs @@ -16,6 +16,7 @@ pub mod rollback; pub mod scan; pub mod update; pub mod vendor; +pub(crate) mod vendor_orphans; pub(crate) mod vendored_backend; pub mod vex; pub(crate) mod vex_consumed; diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 9036a0d73..3965a9ee2 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -515,7 +515,7 @@ pub async fn run(args: RemoveArgs) -> i32 { if let Ok(state) = vendor_state_result { let ledger_matches = vendor_entries_matching(&state, None, &target); if !ledger_matches.is_empty() { - return remove_ledger_only(&args, ledger_matches, state, &telemetry).await; + return remove_ledger_only(&args, &target, ledger_matches, state, &telemetry).await; } } @@ -916,6 +916,8 @@ pub async fn run(args: RemoveArgs) -> i32 { return 1; } } + sweep_target_orphans(&args, &vendor_state, &target, &mut vendor_leg).await; + printed_progress |= loud && vendor_leg.printed; if loud { if printed_progress { @@ -1117,6 +1119,7 @@ pub async fn run(args: RemoveArgs) -> i32 { use crate::commands::vendored_backend::tag_event_mode; use crate::commands::{HOSTED_MODE_LABEL, VENDORED_MODE_LABEL}; let mut vendor_reverted = vendor_leg.reverted; + vendor_reverted.extend(vendor_leg.orphans); tag_event_mode(&mut vendor_reverted, VENDORED_MODE_LABEL); env.events.extend(vendor_reverted); // Hosted unwinds likewise bypass `record` — summary.removed stays @@ -1257,6 +1260,10 @@ struct RemoveVendorLeg { kept: Vec, /// Why each [`Self::kept`] entry was kept (same order). kept_causes: Vec, + /// `Removed`/`vendor_orphan_removed` events (dry runs too, as in + /// `vendor --revert`): uuid dirs of the reverted packages that no + /// ledger entry owns, swept after the reverts (#1610). + orphans: Vec, /// Entries actually reverted and dropped from the ledger (wet runs). reverted_count: usize, /// Entries unwired with their artifact and ledger entry kept @@ -1412,6 +1419,60 @@ async fn revert_vendored_matches( Ok(leg) } +/// Sweep the uuid dirs under `.socket/vendor/` that no ledger entry owns +/// and `target` names (a git merge of two vendor runs keeps both sides' +/// dirs, #1610), as `vendor --revert` does: into `leg.orphans`, with a +/// `skipped` advisory for each one a lockfile still points into (kept) or +/// that could not be deleted. Runs only after a vendored revert, past the +/// remove's last abort point. `--preserve-state` keeps local state, +/// orphans included. +async fn sweep_target_orphans( + args: &RemoveArgs, + state: &VendorState, + target: &Target, + leg: &mut RemoveVendorLeg, +) { + use crate::commands::vendor_orphans::{ + orphan_in_target, orphan_label, orphan_removal_failed, orphan_removed_event, + orphan_still_wired, print_orphan_sweep, sweep_orphan_vendor_dirs_where, + }; + if args.preserve_state || leg.reverted.is_empty() { + return; + } + let sweep = sweep_orphan_vendor_dirs_where( + &args.common.project_root(), + state, + args.common.dry_run, + |unit| orphan_in_target(target, unit), + ) + .await; + let loud = !args.common.json && !args.common.silent; + let warnings = sweep + .still_wired + .iter() + .map(|unit| (unit, orphan_still_wired(unit))) + .chain( + sweep + .failed + .iter() + .map(|(unit, e)| (unit, orphan_removal_failed(unit, e))), + ); + for (unit, warning) in warnings { + if loud { + eprintln!("Warning: {}", warning.detail); + } + leg.skipped.push( + PatchEvent::new(PatchAction::Skipped, orphan_label(unit)) + .with_reason(warning.code, warning.detail), + ); + } + if loud && !sweep.removed.is_empty() { + print_orphan_sweep(args.common.dry_run, &sweep); + leg.printed = true; + } + leg.orphans = sweep.removed.iter().map(orphan_removed_event).collect(); +} + /// Why a hosted unwind stopped: a pin the upstream restore refused (or a /// write failure). Each caller renders its own message (the /// manifest-backed path adds that the manifest was not touched). @@ -1591,6 +1652,7 @@ async fn remove_hosted_only( /// patch can only mean reverting its vendoring. async fn remove_ledger_only( args: &RemoveArgs, + target: &Target, matches: Vec<(String, VendorEntry)>, mut state: VendorState, telemetry: &TelemetryAuth, @@ -1657,10 +1719,11 @@ async fn remove_ledger_only( } let keys: Vec = matches.iter().map(|(k, _)| k.clone()).collect(); - let leg = match revert_vendored_matches(args, &keys, &mut state, telemetry, false).await { + let mut leg = match revert_vendored_matches(args, &keys, &mut state, telemetry, false).await { Ok(leg) => leg, Err(code) => return code, }; + sweep_target_orphans(args, &state, target, &mut leg).await; let mut env = Envelope::new(Command::Remove); env.dry_run = args.common.dry_run; @@ -1675,6 +1738,14 @@ async fn remove_ledger_only( for ev in events { env.record(ev); } + // Swept orphan dirs bypass `record`: `summary.removed` counts the + // reverted entries, not the dirs no entry owned. + let mut orphans = leg.orphans; + crate::commands::vendored_backend::tag_event_mode( + &mut orphans, + crate::commands::VENDORED_MODE_LABEL, + ); + env.events.extend(orphans); if !leg.kept.is_empty() { // Any drift-kept entry means part of the requested removal did // NOT happen: partialFailure (exit 1). When EVERY match kept, the diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 6a480ff54..93b0929dd 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -110,7 +110,7 @@ pub(crate) fn as_question(text: &str) -> String { /// The default (destructive) rollback's confirmation prompt, naming only /// the legs that have work. -fn rollback_prompt(manifest: usize, vendored: usize, hosted: usize) -> String { +fn rollback_prompt(manifest: usize, vendored: usize, hosted: usize, orphans: usize) -> String { let mut clauses: Vec = Vec::new(); if manifest > 0 { clauses.push(format!( @@ -139,6 +139,12 @@ fn rollback_prompt(manifest: usize, vendored: usize, hosted: usize) -> String { plural(hosted, "hosted package", "hosted packages") )); } + if orphans > 0 { + clauses.push(format!( + "delete {} with no ledger record", + plural(orphans, "orphaned vendored dir", "orphaned vendored dirs") + )); + } as_question(&join_clauses(&clauses)) } @@ -852,6 +858,9 @@ struct VendoredLegOutcome { /// could not be saved. failed: Vec<(String, &'static str, String)>, warnings: Vec<(String, String)>, + /// `removed` / `vendor_orphan_removed` events for the orphan uuid + /// dirs (no ledger entry) swept, also on dry-run. + orphans_removed: Vec, } /// Unwire the in-scope vendored entries. `preserve` keeps artifacts and @@ -1063,6 +1072,11 @@ fn build_rollback_envelope(report: &RollbackReport<'_>) -> Envelope { vendor_uuid(key), )); } + // Swept orphan dirs are `removed` on a dry run too (as in + // `vendor --revert`): `verified` would read as a vendored restore. + for event in &leg.orphans_removed { + env.record(event.clone().with_details(vendored(serde_json::json!({})))); + } // ── hosted leg ── let hosted = || serde_json::json!({ "mode": crate::commands::HOSTED_MODE_LABEL }); @@ -1721,10 +1735,44 @@ pub async fn run(args: RollbackArgs) -> i32 { .join(",") ); } + // Uuid dirs under `.socket/vendor/` that no ledger entry owns (the + // losing side of a git merge of two vendor runs, an interrupted run) + // are local state too: a run with work sweeps the ones in its scope + // after the vendored leg, as `vendor --revert` does (#1610). Path-glob + // targets name installed copies, never an orphan. + let sweep_orphans = has_work && project_state && !args.preserve_state && !ledger_unreadable; + let orphan_in_scope = |unit: &socket_patch_core::vendor::path::SweptVendorDir| { + crate::commands::vendor::ecosystem_in_scope(&args.common, &unit.eco) + && (!scoped + || identifiers + .iter() + .any(|id| crate::commands::vendor_orphans::orphan_in_target(id, unit))) + }; if has_work && !args.common.dry_run && !args.preserve_state { + // The orphans the sweep would delete, for the prompt: the reverts + // only shrink the ledger, so the pre-run ledger finds the same set. + let orphans = match &vendor_state_result { + Ok(state) if sweep_orphans => { + crate::commands::vendor_orphans::sweep_orphan_vendor_dirs_where( + &ledger_root, + state, + true, + &orphan_in_scope, + ) + .await + .removed + .len() + } + _ => 0, + }; // Compose only the clauses that apply, so a hosted-only run never // claims manifest entries it does not have. - let prompt = rollback_prompt(manifest_scope.len(), vendor_scope.len(), hosted_scope.len()); + let prompt = rollback_prompt( + manifest_scope.len(), + vendor_scope.len(), + hosted_scope.len(), + orphans, + ); if !crate::ui::confirm(&prompt, true, &args.common) { if !args.common.json && !args.common.silent { println!("{}", crate::ui::CANCELLED); @@ -1773,6 +1821,7 @@ pub async fn run(args: RollbackArgs) -> i32 { // `--preserve-state` keeps artifacts and entries. Skipped // fail-closed when the ledger is unreadable. let mut vendored_leg = VendoredLegOutcome::default(); + let mut reverted_state: Option = None; if !vendor_corrupt && !vendor_scope.is_empty() { let mut vs = vendor_state_result .as_ref() @@ -1783,6 +1832,42 @@ pub async fn run(args: RollbackArgs) -> i32 { keys.sort(); vendored_leg = run_vendored_leg(&args.common, &keys, &mut vs, args.preserve_state).await; + reverted_state = Some(vs); + } + + // ── orphan vendored dirs (see `sweep_orphans`) ─────────────── + // A wired orphan is kept with a warning: deleting it would + // break the next install. + let ledger_now = reverted_state + .as_ref() + .or(vendor_state_result.as_ref().ok()); + if let Some(state) = ledger_now.filter(|_| sweep_orphans && !aborted) { + let sweep = crate::commands::vendor_orphans::sweep_orphan_vendor_dirs_where( + &ledger_root, + state, + args.common.dry_run, + &orphan_in_scope, + ) + .await; + for unit in &sweep.still_wired { + let warning = crate::commands::vendor_orphans::orphan_still_wired(unit); + run_warnings.push((warning.code.to_string(), warning.detail)); + } + for (unit, e) in &sweep.failed { + let warning = crate::commands::vendor_orphans::orphan_removal_failed(unit, e); + run_warnings.push((warning.code.to_string(), warning.detail)); + } + if !args.common.json && !args.common.silent { + crate::commands::vendor_orphans::print_orphan_sweep( + args.common.dry_run, + &sweep, + ); + } + vendored_leg.orphans_removed = sweep + .removed + .iter() + .map(crate::commands::vendor_orphans::orphan_removed_event) + .collect(); } // ── hosted leg ─────────────────────────────────────────────── @@ -5546,31 +5631,36 @@ mod tests { #[test] fn rollback_prompt_singular_plural_and_clauses() { assert_eq!( - rollback_prompt(1, 0, 0), + rollback_prompt(1, 0, 0, 0), "Roll back 1 patch and remove it from the local manifest?" ); assert_eq!( - rollback_prompt(2, 0, 0), + rollback_prompt(2, 0, 0, 0), "Roll back 2 patches and remove them from the local manifest?" ); // Never "..., and restore" after an inner "and". assert_eq!( - rollback_prompt(1, 0, 1), + rollback_prompt(1, 0, 1, 0), "Roll back 1 patch, remove it from the local manifest, and restore 1 hosted \ package to the upstream registry?" ); assert_eq!( - rollback_prompt(0, 0, 3), + rollback_prompt(0, 0, 3, 0), "Restore 3 hosted packages to the upstream registry?" ); assert_eq!( - rollback_prompt(0, 1, 0), + rollback_prompt(0, 1, 0, 0), "Delete 1 vendored artifact and its ledger record?" ); assert_eq!( - rollback_prompt(0, 2, 0), + rollback_prompt(0, 2, 0, 0), "Delete 2 vendored artifacts and their ledger records?" ); + assert_eq!( + rollback_prompt(0, 1, 0, 1), + "Delete 1 vendored artifact and its ledger record and delete 1 orphaned \ + vendored dir with no ledger record?" + ); } #[test] @@ -5960,6 +6050,7 @@ mod tests { "boom".to_string(), )], warnings: Vec::new(), + orphans_removed: Vec::new(), }; let hosted = HostedLegOutcome { reverted: vec!["pkg:npm/h@1.0.0".to_string()], diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index fd6523ca6..6dedfaf7e 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -50,6 +50,10 @@ use crate::args::{apply_env_toggles, GlobalArgs}; use crate::commands::apply::{representative_file, result_to_event, variant_matches_installed}; use crate::commands::bun_preflight::bun_vendor_preflight_pairs; use crate::commands::lock_cli::acquire_or_emit; +use crate::commands::vendor_orphans::{ + orphan_label, orphan_removal_failed, orphan_removed_event, orphan_still_wired, + sweep_orphan_vendor_dirs, +}; use crate::commands::vendored_backend::{ ApplyRequest, KeepCause, RevertedEntry, VendorRevertStep, VendoredBackend, }; @@ -515,75 +519,6 @@ async fn unwired_check_failure( ) } -/// What the orphan sweep did with the uuid dirs no ledger entry owns. -#[derive(Default)] -struct OrphanSweep { - /// Un-ledgered AND unreferenced — deleted (unless `dry_run`). - removed: Vec, - /// Un-ledgered but a project lockfile still points into them — kept. - still_wired: Vec, -} - -/// Uuid dirs under `.socket/vendor//` with no owning `(eco, uuid)` -/// ledger entry (a hand-edited state file, or artifacts left by an -/// interrupted run). Unparseable dirs are never returned (and never -/// deleted). Returns the orphans so callers can emit events / counts. -/// -/// A missing ledger entry does NOT prove missing wiring: lockfiles can -/// still point into `.socket/vendor/` after a deleted state.json or a -/// partial commit. Deleting such a dir would break the next install, so -/// every candidate is checked against the wiring-bearing files first — the -/// same lockfile scan `repair` reports `vendor_ledger_missing` from — and a -/// referenced dir is kept for the caller to warn about. -async fn sweep_orphan_vendor_dirs(cwd: &Path, state: &VendorState, dry_run: bool) -> OrphanSweep { - let recorded_units: HashSet<(&str, &str)> = state - .entries - .values() - .map(|e| (e.ecosystem.as_str(), e.uuid.as_str())) - .collect(); - let candidates: Vec = vendor::path::sweep_vendor_dirs(cwd) - .await - .into_iter() - .filter(|unit| !recorded_units.contains(&(unit.eco.as_str(), unit.uuid.as_str()))) - .collect(); - let mut out = OrphanSweep::default(); - if candidates.is_empty() { - return out; - } - let wired: HashSet<(String, String)> = - crate::commands::vendored_backend::repair::scan_vendor_references(cwd) - .await - .into_iter() - .map(|(eco, uuid, _path)| (eco, uuid)) - .collect(); - // Each removal also prunes the `/` and `vendor/` levels it - // emptied (never `.socket/` — the lock guard owns that level): the - // sweep runs AFTER the per-entry reverts and their ledger saves, so it - // is the last thing that can leave a fully reverted project with empty - // `.socket/vendor//` husks. - let stop_dir = cwd.join(SOCKET_DIR); - for unit in candidates { - if wired.contains(&(unit.eco.clone(), unit.uuid.clone())) { - out.still_wired.push(unit); - continue; - } - if !dry_run { - let _ = remove_tree_and_prune(&unit.dir, &stop_dir).await; - } - out.removed.push(unit); - } - out -} - -/// How an orphan uuid dir is named in events: the PURL recovered from its -/// leaf when the layout is recognizable, else `/`. -fn orphan_label(unit: &vendor::path::SweptVendorDir) -> String { - unit.purls - .first() - .cloned() - .unwrap_or_else(|| format!("{}/{}", unit.eco, unit.uuid)) -} - /// Does `eco` fall inside this run's `--ecosystems` scope? A vendor-ledger /// name counts as the package ecosystem it stands for (a `jvm` entry is /// `maven`, [`vendor::jvm::layout::ledger_ecosystem`]). @@ -4530,27 +4465,18 @@ async fn run_revert(args: &VendorArgs, env: &mut Envelope) -> i32 { // points at (their wiring outlived the ledger). let sweep = sweep_orphan_vendor_dirs(&common.cwd, &state, common.dry_run).await; for unit in &sweep.still_wired { - let label = orphan_label(unit); + record_warning(env, &orphan_label(unit), &orphan_still_wired(unit), common); + } + for (unit, e) in &sweep.failed { record_warning( env, - &label, - &VendorWarning::new( - "vendor_orphan_still_wired", - format!( - "a project lockfile still points at .socket/vendor/{}/{}, which no ledger \ - entry owns; the artifacts were kept (run `socket-patch repair` to re-adopt \ - them into the ledger, then revert again)", - unit.eco, unit.uuid - ), - ), + &orphan_label(unit), + &orphan_removal_failed(unit, e), common, ); } for unit in &sweep.removed { - env.record( - PatchEvent::new(PatchAction::Removed, orphan_label(unit)) - .with_reason("vendor_orphan_removed", "vendored dir had no ledger entry"), - ); + env.record(orphan_removed_event(unit)); } if env.events.is_empty() { @@ -4624,6 +4550,8 @@ pub(crate) struct VendorGcSummary { /// Post-revert rewrites that failed: `("vendor_state_write_failed" | /// "manifest_write_failed", )`. The reverts themselves already /// happened on disk; the stale record is what the caller must report. + /// Also `("cleanup_failed", )` per orphan dir (c) could not + /// delete. pub write_failures: Vec<(&'static str, String)>, /// The wet reverts' reinstall advisories (`code`, `detail`): Bun's /// `vendor_bun_reinstall_required` (#764) and vlt's @@ -4879,10 +4807,13 @@ pub(crate) async fn run_vendor_gc( // (c) orphan uuid dirs, against the post-removal ledger. Dirs a lockfile // still points at are kept, so they are not counted as reclaimed. - out.orphan_dirs = sweep_orphan_vendor_dirs(&common.cwd, &state, dry_run) - .await - .removed - .len(); + // One that could not be deleted is a `cleanup_failed` warning. + let sweep = sweep_orphan_vendor_dirs(&common.cwd, &state, dry_run).await; + out.orphan_dirs = sweep.removed.len(); + for (unit, e) in &sweep.failed { + let warning = orphan_removal_failed(unit, e); + out.write_failures.push((warning.code, warning.detail)); + } out } diff --git a/crates/socket-patch-cli/src/commands/vendor_orphans.rs b/crates/socket-patch-cli/src/commands/vendor_orphans.rs new file mode 100644 index 000000000..e794d9e91 --- /dev/null +++ b/crates/socket-patch-cli/src/commands/vendor_orphans.rs @@ -0,0 +1,205 @@ +//! The orphan sweep `vendor --revert`, `scan --prune`, `rollback` and +//! `remove` share: uuid dirs under `.socket/vendor//` that no ledger +//! entry owns (a hand-edited state file, an interrupted run, the losing +//! side of a git merge of two vendor runs, #1610). + +use std::collections::HashSet; +use std::path::Path; + +use socket_patch_core::constants::SOCKET_DIR; +use socket_patch_core::utils::socket_dir::remove_tree_and_prune; +use socket_patch_core::utils::target::{Target, TargetKind}; +use socket_patch_core::vendor::{self, VendorState, VendorWarning}; + +use crate::json_envelope::{PatchAction, PatchEvent}; + +/// What the orphan sweep did with the uuid dirs no ledger entry owns. +#[derive(Default)] +pub(crate) struct OrphanSweep { + /// Un-ledgered AND unreferenced — deleted (unless `dry_run`). + pub(crate) removed: Vec, + /// Un-ledgered but a project lockfile still points into them — kept. + pub(crate) still_wired: Vec, + /// Un-ledgered and unreferenced, but the delete failed (the error). + pub(crate) failed: Vec<(vendor::path::SweptVendorDir, String)>, +} + +/// Uuid dirs under `.socket/vendor//` with no owning `(eco, uuid)` +/// ledger entry (a hand-edited state file, or artifacts left by an +/// interrupted run). Unparseable dirs are never returned (and never +/// deleted). Returns the orphans so callers can emit events / counts. +/// +/// A missing ledger entry does NOT prove missing wiring: lockfiles can +/// still point into `.socket/vendor/` after a deleted state.json or a +/// partial commit. Deleting such a dir would break the next install, so +/// every candidate is checked against the wiring-bearing files first — the +/// same lockfile scan `repair` reports `vendor_ledger_missing` from — and a +/// referenced dir is kept for the caller to warn about. +pub(crate) async fn sweep_orphan_vendor_dirs( + cwd: &Path, + state: &VendorState, + dry_run: bool, +) -> OrphanSweep { + sweep_orphan_vendor_dirs_where(cwd, state, dry_run, |_| true).await +} + +/// [`sweep_orphan_vendor_dirs`] limited to the orphans `select` accepts: +/// `rollback` and `remove` sweep only the ones in their scope (#1610). +pub(crate) async fn sweep_orphan_vendor_dirs_where( + cwd: &Path, + state: &VendorState, + dry_run: bool, + select: impl Fn(&vendor::path::SweptVendorDir) -> bool, +) -> OrphanSweep { + let recorded_units: HashSet<(&str, &str)> = state + .entries + .values() + .map(|e| (e.ecosystem.as_str(), e.uuid.as_str())) + .collect(); + let mut candidates: Vec = vendor::path::sweep_vendor_dirs(cwd) + .await + .into_iter() + .filter(|unit| !recorded_units.contains(&(unit.eco.as_str(), unit.uuid.as_str()))) + .filter(|unit| select(unit)) + .collect(); + // Directory order is the filesystem's: report the orphans in a stable + // one, so a run's events and lines do not depend on it. + candidates.sort_by(|a, b| (&a.eco, &a.uuid).cmp(&(&b.eco, &b.uuid))); + let mut out = OrphanSweep::default(); + if candidates.is_empty() { + return out; + } + let wired: HashSet<(String, String)> = + crate::commands::vendored_backend::repair::scan_vendor_references(cwd) + .await + .into_iter() + .map(|(eco, uuid, _path)| (eco, uuid)) + .collect(); + // Each removal also prunes the `/` and `vendor/` levels it + // emptied (never `.socket/` — the lock guard owns that level): the + // sweep runs AFTER the per-entry reverts and their ledger saves, so it + // is the last thing that can leave a fully reverted project with empty + // `.socket/vendor//` husks. + let stop_dir = cwd.join(SOCKET_DIR); + for unit in candidates { + if wired.contains(&(unit.eco.clone(), unit.uuid.clone())) { + out.still_wired.push(unit); + continue; + } + if !dry_run { + if let Err(e) = remove_tree_and_prune(&unit.dir, &stop_dir).await { + out.failed.push((unit, e.to_string())); + continue; + } + } + out.removed.push(unit); + } + out +} + +/// How an orphan uuid dir is named in events: the PURL recovered from its +/// leaf when the layout is recognizable, else `/`. +pub(crate) fn orphan_label(unit: &vendor::path::SweptVendorDir) -> String { + unit.purls + .first() + .cloned() + .unwrap_or_else(|| format!("{}/{}", unit.eco, unit.uuid)) +} + +/// The advisory for an orphan uuid dir the sweep kept because a project +/// lockfile still points into it. +pub(crate) fn orphan_still_wired(unit: &vendor::path::SweptVendorDir) -> VendorWarning { + VendorWarning::new( + "vendor_orphan_still_wired", + format!( + "a project lockfile still points at .socket/vendor/{}/{}, which no ledger \ + entry owns; the artifacts were kept (run `socket-patch repair` to re-adopt \ + them into the ledger, then revert again)", + unit.eco, unit.uuid + ), + ) +} + +/// The advisory for an orphan uuid dir the sweep could not delete. +pub(crate) fn orphan_removal_failed( + unit: &vendor::path::SweptVendorDir, + error: &str, +) -> VendorWarning { + VendorWarning::new( + "cleanup_failed", + format!( + "could not delete .socket/vendor/{}/{}, which no ledger entry owns: {error}", + unit.eco, unit.uuid + ), + ) +} + +/// The `removed` event for an orphan uuid dir the sweep deleted (or, on a +/// dry run, would delete). +pub(crate) fn orphan_removed_event(unit: &vendor::path::SweptVendorDir) -> PatchEvent { + PatchEvent::new(PatchAction::Removed, orphan_label(unit)) + .with_reason("vendor_orphan_removed", "vendored dir had no ledger entry") +} + +/// Is the orphan `unit` one `target` names? By a package its leaves name, +/// or, for a uuid target, by its own uuid (also when no leaf parses). +pub(crate) fn orphan_in_target(target: &Target, unit: &vendor::path::SweptVendorDir) -> bool { + (target.kind() == TargetKind::Uuid && target.matches_patch("", &unit.uuid)) + || unit + .purls + .iter() + .any(|purl| target.matches_patch(purl, &unit.uuid)) +} + +/// The human lines for the orphan dirs a `rollback` / `remove` sweep +/// deleted (would delete, on a dry run). Kept and failed ones are +/// warnings, printed by the caller with its other warnings. +pub(crate) fn print_orphan_sweep(dry_run: bool, sweep: &OrphanSweep) { + let verb = if dry_run { "Would remove" } else { "Removed" }; + for unit in &sweep.removed { + println!( + "{verb} orphaned vendored dir .socket/vendor/{}/{} (no ledger entry)", + unit.eco, unit.uuid + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + /// `rollback ` / `remove ` scope their orphan sweep + /// with [`orphan_in_target`]: a package target matches by the leaves' + /// purls, a uuid target by the dir's own uuid, even when no leaf + /// parses (#1610). + #[test] + fn orphan_in_target_matches_package_and_uuid_targets() { + let uuid = "aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa"; + let unit = |purls: &[&str]| vendor::path::SweptVendorDir { + eco: "npm".into(), + uuid: uuid.into(), + dir: PathBuf::from("/x"), + purls: purls.iter().map(|p| p.to_string()).collect(), + }; + let left_pad = unit(&["pkg:npm/left-pad@1.3.0"]); + for hit in [ + "pkg:npm/left-pad@1.3.0", + "pkg:npm/left-pad", + "left-pad", + uuid, + ] { + assert!(orphan_in_target(&Target::parse(hit), &left_pad), "{hit}"); + } + for miss in [ + "pkg:npm/left-pad@1.2.0", + "right-pad", + "bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb", + ] { + assert!(!orphan_in_target(&Target::parse(miss), &left_pad), "{miss}"); + } + let unparsed = unit(&[]); + assert!(orphan_in_target(&Target::parse(uuid), &unparsed)); + assert!(!orphan_in_target(&Target::parse("left-pad"), &unparsed)); + } +} diff --git a/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs b/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs index d1366360b..12c8ccb5a 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs @@ -891,3 +891,204 @@ async fn vendor_refuses_a_linked_vendor_dir_before_writing() { ); assert_eq!(fx.lock_bytes(), fx.original_lock, "lock untouched"); } + +// ───────────────────────────────────────────────────────────────────── +// orphan uuid dirs: rollback and remove sweep them like `vendor --revert` +// ───────────────────────────────────────────────────────────────────── + +/// Another vendor run's uuid for the same package: the side a git merge +/// of two vendor branches drops from state.json but keeps on disk (#1610). +const ORPHAN_UUID: &str = "aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa"; +/// An orphan for a package the scoped runs below do not target. +const OTHER_ORPHAN_UUID: &str = "bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb"; + +/// Vendor the fixture, then plant two uuid dirs no ledger entry owns: +/// a copy of the live left-pad artifact under [`ORPHAN_UUID`] and a +/// `right-pad` tarball under [`OTHER_ORPHAN_UUID`]. +async fn vendored_fixture_with_orphans() -> NpmFixture { + let fx = npm_fixture(); + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0, "vendor"); + let orphan = fx.root().join(format!(".socket/vendor/npm/{ORPHAN_UUID}")); + std::fs::create_dir_all(&orphan).unwrap(); + std::fs::copy(fx.tgz_path(), orphan.join("left-pad-1.3.0.tgz")).unwrap(); + let other = fx + .root() + .join(format!(".socket/vendor/npm/{OTHER_ORPHAN_UUID}")); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(other.join("right-pad-1.0.0.tgz"), b"not a real tarball").unwrap(); + fx +} + +fn orphan_dir(fx: &NpmFixture, uuid: &str) -> PathBuf { + fx.root().join(format!(".socket/vendor/npm/{uuid}")) +} + +/// The `vendor_orphan_removed` events' purls, in event order. +fn orphans_removed(env: &Value) -> Vec { + env["events"] + .as_array() + .expect("events array") + .iter() + .filter(|e| e["action"] == "removed" && e["errorCode"] == "vendor_orphan_removed") + .map(|e| e["purl"].as_str().unwrap_or_default().to_string()) + .collect() +} + +/// A bare `rollback` promises to clean up the local state, so it sweeps +/// the uuid dirs no ledger entry owns, as `vendor --revert` does (#1610). +/// Before the fix the orphans stayed committed and nothing reported them. +#[tokio::test] +async fn unscoped_rollback_sweeps_orphan_vendor_dirs() { + let fx = vendored_fixture_with_orphans().await; + + // A dry run reports both orphans and deletes nothing. + let (code, env) = rollback_cli(fx.root(), &["--dry-run"]); + assert_eq!(code, 0, "dry run exits 0: {env:#}"); + rollback_json::assert_rollback_envelope(&env); + assert_eq!( + orphans_removed(&env), + vec![PURL.to_string(), "pkg:npm/right-pad@1.0.0".to_string()], + "{env:#}" + ); + assert!( + orphan_dir(&fx, ORPHAN_UUID).is_dir(), + "dry run deletes nothing" + ); + assert!(orphan_dir(&fx, OTHER_ORPHAN_UUID).is_dir()); + + let (code, env) = rollback_cli(fx.root(), &[]); + assert_eq!(code, 0, "rollback exits 0: {env:#}"); + assert_eq!(env["status"], "success", "{env:#}"); + rollback_json::assert_rollback_envelope(&env); + assert_eq!( + rollback_json::vendored_reverted(&env), + json!([PURL]), + "{env:#}" + ); + assert_eq!( + orphans_removed(&env), + vec![PURL.to_string(), "pkg:npm/right-pad@1.0.0".to_string()], + "both orphans are reported: {env:#}" + ); + assert_eq!(fx.lock_bytes(), fx.original_lock, "lock restored"); + assert!( + !fx.root().join(".socket/vendor").exists(), + "a full rollback leaves no .socket/vendor/ behind" + ); +} + +/// An orphan a lockfile still points at is kept and named with +/// `vendor_orphan_still_wired`: deleting it would break the next install. +#[tokio::test] +async fn unscoped_rollback_keeps_a_still_wired_orphan() { + let fx = vendored_fixture_with_orphans().await; + // Wire `right-pad` to its orphan dir behind the ledger's back. + let mut lock: Value = serde_json::from_slice(&fx.lock_bytes()).unwrap(); + lock["packages"]["node_modules/right-pad"] = json!({ + "version": "1.0.0", + "resolved": format!("file:.socket/vendor/npm/{OTHER_ORPHAN_UUID}/right-pad-1.0.0.tgz"), + }); + std::fs::write(fx.lock_path(), serde_json::to_vec_pretty(&lock).unwrap()).unwrap(); + + let (code, env) = rollback_cli(fx.root(), &[]); + assert_eq!(code, 0, "rollback exits 0: {env:#}"); + rollback_json::assert_rollback_envelope(&env); + assert_eq!(orphans_removed(&env), vec![PURL.to_string()], "{env:#}"); + assert!( + rollback_json::warning_codes(&env).contains(&"vendor_orphan_still_wired".to_string()), + "the kept orphan is named: {env:#}" + ); + assert!( + !orphan_dir(&fx, ORPHAN_UUID).exists(), + "unwired orphan swept" + ); + assert!( + orphan_dir(&fx, OTHER_ORPHAN_UUID).is_dir(), + "a wired orphan is never deleted" + ); +} + +/// A scoped rollback sweeps only the orphans of the package it targets. +#[tokio::test] +async fn scoped_rollback_sweeps_only_the_targets_orphans() { + let fx = vendored_fixture_with_orphans().await; + let (code, env) = rollback_cli(fx.root(), &[PURL]); + assert_eq!(code, 0, "rollback exits 0: {env:#}"); + rollback_json::assert_rollback_envelope(&env); + assert_eq!(orphans_removed(&env), vec![PURL.to_string()], "{env:#}"); + assert!( + !orphan_dir(&fx, ORPHAN_UUID).exists(), + "target's orphan swept" + ); + assert!( + orphan_dir(&fx, OTHER_ORPHAN_UUID).is_dir(), + "another package's orphan is out of scope" + ); +} + +/// `--preserve-state` keeps local state, orphans included. +#[tokio::test] +async fn preserve_state_rollback_keeps_orphans() { + let fx = vendored_fixture_with_orphans().await; + let (code, env) = rollback_cli(fx.root(), &["--preserve-state"]); + assert_eq!(code, 0, "{env:#}"); + assert!(orphans_removed(&env).is_empty(), "{env:#}"); + assert!(orphan_dir(&fx, ORPHAN_UUID).is_dir()); + assert!(orphan_dir(&fx, OTHER_ORPHAN_UUID).is_dir()); +} + +/// `remove ` sweeps the orphans of the package it removes (#1610). +#[tokio::test] +async fn remove_sweeps_the_targets_orphans() { + let fx = vendored_fixture_with_orphans().await; + let cwd = fx.root().to_str().unwrap().to_string(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &["remove", PURL, "--json", "--offline", "--cwd", &cwd], + ); + let env: Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("remove --json envelope: {e}\n{stdout}\n{stderr}")); + assert_eq!(code, 0, "remove exits 0: {env:#}"); + assert_eq!(env["status"], "success", "{env:#}"); + assert_eq!(orphans_removed(&env), vec![PURL.to_string()], "{env:#}"); + assert!(!fx.tgz_path().exists(), "the ledgered artifact is reverted"); + assert!( + !orphan_dir(&fx, ORPHAN_UUID).exists(), + "target's orphan swept" + ); + assert!( + orphan_dir(&fx, OTHER_ORPHAN_UUID).is_dir(), + "another package's orphan is out of scope" + ); +} + +/// A `remove --dry-run` previews the orphan sweep and deletes nothing; the +/// orphan events never count toward `summary.removed` (the manifest +/// entries dropped). +#[tokio::test] +async fn remove_dry_run_previews_orphans_without_counting_them() { + let fx = vendored_fixture_with_orphans().await; + let cwd = fx.root().to_str().unwrap().to_string(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &[ + "remove", + PURL, + "--dry-run", + "--json", + "--offline", + "--cwd", + &cwd, + ], + ); + let env: Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("remove --json envelope: {e}\n{stdout}\n{stderr}")); + assert_eq!(code, 0, "{env:#}"); + assert_eq!(orphans_removed(&env), vec![PURL.to_string()], "{env:#}"); + assert_eq!(env["summary"]["removed"], 0, "{env:#}"); + assert!(fx.tgz_path().is_file(), "dry run reverts nothing"); + assert!( + orphan_dir(&fx, ORPHAN_UUID).is_dir(), + "dry run deletes nothing" + ); +}