[PR #1762] [MERGED] fix(javascript): honor package exports before sibling files #4802

Closed
opened 2026-10-31 00:55:14 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1762
Author: @shoyann
Created: 10/11/2026
Status: ✅ Merged
Merged: 10/11/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/package-exports-before-sibling-files


📝 Commits (4)

  • 1879b07 fix(javascript): honor package exports before sibling files
  • 949bce3 fix(javascript): preserve CommonJS lookup for percent package names
  • 08a1402 fix(javascript): align package metadata and self resolution
  • 2168576 fix(javascript): reject percent signs in ESM package names

📊 Changes

3 files changed (+755 additions, -101 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+109 -19)
📝 tests/acceptance/applications/packageSubpathsCli.test.ts (+149 -70)
📝 tests/boundary/javascript/packageConditionalExports.test.ts (+497 -12)

📄 Description

Summary

Fixes #1761.

JavaScript analysis can resolve require("fixture") or import ... from "fixture" to node_modules/fixture.js even when fixture/package.json exports a different file or rejects the request. Consult package exports at the appropriate lookup stage while preserving Node's root metadata semantics, CommonJS legacy lookup and package self-reference precedence.

Problem and expected behavior

With both node_modules/fixture.js and a package declaring "exports": "./actual.cjs", Node selects node_modules/fixture/actual.cjs; REA previously reported the sibling as resolved. Blocked, missing, and invalid exports also incorrectly produced successful sibling edges.

The Node resolution algorithm checks eligible package exports before file and directory fallback. Absent/null exports and ignored boolean/numeric root values retain CommonJS file-first behavior. Ordinary CommonJS dependency lookup excludes literal % from exports package names, so an outside require("fi%xture") selects node_modules/fi%xture.js. A matching self-reference from inside that named package instead uses its exports before dependency lookup, even when the name contains %.

Change and scope

Normalize package metadata once when reading it: retain string/object root exports and leave nested target values intact for validation. Read only same-container metadata. Before CommonJS dependency lookup, check the source's nearest package scope for an exact-name or subpath self-reference; stop at the first package metadata or a node_modules boundary. Self-reference exports share the existing target selection and refusal handling.

For ordinary dependency lookup, apply Node's CommonJS exports-name eligibility before metadata can preempt a sibling file; compare the matched name with the parsed package name to handle scoped names correctly. Ineligible names retain legacy file/directory lookup, including main fallback and subpaths whose parent package metadata is invalid. Explicit ESM imports reject percent signs anywhere in the package name before metadata or sibling lookup. Percent-containing subpaths of eligible packages still use exports. Preserve exact target selection, refusal outcomes, directory lookup, and source provenance. ESM self-resolution is outside this change.

Contract and boundary impact

  • Semantic owner and earliest changed stage: application-layer JavaScript artifact path resolution, before graph construction.
  • CLI and MCP/tool-catalog contract: no schema or tool changes; existing analysis results name the correct target or unresolved outcome.
  • Provider, bridge, target-format, or platform compatibility: static JavaScript artifact inventories; no provider or bridge changes.
  • Evidence, artifact, provenance, or reconstruction contract: corrected module edges retain the caller's specifier, source digest, and source range.
  • Process execution, authorization, cleanup, or containment impact: production remains an in-memory inventory lookup; same-container isolation remains enforced. Only trusted test fixtures execute through the Node oracle.
  • Generated metadata, package, or installation impact: none.

Evidence and regression coverage

  • Added real Node import/require comparisons for selected conditional exports, blocked roots, missing targets, and invalid targets in the presence of sibling files.
  • Preserved CommonJS absent/null exports behavior and foreign-container metadata isolation.
  • Added real Node comparisons for percent-containing unscoped names, scoped names and scope names, legacy main fallback, subpaths with malformed parent metadata, and percent subpaths of eligible packages.
  • Added Node comparisons for ignored boolean/numeric root exports, invalid nested scalar targets, CommonJS self roots/subpaths, blocked self exports, nested dependency precedence and nearest-scope boundaries.
  • Added Node comparisons rejecting percent-containing ESM package names, including scoped names, while retaining percent subpaths of valid packages. The public CLI/MCP journey verifies the rejected ESM edge and its source identity alongside CommonJS behavior.
  • Extended the compiled CLI and stdio MCP journey with scoped, blocked, scalar-root and percent-containing packages plus an internal self-reference. Source digest and source-range assertions distinguish the outside reference from the package's own caller.
  • All 84 focused tests pass, including regression cases confirmed to fail before the corresponding fix.
  • Representative results: a scoped package resolves to node_modules/@scope/selected/actual.cjs; a blocked root returns resolved_path: null, resolution_status: "external"; an outside require("fi%xture") resolves to node_modules/fi%xture.js, while the package's own reference selects node_modules/fi%xture/actual.cjs.
  • Remaining proof gaps: native Windows was not run. Complete Node module-loader compatibility is not claimed.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts — 84 passed.
  • npm run test:local -- src/application/javascript/*.test.ts — 17 files, 274 passed.
  • npm run check — typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards passed; lint reported 728 warnings and zero errors.
  • Pre-commit formatting/lint and pre-push npm run check:fast — passed.
  • git diff --check and independent correctness/security review — passed.

Executed on Ubuntu/WSL x64 with pinned Node 24.18.0 and npm 11.16.0.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: Linux Node oracle, public CLI, and stdio MCP ran. This static resolver change does not use a native analysis provider or browser.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new execution or I/O in production; container checks and input/evidence identity are preserved.

Review checklist

  • This PR addresses a concrete problem.
  • The PR has one focused outcome and a conventional title.
  • Related issue is linked.
  • Tests cover changed observable behavior and meaningful refusal paths.
  • No owning docs, contract, or generated metadata changes are required.
  • User-visible CLI/MCP changes include representative output.
  • Final diff checked for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1762 **Author:** [@shoyann](https://github.com/shoyann) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/package-exports-before-sibling-files` --- ### 📝 Commits (4) - [`1879b07`](https://github.com/morluto/rea/commit/1879b07a0baf62b6fcf745bbfb705ab6d5e0ba6d) fix(javascript): honor package exports before sibling files - [`949bce3`](https://github.com/morluto/rea/commit/949bce39ea3e779615a60d3c7198e3765ca773d7) fix(javascript): preserve CommonJS lookup for percent package names - [`08a1402`](https://github.com/morluto/rea/commit/08a140201d65d0717a0778c7ff506eef5840a6eb) fix(javascript): align package metadata and self resolution - [`2168576`](https://github.com/morluto/rea/commit/21685762c03c5d174aa07d9412d45244b42bf866) fix(javascript): reject percent signs in ESM package names ### 📊 Changes **3 files changed** (+755 additions, -101 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+109 -19) 📝 `tests/acceptance/applications/packageSubpathsCli.test.ts` (+149 -70) 📝 `tests/boundary/javascript/packageConditionalExports.test.ts` (+497 -12) </details> ### 📄 Description ## Summary Fixes #1761. JavaScript analysis can resolve `require("fixture")` or `import ... from "fixture"` to `node_modules/fixture.js` even when `fixture/package.json` exports a different file or rejects the request. Consult package exports at the appropriate lookup stage while preserving Node's root metadata semantics, CommonJS legacy lookup and package self-reference precedence. ## Problem and expected behavior With both `node_modules/fixture.js` and a package declaring `"exports": "./actual.cjs"`, Node selects `node_modules/fixture/actual.cjs`; REA previously reported the sibling as resolved. Blocked, missing, and invalid exports also incorrectly produced successful sibling edges. The [Node resolution algorithm](https://nodejs.org/download/release/v24.18.0/docs/api/modules.html#all-together) checks eligible package exports before file and directory fallback. Absent/null exports and ignored boolean/numeric root values retain CommonJS file-first behavior. Ordinary CommonJS dependency lookup excludes literal `%` from exports package names, so an outside `require("fi%xture")` selects `node_modules/fi%xture.js`. A matching self-reference from inside that named package instead uses its exports before dependency lookup, even when the name contains `%`. ## Change and scope Normalize package metadata once when reading it: retain string/object root exports and leave nested target values intact for validation. Read only same-container metadata. Before CommonJS dependency lookup, check the source's nearest package scope for an exact-name or subpath self-reference; stop at the first package metadata or a `node_modules` boundary. Self-reference exports share the existing target selection and refusal handling. For ordinary dependency lookup, apply Node's CommonJS exports-name eligibility before metadata can preempt a sibling file; compare the matched name with the parsed package name to handle scoped names correctly. Ineligible names retain legacy file/directory lookup, including `main` fallback and subpaths whose parent package metadata is invalid. Explicit ESM imports reject percent signs anywhere in the package name before metadata or sibling lookup. Percent-containing subpaths of eligible packages still use exports. Preserve exact target selection, refusal outcomes, directory lookup, and source provenance. ESM self-resolution is outside this change. ## Contract and boundary impact - Semantic owner and earliest changed stage: application-layer JavaScript artifact path resolution, before graph construction. - CLI and MCP/tool-catalog contract: no schema or tool changes; existing analysis results name the correct target or unresolved outcome. - Provider, bridge, target-format, or platform compatibility: static JavaScript artifact inventories; no provider or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: corrected module edges retain the caller's specifier, source digest, and source range. - Process execution, authorization, cleanup, or containment impact: production remains an in-memory inventory lookup; same-container isolation remains enforced. Only trusted test fixtures execute through the Node oracle. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Added real Node import/require comparisons for selected conditional exports, blocked roots, missing targets, and invalid targets in the presence of sibling files. - Preserved CommonJS absent/null exports behavior and foreign-container metadata isolation. - Added real Node comparisons for percent-containing unscoped names, scoped names and scope names, legacy `main` fallback, subpaths with malformed parent metadata, and percent subpaths of eligible packages. - Added Node comparisons for ignored boolean/numeric root exports, invalid nested scalar targets, CommonJS self roots/subpaths, blocked self exports, nested dependency precedence and nearest-scope boundaries. - Added Node comparisons rejecting percent-containing ESM package names, including scoped names, while retaining percent subpaths of valid packages. The public CLI/MCP journey verifies the rejected ESM edge and its source identity alongside CommonJS behavior. - Extended the compiled CLI and stdio MCP journey with scoped, blocked, scalar-root and percent-containing packages plus an internal self-reference. Source digest and source-range assertions distinguish the outside reference from the package's own caller. - All 84 focused tests pass, including regression cases confirmed to fail before the corresponding fix. - Representative results: a scoped package resolves to `node_modules/@scope/selected/actual.cjs`; a blocked root returns `resolved_path: null`, `resolution_status: "external"`; an outside `require("fi%xture")` resolves to `node_modules/fi%xture.js`, while the package's own reference selects `node_modules/fi%xture/actual.cjs`. - Remaining proof gaps: native Windows was not run. Complete Node module-loader compatibility is not claimed. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts` — 84 passed. - `npm run test:local -- src/application/javascript/*.test.ts` — 17 files, 274 passed. - `npm run check` — typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards passed; lint reported 728 warnings and zero errors. - Pre-commit formatting/lint and pre-push `npm run check:fast` — passed. - `git diff --check` and independent correctness/security review — passed. Executed on Ubuntu/WSL x64 with pinned Node 24.18.0 and npm 11.16.0. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux Node oracle, public CLI, and stdio MCP ran. This static resolver change does not use a native analysis provider or browser. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new execution or I/O in production; container checks and input/evidence identity are preserved. ## Review checklist - [x] This PR addresses a concrete problem. - [x] The PR has one focused outcome and a conventional title. - [x] Related issue is linked. - [x] Tests cover changed observable behavior and meaningful refusal paths. - [x] No owning docs, contract, or generated metadata changes are required. - [x] User-visible CLI/MCP changes include representative output. - [x] Final diff checked for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 00:55:14 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#4802
No description provided.