diff --git a/mcpjam-inspector/server/services/evals/__tests__/convex-sanitize.test.ts b/mcpjam-inspector/server/services/evals/__tests__/convex-sanitize.test.ts index 4968e94be5..7fc58589b8 100644 --- a/mcpjam-inspector/server/services/evals/__tests__/convex-sanitize.test.ts +++ b/mcpjam-inspector/server/services/evals/__tests__/convex-sanitize.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { desanitizeFromConvexTransport, sanitizeForConvexTransport, + toPersistedToolCalls, } from "../convex-sanitize.js"; describe("sanitizeForConvexTransport", () => { @@ -47,3 +48,68 @@ describe("sanitizeForConvexTransport", () => { ).toEqual(original); }); }); + +describe("toPersistedToolCalls", () => { + it("returns an empty array for an iteration that called no tools", () => { + // The common case: `actualToolCalls: []` is what a no-tool iteration + // persists, and the validator requires an array, not a missing field. + expect(toPersistedToolCalls([])).toEqual([]); + }); + + it("keeps exactly the fields the backend validator accepts", () => { + expect( + toPersistedToolCalls([ + { + toolName: "connector_list", + arguments: { includeSourceConnectors: true }, + }, + { + toolName: "search", + arguments: { q: "coffee" }, + toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM", + }, + ]), + ).toEqual([ + { + toolName: "connector_list", + arguments: { includeSourceConnectors: true }, + }, + { + toolName: "search", + arguments: { q: "coffee" }, + toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM", + }, + ]); + }); + + it("drops a field the strict validator would reject (CONVEX-1QF)", () => { + // `updateTestIteration.actualToolCalls` is a strict `v.object`: an + // unrecognized key fails the whole finalize call, so the boundary has to + // project rather than trust whatever the runner attached upstream. + const calls = toPersistedToolCalls([ + { + toolName: "connector_list", + arguments: { includeSourceConnectors: true }, + toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM", + providerExecuted: true, + state: "output-available", + } as never, + ]); + + expect(Object.keys(calls[0]!).sort()).toEqual([ + "arguments", + "toolCallId", + "toolName", + ]); + }); + + it("omits toolCallId rather than sending undefined when absent", () => { + // `v.optional(v.string())` accepts a missing key; an explicit `undefined` + // is what Convex serialization rejects. + const [call] = toPersistedToolCalls([ + { toolName: "echo", arguments: {}, toolCallId: undefined }, + ]); + + expect("toolCallId" in call!).toBe(false); + }); +}); diff --git a/mcpjam-inspector/server/services/evals/__tests__/finalize-iteration.test.ts b/mcpjam-inspector/server/services/evals/__tests__/finalize-iteration.test.ts index 70f5acb80a..4045dc0b3c 100644 --- a/mcpjam-inspector/server/services/evals/__tests__/finalize-iteration.test.ts +++ b/mcpjam-inspector/server/services/evals/__tests__/finalize-iteration.test.ts @@ -186,6 +186,41 @@ describe("finalizeEvalIteration", () => { expect(update!.args.messages).toBeDefined(); }); + test("actualToolCalls reaches the wire projected to the persisted shape", async () => { + // CONVEX-1QF: `updateTestIteration.actualToolCalls` is a strict Convex + // `v.object`, so one unrecognized field fails the whole finalize rather + // than being dropped. The mapper's unit tests cover the projection; this + // pins that finalize actually applies it to what it sends. + const { client, calls } = makeClient({}); + await finalizeEvalIteration({ + convexClient: client, + iterationId: "iter1", + passed: true, + toolsCalled: [ + { + toolName: "connector_list", + arguments: { includeSourceConnectors: true }, + toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM", + // Not in the validator — the shape the runner could drift into. + providerExecuted: true, + } as never, + ], + usage: usageZero, + messages, + }); + const update = calls.find( + (c) => c.ref === "testSuites:updateTestIteration", + ); + expect(update).toBeDefined(); + expect(update!.args.actualToolCalls).toEqual([ + { + toolName: "connector_list", + arguments: { includeSourceConnectors: true }, + toolCallId: "toolu_01SCzFBPBXj3sQjxBSxaQcoM", + }, + ]); + }); + test("W1 fallback omits systemPrompt when unset", async () => { const { client, calls } = makeClient({ appendThrows: new Error("fanout pre-turn failure"), diff --git a/mcpjam-inspector/server/services/evals/convex-sanitize.ts b/mcpjam-inspector/server/services/evals/convex-sanitize.ts index 4154b14003..3bb9ac655d 100644 --- a/mcpjam-inspector/server/services/evals/convex-sanitize.ts +++ b/mcpjam-inspector/server/services/evals/convex-sanitize.ts @@ -2,3 +2,47 @@ export { sanitizeForConvexTransport, desanitizeFromConvexTransport, } from "@/shared/convex-sanitize"; + +/** + * The exact persisted shape of one `testIteration.actualToolCalls` entry — + * mirrors the backend's `evalIterationToolCallValidator` (mcpjam-backend + * `convex/lib/evalAnalysis.ts`), which is a strict `v.object`. + */ +export type PersistedToolCall = { + toolName: string; + arguments: Record; + toolCallId?: string; +}; + +/** + * Project tool calls onto exactly the fields that + * `updateTestIteration.actualToolCalls` accepts, dropping anything else. + * + * Convex object validators are STRICT: one unrecognized field is a hard + * `ArgumentValidationError`, not a silent drop — so an extra key here doesn't + * degrade a write, it fails the whole iteration finalize. That is what + * CONVEX-1QF was: the runner began attaching `toolCallId` (inspector #4308, to + * filter policy-blocked calls by id) and every eval iteration carrying a tool + * call stopped persisting until the validator was widened (backend #1134). + * + * Each producer currently builds these objects field-by-field, so today's + * payload is already clean. This makes that a property of the boundary rather + * than of every producer remembering: the runner reads tool calls out of + * `any`-typed AI SDK step objects, so the next field added upstream would + * otherwise reach the validator the same way. + */ +export function toPersistedToolCalls( + toolCalls: ReadonlyArray<{ + toolName: string; + arguments: Record; + toolCallId?: string; + }>, +): PersistedToolCall[] { + return toolCalls.map((call) => ({ + toolName: call.toolName, + arguments: call.arguments, + ...(typeof call.toolCallId === "string" + ? { toolCallId: call.toolCallId } + : {}), + })); +} diff --git a/mcpjam-inspector/server/services/evals/finalize-iteration.ts b/mcpjam-inspector/server/services/evals/finalize-iteration.ts index 764be8036b..d43689db17 100644 --- a/mcpjam-inspector/server/services/evals/finalize-iteration.ts +++ b/mcpjam-inspector/server/services/evals/finalize-iteration.ts @@ -10,7 +10,10 @@ import type { import { logger } from "../../utils/logger.js"; import { uploadVideoBlob } from "../../utils/mcp-app-widget-capture.js"; import type { UsageTotals } from "./types.js"; -import { sanitizeForConvexTransport } from "./convex-sanitize.js"; +import { + sanitizeForConvexTransport, + toPersistedToolCalls, +} from "./convex-sanitize.js"; import { emitBrowserEvalMetrics } from "./browser-eval-metrics.js"; import { serializeBrowserStepsForBackend, @@ -964,7 +967,9 @@ export async function finalizeEvalIteration( iterationId, status: iterationStatus === "completed" ? "completed" : iterationStatus, result, - actualToolCalls: sanitizeForConvexTransport(toolsCalled), + actualToolCalls: sanitizeForConvexTransport( + toPersistedToolCalls(toolsCalled), + ), tokensUsed: usage.totalTokens ?? 0, ...(useW1Fallback ? {