From eb69763d27e29fe3247c008925016e4c73cb6abe Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Thu, 8 Oct 2026 09:04:35 -0700 Subject: [PATCH] fix(process): name the reason two captures cannot be compared (#1080) * fix(process): name the reason two captures cannot be compared compare-process-captures rejected captures whose comparison contracts differ, but the reason never reached the caller. The CLI printed "Process command could not complete... run `rea doctor`". MCP wrapped the error in an AnalysisInputError with no issues ("Analysis input is invalid. Check the arguments"). A stale capture under max_capture_age_ms got the same treatment. Throw typed input issues that name the differing contract fields, or the stale side with its completion time and age, and let runDerivedOperation pass AnalysisInputError through unchanged. Document what the comparison contract covers. Co-Authored-By: Claude Opus 5.5 * fix(process): report CLI trace-spec issues under the MCP input path The CLI reported an invalid trace specification under operation "compare-process-captures" with root-relative issue paths such as ["events"]. MCP reports the same input as compare_process_captures with paths under trace_spec. Use the canonical operation name and prefix trace_spec so both surfaces describe the same input identically. Co-Authored-By: Claude Opus 5.5 * fix(process): name the side and constraint for unusable capture Evidence Adjacent compare_process_captures failures still lost their reason. Over MCP, Evidence from another workflow, tampered Evidence and an invalid capture result all became "Evidence is invalid or has changed". The CLI said only "Capture evidence is malformed". Parse each side separately. Evidence from another workflow is an input issue on left or right that names the operation and predicate it has. Tampered Evidence and invalid capture results stay integrity mismatches, and their message now carries the validation detail. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/process-capture.md | 9 +- src/application/process/ProcessCli.ts | 17 +- .../process/processCapture.validation.test.ts | 26 ++- src/domain/process/processComparison.ts | 61 ++++- src/server/registerProcessComparisonTool.ts | 129 ++++++----- src/server/runDerivedOperation.ts | 4 +- .../processComparisonDiagnosticsMcp.test.ts | 210 ++++++++++++++++++ 7 files changed, 381 insertions(+), 75 deletions(-) create mode 100644 tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts diff --git a/docs/process-capture.md b/docs/process-capture.md index 78d84bf910..20195bdc4a 100644 --- a/docs/process-capture.md +++ b/docs/process-capture.md @@ -130,7 +130,12 @@ causal evidence. Trace comparison returns `unknown` when a capture lacks the required complete event journal or contains relevant residual unknowns. Each capture carries commitments for the selected scenario, executable, -comparison contract, and normalization rules. Comparison rejects captures -whose comparison contracts differ. Captures are local Evidence files; keep +comparison contract, and normalization rules. The comparison contract covers +the working directory, explicit environment, filesystem observation paths, +terminal size, scripted events, timeouts, limits, and normalization; the +executable and its arguments may differ. Comparison rejects captures whose +comparison contracts differ and names the differing fields, so run both +scenarios from the same absolute working directory and observation paths. +Captures are local Evidence files; keep their source artifacts and invocation context available when interpreting a difference. diff --git a/src/application/process/ProcessCli.ts b/src/application/process/ProcessCli.ts index 3d0e537491..7c60613ef2 100644 --- a/src/application/process/ProcessCli.ts +++ b/src/application/process/ProcessCli.ts @@ -5,6 +5,7 @@ import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { createEvidence, parseEvidence } from "../../domain/evidence.js"; +import { describeValidationFailure } from "../../domain/evidenceBundle.js"; import { jsonValueSchema } from "../../domain/jsonValue.js"; import { projectInputIssues } from "../../domain/inputIssueProjection.js"; import { processTraceSpecificationSchema } from "../../domain/process/processTraceComparison.js"; @@ -120,7 +121,7 @@ const parseCaptureEvidence = (input: unknown) => { { cause }, projectInputIssues(cause.issues, input), ); - throw invalidCaptureEvidence(); + throw invalidCaptureEvidence(cause); } if ( evidence.operation !== "capture_process_scenario" || @@ -140,24 +141,28 @@ const parseCaptureEvidence = (input: unknown) => { locations: evidence.locations, }; } catch (cause: unknown) { - throw invalidCaptureEvidence(); + throw invalidCaptureEvidence(cause); } }; const parseTraceSpecification = (input: unknown) => { const parsed = processTraceSpecificationSchema.safeParse(input); if (parsed.success) return parsed.data; + // Match the MCP boundary: the operation name and a trace_spec-rooted path. throw new AnalysisInputError( - "compare-process-captures", + "compare_process_captures", { cause: parsed.error }, - projectInputIssues(parsed.error.issues, input), + projectInputIssues(parsed.error.issues, input).map((issue) => ({ + ...issue, + path: ["trace_spec", ...issue.path], + })), ); }; -const invalidCaptureEvidence = (): ProcessCliFailure => +const invalidCaptureEvidence = (cause: unknown): ProcessCliFailure => new ProcessCliFailure( "invalid_input", - "Capture evidence is malformed. Create new capture evidence, then try again.", + `Capture evidence is malformed (${describeValidationFailure(cause)}). Create new capture evidence, then try again.`, ); const readJson = async (path: string): Promise => { diff --git a/src/domain/process/processCapture.validation.test.ts b/src/domain/process/processCapture.validation.test.ts index 912b7c16d5..558d4743c7 100644 --- a/src/domain/process/processCapture.validation.test.ts +++ b/src/domain/process/processCapture.validation.test.ts @@ -241,11 +241,33 @@ it("requires compatible contracts and enforces capture age through a clock seam" }), }, }), - ).toThrow("incompatible comparison contracts"); + ).toThrow( + expect.objectContaining({ + issues: [ + expect.objectContaining({ + path: ["right"], + message: expect.stringContaining( + "incompatible comparison contracts; these scenario fields differ: changed", + ), + expected: ["changed"], + }), + ], + }), + ); expect(() => compareProcessCaptures(capture, capture, { maxCaptureAgeMs: 1, now: () => Date.parse("2026-01-01T00:00:01.000Z"), }), - ).toThrow("max_capture_age_ms"); + ).toThrow( + expect.objectContaining({ + issues: [ + expect.objectContaining({ + path: ["max_capture_age_ms"], + reason: "out_of_range", + message: expect.stringContaining("left completed at"), + }), + ], + }), + ); }); diff --git a/src/domain/process/processComparison.ts b/src/domain/process/processComparison.ts index b6e5a8cc0a..1aa052bc44 100644 --- a/src/domain/process/processComparison.ts +++ b/src/domain/process/processComparison.ts @@ -2,6 +2,7 @@ import canonicalize from "canonicalize"; import { z } from "zod"; import type { ProcessCapture } from "./processCapture.js"; +import { AnalysisInputError } from "../analysisErrorCore.js"; import { jsonValueSchema } from "../jsonValue.js"; import { compareProcessTraces, @@ -14,6 +15,8 @@ import { traceCoversObservedDimension, } from "./processTraceDimensionProjection.js"; +const OPERATION = "compare_process_captures"; + /** Comparison classification that never equates incomplete evidence. */ export const comparisonStatusSchema = z.enum([ "unchanged", @@ -228,22 +231,60 @@ const assertComparable = ( if ( left.manifest.comparison_contract_sha256 !== right.manifest.comparison_contract_sha256 - ) - throw new TypeError( - "Process captures have incompatible comparison contracts", + ) { + const fields = differingContractFields( + left.manifest.comparison_contract, + right.manifest.comparison_contract, ); + throw new AnalysisInputError(OPERATION, undefined, [ + { + path: ["right"], + reason: "invalid_value", + message: `Process captures have incompatible comparison contracts${fields.length === 0 ? "" : `; these scenario fields differ: ${fields.join(", ")}`}. Capture both scenarios with the same values for these fields, then compare them.`, + expected: fields, + }, + ]); + } if (options.maxCaptureAgeMs === undefined) return; const maxCaptureAgeMs = options.maxCaptureAgeMs; const now = (options.now ?? Date.now)(); - if ( - [left, right].some( - ({ manifest }) => - now - Date.parse(manifest.completed_at) > maxCaptureAgeMs, - ) - ) - throw new TypeError("Process capture exceeds max_capture_age_ms"); + const stale = ( + [ + ["left", left], + ["right", right], + ] as const + ).flatMap(([side, { manifest }]) => { + const age = now - Date.parse(manifest.completed_at); + return age > maxCaptureAgeMs + ? [ + `${side} completed at ${manifest.completed_at} (${String(age)} ms ago)`, + ] + : []; + }); + if (stale.length > 0) + throw new AnalysisInputError(OPERATION, undefined, [ + { + path: ["max_capture_age_ms"], + reason: "out_of_range", + message: `Process capture exceeds max_capture_age_ms ${String(maxCaptureAgeMs)}: ${stale.join("; ")}. Recapture the stale scenario or raise max_capture_age_ms.`, + }, + ]); }; +/** Name top-level contract fields whose canonical values differ. */ +const differingContractFields = ( + left: Readonly>, + right: Readonly>, +): string[] => + [...new Set([...Object.keys(left), ...Object.keys(right)])] + .filter( + (field) => + !Object.hasOwn(left, field) || + !Object.hasOwn(right, field) || + canonicalize(left[field]) !== canonicalize(right[field]), + ) + .sort(); + const truncatedComparison = (): ProcessCaptureComparison => ({ status: "truncated", terminal: "truncated", diff --git a/src/server/registerProcessComparisonTool.ts b/src/server/registerProcessComparisonTool.ts index 3b47d9c423..bff0492cb0 100644 --- a/src/server/registerProcessComparisonTool.ts +++ b/src/server/registerProcessComparisonTool.ts @@ -2,6 +2,9 @@ import type { McpServer } from "@modelcontextprotocol/server"; import type { BinarySessionPort } from "../application/binary/BinarySession.js"; import { toolContract } from "../contracts/toolContracts.js"; +import { AnalysisInputError } from "../domain/analysisErrorCore.js"; +import { type AnalysisError } from "../domain/analysisErrorBase.js"; +import { describeValidationFailure } from "../domain/evidenceBundle.js"; import { EvidenceIntegrityError } from "../domain/evidenceErrors.js"; import { createEvidence, @@ -13,9 +16,10 @@ import { jsonValueSchema } from "../domain/jsonValue.js"; import { compareProcessCaptures, parseProcessCapture, + type ProcessCapture, } from "../domain/process/processCapture.js"; import type { RecordUnknownInput } from "../domain/residualUnknown.js"; -import { err } from "../domain/result.js"; +import { err, ok, type Result } from "../domain/result.js"; import { recordDerivedEvidence } from "./recordDerivedEvidence.js"; import { recordSessionEvidenceSources } from "./sessionEvidence.js"; import { runDerivedOperation } from "./runDerivedOperation.js"; @@ -33,6 +37,57 @@ const sourceLocations = ( right: readonly EvidenceLocation[] | undefined, ): readonly EvidenceLocation[] => [...(left ?? []), ...(right ?? [])]; +/** + * Parse one side and name the failed constraint: tampered Evidence, Evidence + * from another workflow, or an invalid capture result. + */ +const parseCaptureSide = ( + operation: string, + side: "left" | "right", + value: unknown, +): Result< + { readonly record: Evidence; readonly capture: ProcessCapture }, + AnalysisError +> => { + let record: Evidence; + try { + record = parseEvidence(value); + } catch (cause: unknown) { + return err( + new EvidenceIntegrityError(`${side} Evidence failed validation`, { + cause, + userMessage: `The ${side} Evidence failed validation (${describeValidationFailure(cause)}). Supply unmodified capture Evidence from capture_process_scenario.`, + }), + ); + } + if ( + record.operation !== PROCESS_CAPTURE_EVIDENCE.operation || + record.predicate_type !== PROCESS_CAPTURE_EVIDENCE.predicate + ) + return err( + new AnalysisInputError(operation, undefined, [ + { + path: [side], + reason: "invalid_value", + message: `Expected process capture Evidence (operation ${PROCESS_CAPTURE_EVIDENCE.operation}, predicate ${PROCESS_CAPTURE_EVIDENCE.predicate}); the ${side} Evidence has operation ${record.operation} and predicate ${record.predicate_type}.`, + }, + ]), + ); + try { + return ok({ + record, + capture: parseProcessCapture(record.normalized_result), + }); + } catch (cause: unknown) { + return err( + new EvidenceIntegrityError(`${side} process capture failed validation`, { + cause, + userMessage: `The ${side} Evidence has an invalid process capture result (${describeValidationFailure(cause)}). Recreate the capture with capture_process_scenario.`, + }), + ); + } +}; + /** Register deterministic process-capture comparison and contradiction tracking. */ export const registerProcessComparisonTool = ( server: McpServer, @@ -44,59 +99,25 @@ export const registerProcessComparisonTool = ( contract.name, toolRegistrationOptions(contract), async (input, context) => { - let leftRecord: Evidence; - let rightRecord: Evidence; - try { - leftRecord = parseEvidence(input.left); - rightRecord = parseEvidence(input.right); - if ( - leftRecord.operation !== PROCESS_CAPTURE_EVIDENCE.operation || - rightRecord.operation !== PROCESS_CAPTURE_EVIDENCE.operation || - leftRecord.predicate_type !== PROCESS_CAPTURE_EVIDENCE.predicate || - rightRecord.predicate_type !== PROCESS_CAPTURE_EVIDENCE.predicate - ) - throw new TypeError("Expected process capture Evidence"); - } catch (cause: unknown) { - return toCallToolResult( - err( - new EvidenceIntegrityError( - cause instanceof Error ? cause.message : "Invalid Evidence", - ), - ), - contract, - ); - } - let comparison: ReturnType; - let leftCapture: ReturnType; - let rightCapture: ReturnType; - try { - leftCapture = parseProcessCapture(leftRecord.normalized_result); - rightCapture = parseProcessCapture(rightRecord.normalized_result); - const computed = await runDerivedOperation(context, contract.name, () => - compareProcessCaptures(leftCapture, rightCapture, { - ...(input.max_capture_age_ms === undefined - ? {} - : { maxCaptureAgeMs: input.max_capture_age_ms }), - ...(input.trace_spec === undefined - ? {} - : { traceSpecification: input.trace_spec }), - now, - }), - ); - if (!computed.ok) return toCallToolResult(computed, contract); - comparison = computed.value; - } catch (cause: unknown) { - return toCallToolResult( - err( - new EvidenceIntegrityError( - cause instanceof Error - ? cause.message - : "Invalid process capture", - ), - ), - contract, - ); - } + const left = parseCaptureSide(contract.name, "left", input.left); + if (!left.ok) return toCallToolResult(left, contract); + const right = parseCaptureSide(contract.name, "right", input.right); + if (!right.ok) return toCallToolResult(right, contract); + const { record: leftRecord, capture: leftCapture } = left.value; + const { record: rightRecord, capture: rightCapture } = right.value; + const computed = await runDerivedOperation(context, contract.name, () => + compareProcessCaptures(leftCapture, rightCapture, { + ...(input.max_capture_age_ms === undefined + ? {} + : { maxCaptureAgeMs: input.max_capture_age_ms }), + ...(input.trace_spec === undefined + ? {} + : { traceSpecification: input.trace_spec }), + now, + }), + ); + if (!computed.ok) return toCallToolResult(computed, contract); + const comparison = computed.value; const evidence = createEvidence(undefined, PROCESS_PROVIDER, { predicateType: "rea.process-comparison", operation: contract.name, diff --git a/src/server/runDerivedOperation.ts b/src/server/runDerivedOperation.ts index 1fbdf39ba2..0a7a55da67 100644 --- a/src/server/runDerivedOperation.ts +++ b/src/server/runDerivedOperation.ts @@ -33,8 +33,10 @@ export const runDerivedOperation = async ( try { value = compute(); } catch (cause: unknown) { + // Typed causes already name the failed constraint; keep their issues. return err( - cause instanceof AnalysisCancelledError + cause instanceof AnalysisCancelledError || + cause instanceof AnalysisInputError ? cause : new AnalysisInputError(operation, { cause }), ); diff --git a/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts b/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts new file mode 100644 index 0000000000..81722a3847 --- /dev/null +++ b/tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts @@ -0,0 +1,210 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { expect, it, onTestFinished } from "vitest"; + +import { compareProcessEvidenceFiles } from "../../../src/application/process/ProcessCli.js"; +import { PROCESS_PROVIDER } from "../../../src/application/process/ProcessEvidence.js"; +import { EMPTY_PROCESS_CAPTURE_EXAMPLE } from "../../../src/contracts/process/processCaptureExample.js"; +import { createEvidence } from "../../../src/domain/evidence.js"; +import { jsonValueSchema } from "../../../src/domain/jsonValue.js"; +import { digestProcessCommitment } from "../../../src/domain/process/processCapture.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const captureEvidence = ( + side: "left" | "right", + comparisonContract: Readonly> = {}, +) => + createEvidence(undefined, PROCESS_PROVIDER, { + predicateType: "rea.process-capture", + operation: "capture_process_scenario", + parameters: { side }, + result: jsonValueSchema.parse({ + ...EMPTY_PROCESS_CAPTURE_EXAMPLE, + manifest: { + ...EMPTY_PROCESS_CAPTURE_EXAMPLE.manifest, + comparison_contract: comparisonContract, + comparison_contract_sha256: digestProcessCommitment(comparisonContract), + }, + }), + confidence: "observed", + authority: "controlled-replay", + }); + +const left = captureEvidence("left", { working_directory: "/work/reference" }); +const right = captureEvidence("right", { + working_directory: "/work/reconstruction", +}); + +const connect = async () => { + const session = createTestBinarySession(() => { + throw new Error("Process comparison must not launch a provider"); + }); + const server = createServer(session, session); + const client = new Client({ name: "process-comparison-test", version: "1" }); + onTestFinished(async () => { + await Promise.allSettled([client.close(), server.close(), session.close()]); + }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + return client; +}; + +it("names differing comparison contract fields through MCP and the CLI", async () => { + const client = await connect(); + const response = await client.callTool({ + name: "compare_process_captures", + arguments: { left, right }, + }); + expect(response.isError).toBe(true); + const issue = { + path: ["right"], + reason: "invalid_value", + message: expect.stringContaining( + "these scenario fields differ: working_directory", + ), + expected: ["working_directory"], + }; + expect(response.structuredContent).toMatchObject({ + error: { code: "invalid_request", details: { issues: [issue] } }, + }); + + const root = await createTestTempDirectory("rea-process-contract-"); + const leftPath = join(root, "left.json"); + const rightPath = join(root, "right.json"); + await writeFile(leftPath, JSON.stringify(left)); + await writeFile(rightPath, JSON.stringify(right)); + expect(await compareProcessEvidenceFiles(leftPath, rightPath)).toMatchObject({ + error: "Process command failed", + code: "invalid_request", + details: { operation: "compare_process_captures", issues: [issue] }, + }); +}); + +it("names the stale capture that exceeds max_capture_age_ms", async () => { + const client = await connect(); + const response = await client.callTool({ + name: "compare_process_captures", + arguments: { + left: captureEvidence("left"), + right: captureEvidence("right"), + max_capture_age_ms: 1000, + }, + }); + expect(response.isError).toBe(true); + expect(response.structuredContent).toMatchObject({ + error: { + code: "invalid_request", + details: { + issues: [ + { + path: ["max_capture_age_ms"], + reason: "out_of_range", + message: expect.stringContaining( + `left completed at ${EMPTY_PROCESS_CAPTURE_EXAMPLE.manifest.completed_at}`, + ), + }, + ], + }, + }, + }); +}); + +it("reports CLI trace specification issues like the MCP trace_spec input", async () => { + const root = await createTestTempDirectory("rea-process-trace-spec-"); + const capturePath = join(root, "capture.json"); + const specPath = join(root, "spec.json"); + await writeFile(capturePath, JSON.stringify(captureEvidence("left"))); + await writeFile(specPath, JSON.stringify({ language: {} })); + expect( + await compareProcessEvidenceFiles(capturePath, capturePath, specPath), + ).toMatchObject({ + code: "invalid_request", + details: { + operation: "compare_process_captures", + issues: expect.arrayContaining([ + expect.objectContaining({ path: ["trace_spec", "events"] }), + ]), + }, + }); +}); + +it("names the side and constraint when Evidence is not a usable capture", async () => { + const client = await connect(); + const capture = captureEvidence("right"); + const unrelated = createEvidence(undefined, PROCESS_PROVIDER, { + operation: "other", + parameters: {}, + result: {}, + }); + const wrongKind = await client.callTool({ + name: "compare_process_captures", + arguments: { left: unrelated, right: capture }, + }); + expect(wrongKind.structuredContent).toMatchObject({ + error: { + code: "invalid_request", + details: { + issues: [ + { + path: ["left"], + reason: "invalid_value", + message: expect.stringContaining( + "the left Evidence has operation other", + ), + }, + ], + }, + }, + }); + + const tampered = await client.callTool({ + name: "compare_process_captures", + arguments: { + left: capture, + right: { ...capture, parameters: { side: "changed" } }, + }, + }); + expect(tampered.structuredContent).toMatchObject({ + error: { + code: "evidence_integrity_mismatch", + message: expect.stringContaining( + "The right Evidence failed validation (Evidence semantic identifier does not match its record)", + ), + }, + }); + + const invalidResult = createEvidence(undefined, PROCESS_PROVIDER, { + predicateType: "rea.process-capture", + operation: "capture_process_scenario", + parameters: {}, + result: { manifest: {} }, + }); + const malformed = await client.callTool({ + name: "compare_process_captures", + arguments: { left: capture, right: invalidResult }, + }); + expect(malformed.structuredContent).toMatchObject({ + error: { + code: "evidence_integrity_mismatch", + message: expect.stringContaining( + "The right Evidence has an invalid process capture result (at ", + ), + }, + }); + + const root = await createTestTempDirectory("rea-process-malformed-"); + const leftPath = join(root, "left.json"); + const rightPath = join(root, "right.json"); + await writeFile(leftPath, JSON.stringify(capture)); + await writeFile(rightPath, JSON.stringify(invalidResult)); + expect(await compareProcessEvidenceFiles(leftPath, rightPath)).toMatchObject({ + category: "invalid_input", + message: expect.stringContaining("Capture evidence is malformed (at "), + }); +});