GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — v5: fix partial-stage repair bug, cut redundant downloads by mikolalysenko · Pull Request #292 · SocketDev/socket-patch Conversation
|
| Finding | Outcome |
|---|---|
| F67 latent bug: the disk stager treated a diff archive as covering created files | done. I wrote the failing tests first. On the base, apply --offline with only diffs on disk patched index.js, then failed on the created new.js and left the package half-patched. Online apply never fetched the created file's blob, and after a default repair an offline apply still failed. Coverage is now counted per file: a diff covers only files with a beforeHash. Online apply fetches just the created files' blobs, and diff-mode repair downloads them too. That repair's JSON gets one extra downloaded event with mode: "file". |
F28: the vendored path POSTed /patches/package once per uuid |
done. The vendor download plan's first call now sends one reference request, in chunks of 500, for the plan from its own position onward. That request replaces the call's own single-uuid request and uses the same retry ladder, so an outage costs what the serial loop paid. Positions the loop already passed over are never granted. A pending_build answer is asked again at that package's turn. Hosted scan's fetch_registry_references is chunked at 500 too; above that the endpoint returns 400. |
| F70/F71: qualifier-less pypi patches downloaded the served sdist before rejecting it | done (one fix). A pypi reference whose artifact isn't a .whl is refused before the GET, in both the vendor loop and the download plan. Outcomes are unchanged: auto still warns vendor_prebuilt_unavailable and builds locally, and service still refuses. |
| F80: auto mode eagerly fetched pristine upstream for uninstalled packages | done for npm, golang and composer; cargo already deferred. The deferral applies only when the fetch would really download. The fetchers' pre-download refusals are now one shared registry_fetch::refusal_before_download, used by both the fetch and the deferral: foreign yarn berry cacheKey, a go module go fetches without a proxy, a composer entry with no dist URL. The npm and yarn classic inventories no longer treat a file: or git resolution's hash as a registry integrity. pypi and gem keep the eager fetch, because the loop's installed-variant probe reads the pristine tree before the backend runs. nuget and maven have no fetch rung and stay frozen. |
| F59: vex refetches every record per run | deferred. Re-verified on the current base: within one run, vex already uses one client and fetches each distinct uuid once (vex_sources::fetch_records). The remaining waste is across runs, and fixing it needs a persisted per-uuid record cache. That would be new project or user state in a ledger-free hosted mode, and it could hide a withdrawn patch from VEX. It needs an owner decision and belongs with WS6 (#282). |
| F60: hosted scan downloads each patched wheel in full to read METADATA | deferred. Needs server support: depscan would persist PEP 658 metadata and its hash and return them from /patches/package. The serve proxy has no Range support to read the file partially. |
Savings
- F28: a vendored run with N planned service downloads now makes ⌈N/500⌉ reference POSTs instead of N, each costing a round trip plus a quota unit. The depscan fixture's 71 grants become 1 POST.
- F70/F71: every qualifier-less pypi patch in a vendored run saves one full sdist download, up to the 256 MiB cap.
- F80: every not-installed npm, golang or composer package the service serves saves a registry download and verification.
- F67 is a correctness fix; it adds at most the created files' blobs.
- Diff: about +1450 / −170 lines across 16 files, mostly tests.
Tests
tests/diff_created_file_e2e.rs(5): three failed on the base before the fix. They cover offline apply, online apply, repair followed by offline apply, offline repair, and the repair JSON event.fetch_stageunits.vendor_prefetchbatch tests, run against a mock that answers every uuid in a request:- one POST serves the whole plan
- an outage costs the serial loop's ladders
- a building package is asked again
- the batch is capped at 500
- the batch starts at the first call
fetch_registry_referenceschunking.- The pypi sdist tests assert that no GET is made. I confirmed they fail without the fix.
refusal_before_downloadmatches each fetcher's first refusals.scan_vendor_e2e::vendor_auto_takes_a_missing_package_from_the_service_without_the_registry: no registry request, and under--offlinezero requests to either server. I confirmed it fails when deferral is limited to cargo.- yarn git and npm
file:inventory tests. - Local results after the latest merges (a77ecad, b2df60b):
cargo clippy --workspace --all-targets -D warningsis clean.- The cli lib tests pass (805).
- These e2e suites pass:
diff_created_file_e2e(5),scan_vendor_e2e(32),covgap_commands_scan_hosted(50),covgap_commands_vex(12),e2e_vex(16),e2e_vex_vendor(21),hosted_memory_engine(28),hosted_memory_parity(25). covgap_commands_vendor(43) andrepair_invariants(22) pass when run as a non-root user; their chmod-based cases can't fail under root.
- Two adversarial reviews: correctness, and coverage/offline/owner decisions. Their confirmed findings (the repair double count, batch over-granting, non-registry npm entries, docs) are fixed in 7cfdbd4.
CI failures that come from the base (details in the comments; each reproduces on a clean base checkout, and this PR doesn't touch the code involved):
- vlt
install-proof(0.0.0-1, 0.0.0-11): after WS3: one lockfile model per ecosystem #281, hosted rollback ine2e_redirect_vlt_build(hosted_rollback_byte_exact,hosted_idempotence,hosted_crlf_lock) writesvlt-lock.jsonback without each node's tarball URL. - yarn-classic 1.0.2, 1.6.0, 1.7.0, 1.9.4: after v5 WS1+WS2: ledger-free hosted mode, upstream-restore rollback, vendor ejects hosted projects #280,
mode_migration_npm::classic_vendored_then_hosted_takeover_leaves_pure_hostedfails because the upstream restore refuses a pre-1.10 lock entry, which has nointegrityfield.
On b2df60b every other job is green. Three production-service cells each failed once and passed on their one re-run: Pipenv 2026.8.0 crlf hosted, vlt 1.2.0 vendored-direct (ubuntu), and Poetry 2.0.1 direct (ubuntu).
Process note: early on I force-pushed once, after amending my own not-yet-reviewed commit. Everything since is plain commits and merge commits.
🤖 Generated with Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
8342baf to
ea2dedf
Compare
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
|
[agent] CI note on the red checks. This PR is still a draft, waiting on #283.
Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- 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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
|
#283 landed on release/v5-prerelease as 06437d2; please merge origin/release/v5-prerelease again, resolve conflicts, get green, and keep it ready. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] ready to land This is head 9a10303, which is CI on 9a10303: The remaining reds also fail on #283 on the same base:
Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Resolved by another fix: Git HTTPS locks deferred as registry
- The bug was already fixed in commit 7ce6840 which filters git resolutions (including .git URLs and codeload.github.com) from the Yarn classic registry view and clears integrity fields on git dependencies.
Or push these changes by commenting:
@cursor push 62e896c51a
Preview (62e896c51a)
diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs b/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs --- a/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/mod.rs @@ -65,8 +65,8 @@ pub(crate) mod pnpm; pub(crate) mod pypi; pub(crate) mod recover; +pub mod view; pub(crate) mod vlt; -pub mod view; pub(crate) mod wired; pub(crate) mod yarn; @@ -365,7 +365,16 @@ /// cannot reproduce; such entries stay listed for discovery but the fetch /// layer's integrity rule decides fetchability). fn http_url(raw: &str) -> Option<String> { - (raw.starts_with("https://") || raw.starts_with("http://")).then(|| raw.to_string()) + if !(raw.starts_with("https://") || raw.starts_with("http://")) { + return None; + } + // A `.git` suffix (with or without a `#fragment`) is a git repository, + // not a tarball artifact the registry conventions serve. + let before_fragment = raw.split('#').next().unwrap(); + if before_fragment.ends_with(".git") { + return None; + } + Some(raw.to_string()) } /// ARCHITECTURE GUARD (module docs): each per-format file is laid out as diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -907,6 +907,35 @@ assert_eq!(e.integrity, LockIntegrity::None); } +/// An https URL ending in `.git` is a git repository, not a registry +/// tarball: the `#<commit>` fragment is not a tarball sha1, and a +/// standalone integrity field verifies nothing the registry serves. +#[tokio::test] +async fn yarn_classic_dot_git_https_url_is_not_a_registry_entry() { + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "yarn.lock", + "# yarn lockfile v1\n\n\ + \"[email protected]\":\n\ + \x20 version \"1.0.0\"\n\ + \x20 resolved \"https://github.com/o/https-git.git#0123456789abcdef0123456789abcdef01234567\"\n\ + \n\ + \"with-integrity@git+https://example.com/with-integrity.git\":\n\ + \x20 version \"2.0.0\"\n\ + \x20 resolved \"git+https://example.com/with-integrity.git#abc\"\n\ + \x20 integrity sha512-standaloneIntegrity==\"\n", + ) + .await; + let (_, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); + let e1 = entry(&entries, "https-git"); + assert_eq!(e1.resolved, None); + assert_eq!(e1.integrity, LockIntegrity::None); + let e2 = entry(&entries, "with-integrity"); + assert_eq!(e2.resolved, None); + assert_eq!(e2.integrity, LockIntegrity::None); +} + // ── yarn berry ──────────────────────────────────────────────────────── const YARN_BERRY: &str = "# This file is generated by running \"yarn install\" inside your project. diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/yarn.rs b/crates/socket-patch-core/src/vendor/lock_inventory/yarn.rs --- a/crates/socket-patch-core/src/vendor/lock_inventory/yarn.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/yarn.rs @@ -141,7 +141,8 @@ // `resolved "url#sha1hex"` — the fragment is the legacy verifier of // a registry tarball. A git resolution's fragment is a commit id, // which verifies nothing a registry fetch could download. - let (resolved, sha1_hex) = match classic_field(&block.lines, "resolved") { + let resolved_raw = classic_field(&block.lines, "resolved"); + let (resolved, sha1_hex) = match resolved_raw { Some(raw) => { let (url, sha1) = split_resolved_sha1(raw); match http_url(url) { @@ -151,10 +152,16 @@ } None => (None, None), }; - let integrity = classic_field(&block.lines, "integrity") - .map(|i| LockIntegrity::Sri(i.to_string())) - .or(sha1_hex.map(LockIntegrity::Sha1Hex)) - .unwrap_or(LockIntegrity::None); + // A non-registry resolution (`file:`, `git+…`, `.git`) records the + // integrity of an artifact no registry serves: nothing a registry + // fetch could verify against. + let integrity = match (&resolved, resolved_raw) { + (None, Some(_)) => LockIntegrity::None, + _ => classic_field(&block.lines, "integrity") + .map(|i| LockIntegrity::Sri(i.to_string())) + .or(sha1_hex.map(LockIntegrity::Sha1Hex)) + .unwrap_or(LockIntegrity::None), + }; out.push(LockfileEntry::npm(name, version, resolved, integrity)); } out
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 9a10303. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
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) <[email protected]> Claude-Session: https://claude.ai/code/session_01WzdTEhubve9yWfqBE7vAsB
|
#281 landed on release/v5-prerelease as 73c0c4f; please merge origin/release/v5-prerelease again, resolve conflicts, get green, and keep it ready. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] I merged The vlt I reproduced the same three assertion failures on a clean Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] The I reproduced it locally with yarn 1.6.0. yarn releases before 1.10 never write an
With that edit the test passes on yarn 1.10.1 and still fails on 1.6.0. Proposed fix, for #280's owners: serve the real registry's Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
#279 landed on release/v5-prerelease as f6bdad5; please merge origin/release/v5-prerelease again, resolve conflicts, get green, and keep it ready. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] I merged Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] On 6f61425, Pipenv compatibility The same workflow passed on this PR's previous head b2aea01 and on #279's final head, so so far it has failed only on their merge. Locally I can't tell a regression from a flake: the cell fails every time on 6f61425, f6bdad5 and 73c0c4f alike, so the failure here comes from this environment (Python 3.12 instead of CI's interpreters). I've re-run the failed job once. If it fails again I'll treat it as a real interaction between #279 and this PR and fix it here. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
#282 landed on release/v5-prerelease as 14a9cb0; please merge origin/release/v5-prerelease again, resolve conflicts, get green, and keep it ready. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…aste-w4 # Conflicts: # CHANGELOG.md
|
[agent] I merged Locally:
On the previous head, 6f61425, CI finished with only the base-caused reds described above: vlt Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] On a77ecad, vlt This looks like a flake, not a regression from the #282 merge:
I can't reproduce it locally because this sandbox's network policy blocks Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
#291 landed on release/v5-prerelease as f9cb7e1; please merge origin/release/v5-prerelease again, resolve conflicts, get green, and keep it ready. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] I merged Locally:
On a77ecad the vlt Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] On b2df60b, Poetry This doesn't look like a code change:
Once the Poetry run finishes I'll re-run the failed job once. If it fails again I'll treat it as real and use the job's Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Coordinator: #292 is next in the landing order (#287 is skipped until it is reduced to docs-only and marked ready). One thing blocks it: there is no Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[agent] ready to land Head b2df60b already contains the base, f9cb7e1. The Poetry Remaining reds on b2df60b. Each one also fails on the base, and this PR doesn't touch the code involved:
Every other check on b2df60b is green: CI 182/186; the Pipenv, pnpm, npm, Go, PDM, Bun and Poetry workflows; and vlt apart from the four jobs above. There are no open review threads. The Cursor Bugbot finding was resolved in 7ce6840. Generated by Claude Code |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
a7b0d00
into
release/v5-prerelease
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01N9YdJqaGiT9Jf5LN1hDFhB
Brings in #292, which fixes the partial-stage repair bug and cuts redundant downloads. No conflicts; clippy and the policy, in-memory, parity, e2e policy, parser and help suites pass. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01BKsyzefGhAnPkYmXCwq3H3
#292 fixes created-file coverage for diff archives and has pypi refuse non-wheel references before downloading. Conflicts, resolved against this branch's removal of .socket/packages and the shared service policy: - fetch_stage: keep #292's per-file coverage check (a diff covers only files that exist before the patch), without the package-archive arm. - repair: take #292's download_pass helper and its created-file blob top-up; SourcePaths and PatchSources lose the packages path. - pypi: keep ServicePolicy; add #292's PYPI_NOT_A_WHEEL refusal (warns under auto, refuses under service) and its shared constant. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Brings in #292: vendored runs ask for every planned package's download reference in one request, and a patch that creates a file now fetches that file's blob even when its diff archive is cached. The only conflict was two new CHANGELOG entries in the same place; both are kept. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01AvGSu5jGg1z3f3sePc2eHC
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
2 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.