diff --git a/src/application/EvidenceBundleCommands.ts b/src/application/EvidenceBundleCommands.ts index 0da3db244f..3205f867ca 100644 --- a/src/application/EvidenceBundleCommands.ts +++ b/src/application/EvidenceBundleCommands.ts @@ -4,7 +4,6 @@ import { requireAuthenticationHeadroom } from "./JsonAuthenticationHeadroom.js"; import { EvidenceFileError, - EvidenceIntegrityError, type PublishedEvidenceFile, } from "../domain/evidenceErrors.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; @@ -256,15 +255,7 @@ export const compareEvidenceBundlesCommand = async (input: { ]); if (!left.ok) return left; if (!right.ok) return right; - try { - return ok(jsonValueSchema.parse(compareBundles(left.value, right.value))); - } catch (cause: unknown) { - return err( - new EvidenceIntegrityError("Evidence bundle comparison failed", { - cause, - }), - ); - } + return ok(jsonValueSchema.parse(compareBundles(left.value, right.value))); }; const projectWrite = ( diff --git a/src/domain/bundleComparison.test.ts b/src/domain/bundleComparison.test.ts index 866c09be23..f91aff856e 100644 --- a/src/domain/bundleComparison.test.ts +++ b/src/domain/bundleComparison.test.ts @@ -5,6 +5,7 @@ import { bundleComparisonResultSchema, compareBundles, } from "./bundleComparison.js"; +import { AnalysisInputError } from "./analysisErrorCore.js"; import { createEvidence } from "./evidence.js"; import { createEvidenceBundle } from "./evidenceBundle.js"; import { @@ -202,7 +203,7 @@ describe("bundle comparison", () => { right_evidence_id: rightRecord.evidence_id, }, ]), - ).toThrow(/missing left evidence/u); + ).toThrow(AnalysisInputError); expect(() => compareBundles(left, right, [ { @@ -214,7 +215,7 @@ describe("bundle comparison", () => { right_evidence_id: rightRecord.evidence_id, }, ]), - ).toThrow(/one-to-one/u); + ).toThrow(AnalysisInputError); expect(() => compareBundles({ ...left, records: [leftRecord, leftRecord] }, right), ).toThrow(/duplicate record IDs/u); diff --git a/src/domain/bundleComparison.ts b/src/domain/bundleComparison.ts index 6e918e22f5..f2b5dd9905 100644 --- a/src/domain/bundleComparison.ts +++ b/src/domain/bundleComparison.ts @@ -7,6 +7,7 @@ import { serializeEvidenceBundle, type EvidenceBundle, } from "./evidenceBundle.js"; +import { AnalysisInputError } from "./analysisErrorCore.js"; import type { ResidualUnknown } from "./residualUnknown.js"; import { digestSchema } from "./../domain/digests.js"; import { prefixedDigestSchema } from "./../domain/digests.js"; @@ -130,20 +131,31 @@ const compareRecords = ( const pairedRight = new Set(); const changes: BundleChange[] = []; let changed = 0; - for (const pair of pairs) { + for (const [index, pair] of pairs.entries()) { if (!leftIds.has(pair.left_evidence_id)) - throw new TypeError( + throw recordPairFailure( + index, + "left_evidence_id", `Explicit pair references missing left evidence ${pair.left_evidence_id}`, ); if (!rightIds.has(pair.right_evidence_id)) - throw new TypeError( + throw recordPairFailure( + index, + "right_evidence_id", `Explicit pair references missing right evidence ${pair.right_evidence_id}`, ); - if ( - pairedLeft.has(pair.left_evidence_id) || - pairedRight.has(pair.right_evidence_id) - ) - throw new TypeError("Explicit evidence pairs must be one-to-one"); + if (pairedLeft.has(pair.left_evidence_id)) + throw recordPairFailure( + index, + "left_evidence_id", + "Explicit evidence pairs must be one-to-one", + ); + if (pairedRight.has(pair.right_evidence_id)) + throw recordPairFailure( + index, + "right_evidence_id", + "Explicit evidence pairs must be one-to-one", + ); pairedLeft.add(pair.left_evidence_id); pairedRight.add(pair.right_evidence_id); if (pair.left_evidence_id === pair.right_evidence_id) continue; @@ -198,6 +210,15 @@ const compareRecords = ( }; }; +const recordPairFailure = ( + index: number, + field: keyof RecordPair, + message: string, +): AnalysisInputError => + new AnalysisInputError("compare_bundles", { userMessage: message }, [ + { path: ["record_pairs", index, field], reason: "invalid_value", message }, + ]); + const compareUnknowns = (left: EvidenceBundle, right: EvidenceBundle) => { const leftHistories = histories(left.unknowns); const rightHistories = histories(right.unknowns); diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index a5ae67f41d..cbe44655d5 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -367,6 +367,119 @@ describe("bundle comparison MCP integration", () => { }); }); +describe("bundle comparison record selections", () => { + it.each([ + { + label: "missing left evidence", + pairs: ( + _left: readonly ReturnType[], + right: readonly ReturnType[], + ) => [ + { + left_evidence_id: sourceEvidence("absent-left").evidence_id, + right_evidence_id: right[0]!.evidence_id, + }, + ], + path: ["record_pairs", 0, "left_evidence_id"], + message: "missing left evidence", + }, + { + label: "missing right evidence", + pairs: ( + left: readonly ReturnType[], + _right: readonly ReturnType[], + ) => [ + { + left_evidence_id: left[0]!.evidence_id, + right_evidence_id: sourceEvidence("absent-right").evidence_id, + }, + ], + path: ["record_pairs", 0, "right_evidence_id"], + message: "missing right evidence", + }, + { + label: "duplicate left evidence", + pairs: ( + left: readonly ReturnType[], + right: readonly ReturnType[], + ) => [ + { + left_evidence_id: left[0]!.evidence_id, + right_evidence_id: right[0]!.evidence_id, + }, + { + left_evidence_id: left[0]!.evidence_id, + right_evidence_id: right[1]!.evidence_id, + }, + ], + path: ["record_pairs", 1, "left_evidence_id"], + message: "one-to-one", + }, + { + label: "duplicate right evidence", + pairs: ( + left: readonly ReturnType[], + right: readonly ReturnType[], + ) => [ + { + left_evidence_id: left[0]!.evidence_id, + right_evidence_id: right[0]!.evidence_id, + }, + { + left_evidence_id: left[1]!.evidence_id, + right_evidence_id: right[0]!.evidence_id, + }, + ], + path: ["record_pairs", 1, "right_evidence_id"], + message: "one-to-one", + }, + ])( + "reports the precise path for $label without publishing Evidence", + async ({ pairs, path, message }) => { + const root = await createTestTempDirectory( + "rea-bundle-pairs-invalid-mcp-", + ); + roots.push(root); + const left = [sourceEvidence("left-1"), sourceEvidence("left-2")]; + const right = [sourceEvidence("right-1"), sourceEvidence("right-2")]; + const leftPath = join(root, "left.json"); + const rightPath = join(root, "right.json"); + await Promise.all([ + writeFile( + leftPath, + serializeEvidenceBundle(createEvidenceBundle(left)), + ), + writeFile( + rightPath, + serializeEvidenceBundle(createEvidenceBundle(right)), + ), + ]); + const connected = await connect(); + try { + const result = await connected.client.callTool({ + name: "compare_bundles", + arguments: { + left_bundle_path: leftPath, + right_bundle_path: rightPath, + record_pairs: pairs(left, right), + }, + }); + expect(parseMcpToolError(result).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + message: expect.stringContaining(message), + details: { + issues: [expect.objectContaining({ path })], + }, + }); + expect(connected.session.exportEvidenceBundle().records).toEqual([]); + } finally { + await connected.close(); + } + }, + ); +}); + const connect = async () => { const session = createTestBinarySession(() => ({ health: () => Promise.resolve(), @@ -381,6 +494,7 @@ const connect = async () => { await client.connect(clientTransport); return { client, + session, close: async () => { await Promise.allSettled([ client.close(), diff --git a/tests/process-global/bundleComparisonFailures.test.ts b/tests/process-global/bundleComparisonFailures.test.ts new file mode 100644 index 0000000000..31e4d6cf19 --- /dev/null +++ b/tests/process-global/bundleComparisonFailures.test.ts @@ -0,0 +1,64 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it, onTestFinished, vi } from "vitest"; +import { z } from "zod"; + +import { createCli } from "../../src/cli.js"; +import { compareEvidenceBundlesCommand } from "../../src/application/EvidenceBundleCommands.js"; +import { bundleComparisonResultSchema } from "../../src/domain/bundleComparison.js"; +import { + createEvidenceBundle, + serializeEvidenceBundle, +} from "../../src/domain/evidenceBundle.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +it.each([ + new TypeError("comparison output invariant failed"), + new RangeError("comparison computation failed"), + new z.ZodError([ + { code: "custom", path: ["status"], message: "invalid generated result" }, + ]), +])( + "keeps an internal comparison failure distinct from damaged Evidence: $name", + async (failure) => { + const directory = await createTestTempDirectory( + "rea-bundle-compute-failure-", + ); + const path = join(directory, "bundle.json"); + await writeFile(path, serializeEvidenceBundle(createEvidenceBundle([]))); + // Reach output validation through the actual command, reader, authentication + // and comparison. This fault is generated output, not the caller's bundle. + const output = vi + .spyOn(bundleComparisonResultSchema, "parse") + .mockImplementation(() => { + throw failure; + }); + onTestFinished(() => output.mockRestore()); + + const originalExitCode = process.exitCode; + onTestFinished(() => { + process.exitCode = originalExitCode; + }); + process.exitCode = 0; + let stdout = ""; + let exitCode = 0; + await createCli({}).serve(["compare", path, path, "--json"], { + env: {}, + stdout: (chunk) => { + stdout += chunk; + }, + exit: (code) => { + exitCode = code; + }, + }); + expect(exitCode || Number(process.exitCode ?? 0)).toBe(1); + expect(JSON.parse(stdout)).toEqual({ + code: "UNKNOWN", + message: failure.message, + }); + await expect( + compareEvidenceBundlesCommand({ leftPath: path, rightPath: path }), + ).rejects.toBe(failure); + }, +);