From e09cfe9b5970d75da478c9dd741451cc2be5c91a Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Sat, 10 Oct 2026 13:47:15 +0800 Subject: [PATCH] fix(javascript): invalidate references stored through property targets (#1445) * fix(javascript): invalidate references stored through property targets * refactor(javascript): share rest keys and return-shape fixtures --- docs/javascript-application-workflows.md | 6 +- src/domain/javascript/javascriptAstValues.ts | 10 ++ .../javascript/javascriptSemanticAnalysis.ts | 14 +-- .../javascriptSemanticIterationSources.ts | 69 ++++++++++--- .../javascriptIterationAliases.test.ts | 81 ++------------- .../applications/javascriptSlotStores.test.ts | 98 +++++++++++++++++++ tests/fixtures/javascriptReturnShapes.ts | 91 +++++++++++++++++ 7 files changed, 271 insertions(+), 98 deletions(-) create mode 100644 tests/acceptance/applications/javascriptSlotStores.test.ts create mode 100644 tests/fixtures/javascriptReturnShapes.ts diff --git a/docs/javascript-application-workflows.md b/docs/javascript-application-workflows.md index f26c16d1f..bdc0dcf21 100644 --- a/docs/javascript-application-workflows.md +++ b/docs/javascript-application-workflows.md @@ -183,8 +183,10 @@ Direct return expressions, including expression-bodied arrows, are evaluated through an execution-free value lattice. Literal object fields and direct return sites are represented in the result. Calls, dynamic spreads, computed keys, and parser recovery remain partial or unknown. -Objects and arrays passed to calls, constructors, or tagged templates, or used -as method receivers, are not assumed unchanged after the invocation. Aliases +Objects and arrays passed to calls, constructors, or tagged templates, used +as method receivers, or stored through property targets are not assumed +unchanged afterward. Property stores conservatively retain reference escape +uncertainty rather than pretending to execute later writes. Aliases and shared children in spread and rest copies retain that uncertainty; copied primitive slots and unrelated containing properties remain known. Object rest excludes consumed keys, array rest excludes the consumed prefix for known diff --git a/src/domain/javascript/javascriptAstValues.ts b/src/domain/javascript/javascriptAstValues.ts index f0a0528c0..5479c8d1f 100644 --- a/src/domain/javascript/javascriptAstValues.ts +++ b/src/domain/javascript/javascriptAstValues.ts @@ -76,6 +76,16 @@ export const semanticStaticPropertyKey = ( return !computed && t.isIdentifier(property) ? property.name : null; }; +/** Read exact consumed object-pattern keys, leaving dynamic keys unknown. */ +export const semanticObjectPatternKeys = ( + pattern: t.ObjectPattern, +): readonly string[] => + pattern.properties.flatMap((property) => { + if (t.isRestElement(property)) return []; + const key = semanticStaticPropertyKey(property.key, property.computed); + return key === null ? [] : [key]; + }); + /** * Display an exact JavaScript string as nonempty label text. Graph labels and * artifact-local keys are nonempty, so the legal empty value is shown as `""`; diff --git a/src/domain/javascript/javascriptSemanticAnalysis.ts b/src/domain/javascript/javascriptSemanticAnalysis.ts index 1cf53fb31..452d29074 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.ts @@ -14,7 +14,10 @@ import { immutableSemanticBindings, immutableSemanticScopes, } from "./javascriptSemanticProjection.js"; -import { semanticStaticPropertyKey } from "./javascriptAstValues.js"; +import { + semanticObjectPatternKeys, + semanticStaticPropertyKey, +} from "./javascriptAstValues.js"; import type { JavaScriptSemanticAnalysisState, JavaScriptSemanticBindingState, @@ -880,14 +883,7 @@ const bindPattern = (input: BindPatternInput): void => { referenceOnly: true, copyKind: "object-rest", copyProjectionOffset: projection.length, - copyExcludedKeys: pattern.properties.flatMap((property) => { - if (t.isRestElement(property)) return []; - const key = semanticStaticPropertyKey( - property.key, - property.computed, - ); - return key === null ? [] : [key]; - }), + copyExcludedKeys: semanticObjectPatternKeys(pattern), }); } else { const name = semanticStaticPropertyKey(property.key, property.computed); diff --git a/src/domain/javascript/javascriptSemanticIterationSources.ts b/src/domain/javascript/javascriptSemanticIterationSources.ts index c83eb94ad..ea3708e51 100644 --- a/src/domain/javascript/javascriptSemanticIterationSources.ts +++ b/src/domain/javascript/javascriptSemanticIterationSources.ts @@ -1,14 +1,21 @@ import * as t from "@babel/types"; -import { semanticStaticPropertyKey } from "./javascriptAstValues.js"; -import { semanticArrayIndex } from "./javascriptSemanticPropertyPaths.js"; +import { + semanticObjectPatternKeys, + semanticStaticPropertyKey, +} from "./javascriptAstValues.js"; +import { + semanticArrayIndex, + type JavaScriptSemanticPropertyPath, +} from "./javascriptSemanticPropertyPaths.js"; +import { compareUnicodeCodePoints } from "../unicodeCodePointOrder.js"; interface IterationSourceValue { readonly node: t.Node; - readonly projection: readonly (string | number | null)[]; + readonly projection: JavaScriptSemanticPropertyPath; } -/** An iterator receiver or yielded reference store, with default guards. */ +/** An iterator receiver or property reference store, with default guards. */ export interface SemanticIterationSource extends IterationSourceValue { readonly kind: "iteration" | "reference-store"; readonly mutation: t.Node; @@ -69,11 +76,25 @@ const visit = ( guards: SourceGuards = {}, ): void => { const { sources, mutation } = context; - if (t.isForOfStatement(mutation) && t.isMemberExpression(pattern)) { - // Storing a yielded reference in a property has no lexical loop binding. - // Preserve its escape rather than assuming later writes cannot reach it. + if (t.isMemberExpression(pattern)) { + // Property stores have no lexical alias binding. Preserve the selected + // reference's escape rather than claiming later writes cannot reach it. if (source !== undefined) - add(sources, source, mutation, guards, "reference-store"); + add( + sources, + source.restStartIndex === undefined + ? source + : { + node: source.node, + projection: [ + ...source.projection, + { excludedKeys: [], startIndex: source.restStartIndex }, + ], + }, + mutation, + guards, + "reference-store", + ); return; } if (t.isTSParameterProperty(pattern)) { @@ -104,8 +125,29 @@ const visit = ( } if (t.isObjectPattern(pattern)) { for (const property of pattern.properties) { - // Object rest creates a fresh object and has no iterable binding pattern. - if (t.isRestElement(property)) continue; + if (t.isRestElement(property)) { + // A rest copy owns its scalar slots; only selected child references + // escape when that fresh copy is stored through a member target. + visit( + context, + property.argument, + source === undefined + ? undefined + : { + node: source.node, + projection: [ + ...source.projection, + { + excludedKeys: [ + ...new Set(semanticObjectPatternKeys(pattern)), + ].sort(compareUnicodeCodePoints), + }, + ], + }, + guards, + ); + continue; + } visit( context, property.value, @@ -144,7 +186,7 @@ const visit = ( visit(context, pattern.argument, undefined, guards); }; -/** Describe implicit iteration sources without deciding their runtime values. */ +/** Describe iterator receivers and property reference stores without evaluation. */ export const semanticIterationSources = ( node: t.Node, parent: t.Node | null, @@ -153,7 +195,10 @@ export const semanticIterationSources = ( const context = { sources, mutation: node }; if (t.isVariableDeclarator(node) && node.init != null) visit(context, node.id, { node: node.init, projection: [] }); - else if (t.isAssignmentExpression(node) && node.operator === "=") + else if ( + t.isAssignmentExpression(node) && + ["=", "||=", "&&=", "??="].includes(node.operator) + ) visit(context, node.left, { node: node.right, projection: [] }); else if (t.isForOfStatement(node)) { add(sources, { node: node.right, projection: [] }, node, {}); diff --git a/tests/acceptance/applications/javascriptIterationAliases.test.ts b/tests/acceptance/applications/javascriptIterationAliases.test.ts index 103995480..a7f039eb8 100644 --- a/tests/acceptance/applications/javascriptIterationAliases.test.ts +++ b/tests/acceptance/applications/javascriptIterationAliases.test.ts @@ -1,13 +1,9 @@ -import { Client } from "@modelcontextprotocol/client"; -import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; -import { writeFile } from "node:fs/promises"; -import { join, resolve } from "node:path"; import { expect } from "vitest"; -import { toolContract } from "../../../src/contracts/toolContracts.js"; -import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; -import { projectedExportReturnShapesSchema } from "../../../src/domain/javascript/javascriptExportShapeComparisonSchemas.js"; -import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { + verifyJavaScriptReturnShapes, + type JavaScriptReturnFields, +} from "../../fixtures/javascriptReturnShapes.js"; import { cliTest } from "../../support/cli/cliFixture.js"; const source = [ @@ -37,27 +33,7 @@ const source = [ "export function shadowed() { const t = { x: 1 }; const shared = { x: 3 }; for (const t of [shared]) t.x = 2; return t.x; }", ].join("\n"); -const assertReturns = (value: unknown): void => { - const analysis = javascriptApplicationAnalysisResultSchema.parse(value); - const projections = analysis.graph.nodes - .flatMap(({ observations }) => observations) - .filter( - ({ properties }) => properties.semantic_role === "export-return-shapes", - ) - .map(({ properties }) => - projectedExportReturnShapesSchema.parse(properties), - ); - const fields = (name: string) => { - const projection = projections.find( - ({ exported_name }) => exported_name === name, - ); - if (projection === undefined) throw new Error(`Missing export ${name}`); - expect(projection.return_shape_coverage).toMatchObject({ - status: "complete", - projection_complete: true, - }); - return projection.static_return_shapes.flatMap(({ fields }) => fields); - }; +const assertReturns = (fields: JavaScriptReturnFields): void => { expect(fields("inline")).toEqual( expect.arrayContaining([ expect.objectContaining({ path: "/x", state: "unknown" }), @@ -95,51 +71,6 @@ const assertReturns = (value: unknown): void => { cliTest( "preserves yielded-object mutation uncertainty through CLI and stdio MCP", - async ({ cli }) => { - const root = await createTestTempDirectory("rea-iteration-aliases-"); - await writeFile(join(root, "app.js"), source); - const cliResponse = await cli.run({ - arguments: [ - "analyze-javascript-application", - root, - "--artifact-format", - "directory", - "--json", - ], - }); - expect(cliResponse.exitCode).toBe(0); - const contract = toolContract("analyze_javascript_application"); - const cliEvidence = contract.outputSchema.parse(cliResponse.json); - assertReturns(cliEvidence.normalized_result); - const client = new Client({ name: "iteration-aliases-e2e", version: "1" }); - const transport = new StdioClientTransport({ - command: process.execPath, - args: [resolve("scripts/rea.mjs"), "mcp"], - cwd: process.cwd(), - env: { PATH: process.env.PATH ?? "" }, - stderr: "pipe", - }); - transport.stderr?.on("data", () => undefined); - try { - await client.connect(transport); - const response = await client.callTool({ - name: "analyze_javascript_application", - arguments: { input_path: root, format: "directory" }, - }); - expect(response.isError).not.toBe(true); - const evidence = contract.outputSchema.parse(response.structuredContent); - expect(evidence.evidence_id).toBe(cliEvidence.evidence_id); - assertReturns(evidence.normalized_result); - const text = response.content.find((item) => item.type === "text"); - if (text?.type !== "text") throw new Error("Missing MCP text result"); - assertReturns( - contract.outputSchema.parse(JSON.parse(text.text)).normalized_result, - ); - await client.ping(); - } finally { - await client.close(); - await transport.close(); - } - }, + ({ cli }) => verifyJavaScriptReturnShapes(cli, source, assertReturns), 120_000, ); diff --git a/tests/acceptance/applications/javascriptSlotStores.test.ts b/tests/acceptance/applications/javascriptSlotStores.test.ts new file mode 100644 index 000000000..3af7e3eee --- /dev/null +++ b/tests/acceptance/applications/javascriptSlotStores.test.ts @@ -0,0 +1,98 @@ +import { expect } from "vitest"; + +import { + verifyJavaScriptReturnShapes, + type JavaScriptReturnFields, +} from "../../fixtures/javascriptReturnShapes.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +const stores = { + object: "const o = {a: {x: 0}}; o.a = shared; o.a.x = 2;", + array: "const arr = [{x: 0}]; arr[0] = shared; arr[0].x = 2;", + captured: "const o = {a: {x: 0}}; o.a = shared; const t = o.a; t.x = 2;", + nested: "const o = {a: {b: {x: 0}}}; o.a.b = shared; o.a.b.x = 2;", + quoted: 'const o = {}; o["a/b~c"] = shared; o["a/b~c"].x = 2;', + dynamic: 'const o = {}; const key = "entry"; o[key] = shared; o[key].x = 2;', + receiverAlias: "const o = {}; const t = o; t.a = shared; o.a.x = 2;", + transitive: "const p = {}; const o = {}; o.a = p; p.b = shared; o.a.b.x = 2;", + overwritten: "const o = {}; o.a = {x: 0}; o.a = shared; o.a.x = 2;", + replacedReference: + "const o = {}; const other = {x: 0}; o.a = other; o.a = shared; o.a.x = 2;", + copiedDestination: + "const o = {}; o.a = shared; const copy = {...o}; copy.a.x = 2;", + iteration: "const arr = []; arr[0] = shared; for (const t of arr) t.x = 2;", + nestedArray: "const o = {}; o.a = [shared]; o.a[0].x = 2;", + orAssignment: "const o = {a: null}; o.a ||= shared; o.a.x = 2;", + nullishAssignment: "const o = {a: null}; o.a ??= shared; o.a.x = 2;", + andAssignment: "const o = {a: {x: 0}}; o.a &&= shared; o.a.x = 2;", + objectPattern: "const o = {}; ({child: o.a} = {child: shared}); o.a.x = 2;", + arrayPattern: "const o = {}; [o.a] = [shared]; o.a.x = 2;", + defaultPattern: "const o = {}; ({child: o.a = shared} = {}); o.a.x = 2;", + objectRestMember: + "const o = {}; ({...o.a} = {child: shared}); o.a.child.x = 2;", + arrayRestMember: "const o = {}; [...o.a] = [shared]; o.a[0].x = 2;", + iteratedObjectRest: + "const o = {}; for ({...o.a} of [{child: shared}]) o.a.child.x = 2;", + iteratedArrayRest: "const o = {}; for ([...o.a] of [[shared]]) o.a[0].x = 2;", + classField: + "class Holder {} const o = new Holder(); o.a = shared; o.a.x = 2;", +}; +const source = [ + ...Object.entries(stores).map( + ([name, body]) => + `export function ${name}() { const shared = {x: 1}; ${body} return shared.x; }`, + ), + "export function primitive() { const n = 1; const o = {}; o.a = n; o.a = 2; return n; }", + "export function nestedChild() { const shared = {inner: {x: 1}}; const o = {}; o.a = shared; o.a.inner.x = 2; return shared.inner.x; }", + "export function readOnlyStore() { const shared = {x: 1}; const o = {}; o.a = shared; const captured = o.a.x; return shared.x; }", + "export function copiedScalar() { const shared = {x: 1}; const o = {}; o.a = {...shared}; o.a.x = 2; return shared.x; }", + "export function copiedArrayScalar() { const shared = {x: 1}; const o = {}; o.a = [shared.x]; o.a[0] = 2; return shared.x; }", + "export function unrelated() { const shared = {x: 1}; const other = {x: 9}; const o = {}; o.a = shared; o.a.x = 2; return other.x; }", + "export function copiedChild() { const child = {x: 1}; const parent = {child, keep: 7}; const o = {}; o.a = {...parent}; o.a.child.x = 2; return {child: child.x, keep: parent.keep}; }", + "export function objectRestExclusion() { const child = {x: 1}; const skip = {x: 3}; const parent = {child, skip, keep: 7}; const o = {}; let ignored; ({skip: ignored, ...o.a} = parent); o.a.child.x = 2; return {child: child.x, skip: skip.x, keep: parent.keep}; }", + "export function arrayRestOffset() { const child = {x: 1}; const skip = {x: 3}; const items = [skip, child]; const o = {}; let ignored; [ignored, ...o.a] = items; o.a[0].x = 2; return {child: child.x, skip: items[0].x}; }", +].join("\n"); + +const assertReturns = (fields: JavaScriptReturnFields): void => { + for (const name of [...Object.keys(stores), "nestedChild"]) { + const field = fields(name).find(({ path }) => path === ""); + expect(field, name).toBeDefined(); + expect(field?.state, name).toMatch(/^(unknown|literal)$/); + if (field?.state === "literal") expect(field.value, name).toBe(2); + } + const readOnly = fields("readOnlyStore").find(({ path }) => path === ""); + expect(readOnly?.state).toMatch(/^(unknown|literal)$/); + if (readOnly?.state === "literal") expect(readOnly.value).toBe(1); + for (const [name, value] of Object.entries({ + primitive: 1, + copiedScalar: 1, + copiedArrayScalar: 1, + unrelated: 9, + })) + expect(fields(name), name).toContainEqual( + expect.objectContaining({ path: "", state: "literal", value }), + ); + const child = fields("copiedChild").find(({ path }) => path === "/child"); + expect(child?.state).toMatch(/^(unknown|literal)$/); + if (child?.state === "literal") expect(child.value).toBe(2); + expect(fields("copiedChild")).toContainEqual( + expect.objectContaining({ path: "/keep", state: "literal", value: 7 }), + ); + for (const name of ["objectRestExclusion", "arrayRestOffset"]) { + const child = fields(name).find(({ path }) => path === "/child"); + expect(child?.state, name).toMatch(/^(unknown|literal)$/); + if (child?.state === "literal") expect(child.value, name).toBe(2); + expect(fields(name), name).toContainEqual( + expect.objectContaining({ path: "/skip", state: "literal", value: 3 }), + ); + } + expect(fields("objectRestExclusion")).toContainEqual( + expect.objectContaining({ path: "/keep", state: "literal", value: 7 }), + ); +}; + +cliTest( + "does not report stale literals after storing references in member slots", + ({ cli }) => verifyJavaScriptReturnShapes(cli, source, assertReturns), + 120_000, +); diff --git a/tests/fixtures/javascriptReturnShapes.ts b/tests/fixtures/javascriptReturnShapes.ts new file mode 100644 index 000000000..02c66bc84 --- /dev/null +++ b/tests/fixtures/javascriptReturnShapes.ts @@ -0,0 +1,91 @@ +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { expect } from "vitest"; + +import { toolContract } from "../../src/contracts/toolContracts.js"; +import { javascriptApplicationAnalysisResultSchema } from "../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { projectedExportReturnShapesSchema } from "../../src/domain/javascript/javascriptExportShapeComparisonSchemas.js"; +import { createTestTempDirectory } from "./temporaryDirectory.js"; +import type { TestCli } from "../support/cli/cliFixture.js"; + +const exportedReturnFields = (value: unknown) => { + const analysis = javascriptApplicationAnalysisResultSchema.parse(value); + const projections = analysis.graph.nodes + .flatMap(({ observations }) => observations) + .filter( + ({ properties }) => properties.semantic_role === "export-return-shapes", + ) + .map(({ properties }) => + projectedExportReturnShapesSchema.parse(properties), + ); + return (name: string) => { + const projection = projections.find( + ({ exported_name }) => exported_name === name, + ); + if (projection === undefined) throw new Error(`Missing export ${name}`); + expect(projection.return_shape_coverage).toMatchObject({ + status: "complete", + projection_complete: true, + }); + return projection.static_return_shapes.flatMap(({ fields }) => fields); + }; +}; + +/** Select complete return fields from a real JavaScript application result. */ +export type JavaScriptReturnFields = ReturnType; + +/** Exercise identical return assertions through the compiled CLI and stdio MCP. */ +export const verifyJavaScriptReturnShapes = async ( + cli: TestCli, + source: string, + check: (fields: JavaScriptReturnFields) => void, +): Promise => { + const root = await createTestTempDirectory("rea-return-shapes-"); + await writeFile(join(root, "app.js"), source); + const cliResponse = await cli.run({ + arguments: [ + "analyze-javascript-application", + root, + "--artifact-format", + "directory", + "--json", + ], + }); + expect(cliResponse.exitCode).toBe(0); + const contract = toolContract("analyze_javascript_application"); + const cliEvidence = contract.outputSchema.parse(cliResponse.json); + check(exportedReturnFields(cliEvidence.normalized_result)); + const client = new Client({ name: "return-shapes-e2e", version: "1" }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [resolve("scripts/rea.mjs"), "mcp"], + cwd: process.cwd(), + env: { PATH: process.env.PATH ?? "" }, + stderr: "pipe", + }); + transport.stderr?.on("data", () => undefined); + try { + await client.connect(transport); + const response = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: root, format: "directory" }, + }); + expect(response.isError).not.toBe(true); + const evidence = contract.outputSchema.parse(response.structuredContent); + expect(evidence.evidence_id).toBe(cliEvidence.evidence_id); + check(exportedReturnFields(evidence.normalized_result)); + const text = response.content.find((item) => item.type === "text"); + if (text?.type !== "text") throw new Error("Missing MCP text result"); + check( + exportedReturnFields( + contract.outputSchema.parse(JSON.parse(text.text)).normalized_result, + ), + ); + await client.ping(); + } finally { + await client.close(); + await transport.close(); + } +};