Repository navigation
Fix Deno global npm tools missed by -g (#444) - #1719
Conversation
Assisted-by: Claude Code:claude-opus-5-5
`deno install -g` tools load their npm dependencies from Deno's shared npm cache, $DENO_DIR/npm/<registry>/<name>/<version>/. Global mode only looked in npm, pnpm, yarn and bun node_modules roots, so scan -g reported success with no findings, apply -g and rollback -g said the package was not installed, and no --global-prefix could reach the copy Deno runs. Global mode now adds every $DENO_DIR/npm/<registry> dir as an npm root, and a --global-prefix naming one is read the same way. Each copy, including Deno's <version>_<n> peer-dependency copies, is matched by its package.json, so apply and rollback patch every copy. The JSR crawler no longer misreads such a prefix as JSR packages. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
A root was read as Deno's npm cache whenever its parent dir was named npm, so a project under ~/src/npm with a custom modules folder lost its node_modules walk, and the JSR crawler skipped JSR trees kept under such a dir. A root now also has to hold Deno's cached <name>/registry.json, and a relative --global-prefix is judged by its absolute path. Deno cache roots now come after the macOS fallback roots, so a copy another global root also holds is still reported there. A purl listed twice records each copy once, the capital-name fallback looks only at Deno's encoded _<...> dirs, and scan skips reading <version>_<n> copies whose <version> dir is already there. The CLI contract documents the new global root. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
changed the title
|
BugBot review Generated by Claude Code |
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
|
Labeled Ready for review by the burn-down agent.
Generated by Claude Code |
|
Labeled Ready for review by the burn-down agent.
Generated by Claude Code |
Final review briefWhat it does. Risk: low-medium. Additive and limited to global mode / a Look here
Verified
Changes I made: none. Open questions: none blocking. The PR is Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
|
[agent] The merge queue dropped this PR at 15:40 UTC, and the failure isn't from this change. In queue run 40348796795, the Generated by Claude Code |
|
CI janitor: removed this PR from the merge queue at 15:40 UTC because its merge_group run was hung. Generated by Claude Code |
|
[final reviewer] Re-added to the merge queue at 15:50 UTC, as the CI janitor asked. The 15:40 removal was a hung Generated by Claude Code |
Resolve the crawl_all_sync conflict with the relocated Yarn pnpm-linker store walk (#859): Deno npm cache roots keep their own layout reader, and node_modules roots keep the new store walk. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Deno stores a registry served below a path (Artifactory, CodeArtifact, GitLab: https://host/api/npm/) as nested dirs under $DENO_DIR/npm, one per path segment, e.g. npm/<host>/api/npm/<name>/<version>/. Only a dir directly under npm/ was recognized, so -g and a --global-prefix naming such a registry still reported its packages not installed. A registry dir now only needs an ancestor named npm plus Deno's registry.json marker, and -g walks down from $DENO_DIR/npm to every dir holding that marker. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
When a global tool's deno.lock already pins a tarball, Deno downloads only the tarball and writes no registry.json for the package. Such a registry dir was not recognized as Deno's cache, so apply -g left that copy unpatched while the tool kept running it. A registry dir now counts as Deno's cache when some package dir has no package.json of its own and holds either Deno's registry.json or a version dir with a package.json. A node_modules package dir always has its own package.json, so node_modules trees keep their usual walk. Checked with Deno 2.9.7: apply -g patches both the lock-installed copy and a path-registry copy, and the tool runs the patched code. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
One host can serve a registry at its root and another below a path, which Deno stores as <host>/<pkg>/ beside <host>/api/npm/<pkg>/. The -g walk stopped at the first dir holding packages, so the nested registry was never crawled. It now keeps walking that dir's children that are not Deno package or scope dirs. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
When a host serves a registry at its root and another below a path, a cached root package named like the path's first segment (api beside api/npm/) hid the nested registry from -g, because the walk skipped any child that looked like a package. A Deno package dir holds only version dirs, so the walk now continues into every non-version subdir of each child of a registry dir. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
The previous change walked only the grandchildren of a registry dir, so a registry one segment below a root registry on the same host (<host>/npm/<pkg>/) was never checked. A child that is not a Deno package dir is now walked like any other dir; only a child that is a package has its non-version subdirs walked instead. Refs #444 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
Final reviewer: re-added to the merge queue at 17:50 UTC on 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 40ef0c1. Configure here.
| // other subdirs are walked. | ||
| for child in children.into_iter().filter(|name| !name.starts_with('@')) { | ||
| let child = dir.join(child); | ||
| if is_deno_package_dir(&child) { |
There was a problem hiding this comment.
[agent] A one-segment path registry is still missed when the root registry on the same host has cached a package named like that segment. Deno lays out both https://host/ and https://host/npm/ under $DENO_DIR/npm/host/, so host/npm/ holds the root registry's package npm (host/npm/10.9.0/) and the nested registry's packages (host/npm/foo/1.0.0/). Here is_deno_package_dir(host/npm) is true, so only host/npm/foo is queued. holds_deno_packages(host/npm/foo) is false because its children are version dirs that have a package.json, so host/npm never becomes a root. -g scan and apply then leave every package from https://host/npm/ unpatched with no warning. global_mode_finds_one_segment_registry_under_root_registry doesn't cache a package npm at the host root, so it doesn't cover this.
Fix: when a child is a package dir, also check its non-version subdirs with holds_deno_packages (ignoring digit-named dirs). If they hold packages, push the child itself as a registry root, the same as the non-package branch does.
LLM Description written by Claude Code:claude-opus-5-5
Refs #444
What it does
deno install -gtools load their npm dependencies from Deno's shared npmcache,
$DENO_DIR/npm/<registry-host>/<name>/<version>/(scoped:@scope/name/<version>/, extra peer-dependency copies:<version>_<n>/).socket-patch's global mode never looked there.
scan -greported successwith zero findings,
apply -g/rollback -greportedpackage_not_installed, and no--global-prefixvalue could reach the copyDeno actually runs.
After this PR:
--globaladds every$DENO_DIR/npm/<registry-host>dir (all exceptjsr.io) as an npm root. The Deno roots come after the npm, pnpm, yarn,bun and macOS fallback roots.
DENO_DIRresolution is shared with theDeno crawler: the env var, else the per-platform cache dir.
https://host/api/npm/), which Deno stores as nested dirsnpm/<host>/api/npm/, are found too (verified with Deno 2.9.7).--global-prefixnaming such a registry dir is read in Deno's layoutinstead of as a
node_modulestree.find_by_purlsreturns every copy (<version>first, then each<version>_<n>), each confirmed by itspackage.json. A name withcapitals, which Deno stores under an encoded
_<...>dir, is matchedagainst those dirs by
package.jsonidentity. Scan reads one copy perpurl.
turned
@types/node/20.0.0into a boguspkg:jsr/purl).Scan, apply, rollback and vex all reach the npm crawler through
get_node_modules_paths,crawl_allandfind_by_purls, so all of themget the change.
Root cause
NpmCrawler::get_global_node_modules_pathsonly knewnode_modules-shapedglobal roots, and every root went through the
node_moduleswalker.Detecting the layout. A root counts as a Deno npm-cache root only if:
npm;node_modulesorjsr.io;package.jsonof its own (anode_modulespackage dir always has one), plus either Deno'sregistry.jsonor a version dir holding apackage.json. Deno skipsregistry.jsonwhen adeno.lockalready pins the tarball, so the version dirs alone have to count.-gwalks down from$DENO_DIR/npm(up to 8 levels) to every such dir, and below one keeps walking its children that aren't packages, plus the non-version subdirs of those that are (a Deno package dir holds only version dirs), so registries served below a path (https://host/api/npm/→npm/<host>/api/npm/, as Artifactory, CodeArtifact and GitLab serve) are found too. A relative prefix is judged by its absolute path. Both cases were checked with Deno 2.9.7:apply -gpatched a lock-installed npmjs copy and a path-registry copy, and the tool ran the patched code from each.Why
Refs, notFixesThe main defect (Deno global npm tools invisible to
-gand to everyprefix) is fixed. Two parts of #444 are left out:
bin/.<tool>/node_modulesdecoy still reportsappliedif a user points--global-prefixat it by hand.-gnow findsthe real cache copy, so the workaround that led there is no longer needed.
Refusing that prefix needs Deno-version-specific detection, best done as
its own change.
--global-prefix $DENO_DIRand--global-prefix $DENO_DIR/npm(the parentdirs) still aren't expanded; the registry dir itself works.
Related, unchanged: a Deno project without
nodeModulesDir(local mode)also runs npm packages from this cache, and local mode doesn't crawl it.
Test evidence
Red on
main, green here:crawler_npm_e2e::deno_npm_cache_prefix_crawls_name_version_layout(scanover the registry-dir prefix found nothing)
crawler_npm_e2e::deno_npm_cache_find_by_purls_returns_every_copy(apply,rollback and vex lookup: plain, scoped,
_1copy, identity mismatch,absent)
crawler_npm_e2e::global_mode_includes_deno_npm_cache(-gwithDENO_DIR)ecosystem_dispatch_e2e::rollback_dispatch_deno_npm_cache_globaland..._global_prefix(CLI rollback restores the cached copy; onmainboth reported the package not installed)
Other new tests:
deno_npm_cache_find_by_purls_rejects_traversal..._records_each_copy_once..._resolves_encoded_capital_namenon_deno_root_under_npm_dir_keeps_node_modules_walkdeno_crawler::tests::crawl_all_skips_deno_npm_cache_prefixReal Deno 2.9.7 repro from the issue (
deno install -gof a tool importingnpm:is-odd@3.0.1, local manifest whose patch appendsconsole.log('SP-PATCHED')):socket-patch apply -g --offline --json→success,appliedSP-PATCHEDrollback -g→rolledBack: 1On
mainthe sameapply -greportspackage_not_installed.Local checks:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test --workspace --all-features: 234 suites pass. 11 tests failidentically on
origin/mainin this sandbox: 9 write-failure tests thatrely on read-only dirs (the sandbox runs as root), and 2
e2e_bun_lockbtests that fetch a blob from the real patch API, which thesandbox proxy blocks. CI runs them unprivileged and with network.
npm/,pypi/,gem/) only dispatch to the binary and need nochange.
Per-issue checklist
scan -g,apply -g,rollback -gand--global-prefix: the tests abovehttps://claude.ai/code/session_01XrEsEQZ2tQzsX2cz5QKhX3
Note
Medium Risk
Changes global package discovery and path resolution for apply/rollback; mistakes could patch wrong dirs or miss installs, but behavior is gated on Deno-specific layout heuristics with broad test coverage.
Overview
Global
scan/apply/rollback/vexnow treat Deno's shared npm cache ($DENO_DIR/npm/<registry>/…) as npm roots alongside existingnode_modules-style globals.--globalwalks every registry dir under that tree (skippingjsr.io, including path-shaped registries like<host>/api/npm), and--global-prefixpointing at a registry dir is interpreted in Deno's<name>/<version>/layout instead of as anode_modulestree.The npm crawler adds layout detection (
registry.jsonor version dirs without a package-levelpackage.json), dedicated gather/resolve paths for peer-dependency copies (<version>_<n>), and package.json-based matching for encoded capitalized package names. The Deno JSR crawler returns no paths when the prefix is a Deno npm-cache registry, so scoped npm packages are not misreported aspkg:jsr/….CLI_CONTRACT.mddocuments the new global root behavior. Coverage is extended with crawler unit/e2e tests and CLI rollback dispatch tests for--globaland--global-prefix.Reviewed by Cursor Bugbot for commit 40ef0c1. Configure here.
Generated by Claude Code