From 9f67e44e5d451d705bae9a579ad9802d5c9f097c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:26:48 +0800 Subject: [PATCH] fix(javascript): preserve loader-specific package ancestry Require a package directory for bare ESM imports. Continue CommonJS root and subpath lookup through ancestor packages after ordinary misses, while keeping exports authority, unavailable metadata, and failed nonempty main entrypoints terminal. Carry failed-main ownership in the internal resolution result and retain the nearest local miss after exhausting ancestors. Consolidate package selection at its owning boundary and qualify loader differences with real Node 24, CLI, and MCP regressions. --- .../JavaScriptArtifactPathResolution.test.ts | 186 ++++++++++++++++++ .../JavaScriptArtifactPathResolution.ts | 55 ++++-- .../JavaScriptModuleLookupCli.test.ts | 85 ++++++-- 3 files changed, 296 insertions(+), 30 deletions(-) diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index 2a05af6eb7..e44d2666ac 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -121,7 +121,66 @@ describe("loader-owned path lookup", () => { }), ).toMatchObject({ resolved_path: "app/feature.cjs" }); }); +}); +describe("bare package root file lookup", () => { + it("uses loader-specific bare package roots when only a sibling .js file exists", () => { + const absentDirectory = fileMap([ + file("app/entry.mjs", "root"), + file("app/node_modules/fixture.js", "root"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files: absentDirectory, + }), + ).toMatchObject({ resolution_status: "external", resolved_path: null }); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "require", + files: absentDirectory, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture.js", + }); + + const packageDirectory = fileMap([ + file("app/entry.mjs", "root"), + file("app/node_modules/fixture.js", "root"), + file("app/node_modules/fixture/package.json", "root", "{}"), + ]); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "import", + files: packageDirectory, + }), + ).toMatchObject({ resolution_status: "not-found", resolved_path: null }); + expect( + resolve({ + declaredPath: "fixture", + sourcePath: "app/entry.mjs", + context: "module-specifier", + moduleKind: "require", + files: packageDirectory, + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture.js", + }); + }); +}); + +describe("legacy package main lookup", () => { it("treats an empty package main as absent and resolves the root index", () => { const files = fileMap([ file("app/src/entry.cjs", "root"), @@ -208,6 +267,133 @@ describe("loader-owned path lookup", () => { }); }); +describe("CommonJS bare package ancestor lookup", () => { + it.each([ + ["absent metadata", undefined, true], + ["empty metadata", "{}", true], + ["empty main", '{"main":""}', true], + ["truthy missing main", '{"main":"missing"}', false], + ] as const)( + "handles %s before an ancestor package for both loaders", + (_name, packageJson, commonJsUsesAncestor) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/placeholder.js", "root"), + ...(packageJson === undefined + ? [] + : [ + file( + "app/src/node_modules/fixture/package.json", + "root", + packageJson, + ), + ]), + file("app/node_modules/fixture/index.js", "root"), + ]); + for (const moduleKind of ["import", "require"] as const) { + const resolution = resolve({ + declaredPath: "fixture", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind, + files, + }); + if (moduleKind === "require" && commonJsUsesAncestor) { + expect(resolution).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/node_modules/fixture/index.js", + }); + } else { + expect(resolution).toMatchObject({ + resolution_status: "not-found", + resolved_path: null, + }); + } + } + }, + ); +}); + +describe("CommonJS bare package subpath ancestor lookup", () => { + it.each([ + ["absent subpath", undefined, true], + ["empty subpath metadata", "{}", true], + ["truthy missing subpath main", '{"main":"missing"}', false], + ] as const)( + "handles %s before an ancestor package for both loaders", + (_name, packageJson, commonJsUsesAncestor) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/placeholder.js", "root"), + ...(packageJson === undefined + ? [] + : [ + file( + "app/src/node_modules/fixture/parser/package.json", + "root", + packageJson, + ), + file( + "app/src/node_modules/fixture/parser/placeholder.js", + "root", + ), + ]), + file("app/node_modules/fixture/parser.js", "root"), + ]); + const commonJsResolution = resolve({ + declaredPath: "fixture/parser", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "require", + files, + }); + expect(commonJsResolution).toMatchObject({ + resolved_path: commonJsUsesAncestor + ? "app/node_modules/fixture/parser.js" + : null, + }); + const esmResolution = resolve({ + declaredPath: "fixture/parser", + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "import", + files, + }); + expect(esmResolution.resolved_path).toBeNull(); + }, + ); +}); + +describe("CommonJS bare package exhausted lookup", () => { + it.each([ + ["empty main", "fixture", '{"main":""}', "app/src/node_modules/fixture"], + [ + "missing subpath", + "fixture/parser", + "{}", + "app/src/node_modules/fixture/parser", + ], + ] as const)( + "preserves not-found for an exhausted %s lookup", + (_name, declaredPath, packageJson, packageDirectory) => { + const files = fileMap([ + file("app/src/entry.js", "root"), + file("app/src/node_modules/fixture/package.json", "root", packageJson), + file(`${packageDirectory}/placeholder.js`, "root"), + ]); + expect( + resolve({ + declaredPath, + sourcePath: "app/src/entry.js", + context: "module-specifier", + moduleKind: "require", + files, + }), + ).toMatchObject({ resolution_status: "not-found", 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"); diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index 481a40453e..9845e8c033 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -100,6 +100,8 @@ type CandidateResolution = readonly resolvedPath: null; readonly status: UnresolvedArtifactPathStatus; readonly limitations: readonly string[]; + /** A truthy package main failed, so Node must not try ancestor packages. */ + readonly stopsAncestorLookup?: true; }; /** Resolve one declaration while preserving its original input identity. */ @@ -274,23 +276,39 @@ const bareModuleCandidate = ( } const source = input.files.get(input.sourcePath); let directory = posix.dirname(input.sourcePath); + let nearestNotFound: CandidateResolution | null = null; while (true) { const candidate = posix.join(directory, dependencyDirectory); const metadata = readPackageMetadata(input, candidate); - if (request.dependencyExportsEligible && metadata.kind !== "absent") - return resolvePackageDependency(input, candidate, request, metadata); - if (request.packageSubpath === ".") { - const direct = resolveFileCandidates( - input, - fileCandidates(candidate, input.moduleKind), - ); - if (direct !== null) return direct; + const continueCommonJsLookup = + input.moduleKind === "require" && + (metadata.kind === "absent" || + (metadata.kind === "available" && + (!request.dependencyExportsEligible || + metadata.exports === undefined))); + const packageResolution = + metadata.kind !== "absent" || + hasContainerDirectory(input.files, candidate, source?.container_sha256) + ? resolvePackageDependency(input, candidate, request, metadata) + : request.packageSubpath === "." && input.moduleKind !== "import" + ? resolveFileCandidates( + input, + fileCandidates(candidate, input.moduleKind), + ) + : null; + if (packageResolution !== null) { + if ( + continueCommonJsLookup && + packageResolution.status === "not-found" && + packageResolution.stopsAncestorLookup !== true + ) { + nearestNotFound ??= packageResolution; + } else return packageResolution; } - if (hasContainerDirectory(input.files, candidate, source?.container_sha256)) - return resolvePackageDependency(input, candidate, request, metadata); if (directory === "." || directory === "") break; directory = posix.dirname(directory); } + if (nearestNotFound !== null) return nearestNotFound; return unresolvedCandidate("external", [ "No matching bare package was inventoried in an enclosing node_modules directory.", ]); @@ -425,10 +443,12 @@ const resolvePackageDependency = ( if (rawExports === undefined) { if (subpath === ".") return ( - resolveFileCandidates( - input, - fileCandidates(directory, input.moduleKind), - ) ?? resolveDirectory(input, directory, metadata, new Set()) + (input.moduleKind === "import" + ? null + : 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 @@ -831,6 +851,8 @@ const resolveDirectory = ( ]) ?? resolveFileCandidates(input, indexCandidates(directory, input.moduleKind)); if (selected !== null) return selected; + if (input.moduleKind === "require") + return { ...notFoundCandidate(), stopsAncestorLookup: true }; if (input.moduleKind !== undefined) return notFoundCandidate(); return resolveCandidate( entryInput, @@ -1102,7 +1124,10 @@ const packageExportConditions = ( "default", ]); -const notFoundCandidate = (): CandidateResolution => ({ +const notFoundCandidate = (): Extract< + CandidateResolution, + { readonly resolvedPath: null } +> => ({ resolvedPath: null, status: "not-found", limitations: [ diff --git a/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts index e57f96adb4..3e33183278 100644 --- a/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts +++ b/tests/acceptance/applications/JavaScriptModuleLookupCli.test.ts @@ -21,6 +21,10 @@ const createModuleLookupFixture = async () => { 'import "./exact.js";', 'import fallback from "empty-main";', 'import "nested-main";', + 'import "file-only";', + 'import "ancestor-empty";', + 'import "ancestor-blocked";', + 'import "ancestor-sub/parser";', "void fallback;", ].join("\n"), "esm/extensionless.js": "module.exports = 1;", @@ -36,6 +40,10 @@ const createModuleLookupFixture = async () => { 'require("./default-node");', 'require("empty-main");', 'require("nested-main");', + 'require("file-only");', + 'require("ancestor-empty");', + 'require("ancestor-blocked");', + 'require("ancestor-sub/parser");', ].join("\n"), "cjs/extensionless.cjs": "module.exports = 1;", "cjs/feature.cjs": "module.exports = 2;", @@ -50,8 +58,12 @@ const createModuleLookupFixture = async () => { 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"];', + "node_modules/file-only.js": "module.exports = 7;", + "node_modules/ancestor-empty/index.js": "module.exports = 8;", + "node_modules/ancestor-blocked/index.js": "module.exports = 9;", + "node_modules/ancestor-sub/parser.js": "module.exports = 10;", + "esm/runtime-control.mjs": [ + 'const specifiers = ["./extensionless", "./directory", "./explicit.js", "./exact.js", "empty-main", "nested-main", "file-only", "ancestor-empty", "ancestor-blocked", "ancestor-sub/parser"];', "const results = [];", "for (const specifier of specifiers) {", " try { await import(specifier); results.push({ specifier, ok: true }); }", @@ -59,8 +71,8 @@ const createModuleLookupFixture = async () => { "}", "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"];', + "cjs/runtime-control.cjs": [ + 'const specifiers = ["./extensionless", "./feature", "./feature.cjs", "./default-js", "./default-json", "empty-main", "nested-main", "file-only", "ancestor-empty", "ancestor-blocked", "ancestor-sub/parser"];', "const results = [];", "for (const specifier of specifiers) {", " try { require(specifier); results.push({ specifier, ok: true }); }", @@ -69,6 +81,15 @@ const createModuleLookupFixture = async () => { "console.log(JSON.stringify(results));", ].join("\n"), }); + for (const directory of ["esm", "cjs"]) + await writeFixtureFiles(root, { + [`${directory}/node_modules/ancestor-empty/package.json`]: JSON.stringify( + { main: "" }, + ), + [`${directory}/node_modules/ancestor-blocked/package.json`]: + JSON.stringify({ main: "missing" }), + [`${directory}/node_modules/ancestor-sub/package.json`]: "{}", + }); return root; }; @@ -77,11 +98,11 @@ cliTest.for(["CLI", "stdio MCP"] as const)( async (surface, { cli }) => { const root = await createModuleLookupFixture(); const [esmControl, cjsControl] = await Promise.all([ - execute(process.execPath, ["runtime-control.mjs"], { + execute(process.execPath, ["esm/runtime-control.mjs"], { cwd: root, timeout: 5_000, }), - execute(process.execPath, ["runtime-control.cjs"], { + execute(process.execPath, ["cjs/runtime-control.cjs"], { cwd: root, timeout: 5_000, }), @@ -98,40 +119,56 @@ cliTest.for(["CLI", "stdio MCP"] as const)( }>; expect(esmResults).toEqual([ { - specifier: "./esm/extensionless", + specifier: "./extensionless", ok: false, code: "ERR_MODULE_NOT_FOUND", }, { - specifier: "./esm/directory", + specifier: "./directory", ok: false, code: "ERR_UNSUPPORTED_DIR_IMPORT", }, { - specifier: "./esm/explicit.js", + specifier: "./explicit.js", ok: false, code: "ERR_MODULE_NOT_FOUND", }, - { specifier: "./esm/exact.js", ok: true }, + { specifier: "./exact.js", ok: true }, { specifier: "empty-main", ok: true }, { specifier: "nested-main", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { specifier: "file-only", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { specifier: "ancestor-empty", ok: false, code: "ERR_MODULE_NOT_FOUND" }, + { + specifier: "ancestor-blocked", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, + { + specifier: "ancestor-sub/parser", + ok: false, + code: "ERR_MODULE_NOT_FOUND", + }, ]); expect(cjsResults).toEqual([ { - specifier: "./cjs/extensionless", + specifier: "./extensionless", ok: false, code: "MODULE_NOT_FOUND", }, { - specifier: "./cjs/feature", + specifier: "./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: "./feature.cjs", ok: true }, + { specifier: "./default-js", ok: true }, + { specifier: "./default-json", ok: true }, { specifier: "empty-main", ok: true }, { specifier: "nested-main", ok: false, code: "MODULE_NOT_FOUND" }, + { specifier: "file-only", ok: true }, + { specifier: "ancestor-empty", ok: true }, + { specifier: "ancestor-blocked", ok: false, code: "MODULE_NOT_FOUND" }, + { specifier: "ancestor-sub/parser", ok: true }, ]); const { graph } = await analyzeJavaScriptDirectory(surface, cli, root); @@ -142,6 +179,10 @@ cliTest.for(["CLI", "stdio MCP"] as const)( ["import", "./exact.js", "esm/exact.js", "resolved"], ["import", "empty-main", "node_modules/empty-main/index.js", "resolved"], ["import", "nested-main", null, "not-found"], + ["import", "file-only", null, "external"], + ["import", "ancestor-empty", null, "not-found"], + ["import", "ancestor-blocked", null, "not-found"], + ["import", "ancestor-sub/parser", null, "not-found"], ["require", "./extensionless", null, "not-found"], ["require", "./feature", null, "not-found"], ["require", "./feature.cjs", "cjs/feature.cjs", "resolved"], @@ -150,6 +191,20 @@ cliTest.for(["CLI", "stdio MCP"] as const)( ["require", "./default-node", "cjs/default-node.node", "resolved"], ["require", "empty-main", "node_modules/empty-main/index.js", "resolved"], ["require", "nested-main", null, "not-found"], + ["require", "file-only", "node_modules/file-only.js", "resolved"], + [ + "require", + "ancestor-empty", + "node_modules/ancestor-empty/index.js", + "resolved", + ], + ["require", "ancestor-blocked", null, "not-found"], + [ + "require", + "ancestor-sub/parser", + "node_modules/ancestor-sub/parser.js", + "resolved", + ], ] as const; for (const [kind, specifier, resolvedPath, status] of expectations)