Repository navigation
Fix npm lock-entry refusals missed by vendored preview (#1976) - #1977
Mikola Lysenko (mikolalysenko) merged 5 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A vendored run refuses a package its backend cannot wire from the lock text alone before it fetches the patch view. npm package-lock projects were left out, so a package installed from a URL or git spec, or a workspace member, was downloaded first and only then refused. lock_text_refusals now answers for package-lock too, in the backend's own words, so the download phase skips the fetch (#1976). Assisted-by: Claude Code:claude-opus-5-5
A vendored dry run and the --max-new-patches rollout plan never asked which packages the backend refuses on the lock text alone. An npm package installed from a URL or git spec, or a workspace member, previewed as vendorable and took the only rollout slot on every run, so a fixable patch ranked below it was deferred forever. Both now ask the same lock-text refusals the download phase raises, with the same scope, so such a package previews would_refuse with the backend's code and holds no rollout slot (#1976). Assisted-by: Claude Code:claude-opus-5-5
The preview and rollout plan read the lock-text refusals through their own copy of the download phase's filters, and the copies disagreed on the ledger: a same-uuid entry written by plain vendor previewed as already vendored while the wet run still refused it. Both now go through one helper, unclaimed_lock_text_refusals, so the dry run and the wet run ask the same purls. npm purls are not asked again when the project-level package-lock refusal already covers them, and the contract's dry-run error-code row names the new codes. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
Ready for review at
Slack announcement didn't go out this run (the Slack send tool wasn't available), so the next run will retry it. Generated by Claude Code |
Final review briefWhat it does: In npm package-lock projects, packages whose lock entry can't be rewritten (URL, git or Risk: low to medium. It fails closed: a refusal is only raised when the real backend, run as a dry run, refuses with the same code. Exit and status semantics don't change, because the wet run already refused these packages. The medium part: it changes which patches are fetched, previewed and given slots across every vendored npm and cargo scan, and adds extra lockfile discovery to every wet vendored scan. Look here
Verified
Non-blocking notes
Auto-merge is armed, so approving sends it straight to the merge queue. Generated by Claude Code |
|
[agent] On the final review brief's non-blocking notes. None of them is worth a push that would restart CI and drop the armed auto-merge, so the head stays at
Generated by Claude Code |
Wenxin Jiang (Wenxin-Jiang)
left a comment
There was a problem hiding this comment.
[agent] Reviewed 6879d83d4c01714212dc9ebdca968a6ccfc5a09e. Checked package-lock entry refusal propagation to download, preview and rollout, including hosted/ledger exemptions and reachability scope. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.
#1930 made the rollout plan read the dry-run preview, so the lock-text refusal now lives only in the preview and the separate planning pass is gone. Also corrects the contract: only a detached ledger entry already holding this uuid's record is left out. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 f269b04. Configure here.
|
Burn-down agent: re-applied Ready for review at
Generated by Claude Code |
|
[final reviewer] Re-enqueued at Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1976
Summary
In an npm package-lock project, a vendored dry run and the
--max-new-patchesrollout plan now see the backend's per-entryrefusals. These cover a package installed from a URL or git spec (or
bundled, shrinkwrapped or linked), which is
vendor_lock_entry_not_rewritable,and a workspace member, which is
vendor_workspace_member. Such a package:would_refusewith the backend's code and words, instead ofverified("Would download and vendor …");longer deferred on every run;
failedeventwith the same code before the download, like the pnpm / yarn / cargo
lock-text refusals already do.
The wet run still refuses the package (exit 1,
partialFailure), thesame way the v1-lock case does.
Root cause
A vendored run asks
socket_patch_core::vendor::lock_text_refusalswhichpackages the backend refuses on the lock text alone, but there were two gaps:
npm_flavor::lock_text_refusalscovered pnpm, yarn classic and yarnberry only. A test explicitly pinned package-lock as "refuses nothing".
The package-lock backend runs every one of those gates in
preflightbefore it reads the package or the patch, so the same dry-run
backend call answers exactly.
preview_vendor) and the rollout planning pass(
preflight_refused_purls) never asked the lock-text refusals at all.They asked only the project-level
npm_lock_vendor_preflight(thev1-lock refusal).
Changes
npm_flavor::lock_text_refusals: addNpmLockFlavor::PackageLock.agent_download::unclaimed_lock_text_refusals: the download phase'slock-text scope, extracted so the preview and plan use the same
filters: hosted pins left to their takeover, the ledger's
detached-record idempotency skip, and only packages that reach the
backend.
scan/vendor_flow.rs:preview_vendorandpreflight_refused_purlsask it. npm purls are skipped when the project-level package-lock
refusal already refuses them all.
CLI_CONTRACT.md: the "Lock-text refusals before the download"paragraph now covers package-lock and the dry run / rollout plan, and
the dry-run error-code row lists the lock-text codes.
The same preview gap also existed for non-hosted pnpm / yarn / cargo
lock-text refusals. The shared helper closes it for them too, because the
cause is the same.
Test evidence
Regression tests (red without the fix, green with it):
vendor::npm_flavor::lock_text_refusal_tests::package_lock_entry_refusals_are_the_backends_own_wordswould_refuse(all three shapes)scan::vendor_flow::npm_lock_entry_refusal_tests::preview_refuses_a_package_lock_entry_the_backend_refuses--max-new-patchesplanning gives the slot to is-number (all three shapes)…::planning_drops_a_package_lock_entry_the_backend_refuses…::preview_asks_a_same_uuid_entry_the_wet_run_still_gatesRed run with only the core flavor line reverted:
url: left-pad must preview would_refuse, got WouldVendorandurl: only left-pad is refused(2 failed). The core test was red beforeits fix as well.
Local runs (Linux, npm 10.9.9, node 22):
cargo fmt --all -- --check: ok.cargo clippy --workspace --all-features -- -D warnings: ok.cargo test --workspace --all-features --no-fail-fast: 236 targets ok. Four targets have failures that are environmental only:pypi_poetry/pypi_requirements/vlt_heal/copy_treeand CLIin_process_redirect/repair.e2e_bun_lockbuses a local bun 1.4.4 (CI pins 1.4.2) and makes a live blob fetch.scan(247),get(173),vendor(151),scan_vendor_e2e(59) and the npm capstonese2e_vendor_npm_build/e2e_redirect_npm_buildwithSOCKET_PATCH_NPM_E2E_REQUIRED=1 --include-ignored: all pass.No wrapper (
npm/,pypi/,gem/) changes are needed. They only dispatch to the binary.Code review
I ran
/code-review highand fixed these findings: the ledger-scopemismatch (now one shared helper), the stale docs in
agent_download,preview_vendorand the contract table, and the wasted gate run whenthe v1-lock refusal already covers every npm purl. Not fixed here:
hosted_claimed_purlsis read once per helper. This is pre-existingstructure. Fix vendored planning ignoring PyPI refusals (#1908) #1930 already rewrites the planning pass to read the preview
(
VendorPreview::refused_purls), which removes the duplicate. When thetwo PRs meet, the lock-text arm here simply stays in
preview_vendor_with.(
prior: None). The lookup only runs for a refused package the lockdoesn't resolve to a registry source, so the cost is limited to the
packages this PR refuses.
detail. That is the existing design for the pnpm/yarn flavors and is
only paid per refused package.
https://claude.ai/code/session_011f9aeNdbWyts21KPvAsH9u
Note
Medium Risk
Changes vendored scan/get decision paths (preflight, dry-run, rollout caps) for npm package-lock projects; behavior should match existing wet-run refusals but affects when network fetches and slot allocation occur.
Overview
Fixes #1976: vendored dry-run,
--max-new-patchesplanning, and the wet download phase now agree on npm package-lock per-entry refusals (vendor_lock_entry_not_rewritable,vendor_workspace_member), not only pnpm/yarn/cargo.Core:
npm_flavor::lock_text_refusalsincludesNpmLockFlavor::PackageLock, so lock-text gates run before any patch view fetch for URL/git/linked/bundled entries and workspace members.CLI:
unclaimed_lock_text_refusalscentralizes the same hosted-pin, detached-ledger, and “reaches backend” filters for download,preview_vendor, and rollout. Dry-run emitswould_refusewith backend codes; refused packages no longer consume rollout slots or show as vendorable.Docs:
CLI_CONTRACT.mddocuments package-lock in lock-text refusals and the dry-run error-code row.Reviewed by Cursor Bugbot for commit f269b04. Configure here.
Generated by Claude Code