fix(global virtual store): bridge the resolution paths a store slot lost

[zkochan]

Closes #10588.

Under pnpm's global virtual store a package's real directory sits outside the project, so the ancestor walk that used to satisfy its undeclared requires reaches nothing. The bridge in hoisted-resolution-bridge.ts restored one of the two directories that walk passed through, and only for require — not for tsc. This restores both, on both resolvers.

Both directories, not just the hoisted one

pnpm hoists only non-direct dependencies into node_modules/.pnpm/node_modules. A direct dependency of the root lives in the root's own node_modules and nowhere else, so the walk out of a store slot reached two directories with disjoint contents and the bridge covered one. Measured in this repo:

mocha          hoisted:no  root:yes
oxlint         hoisted:no  root:yes
cross-env      hoisted:no  root:yes
lodash.get     hoisted:yes root:no

hoistedResolutionDirs(root) now returns both, in walk order, and ensureHoistedDependencyResolution puts both on NODE_PATH — the hoisted one still first, so it keeps winning exactly as the walk had it. The ESM loader mirrors the entry list, so it follows automatically.

The type-resolution half

TypeScript reads neither NODE_PATH nor the ESM loader, so under this layout a .d.ts in a store slot resolves react to react/index.js, gets no typings, and every type derived from it degrades quietly. --traceResolution on this repo:

Resolving 'react' from STORE/@teambit/evangelist.input.checkbox.label/.../checkbox-label.d.ts
  → STORE/@/react/19.2.7/.../react/index.js          # untyped

Resolving 'react' from components/ui/.../code-compare-editor-settings.tsx
  → STORE/@types/react/19.2.14/.../index.d.ts

The two copies never unify, and it surfaces as Property 'children' does not exist on type 'IntrinsicAttributes & CheckboxLabelProps' — a message that names a prop rather than the cause. This repo already patches it by hand in tsconfig.json for its own tsc, for two packages, in one file.

TypeScript's only lever is paths, which is a redirect where NODE_PATH is a fallback, so a blanket mapping does real damage. The mapping is therefore kept to what the walk actually used to find. Full-repo tsc --noEmit, one rule at a time:

mapping	errors
every @types package from both directories	135 — overrides glob, react-router-dom and everything else that ships its own modern types
@types/x only where x itself ships no typings	9
+ @teambit/* from the hoisted directory and the root	5 — the hoisted directory's older transitive copies win
+ @teambit/* from the root only	0

So: a @types/x mapping only when x ships no typings of its own — that is the case the walk-up existed for, since a self-typed x stopped the walk before it ever reached @types/x — and @teambit/* from the root alone, because core aspects have to be the single copy from the running installation (the invariant DependencyLinker already maintains).

Applied in TypescriptMain.createCompiler after every transformer, so anything configured deliberately keeps its mapping and the bridge only fills in what would otherwise resolve to nothing. Two roots, each gated on the layout its own last install recorded: the workspace, whose components are what gets compiled, and the running installation, whose core aspects the compiled program reaches through the links DependencyLinker writes. A bvm installation is project-local and stays out entirely.

Verification

• bit test on both components: 119 passing, 16 of them new.
• New e2e case, global-virtual-store.e2e.ts → "building an aspect": builds an aspect in a workspace with enableGlobalVirtualStore: true. Passes on this branch; with the TypeScript half reverted and the aspect recompiled it fails with 47 error TS, so it fails for the reason it exists.
• e2e/harmony/extensions-config-diff.e2e.ts, which is what surfaced this: 7 passing, 0 failing, 0 TypeScript errors. On a global-virtual-store workspace before this change it failed in its before hook with Failed task 1: "teambit.compilation/compiler:TSCompiler" and 188 errors attributed to one component.
• npm run lint clean.

Not included

• The hand-written paths block in this repo's tsconfig.json stays. npm run lint runs tsc directly against that file and never goes through bit's compiler, so those pins are still load-bearing; retiring them is a separate change with its own verification.
• No e2e for the runtime half. hoistedResolutionDirs is unit-tested, but exercising a phantom require of a root-direct dependency from a store slot needs a published fixture package that under-declares one, and a test that would pass with or without the change is worse than none. The type-side e2e covers the shared directory logic end to end.

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix global virtual store resolution bridge for runtime and TypeScript

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Restore both hoisted and root node_modules resolution paths under pnpm global virtual store.
• Bridge TypeScript type resolution via targeted tsconfig.paths mappings.
• Add unit + e2e coverage for global-virtual-store aspect builds and path selection.

Diagram

graph TD
  WS["Workspace root"] --> HRD["hoistedResolutionDirs()"] --> RT["Runtime bridge"] --> NR["Node resolve"]
  SI["Bit install root"] --> HRD
  HRD --> TT["TS paths bridge"] --> TR["TypeScript resolve"]
  SS["Store-slot package"] --> NR
  SS --> TR

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Map all @types via tsconfig.paths under global virtual store

• ➕ Simpler implementation (no per-package typing checks).
• ➕ Maximizes chance of finding typings in any workspace directory.
• ➖ Breaks correctness: overrides packages that ship their own (often newer) types.
• ➖ Turns a former fallback into a redirect, causing widespread type regressions (as noted in PR description).

2. Require explicit dependency declarations / eliminate phantom dependencies

• ➕ Removes reliance on NODE_PATH/paths fallbacks entirely.
• ➕ More deterministic runtime and type resolution long-term.
• ➖ Not always feasible for core aspects and existing published packages.
• ➖ Large ecosystem change; high migration cost and long tail of breakage.

3. Custom TypeScript resolution host / plugin to emulate NODE_PATH fallback

• ➕ Closer semantic match to Node’s fallback behavior than paths redirects.
• ➕ Potentially avoids per-package @types mapping logic.
• ➖ Higher maintenance and integration risk; TS resolver APIs are complex and version-sensitive.
• ➖ Harder to reason about and to support across tools that invoke tsc differently.

Recommendation: Keep the PR’s approach: share a single source of truth for the two lost directories (hoisted + root node_modules) and apply it to both runtime (NODE_PATH/ESM loader) and TypeScript (selective compilerOptions.paths). The selective @types mapping (only when the runtime package lacks its own typings) and rooting @teambit/* to the workspace node_modules preserves correctness and avoids widespread type overrides that blanket paths mappings would cause.

Files changed (7) +331 / -25

Enhancement (1) +7 / -1

index.tsExport hoistedResolutionDirs and selfInstallationRoot from dependency-resolver +7/-1

Export hoistedResolutionDirs and selfInstallationRoot from dependency-resolver

• Extends the dependency-resolver barrel exports to include hoistedResolutionDirs() and selfInstallationRoot(), enabling TypeScript to reuse the same resolution-root logic.

scopes/dependencies/dependency-resolver/index.ts

Bug fix (3) +185 / -23

hoisted-resolution-bridge.tsRestore both resolution directories and export install-root detection +47/-21

Restore both resolution directories and export install-root detection

• Introduces hoistedResolutionDirs(root) and updates ensureHoistedDependencyResolution() to prepend both directories to NODE_PATH idempotently, preserving the original walk precedence. Also exports selfInstallationRoot() for reuse by the TypeScript bridge and updates documentation/comments to reflect runtime + type-resolution responsibilities.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts

global-virtual-store-type-paths.tsAdd TypeScript paths bridge for global virtual store type resolution +104/-0

Add TypeScript paths bridge for global virtual store type resolution

• Implements targeted compilerOptions.paths generation for global virtual store layouts by scanning @types entries across the same resolution directories used at runtime. Avoids overriding self-typed packages and forces @teambit/* to resolve only from the root node_modules to prevent older hoisted core-aspect copies from winning.

scopes/typescript/typescript/global-virtual-store-type-paths.ts

typescript.main.runtime.tsApply TypeScript type-resolution bridge after tsconfig transformations +34/-2

Apply TypeScript type-resolution bridge after tsconfig transformations

• Integrates the new type-path bridge into TypescriptMain.createCompiler() so it runs last and yields to explicitly configured tsconfig paths. Gates bridging per-root via isGlobalVirtualStoreLayout() and applies it to both the workspace root and the detected running installation root.

scopes/typescript/typescript/typescript.main.runtime.ts

Tests (3) +139 / -1

global-virtual-store.e2e.tsAdd e2e coverage for aspect builds under global virtual store +22/-0

Add e2e coverage for aspect builds under global virtual store

• Adds a new e2e scenario that builds/tag an aspect in a workspace with enableGlobalVirtualStore enabled. Asserts the build output contains no TypeScript errors, validating the type-resolution bridge end-to-end.

e2e/harmony/global-virtual-store.e2e.ts

hoisted-resolution-bridge.spec.tsUnit-test hoistedResolutionDirs directory selection and ordering +26/-1

Unit-test hoistedResolutionDirs directory selection and ordering

• Extends the existing bridge spec to validate that both the hoisted directory and the root node_modules are returned in the correct walk order, and that missing installs return an empty list.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts

global-virtual-store-type-paths.spec.tsAdd unit tests for TypeScript global virtual store path mapping rules +91/-0

Add unit tests for TypeScript global virtual store path mapping rules

• Adds focused tests for @types scope unmangling, selective mapping only when runtime packages lack typings, hoisted-vs-root precedence, and @teambit/* being pinned to the root node_modules only.

scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1) 📜 Skill insights (0)

1. Wrong typed-package detection ✓ Resolved 🐞 Bug ≡ Correctness

Description

shipsOwnTypes() only treats a package as self-typed when package.json has types/typings or
when a root-level index.d.ts exists, but it claims to detect an index.d.ts “beside the entry
point” and does not actually inspect the entry point path. This can misclassify typed packages as
untyped and incorrectly add a paths redirect to @types/*, overriding the package’s own typings
(TypeScript paths is a redirect, not a fallback).

Code

scopes/typescript/typescript/global-virtual-store-type-paths.ts[R55-58]

+    if (typeof manifest.types === 'string' || typeof manifest.typings === 'string') return true;
+    // the implicit form: no `types` field, but an index.d.ts beside the entry point
+    return fs.existsSync(path.join(packageDir, 'index.d.ts'));
+  }

Evidence

The code’s own comment says it’s checking for an index.d.ts beside the entry point, but the
implementation only checks /index.d.ts, which will miss non-root entry points; the tests also only
cover types field and root index.d.ts, reinforcing that other layouts aren’t handled.

scopes/typescript/typescript/global-virtual-store-type-paths.ts[41-60]
scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts[40-50]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`shipsOwnTypes()` is used to decide whether to create a TypeScript `compilerOptions.paths` redirect from `x` -> `node_modules/@types/x`. Its current heuristic only checks `package.json.types|typings` or `<pkg>/index.d.ts`, even though the comment says it should detect an `index.d.ts` beside the package entry point.
This can misclassify packages that ship declarations in common non-root layouts (e.g. `main: "dist/index.js"` with `dist/index.d.ts`, conditional `exports` with `types`, `typesVersions`, etc.), and then incorrectly redirect to `@types/*` which can override correct bundled typings.
## Issue Context
The bridge is intended to be conservative because `paths` is a redirect (it can override correct resolutions). Misclassification here is a correctness bug.
## Fix Focus Areas
- scopes/typescript/typescript/global-virtual-store-type-paths.ts[41-60]
- scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts[34-50]
## Implementation notes
- Extend detection beyond `types|typings` and root `index.d.ts`:
- If `package.json.main` is a string, check for a sibling declaration by replacing the extension (`.js/.cjs/.mjs`) with `.d.ts` and/or checking `path.join(packageDir, dirname(main), 'index.d.ts')`.
- Consider `typesVersions` presence as “ships types”.
- Consider `exports` objects containing `types` (including conditional exports) as “ships types”.
- Add a unit test for a package with `main: "dist/index.js"` and `dist/index.d.ts` but no `types` field: it should be treated as typed and therefore NOT mapped to `@types/*`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. ESM loader stale order ✓ Resolved 🐞 Bug ≡ Correctness

Description

ensureHoistedDependencyResolution() can reorder NODE_PATH to restore ancestor-walk precedence, but
registerEsmNodePathLoader() won’t re-register when only the order changes, so the ESM loader can
keep resolving using the previous precedence. This can make ESM resolution diverge from CommonJS
after a reorder within the same process and also keep a stale --import flag for child processes.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[R253-256]

+  const next = [...dirs, ...untouched].join(path.delimiter);
+  if (next !== existing) {
+    process.env.NODE_PATH = next;
  // `NODE_PATH` is read once when the module system initializes, so a later assignment only

Evidence

The PR now rebuilds NODE_PATH in canonical walk order, but the ESM loader registration
short-circuits when all current NODE_PATH entries are already in a Set, which ignores ordering.
Since the generated loader source captures dirs and iterates them sequentially, order-only changes
won’t be reflected without a re-registration.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[239-260]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[279-309]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[56-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureHoistedDependencyResolution()` can rebuild/reorder `NODE_PATH` (to restore “walk order”), but `registerEsmNodePathLoader()` only re-registers when the directory *set* grows. Because the ESM loader source inlines the ordered `dirs` list at registration time, an order-only change can leave ESM resolution using stale precedence (and `NODE_OPTIONS --import` can remain stale too).
## Issue Context
The bridge now intentionally reorders entries, so the ESM side needs to track ordered changes as well, not just membership changes.
## Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[239-309]
### Suggested implementation direction
- Track the last registered ordered list (e.g., `lastRegisteredNodePath = dirs.join(path.delimiter)`), not only a Set.
- Re-register (and replace `NODE_OPTIONS` flag) whenever the ordered list differs, even if all entries were previously registered.
- Keep the existing “skip on older Node” guard (`typeof nodeModule.register !== 'function'`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Type-paths contract mismatch 🐞 Bug ⚙ Maintainability

Description

globalVirtualStoreTypePaths() is documented as returning an empty mapping when the root is not on
the global virtual store, but the implementation only checks for the presence of node_modules
directories and can return mappings for non-GVS layouts too. The current caller does gate it via
isGlobalVirtualStoreLayout(), but the helper’s standalone contract is inaccurate and is easy to
misuse elsewhere.

Code

scopes/typescript/typescript/global-virtual-store-type-paths.ts[R121-124]

+ * `paths` entries that let a store slot resolve the types it used to reach by walking up out of
+ * `node_modules/.pnpm`. Empty for a root that is not on the global virtual store - callers gate on
+ * `isGlobalVirtualStoreLayout` - or one whose directories no longer exist.
+ *

Evidence

The JSDoc states the result is empty outside GVS, but the implementation has no layout check (it
only checks hoistedResolutionDirs(root)), so it can produce mappings whenever node_modules
exists. The only current caller shown does apply the layout gate, which is why this is an
API/contract mismatch rather than an immediate integration bug.

scopes/typescript/typescript/global-virtual-store-type-paths.ts[120-145]
scopes/typescript/typescript/typescript.main.runtime.ts[159-166]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`globalVirtualStoreTypePaths()` claims (via JSDoc) that it returns an empty mapping when the root is not using pnpm’s global virtual store, but the function does not check the layout at all—it only checks whether `hoistedResolutionDirs(root)` finds existing directories.
### Issue Context
Today, `TypescriptMain.bridgeTypeResolution()` correctly gates calls by `isGlobalVirtualStoreLayout(root)`, so there’s no demonstrated runtime bug in this PR’s integration path. However, the helper’s contract is misleading and invites incorrect future call sites.
### Fix Focus Areas
- scopes/typescript/typescript/global-virtual-store-type-paths.ts[120-145]
### Suggested fix
Choose one:
1) **Enforce the documented behavior**: add an `isGlobalVirtualStoreLayout(root)` check inside `globalVirtualStoreTypePaths()` (you’ll need to import it from dependency-resolver).
2) **Fix the documentation**: update the JSDoc to state that callers must gate with `isGlobalVirtualStoreLayout()` and that this function only derives mappings from the directories returned by `hoistedResolutionDirs()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Long lines break Prettier 📘 Rule violation ⚙ Maintainability

Description

New/modified lines exceed the configured Prettier printWidth and are not formatted as Prettier
would output. This will cause npm run prettier:check to fail and creates avoidable formatting
churn.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[228]

+  return [path.join(root, 'node_modules', '.pnpm', 'node_modules'), path.join(root, 'node_modules')].filter((dir) =>

Evidence

PR Compliance ID 3 requires changes to conform to Prettier formatting. The repo Prettier
configuration sets printWidth to 120, but the modified hoistedResolutionDirs() return statement
and the NODE_OPTIONS assignment are written as long single-line expressions that exceed this width
and would be reformatted by Prettier, causing prettier:check diffs.

CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting: CLAUDE.md: Code Must Conform to Prettier Formatting
.prettierrc[1-5]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[227-231]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[303-307]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Some newly added/modified lines are not formatted according to the repo’s Prettier config (`printWidth: 120`), and appear to exceed the configured line width.
## Issue Context
Compliance requires that code changes conform to Prettier formatting so `npm run prettier:check` passes.
## Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[227-231]
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[303-307]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View review recommended (6)

5. Installation fallback resolution 🐞 Bug ≡ Correctness

Description

hoistedResolutionDirs() now includes the root node_modules, so ensureSelfInstallationBridge() adds
the Bit installation’s root node_modules to NODE_PATH before the workspace bridge runs. Because
ensureHoistedDependencyResolution() preserves non-owned existing NODE_PATH entries, the
installation’s node_modules remains as a fallback for the workspace, which can mask missing
dependencies or resolve a different package copy than the original ancestor-walk would allow.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[R227-230]

+export function hoistedResolutionDirs(root: string): string[] {
+  return [path.join(root, 'node_modules', '.pnpm', 'node_modules'), path.join(root, 'node_modules')].filter((dir) =>
+    fs.existsSync(dir)
+  );

Evidence

The bridge now always includes /node_modules (in addition to the hoisted node_modules), and
bootstrap calls the self-installation bridge before the workspace bridge.
ensureHoistedDependencyResolution rebuilds NODE_PATH as [...dirs, ...untouched], where untouched
keeps any existing entries not matching the current root’s owned dirs, so installation entries
remain as fallback after bridging the workspace.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[210-260]
scopes/harmony/bit/bootstrap.ts[79-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureSelfInstallationBridge()` is invoked before the workspace bridge during CLI bootstrap, and `hoistedResolutionDirs()` now returns both the hoisted dir and the root `node_modules`. This causes the installation root `node_modules` to be placed on `NODE_PATH` globally, and because `ensureHoistedDependencyResolution()` keeps existing non-owned `NODE_PATH` entries, the installation’s `node_modules` remains as a fallback after the workspace’s entries.
This can allow workspace compilation/runtime to resolve undeclared or missing dependencies from the Bit installation instead of failing (or instead of following the original parent-walk behavior), and can select a different copy/version than intended.
## Issue Context
- Bootstrap bridges self-installation first, then workspace.
- The bridge explicitly preserves entries it does not consider “owned” by the current root.
## Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[210-260]
- scopes/harmony/bit/bootstrap.ts[79-105]
## Suggested fix direction
Implement explicit ownership tracking for bridge-added entries and avoid keeping prior bridge entries implicitly:
- Track bridge-inserted dirs in a module-level set (canonicalized), and when rebuilding `NODE_PATH`, filter out any previously-inserted bridge dirs (across roots) before appending untouched user-owned entries.
- Then explicitly re-add only the desired roots’ bridge dirs in a deterministic order (e.g., workspace first, then self-installation), rather than relying on “whatever was already in NODE_PATH”.
- If the installation root `node_modules` is not strictly required for the self-installation bridge, consider adding only the hoisted dir for the self-installation case to reduce unintended fallback surface area.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. NODE_PATH not normalized ✓ Resolved 🐞 Bug ☼ Reliability

Description

ensureHoistedDependencyResolution() treats NODE_PATH entries as owned only when they exactly
string-match the canonical dirs, so equivalent spellings (e.g. trailing slashes or different
normalization) will be kept as “untouched” while the canonical entry is also added. This can leave
duplicate bridge-owned directories in NODE_PATH and register redundant ESM loader search bases,
making resolution order/behavior harder to reason about.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[R233-236]

+  const untouched = (existing?.split(path.delimiter).filter(Boolean) ?? []).filter((dir) => !dirs.includes(dir));
+  // rebuilt rather than prepended, because order is resolution order and an entry already present
+  // is not necessarily in front of the one it has to beat: a bit that bridged the hoisted
+  // directory alone leaves it in `NODE_PATH` for its children, and adding the root's node_modules

Evidence

The bridge-owned directories are generated canonically via path.join(...), while existing
NODE_PATH entries are split and compared with exact string equality. registerEsmNodePathLoader then
mirrors NODE_PATH entries verbatim into the ESM loader registration set, so any duplicate spellings
persist into ESM resolution inputs.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[217-245]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[270-284]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureHoistedDependencyResolution()` removes/keeps existing `NODE_PATH` entries by exact string equality (`dirs.includes(dir)`), but the bridge-owned directories are generated canonically via `path.join(...)`. If `NODE_PATH` already contains an equivalent path with a different spelling (common examples: trailing path separator, different normalization), it is treated as foreign and retained, while the canonical bridge entry is also added. This results in duplicate effective search bases and can also cause redundant ESM loader registration inputs.
### Issue Context
This PR expands the bridge to add *two* directories (hoisted + root `node_modules`) and rebuilds `NODE_PATH` to enforce walk order. That makes exact-string matching more likely to retain an existing equivalent root `node_modules` entry (previously the bridge did not add root `node_modules` at all).
### Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[217-245]
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[265-294]
### Suggested fix
- Introduce a small path-normalization function for comparison (e.g., `path.resolve`, `path.normalize`, and trimming a trailing separator where safe).
- When computing `untouched`, compare normalized forms so that bridge-owned entries are removed even if their spelling differs.
- Optionally dedupe the final list by normalized value while preserving the original relative order for truly-foreign entries.
- Keep the *spelled* entries written back to `NODE_PATH` consistent/canonical to stabilize subsequent runs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Misses .d.mts typings ✓ Resolved 🐞 Bug ≡ Correctness

Description

declarationsBesideEntry() only checks for adjacent *.d.ts files, so shipsOwnTypes() can wrongly
treat a package as untyped when its declarations are *.d.mts/*.d.cts and then add a tsconfig "paths"
redirect to @types/* that overrides the package’s own typings. This repo already documents
dependencies shipping types as .d.mts, so this omission can affect real type-checking behavior under
the bridge.

Code

scopes/typescript/typescript/global-virtual-store-type-paths.ts[R65-68]

+  const withoutExtension = resolved.replace(/\.(js|cjs|mjs|jsx)$/, '');
+  return (
+    fs.existsSync(`${withoutExtension}.d.ts`) ||
+    fs.existsSync(path.join(resolved, 'index.d.ts')) ||

Evidence

The new type-bridge relies on declarationsBesideEntry() to infer whether a runtime package ships
types; it currently only probes .d.ts. The repository already notes real dependencies where
typings are .d.mts, demonstrating this inference can be incomplete and lead to incorrect
redirects.

scopes/typescript/typescript/global-virtual-store-type-paths.ts[62-71]
components/ui/diff-viewer/shiki-imports.ts[3-10]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`declarationsBesideEntry()` only considers `.d.ts` when determining whether a runtime package “ships its own types”. Packages that ship declarations as `.d.mts` / `.d.cts` can be misclassified as untyped, causing `globalVirtualStoreTypePaths()` to add a `paths` redirect to `@types/*` that can override the package’s real typings.
## Issue Context
The bridge is intentionally conservative because `paths` is a redirect (not a fallback). A false-negative in `shipsOwnTypes()` is high-impact because it can change which declarations TypeScript uses.
## Fix Focus Areas
- scopes/typescript/typescript/global-virtual-store-type-paths.ts[62-71]
- scopes/typescript/typescript/global-virtual-store-type-paths.spec.ts[40-87]
## Suggested fix
1. Extend `declarationsBesideEntry()` to also detect:
- adjacent `*.d.mts` / `*.d.cts` files when the entry is `.mjs` / `.cjs` (and consider `index.d.mts` / `index.d.cts` for directory entry points).
- (Optional hardening) if `main` is absent/empty, consider checking common entry candidates or the manifest’s `exports` target(s) for adjacent declaration files, not only `index.js`.
2. Add a regression test that creates a fake package with an entrypoint + adjacent `.d.mts` (or `.d.cts`) and asserts `globalVirtualStoreTypePaths()` does **not** create an `@types` mapping for it when `@types/<pkg>` exists.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Tests leak module paths ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new ensureHoistedDependencyResolution() unit tests restore NODE_PATH/NODE_OPTIONS but
don’t restore Node’s derived global module search paths or prevent the in-process ESM loader
registration performed by the function. This can pollute the process for subsequent tests
(order-dependent resolution / intermittent failures), especially on Node versions where
module.register exists.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts[R84-89]

+  afterEach(() => {
+    if (nodePath === undefined) delete process.env.NODE_PATH;
+    else process.env.NODE_PATH = nodePath;
+    if (nodeOptions === undefined) delete process.env.NODE_OPTIONS;
+    else process.env.NODE_OPTIONS = nodeOptions;
+    fs.removeSync(root);

Evidence

The tests invoke ensureHoistedDependencyResolution() but only restore environment variables;
however, the implementation also mutates Node’s internal module path state via _initPaths() and
may register an ESM loader with module.register(), both of which persist beyond the test unless
explicitly reset/avoided.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts[78-90]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[229-247]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[265-283]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new tests call `ensureHoistedDependencyResolution()`, which mutates process-global module resolution state via `module._initPaths()` and may register an ESM loader via `module.register()`. The test `afterEach()` only restores env vars, leaving module resolution state polluted for the rest of the test process.
## Issue Context
- `ensureHoistedDependencyResolution()` updates `process.env.NODE_PATH` and then calls `require('module')._initPaths()` (process-global side effect).
- It also calls `registerEsmNodePathLoader()`, which may call `node:module.register()` on supported Node versions (process-lifetime side effect).
- The new test suite restores `NODE_PATH`/`NODE_OPTIONS` but does not re-run `_initPaths()` after restoration and does not prevent/undo loader registration.
## Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.spec.ts[78-90]
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[229-247]
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[265-283]
## Suggested fix
1. In the spec `afterEach()`, after restoring `process.env.NODE_PATH`, call `(require('module') as any)._initPaths()` to recompute `Module.globalPaths` based on the restored env.
2. Prevent loader registration during these unit tests to avoid irreversible process-wide changes:
- Temporarily stub `require('module').register` (if present) to `undefined` or a no-op in `beforeEach()`, and restore it in `afterEach()`.
- Alternatively, run these tests in a subprocess / isolate with a separate Node process if stubbing is undesirable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

9. NODE_PATH order not enforced ✓ Resolved 🐞 Bug ≡ Correctness

Description

ensureHoistedDependencyResolution() only prepends missing entries; if both bridge directories are
already present in NODE_PATH but in the wrong order, it will not reorder them, allowing root
node_modules to shadow the hoisted directory. This breaks the intended “walk order” precedence and
can resolve a different package copy/version than the original ancestor walk.

Code

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[R233-236]

+  const known = new Set(existing?.split(path.delimiter).filter(Boolean) ?? []);
+  const added = dirs.filter((dir) => !known.has(dir));
+  if (added.length) {
+    // prepended in walk order, so the hoisted directory keeps winning over the root's own

Evidence

The module documents that the walk must see the hoisted directory first and that entries are
prepended to keep the hoisted directory winning. However, the implementation uses a Set to track
membership and only mutates NODE_PATH when it finds missing entries, so it cannot correct a
pre-existing reversed order.

scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[207-243]
scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[217-221]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ensureHoistedDependencyResolution()` treats `NODE_PATH` as idempotent based on *set membership* only. Since order is semantically significant for resolution, an existing `NODE_PATH` that already contains both bridge directories but with `root/node_modules` before `root/node_modules/.pnpm/node_modules` will not be corrected, and the wrong directory may win.
## Issue Context
`hoistedResolutionDirs(root)` returns directories in the intended walk order (hoisted first, then root `node_modules`). The updated implementation only updates `NODE_PATH` when it detects *new* paths to add, so it never fixes an incorrect existing ordering.
## Fix Focus Areas
- scopes/dependencies/dependency-resolver/hoisted-resolution-bridge.ts[217-244]
## Implementation notes
- Parse `existingParts = (process.env.NODE_PATH ?? '').split(path.delimiter).filter(Boolean)`.
- Compute `managed = hoistedResolutionDirs(workspaceRoot)`.
- Build `nextParts = [...managed, ...existingParts.filter(p => !managed.includes(p))]` (this both dedupes the managed entries and enforces their order at the front while preserving unrelated NODE_PATH entries afterward).
- If `nextParts.join(path.delimiter) !== process.env.NODE_PATH`, assign it and call `module._initPaths()`.
- Keep calling `registerEsmNodePathLoader()` after ensuring the env var, so ESM mirrors the corrected order too.
- Add/adjust a unit test that sets `NODE_PATH` with the two dirs reversed and asserts the function reorders them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

10. Sync FS scan per compiler 🐞 Bug ➹ Performance

Description

createCompiler() now calls bridgeTypeResolution() which may synchronously scan
node_modules/@types and read many package.json files to build paths mappings. Under global
virtual store layouts, repeated compiler construction can repeatedly do this synchronous work,
adding avoidable latency to compilation flows.

Code

scopes/typescript/typescript/typescript.main.runtime.ts[R141-145]

TypescriptAspect.id,
this.logger,
afterMutationWithoutTsconfig,
-      afterMutation.raw.tsconfig,
+      this.bridgeTypeResolution(afterMutation.raw.tsconfig),
tsModule as any

Evidence

Compiler creation now always calls the bridge hook, and the bridge implementation uses synchronous
directory iteration and synchronous manifest reads (readdirSync, readFileSync), with no caching,
so repeated compiler creation repeats filesystem work.

scopes/typescript/typescript/typescript.main.runtime.ts[130-174]
scopes/typescript/typescript/global-virtual-store-type-paths.ts[71-89]
scopes/typescript/typescript/global-virtual-store-type-paths.ts[46-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TypescriptMain.createCompiler()` calls `bridgeTypeResolution()` during compiler construction. When the workspace/installation is on a global virtual store layout, `bridgeTypeResolution()` calls `globalVirtualStoreTypePaths()`, which synchronously reads directories and parses manifests.
This is correct functionally, but doing it synchronously for each compiler creation can add noticeable overhead in large workspaces.
## Issue Context
The code explicitly avoided caching because installs can switch layouts mid-process; caching can still be done safely with invalidation based on install/layout markers.
## Fix Focus Areas
- scopes/typescript/typescript/typescript.main.runtime.ts[130-174]
- scopes/typescript/typescript/global-virtual-store-type-paths.ts[71-89]
- scopes/typescript/typescript/global-virtual-store-type-paths.ts[46-58]
## Implementation notes
- Cache results per `root` (workspace root and installation root) with a lightweight invalidation strategy, for example:
- Key by `root` + `.modules.yaml` mtime (and/or `node_modules/@types` mtime).
- Recompute when the mtime changes.
- Alternatively, compute once per process per root during the install flow when the layout is known, and reuse thereafter.
- Keep the current behavior for the “layout can change mid-process” case by invalidating on `.modules.yaml` changes rather than disabling caching entirely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 2f83431

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit d539648

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit fa00cbd

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit a9abe6a

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit f2450a0

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 261fb25

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit f5d7435

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 82540b4