mirror of
https://github.com/morluto/rea.git
synced 2026-11-10 00:36:35 +00:00
fix(javascript): distinguish Promise and FileHandle releases (#9584)
* fix(javascript): distinguish Promise and FileHandle releases * fix(javascript): preserve reassigned handle candidates * test(javascript): select unisolated route fixtures explicitly * test(javascript): distinguish package scope in route fixtures * test(apple): separate identity and refusal fixture groups * fix(javascript): retain uncertain releases after opaque receiver writes --------- Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
1 parent
da3bb5aa7b
commit
759817ccbe
6 files changed
+335
-11
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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([
|
||||
|
||||
@@ -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" }]);
|
||||
});
|
||||
@@ -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<t.Node>(
|
||||
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<t.Node>,
|
||||
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)
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user