Skip to content

Fix Deno -g missing a one-segment path registry (#1784) - #1785

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-deno-nested-registry-package-dir
Oct 28, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-deno-nested-registry-package-dir

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1784

Summary

When one host serves an npm registry at its root (https://host/) and another one path segment below it (https://host/npm/), and the root registry has cached a package named like that segment (npm), Deno puts both in $DENO_DIR/npm/host/npm/. socket-patch -g scan and apply then silently skipped every package of the nested registry. They now find both registries.

Root cause

deno_npm_registry_dirs (crates/socket-patch-core/src/crawlers/npm_crawler.rs) treated a registry child that is a package dir (host/npm, with version dir 10.9.0/) as a package only, and queued just its non-version subdirs (host/npm/foo). foo holds version dirs, not packages, so host/npm never became a root. The two-segment case (host/api/npm/) already worked because the extra level made host/api/npm a root.

Fix

  • A package-dir child whose other (non-version) subdirs hold packages is also a registry root.
  • Such a registry skips the outer package's version dirs, both in the walk (so a package's bundled node_modules/fixtures are not walked as registries) and in the registry crawl (so a nested dist/package.json is not reported as a package).
  • One helper, is_deno_version_dir (digit-named with a package.json), now decides "version dir" in the walk, the package-dir check and the crawl, so a package named from a digit (7zip-bin) is handled the same everywhere and only digit-named dirs are stat'ed.

Wrappers under npm/, pypi/, gem/ only dispatch to the binary; no parallel change needed.

Tests

Issue Regression test
#1784 crawler_npm_e2e::global_mode_finds_one_segment_registry_behind_same_named_package (plain foo and scoped @scope/foo; roots, crawl_all, find_by_purls)
  • Red before the fix (commit f573480): left: [".../registry.example.com"] right: [".../registry.example.com", ".../registry.example.com/npm"].
  • Green after: cargo test -p socket-patch-core --all-features --test crawler_npm_e2e → 121 passed.
  • Each version-dir skip is covered: removing either one makes the new test fail (checked locally).
  • cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings pass.
  • cargo test --workspace --all-features --no-fail-fast: everything passes except tests unrelated to this diff that fail in this sandbox only: 9 permission-injection tests (the sandbox runs as uid 0, so chmod 0555 does not block writes) and 2 e2e_bun_lockb tests (one needs network blob download; one is Nightly e2e_bun_lockb red: offline takeover test predates #1648 #1770, also noted on Fix interrupted vendor eject unpatching npm (#1782) #1783).

Code review

/code-review high ran on the diff. Fixed: redundant/extra stats, inconsistent version-dir tests, uncovered guards, stale doc. Not fixed: the DenoDirGuard test helper is copy-pasted across 7 Deno tests in crawler_npm_e2e.rs. That duplication predates this PR, and folding it would touch unrelated tests.

https://claude.ai/code/session_011f9aeSf1TwmHuVC3BMMvwo


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A registry served one path segment below a root registry on the same
host is missed by -g when the root registry has also cached a package
named like that segment. Add a regression test for #1784, both for
plain and scoped packages in the nested registry.

Assisted-by: Claude Code:claude-opus-5-5
When one host serves an npm registry at its root and another one path
segment below it, and the root registry has cached a package named
like that segment, -g scan and apply silently skipped every package of
the nested registry. The package dir is now also taken as a registry
root when its other subdirs hold packages, and that registry's crawl
skips the version dirs of the outer package.

Fixes #1784

Assisted-by: Claude Code:claude-opus-5-5
Review follow-up for #1784. The registry walk, the package-dir check
and the registry crawl now use one test for a package's version dir
(named from a digit, with a package.json), so a package named from a
digit is treated the same everywhere and only digit-named dirs are
stat'ed. The walk reuses the package dir's listing instead of reading
it twice. The regression test now stages a nested manifest and a
registry-shaped fixture inside the outer package, so both version-dir
skips are exercised.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 28, 2026 15:12
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor

cursor Bot commented Oct 28, 2026

Copy link
Copy Markdown

Bugbot is paused — your team has an unpaid invoice

Bugbot is paused because your team has an unpaid invoice.

Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically.

(requestId: 7ccd74d1-2130-408b-940c-484479c481c5)

@cursor

cursor Bot commented Oct 28, 2026

Copy link
Copy Markdown

Bugbot is paused — your team has an unpaid invoice

Bugbot is paused because your team has an unpaid invoice.

Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically.

(requestId: d1ff444b-0ce5-4c46-9bfd-46b0f5873a9d)

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

1 similar comment
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 a1d89fd. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 28, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Ready for review at a1d89fd (burn-down agent).

  • CI: 82 check runs: 32 success, 50 skipped, all green (Cursor Bugbot included).
  • Bugbot reviewed a1d89fd: no issues found. No unresolved review threads.
  • Slack announcement not sent this run (Slack posting unavailable); the next run retries.

Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: In the Deno npm-cache registry walk, a registry child that is a package dir (for example host/npm) now also counts as a registry root when its non-version subdirs hold Deno packages. Those come from a nested https://host/npm/ registry, and deno -g used to miss them (#1784). The outer package's version dirs (digit-named with a package.json) are skipped both in the walk and in package listing, so bundled node_modules, fixtures and dist/package.json are not reported.

Risk: low. Only Deno-cache roots are touched. In Deno's real layout a package dir holds only version dirs plus registry.json, so the new branch fires only when a nested registry is actually present. Recursion stays capped at depth 8, and symlinked dirs are not descended.

Look here

Verified

  • Traced for false or duplicate packages:
    • The root registry still reports npm@10.9.0 once.
    • The nested root reports only its own packages.
    • Package names that start with a digit are not mistaken for version dirs.
  • cargo test -p socket-patch-core --test crawler_npm_e2e: 121 pass. --lib npm_crawler: 105 pass.
  • With the old npm_crawler.rs, the new test fails.
  • Mutation-checked both version-dir skips: removing either one makes the test fail.
  • cargo clippy -p socket-patch-core -D warnings: clean.
  • CI on the head: skipped:50 success:32. Bugbot: success. CHANGELOG.md is untouched. The description matches the code.

Changes I made: none.

Open questions (non-blocking nits)

  • crawler_npm_e2e.rs:4782: &[foo_purl.clone()] trips clippy::cloned_ref_to_slice_refs, but only under --all-targets, which CI doesn't run. The fix is std::slice::from_ref(&foo_purl).
  • A digit-named dir with no package.json (for example a partial extract) is now walked as an ordinary dir instead of dropped by its name. The depth cap bounds it.

Auto-merge (squash) is armed, so approving sends this PR straight to the merge queue.


Generated by Claude Code

Merged via the queue into main with commit 7bb0d76 Oct 28, 2026
82 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-deno-nested-registry-package-dir branch October 28, 2026 21:15
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.

Deno -g misses a one-segment path registry behind a same-named cached package

3 participants