morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4382] [Bug] Reference source graphs omit CommonJS dependencies written as static templates #1739

New issue

Closed

opened 2026-10-16 21:09:42 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-16 21:09:42 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4382

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

On both tested source baselines, require("mod") produces a parsed external requires relationship, while require(`mod`) produces no relationship, parse failure, or limitation. Template dynamic import() is retained as an unknown/partial <dynamic-import> relationship. Backtick specifiers in static ESM import/export declarations are invalid JavaScript and produce Babel parse failures; they are excluded from this report.

Reproduction

Prepare source checkouts at both baselines below and run npm ci in each. The main checkout includes TSX 4.23.15; the release checkout does not include a TSX runner. In the main checkout, run export REA_TSX="$PWD/node_modules/.bin/tsx" and retain that absolute runner path while testing either checkout. Save this complete source-boundary probe as repro.mts at each checkout root:

import assert from 'node:assert/strict';
import {pathToFileURL} from 'node:url';
const root=process.env.REA_CHECKOUT!;
const {parseReferenceSourceImports:parse}=await import(pathToFileURL(root+'/src/domain/referenceSourceImportParsing.ts').href);
const run=(s:string)=>parse('fixture.cjs',new TextEncoder().encode(s),'JavaScript');
for(const source of ['require("mod")','require(`mod`)','import("mod")','import(`mod`)','import(`./${name}.js`)','import x from `mod`;','export * from `mod`;']) console.log(JSON.stringify({source,result:run(source)}));
assert.equal(run('require("mod")').relationships[0]?.to,'mod');
assert.deepEqual(run('require(`mod`)'),{relationships:[],parse_failures:[],limitations:[]});
assert.equal(run('import(`mod`)').relationships[0]?.to,'<dynamic-import>');
assert.equal(run('import(`mod`)').relationships[0]?.parse_state,'partial');
assert.equal(run('import(`./${name}.js`)').relationships[0]?.resolution,'unknown');
assert.ok(run('import x from `mod`;').parse_failures.length);
assert.ok(run('export * from `mod`;').parse_failures.length);

From each checkout root, run REA_CHECKOUT="$PWD" "$REA_TSX" repro.mts under Node v24.18.0. I executed this probe once on each baseline using the main checkout’s TSX runner for both; both exited 0 with the asserted omission and controls. No module is evaluated or fetched. This executes the production parser directly, not the compiled CLI or MCP workflow.

Expected behavior

The supported historical-source CommonJS parser should retain a dependency for an unshadowed loader called with an exact no-substitution template, just as it does for the equivalent quoted string, or explicitly report the lost coverage. The parser advertises ESM/CommonJS dependency extraction; a template without substitutions supplies an exact string. This report requests restoration of the missing CommonJS dependency only.

Impact and scope

A valid CommonJS dependency silently disappears at the historical-source graph producer for this spelling. This is bounded evidence incompleteness. Frequency in real source trees and downstream reconstruction effects are unknown. The quoted spelling works in the probe, but rewriting an original reference artifact changes its source identity; no practical workaround preserving its bytes was verified.

REA version

Main source 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd; latest published rea-agents 6.5.0 source 3b79af28e80785afce9f5998d8f03289402a1847.

Runtime and platform

Node v24.18.0; macOS arm64; locked checkout dependencies; standalone TSX source probe. No native provider.

Target and artifact context

Synthetic fixture.cjs source bytes passed to reference-source import parsing; no application execution.

Interface

Other

Earliest failure stage

Artifact or evidence registration

Relevant output or evidence

The template-require result on both baselines is:

{"source":"require(`mod`)","result":{"relationships":[],"parse_failures":[],"limitations":[]}}

Parser and admission gates accept strings or calls in isModuleExpression, and literal extraction accepts only StringLiteral. The CommonJS extraction loop at lines 245–265 filters the no-substitution template out before emitting any edge. Its exported parser at lines 323–390 receives source bytes and collects these relationships. The .cjs fixture establishes an explicit CommonJS wrapper; the binding owner admits the unshadowed require call.

Source-inspected supported reachability: CLI import calls the historical-source workflow, which reads the selected tree and passes it to parseReferenceSourceEntries. The production consumer identifies JavaScript, passes its bytes to this parser and retains only returned relationships. The probe observes this producer boundary; no full CLI/MCP import, graph persistence, or downstream reconstruction was executed.

Historical-source documentation describes the full graph with file hashes, relationships and parse-failure diagnostics. Existing exact-literal semantics recognize expression-free templates as exact strings.

Typed outcome or recovery expected

With the same .cjs fixture, the template and quoted CommonJS forms should both retain to: "mod", kind: "requires", resolution: "external", and parse_state: "parsed", with no parse failure. Preserve valid quoted imports and the explicit unknown/partial relationship for the genuinely computed dynamic-import control; do not invent an exact target for it. Invalid static ESM template declarations must continue to report parse failures.

Analysis or suspected cause

Source-derived: the module-expression gate excludes template literals, so the CommonJS branch emits neither an edge nor a coverage diagnostic for the valid exact template. The quoted control rules out blanket rejection of the .cjs fixture or its loader binding.

Additional context

Closed #2841, #2599, and merged #2842 cover captured-browser bundles and source-map-original producers, not historical-source import. Their fix leaves this parser unchanged. The current module-reference view work projects already retained JavaScript application facts and does not parse historical-reference sources. This is a separate producer and restoration signal; no claim of a regression from those fixes is made.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A valid exact CommonJS dependency silently disappears at the historical-source graph producer while the quoted spelling is retained; observed scope is the parser boundary only.
Workaround status: unverified
Workaround basis: The quoted spelling works in the probe, but rewriting a reference artifact changes its source identity; no equivalent import preserving the original bytes was verified.

Originally created by @coygeek on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4382 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior On both tested source baselines, `require("mod")` produces a parsed external `requires` relationship, while ``require(`mod`)`` produces no relationship, parse failure, or limitation. Template dynamic `import()` is retained as an unknown/partial `<dynamic-import>` relationship. Backtick specifiers in static ESM import/export declarations are invalid JavaScript and produce Babel parse failures; they are excluded from this report. ### Reproduction Prepare source checkouts at both baselines below and run `npm ci` in each. The main checkout includes TSX 4.23.15; the release checkout does not include a TSX runner. In the main checkout, run `export REA_TSX="$PWD/node_modules/.bin/tsx"` and retain that absolute runner path while testing either checkout. Save this complete source-boundary probe as `repro.mts` at each checkout root: ```typescript import assert from 'node:assert/strict'; import {pathToFileURL} from 'node:url'; const root=process.env.REA_CHECKOUT!; const {parseReferenceSourceImports:parse}=await import(pathToFileURL(root+'/src/domain/referenceSourceImportParsing.ts').href); const run=(s:string)=>parse('fixture.cjs',new TextEncoder().encode(s),'JavaScript'); for(const source of ['require("mod")','require(`mod`)','import("mod")','import(`mod`)','import(`./${name}.js`)','import x from `mod`;','export * from `mod`;']) console.log(JSON.stringify({source,result:run(source)})); assert.equal(run('require("mod")').relationships[0]?.to,'mod'); assert.deepEqual(run('require(`mod`)'),{relationships:[],parse_failures:[],limitations:[]}); assert.equal(run('import(`mod`)').relationships[0]?.to,'<dynamic-import>'); assert.equal(run('import(`mod`)').relationships[0]?.parse_state,'partial'); assert.equal(run('import(`./${name}.js`)').relationships[0]?.resolution,'unknown'); assert.ok(run('import x from `mod`;').parse_failures.length); assert.ok(run('export * from `mod`;').parse_failures.length); ``` From each checkout root, run `REA_CHECKOUT="$PWD" "$REA_TSX" repro.mts` under Node v24.18.0. I executed this probe once on each baseline using the main checkout’s TSX runner for both; both exited 0 with the asserted omission and controls. No module is evaluated or fetched. This executes the production parser directly, not the compiled CLI or MCP workflow. ### Expected behavior The supported historical-source CommonJS parser should retain a dependency for an unshadowed loader called with an exact no-substitution template, just as it does for the equivalent quoted string, or explicitly report the lost coverage. The parser advertises ESM/CommonJS dependency extraction; a template without substitutions supplies an exact string. This report requests restoration of the missing CommonJS dependency only. ### Impact and scope A valid CommonJS dependency silently disappears at the historical-source graph producer for this spelling. This is bounded evidence incompleteness. Frequency in real source trees and downstream reconstruction effects are unknown. The quoted spelling works in the probe, but rewriting an original reference artifact changes its source identity; no practical workaround preserving its bytes was verified. ### REA version Main source `8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd`; latest published rea-agents 6.5.0 source `3b79af28e80785afce9f5998d8f03289402a1847`. ### Runtime and platform Node v24.18.0; macOS arm64; locked checkout dependencies; standalone TSX source probe. No native provider. ### Target and artifact context Synthetic `fixture.cjs` source bytes passed to reference-source import parsing; no application execution. ### Interface Other ### Earliest failure stage Artifact or evidence registration ### Relevant output or evidence The template-require result on both baselines is: ```json {"source":"require(`mod`)","result":{"relationships":[],"parse_failures":[],"limitations":[]}} ``` [Parser and admission gates](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/domain/referenceSourceImportParsing.ts#L70-L110) accept strings or calls in `isModuleExpression`, and literal extraction accepts only `StringLiteral`. The CommonJS extraction loop at lines 245–265 filters the no-substitution template out before emitting any edge. Its exported parser at lines 323–390 receives source bytes and collects these relationships. The `.cjs` fixture establishes an explicit CommonJS wrapper; the [binding owner](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/domain/referenceSourceRequireBindings.ts#L19-L80) admits the unshadowed `require` call. Source-inspected supported reachability: [CLI import](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/cli/utilityCommands.ts#L315-L336) calls [the historical-source workflow](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/application/ReferenceSourceImport.ts#L201-L252), which reads the selected tree and passes it to `parseReferenceSourceEntries`. [The production consumer](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/application/ReferenceSourceImportEntries.ts#L323-L366) identifies JavaScript, passes its bytes to this parser and retains only returned relationships. The probe observes this producer boundary; no full CLI/MCP import, graph persistence, or downstream reconstruction was executed. [Historical-source documentation](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/docs/cli.md#L234-L266) describes the full graph with file hashes, relationships and parse-failure diagnostics. [Existing exact-literal semantics](https://github.com/morluto/rea/blob/8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd/src/domain/javascript/javascriptAstValues.ts#L47-L69) recognize expression-free templates as exact strings. ### Typed outcome or recovery expected With the same `.cjs` fixture, the template and quoted CommonJS forms should both retain `to: "mod"`, `kind: "requires"`, `resolution: "external"`, and `parse_state: "parsed"`, with no parse failure. Preserve valid quoted imports and the explicit unknown/partial relationship for the genuinely computed dynamic-import control; do not invent an exact target for it. Invalid static ESM template declarations must continue to report parse failures. ### Analysis or suspected cause Source-derived: the module-expression gate excludes template literals, so the CommonJS branch emits neither an edge nor a coverage diagnostic for the valid exact template. The quoted control rules out blanket rejection of the `.cjs` fixture or its loader binding. ### Additional context Closed [#2841](https://github.com/morluto/rea/issues/2841), [#2599](https://github.com/morluto/rea/issues/2599), and merged [#2842](https://github.com/morluto/rea/pull/2842) cover captured-browser bundles and source-map-original producers, not historical-source import. Their fix leaves this parser unchanged. The current module-reference view work projects already retained JavaScript application facts and does not parse historical-reference sources. This is a separate producer and restoration signal; no claim of a regression from those fixes is made. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A valid exact CommonJS dependency silently disappears at the historical-source graph producer while the quoted spelling is retained; observed scope is the parser boundary only. - Workaround status: unverified - Workaround basis: The quoted spelling works in the probe, but rewriting a reference artifact changes its source identity; no equivalent import preserving the original bytes was verified.

bsakaguchi commented

2026-10-16 21:09:43 +00:00

Author

Owner

@hemv-857 commented on GitHub (Oct 16, 2026):

Claiming this one.

Root cause (source-inspected on main): in src/domain/referenceSourceImportParsing.ts, the require branch is gated by isModuleExpression (L85-88), which narrows to StringLiteral | CallExpression. A no-substitution require(`mod`) passes the callee and unshadowed-binding checks (referenceSourceRequireBindings.ts is arg-shape agnostic), then gets filtered at the L249-265 gate before moduleSpecifierFromExpression can run — so it exits with relationships: [], parse_failures: [], limitations: [], exactly the silent omission in the report. The extractor itself is also StringLiteral-only (L95-104), and safeModuleName (L69-72) likewise only reads StringLiteral (that path serves ImportExpression sources, which must keep the <dynamic-import> partial).

Approach: admit expression-free TemplateLiteral at the CommonJS require gate and extract its single cooked quasi, reusing the existing exact-literal semantics from readExactJavaScriptLiteral (src/domain/javascript/javascriptAstValues.ts), which already treats zero-expression templates as exact strings. Scope kept to the require path per the report: static ESM template declarations must keep their Babel parse failures, and dynamic import() (template or computed) keeps the explicit <dynamic-import> partial with no invented target.

Test: boundary workflow test (.cjs fixture, template + quoted controls) asserting to, kind: "requires", resolution: "external", parse_state: "parsed" for both spellings, with the computed dynamic-import control unchanged.

@hemv-857 commented on GitHub (Oct 16, 2026): Claiming this one. **Root cause (source-inspected on main):** in `src/domain/referenceSourceImportParsing.ts`, the require branch is gated by `isModuleExpression` (L85-88), which narrows to `StringLiteral | CallExpression`. A no-substitution `` require(`mod`) `` passes the callee and unshadowed-binding checks (`referenceSourceRequireBindings.ts` is arg-shape agnostic), then gets filtered at the L249-265 gate before `moduleSpecifierFromExpression` can run — so it exits with `relationships: [], parse_failures: [], limitations: []`, exactly the silent omission in the report. The extractor itself is also StringLiteral-only (L95-104), and `safeModuleName` (L69-72) likewise only reads StringLiteral (that path serves `ImportExpression` sources, which must keep the `<dynamic-import>` partial). **Approach:** admit expression-free `TemplateLiteral` at the CommonJS require gate and extract its single cooked quasi, reusing the existing exact-literal semantics from `readExactJavaScriptLiteral` (`src/domain/javascript/javascriptAstValues.ts`), which already treats zero-expression templates as exact strings. Scope kept to the require path per the report: static ESM template declarations must keep their Babel parse failures, and dynamic `import()` (template or computed) keeps the explicit `<dynamic-import>` partial with no invented target. **Test:** boundary workflow test (`.cjs` fixture, template + quoted controls) asserting `to`, `kind: "requires"`, `resolution: "external"`, `parse_state: "parsed"` for both spellings, with the computed dynamic-import control unchanged.

bsakaguchi commented

2026-10-16 21:09:43 +00:00

Author

Owner

@hemv-857 commented on GitHub (Oct 16, 2026):

Draft PR opened: #4511.

RED→GREEN: the new boundary test fails on the templated assertion alone on the base revision (quoted and all four issue controls pass), and passes with the gate + exact-literal extraction change. Direct parser assertions cover the typed outcome verbatim: template and quoted requires both return to: "mod", kind: "requires", resolution: "external", parse_state: "parsed"; computed require stays silent; import() template/computed stay <dynamic-import> partial with resolution: "unknown" for the computed form; static ESM template declarations still report parse failures. Six reference-source suites (26 tests) and npm run check:fast are clean; one stale expectation in referenceSourceComputedRequire.test.ts encoded the old omission and was updated for the now-correct edge.

@hemv-857 commented on GitHub (Oct 16, 2026): Draft PR opened: #4511. RED→GREEN: the new boundary test fails on the `templated` assertion alone on the base revision (quoted and all four issue controls pass), and passes with the gate + exact-literal extraction change. Direct parser assertions cover the typed outcome verbatim: template and quoted requires both return `to: "mod"`, `kind: "requires"`, `resolution: "external"`, `parse_state: "parsed"`; computed require stays silent; `import()` template/computed stay `<dynamic-import>` partial with `resolution: "unknown"` for the computed form; static ESM template declarations still report parse failures. Six reference-source suites (26 tests) and `npm run check:fast` are clean; one stale expectation in `referenceSourceComputedRequire.test.ts` encoded the old omission and was updated for the now-correct edge.

bsakaguchi closed this issue

2026-10-31 00:32:28 +00:00

bsakaguchi referenced this issue

2026-10-31 00:55:03 +00:00

[PR #1739] [MERGED] test(evidence): cover workflow question whitespace normalization #4786