From 20aeaf110fe10c0d99ea6819678e3aa2350c41dd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:28:59 +0800 Subject: [PATCH] fix(javascript): preserve incomplete package and artifact identity facts (#10032) fix(javascript): preserve artifact identity and manifest uncertainty --- ...ScriptArtifactAnalysis.packageType.test.ts | 124 ++++++++++-------- .../javascript/JavaScriptArtifactAnalysis.ts | 4 + .../JavaScriptArtifactPathResolution.test.ts | 27 ++++ .../JavaScriptArtifactPathResolution.ts | 43 +++--- .../JavaScriptJsonDocuments.test.ts | 7 +- .../javascript/JavaScriptJsonDocuments.ts | 1 + 6 files changed, 133 insertions(+), 73 deletions(-) diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.packageType.test.ts b/src/application/javascript/JavaScriptArtifactAnalysis.packageType.test.ts index 920769b4e6..c743c7420f 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.packageType.test.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.packageType.test.ts @@ -26,58 +26,72 @@ const included = ( text: { included: true, value: text }, }); -it("leaves the package type unknown behind a nearer unavailable manifest", async () => { - const files = [ - included("package.json", "package-json", '{"type":"module"}'), - included("main.js", "javascript", 'require("./dep.cjs");'), - included("nested/main.js", "javascript", 'require("../dep.cjs");'), - included( - "nested/inner/package.json", - "package-json", - '{"type":"commonjs"}', - ), - included("nested/inner/main.js", "javascript", 'require("../../dep.cjs");'), - ]; - const unavailable: JavaScriptUnanalyzedArtifactFile = { - path: "nested/package.json", - container_sha256: container, - sha256: null, - bytes: null, - inventory_artifact_id: null, - kind: "package-json", - limitations: ["Inventoried package bytes are unavailable."], - }; - const observed = new Map(); - const javascript = analyzeJavaScriptStaticSource(""); - const worker = { - analyze: async ( - file: JavaScriptArtifactFile, - _capacity: unknown, - _signal?: AbortSignal, - _onPhase?: unknown, - packageType?: JavaScriptPackageType, - ) => { - observed.set(file.path, packageType); - return ok({ javascript, module: null, projection: null }); - }, - }; - await analyzeAndAdoptJavaScriptArtifactFiles( - { - files, - unavailable_relevant_files: [unavailable], - containers: [], - text_bytes_read: 0, - invalid_utf8_files: 0, - }, - worker, - createJavaScriptSemanticGraphProjection(), - async () => {}, - ); - // The nearest package.json owns a source's scope even when its bytes are - // unavailable, so the root type must not reach nested/main.js. - expect(Object.fromEntries(observed)).toEqual({ - "main.js": "module", - "nested/main.js": "unknown", - "nested/inner/main.js": "commonjs", - }); -}); +it.each(["complete", "unknown"] as const)( + "keeps nearer package scope unknown when unavailable or unobserved (%s inventory)", + async (status) => { + const files = [ + included("package.json", "package-json", '{"type":"module"}'), + included("main.js", "javascript", 'require("./dep.cjs");'), + included("missing/main.js", "javascript", 'require("../dep.cjs");'), + included("nested/main.js", "javascript", 'require("../dep.cjs");'), + included( + "nested/inner/package.json", + "package-json", + '{"type":"commonjs"}', + ), + included( + "nested/inner/main.js", + "javascript", + 'require("../../dep.cjs");', + ), + ]; + const unavailable: JavaScriptUnanalyzedArtifactFile = { + path: "nested/package.json", + container_sha256: container, + sha256: null, + bytes: null, + inventory_artifact_id: null, + kind: "package-json", + limitations: ["Inventoried package bytes are unavailable."], + }; + const observed = new Map(); + const javascript = analyzeJavaScriptStaticSource(""); + const worker = { + analyze: async ( + file: JavaScriptArtifactFile, + _capacity: unknown, + _signal?: AbortSignal, + _onPhase?: unknown, + packageType?: JavaScriptPackageType, + ) => { + observed.set(file.path, packageType); + return ok({ javascript, module: null, projection: null }); + }, + }; + await analyzeAndAdoptJavaScriptArtifactFiles( + { + files, + unavailable_relevant_files: [unavailable], + containers: [], + text_bytes_read: 0, + invalid_utf8_files: 0, + unanalyzed_files: { + coverage: { scope: "container-entries", status }, + get: () => undefined, + hasDirectory: () => false, + }, + }, + worker, + createJavaScriptSemanticGraphProjection(), + async () => {}, + ); + // The nearest package.json owns a source's scope even when its bytes are + // unavailable, so the root type must not reach nested/main.js. + expect(Object.fromEntries(observed)).toEqual({ + "main.js": "module", + "missing/main.js": status === "complete" ? "module" : "unknown", + "nested/main.js": "unknown", + "nested/inner/main.js": "commonjs", + }); + }, +); diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.ts b/src/application/javascript/JavaScriptArtifactAnalysis.ts index b48f50851a..bbed58a4d3 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.ts @@ -486,6 +486,10 @@ export const createJavaScriptPackageTypeLookup = ( const type = document.packageFields.type; return type === "module" || type === "commonjs" ? type : "unspecified"; } + // An unvisited nearer manifest may override an observed ancestor's type. + const coverage = fileSet.unanalyzed_files?.coverage; + if (coverage !== undefined && coverage.status !== "complete") + return "unknown"; if (directory === "." || directory === "" || directory === "/") break; } return "unknown"; diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index 653747c6f1..6b6cd8e945 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -190,6 +190,33 @@ describe("relative ESM URL separators", () => { }); describe("literal POSIX inventory base segments", () => { + it.each([ + ["app/\ud800/main.js", "./dep.js", null], + ["app/\udfff/main.js", "./dep.js", null], + ["app/\ud800/main.js", "../dep.js", "app/dep.js"], + ["app/\ud800.js", "./dep.js", "app/dep.js"], + ["app/😀%literal/main.js", "./dep.js", "app/😀%literal/dep.js"], + ["app/%2F/main.js", "./dep.js", "app/%2F/dep.js"], + ] as const)( + "preserves URL-reference base uncertainty for %j with %s", + (sourcePath, declaredPath, target) => { + expect( + resolve({ + declaredPath, + sourcePath, + context: "url-reference", + files: fileMap([ + file(sourcePath, "root"), + file(target ?? "app/�/dep.js", "root"), + ]), + }), + ).toMatchObject({ + declared_path: declaredPath, + resolved_path: target, + resolution_status: target === null ? "unavailable" : "resolved", + }); + }, + ); it("validates the final URL after a parent reference removes the encoded base segment", () => { expect( resolve({ diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index cfddaf68ec..702b36bad3 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -128,17 +128,15 @@ const EXTENSIONS = [ // A private URL namespace for logical artifact paths, including root packages. // Encode inventory segments so filename punctuation remains literal in the base. const PACKAGE_ARTIFACT_ROOT_URL = new URL("file:///rea-artifact/"); -const encodePackageMetadataUrl = (path: string): URL => - new URL( - path.split("/").map(encodeURIComponent).join("/"), - PACKAGE_ARTIFACT_ROOT_URL, - ); +const encodeArtifactPathUrl = (path: string, root: URL): URL => + new URL(path.split("/").map(encodeURIComponent).join("/"), root); // URL identity cannot represent lone UTF-16 surrogates. Use request-local // opaque segments only for structural admission, never inventory lookup. A // target cannot spell these identities after URL's tab/newline preprocessing. -const packageMetadataUrl = ( +const artifactPathUrl = ( path: string, relativeTarget: string, + root: URL = PACKAGE_ARTIFACT_ROOT_URL, ): Result< URL, { @@ -148,7 +146,7 @@ const packageMetadataUrl = ( } > => { try { - return ok(encodePackageMetadataUrl(path)); + return ok(encodeArtifactPathUrl(path, root)); } catch (cause: unknown) { if (!(cause instanceof URIError)) throw cause; const reserved = new Set( @@ -175,9 +173,9 @@ const packageMetadataUrl = ( }) .join("/"); return err({ - structuralUrl: new URL(structuralPath, PACKAGE_ARTIFACT_ROOT_URL), + structuralUrl: new URL(structuralPath, root), structuralSegments, - limitation: `Package metadata path ${JSON.stringify(path)} contains an unpaired UTF-16 surrogate and cannot be represented as a URL.`, + limitation: `Artifact path ${JSON.stringify(path)} contains an unpaired UTF-16 surrogate and cannot be represented as a URL.`, }); } }; @@ -680,7 +678,7 @@ const contextualCandidate = ( return unresolvedCandidate("rejected", [ "The module URL escapes the selected logical artifact root.", ]); - const sourceUrl = packageMetadataUrl(input.sourcePath, declared); + const sourceUrl = artifactPathUrl(input.sourcePath, declared); const base = sourceUrl.ok ? sourceUrl.value : sourceUrl.error.structuralUrl; @@ -728,11 +726,15 @@ const contextualCandidate = ( // after dot segments reach the root. Parse before percent decoding so an // encoded drive-like filename stays literal. The synthetic base establishes // only this ambiguity; it does not establish the source's runtime scheme. - const sourceUrlPath = input.sourcePath - .split("/") - .map(encodeURIComponent) - .join("/"); - const fileReference = URL.parse(declared, `file:///${sourceUrlPath}`); + const sourceUrl = artifactPathUrl( + input.sourcePath, + declared, + new URL("file:///"), + ); + const fileReference = URL.parse( + declared, + sourceUrl.ok ? sourceUrl.value : sourceUrl.error.structuralUrl, + ); if (fileReference === null) return unresolvedCandidate("rejected", [ "The URL reference cannot be parsed as a URL.", @@ -746,6 +748,13 @@ const contextualCandidate = ( return unresolvedCandidate("rejected", [ "The URL reference path percent-decodes to NUL or to bytes that are not UTF-8.", ]); + if ( + !sourceUrl.ok && + fileReference.pathname + .split("/") + .some((segment) => sourceUrl.error.structuralSegments.has(segment)) + ) + return unresolvedCandidate("unavailable", [sourceUrl.error.limitation]); const candidate = urlRelativeCandidate( posix.dirname(input.sourcePath), decoded, @@ -1125,7 +1134,7 @@ const resolveLegacyEsmSubpath = ( depth--; } else if (segment !== ".") depth++; } - const metadataUrl = packageMetadataUrl( + const metadataUrl = artifactPathUrl( posix.join(directory, "package.json"), subpath, ); @@ -1952,7 +1961,7 @@ const selectStringPackageTarget = ( `contains forbidden path segment ${JSON.stringify(forbidden)} before URL normalization`, mapKind, ); - const metadataUrl = packageMetadataUrl(packagePath, value); + const metadataUrl = artifactPathUrl(packagePath, value); const packageUrl = metadataUrl.ok ? metadataUrl.value : metadataUrl.error.structuralUrl; diff --git a/src/application/javascript/JavaScriptJsonDocuments.test.ts b/src/application/javascript/JavaScriptJsonDocuments.test.ts index 2a09772a4b..344c72351c 100644 --- a/src/application/javascript/JavaScriptJsonDocuments.test.ts +++ b/src/application/javascript/JavaScriptJsonDocuments.test.ts @@ -26,7 +26,7 @@ it("retains only declared package metadata despite inherited host fields", () => declaredFile.text = { included: true, value: - '{"name":"source-name","version":"1.0.0","main":null,"browser":"source-browser.cjs"}', + '{"name":"source-name","version":"1.0.0","main":null,"browser":"source-browser.cjs","dependencies":{"next":"source-next"}}', }; const inherited = { name: "host-name", @@ -35,6 +35,9 @@ it("retains only declared package metadata despite inherited host fields", () => renderer: "host-renderer.cjs", browser: "host-browser.cjs", module: "host-module.mjs", + dependencies: { next: "host-next" }, + devDependencies: { next: "host-next-dev" }, + peerDependencies: { next: "host-next-peer" }, }; const previous = Object.fromEntries( Object.keys(inherited).map((key) => [ @@ -63,6 +66,8 @@ it("retains only declared package metadata despite inherited host fields", () => } expect(absent.status).toBe("included"); expect(declared.status).toBe("included"); + expect(absent).toMatchObject({ nextDependency: "unproven" }); + expect(declared).toMatchObject({ nextDependency: "declared" }); if (absent.packageFields === null || declared.packageFields === null) throw new Error("Expected package metadata fields"); expect(Object.getPrototypeOf(absent.packageFields)).toBeNull(); diff --git a/src/application/javascript/JavaScriptJsonDocuments.ts b/src/application/javascript/JavaScriptJsonDocuments.ts index a4f96d1d45..1fb39ce3db 100644 --- a/src/application/javascript/JavaScriptJsonDocuments.ts +++ b/src/application/javascript/JavaScriptJsonDocuments.ts @@ -151,6 +151,7 @@ const parseDocument = ( const nextDeclared = snapshot.kind === "package-json" && ["dependencies", "devDependencies", "peerDependencies"].some((key) => { + if (!Object.hasOwn(record, key)) return false; const dependencies: unknown = record[key]; if ( typeof dependencies !== "object" ||