[PR #2061] [MERGED] test(javascript): verify CommonJS file URL boundaries via CLI and MCP #4996

New issue

Closed

opened 2026-10-31 00:57:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:57:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2061
Author: @xu91102
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @baron

Base: main ← Head: fix/commonjs-file-url-specifiers

📝 Commits (5)

b8aff06 fix(javascript): keep CommonJS file URL strings unresolved
0369f1c test(javascript): verify CommonJS file URL boundaries via CLI and MCP
1c87db9 test(mcp): align completion checks with warm provider state
ee339d1 test(javascript): remove superseded file URL resolver coverage
796e338 chore: merge upstream completion fixture fixes

📊 Changes

2 files changed (+52 additions, -22 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.test.ts (+0 -21)
📝 tests/acceptance/applications/packageSubpathsCli.test.ts (+52 -1)

📄 Description

Summary

Add public acceptance coverage for CommonJS file-URL rejection, valid relative require paths, and ESM file-URL resolution in the existing CLI/MCP workflow.

Refs #2057

Problem and expected behavior

Node rejects require("file:///dep.cjs") with MODULE_NOT_FOUND even when the URI names an existing fixture file. The analyzer must retain that unresolved boundary, while require("./dep.cjs") loads 42 and an ESM file URL resolves inside the artifact.

Change and scope

The original production fix is already in main through e4532b5376. Merge pinned main 621cee1 and retain its resolver. This PR contributes public regression coverage in packageSubpathsCli.test.ts, removes the superseded resolver unit case with the same two outcomes, with no production changes relative to pinned main 621cee1.

CI baseline: the prior failures were independently reproduced on c22111e. Upstream commit 621cee192d (PR #2137) now fixes the same warm-only completion fixture expectations. Merge that commit and retain its exact test/verifier implementation. The final contribution contains no completion-fixture or package-verifier changes relative to pinned main 621cee1.

Contract and boundary impact

Semantic owner and earliest changed stage: Existing packageSubpaths CLI/MCP acceptance workflow; no production modification relative to main.
CLI and MCP/tool-catalog contract: public schemas and operations unchanged.
Provider, bridge, target-format, or platform compatibility: local source analysis; no provider or bridge changes.
Evidence, artifact, provenance, or reconstruction contract: original source bytes, SHA, declarations and ranges preserved. Private worker schemas remain strict.
Process execution, authorization, cleanup, or containment impact: none in production. Native controls use owned fixtures and repository temporary-directory cleanup.
Generated metadata, package, or installation impact: none; no generated artifacts committed.

Evidence and regression coverage

Tests updated: Reuse the existing two CLI/MCP workflows. Native createRequire rejects both the logical file URI and a URI naming an existing fixture file; relative resolution/execution returns 42. Assert semantic and legacy unresolved URI relationships, an ESM URI positive control, and original declared strings, source SHA and ranges. Remove only the older CommonJS-external/ESM-resolved module test now covered by these public workflows; retain other resolver boundary tests.
Executed evidence: The original issue fixture was replayed twice through the compiled CLI, alongside native URI/relative controls. Four old-head ADB/CLI-option failures were reproduced; those files pass on current main and this updated branch.
User-visible CLI/MCP output: The CommonJS file-URI relationship reports external/null and its legacy reference has resolved_path null. Equivalent relative require and the ESM file-URL control remain resolved.
Remaining proof gaps: Actual execution covers Node 24.18.0 on macOS arm64, compiled CLI and real stdio MCP. Native binary providers/live browsers are outside this source-analysis change.
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 run test:focused with 9 selected files — 185 tests passed. Includes the existing public CLI/MCP workflow, resolver/provider/cancellation/worker boundaries, and readiness checks.
npm run check — typecheck, lint, format, knip, metadata and architecture guards passed; 0 lint errors. Existing lint warnings remain.
npm run verify:test-discovery — all 915 test files discovered exactly once.
Original issue fixture — two compiled-CLI replays and real Node URI/relative controls passed, preserving original source SHA.
Independent frozen-contribution review — Standards and Spec passed.
Joint integration of all three final contributions — automatic source merge, 42 selected files / 821 tests, npm run check and test discovery passed. The disposable integration checkout was restored clean.
Upstream CI: PASS (completed successfully) for 796e33860d34dac6f270a49107ec13fe28cf2b1f. Earlier successful runs are not substituted for this head.

Compatibility, safety, and release

Breaking changes or migrations: none.
Real Hopper/Ghidra, browser, or OS coverage: Node/TypeScript source workflows executed on macOS; binary-provider/live-browser execution not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment impact: no new authorization, host access, target execution or containment behavior in production.

Review checklist

Concrete problem, one focused outcome and conventional title.
Related issue linked above.
Observable behavior and meaningful failure paths covered.
Owning contracts and metadata checked; no public schema or metadata update required.
Representative CLI/MCP outcomes included.
Final diff checked 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/2061 **Author:** [@xu91102](https://github.com/xu91102) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/commonjs-file-url-specifiers` --- ### 📝 Commits (5) - [`b8aff06`](https://github.com/morluto/rea/commit/b8aff0672331eb3d9b770bc4f1b801fc31d52263) fix(javascript): keep CommonJS file URL strings unresolved - [`0369f1c`](https://github.com/morluto/rea/commit/0369f1c1c9d4ce61a3d8c06a3801133dcd2e400f) test(javascript): verify CommonJS file URL boundaries via CLI and MCP - [`1c87db9`](https://github.com/morluto/rea/commit/1c87db913a2158e30fa4a96c838eabf52d471489) test(mcp): align completion checks with warm provider state - [`ee339d1`](https://github.com/morluto/rea/commit/ee339d13009acfa5974b77ccaa1a87fbeb3158a7) test(javascript): remove superseded file URL resolver coverage - [`796e338`](https://github.com/morluto/rea/commit/796e33860d34dac6f270a49107ec13fe28cf2b1f) chore: merge upstream completion fixture fixes ### 📊 Changes **2 files changed** (+52 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.test.ts` (+0 -21) 📝 `tests/acceptance/applications/packageSubpathsCli.test.ts` (+52 -1) </details> ### 📄 Description ## Summary Add public acceptance coverage for CommonJS file-URL rejection, valid relative require paths, and ESM file-URL resolution in the existing CLI/MCP workflow. Refs #2057 ## Problem and expected behavior Node rejects require("file:///dep.cjs") with MODULE_NOT_FOUND even when the URI names an existing fixture file. The analyzer must retain that unresolved boundary, while require("./dep.cjs") loads 42 and an ESM file URL resolves inside the artifact. ## Change and scope The original production fix is already in main through e4532b537663026c3e5b784380eecd009c959a36. Merge pinned main 621cee1 and retain its resolver. This PR contributes public regression coverage in packageSubpathsCli.test.ts, removes the superseded resolver unit case with the same two outcomes, with no production changes relative to pinned main 621cee1. CI baseline: the prior failures were independently reproduced on c22111e. Upstream commit 621cee192d582c87ca3d254d4cb22c9d209d3aea (PR #2137) now fixes the same warm-only completion fixture expectations. Merge that commit and retain its exact test/verifier implementation. The final contribution contains no completion-fixture or package-verifier changes relative to pinned main 621cee1. ## Contract and boundary impact - Semantic owner and earliest changed stage: Existing packageSubpaths CLI/MCP acceptance workflow; no production modification relative to main. - CLI and MCP/tool-catalog contract: public schemas and operations unchanged. - Provider, bridge, target-format, or platform compatibility: local source analysis; no provider or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: original source bytes, SHA, declarations and ranges preserved. Private worker schemas remain strict. - Process execution, authorization, cleanup, or containment impact: none in production. Native controls use owned fixtures and repository temporary-directory cleanup. - Generated metadata, package, or installation impact: none; no generated artifacts committed. ## Evidence and regression coverage - Tests updated: Reuse the existing two CLI/MCP workflows. Native createRequire rejects both the logical file URI and a URI naming an existing fixture file; relative resolution/execution returns 42. Assert semantic and legacy unresolved URI relationships, an ESM URI positive control, and original declared strings, source SHA and ranges. Remove only the older CommonJS-external/ESM-resolved module test now covered by these public workflows; retain other resolver boundary tests. - Executed evidence: The original issue fixture was replayed twice through the compiled CLI, alongside native URI/relative controls. Four old-head ADB/CLI-option failures were reproduced; those files pass on current main and this updated branch. - User-visible CLI/MCP output: The CommonJS file-URI relationship reports external/null and its legacy reference has resolved_path null. Equivalent relative require and the ESM file-URL control remain resolved. - Remaining proof gaps: Actual execution covers Node 24.18.0 on macOS arm64, compiled CLI and real stdio MCP. Native binary providers/live browsers are outside this source-analysis change. - [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 run test:focused` with 9 selected files — 185 tests passed. Includes the existing public CLI/MCP workflow, resolver/provider/cancellation/worker boundaries, and readiness checks. - `npm run check` — typecheck, lint, format, knip, metadata and architecture guards passed; 0 lint errors. Existing lint warnings remain. - `npm run verify:test-discovery` — all 915 test files discovered exactly once. - Original issue fixture — two compiled-CLI replays and real Node URI/relative controls passed, preserving original source SHA. - Independent frozen-contribution review — Standards and Spec passed. - Joint integration of all three final contributions — automatic source merge, 42 selected files / 821 tests, `npm run check` and test discovery passed. The disposable integration checkout was restored clean. - Upstream CI: [PASS (completed successfully)](https://github.com/morluto/rea/actions/runs/38238297682) for `796e33860d34dac6f270a49107ec13fe28cf2b1f`. Earlier successful runs are not substituted for this head. ## Compatibility, safety, and release - Breaking changes or migrations: none. - Real Hopper/Ghidra, browser, or OS coverage: Node/TypeScript source workflows executed on macOS; binary-provider/live-browser execution not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment impact: no new authorization, host access, target execution or containment behavior in production. ## Review checklist - [x] Concrete problem, one focused outcome and conventional title. - [x] Related issue linked above. - [x] Observable behavior and meaningful failure paths covered. - [x] Owning contracts and metadata checked; no public schema or metadata update required. - [x] Representative CLI/MCP outcomes included. - [x] Final diff checked 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:57:49 +00:00