[PR #3514] [CLOSED] fix(javascript): preserve HTML URL path identity #5794

New issue

Closed

opened 2026-10-31 01:09:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:29 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: codex/fix-html-url-path-identity

📝 Commits (2)

7aaaba7 fix(javascript): preserve HTML URL path identity
d6510cf Merge branch 'main' of https://github.com/morluto/rea into codex/fix-html-url-path-identity

📊 Changes

4 files changed (+294 additions, -11 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+23 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+28 -11)
📝 src/application/javascript/JavaScriptHtmlBaseResolution.test.ts (+69 -0)
📝 tests/boundary/filesystem/javascript-applications/javascriptHtmlReferenceResolution.test.ts (+174 -0)

📄 Description

Status

Deferred by the author after rechecking current main: #3792 (90c6922) already handles repeated URL separators, including valid cases where dot segments remove an empty segment. This older patch would conservatively reject some of those now-resolved references. A remaining directory-form URL issue deserves a narrower follow-up on the current parser. The branch is retained; this PR is not being proposed for merge in its current form.

Summary

Prevent offline HTML reconstruction from linking a script URL to a different local file after filesystem normalization discards URL path information.

Problem and expected behavior

With renderer/local.js inventoried, this declaration incorrectly creates a loads edge to that file:

<script src="./local.js/."></script>

The URL resolves to /renderer/local.js/, a directory. Repeated separators also matter: assets//app.js must not select assets/app.js; a base such as assets// or /assets//../ must not silently choose a collapsed path. The existing URL-reference resolver already preserves these distinctions.

This is an independently reproduced residual HTML path-resolution defect. No separate issue was opened. The nearby #2940 handles omitted inline script bodies, a different boundary.

Change and scope

Reuse the existing directory-form check for root-relative, document-relative, and base-relative HTML references. Share the existing repeated-separator guard between HTML declarations, local bases, and URL references before canonical filesystem joining. Directory targets remain not-found; URL paths that cannot retain empty segments in canonical artifact names remain unavailable.

Contract and boundary impact

Semantic owner: JavaScriptArtifactPathResolution at HTML declaration/base path interpretation.
Scope: ordinary static, read-only correctness fix; no new provider or authority, advance scope decision not applicable.
CLI/MCP schemas and catalogs are unchanged. Graph output drops false loads links and retains original declarations, bases, and resolution limitations.
CommonJS, filesystem-expression, and ESM execution branches retain their current behavior; explicit filesystem repeated-separator controls pass.
No target execution, process mutation, network, package, installation, or generated metadata changes.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Source regression baseline on unmodified 92a0edf1875f74528910db461ed03f782b7b59b3: 11 failed, 202 passed across the two resolver files. After the fix: 213 passed.
Actual disk inventory → content verification → production HTML parser → in-process source analysis → validated graph probe on the same fixture: 5 loads before, including 3 wrong links; 2 correct loads after. The six unresolved declarations retain original script/base values and Evidence limitations. This probe does not cover the supervised worker, public CLI/MCP, or a live browser.
Two regressions in the existing filesystem reconstruction test assert only the correct ordinary URL loads, the target identities, and retained uncertainty/evidence for directory and empty-segment cases.
Independent production-resolver/WHATWG URL check: 169 base/reference combinations; all 45 resolved cases matched the URL pathname, with no false resolved match observed.

Final validation was repeated after fetching and merging main eab789aed74acd6ef7c15d509e0f3de350be2733, at head d6510cfe1a95dc93b0013670aba615d3085407ec. The contribution diff remains four files; the baseline reproduction above remains on the original unmodified 92a0edf producer.

Validation performed

Windows, Node 24.18.0/npm 11.16.0, locked dependencies:

npm run test:local -- src/application/javascript/JavaScriptHtmlBaseResolution.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts — 213 passed.
npm run check after syncing main and the final diff — all 6 tasks, module boundaries, and architecture guards passed; lint 0 errors, 1,384 warnings.
git diff --check — passed.
npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptHtmlReferenceResolution.test.ts — runtime preparation succeeded; exit 1, all 3 failed before semantic assertions because Windows native/windows/build/manifest.json is missing. This is not a passing boundary result. The new reconstruction regressions still need a supported CI host.

Compatibility, safety, and release

No schema breaking change, migration, or release metadata change. The fix preserves uncertainty instead of claiming a different file was loaded. Public CLI/MCP and Windows native-worker coverage remain unverified locally; no native mocks or authority workarounds were used.

Author readiness

Human owner: @ShuaixinHuang

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.
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 (no changes required).
Supported-host reconstruction boundary and required current-head CI checks have passed.
Human initial-readiness declaration is complete.

_{🔄 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/3514 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-html-url-path-identity` --- ### 📝 Commits (2) - [`7aaaba7`](https://github.com/morluto/rea/commit/7aaaba7e5d820902cd55c1c6ffe5d13e065aced5) fix(javascript): preserve HTML URL path identity - [`d6510cf`](https://github.com/morluto/rea/commit/d6510cfe1a95dc93b0013670aba615d3085407ec) Merge branch 'main' of https://github.com/morluto/rea into codex/fix-html-url-path-identity ### 📊 Changes **4 files changed** (+294 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+23 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+28 -11) 📝 `src/application/javascript/JavaScriptHtmlBaseResolution.test.ts` (+69 -0) 📝 `tests/boundary/filesystem/javascript-applications/javascriptHtmlReferenceResolution.test.ts` (+174 -0) </details> ### 📄 Description ## Status Deferred by the author after rechecking current main: #3792 (90c6922) already handles repeated URL separators, including valid cases where dot segments remove an empty segment. This older patch would conservatively reject some of those now-resolved references. A remaining directory-form URL issue deserves a narrower follow-up on the current parser. The branch is retained; this PR is not being proposed for merge in its current form. ## Summary Prevent offline HTML reconstruction from linking a script URL to a different local file after filesystem normalization discards URL path information. ## Problem and expected behavior With `renderer/local.js` inventoried, this declaration incorrectly creates a `loads` edge to that file: ```html <script src="./local.js/."></script> ``` The URL resolves to `/renderer/local.js/`, a directory. Repeated separators also matter: `assets//app.js` must not select `assets/app.js`; a base such as `assets//` or `/assets//../` must not silently choose a collapsed path. The existing URL-reference resolver already preserves these distinctions. This is an independently reproduced residual HTML path-resolution defect. No separate issue was opened. The nearby #2940 handles omitted inline script bodies, a different boundary. ## Change and scope Reuse the existing directory-form check for root-relative, document-relative, and base-relative HTML references. Share the existing repeated-separator guard between HTML declarations, local bases, and URL references before canonical filesystem joining. Directory targets remain `not-found`; URL paths that cannot retain empty segments in canonical artifact names remain `unavailable`. ## Contract and boundary impact - Semantic owner: `JavaScriptArtifactPathResolution` at HTML declaration/base path interpretation. - Scope: ordinary static, read-only correctness fix; no new provider or authority, advance scope decision not applicable. - CLI/MCP schemas and catalogs are unchanged. Graph output drops false `loads` links and retains original declarations, bases, and resolution limitations. - CommonJS, filesystem-expression, and ESM execution branches retain their current behavior; explicit filesystem repeated-separator controls pass. - No target execution, process mutation, network, package, installation, or generated metadata changes. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Source regression baseline on unmodified `92a0edf1875f74528910db461ed03f782b7b59b3`: **11 failed, 202 passed** across the two resolver files. After the fix: **213 passed**. - Actual disk inventory → content verification → production HTML parser → in-process source analysis → validated graph probe on the same fixture: **5 loads before, including 3 wrong links; 2 correct loads after**. The six unresolved declarations retain original script/base values and Evidence limitations. This probe does not cover the supervised worker, public CLI/MCP, or a live browser. - Two regressions in the existing filesystem reconstruction test assert only the correct ordinary URL loads, the target identities, and retained uncertainty/evidence for directory and empty-segment cases. - Independent production-resolver/WHATWG `URL` check: **169 base/reference combinations; all 45 resolved cases matched the URL pathname**, with no false resolved match observed. Final validation was repeated after fetching and merging main `eab789aed74acd6ef7c15d509e0f3de350be2733`, at head `d6510cfe1a95dc93b0013670aba615d3085407ec`. The contribution diff remains four files; the baseline reproduction above remains on the original unmodified `92a0edf` producer. ## Validation performed Windows, Node 24.18.0/npm 11.16.0, locked dependencies: - `npm run test:local -- src/application/javascript/JavaScriptHtmlBaseResolution.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts` — **213 passed**. - `npm run check` after syncing main and the final diff — **all 6 tasks, module boundaries, and architecture guards passed**; lint **0 errors, 1,384 warnings**. - `git diff --check` — passed. - `npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptHtmlReferenceResolution.test.ts` — runtime preparation succeeded; **exit 1, all 3 failed before semantic assertions** because Windows `native/windows/build/manifest.json` is missing. This is not a passing boundary result. The new reconstruction regressions still need a supported CI host. ## Compatibility, safety, and release No schema breaking change, migration, or release metadata change. The fix preserves uncertainty instead of claiming a different file was loaded. Public CLI/MCP and Windows native-worker coverage remain unverified locally; no native mocks or authority workarounds were used. ## Author readiness Human owner: @ShuaixinHuang - [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 - [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 (no changes required). - [ ] Supported-host reconstruction boundary and required current-head CI checks have passed. - [x] Human initial-readiness declaration is complete. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:29 +00:00