GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix apply failing when patched deps are skipped (#403) by mikolalysenko · Pull Request #555 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #403

Summary

socket-patch apply no longer exits 1 when the only manifest patches are for packages that the project's lockfile resolves but the package manager deliberately left uninstalled on this host. Examples are a platform-gated optional dependency (fsevents on Linux, @esbuild/<os>-<cpu>) and a devDependency after npm ci --omit=dev. Before this change, CI on "the other" platform and production deploys running npm ci --omit=dev && socket-patch apply failed even though the tree was already correct.

Root cause

Agent-mode apply (crates/socket-patch-cli/src/commands/apply.rs) failed the run when no targeted manifest patch matched an installed package. That happened both on the empty-crawl path (success: unmatched.is_empty()) and on the none_matched path. Neither path looked at the project's lockfiles. If any other patch matched, the same entry was only a warning (exit 0). scan --apply already treats lockfile-only packages as a calm skipped/package_not_installed, as CLI_CONTRACT.md says.

Fix

  • New lockfile_resolved() reads the project lock inventory (ProjectContext::locks, the same inventory behind scan's lockfile supplement) and marks the unmatched purls the lockfiles resolve. It uses the normalize_purl/qualifier-stripped comparison and Composer's purl_eq, mirroring lockfile_only_contains. Global runs have no project lock, so nothing is lockfile-resolved there.
  • Lockfile-resolved purls never fail the run:
    • JSON keeps the skipped/package_not_installed event, with the detail "Resolved by the project lockfile but not installed on this host (lockfile-only)".
    • The human path prints a Note: block instead of the Error: block. --silent/--json mute it.
  • An unmatched purl with no lock evidence still fails an all-miss run, and the error lists only those purls. The wrong---cwd guard is kept: no lock means behavior is unchanged.
  • CLI_CONTRACT.md (package_not_installed row, rollback asymmetry note) and the doc comment on the pinned unmatched_purl_exit_semantics_are_pinned invariant are updated. That invariant's ghost purl has no lock, so both of its halves are unchanged.
  • I added no new async frame to apply_patches_inner's poll state: the lock read is Box::pinned, per the Windows 1 MiB stack note.
  • Wrappers (npm/, pypi/, gem/) only dispatch to the binary, so they need no change.

Test evidence

New suite crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs (cargo test -p socket-patch-cli --all-features --test apply lockfile_only_skip):

  • Without the fix (main's apply.rs): 4 failed, 1 passed. The no_lockfile_all_miss_still_fails control passes on both.
  • With the fix: 5 passed.

Per-issue checklist (#403):

  • Platform-skipped optional dependency (fsevents, os: ["darwin"]): exit 0, success, calm skip event, human note and no Error:. Covered by platform_skipped_optional_dependency_is_a_calm_skip.
  • Hook-style apply --silent: exit 0 and silent. Covered by silent_apply_on_platform_skipped_optional_dependency_exits_zero.
  • npm ci --omit=dev devDependency (dev: true): exit 0. Covered by omitted_dev_dependency_is_a_calm_skip.
  • A purl the lock doesn't resolve still fails, and the error lists only it. Covered by unresolved_purl_still_fails_alongside_a_lock_resolved_one.
  • No lockfile at all (wrong cwd): still exit 1. Covered by no_lockfile_all_miss_still_fails.
  • Real npm 10.9.4 / Node 22 repro from the issue ([email protected], so [email protected] is in the lock but not installed on Linux, with a manifest holding only pkg:npm/[email protected]). apply --offline exits 0 with the note, and apply --offline --silent exits 0.

Local checks:

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --all -- --check: main itself isn't rustfmt-clean (a cargo fmt --all rewrites 129 files on 61cfb9b), and CI doesn't run fmt. I checked that this PR's hunks and the new file are rustfmt-clean, and the only rustfmt --check diff left in apply.rs is a pre-existing one that main also has.
  • cargo test --workspace --all-features --no-fail-fast: 9473 passed, 12 failed, 246 ignored. All 12 failures are permission-based fixtures (chmod read-only dirs or unremovable files, e.g. covgap_commands_vendor::*_state_write_failure_*, vendor::pypi_poetry::tests::wire_write_failure_*, patch::copy_tree::tests::relax_loop_must_not_traverse_symlinked_root). They can't fail as root (the sandbox runs as uid 0), and none of them touch apply. CI runs as non-root.
  • node --test npm/socket-patch/bin/socket-patch.test.mjs: 4/4.

CI on 2b8466b: all 407 check runs finished, 401 success and 6 skipped. Bugbot reviewed 2b8466b and found no issues.


Note

Medium Risk
Changes documented exit-code and failure semantics for apply, which CI and postinstall hooks may gate on, though behavior is narrowed to lockfile-evidenced misses only.

Overview
socket-patch apply no longer exits 1 when manifest patches only target packages the lockfile resolves but that are intentionally absent on disk (e.g. platform optional fsevents, devDependencies after npm ci --omit=dev). That aligns agent apply with scan --apply, which already treated those as benign skips.

The command now cross-checks unmatched manifest purls against ProjectContext lock inventory and splits them into lockfile-resolved vs truly unresolved. Only unresolved purls can trigger partialFailure, stderr Error: lines, or the all-miss failure path; lockfile-only entries still emit skipped / package_not_installed with an explicit lockfile-only detail and an optional human Note: (muted under --json / --silent). CLI_CONTRACT.md and integration tests in lockfile_only_skip.rs pin the behavior; wrong---cwd / no-lock cases stay exit 1.

Reviewed by Cursor Bugbot for commit 2b8466b. Configure here.


Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@claude
Start fix for #403
234d8cc
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 2, 2026
npm apply exits 1 when every patch targets a platform-skipped optional dependency (fsevents, @esbuild/*), so the setup hook fails npm ci and npm install on other OSes #403
Closed
@claude
Skip lockfile-only patches in apply calmly
2b8466b
`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
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the agent/fix-apply-lockfile-only-skip branch from 325d73c to 2b8466b Compare October 2, 2026 12:35
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 2, 2026 12:52
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

cursor[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor[bot]
cursor Bot reviewed Oct 2, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2b8466b. Configure here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 2b8466b0f71994b66cbcfbd26e217eb273238944.

  • CI: 401/401 non-skipped check runs green on head (6 skipped); 0 commits behind main, mergeable (blocked only on required review).
  • Bugbot: reviewed 2b8466b — no issues found; no unresolved review threads.
  • Reviewer focus: the new path where apply treats unmatched purls as "deliberately not installed" when the project lock resolves them — check it can't mask a genuinely missing install.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Reviewed 2b8466b0f71994b66cbcfbd26e217eb273238944. Recommendation: ready to merge as-is from code review.

No actionable correctness/security regressions found. Checked lockfile matching, scoped/qualified PURLs, global-scope exclusion, mixed unresolved/lock-resolved misses, and preservation of actual patch/variant failures.

Validation: cargo test -p socket-patch-cli --all-features --test apply — 93 passed, 1 pre-existing ignored test. The five new skip/control tests pass on this head; against merge-base apply.rs, four fail and the no-lock control passes. Full workspace/platform matrix not rerun.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Second review pass of 2b8466b0f71994b66cbcfbd26e217eb273238944: ready from code review. Lockfile-only misses still change only unmatched-package handling. The current-main PyPI multi-copy apply/fallback changes are in distinct paths; actual installed-copy patch failures remain errors. Global scope and no-lock all-miss runs retain their failure behavior.

The head is unchanged from the prior review, discussions introduce no unresolved finding, and it merges cleanly with current main 73b17db5. Prior exact-head apply suite: 93 passed, 1 pre-existing ignored test. Five new skip/control tests passed; with the earlier base implementation four failed and the no-lock control passed. No duplicate broad test run or code changes were needed for this pass.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marvinsocket
marvinsocket approved these changes Oct 2, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 3a4883c into main Oct 2, 2026
407 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-apply-lockfile-only-skip branch October 2, 2026 17:32
This was referenced Oct 2, 2026
npm agent-mode apply never patches an npm-aliased install (lp@npm:left-pad), yet VEX attests the package not_affected #356
Closed
Agent mode ignores pnpm's virtualStoreDir: transitive dependencies are reported package_not_installed with a custom virtualStoreDir or the global virtual store #362
Closed
Cargo apply on a cold or pruned registry cache now exits 0 as "lockfile-only", then the next cargo build downloads and compiles the unpatched crate (regression from #555) #616
Closed
Agent-mode cargo apply patches the wrong copy when cargo vendor uses a custom directory (or apply runs from a workspace member), yet reports success and VEX attests not_affected #338
Closed
Hosted Composer vex attests not_affected for an unpatched package when config.vendor-dir is absolute or uses ~/ or $HOME/ #686
Open
Fix pnpm global-store transitive deps passing silently (#362) #829
Merged
Fix cargo cold-cache apply exiting 0 (#616) #1310
Merged
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Oct 9, 2026
@mikolalysenko @claude
Fail cargo apply on an unfetched crate again
9f84ebb
Since #555, apply treated a manifest crate that Cargo.lock resolves
but that is not unpacked in $CARGO_HOME/registry/src as a calm
lockfile-only skip: exit 0, status success. On a fresh CI runner or
a pruned cache that crate was simply not fetched yet, so
'socket-patch apply && cargo build --locked' went green and shipped
the unpatched crate (#616).

Cargo crates are no longer lockfile-only: cargo fetch unpacks every
locked crate, target-gated ones included, so a missing one is a real
miss. An all-miss run exits 1 again, as in 4.x, and the JSON detail,
the human error and the warning tell the user to run cargo fetch
first. npm's platform-gated and --omit=dev skips are unchanged.

Fixes #616

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@marvinsocket marvinsocket marvinsocket approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

npm apply exits 1 when every patch targets a platform-skipped optional dependency (fsevents, @esbuild/*), so the setup hook fails npm ci and npm install on other OSes

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mikolalysenko @marvinsocket @claude
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.