Fix gitignored ledger/manifest silently dropped (#1638)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1510,6 +1510,7 @@ command's integration tests check its output.
 | `redirect_yarn_classic_dep_manifest_rewritten` | `warnings[]` (warning) | scan/get `--mode hosted` (yarn classic): the served tarball's own `package.json` declares other dependencies than the pinned block's sub-maps, every descriptor already locked; the block's `dependencies:` / `optionalDependencies:` sub-maps are rewritten to match, in the shape yarn 1 writes for that manifest (#591, #1558). |
 | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, every PyPI flavor, and the JVM trees of Maven, Gradle, sbt and scala-cli): inside a git work tree, `git check-ignore --no-index` reports the new artifact's uuid directory (JVM: its tree root, such as `.socket/vendor/maven2`) as ignored by a rule its own `.gitignore` cannot override (such as a root `.socket/` or `vendor/` rule; the detail names the rule). Remedy: drop that rule for `.socket/vendor/`. Refused before any write, dry run included. A file rule such as `*.tgz`, `*.nupkg`, `*.whl` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root (PyPI's uuid dir also gets the `.gitattributes` that unsets a `*.whl filter=lfs` rule); if a written tarball, directory payload, nupkg or wheel still reads as ignored, the run refuses and removes the uuid dir it created. |
 | `vendor_artifact_gitignore_unchecked` | warning | vendor (vlt, the npm-family tarball flavors, NuGet and every PyPI flavor): git is installed but could not answer the ignore check for the written vendored directory (it failed to start, ran past 30 s, or `rev-parse` / `check-ignore` exited with an error); the package is vendored and the detail names what failed. Remedy: make sure no ignore rule covers `.socket/` before committing. Git absent, or a project outside any work tree, raises nothing. |
+| `vendor_ledger_gitignored` | top-level error (exit 1) | vendor / scan / get `--mode vendored` (every backend) and the hosted→vendored eject: inside a git work tree, `git check-ignore` reports the vendor ledger `.socket/vendor/state.json` as ignored by a file rule (such as `state.json`, `*.json` or `**/state.json`; the message names it), so the commit the run asks for would leave it out and no other checkout could `rollback`, `list` or `vendor --revert` the patches (#1638). A ledger git already tracks is never refused. Refused before any write, dry run included. Remedy: drop the rule or add `!.socket/vendor/state.json`. A rule that ignores `.socket/` or `.socket/vendor/` as a directory is `vendor_artifact_gitignored`: per package from the backends that check it themselves (the npm family, NuGet, PyPI, the JVM trees), and as this same run-level refusal when the run includes any other ecosystem. |
 | `vendor_ledger_entry_missing` | `failed` | vendor (vlt): the only installed copy is vlt's link to a committed vendored directory, but the vendor ledger has no entry for the package; restore `.socket/vendor/state.json` from version control (v5.0: `repair` no longer re-synthesizes it). Replaces the `package_not_installed` skip. |
 | `vendor_variant_ambiguous` | `failed` | vendor / scan / get `--mode vendored` (pypi, gem): the package is not installed and the manifest holds several release variants of it (`?artifact_id=` / `?platform=`), none of which the vendor ledger records, so nothing says which distribution to vendor; install the package or keep one release variant. A variant the ledger records (at any patch uuid) is taken as the wired one and its siblings are left out without an event. |
 | `vendor_artifact_missing` | reason | The recorded artifact is missing; repair requires an online exact redownload. |
@@ -1700,7 +1701,7 @@ One command is **intentionally not** plain-envelope and will stay that way (not
 }
 ```
 
-**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` for each recorded patch it patched (or found patched) and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found.
+**Events per mode.** Agent mode (`get`, `scan --mode agent` / `--sync`): a fetched patch is `downloaded` (new manifest record) or `updated` (+`oldUuid`: it replaced another uuid), one already recorded at the same uuid is `skipped` / `already_in_manifest` (it is still re-applied), a patch that could not be fetched or recorded is `failed` (`download_failed`, `patch_no_applicable_files`, `blob_write_failed`); then the nested apply adds `applied` for each recorded patch it patched (or found patched) and `failed` (apply's `apply_failed` / `package_not_installed` with its `error`, #424) for each it failed — a failing manifest patch the run did not select (the nested apply covers the whole `--ecosystems`-scoped manifest) gets its own `failed` event, and a failure no single patch explains (an unreadable manifest, the yarn PnP refusal, unavailable patch sources) is a purl-less `failed` event, so a failed apply is always `partialFailure` (exit 1). The apply's mismatch overwrites (#1004) are `content_mismatch_overwritten` warnings; a record whose purl the vendor ledger wires at another uuid adds a `vendored_uuid_drift` warning. A run that leaves patches recorded in a manifest git would not commit (inside a git work tree, `git check-ignore` matches the manifest's project-relative path, e.g. a `manifest.json`, `*.json` or `.socket/` rule; a tracked manifest never warns) adds a `manifest_gitignored` warning naming the rule: a fresh checkout's `apply` would find no manifest and apply nothing (#1638). It stays a warning (exit unchanged) because a pipeline that scans and applies in one job never commits `.socket/`. Vendor-owned and lockfile-only selections are `skipped` / `vendored` and `skipped` / `package_not_installed` before any download. Hosted and vendored events are described under "Scan modes" above; every one carries `details.mode`. `get`'s narrowing adds `skipped` events (`package_not_installed`, `yarn_pnp_unsupported`, `pnpm_pnp_unsupported`) and `paidRequired` runs one `skipped` / `paid_required` event per patch found.
 
 **Patch metadata.** `downloaded` / `updated` events (agent and vendored) carry the patch metadata in `details`, the same keys for every command:
 

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

@@ -1320,6 +1320,45 @@ pub(crate) async fn download_patch_records_preflighted(
     }
 }
 
+/// Warning code: git would not commit the manifest (see
+/// [`warn_if_manifest_gitignored`]).
+pub(crate) const MANIFEST_GITIGNORED: &str = "manifest_gitignored";
+
+/// Warn (stderr `[warn]` + `warnings[]`) when git's ignore rules keep the
+/// manifest this run recorded patches in out of the commit (#1638): the
+/// blobs are committed, but a fresh checkout's `socket-patch apply` finds
+/// no manifest, applies nothing and exits 0. Only a warning, because a
+/// pipeline that scans and applies in one job never commits `.socket/`.
+/// A manifest outside the project root is not checked.
+pub(crate) async fn warn_if_manifest_gitignored(
+    project_root: &Path,
+    manifest_path: &Path,
+    quiet: bool,
+    warnings: &mut Vec<RunWarning>,
+) {
+    let Ok(rel) = manifest_path.strip_prefix(project_root) else {
+        return;
+    };
+    let rel: Vec<String> = rel
+        .components()
+        .map(|c| c.as_os_str().to_string_lossy().into_owned())
+        .collect();
+    let rel = rel.join("/");
+    let Some(rules) = socket_patch_core::vendor::file_gitignored(project_root, &rel).await else {
+        return;
+    };
+    let w = format!(
+        "git would not commit {rel} ({rules}), so `socket-patch apply` in other checkouts \
+         (CI, a fresh clone) would find no manifest and apply nothing; remove or narrow that \
+         rule (a `!{rel}` line re-includes the file only when no parent directory is \
+         ignored)"
+    );
+    if !quiet {
+        eprintln!("  [warn] {w}");
+    }
+    warnings.push(RunWarning::new(MANIFEST_GITIGNORED, w));
+}
+
 /// Warning code: the manifest now records a patch uuid the vendor ledger
 /// does not wire for that purl (see [`warn_on_vendored_uuid_drift`]).
 pub(crate) const VENDORED_UUID_DRIFT: &str = "vendored_uuid_drift";
@@ -1712,6 +1751,10 @@ pub async fn download_and_apply_patches_into(
     // download.) The nested apply below skips the vendored purl either way.
     let mut warnings = release_warnings(batch.warnings);
     warn_on_vendored_uuid_drift(&params.project_root(), quiet, &changed, &mut warnings).await;
+    if !recorded.is_empty() {
+        warn_if_manifest_gitignored(&params.project_root(), &manifest_path, quiet, &mut warnings)
+            .await;
+    }
     env.warnings.extend(warnings);
 
     if !quiet {

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

@@ -31,8 +31,9 @@ use crate::commands::agent_download::{
     decide_patch_action, download_patch_records_preflighted, download_patch_records_reusing,
     filter_to_installed_releases, max_vuln_severity, nested_apply_args, patch_event_metadata,
     record_apply_outcome, report_lock_failure, run_nested_apply, run_outcome, tag_mode,
-    unwind_new_blobs, warn_on_vendored_uuid_drift, write_all_patch_blobs, DetachedDownload,
-    PatchAction, VendorRefusals, ALREADY_IN_MANIFEST, BLOB_WRITE_FAILED, RELEASE_NARROWING,
+    unwind_new_blobs, warn_if_manifest_gitignored, warn_on_vendored_uuid_drift,
+    write_all_patch_blobs, DetachedDownload, PatchAction, VendorRefusals, ALREADY_IN_MANIFEST,
+    BLOB_WRITE_FAILED, RELEASE_NARROWING,
 };
 pub use crate::commands::agent_download::{
     download_and_apply_patches_into, download_and_apply_patches_with, DownloadParams, DownloadRun,
@@ -2059,6 +2060,13 @@ async fn save_and_apply_patch(
         warn_on_vendored_uuid_drift(&args.common.project_root(), quiet, &recorded, &mut warnings)
             .await;
     }
+    warn_if_manifest_gitignored(
+        &args.common.project_root(),
+        &manifest_path,
+        quiet,
+        &mut warnings,
+    )
+    .await;
 
     // Progress narration goes to stderr, like the search path's.
     if !quiet {

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

@@ -1397,6 +1397,19 @@ async fn run_check(args: &VendorArgs) -> i32 {
     i32::from(env.summary.failed != 0)
 }
 
+/// Whether every ecosystem in a vendored run has a backend that refuses a
+/// `.socket/vendor/` directory ignore rule per package
+/// (`vendor_artifact_gitignored`): the npm family (vlt included), NuGet,
+/// PyPI and the JVM trees. The others rely on the run-level ledger check.
+fn artifacts_gitignore_checked(mut ecosystems: impl Iterator<Item = Ecosystem>) -> bool {
+    ecosystems.all(|eco| {
+        matches!(
+            eco,
+            Ecosystem::Npm | Ecosystem::Nuget | Ecosystem::Pypi | Ecosystem::Maven
+        )
+    })
+}
+
 /// A refused eject: the JSON error envelope (`status: error`) or an
 /// `Error:` line (printed even under `--silent`). Exit 1; nothing touched.
 fn emit_eject_refusal(common: &GlobalArgs, code: &str, message: &str) -> i32 {
@@ -1728,6 +1741,15 @@ impl EjectSnapshot {
 /// the project to upstream, not to hosted).
 async fn run_eject(args: &VendorArgs, pins: Vec<HostedPin>) -> i32 {
     let common = &args.common;
+    // A ledger git would not commit refuses before anything is fetched or
+    // restored, so the project stays hosted (#1638).
+    let purls: Vec<String> = pins.iter().map(|pin| pin.purl.clone()).collect();
+    let artifacts_checked = artifacts_gitignore_checked(partition_purls(&purls, None).into_keys());
+    if let Some((code, detail)) =
+        vendor::ledger_gitignore_refusal(&common.cwd, artifacts_checked).await
+    {
+        return emit_eject_refusal(common, code, &detail);
+    }
     let (client, use_public_proxy) =
         get_api_client_with_overrides(common.api_client_overrides()).await;
     let telemetry = TelemetryAuth::for_client(&client);
@@ -2761,6 +2783,24 @@ pub(crate) async fn vendor_records_reusing(
             return true;
         }
     };
+    // The ledger must survive the commit the run asks for, or no other
+    // checkout can roll the patches back (#1638): refuse before anything
+    // is written, dry run included.
+    let artifacts_checked = artifacts_gitignore_checked(
+        vendorable_partition
+            .iter()
+            .filter(|(_, purls)| !purls.is_empty())
+            .map(|(eco, _)| *eco),
+    );
+    if let Some((code, detail)) =
+        vendor::ledger_gitignore_refusal(&common.cwd, artifacts_checked).await
+    {
+        if !common.json {
+            eprintln!("Error: {detail}");
+        }
+        env.mark_error(EnvelopeError::new(code, detail));
+        return true;
+    }
     // Pre-stage trees a previous run left behind (it crashed or was
     // interrupted between staging an archive and settling): nothing of
     // this run is staged yet, and the caller holds the apply lock, so every

crates/socket-patch-cli/tests/get/get_failure_paths.rs

@@ -1699,3 +1699,121 @@ async fn get_uuid_json_mismatch_overwrite_is_reported() {
     assert!(!stderr.contains("did not match"), "stderr={stderr}");
     assert_eq!(std::fs::read(&installed).unwrap(), AFTER_BYTES);
 }
+
+/// `git init` `root` with `gitignore`; `false` when git is not installed.
+fn git_project_ignoring(root: &std::path::Path, gitignore: &str) -> bool {
+    if socket_patch_core::utils::process::resolve_tool("git").is_none() {
+        return false;
+    }
+    assert!(std::process::Command::new("git")
+        .arg("-C")
+        .arg(root)
+        .args(["init", "-q"])
+        .status()
+        .unwrap()
+        .success());
+    std::fs::write(root.join(".gitignore"), gitignore).unwrap();
+    true
+}
+
+/// Assert the envelope's `manifest_gitignored` warning names `rule`, or
+/// that there is none.
+fn assert_manifest_warning(json: &serde_json::Value, rule: Option<&str>, what: &str) {
+    let warning = json["warnings"]
+        .as_array()
+        .into_iter()
+        .flatten()
+        .find(|w| w["code"] == "manifest_gitignored");
+    match rule {
+        Some(rule) => {
+            let detail = warning.unwrap_or_else(|| panic!("{what}: json={json}"))["detail"]
+                .as_str()
+                .unwrap();
+            assert!(
+                detail.contains(".socket/manifest.json") && detail.contains(rule),
+                "{detail}"
+            );
+        }
+        None => assert!(warning.is_none(), "{what}: json={json}"),
+    }
+}
+
+/// An agent-mode run that records a patch in a manifest git would not
+/// commit (`manifest.json`, `*.json` + `!package.json`, or a `.socket/`
+/// directory rule) warns `manifest_gitignored`, naming the rule: otherwise
+/// the blobs are committed without the manifest and a fresh checkout's
+/// `apply` silently applies nothing (#1638). A re-included manifest gets
+/// no warning.
+#[tokio::test]
+#[serial]
+async fn agent_record_warns_when_git_ignores_the_manifest() {
+    let server = MockServer::start().await;
+    mount_view_files(&server, UUID, PURL, good_files()).await;
+    let selected = vec![search_result(UUID, PURL)];
+    for (gitignore, rule) in [
+        ("manifest.json\n", Some("manifest.json")),
+        ("*.json\n!package.json\n", Some("*.json")),
+        (".socket/\n", Some(".socket/")),
+        ("*.json\n!package.json\n!.socket/manifest.json\n", None),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        if !git_project_ignoring(tmp.path(), gitignore) {
+            return;
+        }
+        let (code, json) =
+            download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await;
+        assert_eq!(code, 0, "{gitignore:?}: json={json}");
+        assert!(tmp.path().join(".socket/manifest.json").is_file());
+        assert_manifest_warning(&json, rule, gitignore);
+    }
+}
+
+/// A manifest git already tracks keeps being committed whatever the ignore
+/// rules say, so a re-run after a `*.json` rule lands does not warn.
+#[tokio::test]
+#[serial]
+async fn agent_record_does_not_warn_for_a_tracked_manifest() {
+    let server = MockServer::start().await;
+    mount_view_files(&server, UUID, PURL, good_files()).await;
+    let selected = vec![search_result(UUID, PURL)];
+    let tmp = tempfile::tempdir().unwrap();
+    if !git_project_ignoring(tmp.path(), "") {
+        return;
+    }
+    let (code, json) =
+        download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await;
+    assert_eq!(code, 0, "json={json}");
+    assert!(std::process::Command::new("git")
+        .arg("-C")
+        .arg(tmp.path())
+        .args(["add", ".socket/manifest.json"])
+        .status()
+        .unwrap()
+        .success());
+    std::fs::write(tmp.path().join(".gitignore"), "*.json\n").unwrap();
+    let (code, json) =
+        download_and_apply_patches(&selected, &engine_params(tmp.path()), &server.uri()).await;
+    assert_eq!(code, 0, "json={json}");
+    assert_manifest_warning(&json, None, "tracked manifest");
+}
+
+/// `get <uuid>` records through its own save path, which warns the same
+/// way.
+#[tokio::test]
+async fn get_uuid_warns_when_git_ignores_the_manifest() {
+    let server = MockServer::start().await;
+    mount_view_files(&server, UUID, PURL, good_files()).await;
+    let tmp = tempfile::tempdir().unwrap();
+    if !git_project_ignoring(tmp.path(), "manifest.json\n") {
+        return;
+    }
+    let (code, stdout, stderr) =
+        run_get_bin(tmp.path(), &server.uri(), &[UUID, "--save-only", "--json"]);
+    assert_eq!(code, 0, "stdout={stdout}\nstderr={stderr}");
+    assert!(tmp.path().join(".socket/manifest.json").is_file());
+    assert_manifest_warning(
+        &parse_json_envelope(&stdout),
+        Some("manifest.json"),
+        "get <uuid>",
+    );
+}