From 15b2f6f4de776ed3c2bf8cd1337b7307ab578923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 7 Oct 2026 22:48:49 +0200 Subject: [PATCH] Declare every MCP output field and read live rows in discovery list_metrics declares availability, the Coach-backed reads declare searchedWindow and available, and get_intraday_pulse declares min and max per bucket. An integration test validates every tool's real output against its advertised schema under a strict 2020-12 validator. The metric-status discovery rows and the search clinical-signal probe filter deletedAt, guarded for every measurement read in the MCP and Coach tool trees. --- .../measurement-live-rows-guard.test.ts | 112 ++++ src/lib/mcp/rich-reads.ts | 5 + src/lib/mcp/tools.ts | 47 ++ .../mcp-output-schema-conformance.test.ts | 504 ++++++++++++++++++ 4 files changed, 668 insertions(+) create mode 100644 src/lib/mcp/__tests__/measurement-live-rows-guard.test.ts create mode 100644 tests/integration/mcp-output-schema-conformance.test.ts diff --git a/src/lib/mcp/__tests__/measurement-live-rows-guard.test.ts b/src/lib/mcp/__tests__/measurement-live-rows-guard.test.ts new file mode 100644 index 000000000..8b18efe71 --- /dev/null +++ b/src/lib/mcp/__tests__/measurement-live-rows-guard.test.ts @@ -0,0 +1,112 @@ +/** + * Guard: every measurement read on the MCP surface and in the Coach tools + * reads live rows only. + * + * A deleted measurement is a tombstone (`deletedAt` set), not a missing row. + * Two presence probes on the MCP side (`metricStatusDiscoveryRows` and the + * `search` clinical-signal probe) grouped by type without `deletedAt: null`: + * a type whose readings had all been deleted still counted as present, and + * because the live-rows partial index (`measurements_live_covering_idx`, + * `WHERE deleted_at IS NULL`) only serves a query that carries the same + * predicate, the probe walked every tombstone of the account (about a second + * against 23 ms on a large production record). + * + * The guard reads the source of both trees, finds every + * `prisma.measurement.(` call, takes its argument up to the matching + * parenthesis, and asserts the argument names `deletedAt`. Unique-key + * lookups (`findUnique`) are idempotency checks on a key a tombstone still + * holds, and are left out on purpose. Raw SQL against `measurements` must + * carry `deleted_at` the same way. + */ +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; +import { describe, expect, it } from "vitest"; + +const ROOTS = ["src/lib/mcp", "src/lib/ai/coach/tools"]; +const READS = + /\bprisma\.measurement\.(findMany|findFirst|count|groupBy|aggregate)\s*\(/g; +const RAW = /\b(?:FROM|JOIN)\s+"?measurements"?\b/gi; + +function sourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) { + return name === "__tests__" ? [] : sourceFiles(path); + } + return /\.tsx?$/.test(name) && !/\.test\.tsx?$/.test(name) ? [path] : []; + }); +} + +/** The text from `open` (an opening parenthesis) to its matching close. */ +function balanced(text: string, open: number): string { + let depth = 0; + for (let i = open; i < text.length; i += 1) { + if (text[i] === "(") depth += 1; + else if (text[i] === ")") { + depth -= 1; + if (depth === 0) return text.slice(open, i + 1); + } + } + return text.slice(open); +} + +interface Site { + at: string; + argument: string; +} + +function readSites(): { calls: Site[]; raw: Site[] } { + const calls: Site[] = []; + const raw: Site[] = []; + for (const root of ROOTS) { + for (const file of sourceFiles(join(process.cwd(), root))) { + const text = readFileSync(file, "utf8"); + const name = relative(process.cwd(), file); + const lineOf = (index: number) => text.slice(0, index).split("\n").length; + for (const match of text.matchAll(READS)) { + const open = (match.index ?? 0) + match[0].length - 1; + calls.push({ + at: `${name}:${lineOf(match.index ?? 0)}`, + argument: balanced(text, open), + }); + } + for (const match of text.matchAll(RAW)) { + // The statement around it: up to the closing backtick of the + // tagged template the query lives in. + const start = match.index ?? 0; + const end = text.indexOf("`", start); + raw.push({ + at: `${name}:${lineOf(start)}`, + argument: text.slice(start, end === -1 ? undefined : end), + }); + } + } + } + return { calls, raw }; +} + +describe("measurement reads on the MCP surface and in the Coach tools", () => { + const { calls, raw } = readSites(); + + it("finds the reads it is meant to check", () => { + // A matcher that matches nothing proves nothing: the two probes this + // guard was written for, the rich-read counts and the availability + // probe are all in these trees. + expect(calls.length).toBeGreaterThanOrEqual(5); + expect(raw.length).toBeGreaterThanOrEqual(1); + }); + + it("filters every Prisma read to live rows", () => { + const missing = calls + .filter((site) => !/\bdeletedAt\b/.test(site.argument)) + .map((site) => site.at); + expect(missing).toEqual([]); + }); + + it("filters every raw read to live rows", () => { + const missing = raw + .filter((site) => !/\bdeleted_at\b/.test(site.argument)) + .map((site) => site.at); + expect(missing).toEqual([]); + }); +}); diff --git a/src/lib/mcp/rich-reads.ts b/src/lib/mcp/rich-reads.ts index 2dcae91f3..a0f9efcaa 100644 --- a/src/lib/mcp/rich-reads.ts +++ b/src/lib/mcp/rich-reads.ts @@ -481,11 +481,16 @@ export async function metricStatusDiscoveryRows(userId: string): Promise< metric: string; }> > { + // v1.41.2 — live rows only. Without `deletedAt: null` a type whose rows + // were all deleted still counted as present, and the grouped count could + // not use the live-rows partial index (`measurements_live_covering_idx`), + // so it walked every tombstone on the way. const rows = await prisma.measurement.groupBy({ by: ["type"], where: { userId, type: { in: MCP_METRIC_STATUS_DISCOVERY.map((s) => s.measurementType) }, + deletedAt: null, }, _count: { _all: true }, }); diff --git a/src/lib/mcp/tools.ts b/src/lib/mcp/tools.ts index befde5e00..ad31be703 100644 --- a/src/lib/mcp/tools.ts +++ b/src/lib/mcp/tools.ts @@ -537,11 +537,15 @@ function searchAndFetchTools(): McpToolDefinition[] { ...MCP_CLINICAL_SIGNALS, ...MCP_METRIC_STATUS_DISCOVERY, ]; + // v1.41.2 — live rows only: a deleted reading is not a signal the + // record holds, and the filter lets the probe use the live-rows + // partial index instead of walking the tombstones. const discoverablePresent = await prisma.measurement.groupBy({ by: ["type"], where: { userId: ctx.userId, type: { in: discoverableSignals.map((s) => s.measurementType) }, + deletedAt: null, }, }); const presentTypes = new Set(discoverablePresent.map((r) => r.type)); @@ -815,6 +819,31 @@ const coachReadOutput: z.ZodRawShape = { reason: z.string().optional(), data: z.unknown().optional(), grounding: z.string().optional(), + // v1.41.2 — a miss describes its own scope and, when the record holds rows + // the window could not reach, what it holds (`CoachToolResult`). The + // executor has returned both since v1.37; undeclared, they made a strict + // client refuse every out-of-window read (the #1170 class). + searchedWindow: z.string().optional(), + available: z + .object({ + count: z.number(), + firstDate: z.string(), + lastDate: z.string(), + series: z + .array( + z.object({ + series: z.string(), + unit: z.string().nullable(), + count: z.number(), + mean: z.number(), + min: z.number(), + max: z.number(), + }), + ) + .optional(), + reachableWithWindow: z.string().nullable(), + }) + .optional(), }; /** @@ -895,6 +924,20 @@ const listMetricsOutput: z.ZodRawShape = { present: z.boolean(), count: z.number().optional(), metric: z.string().optional(), + // #1170 — what the record holds for a domain the window found + // nothing in (`InventoryEntry.availability`). The handler always + // returned it; the advertised schema did not declare it, and the + // output schema closes every object, so a strict client refused the + // whole result. + availability: z + .object({ + state: z.string(), + count: z.number().optional(), + firstDate: z.string().optional(), + lastDate: z.string().optional(), + reachableWithWindow: z.string().nullable(), + }) + .optional(), }), ) .optional(), @@ -1211,6 +1254,10 @@ const getIntradayPulseOutput: z.ZodRawShape = { startMinute: z.number(), mean: z.number(), count: z.number(), + // v1.41.2 — the bucket's low / high where the source carries a + // spread (`IntradayHrBucket`); returned since v1.30.7, never declared. + min: z.number().optional(), + max: z.number().optional(), }), ) .optional(), diff --git a/tests/integration/mcp-output-schema-conformance.test.ts b/tests/integration/mcp-output-schema-conformance.test.ts new file mode 100644 index 000000000..ec72c2d0a --- /dev/null +++ b/tests/integration/mcp-output-schema-conformance.test.ts @@ -0,0 +1,504 @@ +/** + * Every MCP tool's real output validates against the output schema + * `tools/list` advertises, under a strict JSON Schema 2020-12 validator. + * + * The advertised output schema closes every object (`additionalProperties: + * false`, the `io: "output"` rendering). The SDK checks `structuredContent` + * against the Zod shape on the server, and a Zod object strips a key it does + * not declare instead of refusing it, so a handler that returns a field its + * schema never declared passes every server-side check while a strict client + * refuses the whole result. That is how `list_metrics` shipped + * `availability` undeclared (#1170). + * + * This file closes the class rather than the one field: a real server over + * an in-memory transport, a real Postgres record with enough in it that the + * tools return their full shapes (present series, a history the default + * window cannot reach, labs, a medication, a workout, sleep stages, mood, + * clinical signals), and every advertised tool called with arguments that + * reach its main branches. Each `structuredContent` is validated against the + * schema exactly as it was advertised. A tool the case table does not cover + * fails the run, so a new tool cannot ship without joining it. + * + * It also pins the live-rows rule on the two presence probes: a measurement + * type whose readings were all deleted is not listed as present. + */ +import { beforeAll, describe, expect, it } from "vitest"; +import Ajv2020 from "ajv/dist/2020"; + +process.env.APP_URL ??= "http://localhost"; + +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; + +import { createMcpServer } from "@/lib/mcp/server"; +import { + MCP_CLINICAL_SIGNALS, + MCP_METRIC_STATUS_DISCOVERY, +} from "@/lib/mcp/rich-reads"; +import type { McpAuthContext } from "@/lib/mcp/auth"; + +import { getPrismaClient, truncateAllTables } from "./setup"; + +const DAY = 86_400_000; +const now = Date.now(); +const daysAgo = (n: number, hour = 8) => { + const d = new Date(now - n * DAY); + d.setUTCHours(hour, 0, 0, 0); + return d; +}; +const isoDay = (d: Date) => d.toISOString().slice(0, 10); + +/** A discovery metric with live rows, and one with only tombstones. */ +const LIVE_STATUS = MCP_METRIC_STATUS_DISCOVERY.find( + (s) => s.key === "CARDIO_RECOVERY", +)!; +const DELETED_STATUS = MCP_METRIC_STATUS_DISCOVERY.find( + (s) => s.key === "WRIST_TEMPERATURE", +)!; +const LIVE_SIGNAL = MCP_CLINICAL_SIGNALS.find((s) => s.key === "PAIN_NRS")!; +const DELETED_SIGNAL = MCP_CLINICAL_SIGNALS.find( + (s) => s.key === "GRIP_STRENGTH", +)!; + +let userId = ""; + +async function seedRecord(): Promise { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + const user = await prisma.user.create({ + data: { + username: "mcp-conformance", + email: "mcp-conformance@example.test", + timezone: "UTC", + createdAt: new Date(now - 800 * DAY), + }, + }); + const id = user.id; + + type Row = { + userId: string; + type: string; + value: number; + unit: string; + measuredAt: Date; + source?: string; + sleepStage?: string; + deletedAt?: Date; + }; + const rows: Row[] = []; + const daily = ( + type: string, + unit: string, + days: number, + value: (i: number) => number, + extra: Partial = {}, + ) => { + for (let i = 1; i <= days; i += 1) { + rows.push({ + userId: id, + type, + unit, + value: value(i), + measuredAt: daysAgo(i), + ...extra, + }); + } + }; + daily("WEIGHT", "kg", 120, (i) => 80 + (i % 7) * 0.2); + daily("BODY_FAT", "%", 60, (i) => 22 + (i % 5) * 0.1); + daily("BLOOD_PRESSURE_SYS", "mmHg", 60, (i) => 118 + (i % 9)); + daily("BLOOD_PRESSURE_DIA", "mmHg", 60, (i) => 76 + (i % 6)); + daily("RESTING_HEART_RATE", "bpm", 60, (i) => 58 + (i % 4)); + daily("HEART_RATE_VARIABILITY", "ms", 60, (i) => 45 + (i % 10)); + daily("ACTIVITY_STEPS", "count", 60, (i) => 6_000 + i * 37); + daily("BLOOD_GLUCOSE", "mg/dL", 30, (i) => 90 + (i % 20)); + daily(LIVE_STATUS.measurementType, "s", 30, (i) => 25 + (i % 3)); + daily(LIVE_SIGNAL.measurementType, "score", 20, (i) => i % 6); + // A domain the default window cannot reach: readings 200 days back only. + for (let i = 0; i < 6; i += 1) { + rows.push({ + userId: id, + type: "VO2_MAX", + unit: "mL/kg/min", + value: 41 + i * 0.3, + measuredAt: daysAgo(200 + i), + }); + } + // Pulse through yesterday, hourly, so the intraday read has a day. + for (let i = 1; i <= 14; i += 1) { + for (let h = 0; h < 24; h += 2) { + rows.push({ + userId: id, + type: "PULSE", + unit: "bpm", + value: 60 + ((i + h) % 25), + measuredAt: daysAgo(i, h), + }); + } + } + // Sleep stages per night. + for (let i = 1; i <= 30; i += 1) { + for (const [stage, minutes] of [ + ["CORE", 210], + ["DEEP", 60], + ["REM", 90], + ["AWAKE", 15], + ] as const) { + rows.push({ + userId: id, + type: "SLEEP_DURATION", + unit: "minutes", + value: minutes, + measuredAt: daysAgo(i, 6), + source: "APPLE_HEALTH", + sleepStage: stage, + }); + } + } + // Tombstones only: these two types must not count as present. + daily(DELETED_STATUS.measurementType, "°C", 40, () => 0.2, { + deletedAt: new Date(now - DAY), + }); + daily(DELETED_SIGNAL.measurementType, "kg", 40, () => 38, { + deletedAt: new Date(now - DAY), + }); + await prisma.measurement.createMany({ + data: rows.map((r) => ({ source: "MANUAL", ...r })) as never, + }); + + await prisma.labResult.createMany({ + data: [ + { + userId: id, + analyte: "LDL", + value: 3.1, + unit: "mmol/L", + takenAt: daysAgo(40), + }, + { + userId: id, + analyte: "LDL", + value: 2.8, + unit: "mmol/L", + takenAt: daysAgo(10), + }, + { + userId: id, + analyte: "HbA1c", + value: 5.4, + unit: "%", + takenAt: daysAgo(10), + }, + ], + }); + const med = await prisma.medication.create({ + data: { + userId: id, + name: "Ramipril", + dose: "5mg", + active: true, + createdAt: daysAgo(60), + schedules: { + create: { + windowStart: "08:00", + windowEnd: "09:00", + timesOfDay: ["08:00"], + scheduleType: "SCHEDULED", + }, + }, + }, + }); + for (let i = 1; i <= 14; i += 1) { + await prisma.medicationIntakeEvent.create({ + data: { + userId: id, + medicationId: med.id, + scheduledFor: daysAgo(i, 8), + takenAt: i % 5 === 0 ? null : daysAgo(i, 8), + skipped: i % 5 === 0, + source: "REMINDER", + }, + }); + } + await prisma.workout.create({ + data: { + userId: id, + sportType: "RUNNING", + startedAt: daysAgo(3, 17), + endedAt: new Date(daysAgo(3, 17).getTime() + 40 * 60_000), + durationSec: 2_400, + source: "MANUAL", + }, + }); + for (let i = 1; i <= 10; i += 1) { + await prisma.moodEntry.create({ + data: { + userId: id, + date: isoDay(daysAgo(i)), + mood: "GUT", + score: 3 + (i % 3), + moodLoggedAt: daysAgo(i, 20), + }, + }); + } + return id; +} + +function ctxFor(id: string): McpAuthContext { + return { + userId: id, + tokenId: "token-conformance", + scopes: ["health:read", "health:write"], + binding: `${id}:token-conformance`, + canRead: true, + canWrite: true, + }; +} + +async function connect(id: string) { + const server = createMcpServer(ctxFor(id)); + const client = new Client({ name: "conformance", version: "1.0.0" }); + const [clientSide, serverSide] = InMemoryTransport.createLinkedPair(); + await Promise.all([server.connect(serverSide), client.connect(clientSide)]); + return { + client, + close: async () => { + await client.close(); + await server.close(); + }, + }; +} + +/** + * The arguments each tool is called with: one entry per call, chosen to reach + * the tool's main branches. `fetch` takes its ids from `search` at run time. + */ +const CASES: Record>> = { + list_metrics: [{}], + get_metric_series: [ + { metric: "weight", window: "last90days" }, + { metric: "bp", window: "last30days" }, + { metric: "sleep", window: "last30days" }, + { metric: "vo2_max", window: "last30days" }, + ], + get_medication_compliance: [{ window: "last30days" }], + get_labs: [{}, { analyte: "LDL", history: true }, { analyte: "Ferritin" }], + get_correlations: [{}], + get_correlation: [{ metricA: "weight", metricB: "resting_hr" }], + compare_metric: [ + { metric: "weight", window: "last7days", windowB: "last30days" }, + { metric: "weight", metricB: "body_fat", window: "last30days" }, + { metric: LIVE_STATUS.key, window: "last30days" }, + ], + get_metric_baseline: [{ metric: "resting_hr" }, { metric: LIVE_SIGNAL.key }], + detect_changepoints: [{ metric: "weight", window: "last90days" }], + get_glucose_panel: [{ window: "last30days" }], + get_sleep: [{ window: "last30days" }], + get_workouts: [{ window: "last30days" }], + get_illness_recovery: [{}], + get_cycle: [{}], + get_metrics: [ + { metrics: ["weight", "bp", "steps"], window: "last30days" }, + { metrics: ["hrv", "vo2_max"] }, + ], + get_medication_schedule: [{}], + get_integration_status: [{}], + get_preventive_care: [{}], + get_nutrients: [{}, { nutrient: "water", days: 7 }], + get_intraday_pulse: [{ date: isoDay(daysAgo(1)) }, {}], + get_ecg_recordings: [{}], + get_visits: [{}, { months: 12 }], + search: [ + { query: "blood pressure" }, + { query: "LDL" }, + { query: "pain" }, + { query: "ramipril" }, + ], + fetch: [], + log_measurement: [ + { type: "WEIGHT", value: 80.4, unit: "kg", idempotencyKey: "conf-w-1" }, + { + type: "WEIGHT", + value: 80.4, + unit: "kg", + idempotencyKey: "conf-w-1", + confirm: true, + }, + ], + log_mood: [ + { score: 4, idempotencyKey: "conf-m-1" }, + { score: 4, idempotencyKey: "conf-m-1", confirm: true }, + ], + log_blood_pressure: [ + { systolic: 121, diastolic: 79, idempotencyKey: "conf-bp-1" }, + { + systolic: 121, + diastolic: 79, + idempotencyKey: "conf-bp-1", + confirm: true, + }, + ], +}; + +beforeAll(async () => { + userId = await seedRecord(); +}); + +describe("MCP tool outputs against their advertised schemas", () => { + it("every structuredContent validates under a strict 2020-12 validator", async () => { + const { client, close } = await connect(userId); + try { + const { tools } = await client.listTools(); + expect(tools.length).toBeGreaterThan(20); + const uncovered = tools + .map((t) => t.name) + .filter((name) => !(name in CASES)); + expect(uncovered, "add every new tool to CASES").toEqual([]); + + const ajv = new Ajv2020({ strict: true, allErrors: true }); + const failures: string[] = []; + let validated = 0; + // Tools that answered with data, not only a miss: a run where every + // read came back empty would validate nothing but the miss shape. + const presentReads = new Set(); + const fetchIds: string[] = []; + + const check = async ( + name: string, + schema: Record | undefined, + args: Record, + ) => { + const label = `${name}(${JSON.stringify(args)})`; + let result: Awaited>; + try { + // The SDK client validates against the advertised schema too, with + // its own validator; its refusal is a failure of the same kind. + result = await client.callTool({ name, arguments: args }); + } catch (err) { + failures.push(`${label}: ${(err as Error).message}`); + return undefined; + } + if (result.isError) { + failures.push( + `${label}: tool error ${JSON.stringify(result.content)}`, + ); + return undefined; + } + expect(schema, `${name} advertises an output schema`).toBeDefined(); + const structured = result.structuredContent; + if (!structured) { + failures.push(`${label}: no structuredContent`); + return undefined; + } + const validate = ajv.compile(schema!); + if (!validate(structured)) { + failures.push( + `${label}: ${ajv.errorsText(validate.errors, { separator: "; " })}`, + ); + } + validated += 1; + if ((structured as { present?: unknown }).present === true) { + presentReads.add(name); + } + return structured; + }; + + for (const tool of tools) { + if (tool.name === "fetch") continue; + for (const args of CASES[tool.name]) { + const out = await check( + tool.name, + tool.outputSchema as Record | undefined, + args, + ); + if (tool.name === "search" && out) { + for (const hit of (out as { results: Array<{ id: string }> }) + .results) { + fetchIds.push(hit.id); + } + } + } + } + + const fetchTool = tools.find((t) => t.name === "fetch")!; + const distinct = [...new Set(fetchIds)]; + expect(distinct.length).toBeGreaterThan(3); + for (const id of distinct) { + await check( + "fetch", + fetchTool.outputSchema as Record, + { id }, + ); + } + + expect(failures).toEqual([]); + expect(validated).toBeGreaterThan(Object.keys(CASES).length); + expect(presentReads.size).toBeGreaterThanOrEqual(12); + } finally { + await close(); + } + }); + + it("list_metrics carries availability, and it is declared", async () => { + const { client, close } = await connect(userId); + try { + const result = await client.callTool({ + name: "list_metrics", + arguments: {}, + }); + const metrics = ( + result.structuredContent as { + metrics: Array<{ present: boolean; availability?: unknown }>; + } + ).metrics; + // The seeded VO2 max history sits outside the default window: the + // row is absent but says what the record holds. Without it the + // conformance run above would not have exercised the field. + expect(metrics.some((m) => !m.present && m.availability)).toBe(true); + } finally { + await close(); + } + }); +}); + +describe("tombstones on the presence probes", () => { + it("list_metrics lists a live discovery metric and not one whose rows were all deleted", async () => { + const { client, close } = await connect(userId); + try { + const result = await client.callTool({ + name: "list_metrics", + arguments: {}, + }); + const metrics = ( + result.structuredContent as { + metrics: Array<{ metric?: string; present: boolean }>; + } + ).metrics; + const byMetric = new Map(metrics.map((m) => [m.metric, m])); + expect(byMetric.get(LIVE_STATUS.key)?.present).toBe(true); + expect(byMetric.get(DELETED_STATUS.key)?.present ?? false).toBe(false); + } finally { + await close(); + } + }); + + it("search finds a live clinical signal and not one whose rows were all deleted", async () => { + const { client, close } = await connect(userId); + try { + const ids = async (query: string) => + ( + (await client.callTool({ name: "search", arguments: { query } })) + .structuredContent as { results: Array<{ id: string }> } + ).results.map((r) => r.id); + expect(await ids("pain")).toContain(`metric:${LIVE_SIGNAL.key}`); + expect(await ids("grip strength")).not.toContain( + `metric:${DELETED_SIGNAL.key}`, + ); + expect(await ids("wrist temperature")).not.toContain( + `metric:${DELETED_STATUS.key}`, + ); + } finally { + await close(); + } + }); +});