Repository navigation
Fix npm-family restore ignoring project registry (#908, #521) - #918
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Hosted rollback and remove looked up a package's version document on the default registry (npmjs or SOCKET_NPM_REGISTRY) only. On a project that installs from a mirror whose tarball URLs are off the usual path, that broke the restored lock: - yarn berry wrote a bare npm: locator, so a cold-cache install asked the mirror for a path it never serves and failed with a 404 (#908). - vlt rebuilt slot [3] as <registry>/<name>/-/<leaf>-<ver>.tgz instead of the URL the registry advertises, which vlt ci can 404 on (#521). The restore now reads the document from the registry the project resolves the package against (.yarnrc.yml npmRegistryServer, the vlt node's registry) and vlt takes slot [3] from its dist.tarball. If that registry can't be read (for example it needs credentials), the old default-registry lookup is used and upstream_registry_fallback warns. Fixes #908 Fixes #521 Assisted-by: Claude Code:claude-opus-5-5
77ed2e7 to
21413f1
Compare
Keeps clippy's type_complexity lint quiet for the restore client's registry-keyed version-document cache. Assisted-by: Claude Code:claude-opus-5-5
A scoped package in a .yarnrc.yml with an npmScopes block may resolve against its scope's registry rather than npmRegistryServer, so berry restore keeps reading its document from the default registry, as before, instead of asking a registry that may not host it. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
main's test suite is red: the Gradle cache, jar and Maven sidecar code from #646 hashes inline, which the digest guard test from #865 forbids, so coverage and the macOS/Windows test jobs fail on every PR. This is the same change as #878, ported so this PR can go green; it no-ops once #878 lands. Assisted-by: Claude Code:claude-opus-5-5
|
[agent] Generated by Claude Code |
|
BugBot review Generated by Claude Code |
|
[agent] Blocked: I can't read the job log. The agent environment's network policy denies the Actions log host ( Needed: the name of the failing test (or the log tail from the Generated by Claude Code |
The npm dist cache is now keyed by registry base, and these two tests seed it under npm_registry_base(), which reads SOCKET_NPM_REGISTRY. Serial vlt/bun tests set that variable, so when one ran in parallel the lookup key no longer matched the seeded entry and the test fetched left-pad from the other test's mock server (404). That is the test (windows-latest) failure on 48798c4. Serializing them with the env-mutating tests closes the race. Co-Authored-By: Claude <noreply@anthropic.com>
|
[agent] Unblocked: the GitHub MCP can read the job log. Root cause: this PR keys the npm dist cache by registry base. The test seeds that cache under Generated by Claude Code |
|
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 9c52af9. Configure here.
|
Burn-down agent: labeled Ready for review at
Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #908
Fixes #521
Summary
Hosted
rollback/remove(and the hosted → vendored takeover) restorea yarn berry or vlt lock entry from the npm version document. That document
was always read from the default registry, never from the registry the
project installs from. On a mirror whose tarball URLs are off the usual
<registry>/<name>/-/<leaf>-<ver>.tgzpath, the restored lock then pointedyarn / vlt at a URL the mirror never serves, so cold-cache installs 404'd
while socket-patch reported success.
Root cause (shared)
upstream::npm::fetch_dists→UpstreamClient::npm_distonly ever askednpm_registry_base()(SOCKET_NPM_REGISTRYor registry.npmjs.org):::__archiveUrl=binding (#817 fix incomplete): the restore looks updist.tarballon npmjs, not the project'snpmRegistryServer, so cold installs 404 #908 (yarn berry):restore_berryreadnpmRegistryServeronly forthe "is this URL conventional?" check. The
dist.tarballit checked wasnpmjs's, which is conventional, so it wrote a bare
name@npm:<v>locator instead of the mirror's
::__archiveUrl=binding./<name>/-/<leaf>-<ver>.tgzURL instead of the registry's dist.tarball, so the next coldvlt ci404s #521 (vlt): because the document didn't come from the node'sregistry, the vlt restore couldn't use its
dist.tarball, so itsynthesized the conventional URL for slot [3].
Fix
UpstreamClient::npm_dist_on(base, name, version)reads the documentfrom a given registry. The cache is keyed per registry, and
npm_distis
npm_dist_on(npm_registry_base(), …).fetch_dists_on(wanted, registry, …)reads each document from theregistry the project resolves the package against. npmjs, the
registry.yarnpkg.com alias and
SOCKET_NPM_REGISTRYcount as thedefault, so behavior there is unchanged. If the project's registry can't
be read (e.g. a private mirror that wants credentials the restore doesn't
send), it falls back to the default registry's document, as before, and
warns with the new additive code
upstream_registry_fallback..yarnrc.ymlnpmRegistryServer. vlt passes the node'sregistry_baseand writes slot [3] from that registry'sdist.tarball.The lock's slot-[3] presence convention is unchanged.
fetch_dists(package-lock, pnpm, bun, classic) keeps the defaultlookup.
Test evidence
Each regression test fails on
main(9c43dfc) and passes on this branch:in_process_redirect::yarn_berry_rollback_reads_the_tarball_from_the_project_registry(mirror vianpmRegistryServer, CDNdist.tarball, default registry serving conventional URLs; expects the::__archiveUrl=binding back byte for byte)upstream::vlt::tests::restore_takes_slot3_from_the_node_registrys_dist_tarball(node registry advertises/_cdn/files/…; expects that URL and the node registry's integrity)/mirror/left-pad/-/left-pad-1.3.0.tgz, default registry's integrity)upstream::vlt::tests::unreadable_node_registry_falls_back_with_a_warningupstream::npm::tests::berry_reads_the_project_registry_except_for_npm_scopes,npmjs_and_its_yarnpkg_alias_are_the_default_registryLocal runs:
cargo test -p socket-patch-core --all-features --lib upstream::: 87 passedcargo test -p socket-patch-cli --all-features --test in_process_redirect yarn_berry: 8 passedcargo clippy --workspace --all-features -- -D warnings: cleanrustfmt --checkon the touched files: clean.mainitself isn'tcargo fmt-clean (about 120 files) and CI has no fmt gate, so this PRformats only what it touches.
cargo test -p socket-patch-core --all-features --lib: 5249 passed and 5failed. None of the failures are in this PR's code, and all 5 also fail
on
main:utils::digest::tests::production_digests_go_through_the_helpers(Gradle/JVM files that hash inline; Route Gradle digests through utils::digest #878 fixes it), plus four
permission-based tests (
copy_treesymlinked root,vlt_healunremovablelock, poetry/requirements write failure) that can't fail a write when
the sandbox runs as root.
cargo test --workspace --all-featuresbuild used up thissession's disk allowance (ENOSPC while linking), so CI runs the remaining
CLI suites.
Ported main fix
main(9c43dfc) is red oncoverage/test (macos|windows)because ofutils::digest::tests::production_digests_go_through_the_helpers. This PRcarries #878's three-file fix (48798c4), which becomes a no-op once #878
merges.
Follow-ups (not in this PR)
.yarnrc.ymlnpmScopesblock keeps thedefault-registry lookup. There's no nested YAML reader in core yet.
dist.tarball. They aren't reported broken, but they have the sameshape if a project's
.npmrcregistry=mirror uses off-path URLs.🤖 Generated with Claude Code
Note
Medium Risk
Changes hosted unwind registry I/O and lock rewriting for Yarn Berry and vlt; incorrect mirror handling could still affect installs when fallback triggers, but default-registry projects are unchanged and failures remain explicit via warnings or refusals.
Overview
Hosted rollback/remove (and hosted→vendored takeover) used to resolve npm-family lock entries only from the default registry (
SOCKET_NPM_REGISTRY/ npmjs). That broke projects on mirrors whosedist.tarballURLs differ from the conventional path: Yarn Berry lost::__archiveUrl=bindings (#908) and vlt slot [3] was synthesized instead of using the mirror’s URL (#521).The restore path now fetches each package’s version document from the registry the project resolves against—Yarn Berry via
.yarnrc.ymlnpmRegistryServer(with a scoped-package caveat whennpmScopesis present), vlt via each node’s registry base and that document’sdist.tarballfor slot [3].UpstreamClient::npm_dist_onand a per-registry cache back this; package-lock/pnpm/bun/classic still use the default lookup viafetch_dists. If the project registry can’t be read, behavior falls back to the default document and emits the additive warningupstream_registry_fallback.CLI_CONTRACT.md documents the lookup rules and the new warning. An integration test covers Berry rollback with a mirror; unit tests cover vlt slot [3], fallback, and registry helpers. Touched Gradle/JVM/Maven code paths now use shared
utils::digesthelpers (aligned with #878).Reviewed by Cursor Bugbot for commit 9c52af9. Configure here.
Generated by Claude Code