Skip to content

Commit 2b8466b

Browse files
committed
Skip lockfile-only patches in apply calmly
`socket-patch apply` exited 1 when every manifest patch targeted a package the project's lockfile resolves but the package manager deliberately left uninstalled on this host: a platform-gated optional dependency like fsevents or @esbuild/<os>-<cpu>, or a devDependency after `npm ci --omit=dev`. CI on other platforms and production deploys failed although the tree was already correct. Such purls are now a calm package_not_installed skip with a lockfile-only detail and a human note, matching scan --apply. An unmatched purl that no lock resolves still fails the all-miss run, so the wrong --cwd guard is kept. Fixes #403 Assisted-by: Claude Code:claude-opus-5-5
1 parent 234d8cc commit 2b8466b

5 files changed

Lines changed: 398 additions & 18 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,7 +1129,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
11291129
| Tag | Action(s) | Context |
11301130
|---------------------------|------------------|---------|
11311131
| `already_patched` | `skipped` | apply: every file's hash already matches `afterHash`. |
1132-
| `package_not_installed` | `skipped` | apply: manifest entry has no matching installed package. |
1132+
| `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`). |
11331133
| `apply_failed` | `failed` | apply: hash mismatch, write error, archive read error. |
11341134
| `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. |
11351135
| `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`). |
@@ -1291,7 +1291,7 @@ The remaining commands still emit their pre-v3.0 ad-hoc JSON shapes and will mig
12911291

12921292
- ⏳ `scan` — still emits the discovery + `apply.patches[*]` + `gc.*` shape documented in earlier drafts of this file.
12931293
- ⏳ `get` — still emits per-patch action arrays.
1294-
- ⏳ `rollback` — still emits per-package result records. Additive (v3.5): a manifest entry with no matching installed package appears in `results[]` as a marker record `{ "purl", "path": null, "skipped": "package_not_installed" }` — no `success`/`error` keys, never counted in `rolledBack`/`failed`, never flips the status or exit code (rollback's job is "make the tree unpatched"; a not-installed package already satisfies that end state, deliberately asymmetric with apply's exit-1-on-unmatched). v5.0 keeps that legacy shape and adds the ALWAYS-PRESENT keys `warnings[]` (`{code, detail}` objects, now populated), `vendored` (meaning narrowed — MAJOR), `vendoredReverted`, `vendoredPreserved`, `vendoredKept` (`{purl, reason}`), `hosted` (`{reverted, failed: [{purl, error}], unsupported, editedFiles}`), `manifest` (`{removedEntries, preserved}`), `gc` (`{skipped: true}` \| `{removedBlobs, removedDiffArchives, removedPackageArchives, bytesFreed}`), and `paths` — full key semantics and exit rules in the [Rollback command contract](#rollback-command-contract-v50).
1294+
- ⏳ `rollback` — still emits per-package result records. Additive (v3.5): a manifest entry with no matching installed package appears in `results[]` as a marker record `{ "purl", "path": null, "skipped": "package_not_installed" }` — no `success`/`error` keys, never counted in `rolledBack`/`failed`, never flips the status or exit code (rollback's job is "make the tree unpatched"; a not-installed package already satisfies that end state, deliberately asymmetric with apply's exit-1 on an all-miss run whose unmatched purls are not lockfile-resolved). v5.0 keeps that legacy shape and adds the ALWAYS-PRESENT keys `warnings[]` (`{code, detail}` objects, now populated), `vendored` (meaning narrowed — MAJOR), `vendoredReverted`, `vendoredPreserved`, `vendoredKept` (`{purl, reason}`), `hosted` (`{reverted, failed: [{purl, error}], unsupported, editedFiles}`), `manifest` (`{removedEntries, preserved}`), `gc` (`{skipped: true}` \| `{removedBlobs, removedDiffArchives, removedPackageArchives, bytesFreed}`), and `paths` — full key semantics and exit rules in the [Rollback command contract](#rollback-command-contract-v50).
12951295

12961296
One command is **intentionally not** plain-envelope and will stay that way (not migration debt):
12971297

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

Lines changed: 108 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use socket_patch_core::patch::redirect::golang_local::{
1616
};
1717
use socket_patch_core::telemetry::{track_patch_applied, track_patch_apply_failed};
1818
use socket_patch_core::utils::purl::parse_golang_purl;
19-
use socket_patch_core::utils::purl::{normalize_purl, strip_purl_qualifiers};
19+
use socket_patch_core::utils::purl::{normalize_purl, purl_eq, strip_purl_qualifiers};
2020
use socket_patch_core::vendor::purl_keys_cover;
2121
use std::collections::{HashMap, HashSet};
2222
use std::path::{Path, PathBuf};
@@ -999,6 +999,7 @@ pub(crate) async fn run_locked(
999999
success,
10001000
results,
10011001
unmatched,
1002+
lockfile_only,
10021003
run_warnings,
10031004
fallback_skips,
10041005
targeted,
@@ -1129,11 +1130,14 @@ pub(crate) async fn run_locked(
11291130
// had no installed package on disk — emit one Skipped
11301131
// event per purl so downstream consumers can surface them.
11311132
for purl in &unmatched {
1133+
let detail = if lockfile_only.contains(purl) {
1134+
LOCKFILE_ONLY_DETAIL
1135+
} else {
1136+
"No installed package matches this PURL"
1137+
};
11321138
env.record(
1133-
PatchEvent::new(PatchAction::Skipped, purl.clone()).with_reason(
1134-
"package_not_installed",
1135-
"No installed package matches this PURL",
1136-
),
1139+
PatchEvent::new(PatchAction::Skipped, purl.clone())
1140+
.with_reason("package_not_installed", detail),
11371141
);
11381142
}
11391143
// Best-effort gem-env fallback-home copies left unpatched:
@@ -1351,6 +1355,11 @@ struct ApplyOutcome {
13511355
results: Vec<ApplyResult>,
13521356
/// In-scope manifest purls with no installed package on disk.
13531357
unmatched: Vec<String>,
1358+
/// The subset of [`Self::unmatched`] the project's own lockfiles
1359+
/// resolve: deliberately not installed on this host (a platform-gated
1360+
/// optional dependency, a devDependency under `--omit=dev`), so a calm
1361+
/// skip that never fails the run (#403).
1362+
lockfile_only: HashSet<String>,
13541363
/// Run-level advisories: JSON `warnings[]`, and one gated stderr line
13551364
/// each on the human path (`--silent` = errors only) except the
13561365
/// sources-unavailable codes (already printed by the stager): the gem
@@ -1669,6 +1678,7 @@ async fn apply_patches_inner(
16691678
success: false,
16701679
results: Vec::new(),
16711680
unmatched: Vec::new(),
1681+
lockfile_only: HashSet::new(),
16721682
run_warnings: vec![stage_failure_warning(args.common.offline)],
16731683
fallback_skips: Vec::new(),
16741684
targeted: target_manifest_purls.len(),
@@ -1700,6 +1710,7 @@ async fn apply_patches_inner(
17001710
success: true,
17011711
results: Vec::new(),
17021712
unmatched: Vec::new(),
1713+
lockfile_only: HashSet::new(),
17031714
run_warnings: Vec::new(),
17041715
fallback_skips: Vec::new(),
17051716
targeted: 0,
@@ -1791,20 +1802,24 @@ async fn apply_patches_inner(
17911802
);
17921803
let mut unmatched = unmatched;
17931804
unmatched.sort();
1805+
let lockfile_only = Box::pin(lockfile_resolved(&args.common, &unmatched)).await;
1806+
let unresolved = unresolved_purls(&unmatched, &lockfile_only);
17941807
// This diagnostic flips the exit code, so it is an error — and it
17951808
// prints even under --silent ("errors only", never a mute exit 1);
17961809
// `--json`
17971810
// mutes stderr and the envelope's `package_not_installed` events
1798-
// are the channel.
1799-
if !unmatched.is_empty() && args.prints_errors() {
1800-
for line in format_none_installed_error(&unmatched) {
1811+
// are the channel. Lockfile-resolved purls never flip it (#403).
1812+
if !unresolved.is_empty() && args.prints_errors() {
1813+
for line in format_none_installed_error(&unresolved) {
18011814
eprintln!("{line}");
18021815
}
18031816
}
1817+
print_lockfile_only_note(args, &unmatched, &lockfile_only);
18041818
return Ok(ApplyOutcome {
1805-
success: unmatched.is_empty(),
1819+
success: unresolved.is_empty(),
18061820
results,
18071821
unmatched,
1822+
lockfile_only,
18081823
run_warnings,
18091824
fallback_skips,
18101825
targeted: target_manifest_purls.len(),
@@ -2169,28 +2184,38 @@ async fn apply_patches_inner(
21692184
&vendored_bases,
21702185
);
21712186
unmatched.sort();
2187+
let lockfile_only = if unmatched.is_empty() {
2188+
HashSet::new()
2189+
} else {
2190+
Box::pin(lockfile_resolved(&args.common, &unmatched)).await
2191+
};
2192+
let unresolved = unresolved_purls(&unmatched, &lockfile_only);
21722193

2194+
// Nothing matched and some purl has no lock evidence either: this
2195+
// fails the run, so it is an error — and errors print even under
2196+
// --silent. Lockfile-resolved purls are deliberately not installed
2197+
// here, so they never fail it (#403).
21732198
let none_matched = !target_manifest_purls.is_empty()
21742199
&& matched_manifest_purls.is_empty()
2175-
&& !all_packages.is_empty();
2200+
&& !all_packages.is_empty()
2201+
&& !unresolved.is_empty();
21762202
if none_matched {
2177-
// Nothing matched: this fails the run, so it is an error — and
2178-
// errors print even under --silent.
21792203
has_errors = true;
21802204
if args.prints_errors() {
2181-
for line in format_none_installed_error(&unmatched) {
2205+
for line in format_none_installed_error(&unresolved) {
21822206
eprintln!("{line}");
21832207
}
21842208
}
2185-
} else if !unmatched.is_empty() && !args.common.silent && !args.common.json {
2209+
} else if !unresolved.is_empty() && !args.common.silent && !args.common.json {
21862210
eprintln!(
21872211
"Warning: {} had no matching installed package:",
2188-
plural(unmatched.len(), "manifest patch", "manifest patches")
2212+
plural(unresolved.len(), "manifest patch", "manifest patches")
21892213
);
2190-
for purl in &unmatched {
2214+
for purl in &unresolved {
21912215
eprintln!(" - {}", normalize_purl(purl));
21922216
}
21932217
}
2218+
print_lockfile_only_note(args, &unmatched, &lockfile_only);
21942219

21952220
// The human summary is printed by `run`, after the per-package list.
21962221

@@ -2204,13 +2229,80 @@ async fn apply_patches_inner(
22042229
success: !has_errors,
22052230
results,
22062231
unmatched,
2232+
lockfile_only,
22072233
run_warnings,
22082234
fallback_skips,
22092235
targeted: target_manifest_purls.len(),
22102236
show_summary: true,
22112237
})
22122238
}
22132239

2240+
/// The `package_not_installed` detail of a lockfile-resolved purl.
2241+
const LOCKFILE_ONLY_DETAIL: &str =
2242+
"Resolved by the project lockfile but not installed on this host (lockfile-only)";
2243+
2244+
/// The `unmatched` purls the project's own lockfiles resolve (#403): the
2245+
/// package manager resolved them but deliberately did not install them on
2246+
/// this host — an `os`/`cpu`-gated optional dependency (`fsevents`,
2247+
/// `@esbuild/<os>-<cpu>`), a devDependency under `npm ci --omit=dev`. The
2248+
/// tree is in its correct end state, so they are calm skips, as `scan
2249+
/// --apply` treats lockfile-only packages. Global runs have no project
2250+
/// lock, so nothing is lockfile-resolved there.
2251+
async fn lockfile_resolved(common: &GlobalArgs, unmatched: &[String]) -> HashSet<String> {
2252+
if unmatched.is_empty() || common.is_global() {
2253+
return HashSet::new();
2254+
}
2255+
let ctx = crate::commands::context::ProjectContext::new(common);
2256+
let entries = &ctx.locks().await.entries;
2257+
let lock_purls: HashSet<String> = entries
2258+
.iter()
2259+
.map(|e| normalize_purl(strip_purl_qualifiers(&e.purl)).into_owned())
2260+
.collect();
2261+
unmatched
2262+
.iter()
2263+
.filter(|p| {
2264+
let base = strip_purl_qualifiers(p);
2265+
lock_purls.contains(normalize_purl(base).as_ref())
2266+
|| (base.starts_with("pkg:composer/")
2267+
&& entries.iter().any(|e| purl_eq(&e.purl, base)))
2268+
})
2269+
.cloned()
2270+
.collect()
2271+
}
2272+
2273+
/// The `unmatched` purls with no lock evidence (sorted input, sorted
2274+
/// output): the ones that can still fail an all-miss run.
2275+
fn unresolved_purls(unmatched: &[String], lockfile_only: &HashSet<String>) -> Vec<String> {
2276+
unmatched
2277+
.iter()
2278+
.filter(|p| !lockfile_only.contains(*p))
2279+
.cloned()
2280+
.collect()
2281+
}
2282+
2283+
/// The human note for lockfile-resolved purls (never an error; muted by
2284+
/// `--silent` and `--json`).
2285+
fn print_lockfile_only_note(
2286+
args: &ApplyArgs,
2287+
unmatched: &[String],
2288+
lockfile_only: &HashSet<String>,
2289+
) {
2290+
if lockfile_only.is_empty() || args.common.silent || args.common.json {
2291+
return;
2292+
}
2293+
eprintln!(
2294+
"Note: {} not installed on this host (resolved by the project lockfile; skipped):",
2295+
plural(
2296+
lockfile_only.len(),
2297+
"manifest patch targets a package",
2298+
"manifest patches target packages"
2299+
)
2300+
);
2301+
for purl in unmatched.iter().filter(|p| lockfile_only.contains(*p)) {
2302+
eprintln!(" - {}", normalize_purl(purl));
2303+
}
2304+
}
2305+
22142306
/// `Error: Failed to patch <purl>: <why>` (stderr, even under --silent).
22152307
fn format_patch_failure(purl: &str, why: &str) -> String {
22162308
format!("Error: Failed to patch {purl}: {why}")

‎crates/socket-patch-cli/tests/apply/apply_invariants.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,11 @@ fn write_partial_match_project(root: &Path) {
635635
/// Resolving either direction changes a documented contract for the
636636
/// other consumer — whoever does it must update BOTH halves of this test
637637
/// and the hook/CI guidance together.
638+
///
639+
/// Narrowed by #403: an unmatched purl the project's lockfiles resolve
640+
/// (deliberately not installed here) never fails the all-miss run — see
641+
/// `lockfile_only_skip.rs`. The ghost purl below has no lock evidence, so
642+
/// both halves still hold for it.
638643
#[test]
639644
fn unmatched_purl_exit_semantics_are_pinned() {
640645
// Mixed manifest: applied + unmatched → exit 0, status success,

0 commit comments

Comments
 (0)