morluto/rea

Fork 0

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

[GH-ISSUE #4509] [Bug] Leading whitespace in ESM specifiers creates false resolved local imports #1793

New issue

Closed

opened 2026-10-16 21:10:18 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

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

Owner

Originally created by @dakjdakd on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4509

Before submitting

I searched the open and closed issues and related PRs for this regression.
The reproduction contains only authored fixtures and no credentials, private paths, or sensitive target data.

Observed behavior

The focused fix and regression test are submitted in https://github.com/morluto/rea/pull/4510. I am working on that implementation alongside this report.

REA's JavaScript application graph resolves an ESM request such as " ./hidden.mjs" to the local file hidden.mjs. Node rejects the same request with ERR_MODULE_NOT_FOUND: the leading space means the original string is a bare package request, not a relative module path.

Both graph projections are affected. The static-import edge and the semantic import relationship retain the original specifier but report resolved_path: "hidden.mjs" and resolution_status: "resolved". In the reduced production-graph reproduction, these edges also carry high confidence and complete coverage. The file exists; the mistake is assigning that file to a request that the loader never classified as a local relative import.

Leading tabs and newlines expose the same ordering problem. A newline inserted between . and / is another useful negative control: removing it before classification turns an invalid package request into relative syntax.

Reproduction

Affected source revision: 180ba03bfddd5916251e81e97a500b1ede7a9eea, package version 6.5.0. The executed reproduction used Node v24.18.0 on Windows x64, locked dependencies, and an authored local application directory. No external application or analysis engine is involved.

The minimal fixture is:

repro-esm-classification/
  package.json
  main.mjs
  hidden.mjs

{"name":"leading-space-fixture","type":"module","main":"main.mjs"}

// main.mjs
import { hidden } from " ./hidden.mjs";
console.log(hidden);

// hidden.mjs
export const hidden = 42;

Run the authored fixture with the pinned Node runtime:

node repro-esm-classification/main.mjs

Node reports ERR_MODULE_NOT_FOUND, with a message identifying the requested package as ' .'. Removing the leading space allows the ordinary relative import to load.

The REA observation was obtained through the real source production boundary: canonical artifact inventory, artifact file reading, AST analysis, and application graph construction. The essential reproduction code is below; use the fixture directory as root:

import { resolve } from "node:path";
import { ArtifactResourceScope } from "./src/artifacts/ArtifactResourceScope.ts";
import { scanCanonicalArtifactInventory } from "./src/artifacts/inventory/scanCanonical.ts";
import { createJavaScriptArtifactReader } from "./src/artifacts/javascript/JavaScriptArtifactReader.ts";
import { readJavaScriptArtifactFiles } from "./src/artifacts/javascript/JavaScriptArtifactFiles.ts";
import { analyzeJavaScriptArtifactFiles } from "./src/application/javascript/JavaScriptArtifactAnalysis.ts";
import { buildJavaScriptArtifactGraph } from "./src/application/javascript/JavaScriptArtifactGraphBuilder.ts";

const root = resolve("repro-esm-classification");
const scope = new ArtifactResourceScope();
const reader = createJavaScriptArtifactReader(root, "directory");
try {
  const snapshot = await scanCanonicalArtifactInventory(root, { resourceScope: scope });
  const files = await readJavaScriptArtifactFiles(reader, snapshot, scope);
  const graph = buildJavaScriptArtifactGraph(
    snapshot, files, analyzeJavaScriptArtifactFiles(files),
  );
  console.log(graph.edges
    .filter(edge => edge.relation === "imports")
    .map(edge => edge.properties));
} finally {
  await reader.close();
  await scope.close();
}

Save this as a repository-root .mts file and run it with the installed tsx runner, for example node node_modules/tsx/dist/cli.mjs repro-esm-classification.mts. This is the same production pipeline used for the observation, rather than a mocked resolver or a previously compiled distribution.

The relevant properties on both resulting import edges are:

{
  "specifier": " ./hidden.mjs",
  "resolved_path": "hidden.mjs",
  "resolution_status": "resolved"
}

Expected behavior and controls

The original declaration must determine whether the request enters local relative/absolute module-path resolution. URL text preprocessing may then transform an admitted ESM path for lookup. It must not manufacture the syntax that admitted the request.

That distinction matters because URL preprocessing is correct for already-relative requests. For example, Node loads "./dep.mjs " and "./de\np.mjs" as dep.mjs. Removing whitespace handling altogether would reintroduce the earlier tab/newline bug.

The following eight cases were executed against Node and the production graph in the proposed regression test. \n and \t represent actual LF and tab characters in the runtime specifier. The graph column shows the expected result, also verified with the proposed correction:

Runtime specifier, shown escaped	Node 24.18.0	Correct graph result
`" ./dep.mjs"`	`ERR_MODULE_NOT_FOUND`	No resolved local path
`"\n./dep.mjs"`	`ERR_MODULE_NOT_FOUND`	No resolved local path
`"\t./dep.mjs"`	`ERR_MODULE_NOT_FOUND`	No resolved local path
`".\n/dep.mjs"`	`ERR_INVALID_MODULE_SPECIFIER`	No resolved local path
`"./dep.mjs"`	Loads `dep.mjs`	Resolved to `dep.mjs`
`"./dep.mjs "`	Loads `dep.mjs`	Resolved to `dep.mjs`
`"./de\np.mjs"`	Loads `dep.mjs`	Resolved to `dep.mjs`
`"./de\tp.mjs"`	Loads `dep.mjs`	Resolved to `dep.mjs`

The graph should retain unresolved relationships and their original specifiers for the negative cases. It does not need to reproduce Node's exception text or discard the import observation.

Analysis and proposed correction

The producing boundary is contextualCandidate in src/application/javascript/JavaScriptArtifactPathResolution.ts. At the affected revision, it calls htmlUrlText(declared) before isRelativeJavaScriptModuleSpecifier(declared) and the leading-slash check. Those checks therefore inspect normalized text rather than the loader's original classification input.

The smallest correction is to capture that relative/absolute path decision before normalization and reuse it at the existing bare/local branch. Keep normalization, declaration validation, query/fragment handling, and percent decoding in their existing order. Keeping the classification fact separate from lookup text fixes both graph consumers through their existing shared resolver, without adding repairs to individual projections.

Relationship to #3214 and #3254

#3214 reports valid relative imports containing internal tabs/newlines being left unresolved. #3254 addresses that false negative. Its actual patch performs normalization inside the relative-path branch, and its description explicitly says leading spaces remain bare because classification still uses the original declaration.

The integration commit 7e5cffa places normalization before that branch. This report concerns the resulting false positive, while preserving the original fix's positive cases. The old review also discussed leading trimming, so the topic is related; this report supplies the contrary native-loader result and a regression covering the classification boundary.

The open package-pattern work in #2018 handles wildcard exports and imports keys. Its current diff leaves this raw-versus-normalized classification in place. The two changes touch related resolution code but address separate decisions: selecting a package-map entry and deciding whether the original ESM request has local-path syntax. I found no separate report or claim for this classification regression in the reviewed issue/PR material.

Impact and verification scope

This is a graph-correctness defect: two independent projections assign an unsupported local dependency and provenance to the same request. Consumers of that graph could make incorrect tracing or comparison decisions; those downstream workflows were not executed, and real-world frequency is unknown.

Area: Evidence, artifacts, or reconstruction. Interface: Other — source production boundary. Earliest failure stage: Native analysis or reconstruction, specifically module-path resolution during JavaScript graph construction. The expected typed outcome is an unresolved edge with a null local path for the negative controls.

The reproduction and proposed correction were verified with real files and a separate Node loader process. CLI/MCP worker execution, other operating systems, and the complete repository test suite are outside the executed coverage. The original artifact bytes and source specifiers remain the evidence; modifying a copied application to make Node load it would describe a different artifact.

Originally created by @dakjdakd on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4509 ## Before submitting - [x] I searched the open and closed issues and related PRs for this regression. - [x] The reproduction contains only authored fixtures and no credentials, private paths, or sensitive target data. ## Observed behavior The focused fix and regression test are submitted in https://github.com/morluto/rea/pull/4510. I am working on that implementation alongside this report. REA's JavaScript application graph resolves an ESM request such as `" ./hidden.mjs"` to the local file `hidden.mjs`. Node rejects the same request with `ERR_MODULE_NOT_FOUND`: the leading space means the original string is a bare package request, not a relative module path. Both graph projections are affected. The static-import edge and the semantic import relationship retain the original specifier but report `resolved_path: "hidden.mjs"` and `resolution_status: "resolved"`. In the reduced production-graph reproduction, these edges also carry high confidence and complete coverage. The file exists; the mistake is assigning that file to a request that the loader never classified as a local relative import. Leading tabs and newlines expose the same ordering problem. A newline inserted between `.` and `/` is another useful negative control: removing it before classification turns an invalid package request into relative syntax. ## Reproduction Affected source revision: `180ba03bfddd5916251e81e97a500b1ede7a9eea`, package version `6.5.0`. The executed reproduction used Node `v24.18.0` on Windows x64, locked dependencies, and an authored local application directory. No external application or analysis engine is involved. The minimal fixture is: ```text repro-esm-classification/ package.json main.mjs hidden.mjs ``` ```json {"name":"leading-space-fixture","type":"module","main":"main.mjs"} ``` ```js // main.mjs import { hidden } from " ./hidden.mjs"; console.log(hidden); ``` ```js // hidden.mjs export const hidden = 42; ``` Run the authored fixture with the pinned Node runtime: ```sh node repro-esm-classification/main.mjs ``` Node reports `ERR_MODULE_NOT_FOUND`, with a message identifying the requested package as `' .'`. Removing the leading space allows the ordinary relative import to load. The REA observation was obtained through the real source production boundary: canonical artifact inventory, artifact file reading, AST analysis, and application graph construction. The essential reproduction code is below; use the fixture directory as `root`: ```ts import { resolve } from "node:path"; import { ArtifactResourceScope } from "./src/artifacts/ArtifactResourceScope.ts"; import { scanCanonicalArtifactInventory } from "./src/artifacts/inventory/scanCanonical.ts"; import { createJavaScriptArtifactReader } from "./src/artifacts/javascript/JavaScriptArtifactReader.ts"; import { readJavaScriptArtifactFiles } from "./src/artifacts/javascript/JavaScriptArtifactFiles.ts"; import { analyzeJavaScriptArtifactFiles } from "./src/application/javascript/JavaScriptArtifactAnalysis.ts"; import { buildJavaScriptArtifactGraph } from "./src/application/javascript/JavaScriptArtifactGraphBuilder.ts"; const root = resolve("repro-esm-classification"); const scope = new ArtifactResourceScope(); const reader = createJavaScriptArtifactReader(root, "directory"); try { const snapshot = await scanCanonicalArtifactInventory(root, { resourceScope: scope }); const files = await readJavaScriptArtifactFiles(reader, snapshot, scope); const graph = buildJavaScriptArtifactGraph( snapshot, files, analyzeJavaScriptArtifactFiles(files), ); console.log(graph.edges .filter(edge => edge.relation === "imports") .map(edge => edge.properties)); } finally { await reader.close(); await scope.close(); } ``` Save this as a repository-root `.mts` file and run it with the installed `tsx` runner, for example `node node_modules/tsx/dist/cli.mjs repro-esm-classification.mts`. This is the same production pipeline used for the observation, rather than a mocked resolver or a previously compiled distribution. The relevant properties on both resulting import edges are: ```json { "specifier": " ./hidden.mjs", "resolved_path": "hidden.mjs", "resolution_status": "resolved" } ``` ## Expected behavior and controls The original declaration must determine whether the request enters local relative/absolute module-path resolution. URL text preprocessing may then transform an admitted ESM path for lookup. It must not manufacture the syntax that admitted the request. That distinction matters because URL preprocessing is correct for already-relative requests. For example, Node loads `"./dep.mjs "` and `"./de\np.mjs"` as `dep.mjs`. Removing whitespace handling altogether would reintroduce the earlier tab/newline bug. The following eight cases were executed against Node and the production graph in the proposed regression test. `\n` and `\t` represent actual LF and tab characters in the runtime specifier. The graph column shows the expected result, also verified with the proposed correction: | Runtime specifier, shown escaped | Node 24.18.0 | Correct graph result | | --- | --- | --- | | `" ./dep.mjs"` | `ERR_MODULE_NOT_FOUND` | No resolved local path | | `"\n./dep.mjs"` | `ERR_MODULE_NOT_FOUND` | No resolved local path | | `"\t./dep.mjs"` | `ERR_MODULE_NOT_FOUND` | No resolved local path | | `".\n/dep.mjs"` | `ERR_INVALID_MODULE_SPECIFIER` | No resolved local path | | `"./dep.mjs"` | Loads `dep.mjs` | Resolved to `dep.mjs` | | `"./dep.mjs "` | Loads `dep.mjs` | Resolved to `dep.mjs` | | `"./de\np.mjs"` | Loads `dep.mjs` | Resolved to `dep.mjs` | | `"./de\tp.mjs"` | Loads `dep.mjs` | Resolved to `dep.mjs` | The graph should retain unresolved relationships and their original specifiers for the negative cases. It does not need to reproduce Node's exception text or discard the import observation. ## Analysis and proposed correction The producing boundary is `contextualCandidate` in `src/application/javascript/JavaScriptArtifactPathResolution.ts`. At the affected revision, it calls `htmlUrlText(declared)` before `isRelativeJavaScriptModuleSpecifier(declared)` and the leading-slash check. Those checks therefore inspect normalized text rather than the loader's original classification input. The smallest correction is to capture that relative/absolute path decision before normalization and reuse it at the existing bare/local branch. Keep normalization, declaration validation, query/fragment handling, and percent decoding in their existing order. Keeping the classification fact separate from lookup text fixes both graph consumers through their existing shared resolver, without adding repairs to individual projections. ## Relationship to #3214 and #3254 [#3214](https://github.com/morluto/rea/issues/3214) reports valid relative imports containing internal tabs/newlines being left unresolved. [#3254](https://github.com/morluto/rea/pull/3254) addresses that false negative. Its actual patch performs normalization inside the relative-path branch, and its description explicitly says leading spaces remain bare because classification still uses the original declaration. The integration commit [7e5cffa](https://github.com/morluto/rea/commit/7e5cffa2ae9dfc9afaeca0715110d4dd41c0dcdd) places normalization before that branch. This report concerns the resulting false positive, while preserving the original fix's positive cases. The old review also discussed leading trimming, so the topic is related; this report supplies the contrary native-loader result and a regression covering the classification boundary. The open package-pattern work in [#2018](https://github.com/morluto/rea/pull/2018) handles wildcard `exports` and `imports` keys. Its current diff leaves this raw-versus-normalized classification in place. The two changes touch related resolution code but address separate decisions: selecting a package-map entry and deciding whether the original ESM request has local-path syntax. I found no separate report or claim for this classification regression in the reviewed issue/PR material. ## Impact and verification scope This is a graph-correctness defect: two independent projections assign an unsupported local dependency and provenance to the same request. Consumers of that graph could make incorrect tracing or comparison decisions; those downstream workflows were not executed, and real-world frequency is unknown. Area: **Evidence, artifacts, or reconstruction**. Interface: **Other — source production boundary**. Earliest failure stage: **Native analysis or reconstruction**, specifically module-path resolution during JavaScript graph construction. The expected typed outcome is an unresolved edge with a null local path for the negative controls. The reproduction and proposed correction were verified with real files and a separate Node loader process. CLI/MCP worker execution, other operating systems, and the complete repository test suite are outside the executed coverage. The original artifact bytes and source specifiers remain the evidence; modifying a copied application to make Node load it would describe a different artifact.

bsakaguchi closed this issue

2026-10-31 00:32:53 +00:00

bsakaguchi commented

2026-10-31 00:32:55 +00:00

Author

Owner

@CassieFox23 commented on GitHub (Oct 21, 2026):

Thanks, dakjdakd, for the Node negative control. From reading main at 01788bac8, not a run: still valid. The file src/application/javascript/JavaScriptArtifactPathResolution.ts:638 passes import specifiers through htmlUrlText. That strips leading spaces, tabs and newlines (src/domain/artifactPathSyntax.ts:38-50). Only then does it check isRelativeJavaScriptModuleSpecifier at line 646. So " ./hidden.mjs" becomes "./hidden.mjs" and resolves. The reference-source importer does not trim (src/domain/referenceSourceImportParsing.ts:76-82). So the two graphs disagree. PR #4510 closed on 2026-10-21 without merging. Likely area: JavaScript path resolution.

🤖 Written with Claude Code (Fable 5.1, verified by Opus 5.5), posted with my OK 🦊

@CassieFox23 commented on GitHub (Oct 21, 2026): Thanks, dakjdakd, for the Node negative control. From reading main at 01788bac8, not a run: still valid. The file `src/application/javascript/JavaScriptArtifactPathResolution.ts:638` passes import specifiers through `htmlUrlText`. That strips leading spaces, tabs and newlines (`src/domain/artifactPathSyntax.ts:38-50`). Only then does it check `isRelativeJavaScriptModuleSpecifier` at line 646. So `" ./hidden.mjs"` becomes `"./hidden.mjs"` and resolves. The reference-source importer does not trim (`src/domain/referenceSourceImportParsing.ts:76-82`). So the two graphs disagree. PR #4510 closed on 2026-10-21 without merging. Likely area: `JavaScript path resolution`. 🤖 Written with Claude Code (Fable 5.1, verified by Opus 5.5), posted with my OK 🦊

bsakaguchi commented

2026-10-31 00:32:55 +00:00

Author

Owner

@morluto commented on GitHub (Oct 23, 2026):

Resolved by github.com/morluto/rea@80d4a74a06. Raw relative-versus-package classification occurs before URL decoding. CLI and stdio MCP controls check leading space and tab/newline/carriage-return specifiers against Node resolve-only oracles. Validation: tests/acceptance/applications/javascriptModuleResolutionSemantics.test.ts passed in the recorded round2/3 execution; this review did not rerun tests. Coverage is limited to the cited control; no broader real-provider, Windows or performance claim.

@morluto commented on GitHub (Oct 23, 2026): Resolved by https://github.com/morluto/rea/commit/80d4a74a06d1b6143e622763d040bc94b70242fa. Raw relative-versus-package classification occurs before URL decoding. CLI and stdio MCP controls check leading space and tab/newline/carriage-return specifiers against Node resolve-only oracles. Validation: `tests/acceptance/applications/javascriptModuleResolutionSemantics.test.ts` passed in the recorded round2/3 execution; this review did not rerun tests. Coverage is limited to the cited control; no broader real-provider, Windows or performance claim.

bsakaguchi referenced this issue

2026-10-31 00:55:25 +00:00

[PR #1799] [MERGED] fix(javascript): retain unbound CommonJS semantic imports #4824

bsakaguchi referenced this issue

2026-10-31 00:55:29 +00:00

[PR #1803] [CLOSED] fix(javascript): retain unbound require semantic module links #4827