[PR #1361] [MERGED] fix(javascript): honor module-sync package exports #2827

Closed
opened 2026-10-17 21:10:44 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1361
Author: @akshitkhokhani
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @morluto

Base: main ← Head: codex/fix-module-sync-exports


📝 Commits (1)

  • d42085f fix(javascript): honor module-sync package exports

📊 Changes

4 files changed (+58 additions, -2 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+2 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+3 -1)
📝 tests/boundary/filesystem/javascriptPackageExportTargets.test.ts (+21 -0)
📝 tests/boundary/filesystem/javascriptPackagePrecedence.test.ts (+32 -1)

📄 Description

Summary

Honor Node's module-sync package export condition during static JavaScript application analysis. A package with exports: { "module-sync": "./sync.mjs", "default": "./fallback.cjs" } currently yields a confidently resolved fallback.cjs relationship even though the supported Node runtime selects sync.mjs.

No linked issue: this small bug was discovered through an owned fixture and reproduced against the compiled CLI and real Node loaders. Searches for module-sync found no issue/PR. Related #591, #606, #1181 and #1333 handle order, fallback, exact targets and invalid configurations; this adds the missing active condition without replacing those rules.

Problem and expected behavior

Node's conditional exports contract activates module-sync for imports and requires. REA's packageExportConditions omits it. This also incorrectly bypasses explicit null or invalid targets under that condition and selects a later default.

Change and scope

Add module-sync to the existing active condition set. Preserve declared-order traversal, nested/array selection, exact-file lookup, admission rules and refusal diagnostics. Extend the existing real-Node/filesystem differential tests and document the condition. No dependencies or public API added.

Contract and boundary impact

  • Semantic owner and earliest changed stage: application package export-condition selection, before artifact target resolution.
  • CLI and MCP/tool-catalog contract: existing schemas and inventory unchanged; shared static analysis produces corrected relationships.
  • Provider, bridge, target-format, or platform compatibility: provider-independent JavaScript directory analysis; verified locally on macOS arm64 with pinned Node 24.18.0 and supported Node 22.23.0.
  • Evidence, artifact, provenance, or reconstruction contract: preserve original sources, paths, digests and existing resolution statuses; correct the selected target or refusal.
  • Process execution, authorization, cleanup, or containment impact: no production execution or authority changes; tests use owned inert packages and repository temporary-directory cleanup.
  • Generated metadata, package, or installation impact: none; authored documentation updated, generated outputs excluded.

Evidence and regression coverage

  • Base: 9c4a68bc19a0f7d39557e3cc039f5877f6550f63.
  • New regressions against unchanged production code: 6 failures / 91 passes across the two affected boundary suites. Failures were wrong leaf selection for import/require and false fallback after invalid/null active targets.
  • After the fix: 234 passes across five relevant suites on Node 24.18.0; the two affected suites additionally pass 97 cases on Node 22.23.0.
  • Real Node subprocesses verify selected paths, loaded owned values, refusal codes, nested conditions, earlier import/default priority and unmatched nested fallthrough. No module mocks.
  • Built public CLI checks: five fixture configurations through both analyze and analyze-javascript-application, 10 successful invocations. Stdout parsed as JSON, exit status was 0, and both import and require relationships matched native resolution. Static analysis succeeds while retaining unresolved/rejected relationships for refused dependencies.
  • Representative corrected relationship: {"module_link_kind":"require","specifier":"fixture","resolved_path":"node_modules/fixture/sync.mjs","resolution_status":"resolved"}; base selected node_modules/fixture/fallback.cjs.
  • Remaining proof gaps: no full local deterministic suite or real binary/browser providers; static target resolution does not establish runtime loadability of arbitrary modules.

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

Pinned Node 24.18.0 / npm 11.16.0, locally on macOS arm64:

  • npm ci — passed; 0 audit vulnerabilities.
  • npm run build:cached — passed.
  • npm run test:focused -- tests/boundary/filesystem/javascriptPackagePrecedence.test.ts tests/boundary/filesystem/javascriptPackageExportTargets.test.ts tests/boundary/javascript/packageConditionalExports.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts — 234 passed, no skips.
  • npm run check — all 6 tasks passed; lint reported warnings, 0 errors.
  • Pre-push npm run check:fast — both typecheck/lint tasks passed; normal commit formatting/lint hooks passed.
  • npm run check:ci — all 8 static tasks passed.
  • npm run docs:check — all 6 tasks passed.
  • npm run verify:test-discovery — all 699 files discovered exactly once.
  • git diff --check — passed.
  • Under Node 22.23.0: node_modules/.bin/vitest run tests/boundary/filesystem/javascriptPackagePrecedence.test.ts tests/boundary/filesystem/javascriptPackageExportTargets.test.ts — 97 passed, no skips.
  • Task-owned CLI verification script — the 10 invocations described above passed; no fixture or local report committed.

Hosted CI observation at d42085fd760f1bd0a0f02e1c7d3737fa670ece2b: run 37983194315 reports action_required, with zero jobs executed and no check results. Maintainer action is needed before hosted validation can proceed. The automated review bot separately reports that its review quota is exhausted; no automated review pass is claimed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; static relationships now follow the standard condition supported by REA's Node range. Existing earlier-condition precedence remains.
  • Real Hopper/Ghidra, browser, or OS coverage: real macOS filesystem and native Node resolver/loader execution; Hopper/Ghidra/IDA/browser verification not run because their behavior is unchanged.
  • Package or release metadata impact: none. Package/install checks and check:pr were not run; full deterministic coverage belongs to CI for this focused fix.
  • Security, privacy, process, or containment review: no credentials, binaries, personal configuration, new authority, or generated artifacts added.

Review checklist

  • This PR addresses a concrete problem.
  • One focused outcome; Conventional Commit title.
  • Reason for no linked issue stated above; overlap search completed.
  • Tests cover changed observable behavior and meaningful failures.
  • Owning documentation updated; contracts and generated metadata unchanged.
  • Representative CLI output included.
  • Final diff reviewed for secrets, unrelated edits and unsupported claims.

Prepared and verified locally with OpenAI Codex assistance. All listed testing was agent-executed; no human manual verification is claimed. No AI coauthor trailer is used.


🔄 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/1361 **Author:** [@akshitkhokhani](https://github.com/akshitkhokhani) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/fix-module-sync-exports` --- ### 📝 Commits (1) - [`d42085f`](https://github.com/morluto/rea/commit/d42085fd760f1bd0a0f02e1c7d3737fa670ece2b) fix(javascript): honor module-sync package exports ### 📊 Changes **4 files changed** (+58 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+2 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+3 -1) 📝 `tests/boundary/filesystem/javascriptPackageExportTargets.test.ts` (+21 -0) 📝 `tests/boundary/filesystem/javascriptPackagePrecedence.test.ts` (+32 -1) </details> ### 📄 Description ## Summary Honor Node's `module-sync` package export condition during static JavaScript application analysis. A package with `exports: { "module-sync": "./sync.mjs", "default": "./fallback.cjs" }` currently yields a confidently resolved `fallback.cjs` relationship even though the supported Node runtime selects `sync.mjs`. No linked issue: this small bug was discovered through an owned fixture and reproduced against the compiled CLI and real Node loaders. Searches for `module-sync` found no issue/PR. Related #591, #606, #1181 and #1333 handle order, fallback, exact targets and invalid configurations; this adds the missing active condition without replacing those rules. ## Problem and expected behavior Node's [conditional exports contract](https://nodejs.org/download/release/v24.18.0/docs/api/packages.html#conditional-exports) activates `module-sync` for imports and requires. REA's `packageExportConditions` omits it. This also incorrectly bypasses explicit null or invalid targets under that condition and selects a later default. ## Change and scope Add `module-sync` to the existing active condition set. Preserve declared-order traversal, nested/array selection, exact-file lookup, admission rules and refusal diagnostics. Extend the existing real-Node/filesystem differential tests and document the condition. No dependencies or public API added. ## Contract and boundary impact - Semantic owner and earliest changed stage: application package export-condition selection, before artifact target resolution. - CLI and MCP/tool-catalog contract: existing schemas and inventory unchanged; shared static analysis produces corrected relationships. - Provider, bridge, target-format, or platform compatibility: provider-independent JavaScript directory analysis; verified locally on macOS arm64 with pinned Node 24.18.0 and supported Node 22.23.0. - Evidence, artifact, provenance, or reconstruction contract: preserve original sources, paths, digests and existing resolution statuses; correct the selected target or refusal. - Process execution, authorization, cleanup, or containment impact: no production execution or authority changes; tests use owned inert packages and repository temporary-directory cleanup. - Generated metadata, package, or installation impact: none; authored documentation updated, generated outputs excluded. ## Evidence and regression coverage - Base: `9c4a68bc19a0f7d39557e3cc039f5877f6550f63`. - New regressions against unchanged production code: **6 failures / 91 passes** across the two affected boundary suites. Failures were wrong leaf selection for import/require and false fallback after invalid/null active targets. - After the fix: **234 passes** across five relevant suites on Node 24.18.0; the two affected suites additionally pass **97 cases** on Node 22.23.0. - Real Node subprocesses verify selected paths, loaded owned values, refusal codes, nested conditions, earlier import/default priority and unmatched nested fallthrough. No module mocks. - Built public CLI checks: five fixture configurations through both `analyze` and `analyze-javascript-application`, **10 successful invocations**. Stdout parsed as JSON, exit status was 0, and both import and require relationships matched native resolution. Static analysis succeeds while retaining unresolved/rejected relationships for refused dependencies. - Representative corrected relationship: `{"module_link_kind":"require","specifier":"fixture","resolved_path":"node_modules/fixture/sync.mjs","resolution_status":"resolved"}`; base selected `node_modules/fixture/fallback.cjs`. - Remaining proof gaps: no full local deterministic suite or real binary/browser providers; static target resolution does not establish runtime loadability of arbitrary modules. 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 Pinned Node 24.18.0 / npm 11.16.0, locally on macOS arm64: - `npm ci` — passed; 0 audit vulnerabilities. - `npm run build:cached` — passed. - `npm run test:focused -- tests/boundary/filesystem/javascriptPackagePrecedence.test.ts tests/boundary/filesystem/javascriptPackageExportTargets.test.ts tests/boundary/javascript/packageConditionalExports.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts` — 234 passed, no skips. - `npm run check` — all 6 tasks passed; lint reported warnings, 0 errors. - Pre-push `npm run check:fast` — both typecheck/lint tasks passed; normal commit formatting/lint hooks passed. - `npm run check:ci` — all 8 static tasks passed. - `npm run docs:check` — all 6 tasks passed. - `npm run verify:test-discovery` — all 699 files discovered exactly once. - `git diff --check` — passed. - Under Node 22.23.0: `node_modules/.bin/vitest run tests/boundary/filesystem/javascriptPackagePrecedence.test.ts tests/boundary/filesystem/javascriptPackageExportTargets.test.ts` — 97 passed, no skips. - Task-owned CLI verification script — the 10 invocations described above passed; no fixture or local report committed. Hosted CI observation at `d42085fd760f1bd0a0f02e1c7d3737fa670ece2b`: [run 37983194315](https://github.com/morluto/rea/actions/runs/37983194315) reports `action_required`, with zero jobs executed and no check results. Maintainer action is needed before hosted validation can proceed. The automated review bot separately reports that its review quota is exhausted; no automated review pass is claimed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; static relationships now follow the standard condition supported by REA's Node range. Existing earlier-condition precedence remains. - Real Hopper/Ghidra, browser, or OS coverage: real macOS filesystem and native Node resolver/loader execution; Hopper/Ghidra/IDA/browser verification not run because their behavior is unchanged. - Package or release metadata impact: none. Package/install checks and `check:pr` were not run; full deterministic coverage belongs to CI for this focused fix. - Security, privacy, process, or containment review: no credentials, binaries, personal configuration, new authority, or generated artifacts added. ## Review checklist - [x] This PR addresses a concrete problem. - [x] One focused outcome; Conventional Commit title. - [x] Reason for no linked issue stated above; overlap search completed. - [x] Tests cover changed observable behavior and meaningful failures. - [x] Owning documentation updated; contracts and generated metadata unchanged. - [x] Representative CLI output included. - [x] Final diff reviewed for secrets, unrelated edits and unsupported claims. Prepared and verified locally with OpenAI Codex assistance. All listed testing was agent-executed; no human manual verification is claimed. No AI coauthor trailer is used. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-17 21:10:44 +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#2827
No description provided.