morluto/rea

Fork 0

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

[PR #3254] [CLOSED] fix(javascript): resolve ESM relative imports with tabs or newlines #5678

New issue

Closed

opened 2026-10-31 01:08:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3254
Author: @paoValle
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/3214-esm-relative-url-text

📝 Commits (1)

b5d6db8 fix(javascript): resolve ESM relative imports with tabs or newlines

📊 Changes

2 files changed (+68 additions, -2 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+60 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+8 -2)

📄 Description

Summary

Fixes #3214.

A relative ESM import whose specifier contains a tab or a newline was reported
not-found, with resolved_path: null, even when the target file is present and
Node loads the same application. Node parses a module specifier as a URL, so the
URL parser's input-text rules run before the single percent-decoding pass; REA ran
only the decoding pass and looked the literal control bytes up as a filename.

Problem and expected behavior

Smallest trigger, from the issue:

app/package.json   {"type":"module"}
app/index.mjs      import "./tab\tname.js"; import "./nl\nname.js"; import "./tabname.js";
app/tabname.js, app/nlname.js

Before the change, node scripts/rea.mjs analyze-javascript-application repro-esm --json
exits 0 and the static-import edges report:

{"specifier": "./nl\nname.js",  "resolution_mode": "import", "resolved_path": null,             "resolution_status": "not-found"}
{"specifier": "./tab\tname.js", "resolution_mode": "import", "resolved_path": null,             "resolution_status": "not-found"}
{"specifier": "./tabname.js",   "resolution_mode": "import", "resolved_path": "app/tabname.js", "resolution_status": "resolved"}

while Node's resolver selects tabname.js, nlname.js, tabname.js for the three
specifiers. Expected: both affected edges resolve to the file Node selects, with the
original specifier strings and resolution_mode preserved.

Change and scope

In the ESM branch of contextualCandidate
(src/application/javascript/JavaScriptArtifactPathResolution.ts) the lookup text is
now derived as

const moduleUrlText = stripQueryAndFragment(htmlUrlText(declared));

before decodeModuleUrlPath, and the encoded slash/backslash rejection check uses the
same text. htmlUrlText (src/domain/artifactPathSyntax.ts) is the existing owner of
the URL parser's input-text rules and is already composed with stripQueryAndFragment
for HTML references and package targets, so no new character-level rule and no new
dependency are introduced.

Classification order is unchanged: relative/bare detection still runs on the raw
declaration, matching Node, which decides whether a specifier is relative before
parsing it as a URL. A trailing space on an already-relative specifier now trims like
Node; a leading space is still classified as bare, as Node does (both checked with
import.meta.resolve probes).

Not included: no change to url-reference handling, to file: URLs (control bytes
there stay rejected), or to CommonJS requests (literal filenames, including a literal
tab).

Contract and boundary impact

Semantic owner and earliest changed stage: contextualCandidate, ESM relative module URL path resolution (src/application/javascript/JavaScriptArtifactPathResolution.ts).
Contribution scope fit and linked maintainer decision: not applicable — ordinary defect fix with no target execution, injection, or mutation.
CLI and MCP/tool-catalog contract: none; the affected property values (resolved_path, resolution_status) keep their schema.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: an edge moves from not-found to resolved for the same input; original specifiers and resolution mode are retained.
Target selection, authority, upstream behavior, and cleanup details: not applicable — static analysis of caller-selected inputs, no process or network activity.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none. No generated file, package, or skills/ output changes.

Expected effects: [x] Read existing files or captured data. [x] Documentation or usability only; no target interaction. Both observed CLI runs analyze inert local sources and execute nothing.

Evidence and regression coverage

Tests added: one test in the ESM module URL path decoding block of src/application/javascript/JavaScriptArtifactPathResolution.test.ts, covering tab, newline, CR, a trailing space, the percent-encoded %09 control (resolves to the literal-tab filename), the CommonJS literal-tab control, and a space before the query/fragment (interior URL text, so it stays not-found).
Base reproduction: the issue's fixture on main (e5791229f) before the change — output above.
Bite check: with the source change reverted and the test kept, the focused suite reports 1 failed | 158 passed, failing on the first tab case (not-found instead of resolved).
Remaining proof gaps: Windows was not exercised; the change is platform-independent string preprocessing. No real-provider lane applies.

Validation performed

npm run test:focused -- src/application/javascript/JavaScriptArtifactPathResolution.test.ts — Test Files 1 passed (1), Tests 159 passed (159).
npm run test:focused -- src/application/javascript/JavaScriptArtifactPathResolution.test.ts tests/boundary/filesystem/javascript-applications/javascriptModuleRelationships.test.ts tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts tests/boundary/cli/javascriptSelfImportRelationships.test.ts — Test Files 4 passed (4), Tests 174 passed (174).
npm run check:fast — Tasks: 2 successful, 2 total.
npm run check — Tasks: 6 successful, 6 total; All matched files use the correct format.; Checked resolved module boundaries in 1862 source files.; Architecture guards passed across 1386 source files.
npm run build:cached, then node scripts/rea.mjs analyze-javascript-application <fixture> --json — exit 0, and the three static-import edges report resolved to app/tabname.js, app/nlname.js, app/tabname.js.
Same CLI on the issue's control fixture — the ordinary imports, the percent-encoded %09 ESM specifier (resolved_path: "literal\tname.js") and require("./literal\tname.cjs") (resolved_path: "literal\tname.cjs") all stay resolved; node index.mjs and node require.cjs exit 0.
Node oracle on the fixture — import.meta.resolve selects tabname.js, nlname.js, tabname.js for the three specifiers.

Compatibility, safety, and release

Breaking changes or migration steps: none. For ESM relative specifiers containing literal tabs/newlines or surrounding controls, an edge that was not-found now resolves to the file Node loads; no previously resolved edge changes target.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only; no analysis engine is involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none — string preprocessing over caller-selected declarations, with no new input source or side effect.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed (none required).
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

An AI coding agent drafted this change under my direction; I reviewed the diff and ran the
commands above with the output quoted here.

_{🔄 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/3254 **Author:** [@paoValle](https://github.com/paoValle) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3214-esm-relative-url-text` --- ### 📝 Commits (1) - [`b5d6db8`](https://github.com/morluto/rea/commit/b5d6db88ad2f853c527e891215afc326673aa3c3) fix(javascript): resolve ESM relative imports with tabs or newlines ### 📊 Changes **2 files changed** (+68 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+60 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+8 -2) </details> ### 📄 Description ## Summary Fixes #3214. A relative ESM import whose specifier contains a tab or a newline was reported `not-found`, with `resolved_path: null`, even when the target file is present and Node loads the same application. Node parses a module specifier as a URL, so the URL parser's input-text rules run before the single percent-decoding pass; REA ran only the decoding pass and looked the literal control bytes up as a filename. ## Problem and expected behavior Smallest trigger, from the issue: ``` app/package.json {"type":"module"} app/index.mjs import "./tab\tname.js"; import "./nl\nname.js"; import "./tabname.js"; app/tabname.js, app/nlname.js ``` Before the change, `node scripts/rea.mjs analyze-javascript-application repro-esm --json` exits 0 and the static-import edges report: ``` {"specifier": "./nl\nname.js", "resolution_mode": "import", "resolved_path": null, "resolution_status": "not-found"} {"specifier": "./tab\tname.js", "resolution_mode": "import", "resolved_path": null, "resolution_status": "not-found"} {"specifier": "./tabname.js", "resolution_mode": "import", "resolved_path": "app/tabname.js", "resolution_status": "resolved"} ``` while Node's resolver selects `tabname.js`, `nlname.js`, `tabname.js` for the three specifiers. Expected: both affected edges resolve to the file Node selects, with the original specifier strings and `resolution_mode` preserved. ## Change and scope In the ESM branch of `contextualCandidate` (`src/application/javascript/JavaScriptArtifactPathResolution.ts`) the lookup text is now derived as ```ts const moduleUrlText = stripQueryAndFragment(htmlUrlText(declared)); ``` before `decodeModuleUrlPath`, and the encoded slash/backslash rejection check uses the same text. `htmlUrlText` (`src/domain/artifactPathSyntax.ts`) is the existing owner of the URL parser's input-text rules and is already composed with `stripQueryAndFragment` for HTML references and package targets, so no new character-level rule and no new dependency are introduced. Classification order is unchanged: relative/bare detection still runs on the raw declaration, matching Node, which decides whether a specifier is relative before parsing it as a URL. A trailing space on an already-relative specifier now trims like Node; a leading space is still classified as bare, as Node does (both checked with `import.meta.resolve` probes). Not included: no change to `url-reference` handling, to `file:` URLs (control bytes there stay rejected), or to CommonJS requests (literal filenames, including a literal tab). ## Contract and boundary impact - Semantic owner and earliest changed stage: `contextualCandidate`, ESM relative module URL path resolution (`src/application/javascript/JavaScriptArtifactPathResolution.ts`). - Contribution scope fit and linked maintainer decision: not applicable — ordinary defect fix with no target execution, injection, or mutation. - CLI and MCP/tool-catalog contract: none; the affected property values (`resolved_path`, `resolution_status`) keep their schema. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: an edge moves from `not-found` to `resolved` for the same input; original specifiers and resolution mode are retained. - Target selection, authority, upstream behavior, and cleanup details: not applicable — static analysis of caller-selected inputs, no process or network activity. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. No generated file, package, or `skills/` output changes. Expected effects: [x] Read existing files or captured data. [x] Documentation or usability only; no target interaction. Both observed CLI runs analyze inert local sources and execute nothing. ## Evidence and regression coverage - Tests added: one test in the `ESM module URL path decoding` block of `src/application/javascript/JavaScriptArtifactPathResolution.test.ts`, covering tab, newline, CR, a trailing space, the percent-encoded `%09` control (resolves to the literal-tab filename), the CommonJS literal-tab control, and a space before the query/fragment (interior URL text, so it stays `not-found`). - Base reproduction: the issue's fixture on `main` (`e5791229f`) before the change — output above. - Bite check: with the source change reverted and the test kept, the focused suite reports `1 failed | 158 passed`, failing on the first tab case (`not-found` instead of `resolved`). - Remaining proof gaps: Windows was not exercised; the change is platform-independent string preprocessing. No real-provider lane applies. ## Validation performed - `npm run test:focused -- src/application/javascript/JavaScriptArtifactPathResolution.test.ts` — `Test Files 1 passed (1)`, `Tests 159 passed (159)`. - `npm run test:focused -- src/application/javascript/JavaScriptArtifactPathResolution.test.ts tests/boundary/filesystem/javascript-applications/javascriptModuleRelationships.test.ts tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts tests/boundary/cli/javascriptSelfImportRelationships.test.ts` — `Test Files 4 passed (4)`, `Tests 174 passed (174)`. - `npm run check:fast` — `Tasks: 2 successful, 2 total`. - `npm run check` — `Tasks: 6 successful, 6 total`; `All matched files use the correct format.`; `Checked resolved module boundaries in 1862 source files.`; `Architecture guards passed across 1386 source files.` - `npm run build:cached`, then `node scripts/rea.mjs analyze-javascript-application <fixture> --json` — exit 0, and the three static-import edges report `resolved` to `app/tabname.js`, `app/nlname.js`, `app/tabname.js`. - Same CLI on the issue's control fixture — the ordinary imports, the percent-encoded `%09` ESM specifier (`resolved_path: "literal\tname.js"`) and `require("./literal\tname.cjs")` (`resolved_path: "literal\tname.cjs"`) all stay `resolved`; `node index.mjs` and `node require.cjs` exit 0. - Node oracle on the fixture — `import.meta.resolve` selects `tabname.js`, `nlname.js`, `tabname.js` for the three specifiers. ## Compatibility, safety, and release - Breaking changes or migration steps: none. For ESM relative specifiers containing literal tabs/newlines or surrounding controls, an edge that was `not-found` now resolves to the file Node loads; no previously resolved edge changes target. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only; no analysis engine is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none — string preprocessing over caller-selected declarations, with no new input source or side effect. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed (none required). - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- An AI coding agent drafted this change under my direction; I reviewed the diff and ran the commands above with the output quoted here. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:33 +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#5678

No description provided.

Rows
Columns