[PR #576] [MERGED] fix(javascript): preserve literal CommonJS path punctuation #2077

New issue

Closed

opened 2026-10-16 21:13:59 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:59 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/576
Author: @kaoru0822-kitauji
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: fix/javascript-require-literal-paths

📝 Commits (1)

315c07c fix(javascript): preserve literal CommonJS path punctuation

📊 Changes

8 files changed (+290 additions, -7 deletions)

View changed files

📝 src/application/ElectronBoundaryAnalysis.ts (+3 -0)
📝 src/application/ElectronBoundaryGraphNative.ts (+1 -0)
📝 src/application/JavaScriptArtifactPathResolution.ts (+2 -1)
➕ src/application/JavaScriptModuleSpecifierPaths.test.ts (+77 -0)
➕ src/domain/electronNativeLoaderPaths.test.ts (+51 -0)
📝 src/domain/electronStaticAnalysisNative.ts (+22 -6)
📝 src/domain/electronStaticAnalysisTypes.ts (+1 -0)
➕ tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts (+133 -0)

📄 Description

Summary

Preserve literal # and ? characters in CommonJS filesystem specifiers. REA currently resolves require("./dep.cjs#literal.cjs") to dep.cjs when both files exist, producing confidently wrong graph targets. Node resolves the literal filename.

Found during a repository audit; no matching existing issue was found.

Change and scope

Use literal path handling for known require calls.
Preserve existing URL-suffix handling for ESM imports, HTML references, and callers without a loader kind.
Retain import/require provenance in internal native-addon findings, including both kinds of re-export.
Use that provenance consistently for native-file classification, graph target resolution, and resolved-binding counts.
Keep path-rejection and artifact-container checks intact.

The loader-origin field is internal. Public input/output schemas and graph property shapes remain unchanged, confirmed by the generated-contract checks.

Evidence and regression coverage

The baseline has 13 failures and 14 passing controls across the new tests:

A host-independent matrix covers #/?, require/import/unspecified kinds, and literal-only/stripped-only/both-file inventories.
HTML controls preserve URL suffix handling.
A real-file application fixture checks Node require.resolve and both public require-edge projections, alongside unchanged ESM targets.
Inert native fixtures cover direct imports, requires, CommonJS/ESM re-exports, literal-only/stripped-only/both layouts, and graph/count agreement.
Native classification distinguishes a literal JavaScript filename from an ESM URL pointing at a .node path.

The real-file fixtures use # for portable names; ? is covered by the filesystem-independent matrix. Native addons are never loaded or executed.

Independent review found no blocking issue and passed 76 additional assertions covering repeated/mixed/trailing punctuation, escaped literals, export variants, and rejection safeguards.

Contract and boundary impact

Semantic owner: context-sensitive application path resolution and native-binding AST provenance.
CLI/MCP schemas and tool inventory: unchanged.
Evidence: corrected target paths and binding counts flow through the existing authenticated graph workflow.
Provider support, execution, authorization, filesystem containment, and cleanup: unchanged.
Generated metadata and dependencies: unchanged.

Validation performed

Focused path, native-provenance, public application, and Electron suites: 31 tests passed.
npm run check:fast: passed.
npm run check:ci: all seven static tasks passed.
npm run docs:check: passed; generated contracts remain current.
npm run verify:package: Linux package/install, CLI, and MCP verifier passed.
npm pack --dry-run --json: passed.
git diff --check: passed.
npm run check:pr: static stages passed; full suite reported 1,816 passed, 40 failed, and 1 skipped. The same 40 native bridge failures reproduce on unchanged base 52bf709; this environment rejects Unix-domain socket listeners with EPERM, causing startup failures/timeouts. No tests or permission checks were relaxed.

Compatibility and release

No public schema migration. Node v24.19.0 on Linux supplied the real resolution oracle. Native fixtures contain inert bytes and make no native execution claim. Real Hopper/Ghidra verification was not performed because those providers are unchanged.

Review checklist

One focused Conventional Commit outcome.
Baseline failure and paired semantic controls.
Both native graph and count consumers updated.
Internal-only provenance and unchanged public schemas verified.
Independent review completed.
Final diff checked for unrelated changes 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/576 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/javascript-require-literal-paths` --- ### 📝 Commits (1) - [`315c07c`](https://github.com/morluto/rea/commit/315c07c91e18de53e571b1ed50ac85867fa8ea39) fix(javascript): preserve literal CommonJS path punctuation ### 📊 Changes **8 files changed** (+290 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ElectronBoundaryAnalysis.ts` (+3 -0) 📝 `src/application/ElectronBoundaryGraphNative.ts` (+1 -0) 📝 `src/application/JavaScriptArtifactPathResolution.ts` (+2 -1) ➕ `src/application/JavaScriptModuleSpecifierPaths.test.ts` (+77 -0) ➕ `src/domain/electronNativeLoaderPaths.test.ts` (+51 -0) 📝 `src/domain/electronStaticAnalysisNative.ts` (+22 -6) 📝 `src/domain/electronStaticAnalysisTypes.ts` (+1 -0) ➕ `tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts` (+133 -0) </details> ### 📄 Description ## Summary Preserve literal `#` and `?` characters in CommonJS filesystem specifiers. REA currently resolves `require("./dep.cjs#literal.cjs")` to `dep.cjs` when both files exist, producing confidently wrong graph targets. Node resolves the literal filename. Found during a repository audit; no matching existing issue was found. ## Change and scope - Use literal path handling for known `require` calls. - Preserve existing URL-suffix handling for ESM imports, HTML references, and callers without a loader kind. - Retain import/require provenance in internal native-addon findings, including both kinds of re-export. - Use that provenance consistently for native-file classification, graph target resolution, and resolved-binding counts. - Keep path-rejection and artifact-container checks intact. The loader-origin field is internal. Public input/output schemas and graph property shapes remain unchanged, confirmed by the generated-contract checks. ## Evidence and regression coverage The baseline has 13 failures and 14 passing controls across the new tests: - A host-independent matrix covers `#`/`?`, require/import/unspecified kinds, and literal-only/stripped-only/both-file inventories. - HTML controls preserve URL suffix handling. - A real-file application fixture checks Node `require.resolve` and both public require-edge projections, alongside unchanged ESM targets. - Inert native fixtures cover direct imports, requires, CommonJS/ESM re-exports, literal-only/stripped-only/both layouts, and graph/count agreement. - Native classification distinguishes a literal JavaScript filename from an ESM URL pointing at a `.node` path. The real-file fixtures use `#` for portable names; `?` is covered by the filesystem-independent matrix. Native addons are never loaded or executed. Independent review found no blocking issue and passed 76 additional assertions covering repeated/mixed/trailing punctuation, escaped literals, export variants, and rejection safeguards. ## Contract and boundary impact - Semantic owner: context-sensitive application path resolution and native-binding AST provenance. - CLI/MCP schemas and tool inventory: unchanged. - Evidence: corrected target paths and binding counts flow through the existing authenticated graph workflow. - Provider support, execution, authorization, filesystem containment, and cleanup: unchanged. - Generated metadata and dependencies: unchanged. ## Validation performed - Focused path, native-provenance, public application, and Electron suites: 31 tests passed. - `npm run check:fast`: passed. - `npm run check:ci`: all seven static tasks passed. - `npm run docs:check`: passed; generated contracts remain current. - `npm run verify:package`: Linux package/install, CLI, and MCP verifier passed. - `npm pack --dry-run --json`: passed. - `git diff --check`: passed. - `npm run check:pr`: static stages passed; full suite reported 1,816 passed, 40 failed, and 1 skipped. The same 40 native bridge failures reproduce on unchanged base `52bf709`; this environment rejects Unix-domain socket listeners with `EPERM`, causing startup failures/timeouts. No tests or permission checks were relaxed. ## Compatibility and release No public schema migration. Node v24.19.0 on Linux supplied the real resolution oracle. Native fixtures contain inert bytes and make no native execution claim. Real Hopper/Ghidra verification was not performed because those providers are unchanged. ## Review checklist - [x] One focused Conventional Commit outcome. - [x] Baseline failure and paired semantic controls. - [x] Both native graph and count consumers updated. - [x] Internal-only provenance and unchanged public schemas verified. - [x] Independent review completed. - [x] Final diff checked for unrelated changes 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-16 21:13:59 +00:00