morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

index.ts instead of dep.js #1514

New issue

Closed

opened 2026-10-16 21:06:26 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-16 21:06:26 +00:00

Owner

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

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

rea import-reference-source records require("./dep") in a CommonJS file as an internal dependency on dep/index.ts when both dep.js and dep/index.ts exist. Node resolves the same call to dep.js. The CLI exits successfully with no parse failures, so the saved historical graph points comparison and investigation at code the source does not load. This is a remaining case of closed #3381: its extensionless dep/index fixture now passes, but a directory index with a resolvable extension still outranks the sibling file.

The import exits 0 and records {"from_path":"main.cjs","to":"dep/index.ts","kind":"requires","resolution":"internal","parse_state":"parsed"} with an empty parse_failures array, while Node resolves dep.js. Removing only dep/index.ts changes the relationship to dep.js.

Reproduction

In an empty directory, create the fixture, import it, and run Node's resolve-only control (no fixture file is executed):

mkdir -p source/dep home
printf 'require("./dep");\n' > source/main.cjs
printf 'module.exports = "file";\n' > source/dep.js
printf 'export const wrong = "index";\n' > source/dep/index.ts
HOME="$PWD/home" rea import-reference-source "$PWD/source" --json > graph.json
echo "exit=$?"
python3 -c 'import json; g = json.load(open("graph.json")); print(json.dumps(g["relationships"])); print(g["parse_failures"])'
node -e 'const {createRequire}=require("node:module");const p=require("node:path");const r=p.resolve("source");console.log("Node target:",p.relative(r,createRequire(p.join(r,"main.cjs")).resolve("./dep")))'

Remove only source/dep/index.ts and rerun the import as a control.

Expected behavior

The relationship from main.cjs resolves to dep.js, the file Node loads for require("./dep"), whether or not dep/index.ts is present. The file-before-directory order established by #3381 applies to every directory index candidate, not only the extensionless one.

Impact and scope

CommonJS source trees containing a sibling JavaScript file and a competing TypeScript directory index get a false internal dependency in the committed historical graph, which can direct comparison toward code the source does not load; the inventory itself is intact.

Workaround: available. Node's resolve-only createRequire(...).resolve control shown above identifies the correct target without executing or changing the source, though it does not repair the saved graph.

Reporter's impact estimate: P2 (supported).

REA version

6.5.0, built from source at main commit 034342ba28 (latest npm release is 6.5.0)

Runtime and platform

Node.js v26.10.0; macOS 27.0.1 arm64

Target and artifact context

Local CommonJS source directory imported as historical source

Interface

CLI

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Expected source: closed #3381 (file before directory index for CommonJS require); tests/boundary/cli/referenceSourceDirectoryImports.test.ts:93-97 (fixture meanings are established with Node's resolve-only loader); docs/cli.md:227-242 "Import historical source"
Failure source: observed import-reference-source --json relationships for the fixture above, against Node's createRequire(...).resolve control on the same tree
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

exit=0
[{"from_path": "main.cjs", "to": "dep/index.ts", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}]
[]
Node target: dep.js

Control with dep/index.ts removed:

[{"from_path": "main.cjs", "to": "dep.js", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}]

Source-derived reasoning: src/application/ReferenceSourceImportEntries.ts:199-202 builds candidates by interleaving, for each suffix in RESOLUTION_SUFFIXES (line 140: "", ".ts", ".js", ...), the file candidate and the directory index candidate, so dep/index.ts (suffix .ts) precedes dep.js (suffix .js). src/domain/referenceSourcePathIdentity.ts:29-31 returns the first present candidate, and lines 213-218 of the importer record it as the internal target.

Typed outcome or recovery expected

The historical graph should record an internal requires relationship from main.cjs to dep.js, the file Node resolves, regardless of a competing dep/index.ts.

Restoration check:

On the fixture above, the relationship currently prints "to": "dep/index.ts"; after a fix it prints "to": "dep.js" with resolution: "internal", matching the Node control. Control: with dep/index.ts removed, the relationship still prints "to": "dep.js", and an explicit require("./dep.js") also resolves to dep.js.

Analysis or suspected cause

Source-derived: src/application/ReferenceSourceImportEntries.ts:199-202 interleaves file and directory-index candidates per suffix from RESOLUTION_SUFFIXES (line 140), so dep/index.ts precedes dep.js; src/domain/referenceSourcePathIdentity.ts:29-31 selects the first present candidate, which src/application/ReferenceSourceImportEntries.ts:213-218 records as the internal target.

Additional context

This is a remaining regression of closed #3381 ("Directory index overrides an existing CommonJS file during reference-source import"). That report's fixture used an extensionless dep/index, which is now excluded from directory candidates; the same file-before-directory contract still fails when the index has a resolvable extension such as .ts. Open tracker #3537 cites #3381 but does not report this fixture. The importing file is a .cjs CommonJS module, so Node's loader defines the expected target.

This report came from AI-assisted source review with local reproduction where stated above; the evidence labels distinguish observed runs from source-only reasoning.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3895 ### 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 `rea import-reference-source` records `require("./dep")` in a CommonJS file as an internal dependency on `dep/index.ts` when both `dep.js` and `dep/index.ts` exist. Node resolves the same call to `dep.js`. The CLI exits successfully with no parse failures, so the saved historical graph points comparison and investigation at code the source does not load. This is a remaining case of closed #3381: its extensionless `dep/index` fixture now passes, but a directory index with a resolvable extension still outranks the sibling file. The import exits 0 and records `{"from_path":"main.cjs","to":"dep/index.ts","kind":"requires","resolution":"internal","parse_state":"parsed"}` with an empty `parse_failures` array, while Node resolves `dep.js`. Removing only `dep/index.ts` changes the relationship to `dep.js`. ### Reproduction 1. In an empty directory, create the fixture, import it, and run Node's resolve-only control (no fixture file is executed): ```sh mkdir -p source/dep home printf 'require("./dep");\n' > source/main.cjs printf 'module.exports = "file";\n' > source/dep.js printf 'export const wrong = "index";\n' > source/dep/index.ts HOME="$PWD/home" rea import-reference-source "$PWD/source" --json > graph.json echo "exit=$?" python3 -c 'import json; g = json.load(open("graph.json")); print(json.dumps(g["relationships"])); print(g["parse_failures"])' node -e 'const {createRequire}=require("node:module");const p=require("node:path");const r=p.resolve("source");console.log("Node target:",p.relative(r,createRequire(p.join(r,"main.cjs")).resolve("./dep")))' ``` 2. Remove only `source/dep/index.ts` and rerun the import as a control. ### Expected behavior The relationship from `main.cjs` resolves to `dep.js`, the file Node loads for `require("./dep")`, whether or not `dep/index.ts` is present. The file-before-directory order established by #3381 applies to every directory index candidate, not only the extensionless one. ### Impact and scope CommonJS source trees containing a sibling JavaScript file and a competing TypeScript directory index get a false internal dependency in the committed historical graph, which can direct comparison toward code the source does not load; the inventory itself is intact. Workaround: available. Node's resolve-only `createRequire(...).resolve` control shown above identifies the correct target without executing or changing the source, though it does not repair the saved graph. Reporter's impact estimate: P2 (supported). ### REA version 6.5.0, built from source at main commit 034342ba28c8e56c3746be92b78aa7299afa02bd (latest npm release is 6.5.0) ### Runtime and platform Node.js v26.10.0; macOS 27.0.1 arm64 ### Target and artifact context Local CommonJS source directory imported as historical source ### Interface CLI ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence - Expected source: closed #3381 (file before directory index for CommonJS `require`); tests/boundary/cli/referenceSourceDirectoryImports.test.ts:93-97 (fixture meanings are established with Node's resolve-only loader); docs/cli.md:227-242 "Import historical source" - Failure source: observed `import-reference-source --json` relationships for the fixture above, against Node's `createRequire(...).resolve` control on the same tree - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete ```text exit=0 [{"from_path": "main.cjs", "to": "dep/index.ts", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}] [] Node target: dep.js ``` Control with `dep/index.ts` removed: ```text [{"from_path": "main.cjs", "to": "dep.js", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}] ``` Source-derived reasoning: `src/application/ReferenceSourceImportEntries.ts:199-202` builds candidates by interleaving, for each suffix in `RESOLUTION_SUFFIXES` (line 140: `"", ".ts", ".js", ...`), the file candidate and the directory index candidate, so `dep/index.ts` (suffix `.ts`) precedes `dep.js` (suffix `.js`). `src/domain/referenceSourcePathIdentity.ts:29-31` returns the first present candidate, and lines 213-218 of the importer record it as the internal target. ### Typed outcome or recovery expected The historical graph should record an internal requires relationship from main.cjs to dep.js, the file Node resolves, regardless of a competing dep/index.ts. Restoration check: On the fixture above, the relationship currently prints `"to": "dep/index.ts"`; after a fix it prints `"to": "dep.js"` with `resolution: "internal"`, matching the Node control. Control: with `dep/index.ts` removed, the relationship still prints `"to": "dep.js"`, and an explicit `require("./dep.js")` also resolves to `dep.js`. ### Analysis or suspected cause Source-derived: src/application/ReferenceSourceImportEntries.ts:199-202 interleaves file and directory-index candidates per suffix from RESOLUTION_SUFFIXES (line 140), so dep/index.ts precedes dep.js; src/domain/referenceSourcePathIdentity.ts:29-31 selects the first present candidate, which src/application/ReferenceSourceImportEntries.ts:213-218 records as the internal target. ### Additional context This is a remaining regression of closed #3381 ("Directory index overrides an existing CommonJS file during reference-source import"). That report's fixture used an extensionless `dep/index`, which is now excluded from directory candidates; the same file-before-directory contract still fails when the index has a resolvable extension such as `.ts`. Open tracker #3537 cites #3381 but does not report this fixture. The importing file is a `.cjs` CommonJS module, so Node's loader defines the expected target. This report came from AI-assisted source review with local reproduction where stated above; the evidence labels distinguish observed runs from source-only reasoning.

bsakaguchi closed this issue

2026-10-16 21:06:26 +00:00

bsakaguchi commented

2026-10-16 21:06:28 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

Agree this is the missing half of #3381: Node's CommonJS resolver checks the file extension candidates first and only falls back to directory index resolution, so with both dep.js and dep/index.ts present the sibling file must win. Recording an internal edge to dep/index.ts while Node actually loads dep.js corrupts the graph's "internal" semantics — investigation would be pointed at code the source never executes. The fix should apply file-before-directory ordering to every index candidate (including .ts), and the fixture from #3381 can be extended with a sibling file + competing typed index to lock the precedence down.

@XinwangTao commented on GitHub (Oct 15, 2026): Agree this is the missing half of #3381: Node's CommonJS resolver checks the file extension candidates first and only falls back to directory index resolution, so with both dep.js and dep/index.ts present the sibling file must win. Recording an internal edge to dep/index.ts while Node actually loads dep.js corrupts the graph's "internal" semantics — investigation would be pointed at code the source never executes. The fix should apply file-before-directory ordering to every index candidate (including .ts), and the fixture from #3381 can be extended with a sibling file + competing typed index to lock the precedence down.

bsakaguchi commented

2026-10-16 21:06:28 +00:00

Author

Owner

@xu91102 commented on GitHub (Oct 15, 2026):

Reproduced through the production CLI on main 933f216698fc7162c234e22fefe97315faa80676, Node 24.18.0/macOS arm64: a CommonJS require("./dep") points to dep/index.ts when dep.js is present, while Node's resolve-only control selects dep.js. Removing the typed index restores the correct edge. The importer exits 0 with no parse failures in both cases.

Scope clarification after checking existing compatibility controls: the importer also supports source-recovery suffixes and intentionally preserves some legacy candidate ordering. I am limiting the proposed fix to the separate package-main gating in #3896 rather than introducing an arbitrary new suffix priority here. This issue's reproduced ordering failure remains unresolved by that work. No PR for #3895 has been created.

@xu91102 commented on GitHub (Oct 15, 2026): Reproduced through the production CLI on main `933f216698fc7162c234e22fefe97315faa80676`, Node 24.18.0/macOS arm64: a CommonJS `require("./dep")` points to `dep/index.ts` when `dep.js` is present, while Node's resolve-only control selects `dep.js`. Removing the typed index restores the correct edge. The importer exits 0 with no parse failures in both cases. Scope clarification after checking existing compatibility controls: the importer also supports source-recovery suffixes and intentionally preserves some legacy candidate ordering. I am limiting the proposed fix to the separate package-main gating in #3896 rather than introducing an arbitrary new suffix priority here. This issue's reproduced ordering failure remains unresolved by that work. No PR for #3895 has been created.

bsakaguchi commented

2026-10-16 21:06:28 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

Understood — no rush to fold this into #3896's fix. For a future standalone change, a minimal approach would be to reorder only the existing file candidates ahead of directory-index candidates when both exist for the same require target, without inventing new suffix priorities; the #3381 fixture extended with a sibling .js plus competing typed index should pin that behavior.

@XinwangTao commented on GitHub (Oct 15, 2026): Understood — no rush to fold this into #3896's fix. For a future standalone change, a minimal approach would be to reorder only the existing file candidates ahead of directory-index candidates when both exist for the same require target, without inventing new suffix priorities; the #3381 fixture extended with a sibling .js plus competing typed index should pin that behavior.

bsakaguchi commented

2026-10-16 21:06:29 +00:00

Author

Owner

@morluto commented on GitHub (Oct 16, 2026):

Fixed on main in c0d550b23. Call-form CommonJS resolution now checks file candidates before directory indexes. The dep.js plus dep/index.ts fixture resolves to dep.js, while TypeScript import-equals keeps its separate source-resolution semantics.

The producer regression and public CLI Node resolve-only controls cover file-before-index behavior; this later main fix supersedes the older comment that deferred this issue.

Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [c0d550b23](https://github.com/morluto/rea/commit/c0d550b231f5a9d49eac9e7554184ee938d79de0). Call-form CommonJS resolution now checks file candidates before directory indexes. The dep.js plus dep/index.ts fixture resolves to dep.js, while TypeScript import-equals keeps its separate source-resolution semantics. The producer regression and public CLI Node resolve-only controls cover file-before-index behavior; this later main fix supersedes the older comment that deferred this issue. Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

bsakaguchi commented

2026-10-16 21:06:29 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 16, 2026):

Resolving require("./dep") through the owning syntax's extension order is the right model — a CommonJS require should follow Node's .js-over-index resolution instead of falling into the TS-style directory index. This also ties in nicely with #3896 and the require-extension-order draft #4017. Thanks for landing c0d550b.

@XinwangTao commented on GitHub (Oct 16, 2026): Resolving require("./dep") through the owning syntax's extension order is the right model — a CommonJS require should follow Node's .js-over-index resolution instead of falling into the TS-style directory index. This also ties in nicely with #3896 and the require-extension-order draft #4017. Thanks for landing c0d550b.

bsakaguchi referenced this issue

2026-10-17 21:11:59 +00:00

[PR #1514] [MERGED] feat(mcp): compact input schema profile for size-capped providers (kimi-cli/Moonshot) #2934

bsakaguchi referenced this issue

2026-10-17 21:12:46 +00:00

[PR #1607] [MERGED] fix(setup): preserve configured MCP server environment #3001

bsakaguchi referenced this issue

2026-10-31 00:37:27 +00:00

[GH-ISSUE #5160] [Bug] Codex + kimi-k3: older REA schemas trigger misleading recursion errors; recovery guidance missing #3170