Skip to content

Fix npm lock-entry refusals missed by vendored preview (#1976) - #1977

Merged
Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-npm-lock-entry-preview-refusals
Nov 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 5 commits into
mainfrom
agent/fix-npm-lock-entry-preview-refusals

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

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-patches rollout plan now see the backend's per-entry
refusals. 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:

  • previews would_refuse with the backend's code and words, instead of
    verified ("Would download and vendor …");
  • holds no rollout slot, so a vendorable patch ranked below it is no
    longer deferred on every run;
  • has no patch view fetched in the wet run: it gets a failed event
    with 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), the
same way the v1-lock case does.

Root cause

A vendored run asks socket_patch_core::vendor::lock_text_refusals which
packages the backend refuses on the lock text alone, but there were two gaps:

  1. npm_flavor::lock_text_refusals covered pnpm, yarn classic and yarn
    berry only. A test explicitly pinned package-lock as "refuses nothing".
    The package-lock backend runs every one of those gates in preflight
    before it reads the package or the patch, so the same dry-run
    backend call answers exactly.
  2. The dry-run preview (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 (the
    v1-lock refusal).

Changes

  • core npm_flavor::lock_text_refusals: add NpmLockFlavor::PackageLock.
  • CLI agent_download::unclaimed_lock_text_refusals: the download phase's
    lock-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.
  • CLI scan/vendor_flow.rs: preview_vendor and preflight_refused_purls
    ask 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):

Issue shape Test
#1976 URL spec, git spec, workspace member: core answers the backend's refusal vendor::npm_flavor::lock_text_refusal_tests::package_lock_entry_refusals_are_the_backends_own_words
#1976 dry run previews would_refuse (all three shapes) scan::vendor_flow::npm_lock_entry_refusal_tests::preview_refuses_a_package_lock_entry_the_backend_refuses
#1976 --max-new-patches planning gives the slot to is-number (all three shapes) …::planning_drops_a_package_lock_entry_the_backend_refuses
review: same-uuid non-detached ledger entry still gated (dry run = wet run) …::preview_asks_a_same_uuid_entry_the_wet_run_still_gates

Red run with only the core flavor line reverted:
url: left-pad must preview would_refuse, got WouldVendor and
url: only left-pad is refused (2 failed). The core test was red before
its 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:
    • The container runs as root, so the forced write/remove-failure tests can't make writes fail: core pypi_poetry/pypi_requirements/vlt_heal/copy_tree and CLI in_process_redirect / repair.
    • e2e_bun_lockb uses a local bun 1.4.4 (CI pins 1.4.2) and makes a live blob fetch.
    • All of these are hosted, repair or PyPI paths this PR doesn't touch.
  • After the review fixes: CLI lib (943), scan (247), get (173), vendor (151), scan_vendor_e2e (59) and the npm capstones e2e_vendor_npm_build / e2e_redirect_npm_build with SOCKET_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 high and fixed these findings: the ledger-scope
mismatch (now one shared helper), the stale docs in agent_download,
preview_vendor and the contract table, and the wasted gate run when
the v1-lock refusal already covers every npm purl. Not fixed here:

  • Planning and preview each compute the refusal set, and
    hosted_claimed_purls is read once per helper. This is pre-existing
    structure. 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 the
    two PRs meet, the lock-text arm here simply stays in preview_vendor_with.
  • The preview's installed-copy lookup can't reuse scan's crawl snapshot
    (prior: None). The lookup only runs for a refused package the lock
    doesn't resolve to a registry source, so the cost is limited to the
    packages this PR refuses.
  • Each refused package re-runs a dry-run backend call to get its
    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-patches planning, 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_refusals includes NpmLockFlavor::PackageLock, so lock-text gates run before any patch view fetch for URL/git/linked/bundled entries and workspace members.

CLI: unclaimed_lock_text_refusals centralizes the same hosted-pin, detached-ledger, and “reaches backend” filters for download, preview_vendor, and rollout. Dry-run emits would_refuse with backend codes; refused packages no longer consume rollout slots or show as vendorable.

Docs: CLI_CONTRACT.md documents 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

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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 2, 2026 09:20
@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 Nov 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at 6879d83d.

  • CI: 82/82 checks on the head passed or were skipped (32 success, 50 skipped). No main-wide failures.
  • Bugbot: reviewed 6879d83d, no findings. No unresolved review threads.
  • Mergeable against main, and CHANGELOG.md is untouched.

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

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: In npm package-lock projects, packages whose lock entry can't be rewritten (URL, git or file: specs, bundled, shrinkwrapped or linked) and workspace members are now refused before the patch view is downloaded, the same way pnpm, yarn and cargo already were. The download phase's lock-text logic moves into a shared unclaimed_lock_text_refusals helper, which the vendored dry-run preview and the --max-new-patches rollout plan now also use. Those packages now show would_refuse and no longer take a rollout slot. This also fixes the same preview gap for pnpm, yarn and cargo.

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

  • Edge cases I checked:
    • A re-vendor to a new uuid is not falsely refused, because file:.socket/vendor/... is exempt.
    • A v1 lockfile now refuses earlier, with the same code.
    • Hosted pins are excluded.
    • Bun and vlt are untouched.
    • The Windows test branch writes a real directory instead of a symlink.
  • Revert checks:
    • Without PackageLock in the core gate, the core test and all 3 CLI regression tests fail.
    • With the CLI wiring stubbed out, all 3 CLI regression tests fail.
  • Local tests:
    • cargo test: core lock_text_refusal 8/8, CLI lib vendor_flow/lock_text/agent_download 28/28.
    • Integration tests: scan_vendor_e2e 59, get 173, scan 247 and vendor 151, all passing.
    • The CI-form clippy (-p core -p cli --all-features -D warnings) is clean.
  • On GitHub: CI 82/82 passed or skipped on the head (ci-ok and clippy green). Bugbot reviewed with no findings. No unresolved threads. CHANGELOG.md is untouched.

Non-blocking notes

  • A wet vendored scan now runs hosted discovery and the lock-text gate twice, once in planning and again in the download phase. With this PR, that happens for any npm or cargo selection, not only when a pnpm-lock.yaml exists. The PR defers this duplication to Fix vendored planning ignoring PyPI refusals (#1908) #1930. A cheap follow-up is to compute claimed only when the core gate returns something.
  • CLI_CONTRACT.md:350 says "same-uuid ledger entries left out". Only detached same-uuid entries are left out, so it should say that.
  • No test covers the wet download-phase refusal for package-lock. The preview, plan and core tests pin the behavior.

Auto-merge is armed, so approving sends it straight to the merge queue.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[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 6879d83:


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[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.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 2, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 2, 2026
#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
@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.

✅ 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.

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: re-applied Ready for review at f269b04d4ca8b0166f20a3c288b8953785869123.

  • I removed the label briefly while starting a conflict fix, then backed off when I saw another agent had already merged main.
  • Mergeable, CI 50/50 passed or skipped on this head, and Bugbot reviewed this head with no findings.
  • The earlier approval was on the pre-merge head, so it needs re-approval.

Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[final reviewer] Re-enqueued at f269b04d4: approved (approval stays valid after the post-approval pushes), ci-ok and clippy green, mergeable, no unresolved threads. Armed squash auto-merge, which put it back into the merge queue after the earlier dequeue.


Generated by Claude Code

Merged via the queue into main with commit 480f3a0 Nov 2, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-lock-entry-preview-refusals branch November 2, 2026 18:00
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