GeezerVison is showing github.com — Fix vlt vendoring of prepare-family scripts (#1633) by mikolalysenko · Pull Request #1637 · SocketDev/socket-patch
Conversation
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1633
Summary
Vendored vlt mode now refuses a package whose preprepare or postprepare script vlt build would run inside the committed vendor directory. Before, such a package was vendored with exit 0, and the next vlt ci rewrote the artifact. After that vendor --check and vex failed, and repair looped.
Root cause
vlt_build_steps (crates/socket-patch-core/src/vendor/npm_dir.rs) lists what vlt build runs in a package's own directory. Vendored vlt refuses any package with a non-empty result (vendor_vlt_build_scripts_unsupported). The list had preinstall, install, postinstall and prepare. vlt also runs preprepare and postprepare for a prepable (file:) node, and each one runs even when it is the package's only script: vlt's nodeNeedsBuild tests prepare || preprepare || postprepare.
Changes
Test evidence
Red→green regression tests for #1633. Each was run failing before the fix and passing after it:
vendor::npm_dir::tests::vlt_build_steps_name_every_script_vlt_build_runs_in_place: preprepare-only and postprepare-only manifests (before: left: []).
vendor::vlt_lock::tests::the_preflight_decides_every_refusal_without_writing: preflight refuses both scripts.
vendor::vlt_lock::tests::a_prepare_family_only_package_refuses_on_the_local_build: a full vendor run refuses both scripts, writes nothing, and leaves the lock untouched.
vendor::vlt_lock::tests::a_dir_vendored_with_a_postprepare_script_refuses_on_reuse: the committed dir was vendored before the fix, the installed copy is clean, and the dir is refused on reuse (dry run and real run) with the lock unchanged. With the reuse check disabled, this test fails.
Local runs:
cargo fmt --all -- --check: clean
cargo clippy --workspace --all-features -- -D warnings: clean
cargo test --workspace --all-features --no-fail-fast: 232 suites ok. 4 suites had environment-only failures, none in vlt build-step code: e2e_bun_lockb and vlt::scan_redirect_vlt_heal_invalidation_failure_warns need network, and the rest are write-failure/unremovable-file tests that can't fail as root in this container.
e2e_vendor_vlt_build with vlt 1.2.0: couldn't run here, because the container's proxy blocks the suite's npmjs fetch (could not fetch [email protected] from npmjs). CI runs it on Linux/macOS/Windows.
Per-issue checklist
Review notes (/code-review high)
- I didn't share a constant with the six-name list in
bun_lockb.rs. That list is bun.lockb's binary field order for its script table, which is a different contract from "what vlt runs in place". Coupling the two would let a change to one silently change the other.
- The empty "Start fix" commit comes from the agent claim protocol. The PR is squash-merged.
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
vlt runs a package's preprepare and postprepare scripts for a file:
directory dependency, each on its own, just like prepare. Vendored vlt
mode only refused packages with preinstall, install, postinstall or
prepare, so a package whose only script was postprepare got vendored.
The next `vlt ci` then ran that script inside the committed vendor
directory: `vendor --check` and `vex` failed, and `repair` looped.
Such packages now get vendor_vlt_build_scripts_unsupported like the
others, and the docs name all six scripts.
Fixes #1633
Assisted-by: Claude Code:claude-opus-5-5
A project that vendored a postprepare-only package with an earlier
release kept that directory on every re-run, so the next `vlt ci`
still rewrote it. Re-running vendor now refuses such a directory with
the same vendor_vlt_build_scripts_unsupported refusal, pointing to
--mode hosted, instead of reporting success.
The refusal detail now lists several build steps with commas instead
of a chain of "and".
Refs #1633
Assisted-by: Claude Code:claude-opus-5-5
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 8ab8b12. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Labeled Ready for review (burn-down agent).
- Head:
8ab8b124
- CI: all checks green on the head (success/skipped only); mergeable, no conflicts; no
CHANGELOG.md change.
- Bugbot: reviewed
8ab8b124, no findings; 0 open review threads. Already approved by Tanmay Singla (@Tanmay182003) on this head.
- Worth a look: the reuse check in
vlt_lock.rs that refuses already-vendored dirs carrying preprepare/postprepare scripts.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
The merge queue dropped this PR at 18:02 UTC. The failing check was e2e (ubuntu-latest, e2e_vex_build, pipenv:: --ignored, 2026.8.0), in run 39470447347.
- What failed:
pipenv::pipenv_pylock_projects_wire_what_pipenv_installs. uv pip install -r pylock.toml exited 2 with The target environment is not supported by the lockfile.
- Why it isn't this PR's: the diff only touches vlt vendoring (
npm_dir.rs, vlt_lock.rs, docs). It doesn't touch pipenv, pylock or uv. The same leg passed two minutes later on queue commit 947f8dfe for Fix yarn copies with invalid manifest names (#1598) #1605, and that tree already contains this PR's change.
- Fix: none exists yet. The pylock test looks non-deterministic in the uv environment, so it should get its own issue.
I'm re-queuing once. If the leg fails again, I'll treat it as a real failure.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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
Milestone
No milestone
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.
|