morluto/rea

Fork 0

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

[GH-ISSUE #4189] [Bug] Trailing-slash CommonJS require ignores package main in reference import #1639

New issue

Closed

opened 2026-10-16 21:08:18 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-16 21:08:18 +00:00

Owner

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

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

import-reference-source records the wrong dependency for require("./sub/"). With sub/package.json declaring actual.js and both sub/actual.js and sub/index.js present, REA returns a parsed internal edge to sub/index.js. Node resolves the identical specifier to sub/actual.js. Removing only the trailing slash makes REA select the declared main. The source inventory remains available, but the dependency target is incorrect.

Reproduction

From the built source checkout root at the revision below, run this exact executed reproduction with the stated Node executable on PATH. It creates three independent trees, calls the public CLI, asks Node to resolve each literal specifier without executing the source, and removes its disposable fixtures afterward. The second and third trees are controls, not suggested changes to historical source.

python3 - <<'PYTHON'
import json
import os
from pathlib import Path
import subprocess
import tempfile

cases = [
    ("slash-main", "./sub/", True),
    ("plain-main", "./sub", True),
    ("slash-index-only", "./sub/", False),
]
control = 'const {createRequire}=require("node:module");const p=require("node:path");const r=process.argv[1];console.log(p.relative(r,createRequire(p.join(r,"main.cjs")).resolve(process.argv[2])))'
with tempfile.TemporaryDirectory(prefix="rea-reference-main-") as work:
    base = Path(work).resolve()
    home = base / "home"
    home.mkdir()
    env = {"PATH": os.environ["PATH"], "HOME": str(home),
           "XDG_CONFIG_HOME": str(home / "config"),
           "XDG_CACHE_HOME": str(home / "cache")}
    for name, specifier, has_main in cases:
        root = base / name
        (root / "sub").mkdir(parents=True)
        (root / "main.cjs").write_text('require(' + json.dumps(specifier) + ');\n')
        (root / "sub/actual.js").write_text('module.exports = "main";\n')
        (root / "sub/index.js").write_text('module.exports = "index";\n')
        if has_main:
            (root / "sub/package.json").write_text('{"main":"actual.js"}\n')
        imported = subprocess.run(
            ["node", "scripts/rea.mjs", "import-reference-source", str(root), "--json"],
            env=env, capture_output=True, text=True, timeout=30)
        print(name, "CLI exit:", imported.returncode, "stderr:", repr(imported.stderr))
        graph = json.loads(imported.stdout)
        print("schema:", graph["schema"])
        print("relationships:", json.dumps(graph["relationships"]))
        print("parse_failures:", graph["parse_failures"])
        resolved = subprocess.run(
            ["node", "-e", control, str(root), specifier],
            env=env, capture_output=True, text=True, timeout=10)
        print("Node exit:", resolved.returncode, "target:", resolved.stdout.strip())
PYTHON

The exact final fixture reproduced in independent source-build CLI runs, including the final publication review. Every CLI call exited 0 with empty stderr and empty parse_failures. Node's resolve-only calls also exited 0. Observed targets were:

case              REA target       Node target
slash-main        sub/index.js     sub/actual.js
plain-main        sub/actual.js    sub/actual.js
slash-index-only  sub/index.js     sub/index.js

Expected behavior

slash-main should return an internal requires edge to the declared and inventoried sub/actual.js, matching Node and the plain-main control. slash-index-only should continue selecting sub/index.js because it has no package manifest. REA's package-main selection contract explicitly places a nonempty declared main before a directory index. Its compiled CLI coverage supports directory spellings and package-main precedence against Node's resolve-only loader, but does not combine this trailing-slash request with the manifest fixture.

Impact and scope

P2, supported: one bounded CommonJS input produces an incorrect parsed internal dependency while retaining source inventory. No severe harm, broader workflow failure, downstream comparison error or real-world prevalence was measured. This report covers CLI behavior on the stated source build and platform.

A graph-result workaround preserving the original source bytes and identity remains unverified. Node's resolve-only control safely identifies the intended target for manual interpretation, but does not repair the graph. Removing the slash changes the historical input and is only a discriminating control.

REA version

Source-built checkout at 62bf09809ef7bb224028916424f8f07652a20f4f. The source CLI reports 6.5.0; the published package was not reproduced. Current public main was fetched before submission, and the relevant reference importer, parser, CLI, graph contracts and CLI documentation are unchanged. Current-main applicability is source-inspected, not a separate current-main runtime reproduction.

Runtime and platform

Node.js v24.18.0; npm 11.16.0; macOS arm64. Independent source checkouts were compiled with npm run build:cached. The fixture uses isolated HOME/config/cache directories. No external provider, network dependency or execution of imported source is needed. MCP, alternate platforms and published-package artifacts were not exercised.

Target and artifact context

Benign local UTF-8 CommonJS source trees contain main.cjs, sub/actual.js, sub/index.js and, for the first two cases, sub/package.json with {"main":"actual.js"}. The .cjs file uses an ordinary unshadowed require with no loader replacement. Both manifest and declared main were separately confirmed in the failing graph's inventory.

Interface

CLI

Earliest failure stage

Native analysis or reconstruction. The importer records a parsed internal relationship with the wrong target.

Relevant output or evidence

The failing CLI graph contains:

{"from_path":"main.cjs","to":"sub/index.js","kind":"requires","resolution":"internal","parse_state":"parsed"}

The graph schema is HistoricalSourceGraph and parse_failures is []. The same-tree Node control returns sub/actual.js. Independent verification also ran both --json and --format json, which returned the same relevant graph fields. The existing focused command npm run test:focused -- src/application/ReferenceSourceImportEntries.test.ts tests/boundary/cli/referenceSourceDirectoryImports.test.ts passed both files and all four tests in that verification; those tests do not contain the failing combination. No full suite or downstream comparison was executed.

Typed outcome or recovery expected

Proposed closure check: rerun the unchanged reproduction. slash-main must return a parsed internal requires edge to sub/actual.js, matching its Node control. Preserve the plain-main target sub/actual.js, the slash-index-only target sub/index.js, and inventory of the original fixture files. Omitting the require or changing its specifier would not restore the historical-source result.

Analysis or suspected cause

Source-derived: the directory/index recognition guard finds the index candidate but compares its dirname sub with the normalized request sub/. That comparison fails, so the importer records the index directly instead of consulting the declared package main. This is distinct from returning unresolved before any candidate match when the index is absent. The source explains the observed output; this report does not prescribe a particular implementation.

Additional context

Open #3895 requires a sibling dep.js to win over dep/index.ts, without package metadata. This fixture has no sibling file and tests manifest precedence over an existing index. Open #3896 requires a declared main to resolve without an index; its plain-specifier/index-present control already passes here. Its no-index closure can pass while this trailing-slash/index-present guard remains wrong, and correcting this guard can leave that no-index early return unresolved.

Closed #3383 reported ignored package-main precedence for the plain relative spelling. The plain-main control confirms that case is restored at the tested revision; this report identifies the remaining trailing-slash trigger on current source. No duplicate was found in the refreshed open inventory or targeted closed searches. NOT_ENOUGH_INFO: a last-known-good version for this trailing-slash case and installed-release behavior were not established.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4189 ### 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 `import-reference-source` records the wrong dependency for `require("./sub/")`. With `sub/package.json` declaring `actual.js` and both `sub/actual.js` and `sub/index.js` present, REA returns a parsed internal edge to `sub/index.js`. Node resolves the identical specifier to `sub/actual.js`. Removing only the trailing slash makes REA select the declared main. The source inventory remains available, but the dependency target is incorrect. ### Reproduction From the built source checkout root at the revision below, run this exact executed reproduction with the stated Node executable on PATH. It creates three independent trees, calls the public CLI, asks Node to resolve each literal specifier without executing the source, and removes its disposable fixtures afterward. The second and third trees are controls, not suggested changes to historical source. ```sh python3 - <<'PYTHON' import json import os from pathlib import Path import subprocess import tempfile cases = [ ("slash-main", "./sub/", True), ("plain-main", "./sub", True), ("slash-index-only", "./sub/", False), ] control = 'const {createRequire}=require("node:module");const p=require("node:path");const r=process.argv[1];console.log(p.relative(r,createRequire(p.join(r,"main.cjs")).resolve(process.argv[2])))' with tempfile.TemporaryDirectory(prefix="rea-reference-main-") as work: base = Path(work).resolve() home = base / "home" home.mkdir() env = {"PATH": os.environ["PATH"], "HOME": str(home), "XDG_CONFIG_HOME": str(home / "config"), "XDG_CACHE_HOME": str(home / "cache")} for name, specifier, has_main in cases: root = base / name (root / "sub").mkdir(parents=True) (root / "main.cjs").write_text('require(' + json.dumps(specifier) + ');\n') (root / "sub/actual.js").write_text('module.exports = "main";\n') (root / "sub/index.js").write_text('module.exports = "index";\n') if has_main: (root / "sub/package.json").write_text('{"main":"actual.js"}\n') imported = subprocess.run( ["node", "scripts/rea.mjs", "import-reference-source", str(root), "--json"], env=env, capture_output=True, text=True, timeout=30) print(name, "CLI exit:", imported.returncode, "stderr:", repr(imported.stderr)) graph = json.loads(imported.stdout) print("schema:", graph["schema"]) print("relationships:", json.dumps(graph["relationships"])) print("parse_failures:", graph["parse_failures"]) resolved = subprocess.run( ["node", "-e", control, str(root), specifier], env=env, capture_output=True, text=True, timeout=10) print("Node exit:", resolved.returncode, "target:", resolved.stdout.strip()) PYTHON ``` The exact final fixture reproduced in independent source-build CLI runs, including the final publication review. Every CLI call exited 0 with empty stderr and empty `parse_failures`. Node's resolve-only calls also exited 0. Observed targets were: ```text case REA target Node target slash-main sub/index.js sub/actual.js plain-main sub/actual.js sub/actual.js slash-index-only sub/index.js sub/index.js ``` ### Expected behavior `slash-main` should return an internal requires edge to the declared and inventoried `sub/actual.js`, matching Node and the `plain-main` control. `slash-index-only` should continue selecting `sub/index.js` because it has no package manifest. REA's [package-main selection contract](https://github.com/morluto/rea/blob/c7f6b8bad1a34d5efb30ff2a017672dea1fcdabb/src/application/ReferenceSourceImportEntries.ts#L97-L197) explicitly places a nonempty declared main before a directory index. Its [compiled CLI coverage](https://github.com/morluto/rea/blob/c7f6b8bad1a34d5efb30ff2a017672dea1fcdabb/tests/boundary/cli/referenceSourceDirectoryImports.test.ts#L24-L107) supports directory spellings and package-main precedence against Node's resolve-only loader, but does not combine this trailing-slash request with the manifest fixture. ### Impact and scope P2, supported: one bounded CommonJS input produces an incorrect parsed internal dependency while retaining source inventory. No severe harm, broader workflow failure, downstream comparison error or real-world prevalence was measured. This report covers CLI behavior on the stated source build and platform. A graph-result workaround preserving the original source bytes and identity remains unverified. Node's resolve-only control safely identifies the intended target for manual interpretation, but does not repair the graph. Removing the slash changes the historical input and is only a discriminating control. ### REA version Source-built checkout at [62bf09809ef7bb224028916424f8f07652a20f4f](https://github.com/morluto/rea/commit/62bf09809ef7bb224028916424f8f07652a20f4f). The source CLI reports `6.5.0`; the published package was not reproduced. Current public main was fetched before submission, and the relevant reference importer, parser, CLI, graph contracts and CLI documentation are unchanged. Current-main applicability is source-inspected, not a separate current-main runtime reproduction. ### Runtime and platform Node.js v24.18.0; npm 11.16.0; macOS arm64. Independent source checkouts were compiled with `npm run build:cached`. The fixture uses isolated HOME/config/cache directories. No external provider, network dependency or execution of imported source is needed. MCP, alternate platforms and published-package artifacts were not exercised. ### Target and artifact context Benign local UTF-8 CommonJS source trees contain `main.cjs`, `sub/actual.js`, `sub/index.js` and, for the first two cases, `sub/package.json` with `{"main":"actual.js"}`. The `.cjs` file uses an ordinary unshadowed `require` with no loader replacement. Both manifest and declared main were separately confirmed in the failing graph's inventory. ### Interface CLI ### Earliest failure stage Native analysis or reconstruction. The importer records a parsed internal relationship with the wrong target. ### Relevant output or evidence The failing CLI graph contains: ```json {"from_path":"main.cjs","to":"sub/index.js","kind":"requires","resolution":"internal","parse_state":"parsed"} ``` The graph schema is `HistoricalSourceGraph` and `parse_failures` is `[]`. The same-tree Node control returns `sub/actual.js`. Independent verification also ran both `--json` and `--format json`, which returned the same relevant graph fields. The existing focused command `npm run test:focused -- src/application/ReferenceSourceImportEntries.test.ts tests/boundary/cli/referenceSourceDirectoryImports.test.ts` passed both files and all four tests in that verification; those tests do not contain the failing combination. No full suite or downstream comparison was executed. ### Typed outcome or recovery expected Proposed closure check: rerun the unchanged reproduction. `slash-main` must return a parsed internal requires edge to `sub/actual.js`, matching its Node control. Preserve the `plain-main` target `sub/actual.js`, the `slash-index-only` target `sub/index.js`, and inventory of the original fixture files. Omitting the require or changing its specifier would not restore the historical-source result. ### Analysis or suspected cause Source-derived: the [directory/index recognition guard](https://github.com/morluto/rea/blob/c7f6b8bad1a34d5efb30ff2a017672dea1fcdabb/src/application/ReferenceSourceImportEntries.ts#L200-L241) finds the index candidate but compares its dirname `sub` with the normalized request `sub/`. That comparison fails, so the importer records the index directly instead of consulting the declared package main. This is distinct from returning unresolved before any candidate match when the index is absent. The source explains the observed output; this report does not prescribe a particular implementation. ### Additional context Open [#3895](https://github.com/morluto/rea/issues/3895) requires a sibling `dep.js` to win over `dep/index.ts`, without package metadata. This fixture has no sibling file and tests manifest precedence over an existing index. Open [#3896](https://github.com/morluto/rea/issues/3896) requires a declared main to resolve without an index; its plain-specifier/index-present control already passes here. Its no-index closure can pass while this trailing-slash/index-present guard remains wrong, and correcting this guard can leave that no-index early return unresolved. Closed [#3383](https://github.com/morluto/rea/issues/3383) reported ignored package-main precedence for the plain relative spelling. The `plain-main` control confirms that case is restored at the tested revision; this report identifies the remaining trailing-slash trigger on current source. No duplicate was found in the refreshed open inventory or targeted closed searches. NOT_ENOUGH_INFO: a last-known-good version for this trailing-slash case and installed-release behavior were not established.

bsakaguchi commented

2026-10-16 21:08:19 +00:00

Author

Owner

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

Taking this one. Root cause: posix.normalize(posix.join(dirname, "./sub/")) preserves the trailing slash, so normalized is sub/ while posix.dirname(match) is sub — the resolvedThroughIndex comparison in resolveInternalSpecifier fails, and selectPackageMain (the declared-main selection) never runs for trailing-slash specifiers. Plain ./sub normalizes without the slash, so it reaches the package-main pass. Fix: compare against the slash-stripped directory so trailing-slash directory references take the same package-main path as the plain control. Regression test in tests/boundary/cli/referenceSourceDirectoryImports.test.ts combining the trailing-slash request with the manifest fixture.

@hemv-857 commented on GitHub (Oct 15, 2026): Taking this one. Root cause: `posix.normalize(posix.join(dirname, "./sub/"))` preserves the trailing slash, so `normalized` is `sub/` while `posix.dirname(match)` is `sub` — the `resolvedThroughIndex` comparison in `resolveInternalSpecifier` fails, and `selectPackageMain` (the declared-`main` selection) never runs for trailing-slash specifiers. Plain `./sub` normalizes without the slash, so it reaches the package-main pass. Fix: compare against the slash-stripped directory so trailing-slash directory references take the same package-main path as the plain control. Regression test in `tests/boundary/cli/referenceSourceDirectoryImports.test.ts` combining the trailing-slash request with the manifest fixture.

bsakaguchi commented

2026-10-16 21:08:19 +00:00

Author

Owner

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

I independently arrived at the same root cause as @hemv-857: posix.normalize("sub/") keeps the trailing slash, so the resolvedThroughIndex check fails and selectPackageMain is skipped. I opened #4221 around the same time as #4219. Mine strips trailing separators for that comparison only, so unresolved targets like missing/ keep their spelling, and it adds a ./pkg/ case to the existing referenceSourceDirectoryImports CLI test, which checks each fixture against Node's createRequire().resolve. With it, your three cases match Node (sub/actual.js, sub/actual.js, sub/index.js). Happy to close mine in favour of #4219 if maintainers prefer.

cc @morluto @kaoru0822-kitauji @N0zoM1z0

@kharedevansh11 commented on GitHub (Oct 15, 2026): I independently arrived at the same root cause as @hemv-857: `posix.normalize("sub/")` keeps the trailing slash, so the `resolvedThroughIndex` check fails and `selectPackageMain` is skipped. I opened #4221 around the same time as #4219. Mine strips trailing separators for that comparison only, so unresolved targets like `missing/` keep their spelling, and it adds a `./pkg/` case to the existing `referenceSourceDirectoryImports` CLI test, which checks each fixture against Node's `createRequire().resolve`. With it, your three cases match Node (`sub/actual.js`, `sub/actual.js`, `sub/index.js`). Happy to close mine in favour of #4219 if maintainers prefer. cc @morluto @kaoru0822-kitauji @N0zoM1z0

bsakaguchi referenced this issue

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

[GH-ISSUE #2383] [Tracker] Audit analysis and debugging coverage for the TIOBE top 100 languages (October 2026) #794

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Resolved by github.com/morluto/rea@baf16784af. Trailing-slash CommonJS resolution consumes package main before index fallback; public fixture checks nested/main.js instead of its deliberately distinct index.js. Validation: tests/boundary/cli/referenceSourceModuleSemantics.test.ts passed in the recorded round2/3 execution; this review did not rerun tests. Coverage is limited to the cited control; no broader real-provider, Windows or performance claim.

@morluto commented on GitHub (Oct 23, 2026): Resolved by https://github.com/morluto/rea/commit/baf16784af3592534687052431c145daef55cfd1. Trailing-slash CommonJS resolution consumes package main before index fallback; public fixture checks nested/main.js instead of its deliberately distinct index.js. Validation: `tests/boundary/cli/referenceSourceModuleSemantics.test.ts` passed in the recorded round2/3 execution; this review did not rerun tests. Coverage is limited to the cited control; no broader real-provider, Windows or performance claim.

bsakaguchi referenced this issue

2026-10-31 00:54:18 +00:00

[PR #1639] [MERGED] test(android): preserve reason in JADX cleanup-failure teardown poll #4716

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#1639

No description provided.

Rows
Columns