Repository navigation
fix: vendor hosted patches directly from lockfiles - #1648
Mikola Lysenko (mikolalysenko) wants to merge 9 commits into
Conversation
|
[agent] I ran Question for a human: when As written (
Other findings for the author, not verified in depth:
CI on the current head is green for the draft subset (6 passed, 31 skipped). The full matrix and Bugbot haven't run yet. Generated by Claude Code |
|
[agent] Burn-down agent: I ran Question: when
With (a), manifest patches that have no hosted pin are skipped. The run still exits 0, so a fresh checkout installs those packages unpatched. Projects that use a manifest also hit new refusals: Confirmed failures (local
The review also flags the same takeover expectations in Other review findings to handle once (a) or (b) is chosen:
Generated by Claude Code |
(a) is the correct approach for now. If a user wants to vendor the manifest patches they should do a full scan with vendor mode and apply the edits in the lockfiles. |
A plain `vendor` with a hosted pin in scope now ejects exactly the lockfile's hosted patch, even when `.socket/manifest.json` names another patch for the same purl; vendoring manifest patches takes a full `scan --mode vendored`. The hosted→vendored and A→B→A tests expected the old per-purl takeover onto the manifest's patch. Point the eject at the mock API, serve the granted crate (with the sha512 the artifact download checks), and assert the new contract: the hosted UUID is vendored and attested, the manifest-only UUID is neither vendored, reported nor attested, and the manifest is left byte-identical. The fail-closed test now covers both refusals: `--offline` is refused up front (`offline_eject_unavailable`), and an index that cannot re-resolve the crates.io entry still yields `redirect_revert_failed` with the checkout remedy, nothing touched. Co-Authored-By: Claude <noreply@anthropic.com>
|
Burn-down agent: picked this up now that the product question is answered (17:43 comment: (a), Why CI is red on Reproduced locally on this head:
Next step: rewrite each of these as an eject. Mount the hosted record endpoint for Not labeled; state set to Generated by Claude Code |
With a hosted pin in scope, plain `vendor` ejects the lockfile's hosted patch and ignores `.socket/manifest.json`; the bun hosted→vendored and dry-run tests still expected the manifest's patch (uuid_v) to take over the hosted pin, with the per-purl takeover advisories. Point `vendor_cmd` at the mock API so the eject can fetch the record, expect the hosted uuid to be vendored (detached ledger entry, VEX on the view's CVE), and assert that the manifest-only patch is not previewed, reported or vendored and that the manifest is left byte-identical. The dry run now previews the eject (`eject_planned`) instead of the old `vendor_would_revert_redirect` advisory. Co-Authored-By: Claude <noreply@anthropic.com>
The classic and berry hosted→vendored legs expected plain `vendor` to take over the hosted pin with the staged manifest's patch (UUID_V) and announce `vendor_takeover_reverted_redirect`. With a hosted pin in scope `vendor` now ejects the lockfile's hosted patch and ignores the manifest. Point the vendor runs at the mock API (the eject fetches the record from `patches/view`), expect UUID_H and its CVE in the manifest-less VEX matrices with UUID_V as the never-attested patch, and replace the takeover-advisory check with the eject contract: the manifest-only patch is neither reported nor vendored, and the manifest is not rewritten. Co-Authored-By: Claude <noreply@anthropic.com>
The test staged a manifest naming UUID_V over a hosted go.mod pin and expected plain `vendor` to take the pin over with the manifest's patch. With a hosted pin in scope `vendor` now ejects the lockfile's hosted patch (UUID_H) and ignores the manifest; the run reached the public proxy for the record and failed. Point the run at the mock API, serve the hosted patch's prebuilt module from the artifact-service mock, and expect the go.mod replace to name UUID_H. Assert the manifest-only patch is neither reported nor vendored and the manifest is left byte-identical; the existing upstream-restore checks (no hosted replace, go.sum back to its pre-redirect bytes, no ledger) are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The "Takeover reconciliation" contract and the `run_eject` doc comment still described plain `vendor` as a per-purl takeover onto the manifest's patches. With a hosted pin in scope it ejects exactly the lockfile's hosted pins as one planned transition and ignores the manifest; say so, point manifest vendoring at `scan --mode vendored`, and keep the per-purl takeover wording for `scan` / `get --mode vendored` and the no-hosted-pin manifest path. Co-Authored-By: Claude <noreply@anthropic.com>
|
Burn-down agent: following the maintainer's answer (keep (a): vendor only the hosted lockfile pins), I pushed
Generated by Claude Code |
Plain `vendor` with hosted pins in scope now ejects only the lockfile pins and fetches each pinned record from the patch API, so tests that staged a manifest and expected the per-purl takeover failed with `patch_fetch_failed` (coverage job). Point those runs at mock API records (sbt, pypi requirements/pipenv, bun, vlt, pnpm/npm conversion, berry) and assert the eject contract: `eject_planned` on dry runs, `eject_refused` / `offline_eject_unavailable` for refusals, the hosted pin's UUID vendored, and the manifest left untouched. The bun prune test now builds its manifest-tracked entry from a registry lock, since only manifest vendoring without hosted pins produces one. The help hygiene test expects the new one-line vendor summary. The vlt dry/wet test exposed a product gap: the eject skipped the warm vlt store heal the per-purl takeover runs, leaving the hosted store copy and hidden lock behind, so a later `vendor --revert` could keep patched bytes under a registry pin. The eject now heals those store entries once it stands and reports `redirect_vlt_reinstall_required`. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
|
Burn-down agent: pushed
Waiting on CI and Bugbot for Generated by Claude Code |
A pin whose upstream restore was refused already has its failure event. Previewing the backend over a lock where that pin is still hosted added a second `failed` event for the same purl with a backend code, which double counted summary.failed and blamed the wrong step. Also stop listing plain `vendor` as a source of vendor_would_revert_redirect in CLI_CONTRACT.md: with a hosted pin in scope it ejects and reports eject_planned / eject_refused instead. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run 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 840e46b. Configure here.
|
Ready for review (burn-down agent).
Generated by Claude Code |
Final review: needs a product decision before approvalFinal review at The change. On main, What that does to an existing user
Questions for the owner
Verified
Labelled Generated by Claude Code |
| // Hosted pins are the input, even when an agent manifest also exists. | ||
| // The raw lockfile inventory does not require VEX eligibility or an | ||
| // installed copy of a patch. Revert needs only the vendor ledger. | ||
| if !args.revert { |
There was a problem hiding this comment.
[agent] The hosted_wiring_contested refusal now runs for manifest projects even when no hosted pin is in scope, so the "preserve legacy manifest-only vendoring when there are no hosted pins" fallback is unreachable whenever any contested hosted ref exists anywhere in the tree (hosted_inventory isn't ecosystem-scoped).
Scenario: a project with .socket/manifest.json runs vendor --ecosystems pypi while an unrelated npm package.json holds a lockless hosted URL (contested). Main vendors the manifest patches; this head exits 1 with hosted_wiring_contested and vendors nothing. The same happens with zero attributable pins and one contested ref.
Fix: when the manifest exists, check contested_refusal() only inside if !pins.is_empty(), so that with no in-scope pins the run falls through to the manifest arm as before.
Resolve the vendor_eject.rs conflict with #1646 by keeping both new tests: this PR's manifest-plus-hosted-pins eject test and main's user-patch consent test for the eject path. Co-Authored-By: Claude <noreply@anthropic.com>
|
Burn-down agent: merged Generated by Claude Code |
socket-patch vendorcould ignore hosted lockfile pins when an agent manifest existed, and an unrelated Python environment could reject a pinned patch as not installed. Select hosted patches from the lockfiles first, download their prebuilt artifacts into.socket/vendor/, and rewrite the dependency files without requiring installed packages.Reuse the existing lockfile readers and vendoring backend. Hosted conversion skips installed-package discovery and a redundant wiring scan, while reusing the restored-lock preview and failure rollback. Preserve the legacy manifest fallback when no hosted pins are in scope, and update CLI help and documentation.
Validation on current
main(dc0dc755d):cargo clippy --workspace --all-features -- -D warningspassed.cargo fmt --all -- --checkandgit diff --checkpassed./code-review highran onee978d6(burn-down agent). Fixed in840e46b: a pin the restore refused no longer gets a second backendfailedevent, and CLI_CONTRACT.md no longer lists plainvendoras emittingvendor_would_revert_redirect. Not fixed here, with reasons:--offlinerefuses, andhosted_wiring_contestedblocks whenever a hosted pin is in scope, even with a manifest: this is the maintainer's chosen behavior (a) (vendor ejects only the hosted lockfile pins). A warning naming the skipped manifest patches would be a reasonable follow-up.hosted-index.tsvmixed with a pnpmcatalog:pin), so dry and wet runs can disagree in that mix: rare, and the wet run still rolls back cleanly.hosted_inventorynow runs before manifest-driven vendoring (an extra lockfile parse),eject: Option<&mut EjectCapture>also switches off discovery, and the vlt heal-target lookup is duplicated three times: maintainability cleanups, left for a follow-up.Note
Medium Risk
Changes default
vendorbehavior when hosted pins and an agent manifest coexist, with all-or-nothing eject rollback; extensive new tests mitigate correctness risk.Overview
Plain
socket-patch vendornow treats hosted lockfile pins as the source of truth, even when.socket/manifest.jsonexists. It runs a single eject (restore upstream → download prebuilt patches → rewire locks) for every in-scope hosted pin, without requiringnode_modules/site-packages and without using installed-package discovery or manifest-selected UUIDs. Manifest-only patches are skipped (unchanged on disk); vendoring those still requiresscan --mode vendored.The eject path fetches each pinned patch from the Socket API, previews backend refusals against restored lock text in dry-run (
eject_planned/eject_refusedinstead of per-purlvendor_takeover_reverted_redirect), refuses--offlineup front (offline_eject_unavailable), and on success heals stale vlt store copies like a takeover. The vendored backend skips a second hosted-pin discovery whenejectis set.CLI help,
CLI_CONTRACT.md,docs/usage.md, and integration tests are updated across npm, Bun, Cargo, PyPI, sbt, and fresh-checkout scenarios.Reviewed by Cursor Bugbot for commit 840e46b. Configure here.
Generated by Claude Code