Fix repair/prune deleting active patches' restore blobs (#893)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -24,7 +24,7 @@ For task-oriented guidance, start with [usage](../../docs/usage.md),
 | `apply` | — | Agent mode: apply patches from the local manifest |
 | `rollback` | — | **Full-state rollback (v5.0, MAJOR)**: restore original files AND unwind vendored lockfile wiring / restore hosted pins to their upstream registry entries, remove the rolled-back entries from the manifest, and GC their blobs/archives; takes optional variadic positional `targets` (PURL \| UUID \| package name \| path glob). See [Rollback command contract](#rollback-command-contract-v50) |
 | `remove` | — | Restore and remove one patch across hosted, vendored, and agent state; requires positional `identifier`. |
-| `repair` | — | Download missing agent blobs, redownload missing/corrupt vendored artifacts (never re-synthesizing a lost ledger), and clean up unused ones (refuses with `lock_held` when a live process holds the lock; see "Lock lifecycle" below) |
+| `repair` | — | Download missing agent blobs, redownload missing/corrupt vendored artifacts (never re-synthesizing a lost ledger), and clean up unused ones: a blob no manifest patch references (the afterHash and beforeHash blobs of every manifest patch are kept, so an offline `rollback` still has its originals; `scan --prune` keeps the same set) and every obsolete diff or package archive (refuses with `lock_held` when a live process holds the lock; see "Lock lifecycle" below) |
 
 Rows are in `--help` order (v5.0): the hosted/vendored workflow (`scan` → `vex` → `vendor`, with `list` to inspect), then the agent-mode (in-place patching) commands.
 
@@ -145,7 +145,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc
 
 **Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (the per-patch `skipped`/`vendored` records in `apply.patches[]` are unchanged); the detail names the purls and the migration path (`remove <purl>`, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove <purl>` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `<purl>: <path>: patched, but ownership could not be restored to uid N gid M: <error>` and the human line `Warning: <detail>` (stderr, muted by `--silent`); never a status or exit change.
 
-`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:<type>/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor/<eco>/<uuid>` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `failedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview), plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (<code>): <message>.`, one `GC: <detail>.` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so `revertableVendoredEntries` may over-promise what a wet run will actually reclaim.
+`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:<type>/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor/<eco>/<uuid>` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `failedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview), plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (<code>): <message>.`, one `GC: <detail>.` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so `revertableVendoredEntries` may over-promise what a wet run will actually reclaim.
 
 `scan` queries the patch API in `--batch-size` chunks. Authenticated runs POST `/v0/orgs/{slug}/patches/batch`; token-less runs POST `{proxy}/patch/batch` on the public proxy and degrade to per-package `GET /patch/by-package/:purl` requests in two cases: the deployed proxy predates the batch endpoint (legacy proxies answer the POST with their `400 "Unsupported endpoint"` catch-all), or the all-or-nothing batch validation rejects the chunk (e.g. a crawled PURL type the server doesn't recognize, such as `pkg:jsr/…` — the per-package path tolerates those individually, preserving the pre-batch scan semantics). Rate limits and over-capacity 503s surface instead of silently degrading.
 

crates/socket-patch-cli/src/commands/repair.rs

@@ -588,7 +588,7 @@ async fn repair_inner(
     // them goes). The 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 = ArtifactReferences::for_apply(manifest)
+        let sweep = ArtifactReferences::active(manifest)
             .sweep(&socket_dir, args.common.dry_run)
             .await;
         let passes = [

crates/socket-patch-cli/src/commands/rollback.rs

@@ -655,7 +655,7 @@ fn skipped_not_installed_json(purl: &str) -> serde_json::Value {
 /// human path gets. One failed result per affected package keeps the
 /// `failed` counter meaning "packages that failed" (the same per-package
 /// semantics as a mid-run failure) and names each missing blob hash plus
-/// the `socket-patch repair` remedy in machine-readable form, using the
+/// the re-run remedy in machine-readable form, using the
 /// engine's own `missing_blob` verify vocabulary. `reason_for` renders
 /// the per-hash diagnostic (offline gate vs. download failure).
 fn missing_blob_abort_results(
@@ -2414,7 +2414,7 @@ pub(crate) async fn rollback_patches_inner(
                     "Error: {} missing and --offline is set.",
                     plural(missing_blobs.len(), "blob is", "blobs are")
                 );
-                eprintln!("Run \"socket-patch repair\" to download missing blobs.");
+                eprintln!("Re-run without --offline to download the original blobs.");
             }
             let results = missing_blob_abort_results(
                 &abort_manifest,
@@ -2423,7 +2423,7 @@ pub(crate) async fn rollback_patches_inner(
                 |hash| {
                     format!(
                         "Before blob not found: {hash} and --offline prevents fetching. \
-                         Run \"socket-patch repair\" to download missing blobs."
+                         Re-run without --offline to download the original blobs."
                     )
                 },
             );
@@ -2504,7 +2504,7 @@ pub(crate) async fn rollback_patches_inner(
                         .unwrap_or("download failed");
                     format!(
                         "Before blob could not be downloaded: {hash} - {why}. \
-                         Run \"socket-patch repair\" to download missing blobs."
+                         Re-run once the patch API is reachable to download the original blobs."
                     )
                 },
             );

crates/socket-patch-cli/src/commands/scan/gc.rs

@@ -158,7 +158,7 @@ async fn run_gc(
     socket_dir: &Path,
     dry_run: bool,
 ) -> GcSummary {
-    let sweep = ArtifactReferences::for_apply(manifest)
+    let sweep = ArtifactReferences::active(manifest)
         .sweep(socket_dir, dry_run)
         .await;
     let mut warnings = Vec::new();

crates/socket-patch-cli/tests/cli/output_modes_e2e.rs

@@ -314,40 +314,36 @@ fn scan_non_json_no_packages_prints_friendly_message() {
 fn repair_non_json_no_orphans_prints_summary() {
     let tmp = tempfile::tempdir().unwrap();
     write_manifest(tmp.path(), "pkg:npm/repair-target@1.0.0", b"a", b"b");
-    // `write_manifest` writes BOTH the beforeHash and afterHash blobs, but
-    // repair treats `beforeHash` blobs as unused-by-design (they are fetched
-    // on demand during rollback). To exercise the genuine "all in use" path
-    // implied by this test's name, drop the beforeHash blob so the only
-    // remaining blob is the in-use afterHash one.
+    // `write_manifest` writes BOTH the beforeHash and afterHash blobs of
+    // the active patch, and repair keeps both (#893: the beforeHash blob is
+    // an offline rollback's only restore data).
     let blobs = tmp.path().join(".socket/blobs");
     let before_blob = blobs.join(git_sha256(b"a"));
     let after_blob = blobs.join(git_sha256(b"b"));
-    std::fs::remove_file(&before_blob).unwrap();
     assert!(
-        after_blob.exists(),
-        "fixture precondition: afterHash blob present"
+        before_blob.exists() && after_blob.exists(),
+        "fixture precondition: both blobs present"
     );
 
     let (code, stdout, _stderr) = common::run_with_env(tmp.path(), &["repair", "--offline"], &[]);
     assert_eq!(code, 0);
-    // With exactly one in-use blob and no orphans, repair must report the
+    // With two in-use blobs and no orphans, repair must report the
     // all-in-use status (not a removal) and finish. The old check accepted
     // any output containing "Repair complete.", so a repair that wrongly
     // deleted the in-use blob — or skipped the cleanup scan entirely — still
     // passed.
     assert!(
-        stdout.contains("Checked 1 blob: in use."),
-        "no-orphan repair must report the single blob as in-use; got: {stdout}"
+        stdout.contains("Checked 2 blobs: all in use."),
+        "no-orphan repair must report both blobs as in use; got: {stdout}"
     );
     assert!(
         stdout.contains("Repair complete."),
         "non-JSON repair should print the completion summary; got: {stdout}"
     );
-    // Critically: the in-use afterHash blob (the patched file content that
-    // `apply` needs) must NOT be deleted by repair.
+    // Critically: neither in-use blob may be deleted by repair.
     assert!(
-        after_blob.exists(),
-        "repair must preserve the in-use afterHash blob"
+        after_blob.exists() && before_blob.exists(),
+        "repair must preserve the active patch's afterHash and beforeHash blobs"
     );
 }
 
@@ -370,12 +366,16 @@ fn repair_non_json_with_orphans_prints_cleanup_summary() {
     assert_eq!(code, 0);
     // The test name promises a *cleanup* summary, so assert the cleanup
     // actually happened — both in the printed summary and on disk. Pin the
-    // exact count (the orphan blob + the by-design-unused beforeHash blob =
-    // 2) so a repair that removes too few OR too many blobs fails here; the
-    // old `contains("Removed")` accepted any nonzero count.
+    // exact count (only the orphan: the active patch's beforeHash blob is
+    // kept, #893) so a repair that removes too few OR too many blobs fails
+    // here; the old `contains("Removed")` accepted any nonzero count.
+    assert!(
+        stdout.contains("Removed 1 unused blob ("),
+        "repair with orphans must report exactly 1 removed unused blob; got: {stdout}"
+    );
     assert!(
-        stdout.contains("Removed 2 unused blobs"),
-        "repair with orphans must report exactly 2 removed unused blobs; got: {stdout}"
+        blobs.join(git_sha256(b"a")).exists(),
+        "repair must keep the active patch's beforeHash blob"
     );
     assert!(
         !orphan.exists(),

crates/socket-patch-cli/tests/remove_rollback_api_overrides.rs

@@ -241,9 +241,9 @@ fn remove_rollback_downloads_missing_blob_via_flag_overrides() {
     // The blob's on-disk lifecycle: it must have LANDED in .socket/blobs for
     // remove to proceed (the post-download `still_missing` re-check reads the
     // dir; exit 0 below is unreachable otherwise), and then remove's
-    // unused-blob sweep deletes it again — beforeHash blobs are by design
-    // downloaded on-demand and never retained (`cleanup_unused_blobs` keeps
-    // only afterHash blobs, and this patch was just removed anyway).
+    // unused-blob sweep deletes it again — this patch was just removed (and
+    // rolled back), so nothing references its original any more; only the
+    // blobs of patches still in the manifest are retained.
     assert!(
         !socket.join("blobs").join(&before_hash).exists(),
         "remove's unused-blob sweep must not retain the on-demand before-blob.\n\