morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #1888] [CLOSED] fix(javascript): preserve bare package specifier suffixes #4880

New issue

Closed

opened 2026-10-31 00:56:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:56:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1888
Author: @xu91102
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/esm-package-specifier-punctuation

📝 Commits (1)

f11cc3d fix(javascript): preserve bare package specifier suffixes

📊 Changes

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

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+2 -4)
➕ tests/acceptance/applications/packageSpecifierSuffixCli.test.ts (+120 -0)

📄 Description

Summary

Bare ESM package specifiers lost their query/fragment spelling before exports lookup, inventing dependencies that Node rejects and missing explicitly exported punctuated keys. Preserve the full bare package request and apply URL normalization only to URL-like paths. Fixes #1883.

Problem and expected behavior

Given a package exporting only ./feature, Node rejects import "fixture/feature?cache=1" with ERR_PACKAGE_PATH_NOT_EXPORTED. REA previously reported node_modules/fixture/feature.cjs as resolved by silently looking up ./feature. The import now remains external/unresolved. An explicitly exported ./literal?cache=1 still resolves by that exact key.

Change and scope

Keep module-specifier text intact until bare package requests are distinguished from relative/file URLs. Strip query/fragment text in the relative ESM URL branch, retaining existing HTML, source-map URL, file URL, and CommonJS behavior. Package export selection itself is unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: application-layer module-specifier normalization before package resolution.
CLI and MCP/tool-catalog contract: unchanged schemas; both surfaces now report the correct target or unresolved status for the original request.
Provider, bridge, target-format, or platform compatibility: static JavaScript package analysis; no engine or bridge changes.
Evidence, artifact, provenance, or reconstruction contract: preserve the full caller specifier and avoid deriving a target from a different export key.
Process execution, authorization, cleanup, or containment impact: none in production; resolution still uses the existing artifact inventory.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

New compiled CLI and stdio MCP acceptance coverage checks unexported query and fragment requests plus an explicitly exported query-bearing subpath.
Separate real Node subprocesses resolve each request against the same on-disk package: the unexported requests return ERR_PACKAGE_PATH_NOT_EXPORTED, and the explicit full key loads its intended module.
On base fb94d9dde0ccda83ae520ac6fb22b1d283562cf3, both new surface tests failed because REA invented the ./feature target. Both pass after the fix.
A separate actual CLI reproduction produced the wrong resolved target twice while direct Node execution rejected the request.
Existing relative ESM, file URL, CommonJS, conditional exports, and package-subpath tests pass.
Review: self-review of the fixed base, complete patch, and new acceptance test; no actionable findings.
Remaining proof gaps: full deterministic coverage is left to upstream CI; real native engines and browser runtime are outside this static resolution change.
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

All local checks used Node 24.18.0 / npm 11.16.0 on macOS 27.0.1 arm64.

npm run test:focused -- tests/acceptance/applications/packageSpecifierSuffixCli.test.ts before the production fix — 2 expected regression failures.
npm run test:focused -- tests/acceptance/applications/packageSpecifierSuffixCli.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts tests/boundary/javascript/packageConditionalExports.test.ts src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts — 181 tests passed across 5 files, including freshly built CLI/MCP workflows.
npm run check — passed typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards; 758 lint warnings, zero errors.
npm run verify:test-discovery — all 843 test files discovered exactly once.
git diff --check — passed.
Upstream CI — awaiting maintainer approval for this fork contribution (action_required); no CI jobs have executed. This is not counted as a passing check.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: actual Node resolver and REA CLI/MCP on macOS; no native-provider or browser-runtime claim.
Package or release metadata impact: none.
Security, privacy, process, or containment review: artifact-root and container checks remain unchanged; no new process authority or external fetches.

Review checklist

This PR addresses a concrete reproduced problem and has one focused outcome.
The title follows Conventional Commits and the related issue is linked.
Tests cover observable behavior, runtime refusal, and exact export keys.
No owning docs, schemas, or generated metadata require changes.
Representative CLI/MCP output and actual validation are recorded above.
The final diff contains no secrets, unrelated cleanup, or unsupported provider 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/1888 **Author:** [@xu91102](https://github.com/xu91102) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/esm-package-specifier-punctuation` --- ### 📝 Commits (1) - [`f11cc3d`](https://github.com/morluto/rea/commit/f11cc3dc050d99d5d12e399a1ad61301650ac14b) fix(javascript): preserve bare package specifier suffixes ### 📊 Changes **2 files changed** (+122 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+2 -4) ➕ `tests/acceptance/applications/packageSpecifierSuffixCli.test.ts` (+120 -0) </details> ### 📄 Description ## Summary Bare ESM package specifiers lost their query/fragment spelling before exports lookup, inventing dependencies that Node rejects and missing explicitly exported punctuated keys. Preserve the full bare package request and apply URL normalization only to URL-like paths. Fixes #1883. ## Problem and expected behavior Given a package exporting only `./feature`, Node rejects `import "fixture/feature?cache=1"` with `ERR_PACKAGE_PATH_NOT_EXPORTED`. REA previously reported `node_modules/fixture/feature.cjs` as resolved by silently looking up `./feature`. The import now remains external/unresolved. An explicitly exported `./literal?cache=1` still resolves by that exact key. ## Change and scope Keep module-specifier text intact until bare package requests are distinguished from relative/file URLs. Strip query/fragment text in the relative ESM URL branch, retaining existing HTML, source-map URL, file URL, and CommonJS behavior. Package export selection itself is unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: application-layer module-specifier normalization before package resolution. - CLI and MCP/tool-catalog contract: unchanged schemas; both surfaces now report the correct target or unresolved status for the original request. - Provider, bridge, target-format, or platform compatibility: static JavaScript package analysis; no engine or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: preserve the full caller specifier and avoid deriving a target from a different export key. - Process execution, authorization, cleanup, or containment impact: none in production; resolution still uses the existing artifact inventory. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - New compiled CLI and stdio MCP acceptance coverage checks unexported query and fragment requests plus an explicitly exported query-bearing subpath. - Separate real Node subprocesses resolve each request against the same on-disk package: the unexported requests return `ERR_PACKAGE_PATH_NOT_EXPORTED`, and the explicit full key loads its intended module. - On base `fb94d9dde0ccda83ae520ac6fb22b1d283562cf3`, both new surface tests failed because REA invented the `./feature` target. Both pass after the fix. - A separate actual CLI reproduction produced the wrong resolved target twice while direct Node execution rejected the request. - Existing relative ESM, file URL, CommonJS, conditional exports, and package-subpath tests pass. - Review: self-review of the fixed base, complete patch, and new acceptance test; no actionable findings. - Remaining proof gaps: full deterministic coverage is left to upstream CI; real native engines and browser runtime are outside this static resolution change. - [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 All local checks used Node 24.18.0 / npm 11.16.0 on macOS 27.0.1 arm64. - `npm run test:focused -- tests/acceptance/applications/packageSpecifierSuffixCli.test.ts` before the production fix — 2 expected regression failures. - `npm run test:focused -- tests/acceptance/applications/packageSpecifierSuffixCli.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts tests/boundary/javascript/packageConditionalExports.test.ts src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts` — 181 tests passed across 5 files, including freshly built CLI/MCP workflows. - `npm run check` — passed typecheck, lint, formatting, knip, metadata, module boundaries, and architecture guards; 758 lint warnings, zero errors. - `npm run verify:test-discovery` — all 843 test files discovered exactly once. - `git diff --check` — passed. - [Upstream CI](https://github.com/morluto/rea/actions/runs/38153085763) — awaiting maintainer approval for this fork contribution (`action_required`); no CI jobs have executed. This is not counted as a passing check. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: actual Node resolver and REA CLI/MCP on macOS; no native-provider or browser-runtime claim. - Package or release metadata impact: none. - Security, privacy, process, or containment review: artifact-root and container checks remain unchanged; no new process authority or external fetches. ## Review checklist - [x] This PR addresses a concrete reproduced problem and has one focused outcome. - [x] The title follows Conventional Commits and the related issue is linked. - [x] Tests cover observable behavior, runtime refusal, and exact export keys. - [x] No owning docs, schemas, or generated metadata require changes. - [x] Representative CLI/MCP output and actual validation are recorded above. - [x] The final diff contains no secrets, unrelated cleanup, or unsupported provider 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:56:19 +00:00

closed this issue
added the
pull-request
label

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#4880

No description provided.

Rows
Columns