[PR #606] [MERGED] fix(javascript): preserve Node exports fallback stop reasons #2105

New issue

Closed

opened 2026-10-16 21:14:21 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:14:21 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/606
Author: @rudycelekli
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: campaign/rea-exports-fallback-followup-20261005

📝 Commits (3)

99b5ee3 fix(javascript): preserve Node exports fallback stop reasons
54ff342 test(browser): preserve unknown with-scope loader bindings
7c5f359 chore: preserve upstream source-map recovery fixture

📊 Changes

2 files changed (+86 additions, -36 deletions)

View changed files

📝 src/application/JavaScriptArtifactPathResolution.ts (+20 -12)
📝 tests/boundary/javascript/packageConditionalExports.test.ts (+66 -24)

📄 Description

Summary

Follow up on merged #591: preserve Node's distinction between unmatched nested conditions, explicit null, and invalid array fallback entries.

No separate issue: this is a focused correctness follow-up found while verifying the maintainer's expanded condition/array support in #591.

Problem and expected behavior

Real installed-package Node comparisons exposed five remaining differences:

An unmatched nested condition should allow a later outer default; explicit null must still block it.
Arrays should skip invalid targets, stop at their first selected string, and retain the final invalid-versus-null refusal if no string is selected.
A selected file that is absent must remain absent, without falling through to another file.

Change and scope

Add an explicit unmatched outcome to the existing private resolver and stop array selection at the first usable target. Retain the maintainer's active node/node-addons conditions, nested targets, ordered conditions, and refusal diagnostics.

The target check covers the supported relative ./ prefix. This does not establish universal equivalence with Node's export grammar: top-level array exports, subpath patterns, and other target-segment grammar remain outside the existing supported contract.

Contract and boundary impact

Semantic owner and earliest changed stage: application package-export target selection before artifact relationship resolution.
CLI and MCP/tool catalog: existing schemas and tools; corrected target selection uses existing result states.
Provider/platform coverage: real Node 24.18.0 import/require execution against owned temporary packages on macOS.
Evidence/provenance: source-derived artifact relationships retain their paths and identities; native comparisons are executed test evidence.
Execution, authority, cleanup and containment: production still parses caller-selected metadata; tests clean their owned package directories.
Metadata/package/install: no dependency, manifest, generated catalog, or installation changes.

Evidence and regression coverage

The added differential cases fail on current upstream bae87b4: five failed, 29 passed. After the fix, 34 real Node comparison cases plus 28 existing resolver tests pass. Tests retain Node's actual error codes for all-invalid and explicit-null controls. No module mocks are used.

Validation performed

On the final signed head:

npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts src/application/JavaScriptArtifactPathResolution.test.ts — 62 passed.
npm run check — passed, including type checking, lint, formatting, dead-code and metadata checks.
npm run check:fast — passed.

These follow the current ordinary-fix workflow in CONTRIBUTING after #599. check:pr is optional for this routine change and was not run on this head; upstream CI provides full coverage. Package contents and installation paths are unchanged.

Compatibility and remaining limits

No migration or new provider support. Real Hopper/Ghidra verification is not relevant to this application resolver change. Preserve explicit unavailable/external outcomes rather than claiming all Node package forms are supported.

Prepared with AI assistance.

CI baseline reconciliation

The final head 7c5f359bc0 preserves upstream #609’s exact recovered-source fixture through a signed normal merge. The package resolver regression and source-map tests pass together, and npm run check passes on this head. Earlier whole-suite receipts refer to earlier heads; no whole-suite or hosted-CI pass is claimed for this update.

_{🔄 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/606 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `campaign/rea-exports-fallback-followup-20261005` --- ### 📝 Commits (3) - [`99b5ee3`](https://github.com/morluto/rea/commit/99b5ee31e67c22333063e5c9127a737862e2932c) fix(javascript): preserve Node exports fallback stop reasons - [`54ff342`](https://github.com/morluto/rea/commit/54ff34240eef1d59dde68e6ca03b671eb13663d2) test(browser): preserve unknown with-scope loader bindings - [`7c5f359`](https://github.com/morluto/rea/commit/7c5f359bc0407f8c47e2852f9e46152b9d06019f) chore: preserve upstream source-map recovery fixture ### 📊 Changes **2 files changed** (+86 additions, -36 deletions) <details> <summary>View changed files</summary> 📝 `src/application/JavaScriptArtifactPathResolution.ts` (+20 -12) 📝 `tests/boundary/javascript/packageConditionalExports.test.ts` (+66 -24) </details> ### 📄 Description ## Summary Follow up on merged #591: preserve Node's distinction between unmatched nested conditions, explicit `null`, and invalid array fallback entries. No separate issue: this is a focused correctness follow-up found while verifying the maintainer's expanded condition/array support in #591. ## Problem and expected behavior Real installed-package Node comparisons exposed five remaining differences: - An unmatched nested condition should allow a later outer `default`; explicit `null` must still block it. - Arrays should skip invalid targets, stop at their first selected string, and retain the final invalid-versus-null refusal if no string is selected. - A selected file that is absent must remain absent, without falling through to another file. ## Change and scope Add an explicit unmatched outcome to the existing private resolver and stop array selection at the first usable target. Retain the maintainer's active `node`/`node-addons` conditions, nested targets, ordered conditions, and refusal diagnostics. The target check covers the supported relative `./` prefix. This does not establish universal equivalence with Node's export grammar: top-level array exports, subpath patterns, and other target-segment grammar remain outside the existing supported contract. ## Contract and boundary impact - Semantic owner and earliest changed stage: application package-export target selection before artifact relationship resolution. - CLI and MCP/tool catalog: existing schemas and tools; corrected target selection uses existing result states. - Provider/platform coverage: real Node 24.18.0 import/require execution against owned temporary packages on macOS. - Evidence/provenance: source-derived artifact relationships retain their paths and identities; native comparisons are executed test evidence. - Execution, authority, cleanup and containment: production still parses caller-selected metadata; tests clean their owned package directories. - Metadata/package/install: no dependency, manifest, generated catalog, or installation changes. ## Evidence and regression coverage The added differential cases fail on current upstream `bae87b4`: five failed, 29 passed. After the fix, 34 real Node comparison cases plus 28 existing resolver tests pass. Tests retain Node's actual error codes for all-invalid and explicit-null controls. No module mocks are used. ## Validation performed On the final signed head: - `npm run test:focused -- tests/boundary/javascript/packageConditionalExports.test.ts src/application/JavaScriptArtifactPathResolution.test.ts` — 62 passed. - `npm run check` — passed, including type checking, lint, formatting, dead-code and metadata checks. - `npm run check:fast` — passed. These follow the current ordinary-fix workflow in CONTRIBUTING after #599. `check:pr` is optional for this routine change and was not run on this head; upstream CI provides full coverage. Package contents and installation paths are unchanged. ## Compatibility and remaining limits No migration or new provider support. Real Hopper/Ghidra verification is not relevant to this application resolver change. Preserve explicit unavailable/external outcomes rather than claiming all Node package forms are supported. Prepared with AI assistance. ## CI baseline reconciliation The final head 7c5f359bc0407f8c47e2852f9e46152b9d06019f preserves upstream #609’s exact recovered-source fixture through a signed normal merge. The package resolver regression and source-map tests pass together, and `npm run check` passes on this head. Earlier whole-suite receipts refer to earlier heads; no whole-suite or hosted-CI pass is claimed for this update. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:14:21 +00:00