From 6001c1918dbf1c3baea5946e4b8a1572ee47885b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:23:09 +0000 Subject: [PATCH 1/7] Start fix for #1638 Assisted-by: Claude Code:claude-opus-5-5 From 1a5ccb9fa6039c673747d84da0276c0e7290c484 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:31:02 +0000 Subject: [PATCH 2/7] Refuse vendoring when git ignores the ledger A .gitignore rule such as state.json or *.json kept .socket/vendor/state.json out of the commit a vendored run asks for. The scan exited 0, but every other checkout then had the lock wiring and no ledger, so rollback failed with vendor_ledger_missing and list showed no patches. Vendored runs (vendor, scan/get --mode vendored, every backend) now ask git before writing anything and refuse with vendor_ledger_gitignored, naming the rule, dry run included. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 1 + .../socket-patch-cli/src/commands/vendor.rs | 10 ++ .../tests/in_process_vendor.rs | 93 +++++++++++++++++++ crates/socket-patch-core/src/vendor/mod.rs | 3 + .../socket-patch-core/src/vendor/npm_dir.rs | 37 ++++++++ 5 files changed, 144 insertions(+) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 830bb561d..4230aea21 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1508,6 +1508,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 (#591). | | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, 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` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root; if a written tarball, directory payload or nupkg 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 and NuGet): 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): inside a git work tree, `git check-ignore --no-index` 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). 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 reported per package as `vendor_artifact_gitignored` instead. | | `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. | diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 3516e7c49..1cecd72ae 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -2761,6 +2761,16 @@ 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. + if let Some((code, detail)) = vendor::ledger_gitignore_refusal(&common.cwd).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 diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index bd3377f85..f26478bb1 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -457,6 +457,74 @@ async fn dry_run_writes_nothing() { assert_eq!(std::fs::read(fx.installed_index()).unwrap(), ORIG_INDEX); } +/// `git init` `root` and write its `.gitignore`. `false` when git is not +/// installed (the probe has nothing to ask, so the test has nothing to +/// check). +fn git_project_ignoring(root: &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 +} + +/// A `.gitignore` file rule that matches the vendor ledger (`state.json`, +/// `*.json`) would leave it out of the commit the run asks for, so every +/// other checkout has the lock wiring but no ledger and `rollback` fails +/// with `vendor_ledger_missing` (#1638). The run refuses before writing +/// anything, dry run included. +#[tokio::test] +async fn a_gitignored_ledger_refuses_before_any_write() { + for (gitignore, rule) in [ + ("node_modules/\nstate.json\n", "state.json"), + ("node_modules/\n*.json\n!package*.json\n", "*.json"), + ("node_modules/\n**/state.json\n", "**/state.json"), + ] { + for dry_run in [false, true] { + let fx = npm_fixture(); + if !git_project_ignoring(fx.root(), gitignore) { + return; + } + let extra: &[&str] = if dry_run { &["--dry-run"] } else { &[] }; + let (code, env) = vendor_cli(fx.root(), extra); + assert_eq!(code, 1, "{gitignore:?} dry_run={dry_run}: {env:#}"); + assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); + let message = env["error"]["message"].as_str().unwrap(); + assert!( + message.contains(".socket/vendor/state.json") && message.contains(rule), + "the message names the ledger and the rule: {message}" + ); + assert!( + !fx.vendor_dir().exists(), + "nothing is vendored for {gitignore:?}" + ); + assert_eq!(fx.lock_bytes(), fx.original_lock, "lock byte-identical"); + } + } +} + +/// A rule that re-includes the ledger lets the run vendor as usual. +#[tokio::test] +async fn a_re_included_ledger_vendors() { + let fx = npm_fixture(); + if !git_project_ignoring( + fx.root(), + "node_modules/\n*.json\n!package*.json\n!.socket/vendor/state.json\n", + ) { + return; + } + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + assert!(fx.state_path().is_file()); + assert!(fx.tgz_path().is_file()); +} + // ───────────────────────────────────────────────────────────────────── // 4. vendor → revert round-trip // ───────────────────────────────────────────────────────────────────── @@ -3515,6 +3583,31 @@ fn run_scan_vendor(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, Value) /// halves and prunes the vendor tree. Manifest-agnostic on purpose: the /// vendored mode never writes `.socket/manifest.json` (its ledger owns the /// entries), so nothing here reads one. +/// The ledger check sits in front of every vendored backend, not only the +/// npm family the report used (#1638): a gem `scan --mode vendored` whose +/// `.gitignore` matches `state.json` refuses before anything is written. +#[tokio::test] +async fn scan_vendor_gem_refuses_a_gitignored_ledger() { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + if !git_project_ignoring(fx.root(), "vendor/bundle/\nstate.json\n") { + return; + } + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 1, "{env:#}"); + assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); + assert!(!fx.root().join(".socket/vendor").exists(), "{env:#}"); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile.lock")).unwrap(), + GEM_LOCK + ); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile")).unwrap(), + GEM_GEMFILE + ); +} + #[tokio::test] async fn scan_vendor_gem_end_to_end_and_reverts() { let mock = wiremock::MockServer::start().await; diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index ba29bb238..2de8e18cb 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -134,6 +134,9 @@ pub use verify::{ // The hosted→vendored takeover refuses a berry project the backend would // refuse BEFORE it reverts the hosted redirect. pub use npm_common::npm_tarball_gitignore_preflight; +// Git's ignore rules for the state files a vendored or agent-mode run asks +// the user to commit (#1638). +pub use npm_dir::{file_gitignored, ledger_gitignore_refusal, LEDGER_GITIGNORED}; pub use npm_lock::npm_lock_vendor_preflight; pub use yarn_berry_lock::{yarn_berry_vendor_preflight, yarn_berry_vendor_target_preflight}; diff --git a/crates/socket-patch-core/src/vendor/npm_dir.rs b/crates/socket-patch-core/src/vendor/npm_dir.rs index 5088fe54b..2bf356669 100644 --- a/crates/socket-patch-core/src/vendor/npm_dir.rs +++ b/crates/socket-patch-core/src/vendor/npm_dir.rs @@ -534,6 +534,43 @@ pub(crate) async fn ignored_root_refusal( pub(crate) const GITIGNORED: &str = "vendor_artifact_gitignored"; +/// The vendor ledger is ignored by git, so the commit the vendored workflow +/// ends with would leave it out (#1638). +pub const LEDGER_GITIGNORED: &str = "vendor_ledger_gitignored"; + +/// The `vendor_ledger_gitignored` refusal (code, detail) when git would not +/// commit `.socket/vendor/state.json`: a file rule such as `state.json` or +/// `*.json` that no file socket-patch writes overrides. Every other +/// checkout would then get the lock wiring and the artifacts but no ledger, +/// so `rollback`, `list` and `vendor --revert` there cannot see the patch. +/// `None` when `.socket/vendor/` is ignored as a directory: the backends' +/// own `vendor_artifact_gitignored` refusals report that rule per package. +pub async fn ledger_gitignore_refusal(project_root: &Path) -> Option<(&'static str, String)> { + let ledger = super::state::VENDOR_STATE_REL; + let dir = ledger.rsplit_once('/').map_or(ledger, |(dir, _)| dir); + if gitignored(project_root, &[format!("{dir}/")]) + .await + .is_some() + { + return None; + } + let rules = gitignored(project_root, &[ledger.to_string()]).await?; + Some(( + LEDGER_GITIGNORED, + format!( + "git would not commit the vendor ledger {ledger} ({rules}), so other checkouts \ + could not roll back or list the vendored patches; remove that rule or add \ + `!{ledger}` to .gitignore, then vendor again" + ), + )) +} + +/// The rules (as [`gitignored`] names them) that would keep git from +/// committing the project-relative file `rel`, or `None`. +pub async fn file_gitignored(project_root: &Path, rel: &str) -> Option { + gitignored(project_root, &[rel.to_string()]).await +} + /// The vendored dir was written, but git could not say whether it would /// commit it. pub(crate) const GITIGNORE_UNCHECKED: &str = "vendor_artifact_gitignore_unchecked"; From feeb51a970bb5e7dce199e113eb6210eb2df6da0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:37:17 +0000 Subject: [PATCH 3/7] Warn when git ignores the agent manifest A .gitignore rule such as manifest.json kept .socket/manifest.json out of the commit while its blobs went in. In CI or a fresh clone, socket-patch apply then found no manifest, applied nothing and exited 0, so the package stayed unpatched. Agent-mode scan and get now warn with manifest_gitignored, naming the rule, whenever the run leaves patches recorded in an ignored manifest. It stays a warning: a pipeline that scans and applies in one job never commits .socket/. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/commands/agent_download.rs | 42 +++++++++++++ .../tests/get/get_failure_paths.rs | 61 +++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 4230aea21..71fccd75b 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1698,7 +1698,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 --no-index` matches the manifest's project-relative path, e.g. a `manifest.json` or `*.json` rule) 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: diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index e23ef9658..ef37e9505 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -1320,6 +1320,44 @@ 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, +) { + let Ok(rel) = manifest_path.strip_prefix(project_root) else { + return; + }; + let rel: Vec = 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 that rule or \ + add `!{rel}` to .gitignore" + ); + 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 +1750,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(¶ms.project_root(), quiet, &changed, &mut warnings).await; + if !recorded.is_empty() { + warn_if_manifest_gitignored(¶ms.project_root(), &manifest_path, quiet, &mut warnings) + .await; + } env.warnings.extend(warnings); if !quiet { diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index 29b8e5b0d..bb84ba48e 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1699,3 +1699,64 @@ 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 +} + +/// An agent-mode run that records a patch in a manifest git would not +/// commit (`manifest.json`, or `*.json` + `!package.json`) 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")), + ("*.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()); + 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!("{gitignore:?}: json={json}")) + ["detail"] + .as_str() + .unwrap(); + assert!( + detail.contains(".socket/manifest.json") && detail.contains(rule), + "{detail}" + ); + } + None => assert!(warning.is_none(), "{gitignore:?}: json={json}"), + } + } +} From bd5ddd37194136421188adc9d84acdbe86f6bf97 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:39:41 +0000 Subject: [PATCH 4/7] Refuse hosted-to-vendored eject on ignored ledger The standalone vendor eject restores every hosted pin upstream before the vendored engine runs. With a .gitignore rule matching .socket/vendor/state.json it now refuses with vendor_ledger_gitignored before fetching or restoring anything, so the project stays hosted. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 5 ++ .../tests/in_process_vendor.rs | 46 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 1cecd72ae..a18a66d40 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1728,6 +1728,11 @@ impl EjectSnapshot { /// the project to upstream, not to hosted). async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let common = &args.common; + // A ledger git would not commit refuses before anything is fetched or + // restored, so the project stays hosted (#1638). + if let Some((code, detail)) = vendor::ledger_gitignore_refusal(&common.cwd).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); diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index f26478bb1..6a23f42db 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -4711,6 +4711,52 @@ snapshots: assert!(!root.join(".npmrc").exists()); } + /// Hosted → vendored in a git project whose `.gitignore` matches the + /// vendor ledger (#1638): both the eject (no manifest) and the + /// manifest-driven takeover refuse `vendor_ledger_gitignored` before + /// restoring anything, so the project stays hosted. + #[tokio::test] + #[serial] + async fn hosted_then_vendor_with_a_gitignored_ledger_keeps_the_hosted_wiring() { + if socket_patch_core::utils::process::resolve_tool("git").is_none() { + return; + } + for with_manifest in [false, true] { + let server = MockServer::start().await; + mock_hosted_api(&server).await; + let registry = mock_registry(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_package_lock_project(root); + assert!(super::git_project_ignoring( + root, + "node_modules/\nstate.json\n" + )); + assert_eq!(scan_run(hosted_args(root, server.uri())).await, 0); + let hosted_lock = std::fs::read(root.join("package-lock.json")).unwrap(); + let hosted_npmrc = std::fs::read(root.join(".npmrc")).unwrap(); + assert!(String::from_utf8_lossy(&hosted_lock).contains(HOSTED_URL)); + + if with_manifest { + seed_manifest_and_blob(root); + } + let (code, env) = vendor_online_cli(root, ®istry, PATCH_ORIGIN, &[]); + assert_eq!(code, 1, "with_manifest={with_manifest}: {env:#}"); + assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); + assert_eq!( + std::fs::read(root.join("package-lock.json")).unwrap(), + hosted_lock, + "the hosted lock must be untouched" + ); + assert_eq!( + std::fs::read(root.join(".npmrc")).unwrap(), + hosted_npmrc, + "the hosted .npmrc must be untouched" + ); + assert!(!root.join(".socket/vendor").exists(), "{env:#}"); + } + } + /// Hosted → vendored in a git project that ignores `.socket/` (#831): /// hosted mode writes nothing there, so the rule is common. The /// gitignore refusal comes BEFORE the takeover restores the upstream From 8caf8d6e2283a8cb1bf7b08b3cddbafbf7e2c653 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:50:15 +0000 Subject: [PATCH 5/7] Honour tracked files and dir rules in ignore check Review follow-ups for the #1638 checks: - Ask git with the index, so a ledger or manifest already tracked (force-added, or committed before the rule) is not reported as ignored: git keeps committing changes to it. - A .socket/ or .socket/vendor/ directory rule now refuses the run as vendor_artifact_gitignored when it includes a backend that does not check that rule per package (gem, PyPI, Cargo, Go, Composer), so those projects no longer lose the ledger and artifacts silently. - One git check-ignore call per run instead of two. - get warns about an ignored manifest like the search path. - The manifest warning no longer suggests a !rule that cannot work under an ignored parent directory. - Move the helpers next to the ledger in vendor/state.rs. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../src/commands/agent_download.rs | 5 +- crates/socket-patch-cli/src/commands/get.rs | 12 ++- .../socket-patch-cli/src/commands/vendor.rs | 24 ++++- .../tests/get/get_failure_paths.rs | 101 ++++++++++++++---- .../tests/in_process_vendor.rs | 79 +++++++++----- crates/socket-patch-core/src/vendor/mod.rs | 8 +- .../socket-patch-core/src/vendor/npm_dir.rs | 90 +++++++--------- crates/socket-patch-core/src/vendor/state.rs | 68 ++++++++++++ 9 files changed, 277 insertions(+), 114 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 71fccd75b..2f3b21a87 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1508,7 +1508,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 (#591). | | `vendor_artifact_gitignored` | `failed` | vendor (vlt, the npm-family tarball flavors — npm, pnpm, bun, yarn classic, yarn berry — NuGet, 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` or `*.jar` is overridden by the `!*` `.gitignore` vendoring writes into the uuid dir or tree root; if a written tarball, directory payload or nupkg 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 and NuGet): 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): inside a git work tree, `git check-ignore --no-index` 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). 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 reported per package as `vendor_artifact_gitignored` instead. | +| `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, 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. | @@ -1698,7 +1698,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. A run that leaves patches recorded in a manifest git would not commit (inside a git work tree, `git check-ignore --no-index` matches the manifest's project-relative path, e.g. a `manifest.json` or `*.json` rule) 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. +**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: diff --git a/crates/socket-patch-cli/src/commands/agent_download.rs b/crates/socket-patch-cli/src/commands/agent_download.rs index ef37e9505..06731252f 100644 --- a/crates/socket-patch-cli/src/commands/agent_download.rs +++ b/crates/socket-patch-cli/src/commands/agent_download.rs @@ -1349,8 +1349,9 @@ pub(crate) async fn warn_if_manifest_gitignored( }; 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 that rule or \ - add `!{rel}` to .gitignore" + (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}"); diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index e84e63eed..b214cc0e8 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/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 { diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index a18a66d40..aa530f7bf 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1399,6 +1399,14 @@ async fn run_check(args: &VendorArgs) -> i32 { /// A refused eject: the JSON error envelope (`status: error`) or an /// `Error:` line (printed even under `--silent`). Exit 1; nothing touched. +/// 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 +/// and the JVM trees. The others rely on the run-level ledger check. +fn artifacts_gitignore_checked(mut ecosystems: impl Iterator) -> bool { + ecosystems.all(|eco| matches!(eco, Ecosystem::Npm | Ecosystem::Nuget | Ecosystem::Maven)) +} + fn emit_eject_refusal(common: &GlobalArgs, code: &str, message: &str) -> i32 { if common.json { let mut env = Envelope::new(Command::Vendor); @@ -1730,7 +1738,11 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let common = &args.common; // A ledger git would not commit refuses before anything is fetched or // restored, so the project stays hosted (#1638). - if let Some((code, detail)) = vendor::ledger_gitignore_refusal(&common.cwd).await { + let purls: Vec = 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) = @@ -2769,7 +2781,15 @@ pub(crate) async fn vendor_records_reusing( // 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. - if let Some((code, detail)) = vendor::ledger_gitignore_refusal(&common.cwd).await { + 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}"); } diff --git a/crates/socket-patch-cli/tests/get/get_failure_paths.rs b/crates/socket-patch-cli/tests/get/get_failure_paths.rs index bb84ba48e..3df8ac2ce 100644 --- a/crates/socket-patch-cli/tests/get/get_failure_paths.rs +++ b/crates/socket-patch-cli/tests/get/get_failure_paths.rs @@ -1716,11 +1716,34 @@ fn git_project_ignoring(root: &std::path::Path, gitignore: &str) -> bool { 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`, or `*.json` + `!package.json`) 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. +/// 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() { @@ -1730,6 +1753,7 @@ async fn agent_record_warns_when_git_ignores_the_manifest() { 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(); @@ -1740,23 +1764,56 @@ async fn agent_record_warns_when_git_ignores_the_manifest() { 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()); - 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!("{gitignore:?}: json={json}")) - ["detail"] - .as_str() - .unwrap(); - assert!( - detail.contains(".socket/manifest.json") && detail.contains(rule), - "{detail}" - ); - } - None => assert!(warning.is_none(), "{gitignore:?}: json={json}"), - } + 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 ` 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 ", + ); } diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 6a23f42db..a1bf5e3c2 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -525,6 +525,27 @@ async fn a_re_included_ledger_vendors() { assert!(fx.tgz_path().is_file()); } +/// A ledger git already tracks keeps being committed whatever the ignore +/// rules say, so a re-run after a `*.json` rule lands is not refused. +#[tokio::test] +async fn a_tracked_ledger_is_not_refused() { + let fx = npm_fixture(); + if !git_project_ignoring(fx.root(), "node_modules/\n") { + return; + } + assert_eq!(vendor_run(vendor_args(fx.root())).await, 0); + assert!(std::process::Command::new("git") + .arg("-C") + .arg(fx.root()) + .args(["add", ".socket/vendor/state.json"]) + .status() + .unwrap() + .success()); + std::fs::write(fx.root().join(".gitignore"), "node_modules/\n*.json\n").unwrap(); + let (code, env) = vendor_cli(fx.root(), &[]); + assert_eq!(code, 0, "{env:#}"); +} + // ───────────────────────────────────────────────────────────────────── // 4. vendor → revert round-trip // ───────────────────────────────────────────────────────────────────── @@ -3575,6 +3596,39 @@ fn run_scan_vendor(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, Value) (code, env) } +/// The ledger check sits in front of every vendored backend, not only the +/// npm family the report used (#1638): a gem `scan --mode vendored` whose +/// `.gitignore` matches `state.json` refuses `vendor_ledger_gitignored` +/// before anything is written. The gem backend has no per-package check of +/// its own for a `.socket/` directory rule, so that refuses here too, as +/// `vendor_artifact_gitignored`. +#[tokio::test] +async fn scan_vendor_gem_refuses_a_gitignored_ledger() { + for (gitignore, want) in [ + ("vendor/bundle/\nstate.json\n", "vendor_ledger_gitignored"), + ("vendor/bundle/\n.socket/\n", "vendor_artifact_gitignored"), + ] { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + if !git_project_ignoring(fx.root(), gitignore) { + return; + } + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 1, "{gitignore:?}: {env:#}"); + assert_eq!(env["error"]["code"], want, "{env:#}"); + assert!(!fx.root().join(".socket/vendor").exists(), "{env:#}"); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile.lock")).unwrap(), + GEM_LOCK + ); + assert_eq!( + std::fs::read_to_string(fx.root().join("Gemfile")).unwrap(), + GEM_GEMFILE + ); + } +} + /// `scan --mode vendored` end to end on a gem project: discover → download → /// vendor lands the gem pair edit (Gemfile pin + `path:`, lock PATH section /// + `(= …)!` DEPENDENCIES pin) and the patched artifact dir, keyed in the @@ -3583,31 +3637,6 @@ fn run_scan_vendor(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, Value) /// halves and prunes the vendor tree. Manifest-agnostic on purpose: the /// vendored mode never writes `.socket/manifest.json` (its ledger owns the /// entries), so nothing here reads one. -/// The ledger check sits in front of every vendored backend, not only the -/// npm family the report used (#1638): a gem `scan --mode vendored` whose -/// `.gitignore` matches `state.json` refuses before anything is written. -#[tokio::test] -async fn scan_vendor_gem_refuses_a_gitignored_ledger() { - let mock = wiremock::MockServer::start().await; - mount_gem_patch_api(&mock, GEM_PURL).await; - let fx = gem_fixture(); - if !git_project_ignoring(fx.root(), "vendor/bundle/\nstate.json\n") { - return; - } - let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); - assert_eq!(code, 1, "{env:#}"); - assert_eq!(env["error"]["code"], "vendor_ledger_gitignored", "{env:#}"); - assert!(!fx.root().join(".socket/vendor").exists(), "{env:#}"); - assert_eq!( - std::fs::read_to_string(fx.root().join("Gemfile.lock")).unwrap(), - GEM_LOCK - ); - assert_eq!( - std::fs::read_to_string(fx.root().join("Gemfile")).unwrap(), - GEM_GEMFILE - ); -} - #[tokio::test] async fn scan_vendor_gem_end_to_end_and_reverts() { let mock = wiremock::MockServer::start().await; diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index 2de8e18cb..7287c9ff8 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -124,8 +124,9 @@ pub use source::PackageSource; pub(crate) use npm_common::is_safe_npm_name; pub use pypi_requirements::requirements_include_names; pub use state::{ - carry_forward_wiring, load_state, lookup_entry, lookup_entry_kv, purl_keys_cover, save_state, - save_state_shared, VendorEntry, VendorState, VENDOR_STATE_REL, + carry_forward_wiring, file_gitignored, ledger_gitignore_refusal, load_state, lookup_entry, + lookup_entry_kv, purl_keys_cover, save_state, save_state_shared, VendorEntry, VendorState, + LEDGER_GITIGNORED, VENDOR_STATE_REL, }; pub use verify::{ artifact_is_file_shaped, check_vendored_artifact, compute_dir_inventory, @@ -134,9 +135,6 @@ pub use verify::{ // The hosted→vendored takeover refuses a berry project the backend would // refuse BEFORE it reverts the hosted redirect. pub use npm_common::npm_tarball_gitignore_preflight; -// Git's ignore rules for the state files a vendored or agent-mode run asks -// the user to commit (#1638). -pub use npm_dir::{file_gitignored, ledger_gitignore_refusal, LEDGER_GITIGNORED}; pub use npm_lock::npm_lock_vendor_preflight; pub use yarn_berry_lock::{yarn_berry_vendor_preflight, yarn_berry_vendor_target_preflight}; diff --git a/crates/socket-patch-core/src/vendor/npm_dir.rs b/crates/socket-patch-core/src/vendor/npm_dir.rs index 2bf356669..38c17f0d1 100644 --- a/crates/socket-patch-core/src/vendor/npm_dir.rs +++ b/crates/socket-patch-core/src/vendor/npm_dir.rs @@ -460,6 +460,23 @@ pub(crate) async fn gitignore_probe( project_root: &Path, paths: &[String], ) -> Result, String> { + Ok(ignoring_rule_lines(project_root, paths, true) + .await? + .map(|lines| summarize_rules(&lines))) +} + +/// The ignoring rules `git check-ignore -v` reports for `paths`, one +/// `::\t` line each (see +/// [`ignoring_rules`]), or `Ok(None)` when none matches, git is absent or +/// the root is not a work tree. `no_index` asks about the paths as if +/// nothing were tracked (the right question for files this run creates); +/// without it a tracked file is never ignored, because git keeps +/// committing changes to it whatever the ignore rules say. +pub(crate) async fn ignoring_rule_lines( + project_root: &Path, + paths: &[String], + no_index: bool, +) -> Result>, String> { let Some(git) = crate::utils::process::resolve_tool("git") else { return Ok(None); }; @@ -478,27 +495,29 @@ pub(crate) async fn gitignore_probe( }; } let input: String = paths.iter().map(|p| format!("{p}\0")).collect(); - let (code, out) = git_output( - &git, - project_root, - &["check-ignore", "-v", "-z", "--no-index", "--stdin"], - Some(input), - ) - .await - .ok_or("`git check-ignore` did not run to completion")?; + let mut check = vec!["check-ignore", "-v", "-z", "--stdin"]; + if no_index { + check.push("--no-index"); + } + let (code, out) = git_output(&git, project_root, &check, Some(input)) + .await + .ok_or("`git check-ignore` did not run to completion")?; if code != 0 && code != 1 { return Err(format!("`git check-ignore` exited {code}")); } let lines = ignoring_rules(&out); - Ok((code == 0 && !lines.is_empty()).then(|| { - let shown: Vec<&str> = lines.iter().take(3).map(String::as_str).collect(); - let more = lines.len().saturating_sub(shown.len()); - let mut detail = shown.join("; "); - if more > 0 { - detail.push_str(&format!("; and {more} more")); - } - detail - })) + Ok((code == 0 && !lines.is_empty()).then_some(lines)) +} + +/// Up to three of `lines` joined for a message, with a count of the rest. +pub(crate) fn summarize_rules(lines: &[String]) -> String { + let shown: Vec<&str> = lines.iter().take(3).map(String::as_str).collect(); + let more = lines.len().saturating_sub(shown.len()); + let mut detail = shown.join("; "); + if more > 0 { + detail.push_str(&format!("; and {more} more")); + } + detail } /// The non-negated matches of `git check-ignore -v -z` output @@ -534,43 +553,6 @@ pub(crate) async fn ignored_root_refusal( pub(crate) const GITIGNORED: &str = "vendor_artifact_gitignored"; -/// The vendor ledger is ignored by git, so the commit the vendored workflow -/// ends with would leave it out (#1638). -pub const LEDGER_GITIGNORED: &str = "vendor_ledger_gitignored"; - -/// The `vendor_ledger_gitignored` refusal (code, detail) when git would not -/// commit `.socket/vendor/state.json`: a file rule such as `state.json` or -/// `*.json` that no file socket-patch writes overrides. Every other -/// checkout would then get the lock wiring and the artifacts but no ledger, -/// so `rollback`, `list` and `vendor --revert` there cannot see the patch. -/// `None` when `.socket/vendor/` is ignored as a directory: the backends' -/// own `vendor_artifact_gitignored` refusals report that rule per package. -pub async fn ledger_gitignore_refusal(project_root: &Path) -> Option<(&'static str, String)> { - let ledger = super::state::VENDOR_STATE_REL; - let dir = ledger.rsplit_once('/').map_or(ledger, |(dir, _)| dir); - if gitignored(project_root, &[format!("{dir}/")]) - .await - .is_some() - { - return None; - } - let rules = gitignored(project_root, &[ledger.to_string()]).await?; - Some(( - LEDGER_GITIGNORED, - format!( - "git would not commit the vendor ledger {ledger} ({rules}), so other checkouts \ - could not roll back or list the vendored patches; remove that rule or add \ - `!{ledger}` to .gitignore, then vendor again" - ), - )) -} - -/// The rules (as [`gitignored`] names them) that would keep git from -/// committing the project-relative file `rel`, or `None`. -pub async fn file_gitignored(project_root: &Path, rel: &str) -> Option { - gitignored(project_root, &[rel.to_string()]).await -} - /// The vendored dir was written, but git could not say whether it would /// commit it. pub(crate) const GITIGNORE_UNCHECKED: &str = "vendor_artifact_gitignore_unchecked"; diff --git a/crates/socket-patch-core/src/vendor/state.rs b/crates/socket-patch-core/src/vendor/state.rs index 44b3237fa..fa74e1d84 100644 --- a/crates/socket-patch-core/src/vendor/state.rs +++ b/crates/socket-patch-core/src/vendor/state.rs @@ -1003,6 +1003,74 @@ pub(crate) async fn write_marker_or_warn( } } +/// The vendor ledger is ignored by git, so the commit the vendored workflow +/// ends with would leave it out (#1638). +pub const LEDGER_GITIGNORED: &str = "vendor_ledger_gitignored"; + +/// Refuse a vendored run whose state git would not commit (#1638): every +/// other checkout would get the lock wiring and the artifacts but no +/// ledger, so `rollback`, `list` and `vendor --revert` there could not see +/// the patches. One `git check-ignore` over `.socket/vendor/` and the +/// ledger, honouring the index (a ledger already tracked keeps being +/// committed): +/// +/// - a rule that ignores `.socket/vendor/` as a directory (`.socket/`, +/// `vendor/`) is `vendor_artifact_gitignored`, unless +/// `artifacts_checked`: every backend in the run refuses that rule per +/// package itself (the npm family, NuGet, the JVM trees), naming the +/// package; +/// - a file rule that matches only the ledger (`state.json`, `*.json`) is +/// `vendor_ledger_gitignored`. +/// +/// `None` when git would commit both, git is absent, the root is not a work +/// tree, or git cannot answer (the fail-open of +/// [`super::npm_dir::gitignored`]). +pub async fn ledger_gitignore_refusal( + project_root: &Path, + artifacts_checked: bool, +) -> Option<(&'static str, String)> { + let dir = format!("{VENDOR_DIR}/"); + let paths = [dir.clone(), VENDOR_STATE_REL.to_string()]; + let lines = super::npm_dir::ignoring_rule_lines(project_root, &paths, false) + .await + .ok()??; + let dir_lines: Vec = lines + .iter() + .filter(|line| line.ends_with(&format!("\t{dir}"))) + .cloned() + .collect(); + if !dir_lines.is_empty() { + return (!artifacts_checked).then(|| { + ( + super::npm_dir::GITIGNORED, + super::npm_dir::gitignored_detail( + VENDOR_DIR, + &super::npm_dir::summarize_rules(&dir_lines), + ), + ) + }); + } + Some(( + LEDGER_GITIGNORED, + format!( + "git would not commit the vendor ledger {VENDOR_STATE_REL} ({}), so other \ + checkouts could not roll back or list the vendored patches; remove that rule \ + or add `!{VENDOR_STATE_REL}` to .gitignore, then vendor again", + super::npm_dir::summarize_rules(&lines) + ), + )) +} + +/// The rules (summarized for a message) that would keep git from +/// committing the project-relative file `rel`. A tracked file is never +/// ignored. `None` also when git is absent or cannot answer. +pub async fn file_gitignored(project_root: &Path, rel: &str) -> Option { + super::npm_dir::ignoring_rule_lines(project_root, &[rel.to_string()], false) + .await + .ok()? + .map(|lines| super::npm_dir::summarize_rules(&lines)) +} + #[cfg(test)] mod tests { use super::*; From 1a25bb333e2fa17426ffdc5b6a0e47b78c158777 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 17:07:19 +0000 Subject: [PATCH 6/7] Pin setup-uv to 0.13.1 for the Pipenv legs uv 0.13.2 refuses the pylock.toml Pipenv writes (environments = []), so the Pipenv e2e legs fail on every PR. Same change as #1639; it no-ops once that lands on main. Assisted-by: Claude Code:claude-opus-5-5 --- .github/workflows/ci.yml | 6 ++++++ .github/workflows/pipenv-compatibility.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eef9a64dd..764e0bbbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1465,6 +1465,12 @@ jobs: - name: Setup uv (Pipenv / pip legs) if: matrix.pipenv != '' || matrix.pip != '' uses: astral-sh/setup-uv@eb1897b8dc4b5d5bfe39a428a8f2304605e0983c # v7.0.0 + with: + # Pin uv: it is the PEP 751 oracle the pylock tests install with, + # and uv 0.13.2 started refusing Pipenv's `environments = []` + # pylock ("target environment is not supported by the lockfile"). + # Bump deliberately, after checking the Pipenv pylock legs pass. + version: "0.13.1" - name: Install the Pythons the Pipenv / pip legs run on if: matrix.pipenv != '' || matrix.pip != '' diff --git a/.github/workflows/pipenv-compatibility.yml b/.github/workflows/pipenv-compatibility.yml index cea9c0e5e..a786e3276 100644 --- a/.github/workflows/pipenv-compatibility.yml +++ b/.github/workflows/pipenv-compatibility.yml @@ -101,6 +101,12 @@ jobs: save-if: ${{ github.ref == 'refs/heads/main' }} - name: Install uv uses: astral-sh/setup-uv@d0cc045d04ccac9d8b7881df0226f9e82c39688e # v6.8.0 + with: + # Pin uv: it is the PEP 751 oracle the pylock tests install with, + # and uv 0.13.2 started refusing Pipenv's `environments = []` + # pylock ("target environment is not supported by the lockfile"). + # Bump deliberately, after checking the Pipenv pylock legs pass. + version: "0.13.1" - name: Install the Pythons the Pipenv releases run on run: uv python install 3.8 3.12 - name: Build the CLI From 6094254b2ca8fe0e669fe754ddeabfce097404cc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 19:10:57 +0000 Subject: [PATCH 7/7] Move emit_eject_refusal's doc comment back onto it artifacts_gitignore_checked was inserted between emit_eject_refusal and its doc comment, so the refusal helper lost its doc and the new helper's doc opened with an unrelated sentence. The PR description already claims this was fixed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Pu6ZWFiGyCEMEvfpnZrnVa --- crates/socket-patch-cli/src/commands/vendor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index aa530f7bf..049ee95dc 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1397,8 +1397,6 @@ async fn run_check(args: &VendorArgs) -> i32 { i32::from(env.summary.failed != 0) } -/// A refused eject: the JSON error envelope (`status: error`) or an -/// `Error:` line (printed even under `--silent`). Exit 1; nothing touched. /// 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 @@ -1407,6 +1405,8 @@ fn artifacts_gitignore_checked(mut ecosystems: impl Iterator) ecosystems.all(|eco| matches!(eco, Ecosystem::Npm | Ecosystem::Nuget | 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 { if common.json { let mut env = Envelope::new(Command::Vendor);