morluto/rea

Fork 0

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

[GH-ISSUE #3896] [Bug] Reference-source import leaves a valid package main unresolved when no index exists #1516

New issue

Closed

opened 2026-10-16 21:06:30 +00:00 by bsakaguchi · 6 comments

bsakaguchi commented

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

Owner

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

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 leaves require("./pkg") unresolved when pkg/package.json declares {"main":"actual.js"} and pkg/actual.js is present but pkg/index.js is absent. Node resolves the same call to pkg/actual.js. Adding an unused pkg/index.js makes REA resolve to pkg/actual.js, so package-main selection only runs after an unrelated index fallback exists. This is a remaining case of closed #3383, whose fixture included an index and now passes.

The import exits 0 and records {"from_path":"main.cjs","to":"pkg","kind":"requires","resolution":"unresolved","parse_state":"parsed"} with an empty parse_failures array, although the manifest and declared main are inventoried and Node resolves pkg/actual.js. With the unused pkg/index.js added, the same import resolves to pkg/actual.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/pkg home
printf 'require("./pkg");\n' > source/main.cjs
printf '{"main":"actual.js"}\n' > source/pkg/package.json
printf 'module.exports = "main";\n' > source/pkg/actual.js
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("./pkg")))'

As a control, add an unused index with printf 'module.exports = "unused";\n' > source/pkg/index.js and rerun the import.

Expected behavior

The relationship from main.cjs resolves internally to pkg/actual.js, the declared and inventoried package main, without requiring a fallback index file to exist.

Impact and scope

Ordinary relative CommonJS packages that declare a non-index main without an index file get an incomplete historical dependency graph although every required file is inventoried; the importer reports uncertainty rather than a false target, so the effect is bounded.

Workaround: available. Node's resolve-only createRequire(...).resolve control shown above identifies the correct dependency 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: src/application/ReferenceSourceImportEntries.ts:103-106 (Node selects a nonempty string main before a directory index); closed #3383 (CommonJS package main must be honored); tests/boundary/cli/referenceSourceDirectoryImports.test.ts:62-65 (package-main precedence fixture); 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": "pkg", "kind": "requires", "resolution": "unresolved", "parse_state": "parsed"}]
[]
Node target: pkg/actual.js

Control with an unused pkg/index.js added:

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

Source-derived reasoning: the candidates built at src/application/ReferenceSourceImportEntries.ts:199-202 contain pkg with each suffix and pkg/index.*, but not the manifest's declared main. With no index present, line 212 returns unresolved immediately; selectPackageMain is reached only at lines 213-217 after a candidate resolves through an index, so the readable manifest never selects the existing main on its own. The existing package-main fixture includes pkg/index.js, which masks this path.

Typed outcome or recovery expected

The historical graph should record an internal requires relationship from main.cjs to pkg/actual.js, the declared and inventoried package main, without needing a fallback index file.

Restoration check:

On the fixture above, the relationship currently prints "to": "pkg", "resolution": "unresolved"; after a fix it prints "to": "pkg/actual.js", "resolution": "internal", matching the Node control. Control: with an unused pkg/index.js added, the relationship continues to print "to": "pkg/actual.js", and a manifest whose main names a missing file continues to fall back to an existing index.

Analysis or suspected cause

Source-derived: src/application/ReferenceSourceImportEntries.ts:199-202 builds candidates without the declared package main; line 212 returns unresolved when no index candidate exists; selectPackageMain runs only at lines 213-217 after resolution through an index.

Additional context

This is a remaining regression of closed #3383 ("Historical-source import ignores CommonJS package main and selects an unrelated index"). That report's fixture included index.js and now resolves correctly; a declared main still fails when no fallback index exists. Closed #2020 concerns empty-main fallback in analyze-javascript-application, a different command and condition. Open tracker #3537 indexes #3383 but does not report this fixture.

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/3896 ### 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` leaves `require("./pkg")` unresolved when `pkg/package.json` declares `{"main":"actual.js"}` and `pkg/actual.js` is present but `pkg/index.js` is absent. Node resolves the same call to `pkg/actual.js`. Adding an unused `pkg/index.js` makes REA resolve to `pkg/actual.js`, so package-main selection only runs after an unrelated index fallback exists. This is a remaining case of closed #3383, whose fixture included an index and now passes. The import exits 0 and records `{"from_path":"main.cjs","to":"pkg","kind":"requires","resolution":"unresolved","parse_state":"parsed"}` with an empty `parse_failures` array, although the manifest and declared main are inventoried and Node resolves `pkg/actual.js`. With the unused `pkg/index.js` added, the same import resolves to `pkg/actual.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/pkg home printf 'require("./pkg");\n' > source/main.cjs printf '{"main":"actual.js"}\n' > source/pkg/package.json printf 'module.exports = "main";\n' > source/pkg/actual.js 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("./pkg")))' ``` 2. As a control, add an unused index with `printf 'module.exports = "unused";\n' > source/pkg/index.js` and rerun the import. ### Expected behavior The relationship from `main.cjs` resolves internally to `pkg/actual.js`, the declared and inventoried package main, without requiring a fallback `index` file to exist. ### Impact and scope Ordinary relative CommonJS packages that declare a non-index `main` without an index file get an incomplete historical dependency graph although every required file is inventoried; the importer reports uncertainty rather than a false target, so the effect is bounded. Workaround: available. Node's resolve-only `createRequire(...).resolve` control shown above identifies the correct dependency 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: src/application/ReferenceSourceImportEntries.ts:103-106 (Node selects a nonempty string `main` before a directory index); closed #3383 (CommonJS package main must be honored); tests/boundary/cli/referenceSourceDirectoryImports.test.ts:62-65 (package-main precedence fixture); 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": "pkg", "kind": "requires", "resolution": "unresolved", "parse_state": "parsed"}] [] Node target: pkg/actual.js ``` Control with an unused `pkg/index.js` added: ```text [{"from_path": "main.cjs", "to": "pkg/actual.js", "kind": "requires", "resolution": "internal", "parse_state": "parsed"}] ``` Source-derived reasoning: the candidates built at `src/application/ReferenceSourceImportEntries.ts:199-202` contain `pkg` with each suffix and `pkg/index.*`, but not the manifest's declared main. With no index present, line 212 returns `unresolved` immediately; `selectPackageMain` is reached only at lines 213-217 after a candidate resolves through an index, so the readable manifest never selects the existing main on its own. The existing package-main fixture includes `pkg/index.js`, which masks this path. ### Typed outcome or recovery expected The historical graph should record an internal requires relationship from main.cjs to pkg/actual.js, the declared and inventoried package main, without needing a fallback index file. Restoration check: On the fixture above, the relationship currently prints `"to": "pkg", "resolution": "unresolved"`; after a fix it prints `"to": "pkg/actual.js", "resolution": "internal"`, matching the Node control. Control: with an unused `pkg/index.js` added, the relationship continues to print `"to": "pkg/actual.js"`, and a manifest whose `main` names a missing file continues to fall back to an existing index. ### Analysis or suspected cause Source-derived: src/application/ReferenceSourceImportEntries.ts:199-202 builds candidates without the declared package main; line 212 returns unresolved when no index candidate exists; selectPackageMain runs only at lines 213-217 after resolution through an index. ### Additional context This is a remaining regression of closed #3383 ("Historical-source import ignores CommonJS package main and selects an unrelated index"). That report's fixture included `index.js` and now resolves correctly; a declared main still fails when no fallback index exists. Closed #2020 concerns empty-main fallback in `analyze-javascript-application`, a different command and condition. Open tracker #3537 indexes #3383 but does not report this fixture. 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:30 +00:00

bsakaguchi commented

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

Author

Owner

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

Reproduced logic is convincing: Node's resolver only consults the index fallback after package main fails, so a directory package with a declared non-index main should resolve to pkg/actual.js directly — leaving it "unresolved" just because pkg/index.js is absent is a real gap versus Node semantics. The fact that adding an unused index.js flips the resolution confirms the package-main branch is being gated on the index fallback existing, which inverts the actual resolver precedence. One suggestion: mirror the closed #3383 fixture but without the index file, and assert the relationship points at pkg/actual.js (not just "resolves").

@XinwangTao commented on GitHub (Oct 15, 2026): Reproduced logic is convincing: Node's resolver only consults the index fallback after package main fails, so a directory package with a declared non-index main should resolve to pkg/actual.js directly — leaving it "unresolved" just because pkg/index.js is absent is a real gap versus Node semantics. The fact that adding an unused index.js flips the resolution confirms the package-main branch is being gated on the index fallback existing, which inverts the actual resolver precedence. One suggestion: mirror the closed #3383 fixture but without the index file, and assert the relationship points at pkg/actual.js (not just "resolves").

bsakaguchi commented

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

Author

Owner

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

Reproduced logic is convincing: Node resolves a directory package with a declared non-index main to pkg/actual.js directly, so leaving it "unresolved" merely because pkg/index.js is absent inverts the actual resolver precedence (index is only a fallback after main). The fact that adding an unused index.js flips the resolution confirms the package-main branch is gated on the index fallback existing. Suggestion: mirror the closed #3383 fixture but without the index file, and assert the relationship targets pkg/actual.js explicitly.

@XinwangTao commented on GitHub (Oct 15, 2026): Reproduced logic is convincing: Node resolves a directory package with a declared non-index main to pkg/actual.js directly, so leaving it "unresolved" merely because pkg/index.js is absent inverts the actual resolver precedence (index is only a fallback after main). The fact that adding an unused index.js flips the resolution confirms the package-main branch is gated on the index fallback existing. Suggestion: mirror the closed #3383 fixture but without the index file, and assert the relationship targets pkg/actual.js explicitly.

bsakaguchi commented

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

Author

Owner

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

Confirmed on current main 933f216698fc7162c234e22fefe97315faa80676 through import-reference-source --json, Node 24.18.0/macOS arm64. With pkg/package.json declaring actual.js, the present pkg/actual.js and no index, the edge remains to: "pkg", resolution: "unresolved"; Node's resolve-only control selects pkg/actual.js. Adding an index makes the importer select the declared main, and the missing-main/existing-index control correctly falls back to pkg/index.js.

I am preparing a focused fix that consults the package manifest independently of whether an index exists, while retaining existing file candidates, directory intent, malformed/unreadable-manifest uncertainty and path boundaries. After checking the compatibility controls, #3895's separate suffix-ordering failure is explicitly outside this fix; it remains reproduced and unresolved. The baseline above was recorded before implementation, and no fix or PR has been published yet.

Verified fix published (2026-10-15)

The fix is committed and normally pushed to the fork commit on codex/reference-source-resolution-order. The reviewed fix is now published as Draft PR #3973. Final-commit CI 38693964922 has completed successfully; the final result is recorded below.

Validation: 17 selected files, 106 tests passed and two existing conditional skips; check (six tasks plus module/architecture guards), documentation (six tasks plus five conformance tests), test discovery, four immutable public CLI controls, and independent Standards/Spec re-review passed on the frozen commit. Unknown/canonically ambiguous manifest identities are retained. The separate #3895 suffix-ordering issue is not fixed by this change. This importer has no public MCP registration, so no stdio importer run is claimed. Normal commit/pre-push hooks preserved the reviewed bytes.

Local execution was macOS arm64 / Node 24.18.0. Windows/Linux/native execution and upstream CI are not claimed; no performance or token-cost improvement is claimed. A single ordinary Draft creation attempt for the first prepared fix was rejected by GitHub with xu91102 does not have the correct permissions to execute CreatePullRequest; the reason remains unknown. No alternate account or creation route was tried.

Publication update: after the maintainer closed earlier proposals #2060 and #3297 as covered on main, a fresh rebuilt-main CLI reproduction and duplicate check still confirmed this issue. The ordinary fork-to-upstream Draft creation succeeded: https://github.com/morluto/rea/pull/3973. The earlier creation denial remains historical evidence; its cause was not established.

Final-head CI result (2026-10-15)

PR #3973, head 48fde102a1d1fbc7dbceaab1530149fdd7fc0320: CI 38693964922 attempt1 completed SUCCESS. Complete raw logs (2,115,363 bytes) and synthetic checkout parents were checked. The portable report aggregates source plus five shards: 15,209 passed / 135 skipped; it is not an additional execution. The fix-specific owner and public CLI regression files actually ran and passed. Linux x64 package baseline succeeded; Windows execution and real Apple E2E lanes were skipped. Local independent review, remote review, CI and human readiness remain separate.

@xu91102 commented on GitHub (Oct 15, 2026): Confirmed on current main `933f216698fc7162c234e22fefe97315faa80676` through `import-reference-source --json`, Node 24.18.0/macOS arm64. With `pkg/package.json` declaring `actual.js`, the present `pkg/actual.js` and no index, the edge remains `to: "pkg", resolution: "unresolved"`; Node's resolve-only control selects `pkg/actual.js`. Adding an index makes the importer select the declared main, and the missing-main/existing-index control correctly falls back to `pkg/index.js`. I am preparing a focused fix that consults the package manifest independently of whether an index exists, while retaining existing file candidates, directory intent, malformed/unreadable-manifest uncertainty and path boundaries. After checking the compatibility controls, #3895's separate suffix-ordering failure is explicitly outside this fix; it remains reproduced and unresolved. The baseline above was recorded before implementation, and no fix or PR has been published yet. ### Verified fix published (2026-10-15) The fix is committed and normally pushed to [the fork commit](https://github.com/xu91102/rea/commit/48fde102a1d1fbc7dbceaab1530149fdd7fc0320) on `codex/reference-source-resolution-order`. The reviewed fix is now published as Draft PR #3973. Final-commit [CI 38693964922](https://github.com/morluto/rea/actions/runs/38693964922) has completed successfully; the final result is recorded below. Validation: 17 selected files, 106 tests passed and two existing conditional skips; check (six tasks plus module/architecture guards), documentation (six tasks plus five conformance tests), test discovery, four immutable public CLI controls, and independent Standards/Spec re-review passed on the frozen commit. Unknown/canonically ambiguous manifest identities are retained. The separate #3895 suffix-ordering issue is not fixed by this change. This importer has no public MCP registration, so no stdio importer run is claimed. Normal commit/pre-push hooks preserved the reviewed bytes. Local execution was macOS arm64 / Node 24.18.0. Windows/Linux/native execution and upstream CI are not claimed; no performance or token-cost improvement is claimed. A single ordinary Draft creation attempt for the first prepared fix was rejected by GitHub with `xu91102 does not have the correct permissions to execute CreatePullRequest`; the reason remains unknown. No alternate account or creation route was tried. Publication update: after the maintainer closed earlier proposals #2060 and #3297 as covered on main, a fresh rebuilt-main CLI reproduction and duplicate check still confirmed this issue. The ordinary fork-to-upstream Draft creation succeeded: https://github.com/morluto/rea/pull/3973. The earlier creation denial remains historical evidence; its cause was not established. ### Final-head CI result (2026-10-15) PR #3973, head `48fde102a1d1fbc7dbceaab1530149fdd7fc0320`: [CI 38693964922](https://github.com/morluto/rea/actions/runs/38693964922) attempt1 completed SUCCESS. Complete raw logs (2,115,363 bytes) and synthetic checkout parents were checked. The portable report aggregates source plus five shards: 15,209 passed / 135 skipped; it is not an additional execution. The fix-specific owner and public CLI regression files actually ran and passed. Linux x64 package baseline succeeded; Windows execution and real Apple E2E lanes were skipped. Local independent review, remote review, CI and human readiness remain separate.

bsakaguchi commented

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

Author

Owner

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

Agreed with the focused fix direction: consulting the package manifest independently of the index fallback matches Node's resolver precedence, and scoping #3895's suffix-ordering failure out of this change is the right call to keep the fix reviewable. The control matrix you recorded (missing-main/existing-index falls back to index.js) covers the key regression risk well.

@XinwangTao commented on GitHub (Oct 15, 2026): Agreed with the focused fix direction: consulting the package manifest independently of the index fallback matches Node's resolver precedence, and scoping #3895's suffix-ordering failure out of this change is the right call to keep the fix reviewable. The control matrix you recorded (missing-main/existing-index falls back to index.js) covers the key regression risk well.

bsakaguchi commented

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

Author

Owner

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

Fixed on main in c0d550b23. Reference-source runtime require resolution consults package main independently of the presence of an index and preserves source-recovery syntax distinctions.

Source/reader and public CLI controls compare declared-main/no-index behavior with the pinned Node resolve-only loader.

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). Reference-source runtime require resolution consults package main independently of the presence of an index and preserves source-recovery syntax distinctions. Source/reader and public CLI controls compare declared-main/no-index behavior with the pinned Node resolve-only loader. 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:34 +00:00

Author

Owner

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

Validating the package.json main field directly when there is no index is exactly right — falling back to heuristics silently drops a legitimate resolution target. This pairs well with #3895 and the c0d550b change; glad to see both landing together. Thanks for closing this out.

@XinwangTao commented on GitHub (Oct 16, 2026): Validating the package.json main field directly when there is no index is exactly right — falling back to heuristics silently drops a legitimate resolution target. This pairs well with #3895 and the c0d550b change; glad to see both landing together. Thanks for closing this out.

bsakaguchi referenced this issue

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

[PR #1516] [MERGED] fix(browser): keep the first DOM attribute when the name is repeated #2936

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#1516

No description provided.

Rows
Columns