Skip to content

Refuse vendored patches the same way everywhere - #2015

Open
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
refactor/drop-vlt-artifact-probefrom
refactor/vendor-refusals-into-core
Open

Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
refactor/drop-vlt-artifact-probefrom
refactor/vendor-refusals-into-core

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Stacked on #2013.

Summary

The vendored refusals a run can decide before it downloads or writes anything are now one core function, socket_patch_core::vendor::refusals::vendor_refusals. It replaces five CLI-side sources that every call site merged by hand:

  • commands/bun_preflight.rs
  • commands/vlt_preflight.rs
  • scan/vendor_flow.rs's npm_lock_refusal
  • the CLI lock_text_refusals wrapper over agent_download::unclaimed_lock_text_refusals
  • pnpm_takeover_lock_text_refusals

Those CLI modules re-implemented the engine's decision order from outside the engine, and covered gaps in core from the CLI (the unreadable-ledger refusal, the rule that vlt beats Bun). Entry points also drifted: each one asked a different subset of the checks.

pub async fn vendor_refusals(
    project_root: &Path,
    candidates: &[(&str, &str)], // (purl, uuid)
    ledger: LedgerLoad<'_>,
    scope: &RefusalScope<'_, '_>, // live hosted pins + installed-copy lookup
) -> VendorRefusals // BTreeMap<purl, VendorRefusal { code, detail }>

It runs the checks in the engine's order, and the first refusal wins:

  1. the npm flavor gate (vlt wins the router; Bun's project refusal with its already-vendored exemption), including vendor_state_unreadable;
  2. the package-lock project refusal (a v1 lock);
  3. the lock-text gates a hosted pnpm pin's takeover meets;
  4. the npm and cargo lock-text gates for every other candidate.

Each check stays in its backend's module; refusals.rs only dispatches and scopes them.

Callers: the scan preview and rollout plan, the scan download phase, both get --mode vendored paths, and the vendor loop (CandidateGates, which also feeds the download plan).

Behavior changes

Codes and detail strings are unchanged. These fix coverage gaps where an entry point skipped checks another one ran:

  1. Scan download phase: it now refuses a v1 package-lock project, or a hosted pnpm pin whose takeover would be refused, before fetching any patch view. Test: download_patch_records_v1_package_lock_refuses_before_any_view.
  2. get <uuid> --mode vendored: every refusal now exits with the error envelope before anything is written; previously only Bun and vlt did. Test: get_uuid_vendored_over_unhosted_v1_lock_refuses_before_any_write.
  3. vendor: it judges all of these before the takeover and the download plan, so a pnpm takeover is refused without first restoring and then rolling back the hosted pin. The now-unreachable package-lock fallback in the takeover is removed. Test: the_plan_leaves_out_a_package_a_v1_package_lock_refuses.

Still in the CLI

  • The gem takeover refusals and the gem and PyPI preview refusals, which depend on CLI-side restore and PyPI routing state.
  • The in-loop takeover checks (berry project, gitignore, berry target, linked vendor dir), which run against the restored pin.

These are candidates for a follow-up.

Validation

  • cargo fmt --check and cargo clippy --workspace --all-features -- -D warnings: clean.
  • Core --lib: 7146 passed, including 12 new vendor::refusals tests moved from the CLI.
  • CLI lib: 862 passed.
  • 27 integration binaries with 0 failures, including get, scan, vendor, repair, rollback, remove, in_process_vendor, the bun/npm-v1/pnpm takeover suites, mode_migration_bun/_vlt, scan_vendor_e2e, and the e2e_vendor_*_build suites.
  • scripts/tests: 384 passed.

Re-verified after rebasing onto #2013: clippy is clean, and the CLI lib (859) plus get, vendor, scan, in_process_vendor, in_process_vendor_bun_takeover, hosted_memory_engine, in_process_redirect, mode_migration_vlt, cli_parse_scan and cli_parse_get all pass.

In core --lib, crawlers::nuget_crawler::tests::solution_roots_and_unrestored_projects_keep_the_walk fails on any machine whose real ~/.nuget/packages holds a package. The test crawls the host's global packages folder, and it passes with an empty HOME. This is unrelated to this stack and also fails on #2013 alone.

Bun and vlt aren't installed locally, so their toolchain-gated cases skipped themselves; the #[ignore]d production tests weren't run.

🤖 Generated with Claude Code


Note

Medium Risk
Changes shared vendored download, dry-run preview, and vendor-loop gating across scan/get/vendor; intentional behavior tightening but high touch area for lockfile and takeover edge cases.

Overview
Vendored-mode checks that block a patch before any view fetch or disk write now go through a single core API, socket_patch_core::vendor::refusals::vendor_refusals, instead of the CLI merging Bun preflight, vlt preflight, npm lock preflight, lock-text helpers, and pnpm takeover logic separately.

The CLI drops bun_preflight.rs and vlt_preflight.rs; agent_download, get, scan/vendor_flow (preview + download), and vendor call vendored_download_refusals / vendor_refusals with one VendorRefusals map in the fetch loop. CLI_CONTRACT.md documents that npm v1 package-lock and hosted-pnpm takeover refusals share that same decision path, including on get <uuid> --mode vendored.

Coverage fixes (same error codes as before): scan’s download phase and get <uuid> now refuse v1 package-lock projects before /patches/view/; vendor applies the full refusal set before hosted→vendored takeover (so pnpm takeover failures don’t briefly un-host). Tests move into refusals.rs in core plus new download/uuid e2e cases; the vlt workflow path filter no longer lists the deleted preflight module.

Reviewed by Cursor Bugbot for commit 42843ef. Configure here.


Generated by Claude Code

A vendored run decides before any download which patches the engine
would refuse, so it never fetches a view, asks for a grant or strips a
hosted pin for a package it cannot vendor. That decision was assembled
in the CLI from five sources (the Bun and vlt preflights, the
package-lock project refusal, the lock-text gates and the pnpm
takeover), and each entry point merged its own subset by hand. They
had drifted: the download phase never asked the package-lock project
refusal or the pnpm takeover gates, `get <uuid>` raised only the Bun
and vlt refusals before the engine, and the vendor loop judged only
Bun and vlt before its takeover.

All of it now lives in one core function,
`socket_patch_core::vendor::refusals::vendor_refusals`, which runs the
checks in the engine's order and scopes the lock-text gates (hosted
pins, the ledger's idempotency skip, packages that never reach their
backend). The scan preview and rollout plan, the download phase, both
`get` paths and the vendor loop call it. The CLI's bun_preflight.rs
and vlt_preflight.rs are gone, and their tests moved to core.

What users see: a v1 package-lock project, or a hosted pnpm pin whose
takeover the pnpm gates refuse, is now refused before its patch view
is fetched; `get <uuid> --mode vendored` raises every such refusal
with its pre-record error envelope and writes nothing; and `vendor`
refuses them before restoring a hosted pin instead of restoring and
rolling back. Codes and details are unchanged.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 4, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Labeled Ready for review by the burn-down agent.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. This PR replaces bun_preflight.rs, vlt_preflight.rs and three CLI refusal helpers (npm lock, hosted pnpm, lock text) with one core function, vendor_refusals in socket-patch-core/src/vendor/refusals.rs. Every caller now reads the same refusal map: the scan preview, the scan/get download phase, both vendored get paths and the vendor loop. The Bun, vlt, unreadable-ledger and already-vendored logic moved over nearly line for line, and I checked each piece against the deleted code; no refusal was dropped.

Risk: medium. This is mostly a move, but more wet paths now apply refusals.

  • The pnpm-takeover lock-text prediction used to gate only the dry-run preview. It now also gates the wet download, get <uuid> and vendor. That is safe as long as the prediction only ever under-predicts.
  • vendor now applies the npm/cargo lock-text gates and the v1 package-lock refusal up front. I checked that the engine (vendor_npm_family, cargo_prelude) runs these same gates before its in-sync path. The tests that a pnpm plain dep or a v2 package-lock still takes over continue to pass.

Look here

  • vendor/refusals.rs:79-148: the order of checks and how each is scoped. The Bun refusal is at 219-243 and vlt at 178-199.
  • vendor.rs:3359: refusals are computed once per run. They are consulted in judge at :2969, and the v1 takeover fallback is removed at :3557.
  • agent_download.rs:767-821:`` the gem takeover refusals are merged in, first one wins. In the fetch loop at :946-975,``` refusals now come before the detached-ledger reuse skip.
  • get.rs:2306-2360: the uuid path refuses before any write.

Verified

  • Read the full diff (13 files, +1311/-1188) against the base refactor/drop-vlt-artifact-probe.
  • All 12 moved tests are present in refusals.rs. The 3 new tests assert codes that only the new path produces.
  • No debug leftovers. CHANGELOG.md is untouched.
  • CRLF is still covered by the pnpm takeover CRLF test.
  • CI: ci-ok and clippy are green, and Bugbot passed.

Changes I made. None.

Open questions (non-blocking)

  • The description says "codes unchanged", but in a few edge cases a different code now wins:

    • In vendor, a lock-text refusal now outranks vendor_dir_symlink_unsupported.
    • A v1 lock now reports vendor_lockfile_version_unsupported rather than vendor_flavor_changed.
    • In the download phase, the v1 refusal now comes before the detached in-sync reuse.

    Each now matches what the preview already reported.

  • Nits for a follow-up:

    • The doc comments at vendor.rs:5497-5505 are misplaced: the new test was inserted between the vlt test's /// comment and its function.
    • The comment at vendor.rs:3486 still mentions "Bun/vlt preflight verdicts".
    • CLI_CONTRACT.md:352 still describes the old ordering.
    • The scan flow computes the refusals twice.

Merge path: this PR is stacked on #2013, so auto-merge is not armed. Once #2013 merges and this PR is retargeted to main, the merge steward will arm it.


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

Development

Successfully merging this pull request may close these issues.

1 participant