Skip to content

Fix vendor --check passing unwired vendored entries (#725) - #730

Merged
Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-vendor-check-wiring-liveness
Oct 5, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-vendor-check-wiring-liveness

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #725

Summary

socket-patch vendor --check now fails a vendored entry when the project's lockfile or config no longer points at its .socket/vendor/ artifact. Before this change it said "committed artifact and wiring verified" and exited 0 after a relock (pipenv lock, uv lock, poetry lock, npm install, a hand edit). A CI gate built on it stayed green while every fresh install got the unpatched package, and vex refused the same checkout with vendor_unwired.

Root cause

commands/vendor.rs::run_check checked every entry's committed artifact bytes, but it checked wiring only for JVM entries (vendor::jvm::apply::check_entry). For npm, PyPI, gem, cargo, Go, Composer and NuGet entries it never looked at the lockfile. The issue reproduced this with Pipenv. The code path is the same for every non-JVM ecosystem, so this fixes all of them.

Fix

For each non-JVM entry whose artifact is healthy, run_check now asks Discovery::vendor_entry_live, the shared vendor-ledger liveness rule that vex (vendor_unwired) and scan (cross-mode takeover) already use. Discovery is built once per run, only when there is a non-JVM entry. An unwired entry produces failed / vendor_check_failed, with a reason that names the missing .socket/vendor/<eco>/<uuid> wiring and says to re-run socket-patch vendor. The run exits 1, as CLI_CONTRACT.md already documents for drift. JVM entries keep their own, more detailed layout check. --check is still offline and read-only: discovery only reads lockfiles. CLI_CONTRACT.md now says that drift includes wiring.

No wrapper changes: the npm, PyPI and gem wrappers only dispatch to the binary.

Tests (red → green)

Issue Test Without fix With fix
#725 (Pipenv relock, including the human output not saying "wiring verified") mode_migration_pypi::vendor_check_fails_after_pipenv_relock FAILED (exit 0) ok
#725, other PyPI locks on the same path vendor_check_fails_after_{requirements_rewrite,poetry_relock,uv_relock,hatch_dependency_reset} 4× FAILED (exit 0) ok
#725, npm (npm install re-resolves the lock) in_process_vendor::vendor_check_fails_when_lock_no_longer_wires_artifact FAILED (exit 0) ok

Each test first checks that the correctly wired project still passes (vendor_check_ok, exit 0). It then restores the pre-vendor lockfile bytes and expects vendor_check_failed, exit 1. The Poetry, uv and Hatch fixtures were factored into stage_* helpers that the existing takeover tests now share. That is a pure refactor.

Local validation

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt: the changed code is rustfmt-clean. main has unrelated unformatted hunks, which this PR leaves alone.
  • cargo test --workspace --all-features: the new tests and the touched suites (mode_migration_pypi 17/17, in_process_vendor) pass. The remaining local failures also fail on unmodified main in this sandbox: chmod-based write-failure tests are ineffective as root, and self-update fixture tests can't run here. CI is the authority for those.

Note

Medium Risk
Changes vendor --check outcomes for previously false-green unwired entries across non-JVM ecosystems, which can flip CI from pass to fail until socket-patch vendor is re-run.

Overview
vendor --check now treats missing lockfile/config wiring as drift, aligning the CI gate with vex's vendor_unwired rule instead of only verifying committed tarball bytes.

After artifact (and existing npm/JVM-specific) checks pass, non-JVM ledger entries are validated via one shared discover_wiring pass and Discovery::vendor_entry_live. If a relock or hand edit dropped references to .socket/vendor/<ecosystem>/<uuid> while the artifact remains, the run emits vendor_check_failed with a wiring reason and exits 1. CLI_CONTRACT.md documents that drift covers wiring as well as the artifact.

Regression tests cover npm lock re-resolution and PyPI paths (Pipenv, Poetry, uv, Hatch, requirements.txt); PyPI fixtures were refactored into shared stage_* helpers for reuse.

Reviewed by Cursor Bugbot for commit 14a5931. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
`vendor --check` only verified wiring for Maven/Gradle entries. For
every other ecosystem it reported "committed artifact and wiring
verified" and exited 0 even after `pipenv lock`, `uv lock`,
`npm install` or a hand edit pointed the lockfile back at the
registry, so a CI gate stayed green while fresh installs got the
unpatched package and `vex` refused the same checkout.

Each non-JVM entry is now judged by the same vendor-ledger liveness
rule `vex` and `scan` use; an unwired entry fails with
`vendor_check_failed` and exit 1. Regression tests cover Pipenv,
requirements.txt, Poetry, uv, Hatch and npm.

Fixes #725

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) force-pushed the agent/fix-vendor-check-wiring-liveness branch from 68af164 to 46e47d9 Compare October 3, 2026 22:37
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 3, 2026 23:26
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

1 similar comment
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

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

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review at head 46e47d95c8.

  • CI: 414/414 completed checks green (3 skipped), no failures or pending runs.
  • Mergeable: no conflicts (blocked only on required review).
  • Bugbot reviewed 46e47d9: no new issues; no unresolved review threads.
  • Reviewer focus: run_check now builds Discovery once and calls vendor_entry_live for non-JVM entries; CLI_CONTRACT.md wording change on drift.

Generated by Claude Code

@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 5, 2026
CLI_CONTRACT.md: merged the vendor --check drift sentence to cover both this PR's wiring-liveness rule and main's package-lock rewire drift (#588); mode_migration_pypi.rs: kept this PR's stage_* helpers and #725 vendor --check tests alongside main's #765 and #699 tests.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

The npm package-lock check (#589) names the exact unwired lock entry;
running the generic liveness rule first replaced that reason, failing
e2e_vex_vendor after merging main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01519c1ZV6MhuxyVisVJ5FYz
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

coverage was red on 77d014e (the merge of main) for two targets:

  • e2e_vex_vendor: fixed in e391912. vendored_npm_patch_with_an_unwired_registry_copy_in_the_same_lock (added on main by Fix npm/Bun VEX attesting a patch a same-lock copy skips (#588) #589) expects vendor --check to name the unwired lock entry. The generic liveness check from this PR ran first and replaced that reason. The npm package-lock check now runs first, and the liveness rule covers whatever it doesn't catch. Locally e2e_vex_vendor (24/24), in_process_vendor and mode_migration_pypi pass, and clippy is clean.
  • -p socket-patch-cli --lib: not caused by this PR. The same target fails coverage on main (4646693, job). It passes locally (837/837), and I couldn't get the failing test name from the log available here. I don't know of an existing fix to port, so it should go green once main is fixed.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Follow-up on the --lib failure I flagged above: I've now identified the failing tests. coverage and test-release both fail on commands::vex_consumed::tests::hosted_expands_alias_only_copies and hosted_reuses_expanded_npm_copies_and_merges_alias_variants. Those tests were added to main by #605 (4646693), and main's own test-release fails on the same two (job). CI only hits them here because it tests this PR merged with current main; this branch doesn't contain them, which is why they passed locally. This PR doesn't touch vex_consumed.rs. They'll clear once main is fixed. test (windows-latest) failed on the same e2e_vex_vendor test that e391912 fixes.


Generated by Claude Code

Brings in the vex_consumed alias test fix (#849) that main's red
test/test-release/coverage jobs were waiting on.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 14a5931. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

3 participants