[PR #7171] fix(javascript): keep parent segments that re-enter the synthetic URL root #7691

Open
opened 2026-11-03 13:39:40 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7171
Author: @devreddy1015
Created: 10/25/2026
Status: 🔄 Open

Base: main ← Head: fix/javascript-synthetic-root-reentry


📝 Commits (1)

  • 1c8d682 fix(javascript): keep parent segments that re-enter the synthetic URL root

📊 Changes

5 files changed (+116 additions, -10 deletions)

View changed files

📝 src/domain/javascript/javascriptSourceMapPaths.ts (+18 -4)
📝 src/domain/javascript/javascriptStaticAnalysisHelpers.ts (+14 -6)
➕ src/domain/javascript/javascriptSyntheticUrlRoot.ts (+45 -0)
📝 tests/acceptance/applications/createRequireFilenameBase.test.ts (+36 -0)
📝 tests/boundary/filesystem/sourceToBundlePaths.test.ts (+3 -0)

📄 Description

Summary

Relative URLs that climb above their base with .. and then re-enter a directory named source lose their parent segments in two places:

  • createRequire(new URL("../source/anchor.cjs", import.meta.url)) was reported as base anchor.cjs, so require("./dep.cjs") resolved to app/dep.cjs. Node loads source/dep.cjs.
  • A source-map source ../../source/a.js from maps.js/main.js.map was reported as the in-artifact path a.js instead of unresolved.

Cause and change

Both sites resolve under a synthetic root made of source/ segments, then call posix.relative. A reference that names source (or %73ource) re-enters that root, and its parent segments vanish. The new pathBelowSyntheticUrlRoot resolves under two differently named roots and keeps the result with more leading ... A reference cannot re-enter both, and the URL parser still does all resolution. Module URLs already have this guard (applyUrlReference in the application layer), but domain code cannot import it.

Evidence

  • A new acceptance case in createRequireFilenameBase.test.ts runs through CLI and stdio MCP with a real Node control. Two new rows are in sourceToBundlePaths.test.ts. All of them fail without the fix and pass with it.
  • src/domain/javascript and src/application/javascript: 4,805 tests pass. 319 of 326 related boundary and acceptance files pass. The other 7 fail locally for environment reasons: Node 26 instead of the pinned 24, and missing generated files. The Node SEA one also fails without this change. npm run check:fast passes.
  • Not run: Node 24, macOS, Windows.

Scope: a static analysis fix within existing boundaries, with no CLI/MCP contract, package or metadata change.

Author readiness

Human owner: @devreddy1015

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

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, 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/7171 **Author:** [@devreddy1015](https://github.com/devreddy1015) **Created:** 10/25/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/javascript-synthetic-root-reentry` --- ### 📝 Commits (1) - [`1c8d682`](https://github.com/morluto/rea/commit/1c8d682b1fe8de7e546c43342b4769b61be346eb) fix(javascript): keep parent segments that re-enter the synthetic URL root ### 📊 Changes **5 files changed** (+116 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSourceMapPaths.ts` (+18 -4) 📝 `src/domain/javascript/javascriptStaticAnalysisHelpers.ts` (+14 -6) ➕ `src/domain/javascript/javascriptSyntheticUrlRoot.ts` (+45 -0) 📝 `tests/acceptance/applications/createRequireFilenameBase.test.ts` (+36 -0) 📝 `tests/boundary/filesystem/sourceToBundlePaths.test.ts` (+3 -0) </details> ### 📄 Description ## Summary Relative URLs that climb above their base with `..` and then re-enter a directory named `source` lose their parent segments in two places: - `createRequire(new URL("../source/anchor.cjs", import.meta.url))` was reported as base `anchor.cjs`, so `require("./dep.cjs")` resolved to `app/dep.cjs`. Node loads `source/dep.cjs`. - A source-map source `../../source/a.js` from `maps.js/main.js.map` was reported as the in-artifact path `a.js` instead of unresolved. ## Cause and change Both sites resolve under a synthetic root made of `source/` segments, then call `posix.relative`. A reference that names `source` (or `%73ource`) re-enters that root, and its parent segments vanish. The new `pathBelowSyntheticUrlRoot` resolves under two differently named roots and keeps the result with more leading `..`. A reference cannot re-enter both, and the URL parser still does all resolution. Module URLs already have this guard (`applyUrlReference` in the application layer), but domain code cannot import it. ## Evidence - A new acceptance case in `createRequireFilenameBase.test.ts` runs through CLI and stdio MCP with a real Node control. Two new rows are in `sourceToBundlePaths.test.ts`. All of them fail without the fix and pass with it. - `src/domain/javascript` and `src/application/javascript`: 4,805 tests pass. 319 of 326 related boundary and acceptance files pass. The other 7 fail locally for environment reasons: Node 26 instead of the pinned 24, and missing generated files. The Node SEA one also fails without this change. `npm run check:fast` passes. - Not run: Node 24, macOS, Windows. Scope: a static analysis fix within existing boundaries, with no CLI/MCP contract, package or metadata change. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @devreddy1015 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [ ] 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, 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
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#7691
No description provided.