diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 9d7edb6f3c..8df00d2e3a 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -314,7 +314,9 @@ that complete Evidence in the session and returns the derived `normalized_result.parent_evidence_id` and `evidence_links` name the retained analysis, so later views and application workflows read it without repeating analysis or moving the complete record through the response. A server without -session retention refuses summary detail with `capability_unavailable`. +session retention refuses summary detail with `capability_unavailable`; +`inspect_artifact`, `inspect_wasm_artifact`, `inspect_macho` and +`decode_interface_builder` refuse before any analysis starts. For `inspect_artifact`, omission also keeps the original request parameters and complete output shape. Delivery selection never reaches the inventory engine; the complete inspection and its atomic inventory Evidence remain retained. diff --git a/src/application/wasm/WasmArtifactService.test.ts b/src/application/wasm/WasmArtifactService.test.ts index c4c9531e5a..a66a65def7 100644 --- a/src/application/wasm/WasmArtifactService.test.ts +++ b/src/application/wasm/WasmArtifactService.test.ts @@ -191,3 +191,25 @@ it("preserves malformed-output failure when cancellation arrives with it", async error: { _tag: "AnalysisOutputError" }, }); }); + +it("gives cancellation precedence over malformed raw input", async () => { + const controller = new AbortController(); + controller.abort(); + let calls = 0; + const service = new WasmArtifactService({ + close: () => Promise.resolve(), + inspect: () => { + calls++; + return Promise.resolve(ok(wasmArtifactExecution())); + }, + }); + const malformed = { path: "relative.wasm" }; + expect( + await service.inspect(malformed, { signal: controller.signal }), + ).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" } }); + expect(await service.inspect(malformed)).toMatchObject({ + ok: false, + error: { _tag: "AnalysisInputError" }, + }); + expect(calls).toBe(0); +}); diff --git a/src/server/registerWasmTools.ts b/src/server/registerWasmTools.ts index c00e140d1b..de4107e9c1 100644 --- a/src/server/registerWasmTools.ts +++ b/src/server/registerWasmTools.ts @@ -7,7 +7,10 @@ import type { EvidenceMcpServer } from "./EvidenceMcpServer.js"; import type { WasmArtifactService } from "../application/wasm/WasmArtifactService.js"; import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js"; import type { EvidenceLookup } from "../application/EvidenceInputResolver.js"; -import { summarizeRetainedAnalysis } from "../application/analysisView/AnalysisViewService.js"; +import { + requireAnalysisViewRetention, + summarizeRetainedAnalysis, +} from "../application/analysisView/AnalysisViewService.js"; import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js"; import { err } from "../domain/result.js"; import { wasmArtifactModule } from "../composition/wasm.js"; @@ -75,12 +78,14 @@ export const registerWasmTools = ( ), ); const { detail, ...input } = parsed.data; + const retention = { recordEvidence, evidenceById }; + if (detail === "summary") { + const ready = requireAnalysisViewRetention(contract.name, retention); + if (!ready.ok) return ready; + } const inspected = await service.inspect(input, options); return detail === "summary" && inspected.ok - ? summarizeRetainedAnalysis(inspected.value, { - recordEvidence, - evidenceById, - }) + ? summarizeRetainedAnalysis(inspected.value, retention) : inspected; }, logger, diff --git a/tests/boundary/mcp/wasmArtifactMcp.test.ts b/tests/boundary/mcp/wasmArtifactMcp.test.ts index aefbb1ddfa..ba378bd5d9 100644 --- a/tests/boundary/mcp/wasmArtifactMcp.test.ts +++ b/tests/boundary/mcp/wasmArtifactMcp.test.ts @@ -14,7 +14,8 @@ import { wasmArtifactSummaryViewResultSchema } from "../../../src/domain/analysi import { createHash } from "node:crypto"; import { z } from "zod"; import { createServer } from "../../../src/server/createServer.js"; -import { ok } from "../../../src/domain/result.js"; +import { err, ok } from "../../../src/domain/result.js"; +import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js"; import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -627,11 +628,14 @@ it("rejects absent or inconsistent WASM source identity before deriving portable await harness.client.ping(); }); -it("requires retention for WASM summary delivery while keeping fixed-server complete delivery", async () => { +it("rejects malformed paths, refuses WASM summary delivery before provider work when retention is unavailable, and keeps fixed-server complete delivery", async () => { const value = wasmArtifactFixture(); + const inspect = vi.fn(() => + Promise.resolve(ok(wasmArtifactExecution(value))), + ); const service = new WasmArtifactService({ close: () => Promise.resolve(), - inspect: () => Promise.resolve(ok(wasmArtifactExecution(value))), + inspect, }); const session = createTestBinarySession(() => { throw new Error("No deep provider"); @@ -645,9 +649,10 @@ it("requires retention for WASM summary delivery while keeping fixed-server comp try { await server.connect(b); await client.connect(a); + const summaryInput = { path: value.artifact.path, detail: "summary" }; const summary = await client.callTool({ name: "inspect_wasm_artifact", - arguments: { path: value.artifact.path, detail: "summary" }, + arguments: summaryInput, }); expect(parseMcpToolError(summary)).toMatchObject({ error: { @@ -655,14 +660,56 @@ it("requires retention for WASM summary delivery while keeping fixed-server comp message: expect.stringContaining("does not retain Evidence"), }, }); - const complete = await client.callTool({ + expect(inspect).not.toHaveBeenCalled(); + const malformed = await client.callTool({ name: "inspect_wasm_artifact", - arguments: { path: value.artifact.path }, + arguments: { + path: "relative.wasm", + glue_paths: ["relative.js"], + detail: "summary", + }, }); - expect(complete.isError).not.toBe(true); - expect(parseEvidence(complete.structuredContent).normalized_result).toEqual( - value, + const rejected = parseMcpToolError(malformed).error; + expect(rejected.code).toBe("invalid_request"); + expect( + z.object({ issues: z.array(z.unknown()) }).parse(rejected.details).issues, + ).toEqual( + [["path"], ["glue_paths", 0]].map((path) => ({ + path, + reason: "invalid_format", + expected: "regex", + message: "must be an absolute local filesystem path", + })), ); + expect(inspect).not.toHaveBeenCalled(); + for (const input of [ + { path: value.artifact.path }, + { path: value.artifact.path, detail: "complete" }, + ]) { + const complete = await client.callTool({ + name: "inspect_wasm_artifact", + arguments: input, + }); + expect(complete.isError).not.toBe(true); + expect( + parseEvidence(complete.structuredContent).normalized_result, + ).toEqual(value); + } + expect(inspect).toHaveBeenCalledTimes(2); + inspect.mockImplementation(() => + Promise.resolve(err(new AnalysisInputError("inspect_wasm_artifact"))), + ); + const refused = await client.callTool({ + name: "inspect_wasm_artifact", + arguments: summaryInput, + }); + expect(parseMcpToolError(refused)).toMatchObject({ + error: { + code: "capability_unavailable", + message: expect.stringContaining("Select detail complete"), + }, + }); + expect(inspect).toHaveBeenCalledTimes(2); await client.ping(); } finally { await client.close();