WS5: one VendoredBackend for vendored apply/revert/repair; cut repair's ledger rebuild - #283
Conversation
vendor, scan/get --mode vendored, vendor --revert, rollback's vendored
leg, remove and repair now go through one VendoredBackend { apply,
revert, repair } over the shared engine (vendor_records_reusing,
dispatch_revert_one_opts). The boxed_* scan shims collapse into one
boxed_vendor_step; the engine future stays boxed inside apply for the
Windows 1 MiB main-thread stack.
repair no longer re-synthesizes vendor ledger entries from lockfiles. A
lockfile reference with no ledger entry fails with vendor_ledger_missing
(artifact-level event: uuid + details.{ecosystem,path}); the remedy is
restoring state.json from version control. Missing or corrupt artifacts
are re-vendored through the same engine as vendor, so the patch
service's prebuilt artifact is downloaded first under --vendor-source
auto, with the local build as the fallback. The fingerprint post-verify,
set-aside of corrupt bytes and carried-inventory refresh are kept.
Removed with the rebuild: repair_vendor.rs, gem Gemfile wiring
reconstruction, and registry_fetch::fetch_npm_unverified. The packing
code (npm_pack, pypi_wheel, berry_zip, registry_fetch, prestage) stays:
depscan does not call it (verified against depscan master 784013d6), but
it is the CLI's own --vendor-source build/auto fallback.
Tests for the reconstruction path are replaced by vendor_ledger_missing
pins per flavor; CLI_CONTRACT, README, CHANGELOG and the v5 plan are
updated.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
vlt-coverage.json still named vlt_repair_reconstructs_the_ledger_from_the_lock, and vendor_vlt_lock_out_of_sync lost the only assertion the coverage check could see when the lock-only reference test switched to vendor_ledger_missing. vendor_vlt_out_of_sync now asserts the refusal detail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
native_binary_alias_and_transitive still expected repair to rebuild a workspace mirror after deleting state.json. Repair now reports that as vendor_ledger_missing (pinned in native_binary_hosted_vendored_takeover_roundtrip), so the leg is gone; the missing/corrupt mirror legs keep running and assert the ledger stays byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
get defaults to hosted since 5e5f5ed, which moved the agent-mode fixtures to --mode agent but missed the #[ignore]d real-toolchain suites (e2e_vlt, e2e_npm, e2e_pypi, e2e_gem, e2e_safety_pnpm). Their plain `get <uuid>` now redirects instead of applying in place, so e.g. vlt_pinned_matrix_agent_get_and_remove saw the copy Absent. This PR touches the vlt-compatibility path filter, which surfaced it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
|
I added the same fix to this PR in f206eba: Generated by Claude Code |
…sist tests A carried-inventory refresh whose ledger write fails now reports vendor_inventory_refreshed next to vendor_state_write_failed and keeps the member-verified rebuild on disk, instead of falling through to vendor_artifact_rebuild_failed and removing it. The persist-failure tests for the removed backfill / anchored / soft reconstruction paths go with them; they only run as non-root, which is why the root sandbox skipped them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
The dead-field clippy fix removed before_hash/after_hash, but the macOS immutable-flag rollback tests read them, so test (macos-latest) no longer compiled. Keep the fields and allow dead_code off macOS only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
|
Many checks on e3717a0 are still running. The vlt Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
left a comment
There was a problem hiding this comment.
v5 review at e3717a0: the shared vendored backend and removal of ledger re-synthesis are useful reductions. However, the new repair adapter changes behavior in two reproducible ways (inline).
I built this head and compared identical fixtures with the previous repair implementation on #279: offline repair regresses from success to failure; service-based repair reports success while changing both the lock and stored fingerprint. These should be fixed before landing.
The simplification that addresses both is to share artifact acquisition while making the operation policy explicit. Repair should retain an immutable expected artifact identity and preserve wiring; ordinary vendor may deliberately select new bytes and rewire. Sharing a general apply method without that distinction imports the wrong side effects.
Also return typed per-package outcomes from the backend. repair currently runs it into a scratch JSON-facing Envelope and interprets event codes back into execution state. Let human/JSON output consume the same result instead. Reuse the shared project context for ledger and format reads.
Validation: clean CLI build; two isolated npm repair reproductions, both compared against the prior implementation. Full workspace/toolchain matrix not rerun.
Review of #283 found two regressions in the shared-engine repair: - A corrupt artifact was moved aside before staging, so its afterHash-verified members were no longer harvested: an offline repair that the previous implementation completed failed with "no local source". The members are now harvested first and passed as the seed. - The post-verify reloaded the ledger the re-vendor had just written, so a service archive with different bytes (same members, new gzip mtime) was committed as `rebuilt` with a rewired lock and new fingerprint. Repair now verifies against the original entry; when the result is not the recorded artifact, that candidate's wiring files and ledger entry are put back from a pre-run snapshot, and a service copy falls back to a build-only rebuild. Legitimate backend migrations of a verified rebuild (the cargo version retag) are kept. Both reproductions are pinned in repair_vendor_e2e. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
|
Thanks for the review. Both P2s are fixed in 6b04d2f and pinned with your reproductions. Replies are on the inline threads. On the design points in the summary:
Validation for 6b04d2f:
Generated by Claude Code |
|
This PR doesn't touch For 6b04d2f's other jobs: in Generated by Claude Code |
|
Restack request from the v5 coordinator (please act now). #280 (ledger-free hosted) is now merged into
|
…endor-backend # Conflicts: # CHANGELOG.md # crates/socket-patch-cli/CLI_CONTRACT.md # crates/socket-patch-cli/src/commands/rollback.rs # crates/socket-patch-cli/src/commands/scan/vendor_flow.rs # crates/socket-patch-cli/tests/covgap_commands_rollback.rs
|
v5 coordinator: #283 is first in the landing order but isn't ready to land yet. On head 5f8573b:
The base-merge requirement is met, since the branch contains the current Generated by Claude Code |
|
vlt This comes from the base, not this PR:
No fix exists on any branch I can see, so there is nothing to port. I'll mark the PR ready once the rest of CI on 5f8573b settles, if it shows only base-inherited reds. Generated by Claude Code |
|
Ready to land. Head: CI summary on 5f8573b. Every failure I've found is one of these three, each inherited from the base branch and explained in earlier comments:
Two macOS hosted-only jobs hit a runner network error: Poetry Review: both P2 threads are fixed in 6b04d2f and resolved. The owner decided typed outcomes can go in a follow-up. Follow-ups (non-blocking):
Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Repair snapshot uses unguarded reads
- Replaced bare tokio::fs::read calls with read_regular_to_bytes in snapshot_wiring and undo_candidate to prevent FIFO blocking and properly handle symlink lockfiles.
Or push these changes by commenting:
@cursor push 89e28f4bcb
Preview (89e28f4bcb)
diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs
--- a/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs
+++ b/crates/socket-patch-cli/src/commands/vendored_backend/repair.rs
@@ -30,7 +30,7 @@
use socket_patch_core::constants::SOCKET_DIR;
use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord};
use socket_patch_core::patch::copy_tree::remove_tree;
-use socket_patch_core::utils::fs::read_regular_to_string;
+use socket_patch_core::utils::fs::{read_regular_to_bytes, read_regular_to_string};
use socket_patch_core::utils::purl::normalize_purl;
use socket_patch_core::vendor::{
self, artifact_is_file_shaped, check_vendored_artifact, load_state, parse_vendor_path,
@@ -1058,14 +1058,10 @@
let mut out = Vec::with_capacity(rels.len());
for rel in rels {
let path = cwd.join(&rel);
- match tokio::fs::symlink_metadata(&path).await {
- Ok(meta) if meta.is_file() => {
- if let Ok(bytes) = tokio::fs::read(&path).await {
- out.push((path, Some(bytes)));
- }
- }
- Ok(_) => {}
- Err(_) => out.push((path, None)),
+ match read_regular_to_bytes(&path).await {
+ Ok(bytes) => out.push((path, Some(bytes))),
+ Err(e) if e.kind() == std::io::ErrorKind::NotFound => out.push((path, None)),
+ Err(_) => {}
}
}
out
@@ -1088,7 +1084,7 @@
async fn undo_candidate(cwd: &Path, c: &Candidate, snapshot: &[(PathBuf, Option<Vec<u8>>)]) {
let wired: HashSet<PathBuf> = c.entry.wiring.iter().map(|w| cwd.join(&w.file)).collect();
for (path, bytes) in snapshot.iter().filter(|(p, _)| wired.contains(p)) {
- if tokio::fs::read(path).await.ok().as_ref() == bytes.as_ref() {
+ if read_regular_to_bytes(path).await.ok().as_ref() == bytes.as_ref() {
continue;
}
match bytes {You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5f8573b. Configure here.
Bugbot on #283: the identity-undo snapshot read lockfiles with bare tokio::fs::read (a FIFO at a wiring path blocks open(2)) and skipped symlinked lockfiles, and the put-back wrote them in place (no stage+fsync+rename, mode bits dropped). The snapshot now reads through read_regular_to_bytes and records a symlinked lockfile's link text; the undo re-links a link that the engine's rename replaced, then writes the target through atomic_write_bytes_preserving_mode. Pinned by repair_identity_undo_follows_a_symlinked_lockfile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
A bare `get` defaults to hosted mode since v5, so the real-vlt get_and_remove leg found the installed copy unpatched. Pass `--mode agent` as #283 does, which this ports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
|
CI settled on
Every review thread is resolved. Ready for the coordinator to squash-merge into Generated by Claude Code |
06437d2
into
release/v5-prerelease
Take #283's deletion of repair_vendor.rs and its ledger-rebuild tests. Keep this PR's help grouping in the README command table with #283's "re-vendor" wording, drop the setup-only `ecosystem_not_setup` row from CLI_CONTRACT, and keep vendor advisories code-free in human output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQaKzoW5dSw9u5pgAAvVRj
…models #283 deletes commands/repair_vendor.rs (this branch's edits there go with it: its flavor sniffs were removed upstream) and moves the vendored wiring list into vendored_backend::repair, which now derives it from formats::registry's VENDORED files as the deleted copy did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BYsX2VfVNKHvfFAJnFryc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHZ8uzdXfkG2zH8ZYDG8ju
* Fix apply of created files from diff caches A diff archive has no delta for a file the patch creates, yet the disk stager counted a cached diff archive as covering the whole patch. With only diffs on disk, `apply --offline` passed the gate, patched the modified files, then failed on the created file's missing blob and left the package half-patched; online `apply` never fetched that blob at all. Coverage is now per file: a diff covers only files with a before-hash, and created files need their blob. Online, a cached diff archive no longer suppresses the download, and the top-up fetches just the created files' blobs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Batch vendor package-reference requests A vendor run asked the patch service for each package's download reference in its own request, though the endpoint takes 500 uuids at once: N round trips and N quota units for N packages. The run's download plan now resolves every planned uuid in one request, sent by the first planned call in place of its own and with the same retries, so an outage costs what it did before. Each package takes its answer from that batch at its turn; one still building is asked again then, as before. Hosted scan's reference lookup is chunked at the endpoint's 500-uuid cap, which it used to exceed with a 400. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Skip downloading pypi sdists vendoring rejects pypi vendoring is wheel-based, yet a pypi patch the service serves as an sdist (every patch without a file qualifier) was downloaded in full, then rejected because it is not a .whl. The service's reference already names the artifact, so a pypi reference whose artifact is not a wheel is now refused before the download, in the vendor loop and in its download plan alike. The outcome is unchanged: `auto` warns and builds the wheel locally, and `service` refuses. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Repair downloads created files' blobs A default (diff-mode) `repair` downloaded only diff archives, but a diff has no delta for a file the patch creates. After such a repair `apply --offline` still could not apply a patch that creates files. In diff mode, repair now also downloads the blobs of created files (and lists them under `--offline` and `--dry-run`), reported as their own blob download. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Defer pristine fetches the service makes moot With the patch service on, `vendor` deferred the registry download of a not-installed package only for cargo; npm, golang and composer packages were downloaded and verified up front even when the service's prebuilt artifact made the pristine copy unnecessary. Those backends also ask the service first and read the pristine tree only on a local-build fallback, so their download is now deferred the same way. A package is deferred only when its fetch would really download: the fetchers' pre-download refusals (a foreign yarn berry cacheKey, a go module go fetches without a proxy, a composer entry with no dist URL) are now one shared check that both the fetch and the deferral use. pypi and gem keep the up-front fetch, which their installed-variant probe reads. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Run the vlt get e2e leg in agent mode A bare `get` defaults to hosted mode since v5, so the real-vlt get_and_remove leg found the installed copy unpatched. Pass `--mode agent` as #283 does, which this ports. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Address review of the waste-review fixes - repair --json no longer counts created-file blobs twice (once under the diff-mode event); the closing line names both failure counts when both passes fail. - The vendor reference batch names the plan from the first call's position on, so a package the loop passed over is never granted. - The npm and yarn classic registry views no longer take a non-http resolution's integrity (a local tarball's hash, a git commit id) as a registry integrity, so such a package is never deferred behind, or vendored from, the service's registry build. - CHANGELOG entries for the new behavior, and the repair event row in CLI_CONTRACT. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB * Keep yarn git deps out of the registry view A yarn classic block resolved to a git repository over plain https (`https://…/repo.git#<commit>`, or a codeload tarball) passed as a registry tarball: its commit id became a sha1 integrity, and an `integrity` field on any git block was kept. With npm now deferring behind the patch service, such a lockfile-only git dependency could be vendored from the service's registry build instead of refusing `vendor_fetch_unverifiable`. A git resolution, over any protocol, now carries no URL and no integrity in the registry view, like npm's non-registry entries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB --------- Co-authored-by: Claude <noreply@anthropic.com>


WS5 from
docs/design/v5-plan.md. Based onrelease/v5-prereleaseat 686e5fb (includes #280, ledger-free hosted).Depscan check (blocking step)
Checked from a fresh clone of SocketDev/depscan master (784013d6). Depscan does not use the CLI's local pack/rebuild path.
workspaces/patches/src/repack/). It never spawnssocket-patch, and it has no Cargo or napi dependency on the socket-patch crates.apply/get/scan/vex, in dev tools and live e2e tests. It also reads fixtures from the submodule.npm_pack,pypi_wheel,berry_zip,registry_fetch,prestage) is kept. It is the CLI's own--vendor-source build/autofallback. Only helpers that just the ledger rebuild used were removed:fetch_npm_unverifiedandgem::reconstruct_gem_wiring.What changed
commands/vendored_backend/withVendoredBackend { apply, revert, repair }.vendor_records_reusing.vendor,scan --mode vendored,get --mode vendored, and v5 WS1+WS2: ledger-free hosted mode, upstream-restore rollback, vendor ejects hosted projects #280's hosted-project eject invendor.boxed_scan_vendor_step*shims,stage_and_vendorandboxed_vendor_recordscollapse into oneboxed_vendor_step.apply, for the 1 MiB main-thread stack on Windows.vendor --revert, the manifest reconcile,rollback's vendored leg and bothremovepaths.repair_vendor.rs(2.6k lines).apply. Under--vendor-source autoit tries the service's prebuilt artifact first and falls back to a local build.--offlinecan still repair it.state.jsonstay byte-identical.vendor_inventory_refreshed).vendor_ledger_missing: an artifact-level event withuuidanddetails.{ecosystem,path}, and exit 1. The remedy is to restorestate.jsonfrom version control. Therollbackandvendormessages for this case now say that too.docs/testing/vlt-coverage.json.getfixtures now pass--mode agent. This covers a gap 5e5f5ed left on the base branch.Behavior notes
vendor's installed-variant probe during repair. Before, repair force-overwrote it. Repair's failure details are now the engine's own messages.vendor_ledger_missingtests for npm, pnpm, yarn berry, bun, bun.lockb and vlt.repair_offline_harvests_a_corrupt_artifacts_valid_membersandrepair_never_rewires_to_different_service_bytes.Validation (head 5f8573b)
cargo clippy --workspace --all-targets --all-features -- -D warnings: clean.cargo test, run locally as root: 10,162 passed, 18 failed. The same 18 fail on the base branch here. They are chmod-based tests that do nothing as root, including v5 WS1+WS2: ledger-free hosted mode, upstream-restore rollback, vendor ejects hosted projects #280's newpartial_lockfile_write_failure_exits_1_and_writes_no_ledger, plus hosted cargo fetches. Re-running the permission-dependent tests as non-root passes them.e2e_redirect_cargo_build: hosted cargo VEX, F77. A separate routine is fixing it.covgap_commands_scan_mod.install-proof/e2e_redirect_vlt_build: hosted rollback returnshosted_wiring_contested. The same failure is in v5 WS1+WS2: ledger-free hosted mode, upstream-restore rollback, vendor ejects hosted projects #280's run 36383945937.ConnectionError/ DNSnodename nor servname). The vendored cases in the same jobs pass. I've re-run them once.🤖 Generated with Claude Code
https://claude.ai/code/session_01DMhWChtaNX5FJYNDq3NBJa
Note
Medium Risk
Touches every vendored workflow and changes recovery when the vendor ledger is missing; incorrect repair/revert behavior could leave lockfiles pointing at bad artifacts, but verification and fail-closed ledger rules limit blast radius.
Overview
Introduces a shared
VendoredBackend(apply/revert/repair) sovendor, vendoredscan/get,rollback,remove, andrepairall use the same vendoring engine and revert policy instead of the deletedrepair_vendor.rsmonolith.repairnow re-vendors broken artifacts like a freshvendorrun (patch-service prebuilt under--vendor-source auto, local build as fallback) and verifies against the existing ledger fingerprint; it no longer rebuilds.socket/vendor/state.jsonfrom lockfile references—those cases fail withvendor_ledger_missingand docs/rollback messages tell users to restorestate.jsonfrom VCS. CI path filters point atvendored_backend/**; contract/README/CHANGELOG reflect the new semantics.Reviewed by Cursor Bugbot for commit 5f8573b. Configure here.