diff --git a/docs/guides/javascript/javascript-application-workflows.md b/docs/guides/javascript/javascript-application-workflows.md index 483239b8e4..e1968273b3 100644 --- a/docs/guides/javascript/javascript-application-workflows.md +++ b/docs/guides/javascript/javascript-application-workflows.md @@ -226,6 +226,12 @@ registrations/removals and dispatch, Node timers and cancellation handles, async `node:child_process` creation and ownership, configuration sources and direct defaults, request construction and response consumers, parse/coercion/ validation boundaries, and built-in resource acquisition/release. +For `fs/promises.open`, a binding receiving the Promise is distinct from one +receiving the awaited FileHandle. Awaiting the Promise later does not replace +the original binding, but `h = await h` can replace it with the FileHandle. +Receiver writes can therefore retain a candidate release and its ownership +ambiguity; a still-known Promise has no direct FileHandle release. Acquisitions +remain retained, and unknown loader authority keeps its existing candidate tag. Reassignments connect retained right-hand reads to the binding or property write. Compound assignments also retain the previous-value input; logical assignments keep conditional inputs as candidates. These edges describe diff --git a/src/domain/javascript/javascriptBuiltinResourceMethods.ts b/src/domain/javascript/javascriptBuiltinResourceMethods.ts index 95f66f250b..791bb254f3 100644 --- a/src/domain/javascript/javascriptBuiltinResourceMethods.ts +++ b/src/domain/javascript/javascriptBuiltinResourceMethods.ts @@ -7,7 +7,11 @@ export const BUILTIN_RESOURCE_METHODS = { } as const; /** - * Release methods the value Node returns from each acquisition actually has. + * Release methods the resource handle from each acquisition actually has. + * + * `fs/promises.open` returns a Promise; the resource owner admits FileHandle + * methods when receiver writes can supply its fulfilled value, retaining + * ambiguity and the existing qualifier for conditional built-in identity. * * `fs.openSync` returns a numeric descriptor and `fs.open` in its callback form * returns `undefined`, so neither has a release method at all; the descriptor is diff --git a/src/domain/javascript/javascriptSemanticAnalysis.builtin_defaults.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.builtin_defaults.test.ts index 38741665e0..d18f80d26f 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.builtin_defaults.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.builtin_defaults.test.ts @@ -562,7 +562,9 @@ it.each([ it("preserves a saved promises object after deleting its parent export", () => { const ir = analyzeCommonJsSemanticFixture(`const fs = require('fs'); const fsp = fs.promises; - delete fs.promises; const handle = fsp.open('kept'); handle.close();`); + delete fs.promises; + async function useHandle() { const handle = await fsp.open('kept'); handle.close(); } + useHandle();`); expect( ir.resourceOperations.map(({ kind, method }) => ({ kind, method })), ).toEqual([ @@ -591,7 +593,9 @@ it("recognizes nested ESM namespace receivers as original built-ins", () => { it("keeps intact fs/promises identity after deleting the fs.promises slot", () => { const ir = analyzeCommonJsSemanticFixture(`const fs=require('fs'); delete fs.promises; - const fsp=require('fs/promises'); const handle=fsp.open('kept'); handle.close();`); + const fsp=require('fs/promises'); + async function useHandle() { const handle=await fsp.open('kept'); handle.close(); } + useHandle();`); expect( ir.resourceOperations.map(({ kind, method }) => ({ kind, method })), ).toEqual([ diff --git a/src/domain/javascript/javascriptSemanticAnalysis.resource_handles.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.resource_handles.test.ts index 2b0d5ea098..6cb01c4a4e 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.resource_handles.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.resource_handles.test.ts @@ -1,16 +1,46 @@ import { expect, it } from "vitest"; -import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; +import { + analyzeJavaScriptSemantics, + analyzeParsedJavaScriptSemantics, +} from "./javascriptSemanticAnalysis.js"; +import { parseJavaScriptSource } from "./javascriptSourceParser.js"; +import { analyzeCommonJsSemanticFixture } from "../../../tests/fixtures/javascriptCommonJsAnalysis.js"; + +const operations = (source: string) => { + const result = analyzeJavaScriptSemantics(source); + expect(result.coverage.status).toBe("complete"); + return result.resourceOperations; +}; const releases = (source: string) => - analyzeJavaScriptSemantics(source) - .resourceOperations.filter(({ kind }) => kind === "release") + operations(source) + .filter(({ kind }) => kind === "release") .map(({ method, linkedResourceIds, resolution }) => ({ method, linked: linkedResourceIds.length, resolution, })); +const commonJsPromise = `const { open } = require("node:fs/promises"); const h = open("input.txt"); h.close();`; + +it("keeps conditional release facts when the loader is unknown", () => { + expect( + operations(commonJsPromise).filter(({ kind }) => kind === "release"), + ).toEqual([expect.objectContaining({ builtinAuthority: "candidate" })]); +}); + +it("does not close a Promise through a proven CommonJS builtin", () => { + const result = analyzeCommonJsSemanticFixture(commonJsPromise); + expect(result.coverage.status).toBe("complete"); + expect( + result.resourceOperations.filter(({ kind }) => kind === "acquire"), + ).toHaveLength(1); + expect( + result.resourceOperations.filter(({ kind }) => kind === "release"), + ).toEqual([]); +}); + it.each([ [ "a declaration", @@ -57,3 +87,114 @@ it.each([ `), ).toEqual([{ method: "destroy", linked: 1, resolution: "partial" }]); }); + +it.each([ + 'const h = open("input.txt"); h.close();', + 'let h; h = open("input.txt"); h.close();', + 'const h = open("input.txt"); await h; h.close();', + 'const h = open("input.txt"); await h.close();', + 'const h = open("input.txt"); h["close"]();', + 'const h = open("input.txt"); const later = () => h.close();', +])("does not release a FileHandle through its Promise: %s", (body) => { + const result = operations(`import { open } from "node:fs/promises"; ${body}`); + expect(result.filter(({ kind }) => kind === "acquire")).toHaveLength(1); + expect(result.filter(({ kind }) => kind === "release")).toEqual([]); +}); + +it.each([ + 'const stream = await await open("input.txt");', + 'const stream = (await (open("input.txt")));', +])("retains the directly awaited FileHandle through %s", (acquire) => { + expect( + releases( + `import { open } from "node:fs/promises"; ${acquire} stream.close();`, + ), + ).toEqual([{ method: "close", linked: 1, resolution: "complete" }]); +}); + +it("keeps the awaited candidate partial when a Promise can replace it", () => { + const result = operations(` + import { open } from "node:fs/promises"; + let h = await open("input.txt"); + if (replace) h = open("other.txt"); + h.close(); + `); + expect(result.filter(({ kind }) => kind === "acquire")).toHaveLength(2); + expect( + result + .filter(({ kind }) => kind === "release") + .map(({ linkedResourceIds, resolution }) => ({ + linked: linkedResourceIds.length, + resolution, + })), + ).toEqual([{ linked: 1, resolution: "partial" }]); +}); + +it.each([ + "h = await h;", + "if (settle) h = await h;", + "h = await (await h);", + "h = await transform(h);", + 'const ready = await h; eval("h = ready");', +])("keeps a candidate FileHandle after receiver write %s", (write) => { + const result = operations(` + import { open } from "node:fs/promises"; + let h = open("input.txt"); + ${write} + h.close(); + `); + const acquired = result.filter(({ kind }) => kind === "acquire"); + expect(acquired).toHaveLength(1); + expect(result.filter(({ kind }) => kind === "release")).toEqual([ + expect.objectContaining({ + linkedResourceIds: [acquired[0]?.resourceId], + resolution: "partial", + }), + ]); +}); + +it.each([ + 'h = open("other.txt"); h.close();', + "h = null; h.close();", + "h.close(); h = await h;", +])("does not invent a fulfilled receiver for %s", (after) => { + expect( + releases( + `import { open } from "node:fs/promises"; let h = open("input.txt"); ${after}`, + ), + ).toEqual([]); +}); + +it("preserves ambiguity through a write from an unknown loader", () => { + const parsed = parseJavaScriptSource( + ` + import { open } from "node:fs/promises"; + const other = require("node:fs/promises"); + let h = open("input.txt"); + h = other.open("other.txt"); + h.close(); + `, + "entry.MJS", + ); + if (parsed === null) throw new Error("Expected an unknown-loader fixture"); + const ir = analyzeParsedJavaScriptSemantics(parsed); + expect(ir.coverage.status).toBe("complete"); + const result = ir.resourceOperations; + const acquired = result.filter(({ kind }) => kind === "acquire"); + expect(acquired).toHaveLength(2); + expect(result.filter(({ kind }) => kind === "release")).toEqual([ + expect.objectContaining({ + linkedResourceIds: acquired.map(({ resourceId }) => resourceId).sort(), + resolution: "partial", + builtinAuthority: "candidate", + }), + ]); +}); + +it("keeps an awaited handle release uncertain after an opaque receiver write", () => { + expect( + releases( + `import {open} from "node:fs/promises";let h=await open("input.txt");eval("h = other");h.close();`, + ), + ).toEqual([{ method: "close", linked: 1, resolution: "partial" }]); +}); diff --git a/src/domain/javascript/javascriptSemanticResources.ts b/src/domain/javascript/javascriptSemanticResources.ts index 2ac936911a..f4f72a4ba9 100644 --- a/src/domain/javascript/javascriptSemanticResources.ts +++ b/src/domain/javascript/javascriptSemanticResources.ts @@ -14,6 +14,8 @@ import { unwrapJavaScriptExpression, } from "./javascriptAstValues.js"; import { isUnmodifiedGlobalReference } from "./javascriptSemanticAmbientWrites.js"; +import { semanticOpaqueBindingReadMayHaveChanged } from "./javascriptSemanticOpaqueEffects.js"; +import { semanticBindingAtRead } from "./javascriptSemanticMutationInitializers.js"; import { resolveSemanticBindingState, type JavaScriptSemanticAnalysisState, @@ -51,6 +53,8 @@ interface ResourceCandidate { readonly method: JavaScriptSemanticResourceOperation["method"]; readonly ownerCallableId: string | null; readonly resultBindingId: string | null; + /** The initial result is the opening Promise; later writes may replace it. */ + readonly resultIsPromise: boolean; } /** Recover built-in filesystem/network acquisition and local release handles. */ @@ -345,6 +349,15 @@ const collectAcquisitions = ( : { builtinAuthority: reference.builtinAuthority }), ownerCallableId: context.callableStack.at(-1) ?? null, resultBindingId: outerDataEffectBinding(node, context), + resultIsPromise: + reference.builtinAuthority === undefined && + reference.specifier === "fs/promises" && + reference.method === "open" && + !context.ancestors.some( + (parent) => + t.isAwaitExpression(parent) && + unwrapJavaScriptExpression(parent.argument).node === node, + ), }); }); return output; @@ -381,8 +394,30 @@ const collectReleases = ( const candidates = binding === undefined ? [] : (byBinding.get(binding.bindingId) ?? []); if (binding === undefined || candidates.length === 0) return; - const linked = candidates.filter((acquisition) => - releaseMethodsFor(acquisition).includes(method), + const opaqueReceiver = semanticOpaqueBindingReadMayHaveChanged( + binding, + member.object, + context.state, + ); + const receiver = candidates.some(({ resultIsPromise }) => resultIsPromise) + ? semanticBindingAtRead(binding, member.object, context.state).binding + : binding; + const knownAcquisitions = new Set( + candidates + .filter(({ builtinAuthority }) => builtinAuthority === undefined) + .map(({ node }) => node), + ); + const linked = candidates.filter( + (acquisition) => + releaseMethodsFor(acquisition).includes(method) && + (!acquisition.resultIsPromise || + opaqueReceiver || + mayReceiveFulfilledHandle( + receiver, + acquisition, + knownAcquisitions, + context.state, + )), ); // No linked acquisition exposes this release method, so the call is not a // release of it at all: Node throws at the call site instead. @@ -399,6 +434,7 @@ const collectReleases = ( ? { builtinAuthority: "candidate" as const } : {}), resolution: + !opaqueReceiver && linked.length === 1 && holdsOnlyAcquisitions(binding, linked, context.state) ? "complete" @@ -408,6 +444,34 @@ const collectReleases = ( return output; }; +/** Unwrap only direct awaits; a standalone await does not write its operand. */ +const resourceInitializer = (node: t.Node) => { + let value = unwrapJavaScriptExpression(node).node; + let awaited = false; + while (t.isAwaitExpression(value)) { + awaited = true; + value = unwrapJavaScriptExpression(value.argument).node; + } + return { value, awaited }; +}; + +/** Unknown receiver writes retain a candidate; known other acquisitions do not alias it. */ +const mayReceiveFulfilledHandle = ( + binding: JavaScriptSemanticBindingState, + acquisition: ResourceCandidate, + knownAcquisitions: ReadonlySet, + state: JavaScriptSemanticAnalysisState, +): boolean => + [...binding.initializers, ...binding.referenceInitializers].some( + ({ node, projection }) => { + if (projection.length > 0) return true; + const { value, awaited } = resourceInitializer(node); + if (value === acquisition.node) return awaited; + if (isNullishPlaceholder(value, state)) return false; + return !knownAcquisitions.has(value); + }, + ); + /** * A lexical handle identifies its acquisition only while every value written * to the binding is that acquisition itself. Any replacement, even a @@ -423,9 +487,7 @@ const holdsOnlyAcquisitions = ( [...binding.initializers, ...binding.referenceInitializers].every( ({ node, projection }) => { if (projection.length > 0) return false; - let value = unwrapJavaScriptExpression(node).node; - if (t.isAwaitExpression(value)) - value = unwrapJavaScriptExpression(value.argument).node; + const { value } = resourceInitializer(node); return ( isNullishPlaceholder(value, state) || linked.some((acquisition) => acquisition.node === value) diff --git a/tests/acceptance/applications/applicationPromiseHandleCliMcp.test.ts b/tests/acceptance/applications/applicationPromiseHandleCliMcp.test.ts new file mode 100644 index 0000000000..6e4d110028 --- /dev/null +++ b/tests/acceptance/applications/applicationPromiseHandleCliMcp.test.ts @@ -0,0 +1,107 @@ +import { Ajv2020 } from "ajv/dist/2020.js"; +import { createHash } from "node:crypto"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest.for(["promise", "awaited", "reassigned", "opaque"] as const)( + "distinguishes the FileHandle carrier %s through CLI and MCP", + async (mode, { cli }) => { + const { client, call } = await connectLocalToolsMcp({ + REA_MCP_TOOL_FAMILIES: "electron", + REA_MCP_INPUT_SCHEMA_PROFILE: "full", + REA_MCP_OUTPUT_SCHEMA_PROFILE: "full", + }); + const definition = (await client.listTools()).tools.find( + ({ name }) => name === "analyze_javascript_application", + ); + if (definition === undefined) + throw new Error("Missing JavaScript analysis definition"); + const validate = new Ajv2020({ + strict: false, + validateFormats: false, + }).compile(definition.inputSchema); + const root = await createTestTempDirectory("rea-promise-handle-"); + const bodies = { + promise: 'const handle = open("input.txt", "r");', + awaited: 'const handle = await open("input.txt", "r");', + reassigned: 'let handle = open("input.txt", "r"); handle = await handle;', + opaque: + 'let handle = open("input.txt", "r"); const ready = await handle; eval("handle = ready");', + }; + const source = `import {open} from "node:fs/promises";\n${bodies[mode]}\nhandle.close();\n`; + const sha = createHash("sha256").update(source).digest("hex"); + await Promise.all([ + writeFile( + join(root, "package.json"), + JSON.stringify({ + name: "promise-handle", + type: "module", + main: "main.js", + }), + ), + writeFile(join(root, "main.js"), source), + ]); + const response = await cli.run({ + arguments: ["analyze-javascript-application", root, "--json"], + }); + expect(response.exitCode, response.stdout).toBe(0); + const args = { input_path: root }; + expect(validate(args), JSON.stringify(validate.errors)).toBe(true); + const mcp = await call(definition.name, args); + expect(mcp.isError, JSON.stringify(mcp)).not.toBe(true); + for (const evidence of [response.json, mcp.structuredContent]) { + const result = javascriptApplicationAnalysisResultSchema.parse( + parseEvidence(evidence).normalized_result, + ); + expect(result.statistics.parsed_javascript_files).toBe(1); + expect(result.statistics.parse_failures).toBe(0); + const graph = result.semantic_graph; + if (graph === undefined) throw new Error("Missing semantic graph"); + expect( + graph.nodes.filter(({ kind }) => kind === "resource"), + ).toHaveLength(1); + expect( + graph.relations.filter(({ relation }) => relation === "acquires"), + ).toHaveLength(1); + expect + .soft(graph.relations.filter(({ relation }) => relation === "releases")) + .toHaveLength(mode === "promise" ? 0 : 1); + if (mode === "reassigned" || mode === "opaque") { + const resource = graph.nodes.find(({ kind }) => kind === "resource"); + expect + .soft( + graph.relations.filter(({ relation }) => relation === "releases"), + ) + .toEqual([ + expect.objectContaining({ + resolution: "candidate", + target_node_id: resource?.node_id, + }), + ]); + expect.soft(graph.unknowns).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + family: "resource-lifecycle", + reason: "ambiguous-target", + relation_kinds: ["releases"], + candidate_node_ids: [resource?.node_id], + }), + ]), + ); + } + expect( + graph.nodes.some( + ({ identity }) => + identity.artifact_sha256 === sha && + identity.module_path === "main.js", + ), + ).toBe(true); + } + }, +);