From 53ed942a85f51ff9f0e4f01cbb53359df476071d Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Sat, 10 Oct 2026 05:11:38 +0800 Subject: [PATCH] fix(mcp): preserve JavaScript workflow identity validation (#1366) Preserve canonical distinct-source and unique-native-observation refinements after resolving inline or retained JavaScript workflow inputs. The shared application entry points now return structured invalid_request errors before graph parsing and computation, avoiding another complete graph capture. Six focused tests, actual compiled CLI/SDK stdio workflows, JSON Schema dialect checks, static/document checks, and a real Notion ASAR analysis/recovery/trace/close workflow passed. All 20 final-head checks, including real JavaScript recovery, passed. Native-provider acceptance is not claimed for this provider-neutral validation change. Fixes #1364 Refs #1065 --- .../JavaScriptApplicationWorkflowService.ts | 35 +- ...riptApplicationVersionComparisonSchemas.ts | 67 ++-- .../javascriptExportShapeComparisonSchemas.ts | 35 +- .../javascriptFeatureTraceSchemas.ts | 33 +- .../cli/workflowInputIdentities.test.ts | 318 ++++++++++++++++++ 5 files changed, 444 insertions(+), 44 deletions(-) create mode 100644 tests/boundary/cli/workflowInputIdentities.test.ts diff --git a/src/application/javascript/JavaScriptApplicationWorkflowService.ts b/src/application/javascript/JavaScriptApplicationWorkflowService.ts index f85330cee2..cdee1f8930 100644 --- a/src/application/javascript/JavaScriptApplicationWorkflowService.ts +++ b/src/application/javascript/JavaScriptApplicationWorkflowService.ts @@ -1,18 +1,24 @@ import { z } from "zod"; -import { - AnalysisInputError, - AnalysisProtocolError, -} from "../../domain/analysisErrorCore.js"; +import { AnalysisProtocolError } from "../../domain/analysisErrorCore.js"; import { type AnalysisError } from "../../domain/analysisErrorBase.js"; import type { Evidence } from "../../domain/evidence.js"; import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; import { compareJavaScriptApplicationVersions } from "../../domain/javascript/javascriptApplicationVersionComparison.js"; -import { compareApplicationVersionsInputSchema } from "../../domain/javascript/javascriptApplicationVersionComparisonSchemas.js"; +import { + compareApplicationVersionsInputSchema, + applicationVersionIdentitiesSchema, +} from "../../domain/javascript/javascriptApplicationVersionComparisonSchemas.js"; import { compareJavaScriptExportShapes } from "../../domain/javascript/javascriptExportShapeComparison.js"; -import { compareJavaScriptExportShapesInputSchema } from "../../domain/javascript/javascriptExportShapeComparisonSchemas.js"; +import { + compareJavaScriptExportShapesInputSchema, + javaScriptExportShapeIdentitiesSchema, +} from "../../domain/javascript/javascriptExportShapeComparisonSchemas.js"; import { traceApplicationFeature } from "../../domain/javascript/javascriptFeatureTrace.js"; -import { traceApplicationFeatureInputSchema } from "../../domain/javascript/javascriptFeatureTraceSchemas.js"; +import { + traceApplicationFeatureInputSchema, + applicationTraceIdentitiesSchema, +} from "../../domain/javascript/javascriptFeatureTraceSchemas.js"; import { err, ok, type Result } from "../../domain/result.js"; import { compareSourceToBundle } from "../../domain/javascript/sourceToBundleComparison.js"; import { compareSourceToBundleInputSchema } from "../../domain/javascript/sourceToBundleComparisonSchemas.js"; @@ -46,6 +52,11 @@ export const traceApplicationFeatureEvidenceValidated = ( input: z.output, ): Result => { const operation = "trace_application_feature"; + const identities = applicationTraceIdentitiesSchema.safeParse(input); + if (!identities.success) + return err( + analysisInputErrorFromIssues(operation, identities.error.issues, input), + ); const sourceResult = parseApplicationGraphEvidence(input.application, [ "application", ]); @@ -105,6 +116,11 @@ export const compareApplicationVersionsEvidenceValidated = ( input: z.output, ): Result => { const operation = "compare_application_versions"; + const identities = applicationVersionIdentitiesSchema.safeParse(input); + if (!identities.success) + return err( + analysisInputErrorFromIssues(operation, identities.error.issues, input), + ); const leftResult = parseApplicationGraphEvidence(input.left, ["left"]); if (!leftResult.ok) return err(applicationGraphEvidenceInputError(operation, leftResult.error)); @@ -223,6 +239,11 @@ export const compareJavaScriptExportShapesEvidenceValidated = ( input: z.output, ): Result => { const operation = "compare_javascript_export_shapes"; + const identities = javaScriptExportShapeIdentitiesSchema.safeParse(input); + if (!identities.success) + return err( + analysisInputErrorFromIssues(operation, identities.error.issues, input), + ); const leftResult = parseApplicationGraphEvidence(input.left, ["left"]); if (!leftResult.ok) return err(applicationGraphEvidenceInputError(operation, leftResult.error)); diff --git a/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts b/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts index e30c74205c..309c95c895 100644 --- a/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts +++ b/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts @@ -10,6 +10,52 @@ const evidenceIdSchema = prefixedDigestSchema("ev"); const nodeIdSchema = prefixedDigestSchema("jag_node"); const textSchema = z.string().min(1); +/** Check paired version identities and per-side native identity uniqueness. */ +const validateApplicationVersionIdentities = ( + input: { + readonly left: { readonly evidence_id: string }; + readonly right: { readonly evidence_id: string }; + readonly left_native_observations: readonly { + readonly evidence_id: string; + }[]; + readonly right_native_observations: readonly { + readonly evidence_id: string; + }[]; + }, + context: z.RefinementCtx, +): void => { + if (input.left.evidence_id === input.right.evidence_id) + context.addIssue({ + code: "custom", + path: ["right"], + message: "Application version Evidence must be distinct", + }); + for (const side of [ + "left_native_observations", + "right_native_observations", + ] as const) { + const ids = input[side].map(({ evidence_id: id }) => id); + if (new Set(ids).size !== ids.length) + context.addIssue({ + code: "custom", + path: [side], + message: "Native observations must be unique on each side", + }); + } +}; + +const evidenceIdentitySchema = z.object({ evidence_id: evidenceIdSchema }); + +/** Validate paired version identities without capturing whole analysis graphs. */ +export const applicationVersionIdentitiesSchema = z + .object({ + left: evidenceIdentitySchema, + right: evidenceIdentitySchema, + left_native_observations: z.array(evidenceIdentitySchema), + right_native_observations: z.array(evidenceIdentitySchema), + }) + .superRefine(validateApplicationVersionIdentities); + /** Two authenticated application versions and their native observations. */ export const compareApplicationVersionsInputSchema = z .strictObject({ @@ -18,26 +64,7 @@ export const compareApplicationVersionsInputSchema = z left_native_observations: z.array(evidenceSchema).default([]), right_native_observations: z.array(evidenceSchema).default([]), }) - .superRefine((input, context) => { - if (input.left.evidence_id === input.right.evidence_id) - context.addIssue({ - code: "custom", - path: ["right"], - message: "Application version Evidence must be distinct", - }); - for (const side of [ - "left_native_observations", - "right_native_observations", - ] as const) { - const ids = input[side].map(({ evidence_id: id }) => id); - if (new Set(ids).size !== ids.length) - context.addIssue({ - code: "custom", - path: [side], - message: "Native observations must be unique on each side", - }); - } - }); + .superRefine(validateApplicationVersionIdentities); const candidateNodesSchema = z.tuple([nodeIdSchema]).rest(nodeIdSchema); const exactMatchSchema = z.strictObject({ diff --git a/src/domain/javascript/javascriptExportShapeComparisonSchemas.ts b/src/domain/javascript/javascriptExportShapeComparisonSchemas.ts index 808066c4d2..5f94250157 100644 --- a/src/domain/javascript/javascriptExportShapeComparisonSchemas.ts +++ b/src/domain/javascript/javascriptExportShapeComparisonSchemas.ts @@ -53,6 +53,32 @@ const sourceRangeSchema = z }); }); +/** Check that export-shape comparisons bind distinct source Evidence. */ +const validateJavaScriptExportShapeIdentities = ( + input: { + readonly left: { readonly evidence_id: string }; + readonly right: { readonly evidence_id: string }; + }, + context: z.RefinementCtx, +): void => { + if (input.left.evidence_id === input.right.evidence_id) + context.addIssue({ + code: "custom", + path: ["right"], + message: "JavaScript export shape Evidence must be distinct", + }); +}; + +const evidenceIdentitySchema = z.object({ evidence_id: evidenceIdSchema }); + +/** Validate export-shape identities without capturing complete source graphs. */ +export const javaScriptExportShapeIdentitiesSchema = z + .object({ + left: evidenceIdentitySchema, + right: evidenceIdentitySchema, + }) + .superRefine(validateJavaScriptExportShapeIdentities); + /** Two authenticated application graphs and exact export selectors. */ export const compareJavaScriptExportShapesInputSchema = z .strictObject({ @@ -63,14 +89,7 @@ export const compareJavaScriptExportShapesInputSchema = z right_module_path: textSchema, right_export_name: textSchema, }) - .superRefine((input, context) => { - if (input.left.evidence_id === input.right.evidence_id) - context.addIssue({ - code: "custom", - path: ["right"], - message: "JavaScript export shape Evidence must be distinct", - }); - }); + .superRefine(validateJavaScriptExportShapeIdentities); const projectedReturnFieldShape = { path: jsonPointerSchema, diff --git a/src/domain/javascript/javascriptFeatureTraceSchemas.ts b/src/domain/javascript/javascriptFeatureTraceSchemas.ts index 517d7c9ad7..ee34bad092 100644 --- a/src/domain/javascript/javascriptFeatureTraceSchemas.ts +++ b/src/domain/javascript/javascriptFeatureTraceSchemas.ts @@ -46,6 +46,29 @@ const applicationFeatureSeedSchema = z.union([ }), ]); +/** Check unique native identities before projecting an application trace. */ +const validateApplicationTraceIdentities = ( + input: { + readonly native_observations: readonly { readonly evidence_id: string }[]; + }, + context: z.RefinementCtx, +): void => { + const ids = input.native_observations.map(({ evidence_id: id }) => id); + if (new Set(ids).size !== ids.length) + context.addIssue({ + code: "custom", + path: ["native_observations"], + message: "Native observations must be unique", + }); +}; + +/** Validate native trace identities without capturing their complete Evidence. */ +export const applicationTraceIdentitiesSchema = z + .object({ + native_observations: z.array(z.object({ evidence_id: evidenceIdSchema })), + }) + .superRefine(validateApplicationTraceIdentities); + /** Evidence-backed application graph and feature seed. */ export const traceApplicationFeatureInputSchema = z .strictObject({ @@ -54,15 +77,7 @@ export const traceApplicationFeatureInputSchema = z seed: applicationFeatureSeedSchema, direction: z.enum(["outgoing", "incoming", "both"]).default("both"), }) - .superRefine((input, context) => { - const ids = input.native_observations.map(({ evidence_id: id }) => id); - if (new Set(ids).size !== ids.length) - context.addIssue({ - code: "custom", - path: ["native_observations"], - message: "Native observations must be unique", - }); - }); + .superRefine(validateApplicationTraceIdentities); const seedMatchSchema = z.strictObject({ node_id: nodeIdSchema, diff --git a/tests/boundary/cli/workflowInputIdentities.test.ts b/tests/boundary/cli/workflowInputIdentities.test.ts new file mode 100644 index 0000000000..a724e7fea2 --- /dev/null +++ b/tests/boundary/cli/workflowInputIdentities.test.ts @@ -0,0 +1,318 @@ +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { mkdir, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { expect, onTestFinished } from "vitest"; + +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; + +const compareContract = toolContract("compare_application_versions"); +const analyzeContract = toolContract("analyze_javascript_application"); +const expectInputFailure = ( + value: unknown, + path: string, + message: string, +): void => { + expect(value).toMatchObject({ + code: "invalid_request", + details: { + issues: [{ path: [path], message: expect.stringContaining(message) }], + }, + }); +}; +const runJson = async ( + cli: TestCli, + root: string, + command: string, + input: unknown, +) => { + const path = join(root, "request.json"); + await writeFile(path, JSON.stringify(input)); + return cli.run({ + arguments: [command, path, "--json"], + cwd: root, + environment: { REA_LOG_LEVEL: "silent" }, + }); +}; +const connectMcp = async (root: string) => { + const client = new Client({ + name: "application-input-identities", + version: "1", + }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [resolve("scripts/rea.mjs"), "mcp"], + cwd: root, + env: { + PATH: process.env.PATH ?? "", + HOME: root, + XDG_CONFIG_HOME: join(root, "config"), + XDG_CACHE_HOME: join(root, "cache"), + REA_LOG_LEVEL: "silent", + }, + }); + onTestFinished(async () => { + try { + await client.close(); + } finally { + await transport.close(); + } + }); + await client.connect(transport); + const tools = new Map( + (await client.listTools()).tools.map((tool) => [tool.name, tool]), + ); + const call = (name: string, arguments_: Record = {}) => { + const definition = tools.get(name); + if (definition === undefined) + throw new Error(`Missing advertised tool ${name}`); + return client.callTool( + { name, arguments: arguments_ }, + { toolDefinition: definition }, + ); + }; + return { client, tools, call }; +}; + +type Application = ReturnType; +type WorkflowContext = { + cli: TestCli; + root: string; + call: Awaited>["call"]; + application: Application; + other: Application; + reference: { kind: string; evidence_id: string }; + otherReference: { kind: string; evidence_id: string }; +}; +const selectors = { + left_module_path: "main.mjs", + left_export_name: "get", + right_module_path: "main.mjs", + right_export_name: "get", +}; +const seed = { kind: "module", value: "main.mjs", match: "contains" }; + +const checkComparisonIdentities = async ({ + cli, + root, + call, + application, + reference, +}: WorkflowContext) => { + for (const [name, command, extra] of [ + ["compare_application_versions", "compare-application-versions", {}], + [ + "compare_javascript_export_shapes", + "compare-javascript-export-shapes", + selectors, + ], + ] as const) { + for (const [a, b] of [ + [application, application], + [reference, reference], + [application, reference], + [reference, application], + ]) { + const response = await call(name, { left: a, right: b, ...extra }); + expect(response.isError).toBe(true); + expectInputFailure( + parseMcpToolError(response).error, + "right", + "Evidence must be distinct", + ); + } + const invalid = await runJson(cli, root, command, { + left: application, + right: application, + ...extra, + }); + expect(invalid.exitCode).toBe(1); + expectInputFailure(invalid.json, "right", "Evidence must be distinct"); + } +}; + +const checkNativeObservationIdentities = async ({ + cli, + root, + call, + application, + other, + reference, + otherReference, +}: WorkflowContext) => { + for (const side of [ + "left_native_observations", + "right_native_observations", + ] as const) { + const input = { + left: application, + right: other, + [side]: [application, application], + }; + const response = await call("compare_application_versions", { + ...input, + left: reference, + right: otherReference, + }); + expect(response.isError).toBe(true); + expectInputFailure( + parseMcpToolError(response).error, + side, + "Native observations must be unique", + ); + const invalid = await runJson( + cli, + root, + "compare-application-versions", + input, + ); + expect(invalid.exitCode).toBe(1); + expectInputFailure( + invalid.json, + side, + "Native observations must be unique", + ); + } + const traceInput = { + application, + seed, + native_observations: [application, application], + }; + const duplicateTrace = await call("trace_application_feature", { + ...traceInput, + application: reference, + }); + expect(duplicateTrace.isError).toBe(true); + expectInputFailure( + parseMcpToolError(duplicateTrace).error, + "native_observations", + "Native observations must be unique", + ); + const invalidTrace = await runJson( + cli, + root, + "trace-application-feature", + traceInput, + ); + expect(invalidTrace.exitCode).toBe(1); + expectInputFailure( + invalidTrace.json, + "native_observations", + "Native observations must be unique", + ); +}; + +cliTest( + "preserves workflow identity validation through CLI and inline, retained and mixed MCP inputs", + async ({ cli }) => { + const root = await createTestTempDirectory( + "rea-application-input-identities-", + ); + const left = join(root, "left"), + right = join(root, "right"); + await mkdir(left); + await mkdir(right); + await writeFile( + join(left, "main.mjs"), + "export function get() { return { observed: 1 }; }\n", + ); + await writeFile( + join(right, "main.mjs"), + "export function get() { return { observed: 2 }; }\n", + ); + const { client, tools, call } = await connectMcp(root); + const first = await call("analyze_javascript_application", { + input_path: left, + format: "directory", + }); + const second = await call("analyze_javascript_application", { + input_path: right, + format: "directory", + }); + expect(first.isError).not.toBe(true); + expect(second.isError).not.toBe(true); + const application = analyzeContract.outputSchema.parse( + first.structuredContent, + ); + const other = analyzeContract.outputSchema.parse(second.structuredContent); + const reference = { + kind: "retained-evidence", + evidence_id: application.evidence_id, + }; + const otherReference = { + kind: "retained-evidence", + evidence_id: other.evidence_id, + }; + const ajv = new Ajv2020({ strict: false, validateFormats: false }); + for (const name of [ + "compare_application_versions", + "compare_javascript_export_shapes", + "trace_application_feature", + ]) { + const definition = tools.get(name); + if (definition?.outputSchema === undefined) + throw new Error("Missing workflow schema"); + expect(ajv.validateSchema(definition.inputSchema)).toBe(true); + const { $schema, ...outputKeywords } = definition.outputSchema; + expect( + ajv.validateSchema( + $schema === undefined + ? outputKeywords + : { ...outputKeywords, $schema }, + ), + ).toBe(true); + } + const context = { + cli, + root, + call, + application, + other, + reference, + otherReference, + }; + await checkComparisonIdentities(context); + await checkNativeObservationIdentities(context); + const compared = await call("compare_application_versions", { + left: reference, + right: otherReference, + }); + expect(compared.isError).not.toBe(true); + const result = compareContract.outputSchema.parse( + compared.structuredContent, + ); + expect(result.normalized_result.evidence_links).toEqual( + [application.evidence_id, other.evidence_id].sort(), + ); + const cliResult = await runJson(cli, root, "compare-application-versions", { + left: application, + right: other, + }); + expect(cliResult.exitCode, cliResult.stdout).toBe(0); + expect(compareContract.outputSchema.parse(cliResult.json)).toEqual(result); + expect( + ( + await call("compare_javascript_export_shapes", { + left: reference, + right: otherReference, + ...selectors, + }) + ).isError, + ).not.toBe(true); + expect( + ( + await call("trace_application_feature", { + application: reference, + seed, + }) + ).isError, + ).not.toBe(true); + await client.ping(); + expect((await call("close_binary")).isError).not.toBe(true); + await client.ping(); + }, +);