Fix cargo cold-cache apply exiting 0 (#616)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1252,7 +1252,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
 | Tag                       | Action(s)        | Context |
 |---------------------------|------------------|---------|
 | `already_patched`         | `skipped`        | apply: every file's hash already matches `afterHash`. |
-| `package_not_installed`   | `skipped`        | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). |
+| `package_not_installed`   | `skipped`        | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). **Cargo is never lockfile-only (v5.0, #616)**: `cargo fetch` unpacks every `Cargo.lock` entry (target-gated ones included), so a locked crate missing from `$CARGO_HOME/registry/src` was simply not fetched yet (a cold CI cache, a pruned `registry/src`) and the next `cargo build` would compile it unpatched; it counts as an unresolved purl (an all-miss run exits 1, as in 4.x) and its detail, the human error and the human warning say to run `cargo fetch` first. |
 | `apply_failed`            | `failed`         | apply: hash mismatch, write error, archive read error. |
 | `no_local_source` | `skipped`/`failed` | Agent patch application cannot obtain the required local or downloaded patch source. Vendored mode consumes complete server artifacts and no longer stages patch blobs. |
 | `offline_missing_sources` / `sources_download_failed` | apply run-level `warnings[]` | apply (additive): the patch sources were unavailable — `--offline` with no local source, or the download left a patch with no source — so nothing was attempted. The envelope keeps its pinned shape (`partialFailure`, empty `events[]`, zero summary, no top-level `error`); the warning is its machine-readable reason (the human path prints the staging `Error:` line on stderr instead, even under `--silent`). |

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

@@ -1271,9 +1271,9 @@ fn collect_apply_failures(
     }
     for purl in unresolved_purls(unmatched, lockfile_only) {
         failures.push(ApplyFailure {
-            purl,
             code: "package_not_installed".to_string(),
-            error: "No installed package matches this PURL".to_string(),
+            error: not_installed_detail(&purl).to_string(),
+            purl,
         });
     }
     failures
@@ -1540,7 +1540,7 @@ pub(crate) async fn run_locked(
                     let detail = if lockfile_only.contains(purl) {
                         LOCKFILE_ONLY_DETAIL
                     } else {
-                        "No installed package matches this PURL"
+                        not_installed_detail(purl)
                     };
                     env.record(
                         PatchEvent::new(PatchAction::Skipped, purl.clone())
@@ -2722,6 +2722,9 @@ async fn apply_patches_inner(
         for purl in &unresolved {
             eprintln!("  - {}", normalize_purl(purl));
         }
+        if let Some(line) = cargo_fetch_hint(&unresolved) {
+            eprintln!("{line}");
+        }
     }
     print_lockfile_only_note(args, &unmatched, &lockfile_only);
 
@@ -3270,7 +3273,8 @@ const LOCKFILE_ONLY_DETAIL: &str =
 /// `@esbuild/<os>-<cpu>`), a devDependency under `npm ci --omit=dev`. The
 /// tree is in its correct end state, so they are calm skips, as `scan
 /// --mode agent` treats lockfile-only packages. Global runs have no project
-/// lock, so nothing is lockfile-resolved there.
+/// lock, so nothing is lockfile-resolved there, and neither is a Cargo
+/// crate ([`lock_resolution_is_calm`]).
 async fn lockfile_resolved(common: &GlobalArgs, unmatched: &[String]) -> HashSet<String> {
     if unmatched.is_empty() || common.is_global() {
         return HashSet::new();
@@ -3280,11 +3284,36 @@ async fn lockfile_resolved(common: &GlobalArgs, unmatched: &[String]) -> HashSet
     let lock_purls: HashSet<PurlKey> = entries.iter().map(|e| PurlKey::new(&e.purl)).collect();
     unmatched
         .iter()
-        .filter(|p| lock_purls.contains(&PurlKey::new(p)))
+        .filter(|p| lock_resolution_is_calm(p) && lock_purls.contains(&PurlKey::new(p)))
         .cloned()
         .collect()
 }
 
+/// Whether a lock-resolved but uninstalled `purl` can be a calm skip.
+/// Not for Cargo (#616): cargo leaves no locked crate out on purpose —
+/// `cargo fetch` unpacks every `Cargo.lock` entry, target-gated ones
+/// included — so a locked crate missing from `$CARGO_HOME/registry/src`
+/// was just not fetched yet (a cold CI cache, a pruned `registry/src`),
+/// and the next `cargo build` downloads or re-extracts it UNPATCHED.
+fn lock_resolution_is_calm(purl: &str) -> bool {
+    Ecosystem::from_purl(purl) != Some(Ecosystem::Cargo)
+}
+
+/// The `package_not_installed` detail of an unmatched purl the lock does
+/// not calmly account for: Cargo's carries the `cargo fetch` remedy.
+fn not_installed_detail(purl: &str) -> &'static str {
+    if Ecosystem::from_purl(purl) == Some(Ecosystem::Cargo) {
+        CARGO_NOT_FETCHED_DETAIL
+    } else {
+        "No installed package matches this PURL"
+    }
+}
+
+/// [`not_installed_detail`] for a Cargo crate.
+const CARGO_NOT_FETCHED_DETAIL: &str =
+    "No unpacked crate source matches this PURL; run `cargo fetch` first so the crate is in \
+     the Cargo registry cache, then re-run apply";
+
 /// The `unmatched` purls with no lock evidence (sorted input, sorted
 /// output): the ones that can still fail an all-miss run.
 fn unresolved_purls(unmatched: &[String], lockfile_only: &HashSet<String>) -> Vec<String> {
@@ -3345,9 +3374,24 @@ fn format_none_installed_error(unmatched: &[String]) -> Vec<String> {
         "Check that the packages are installed and --cwd points to the right directory."
             .to_string(),
     );
+    if let Some(line) = cargo_fetch_hint(unmatched) {
+        lines.push(line.to_string());
+    }
     lines
 }
 
+/// The `cargo fetch` remedy line when any of `unmatched` is a Cargo crate
+/// (#616): cargo unpacks locked crates only when it fetches or builds.
+fn cargo_fetch_hint(unmatched: &[String]) -> Option<&'static str> {
+    unmatched
+        .iter()
+        .any(|p| Ecosystem::from_purl(p) == Some(Ecosystem::Cargo))
+        .then_some(
+            "Cargo crates are patched in the registry cache: run `cargo fetch` first so every \
+             locked crate is unpacked there, then re-run apply.",
+        )
+}
+
 #[cfg(test)]
 mod tests {
     //! Tests for `result_to_event` — the per-package → per-patch event

crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs

@@ -280,3 +280,113 @@ fn no_lockfile_all_miss_still_fails() {
         "{stderr}"
     );
 }
+
+// ---- Cargo: lock-resolved but not fetched is NOT a calm skip (#616) ----
+//
+// A Cargo.lock entry with no unpacked source in `$CARGO_HOME/registry/src`
+// was not deliberately left out: cargo simply has not fetched it yet (a
+// fresh CI runner / clone, or a pruned `registry/src`). The next `cargo
+// build` downloads or re-extracts it UNPATCHED, so `apply` must fail the
+// all-miss run like 4.0.0 did, and tell the user to `cargo fetch` first.
+
+const CARGO_PURL: &str = "pkg:cargo/cfg-if@1.0.0";
+
+/// A binary crate locking `cfg-if 1.0.0` from crates.io.
+fn write_cargo_project(root: &Path) {
+    std::fs::write(
+        root.join("Cargo.toml"),
+        "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n\
+         [dependencies]\ncfg-if = \"=1.0.0\"\n",
+    )
+    .unwrap();
+    std::fs::write(
+        root.join("Cargo.lock"),
+        "# This file is automatically @generated by Cargo.\n\
+         # It is not intended for manual editing.\n\
+         version = 4\n\n\
+         [[package]]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\n \"cfg-if\",\n]\n\n\
+         [[package]]\nname = \"cfg-if\"\nversion = \"1.0.0\"\n\
+         source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\
+         checksum = \"baf1de4339761588bc0d7c0e2b9b9d4ab8c4fa8c1f6c2fb5b9a6e7c3a7d2b0e1\"\n",
+    )
+    .unwrap();
+    std::fs::create_dir_all(root.join("src")).unwrap();
+    std::fs::write(root.join("src/main.rs"), "fn main() {}\n").unwrap();
+}
+
+fn run_cargo_apply(cwd: &Path, cargo_home: &Path, args: &[&str]) -> (i32, String, String) {
+    let mut argv: Vec<&str> = vec!["apply"];
+    argv.extend_from_slice(args);
+    let home = cargo_home.to_str().unwrap();
+    run_with_env(
+        cwd,
+        &argv,
+        &[("SOCKET_TELEMETRY_DISABLED", "1"), ("CARGO_HOME", home)],
+    )
+}
+
+fn assert_cold_cargo_apply_fails(project: &Path, cargo_home: &Path) {
+    let (code, stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline", "--json"]);
+    let v = parse_json_envelope(&stdout);
+    assert_eq!(
+        code, 1,
+        "an unfetched cargo crate must fail apply, not skip calmly; {v}\n{stderr}"
+    );
+    assert_eq!(v["status"], "partialFailure", "{v}");
+    let ev = event(&v, CARGO_PURL);
+    assert_eq!(ev["errorCode"], "package_not_installed", "{ev}");
+    let detail = ev.to_string();
+    assert!(
+        detail.contains("cargo fetch") && !detail.contains("lockfile-only"),
+        "the event must carry the cargo fetch remedy, not the calm lockfile-only detail; {ev}"
+    );
+
+    // Human path: the exit-flipping error plus the actionable remedy.
+    let (code, _stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline"]);
+    assert_eq!(code, 1, "{stderr}");
+    assert!(
+        stderr.contains("Error: The targeted manifest patch matched no installed package:")
+            && stderr.contains(&format!("  - {CARGO_PURL}"))
+            && stderr.contains("cargo fetch"),
+        "the error names the crate and tells the user to run `cargo fetch`; {stderr}"
+    );
+
+    // `--silent` (the hook / CI shape) still prints the error and exits 1.
+    let (code, _stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline", "--silent"]);
+    assert_eq!(code, 1, "{stderr}");
+    assert!(stderr.contains("cargo fetch"), "{stderr}");
+}
+
+/// #616: an empty `$CARGO_HOME` (fresh CI runner): the crate is locked but
+/// was never downloaded.
+#[test]
+fn cargo_crate_on_cold_registry_cache_fails_with_fetch_remedy() {
+    let tmp = tempfile::tempdir().unwrap();
+    let project = tmp.path().join("app");
+    std::fs::create_dir_all(&project).unwrap();
+    write_cargo_project(&project);
+    write_manifest(&project, &[CARGO_PURL]);
+    let cargo_home = tmp.path().join("cargo-home");
+    std::fs::create_dir_all(&cargo_home).unwrap();
+
+    assert_cold_cargo_apply_fails(&project, &cargo_home);
+}
+
+/// #616 pruned variant: the `.crate` archive is cached but `registry/src`
+/// was pruned, so cargo re-extracts the pristine source on the next build.
+#[test]
+fn cargo_crate_with_pruned_registry_src_fails_with_fetch_remedy() {
+    let tmp = tempfile::tempdir().unwrap();
+    let project = tmp.path().join("app");
+    std::fs::create_dir_all(&project).unwrap();
+    write_cargo_project(&project);
+    write_manifest(&project, &[CARGO_PURL]);
+    let cargo_home = tmp.path().join("cargo-home");
+    let cache = cargo_home.join("registry/cache/index.crates.io-1949cf8c6b5b557f");
+    std::fs::create_dir_all(&cache).unwrap();
+    std::fs::write(cache.join("cfg-if-1.0.0.crate"), b"not a real archive").unwrap();
+    std::fs::create_dir_all(cargo_home.join("registry/src/index.crates.io-1949cf8c6b5b557f"))
+        .unwrap();
+
+    assert_cold_cargo_apply_fails(&project, &cargo_home);
+}

docs/ecosystems.md

@@ -947,6 +947,13 @@ crate that means the **shared** `$CARGO_HOME/registry` cache: the patch affects
 project on the machine, and is silently reset by `cargo clean` or a cache prune. Use
 `--mode vendored` for a project-local, committable patch.
 
+Run `cargo fetch` before `apply` on a fresh checkout or CI runner. Cargo unpacks a
+locked crate into `registry/src` only when it fetches or builds, so on a cold or pruned
+cache there is nothing to patch, and the next `cargo build` would compile the pristine
+crate. `apply` therefore treats a crate that `Cargo.lock` resolves but that is not
+unpacked as not installed, not as a calm lockfile-only skip: a run where no targeted
+patch matched exits 1 and names `cargo fetch` as the remedy.
+
 ## Cargo: vendored wiring in Cargo.toml
 
 Vendored mode (v5+) wires a patched crate with a `[patch.crates-io]` path