Coverage audit: 39 bug fixes + test extension to 98.2% line coverage

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -94,7 +94,7 @@ Beyond the globals above, each subcommand defines a small set of local arguments
 
 **Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--apply` / `--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 hosted redirect ledger records scanned package(s) whose hosted lockfile wiring the live lock still proves (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, or `remove <purl>` per package); the detail names the purls and the options (stay `--mode hosted`, or migrate via `scan --mode vendored`) and never advises hand-deleting the ledger. The warning keys on ledger *records* still live at scan time — a flow that pre-reverted the redirect (retiring the records) retires the warning with them, even while the append-only `edits` (revert originals) remain. The interactive 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.
 
-`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob, diff, and package-archive files 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) and the runtime-gated maven/nuget crawlers with their gate off are 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 the apply lock — lock contention skips it without failing the scan): vendored entries whose patch is gone from the manifest are reverted, vendored entries whose dependency is no longer in the lockfile graph are reverted AND their manifest entries dropped (detached entries are exempt from both — they are manifest- and lockfile-invisible by design; a missing or undeterminable lockfile keeps the entry, fail-safe), and orphan `.socket/vendor/<eco>/<uuid>` dirs with no ledger entry are swept. The JSON `gc` sub-object gains `revertedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview).
+`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob, diff, and package-archive files 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) and the runtime-gated maven/nuget crawlers with their gate off are 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 the apply lock — lock contention skips it without failing the scan): vendored entries whose patch is gone from the manifest are reverted, vendored entries whose dependency is no longer in the lockfile graph are reverted AND their manifest entries dropped (detached entries are exempt from both — they are manifest- and lockfile-invisible by design; a missing or undeterminable lockfile keeps the entry, fail-safe), and orphan `.socket/vendor/<eco>/<uuid>` dirs with no ledger entry are swept. The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview). `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/fetch_stage.rs

@@ -74,8 +74,11 @@ pub(crate) enum StageOutcome {
 
 /// Shared offline diagnostic: patches with no usable local source while
 /// `--offline` is set (first five PURLs, then the `repair` hint).
+/// Prints even under `--silent` (errors only, NEVER nothing — an exit-1
+/// run with zero output is undiagnosable); `--json` mutes stderr and the
+/// caller's envelope is the machine channel instead.
 fn report_offline_missing(common: &GlobalArgs, purls: &[&str]) {
-    if common.silent || common.json {
+    if common.json {
         return;
     }
     eprintln!(
@@ -291,7 +294,9 @@ pub(crate) async fn stage_patch_sources(
             &missing_package_archives,
         );
         if !uncovered.is_empty() {
-            if !quiet {
+            // An error, not progress chatter: prints even under --silent
+            // (same rule as report_offline_missing above).
+            if !common.json {
                 eprintln!("Some artifacts could not be downloaded. Cannot apply patches.");
             }
             return Ok(StageOutcome::Unavailable);
@@ -423,7 +428,10 @@ pub(crate) async fn stage_vendor_sources_in_memory(
                     let mut complete = true;
                     for (file, info) in &patch.files {
                         let (Some(b64), Some(hash)) = (&info.blob_content, &info.after_hash) else {
-                            if !quiet {
+                            // An error, not progress chatter: prints even
+                            // under --silent (same rule as
+                            // report_offline_missing above).
+                            if !common.json {
                                 eprintln!("  [error] {purl}: no blob content served for {file}");
                             }
                             complete = false;
@@ -453,7 +461,12 @@ pub(crate) async fn stage_vendor_sources_in_memory(
             }
         }
         if !failed.is_empty() {
-            if !quiet {
+            // An error, not progress chatter: the vendor caller only marks
+            // the envelope (printed exclusively under --json), so muting
+            // this under --silent meant exit 1 with zero output — the
+            // CLI_CONTRACT violation ("errors only", NEVER nothing) fixed
+            // for the disk stager's arms above.
+            if !common.json {
                 eprintln!(
                     "Error: could not fetch patch content for {} patch(es):",
                     failed.len()
@@ -785,4 +798,51 @@ mod tests {
         );
         assert!(!dst.join("subdir").exists(), "directories are not mirrored");
     }
+
+    /// The hardlink-failure copy fallback — the PRIMARY mirror path when
+    /// `.socket/` and the overlay tempdir sit on different filesystems
+    /// (EXDEV; e.g. tmpfs /tmp on Linux). Same-volume tempdirs always
+    /// hardlink, so force the arm deterministically: a DANGLING symlink at
+    /// the destination makes `metadata` err (follows the link — the
+    /// existing-file skip does not fire), makes `hard_link` fail (the link
+    /// occupies the path), and lets `copy` succeed by writing THROUGH the
+    /// link into its target.
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn overlay_dir_falls_back_to_copy_when_hardlink_fails() {
+        let tmp = tempfile::tempdir().unwrap();
+        let src = tmp.path().join("src");
+        let dst = tmp.path().join("dst");
+        std::fs::create_dir_all(&src).unwrap();
+        std::fs::create_dir_all(&dst).unwrap();
+        std::fs::write(src.join("a"), b"from-src").unwrap();
+        // Dangling link: the target does not exist yet.
+        let resolved = tmp.path().join("resolved");
+        std::os::unix::fs::symlink(&resolved, dst.join("a")).unwrap();
+
+        overlay_dir(&src, &dst).await;
+
+        // hard_link never replaces an occupied path, so the entry must
+        // still be the symlink — the bytes can only have arrived via the
+        // copy arm.
+        assert!(
+            dst.join("a")
+                .symlink_metadata()
+                .unwrap()
+                .file_type()
+                .is_symlink(),
+            "the destination entry stays a symlink (hard_link cannot have run)"
+        );
+        assert_eq!(
+            std::fs::read(dst.join("a")).unwrap(),
+            b"from-src",
+            "the mirrored bytes are readable at the destination path"
+        );
+        assert_eq!(
+            std::fs::read(&resolved).unwrap(),
+            b"from-src",
+            "proof the copy arm ran: only a write-through-the-link copy \
+             creates the link target"
+        );
+    }
 }