From c0904b4c1aabfccf2f0ab12f44960a0a1fbbbfae Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:10:32 +0800 Subject: [PATCH] fix(javascript): apply loader-owned module lookup rules Keep relative ESM imports exact, restrict Node legacy probes to .js/.json/.node, and treat empty package main as absent. Stop Node package-main lookup before nested package reinterpretation, while retaining generic artifact inference. Replace stale recursive and .cjs probe expectations. Verify CLI and MCP graph results against real Node 24 runtime controls, including exact suffixes, index fallback, and nested package main refusal. Refs #2020, #2021, #2022 --- .../JavaScriptArtifactPathResolution.test.ts | 194 +++++++++++++++++- .../JavaScriptArtifactPathResolution.ts | 87 +++++--- .../JavaScriptModuleLookupCli.test.ts | 168 +++++++++++++++ 3 files changed, 412 insertions(+), 37 deletions(-) create mode 100644 tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index 8af7aa19d2..2a05af6eb7 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -67,6 +67,147 @@ describe("artifact-local graph identity", () => { }); }); +describe("loader-owned path lookup", () => { + it("keeps relative ESM imports exact", () => { + const files = fileMap([ + file("app/entry.mjs", "root"), + file("app/feature.js", "root"), + file("app/directory/index.js", "root"), + file("app/feature.js.js", "root"), + ]); + for (const declaredPath of ["./feature", "./directory"]) { + expect( + resolve({ + declaredPath, + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + } + expect( + resolve({ + declaredPath: "./feature.js", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files, + }), + ).toMatchObject({ resolved_path: "app/feature.js" }); + }); + + it("does not use .cjs as an extensionless CommonJS probe", () => { + const files = fileMap([ + file("app/entry.cjs", "root"), + file("app/feature.cjs", "root"), + ]); + expect( + resolve({ + declaredPath: "./feature", + sourcePath: "app/entry.cjs", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + expect( + resolve({ + declaredPath: "./feature.cjs", + sourcePath: "app/entry.cjs", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ resolved_path: "app/feature.cjs" }); + }); + + it("treats an empty package main as absent and resolves the root index", () => { + const files = fileMap([ + file("app/src/entry.cjs", "root"), + file("app/node_modules/fixture/package.json", "root", '{"main":""}'), + file("app/node_modules/fixture/index.js", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/src/entry.cjs", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture/index.js", + }); + }); + + it.each(["import", "require"] as const)( + "limits %s package-main probes to Node's file extensions", + (moduleKind) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file( + "app/node_modules/fixture/package.json", + "root", + '{"main":"main"}', + ), + file("app/node_modules/fixture/main.cjs", "root"), + file("app/node_modules/fixture/index.mjs", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind, + files, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + }, + ); + + it.each(["import", "require"] as const)( + "does not recursively resolve a nested package from a %s package main", + (moduleKind) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/node_modules/fixture/package.json", "root", '{"main":"sub"}'), + file( + "app/node_modules/fixture/sub/package.json", + "root", + '{"main":"other"}', + ), + file("app/node_modules/fixture/sub/other.js", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind, + files, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + }, + ); + + it("still rejects non-string package main metadata", () => { + expect( + resolve({ + declaredPath: "./package", + sourcePath: "app/entry.js", + context: "filesystem-expression", + files: fileMap([ + file("app/entry.js", "root"), + file("app/package/package.json", "root", '{"main":false}'), + file("app/package/index.js", "root"), + ]), + }), + ).toMatchObject({ resolution_status: "unavailable", resolved_path: null }); + }); +}); + describe("logical path identity", () => { it("resolves an NFD relative import to the NFC inventory path and rejects a root escape", () => { const nfc = "caf\u00e9".normalize("NFC"); @@ -640,7 +781,6 @@ describe("package subpath directory ownership", () => { it.each([ ["malformed", "{", "not valid package JSON"], ["unavailable", null, "text is unavailable"], - ["cyclic", '{"main":"."}', "cycle"], ])("preserves %s nested directory failures", (_name, text, reason) => { const files = fileMap([ file("app/consumer.js", "root"), @@ -663,10 +803,34 @@ describe("package subpath directory ownership", () => { limitations: [expect.stringContaining(reason)], }); }); + + it("does not recurse through a cyclic nested main during CommonJS lookup", () => { + const files = fileMap([ + file("app/consumer.js", "root"), + file("app/node_modules/fixture/package.json", "root", "{}"), + file( + "app/node_modules/fixture/parser/package.json", + "root", + '{"main":"."}', + ), + ]); + expect( + resolve({ + declaredPath: "fixture/parser", + sourcePath: "app/consumer.js", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ + resolution_status: "not-found", + resolved_path: null, + }); + }); }); describe("package subpath legacy lookup and metadata failures", () => { - it("resolves legacy subpath files and indexes without an exports field", () => { + it("resolves CommonJS legacy subpath files and indexes without exports", () => { const files = fileMap([ file("app/src/consumer.js", "root"), file( @@ -674,7 +838,8 @@ describe("package subpath legacy lookup and metadata failures", () => { "root", '{"main":"./index.cjs"}', ), - file("app/node_modules/fixture/direct.cjs", "root"), + file("app/node_modules/fixture/direct.js", "root"), + file("app/node_modules/fixture/explicit.cjs", "root"), file("app/node_modules/fixture/folder/index.js", "root"), ]); @@ -688,7 +853,19 @@ describe("package subpath legacy lookup and metadata failures", () => { }), ).toMatchObject({ resolution_status: "resolved", - resolved_path: "app/node_modules/fixture/direct.cjs", + resolved_path: "app/node_modules/fixture/direct.js", + }); + expect( + resolve({ + declaredPath: "fixture/explicit.cjs", + sourcePath: "app/src/consumer.js", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture/explicit.cjs", }); expect( resolve({ @@ -1348,18 +1525,17 @@ describe("directory package entrypoint precedence", () => { }); }); - it("retains cycle diagnostics when directory packages refer to each other", () => { + it("retains cycle diagnostics for generic package-entrypoint inference", () => { const files = fileMap([ - file("app/consumer.js", "root"), + file("app/package.json", "root"), file("app/a/package.json", "root", '{"main":"../b"}'), file("app/b/package.json", "root", '{"main":"../a"}'), ]); expect( resolve({ declaredPath: "./a", - sourcePath: "app/consumer.js", - context: "module-specifier", - moduleKind: "require", + sourcePath: "app/package.json", + context: "package-entrypoint", files, }), ).toMatchObject({ diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index 0350143707..481a40453e 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -145,16 +145,21 @@ const resolvePath = (input: ResolveArtifactPathInput): CandidateResolution => { `The exact HTML resource ${confined} was not found among the selected application files.`, ]) ); - return resolveCandidate( - input, - confined, - new Set(), + const isModuleUrlPath = input.context === "module-specifier" && - input.moduleKind !== undefined && - (input.declaredPath.startsWith(".") || input.declaredPath.startsWith("/")) + (input.declaredPath.startsWith(".") || + input.declaredPath.startsWith("/") || + fileUrlPath(input.declaredPath) !== undefined); + const emittedSourceCandidates = + isModuleUrlPath && input.moduleKind !== undefined ? typescriptEmittedSourceCandidates(input.sourcePath, confined) - : [], - ); + : []; + if (isModuleUrlPath && input.moduleKind === "import") + return ( + resolveFileCandidates(input, [confined, ...emittedSourceCandidates]) ?? + notFoundCandidate() + ); + return resolveCandidate(input, confined, new Set(), emittedSourceCandidates); }; const rejectDeclaration = (declared: string): CandidateResolution | null => { @@ -275,7 +280,10 @@ const bareModuleCandidate = ( if (request.dependencyExportsEligible && metadata.kind !== "absent") return resolvePackageDependency(input, candidate, request, metadata); if (request.packageSubpath === ".") { - const direct = resolveFileCandidates(input, fileCandidates(candidate)); + const direct = resolveFileCandidates( + input, + fileCandidates(candidate, input.moduleKind), + ); if (direct !== null) return direct; } if (hasContainerDirectory(input.files, candidate, source?.container_sha256)) @@ -417,8 +425,10 @@ const resolvePackageDependency = ( if (rawExports === undefined) { if (subpath === ".") return ( - resolveFileCandidates(input, fileCandidates(directory)) ?? - resolveDirectory(input, directory, metadata, new Set()) + resolveFileCandidates( + input, + fileCandidates(directory, input.moduleKind), + ) ?? resolveDirectory(input, directory, metadata, new Set()) ); // Resolve legacy ESM subpaths as URLs: suffixes identify the loaded module // but do not belong to its filesystem path. Keep the declared specifier on @@ -753,7 +763,7 @@ const resolveCandidate = ( emittedSourceCandidates: readonly string[] = [], ): CandidateResolution => resolveFileCandidates(input, [ - ...fileCandidates(candidate), + ...fileCandidates(candidate, input.moduleKind), ...emittedSourceCandidates, ]) ?? resolveDirectory( @@ -771,8 +781,10 @@ const resolveDirectory = ( ): CandidateResolution => { if (metadata.kind === "absent") return ( - resolveFileCandidates(input, indexCandidates(directory)) ?? - notFoundCandidate() + resolveFileCandidates( + input, + indexCandidates(directory, input.moduleKind), + ) ?? notFoundCandidate() ); if (metadata.kind === "unavailable") return unresolvedCandidate("unavailable", [metadata.limitation]); @@ -780,15 +792,20 @@ const resolveDirectory = ( return unresolvedCandidate("unavailable", [ `Directory package entrypoint cycle includes ${metadata.path}.`, ]); - const preferred = [ - metadata.main, - ...(input.moduleKind === undefined ? [metadata.module] : []), - ]; - const entry = preferred.find((value) => value !== undefined); + const entry = + metadata.main === "" + ? undefined + : metadata.main !== undefined + ? metadata.main + : input.moduleKind === undefined + ? metadata.module + : undefined; if (entry === undefined) return ( - resolveFileCandidates(input, indexCandidates(directory)) ?? - notFoundCandidate() + resolveFileCandidates( + input, + indexCandidates(directory, input.moduleKind), + ) ?? notFoundCandidate() ); if (typeof entry !== "string" || entry.length === 0) return unresolvedCandidate("unavailable", [ @@ -809,10 +826,12 @@ const resolveDirectory = ( if (typeof confined !== "string") return confined; const selected = resolveFileCandidates(entryInput, [ - ...fileCandidates(confined), - ...indexCandidates(confined), - ]) ?? resolveFileCandidates(input, indexCandidates(directory)); + ...fileCandidates(confined, entryInput.moduleKind), + ...indexCandidates(confined, entryInput.moduleKind), + ]) ?? + resolveFileCandidates(input, indexCandidates(directory, input.moduleKind)); if (selected !== null) return selected; + if (input.moduleKind !== undefined) return notFoundCandidate(); return resolveCandidate( entryInput, confined, @@ -837,13 +856,25 @@ const resolveFileCandidates = ( return null; }; -const fileCandidates = (candidate: string): readonly string[] => [ +const NODE_MODULE_EXTENSIONS = [".js", ".json", ".node"] as const; + +const fileCandidates = ( + candidate: string, + moduleKind?: ResolveArtifactPathInput["moduleKind"], +): readonly string[] => [ candidate, - ...EXTENSIONS.map((extension) => `${candidate}${extension}`), + ...(moduleKind === undefined ? EXTENSIONS : NODE_MODULE_EXTENSIONS).map( + (extension) => `${candidate}${extension}`, + ), ]; -const indexCandidates = (candidate: string): readonly string[] => - EXTENSIONS.map((extension) => posix.join(candidate, `index${extension}`)); +const indexCandidates = ( + candidate: string, + moduleKind?: ResolveArtifactPathInput["moduleKind"], +): readonly string[] => + (moduleKind === undefined ? EXTENSIONS : NODE_MODULE_EXTENSIONS).map( + (extension) => posix.join(candidate, `index${extension}`), + ); type PackageTargetBlockCause = "null" | "empty-array"; diff --git a/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts new file mode 100644 index 0000000000..e57f96adb4 --- /dev/null +++ b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts @@ -0,0 +1,168 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; +import { analyzeJavaScriptDirectory } from "../../support/cli/javascriptApplication.js"; +import { writeFixtureFiles } from "../../support/javascriptApplicationFixture.js"; + +const execute = promisify(execFile); + +const createModuleLookupFixture = async () => { + const root = await createTestTempDirectory("rea-module-lookup-"); + await writeFixtureFiles(root, { + "package.json": JSON.stringify({ name: "module-lookup-fixture" }), + "esm/entry.mjs": [ + 'import "./extensionless";', + 'import "./directory";', + 'import "./explicit.js";', + 'import "./exact.js";', + 'import fallback from "empty-main";', + 'import "nested-main";', + "void fallback;", + ].join("\n"), + "esm/extensionless.js": "module.exports = 1;", + "esm/directory/index.js": "module.exports = 2;", + "esm/explicit.js.js": "module.exports = 3;", + "esm/exact.js": "module.exports = 4;", + "cjs/entry.cjs": [ + 'require("./extensionless");', + 'require("./feature");', + 'require("./feature.cjs");', + 'require("./default-js");', + 'require("./default-json");', + 'require("./default-node");', + 'require("empty-main");', + 'require("nested-main");', + ].join("\n"), + "cjs/extensionless.cjs": "module.exports = 1;", + "cjs/feature.cjs": "module.exports = 2;", + "cjs/default-js.js": "module.exports = 3;", + "cjs/default-json.json": JSON.stringify({ value: 4 }), + // Inventory-only placeholder: never pass this path to Node's runtime. + "cjs/default-node.node": "source-owned opaque native-module placeholder", + "node_modules/empty-main/package.json": JSON.stringify({ main: "" }), + "node_modules/empty-main/index.js": "module.exports = 42;", + "node_modules/nested-main/package.json": JSON.stringify({ main: "sub" }), + "node_modules/nested-main/sub/package.json": JSON.stringify({ + main: "other", + }), + "node_modules/nested-main/sub/other.js": "module.exports = 99;", + "runtime-control.mjs": [ + 'const specifiers = ["./esm/extensionless", "./esm/directory", "./esm/explicit.js", "./esm/exact.js", "empty-main", "nested-main"];', + "const results = [];", + "for (const specifier of specifiers) {", + " try { await import(specifier); results.push({ specifier, ok: true }); }", + " catch (error) { results.push({ specifier, ok: false, code: error.code }); }", + "}", + "console.log(JSON.stringify(results));", + ].join("\n"), + "runtime-control.cjs": [ + 'const specifiers = ["./cjs/extensionless", "./cjs/feature", "./cjs/feature.cjs", "./cjs/default-js", "./cjs/default-json", "empty-main", "nested-main"];', + "const results = [];", + "for (const specifier of specifiers) {", + " try { require(specifier); results.push({ specifier, ok: true }); }", + " catch (error) { results.push({ specifier, ok: false, code: error.code }); }", + "}", + "console.log(JSON.stringify(results));", + ].join("\n"), + }); + return root; +}; + +cliTest.for(["CLI", "stdio MCP"] as const)( + "matches Node module lookup for ESM and CommonJS through %s", + async (surface, { cli }) => { + const root = await createModuleLookupFixture(); + const [esmControl, cjsControl] = await Promise.all([ + execute(process.execPath, ["runtime-control.mjs"], { + cwd: root, + timeout: 5_000, + }), + execute(process.execPath, ["runtime-control.cjs"], { + cwd: root, + timeout: 5_000, + }), + ]); + const esmResults = JSON.parse(esmControl.stdout) as Array<{ + readonly specifier: string; + readonly ok: boolean; + readonly code?: string; + }>; + const cjsResults = JSON.parse(cjsControl.stdout) as Array<{ + readonly specifier: string; + readonly ok: boolean; + readonly code?: string; + }>; + expect(esmResults).toEqual([ + { + specifier: "./esm/extensionless", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, + { + specifier: "./esm/directory", + ok: false, + code: "ERR_UNSUPPORTED_DIR_IMPORT", + }, + { + specifier: "./esm/explicit.js", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, + { specifier: "./esm/exact.js", ok: true }, + { specifier: "empty-main", ok: true }, + { specifier: "nested-main", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + ]); + expect(cjsResults).toEqual([ + { + specifier: "./cjs/extensionless", + ok: false, + code: "MODULE_NOT_FOUND", + }, + { + specifier: "./cjs/feature", + ok: false, + code: "MODULE_NOT_FOUND", + }, + { specifier: "./cjs/feature.cjs", ok: true }, + { specifier: "./cjs/default-js", ok: true }, + { specifier: "./cjs/default-json", ok: true }, + { specifier: "empty-main", ok: true }, + { specifier: "nested-main", ok: false, code: "MODULE_NOT_FOUND" }, + ]); + + const { graph } = await analyzeJavaScriptDirectory(surface, cli, root); + const expectations = [ + ["import", "./extensionless", null, "not-found"], + ["import", "./directory", null, "not-found"], + ["import", "./explicit.js", null, "not-found"], + ["import", "./exact.js", "esm/exact.js", "resolved"], + ["import", "empty-main", "node_modules/empty-main/index.js", "resolved"], + ["import", "nested-main", null, "not-found"], + ["require", "./extensionless", null, "not-found"], + ["require", "./feature", null, "not-found"], + ["require", "./feature.cjs", "cjs/feature.cjs", "resolved"], + ["require", "./default-js", "cjs/default-js.js", "resolved"], + ["require", "./default-json", "cjs/default-json.json", "resolved"], + ["require", "./default-node", "cjs/default-node.node", "resolved"], + ["require", "empty-main", "node_modules/empty-main/index.js", "resolved"], + ["require", "nested-main", null, "not-found"], + ] as const; + + for (const [kind, specifier, resolvedPath, status] of expectations) + expect(graph.edges).toContainEqual( + expect.objectContaining({ + relation: "imports", + properties: expect.objectContaining({ + module_link_kind: kind, + specifier, + resolved_path: resolvedPath, + resolution_status: status, + }), + }), + ); + }, +);