[PR #1916] [MERGED] fix(javascript): normalize package URLs before array selection #4907

Closed
opened 2026-10-31 00:56:39 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: codex/package-target-url-fallback


📝 Commits (1)

  • 04c9a59 fix(javascript): normalize package URLs before array selection

📊 Changes

5 files changed (+318 additions, -33 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+6 -2)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+101 -28)
📝 tests/acceptance/applications/packageImportAliasesCli.test.ts (+34 -3)
📝 tests/boundary/javascript/packageConditionalExports.test.ts (+45 -0)
📝 tests/boundary/javascript/packageImportAliases.test.ts (+132 -0)

📄 Description

Summary

Package imports and exports now normalize each relative target as a URL before deciding whether an array entry escapes the package root. A target such as ["./.\t./outside.cjs", "./fallback.cjs"] selects fallback.cjs, matching Node, instead of losing the edge as not-found.

Follow-up to #1862 for its final review finding, posted after that PR merged.

Problem and expected behavior

The selector checked raw path segments, then committed to a string before URL normalization. Node removes tabs and newlines while constructing the URL, validates package-root containment, and skips an invalid target while array fallback is still available. Decoding and file lookup failures happen after selection and must retain the first selected target's failure.

Change and scope

The shared application selector retains the original target and its normalized URL. Imports and exports use that URL for containment and lookup, with inventory path segments encoded when constructing the package base. This also preserves package filenames containing spaces, Unicode, and URL punctuation. Failed lookups retain the original target and metadata path in their limitations.

CommonJS validates encoded separators across the selected serialized URL; ESM validates its pathname. Those errors and missing files do not select a later array entry. Wildcard expansion, recursive aliases, and external-package import targets keep their existing scope.

Contract and boundary impact

  • Semantic owner and earliest changed stage: JavaScriptArtifactPathResolution target selection, before array fallback is decided.
  • CLI and MCP/tool-catalog contract: existing graph and Evidence fields; no schema or tool-name changes.
  • Provider, bridge, target-format, or platform compatibility: static JavaScript artifact analysis; no provider or bridge changes.
  • Evidence, artifact, provenance, or reconstruction contract: retain the original declaration, target text, metadata path, importing source range, and digest; normalize only the derived lookup URL.
  • Process execution, authorization, cleanup, or containment impact: no new runtime execution; package-root containment is checked before selection and artifact/container confinement remains enforced.
  • Generated metadata, package, or installation impact: none; generated catalogs and build outputs remain uncommitted.

Evidence and regression coverage

  • Tests added or updated: Node 24.18 require/import comparisons for tab/CR/LF escapes, normalized filenames, trailing whitespace, query/fragment suffixes, package-path punctuation, and targets that normalize back inside their package. Refusal cases cover encoded separators, missing files, and retained raw target diagnostics.
  • Base reproduction or other evidence: added regressions failed before the fix on the #1862 branch; Node selected a fallback or normalized file where REA returned not-found.
  • User-visible CLI/MCP output: both compiled surfaces retain #tab-escape and #newline-escape edges to config.cjs; missing normalized targets retain not-found and their original target text.
  • Remaining proof gaps: native Windows execution was not run locally. Provider-engine verification does not apply to this static resolver.

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

  • npm ci — passed with the pinned Node 24.18.0/npm 11.16.0 toolchain.
  • npm run test:focused -- src/domain/javascript/javascriptPackageImportAliases.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts tests/boundary/javascript/packageImportAliases.test.ts tests/boundary/javascript/packageConditionalExports.test.ts tests/acceptance/applications/packageImportAliasesCli.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts tests/acceptance/applications/packageJsonImports.test.ts — 225 tests passed across seven files on current main plus this commit, including compiled CLI/stdio MCP journeys.
  • npm run check — passed, including architecture guards; zero lint errors.
  • npm run docs:check — passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; previously missed valid package targets now resolve with Node's URL normalization and fallback order.
  • Real Hopper/Ghidra, browser, or OS coverage: Node resolver oracles and public CLI/MCP workflows ran on macOS arm64. No real-provider behavior changes are claimed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no network access, installation, or target execution is added to analysis; selected targets remain confined to the owning package and inventoried container.

Review checklist

  • This PR addresses a concrete problem or an agreed enhancement.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported 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/1916 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/package-target-url-fallback` --- ### 📝 Commits (1) - [`04c9a59`](https://github.com/morluto/rea/commit/04c9a593434597804406245cb28847fb6c7896d9) fix(javascript): normalize package URLs before array selection ### 📊 Changes **5 files changed** (+318 additions, -33 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+6 -2) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+101 -28) 📝 `tests/acceptance/applications/packageImportAliasesCli.test.ts` (+34 -3) 📝 `tests/boundary/javascript/packageConditionalExports.test.ts` (+45 -0) 📝 `tests/boundary/javascript/packageImportAliases.test.ts` (+132 -0) </details> ### 📄 Description ## Summary Package imports and exports now normalize each relative target as a URL before deciding whether an array entry escapes the package root. A target such as `["./.\t./outside.cjs", "./fallback.cjs"]` selects `fallback.cjs`, matching Node, instead of losing the edge as `not-found`. Follow-up to #1862 for [its final review finding](https://github.com/morluto/rea/pull/1862#discussion_r4242481167), posted after that PR merged. ## Problem and expected behavior The selector checked raw path segments, then committed to a string before URL normalization. Node removes tabs and newlines while constructing the URL, validates package-root containment, and skips an invalid target while array fallback is still available. Decoding and file lookup failures happen after selection and must retain the first selected target's failure. ## Change and scope The shared application selector retains the original target and its normalized URL. Imports and exports use that URL for containment and lookup, with inventory path segments encoded when constructing the package base. This also preserves package filenames containing spaces, Unicode, and URL punctuation. Failed lookups retain the original target and metadata path in their limitations. CommonJS validates encoded separators across the selected serialized URL; ESM validates its pathname. Those errors and missing files do not select a later array entry. Wildcard expansion, recursive aliases, and external-package import targets keep their existing scope. ## Contract and boundary impact - Semantic owner and earliest changed stage: `JavaScriptArtifactPathResolution` target selection, before array fallback is decided. - CLI and MCP/tool-catalog contract: existing graph and Evidence fields; no schema or tool-name changes. - Provider, bridge, target-format, or platform compatibility: static JavaScript artifact analysis; no provider or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: retain the original declaration, target text, metadata path, importing source range, and digest; normalize only the derived lookup URL. - Process execution, authorization, cleanup, or containment impact: no new runtime execution; package-root containment is checked before selection and artifact/container confinement remains enforced. - Generated metadata, package, or installation impact: none; generated catalogs and build outputs remain uncommitted. ## Evidence and regression coverage - Tests added or updated: Node 24.18 require/import comparisons for tab/CR/LF escapes, normalized filenames, trailing whitespace, query/fragment suffixes, package-path punctuation, and targets that normalize back inside their package. Refusal cases cover encoded separators, missing files, and retained raw target diagnostics. - Base reproduction or other evidence: added regressions failed before the fix on the #1862 branch; Node selected a fallback or normalized file where REA returned `not-found`. - User-visible CLI/MCP output: both compiled surfaces retain `#tab-escape` and `#newline-escape` edges to `config.cjs`; missing normalized targets retain `not-found` and their original target text. - Remaining proof gaps: native Windows execution was not run locally. Provider-engine verification does not apply to this static resolver. 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 - `npm ci` — passed with the pinned Node 24.18.0/npm 11.16.0 toolchain. - `npm run test:focused -- src/domain/javascript/javascriptPackageImportAliases.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts tests/boundary/javascript/packageImportAliases.test.ts tests/boundary/javascript/packageConditionalExports.test.ts tests/acceptance/applications/packageImportAliasesCli.test.ts tests/acceptance/applications/packageSubpathsCli.test.ts tests/acceptance/applications/packageJsonImports.test.ts` — 225 tests passed across seven files on current main plus this commit, including compiled CLI/stdio MCP journeys. - `npm run check` — passed, including architecture guards; zero lint errors. - `npm run docs:check` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; previously missed valid package targets now resolve with Node's URL normalization and fallback order. - Real Hopper/Ghidra, browser, or OS coverage: Node resolver oracles and public CLI/MCP workflows ran on macOS arm64. No real-provider behavior changes are claimed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no network access, installation, or target execution is added to analysis; selected targets remain confined to the owning package and inventoried container. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported 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:39 +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#4907
No description provided.