diff --git a/src/lib/__tests__/doctor-report-module-backstop.test.ts b/src/lib/__tests__/doctor-report-module-backstop.test.ts new file mode 100644 index 000000000..a9ace3b97 --- /dev/null +++ b/src/lib/__tests__/doctor-report-module-backstop.test.ts @@ -0,0 +1,100 @@ +/** + * v1.30.22 — the fail-closed backstop inside `collectDoctorReportData`. + * + * The MCP surfaces now gate before calling, but gating only at call sites is + * exactly the arrangement that failed: `loadFhirContext` has an internal + * backstop for the same aggregate and the routes above it still gate, while + * this function had none and three MCP callers plus a public share link + * forgot. The backstop makes the door itself refuse, so the next caller + * cannot reintroduce the leak by omission. + * + * It throws rather than returning an envelope because reaching it means a + * caller is missing its gate — a bug to surface, not a flow to serve. Callers + * that can degrade gracefully (the MCP resources, the clinician share) gate + * themselves first and never reach it. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { prismaMock } = vi.hoisted(() => { + // Every model access throws, so any DB touch past the backstop is loud. + const boom = () => { + throw new Error("DB touched"); + }; + return { + prismaMock: new Proxy( + {}, + { + get: () => new Proxy({}, { get: () => boom }), + }, + ), + }; +}); +vi.mock("@/lib/db", () => ({ prisma: prismaMock })); +vi.mock("@/lib/modules/gate", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, resolveModuleMap: vi.fn() }; +}); + +import { collectDoctorReportData } from "../doctor-report-data"; +import { resolveModuleMap, MODULE_KEYS } from "@/lib/modules/gate"; + +const RANGE = { + start: new Date("2026-01-01T00:00:00.000Z"), + end: new Date("2026-04-01T00:00:00.000Z"), + days: 90, +}; + +/** Every module on, then apply the overrides. */ +function moduleMap(overrides: Record = {}) { + const out: Record = {}; + for (const key of MODULE_KEYS) out[key] = true; + return { ...out, ...overrides } as Record; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("collectDoctorReportData — doctorReport backstop", () => { + it("throws before any DB read when the module is off", async () => { + await expect( + collectDoctorReportData( + "u1", + RANGE, + // Injected rather than read, so the refusal is attributable to the + // module state and not to a DB stub. + { + moduleMap: moduleMap({ + doctorReport: false, + }) as never, + }, + ), + ).rejects.toThrow(/doctorReport/); + }); + + it("resolves the map itself when none is injected, and still refuses", async () => { + // The real callers do not inject; this is the path that actually protects + // the aggregate in production. + vi.mocked(resolveModuleMap).mockResolvedValue( + moduleMap({ doctorReport: false }) as never, + ); + + await expect(collectDoctorReportData("u1", RANGE)).rejects.toThrow( + /doctorReport/, + ); + expect(resolveModuleMap).toHaveBeenCalledWith("u1"); + }); + + it("does not refuse when the module is on", async () => { + // Positive control. The aggregate needs a full DB behind it, which this + // suite deliberately does not provide — so it will fail on the first + // query. What matters is that it gets THERE: the failure must be the DB + // stub, never the backstop, or the "off" assertions above would pass + // trivially for a function that always throws. + await expect( + collectDoctorReportData("u1", RANGE, { + moduleMap: moduleMap() as never, + }), + ).rejects.toThrow("DB touched"); + }); +}); diff --git a/src/lib/ai/coach/__tests__/correlations-module-gate.test.ts b/src/lib/ai/coach/__tests__/correlations-module-gate.test.ts new file mode 100644 index 000000000..dba6f3418 --- /dev/null +++ b/src/lib/ai/coach/__tests__/correlations-module-gate.test.ts @@ -0,0 +1,104 @@ +/** + * v1.30.22 — the `insights` gate on the correlations reader. + * + * `readCoachCorrelations` is reached from four places — the Coach + * `get_correlations` tool, the MCP `get_correlation` rich read, the + * per-metric "Coach read" strip, and the Coach snapshot — and only the REST + * sibling `/api/insights/correlations` ever gated. The reader also feeds on + * mood, symptom and compliance channels, so a mood-disabled account could + * receive pair rows naming mood with direction, lag, n and r. + * + * The gate lives in the reader, so these assertions cover every caller at + * once. That is the point of the placement: a fifth caller cannot reintroduce + * the gap by forgetting. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); + +const { prismaMock } = vi.hoisted(() => ({ + prismaMock: { + user: { findUnique: vi.fn() }, + measurement: { findMany: vi.fn() }, + moodEntry: { findMany: vi.fn() }, + userSourcePriority: { findMany: vi.fn() }, + }, +})); +vi.mock("@/lib/db", () => ({ prisma: prismaMock })); + +import { readCoachCorrelations } from "../tools/correlations-read"; +import { isModuleEnabled } from "@/lib/modules/gate"; + +const USER = "u1"; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(isModuleEnabled).mockImplementation(async () => true); + prismaMock.user.findUnique.mockResolvedValue({ timezone: "Europe/Berlin" }); + prismaMock.measurement.findMany.mockResolvedValue([]); + prismaMock.moodEntry.findMany.mockResolvedValue([]); + prismaMock.userSourcePriority.findMany.mockResolvedValue([]); +}); + +describe("readCoachCorrelations — insights module gate", () => { + it("gates on `insights`, not on a per-domain key", async () => { + await readCoachCorrelations(USER); + expect(isModuleEnabled).toHaveBeenCalledWith(USER, "insights"); + }); + + it("omits with the module OFF, and reads nothing at all", async () => { + vi.mocked(isModuleEnabled).mockImplementation(async () => false); + + const res = await readCoachCorrelations(USER); + + expect(res.present).toBe(false); + // Distinct from `no_data`: the assistant is told the domain is switched + // off rather than inferring the user has no correlatable history. + expect(res.reason).toBe("module_disabled"); + // The gate short-circuits before any channel is sourced. This is the + // load-bearing assertion — the reader draws on mood / symptom / + // compliance channels, so "computed then hid it" would still have pulled + // a disabled domain's rows into this process. + expect(prismaMock.measurement.findMany).not.toHaveBeenCalled(); + expect(prismaMock.moodEntry.findMany).not.toHaveBeenCalled(); + }); + + it("proceeds to the engine with the module ON", async () => { + const res = await readCoachCorrelations(USER); + // The synthetic fixture has no paired data, so the honest answer is a + // miss — but it must be the ENGINE's miss, reached after the real reads + // ran, not the gate's. + expect(prismaMock.measurement.findMany).toHaveBeenCalled(); + expect(res.reason).not.toBe("module_disabled"); + }); + + it("closes the operator kill-switch path, not only the user toggle", async () => { + // `isModuleEnabled` resolves `operatorAvailable && userEnabled`, so a + // server-wide operator disable arrives here as the same `false`. Asserted + // explicitly because it is the sharper case: the account holder cannot + // override an operator decision, so a leak past it is strictly worse than + // one past a personal preference. + vi.mocked(isModuleEnabled).mockImplementation( + async (_u: string, key: string) => key !== "insights", + ); + + const res = await readCoachCorrelations(USER); + + expect(res.present).toBe(false); + expect(res.reason).toBe("module_disabled"); + expect(prismaMock.measurement.findMany).not.toHaveBeenCalled(); + }); + + it("surfaces a gate failure instead of swallowing it as a miss", async () => { + // The reader's body is fail-soft (any throw degrades to + // `{ present: false }`) so a correlation hiccup never sinks a Coach turn. + // The gate sits OUTSIDE that try/catch on purpose: a broken gate must be + // loud, not silently indistinguishable from "no pattern found". + vi.mocked(isModuleEnabled).mockRejectedValue(new Error("gate down")); + + await expect(readCoachCorrelations(USER)).rejects.toThrow("gate down"); + }); +}); diff --git a/src/lib/ai/coach/__tests__/correlations-read.test.ts b/src/lib/ai/coach/__tests__/correlations-read.test.ts index 2ab4f2cb6..14b72f915 100644 --- a/src/lib/ai/coach/__tests__/correlations-read.test.ts +++ b/src/lib/ai/coach/__tests__/correlations-read.test.ts @@ -13,6 +13,9 @@ const intakeEventFindMany = vi.fn(); const illnessEpisodeFindMany = vi.fn(); const illnessDayLogFindMany = vi.fn(); const labResultFindMany = vi.fn(); +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/db", () => ({ prisma: { measurement: { findMany: (a: unknown) => measurementFindMany(a) }, diff --git a/src/lib/clinician-share/__tests__/share-view-data.test.ts b/src/lib/clinician-share/__tests__/share-view-data.test.ts index f5dc1a1cf..5eac1b659 100644 --- a/src/lib/clinician-share/__tests__/share-view-data.test.ts +++ b/src/lib/clinician-share/__tests__/share-view-data.test.ts @@ -10,6 +10,9 @@ */ import { describe, it, expect, vi, beforeEach } from "vitest"; +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/doctor-report-data", () => ({ collectDoctorReportData: vi.fn(), })); @@ -23,6 +26,7 @@ vi.mock("@/lib/db", () => ({ })); import { loadShareViewData } from "../share-view-data"; +import { isModuleEnabled } from "@/lib/modules/gate"; import { collectDoctorReportData } from "@/lib/doctor-report-data"; import { EMPTY_DOCTOR_REPORT_PREFS } from "@/lib/validations/doctor-report-prefs"; import { prisma } from "@/lib/db"; @@ -350,3 +354,79 @@ describe("loadShareViewData — grouped sections are folded, not silently defaul expect(opts.sections.weight).toBe(true); }); }); + +/** + * v1.30.22 — the owner's `doctorReport` module gates this surface too. + * + * Found while tracing every caller of the whole-record aggregate for the MCP + * fix: this link serves that same payload to an unauthenticated third party + * and never consulted the module key, so an owner (or an operator, via the + * availability switch ANDed above the user toggle) who turned the module off + * still had the full record served. + * + * Degrades to the share's OWN documents-only state rather than throwing: + * `documentOnly` is an existing, load-bearing privacy mode here (report + * `null`, aggregator never called), so a disabled module collapses the link to + * exactly the documents the owner attached — fail-closed for the health record + * without 500-ing a public link. + */ +describe("clinician share — owner doctorReport module gate", () => { + beforeEach(() => { + // Sibling of the suite above, so the outer reset does not reach here. + vi.clearAllMocks(); + vi.mocked(isModuleEnabled).mockImplementation(async () => true); + collect.mockResolvedValue({ patient: { displayName: "Shared record" } }); + }); + + it("aggregates normally with the module on", async () => { + collect.mockResolvedValue({ patient: { displayName: "Shared record" } }); + findDocs.mockResolvedValue([]); + + const res = await loadShareViewData( + ctx({ sectionsJson: { bp: true }, documentOnly: false }), + ); + + expect(res.documentOnly).toBe(false); + expect(res.report).not.toBeNull(); + expect(collect).toHaveBeenCalled(); + }); + + it("collapses to documents-only with the module off", async () => { + vi.mocked(isModuleEnabled).mockImplementation(async () => false); + findDocs.mockResolvedValue([]); + + const res = await loadShareViewData( + ctx({ sectionsJson: { bp: true }, documentOnly: false }), + ); + + expect(res.documentOnly).toBe(true); + expect(res.report).toBeNull(); + // Load-bearing: the aggregator is never called, so no health data leaves + // the DB at all — not built-then-withheld. + expect(collect).not.toHaveBeenCalled(); + }); + + it("gates on the OWNER's module state, not a viewer's", async () => { + // The link is public; there is no viewer session. The owner id must come + // from the frozen share context. + findDocs.mockResolvedValue([]); + await loadShareViewData( + ctx({ sectionsJson: { bp: true }, documentOnly: false }), + ); + expect(isModuleEnabled).toHaveBeenCalledWith("owner-1", "doctorReport"); + }); + + it("closes the operator kill-switch path too", async () => { + vi.mocked(isModuleEnabled).mockImplementation( + async (_u: string, key: string) => key !== "doctorReport", + ); + findDocs.mockResolvedValue([]); + + const res = await loadShareViewData( + ctx({ sectionsJson: { bp: true }, documentOnly: false }), + ); + + expect(res.documentOnly).toBe(true); + expect(collect).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/mcp/__tests__/doctor-report-module-gate.test.ts b/src/lib/mcp/__tests__/doctor-report-module-gate.test.ts new file mode 100644 index 000000000..a58a3e0ed --- /dev/null +++ b/src/lib/mcp/__tests__/doctor-report-module-gate.test.ts @@ -0,0 +1,148 @@ +/** + * v1.30.22 — the `doctorReport` gate on the whole-record aggregate. + * + * `collectDoctorReportData` consulted `resolveModuleMap` for its per-domain + * SECTIONS (mood / sleep / glucose / cycle / labs / recovery / workouts) but + * never for the `doctorReport` key that decides whether the aggregate may be + * assembled at all. Unlike `loadFhirContext` it carried no internal backstop, + * so it was left entirely to callers to remember — and the MCP surface did + * not: the fixed resource, its `{window}` template, and the + * `doctor_visit_summary` prompt each assembled the full record for an account + * with the module off. + * + * REFUSE, not omit. This is the whole record, so there is no honest partial + * answer; `/api/export/health-record` already answers 403 for exactly this + * state, and an empty summary invites the assistant to narrate "nothing on + * file" — a worse failure than an explicit unavailability. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/doctor-report-data", () => ({ + collectDoctorReportData: vi.fn(), +})); +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findUnique: vi.fn(async () => null) }, + medication: { findMany: vi.fn(async () => []) }, + labResult: { findMany: vi.fn(async () => []) }, + }, +})); +vi.mock("@/lib/ai/coach/tools/inventory", () => ({ + buildCoachDataInventory: vi.fn(async () => ({})), +})); +vi.mock("@/lib/ai/coach/tools/executor", () => ({ + executeCoachTool: vi.fn(async () => ({ present: false })), +})); +vi.mock("@/lib/insights/derived/baseline", () => ({ + loadBaselineProfile: vi.fn(async () => ({})), +})); +vi.mock("@/lib/insights/derived-briefing", () => ({ + detectDerivedBriefingSignals: vi.fn(async () => null), +})); +vi.mock("@/lib/insights/illness-cycle-briefing", () => ({ + buildBriefingIllnessCycleContext: vi.fn(async () => null), +})); +vi.mock("@/lib/tz/resolver", () => ({ + resolveUserTimezone: vi.fn(async () => "Europe/Berlin"), +})); +vi.mock("@/lib/mcp/rich-reads", () => ({ + metricStatusDiscoveryRows: vi.fn(async () => []), + MCP_METRIC_STATUS_DISCOVERY: [], + MCP_CLINICAL_SIGNALS: [], + getNutrients: vi.fn(async () => ({ present: false })), + getLabHistory: vi.fn(async () => ({ present: false })), + resolveRichMetric: vi.fn(() => null), +})); + +import { MCP_RESOURCES, MCP_RESOURCE_TEMPLATES } from "../resources"; +import { MCP_PROMPTS } from "../prompts"; +import { isModuleEnabled } from "@/lib/modules/gate"; +import { collectDoctorReportData } from "@/lib/doctor-report-data"; +import type { McpAuthContext } from "../auth"; + +const ctx = { userId: "u1" } as McpAuthContext; + +/** Minimal aggregate the visit summariser can reduce without throwing. */ +const REPORT_FIXTURE = { + period: { + days: 90, + since: "2026-03-29T00:00:00.000Z", + start: "2026-03-29T00:00:00.000Z", + end: "2026-06-27T00:00:00.000Z", + }, + patient: { username: "tester" }, + stats: { WEIGHT: { avg: 80, min: 79, max: 82, count: 30, latest: 80 } }, + compliance: {}, + medications: [], + labs: [], +}; + +const fixedResource = MCP_RESOURCES.find( + (r) => r.uri === "healthlog://report/doctor-visit", +)!; +const templateResource = MCP_RESOURCE_TEMPLATES.find( + (t) => t.uriTemplate === "healthlog://report/doctor-visit/{window}", +)!; +const prompt = MCP_PROMPTS.find((p) => p.name === "doctor_visit_summary")!; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(isModuleEnabled).mockImplementation(async () => true); + vi.mocked(collectDoctorReportData).mockResolvedValue( + REPORT_FIXTURE as unknown as Awaited< + ReturnType + >, + ); +}); + +describe("doctor-visit aggregate — doctorReport module gate", () => { + describe.each([ + { + name: "fixed resource", + run: () => fixedResource.read(ctx, {}), + }, + { + name: "windowed template", + run: () => templateResource.read(ctx, { window: "last30days" }), + }, + { + name: "doctor_visit_summary prompt", + run: () => prompt.run(ctx, {}), + }, + ])("$name", ({ run }) => { + it("serves with the module ON", async () => { + await expect(run()).resolves.toBeDefined(); + expect(collectDoctorReportData).toHaveBeenCalled(); + }); + + it("REFUSES with the module OFF and never assembles the record", async () => { + vi.mocked(isModuleEnabled).mockImplementation(async () => false); + + await expect(run()).rejects.toThrow(/doctorReport/); + // The refusal must precede assembly — the whole point is that the + // record is never built, not that it is built and then withheld. + expect(collectDoctorReportData).not.toHaveBeenCalled(); + }); + + it("refuses on the operator kill-switch path too", async () => { + // `isModuleEnabled` ANDs operator availability over the user toggle, so + // an operator disabling `doctorReport` server-wide arrives as the same + // `false`. The sharper case: no per-account setting can override it. + vi.mocked(isModuleEnabled).mockImplementation( + async (_u: string, key: string) => key !== "doctorReport", + ); + + await expect(run()).rejects.toThrow(/doctorReport/); + expect(collectDoctorReportData).not.toHaveBeenCalled(); + }); + }); + + it("gates on doctorReport specifically", async () => { + await fixedResource.read(ctx, {}); + expect(isModuleEnabled).toHaveBeenCalledWith("u1", "doctorReport"); + }); +}); diff --git a/src/lib/mcp/__tests__/prompts.test.ts b/src/lib/mcp/__tests__/prompts.test.ts index b3d3f9203..b5dc70158 100644 --- a/src/lib/mcp/__tests__/prompts.test.ts +++ b/src/lib/mcp/__tests__/prompts.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); vi.mock("@/lib/doctor-report-data", () => ({ collectDoctorReportData: vi.fn(), diff --git a/src/lib/mcp/__tests__/resources.test.ts b/src/lib/mcp/__tests__/resources.test.ts index cf55c8b57..f65313dc1 100644 --- a/src/lib/mcp/__tests__/resources.test.ts +++ b/src/lib/mcp/__tests__/resources.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/db", () => ({ prisma: { user: { findUnique: vi.fn() }, diff --git a/src/lib/mcp/__tests__/rich-reads-module-gate.test.ts b/src/lib/mcp/__tests__/rich-reads-module-gate.test.ts new file mode 100644 index 000000000..be6811fc2 --- /dev/null +++ b/src/lib/mcp/__tests__/rich-reads-module-gate.test.ts @@ -0,0 +1,180 @@ +/** + * v1.30.22 — the module gate on the rollup-backed rich reads. + * + * These three reads deliberately bypass `buildCoachSnapshot`, which is where + * the module narrowing lives for every Coach-routed read. They therefore + * inherited no gate at all: `get_metric_series` honestly reported + * `{ present: false }` for a glucose-disabled account while + * `get_metric_baseline` handed back the median, the MAD band and today's + * placement for the same metric — on the one wire that egresses to a + * third-party assistant. + * + * Both layers matter and both are asserted here. `isModuleEnabled` resolves + * `operatorAvailable(key) && userEnabled(key)`, so a `false` from it is the + * OPERATOR kill-switch just as much as the user toggle — the sharper case, + * because a leak there defeats a server-wide decision the account holder + * cannot override. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/ai/coach/tools/correlations-read", () => ({ + readCoachCorrelations: vi.fn(async () => ({ present: false })), +})); +vi.mock("@/lib/insights/derived/coach-read", () => ({ + buildCoachReadStrip: vi.fn(), +})); +vi.mock("@/lib/rollups/measurement-read-wmy", async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, readBestGranularityRollups: vi.fn() }; +}); +const { labResult, measurement } = vi.hoisted(() => ({ + labResult: { findMany: vi.fn() }, + measurement: { count: vi.fn(async () => 0), groupBy: vi.fn(async () => []) }, +})); +vi.mock("@/lib/db", () => ({ prisma: { labResult, measurement } })); + +import { + compareMetric, + getMetricBaseline, + detectChangepoints, + metricStatusDiscoveryRows, + MCP_METRIC_STATUS_DISCOVERY, +} from "../rich-reads"; +import { isModuleEnabled } from "@/lib/modules/gate"; +import { buildCoachReadStrip } from "@/lib/insights/derived/coach-read"; +import { readBestGranularityRollups } from "@/lib/rollups/measurement-read-wmy"; +import type { ModuleKey } from "@/lib/modules/gate"; + +const USER = "u1"; + +/** A bucket sequence dense enough for every read to produce a real answer. */ +function buckets(n = 60) { + const start = Date.UTC(2026, 0, 1); + return { + granularity: "DAY" as const, + rows: Array.from({ length: n }, (_, i) => ({ + bucketStart: new Date(start + i * 86_400_000), + // A clean level shift halfway so `detect_changepoints` has something to + // find — the read must be capable of a `present: true` answer, or a + // `present: false` under the gate would prove nothing. + sum: i < n / 2 ? 100 : 160, + count: 1, + min: i < n / 2 ? 100 : 160, + max: i < n / 2 ? 100 : 160, + })), + }; +} + +/** Drive the gate: every module on except the named one. */ +function disableModule(key: ModuleKey) { + vi.mocked(isModuleEnabled).mockImplementation( + async (_userId: string, moduleKey: ModuleKey) => moduleKey !== key, + ); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(isModuleEnabled).mockImplementation(async () => true); + measurement.count.mockResolvedValue(0); + measurement.groupBy.mockResolvedValue([]); + vi.mocked(readBestGranularityRollups).mockResolvedValue( + buckets() as unknown as Awaited< + ReturnType + >, + ); + vi.mocked(buildCoachReadStrip).mockResolvedValue({ + baseline: { median: 100, low: 90, high: 110 }, + learning: false, + latest: { value: 105, at: new Date().toISOString() }, + } as unknown as Awaited>); +}); + +describe("rich reads — per-domain module gate", () => { + // `glucose` → BLOOD_GLUCOSE is the pairing the audit named explicitly: the + // metric where the honest `get_metric_series` and the leaking + // `get_metric_baseline` disagreed for the same account. + describe.each([ + { metric: "glucose", module: "glucose" as ModuleKey }, + { metric: "sleep", module: "sleep" as ModuleKey }, + { metric: "hrv", module: "recovery" as ModuleKey }, + { metric: "sleep_score", module: "sleep" as ModuleKey }, + { metric: "day_strain", module: "workouts" as ModuleKey }, + ])("$metric (owned by $module)", ({ metric, module }) => { + it("get_metric_baseline answers with the module ON", async () => { + const on = await getMetricBaseline(USER, { metric }); + expect(on.present).toBe(true); + }); + + it("get_metric_baseline omits with the module OFF", async () => { + disableModule(module); + const off = await getMetricBaseline(USER, { metric }); + expect(off.present).toBe(false); + expect(off.reason).toBe("module_disabled"); + // The gate must short-circuit BEFORE the engine runs — a read that + // computes the band and then hides it has still loaded the data. + expect(buildCoachReadStrip).not.toHaveBeenCalled(); + }); + + it("compare_metric omits with the module OFF", async () => { + disableModule(module); + const off = await compareMetric(USER, { + metric, + window: "last30days", + windowB: "last90days", + }); + expect(off.present).toBe(false); + expect(off.reason).toBe("module_disabled"); + expect(readBestGranularityRollups).not.toHaveBeenCalled(); + }); + + it("detect_changepoints omits with the module OFF", async () => { + disableModule(module); + const off = await detectChangepoints(USER, { metric }); + expect(off.present).toBe(false); + expect(off.reason).toBe("module_disabled"); + expect(readBestGranularityRollups).not.toHaveBeenCalled(); + }); + }); + + it("refuses the whole comparison when only side B is gated", async () => { + // A one-sided degrade would answer the question anyway using the ungated + // side, so the gated side must sink the entire call. + disableModule("glucose"); + const res = await compareMetric(USER, { + metric: "weight", + metricB: "glucose", + }); + expect(res.present).toBe(false); + expect(res.reason).toBe("module_disabled_b"); + }); + + it("leaves a metric no module owns ungated", async () => { + // Weight/pulse/BP are core clinical figures with no owning module; gating + // them would hide data the user never opted out of. Turning every module + // off must not touch them. + vi.mocked(isModuleEnabled).mockImplementation(async () => false); + const res = await getMetricBaseline(USER, { metric: "weight" }); + expect(res.present).toBe(true); + }); + + it("drops a gated metric from the discovery listing", async () => { + // Discovery is the assistant's map of what exists. Advertising a metric + // that the fetch will then refuse both leaks that the domain is tracked + // and sends the assistant down a dead end. + disableModule("sleep"); + const rows = await metricStatusDiscoveryRows(USER); + const keys = rows.map((r) => r.metric); + expect(keys).not.toContain("SLEEP_SCORE"); + expect(keys).not.toContain("BREATHING_DISTURBANCES"); + // …while an unowned metric in the same listing survives. + expect(MCP_METRIC_STATUS_DISCOVERY.map((s) => s.key)).toContain( + "FALL_COUNT", + ); + expect(keys).toContain("FALL_COUNT"); + }); +}); diff --git a/src/lib/mcp/__tests__/rich-reads.test.ts b/src/lib/mcp/__tests__/rich-reads.test.ts index f786b14cc..e6a4f1460 100644 --- a/src/lib/mcp/__tests__/rich-reads.test.ts +++ b/src/lib/mcp/__tests__/rich-reads.test.ts @@ -1,6 +1,9 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // --- Engine mocks hoisted before importing the module under test. --- +vi.mock("@/lib/modules/gate", () => ({ + isModuleEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); vi.mock("@/lib/ai/coach/tools/correlations-read", () => ({ readCoachCorrelations: vi.fn(), diff --git a/src/lib/modules/__tests__/measurement-scope.test.ts b/src/lib/modules/__tests__/measurement-scope.test.ts new file mode 100644 index 000000000..c9bc9eda8 --- /dev/null +++ b/src/lib/modules/__tests__/measurement-scope.test.ts @@ -0,0 +1,136 @@ +/** + * v1.30.22 — structural guard on the module→data ownership table. + * + * The three MCP leaks all had one root cause: the layer assumed every read + * inherits gating from `buildCoachSnapshot`, which is true for the + * Coach-routed tools and false for the families that deliberately bypass it. + * Fixing the three call sites alone would leave the assumption in place, so + * the ownership map moved to a shared table and the rich reads gate off it. + * + * WHAT THIS TEST FREEZES + * + * 1. Every metric the rich reads can resolve has been LOOKED AT: it either + * has an owning module or is named in `UNSCOPED_REVIEWED_TYPES`. A new + * metric that lands in the resolver without either fails here, which is + * the specific way finding 2 was able to happen — metrics became + * resolvable over MCP without anyone deciding who owned them. + * 2. The table stays consistent with the Coach snapshot's own narrowing, so + * the MCP wire and the Coach prompt cannot drift into disagreeing about + * which module owns which domain. + * 3. `UNSCOPED_REVIEWED_TYPES` stays a review record, not a dumping ground: + * an entry that later gains an owner must leave the list. + * + * WHAT IT CANNOT PROVE — read this before trusting it + * + * - It does NOT prove any read calls the gate. A future `getMetricSummary` + * that calls `resolveRichMetric` directly instead of the module-aware + * resolver would leak exactly as before and this test would stay green. + * Only the per-read tests in `mcp/__tests__/rich-reads-module-gate.test.ts` + * cover that, and only for the reads that exist today. There is no + * compile-time barrier stopping a new caller from reaching the ungated + * resolver — `resolveRichMetric` is still exported because + * `build-efficacy` and the tests use it for pure resolvability checks. + * - It does NOT prove the OWNERSHIP assignments are correct, only that they + * exist. Mapping SLEEP_SCORE to `sleep` is a human judgement; if it truly + * belonged to `recovery`, this test would be just as green. + * - It says nothing about the surfaces that gate on a module key directly + * rather than through a metric (`insights` for correlations, `doctorReport` + * for the aggregate). Those are covered by their own tests. + */ +import { describe, it, expect } from "vitest"; + +import { + MODULE_SCOPED_SOURCES, + UNSCOPED_REVIEWED_TYPES, + moduleForMeasurementType, +} from "../measurement-scope"; +import { MODULE_KEYS, type ModuleKey } from "../gate"; +import { + MCP_METRIC_STATUS_DISCOVERY, + MCP_CLINICAL_SIGNALS, + resolveRichMetric, +} from "@/lib/mcp/rich-reads"; +import { METRIC_STATUS_IDS } from "@/lib/insights/metric-status-registry"; + +/** + * Every `MeasurementType` reachable through the rich-read resolver, collected + * the way an assistant would actually reach them: the discovery listings plus + * every metric-status id the resolver accepts by exact id. + */ +function resolvableTypes(): Set { + const out = new Set(); + for (const sig of MCP_METRIC_STATUS_DISCOVERY) out.add(sig.measurementType); + for (const sig of MCP_CLINICAL_SIGNALS) out.add(sig.measurementType); + for (const id of METRIC_STATUS_IDS) { + const m = resolveRichMetric(id); + if (m) out.add(m.measurementType); + } + for (const name of ["weight", "pulse", "bmi", "glucose", "sleep", "hrv"]) { + const m = resolveRichMetric(name); + if (m) out.add(m.measurementType); + } + return out; +} + +describe("module→data ownership table", () => { + it("has an owner or a written review decision for every resolvable metric", () => { + const unreviewed = [...resolvableTypes()].filter( + (t) => + moduleForMeasurementType(t as never) === null && + !UNSCOPED_REVIEWED_TYPES.has(t as never), + ); + + // A failure here is not a bug to suppress: a metric became readable over + // MCP without anyone deciding whether a module owns it. Either add it to + // `METRIC_STATUS_MODULE_OWNERS` or record it in `UNSCOPED_REVIEWED_TYPES` + // with a reason. + expect(unreviewed).toEqual([]); + }); + + it("never assigns a metric to a module key that does not exist", () => { + for (const type of resolvableTypes()) { + const owner = moduleForMeasurementType(type as never); + if (owner) expect(MODULE_KEYS).toContain(owner); + } + }); + + it("keeps the reviewed-unscoped list free of metrics that now have an owner", () => { + const stale = [...UNSCOPED_REVIEWED_TYPES].filter( + (t) => moduleForMeasurementType(t) !== null, + ); + expect(stale).toEqual([]); + }); + + it("scopes only module keys that actually exist", () => { + for (const key of Object.keys(MODULE_SCOPED_SOURCES)) { + expect(MODULE_KEYS).toContain(key as ModuleKey); + } + }); + + it("resolves the domains the audit named to the modules that own them", () => { + // Spot-checks with real consequences: these are the exact pairings where + // `get_metric_series` (gated, via the snapshot) and `get_metric_baseline` + // (ungated, before this change) disagreed for the same account. + expect(moduleForMeasurementType("BLOOD_GLUCOSE")).toBe("glucose"); + expect(moduleForMeasurementType("SLEEP_DURATION")).toBe("sleep"); + expect(moduleForMeasurementType("HEART_RATE_VARIABILITY")).toBe("recovery"); + expect(moduleForMeasurementType("HRV_RMSSD")).toBe("recovery"); + expect(moduleForMeasurementType("SLEEP_SCORE")).toBe("sleep"); + expect(moduleForMeasurementType("BREATHING_DISTURBANCES")).toBe("sleep"); + expect(moduleForMeasurementType("DAY_STRAIN")).toBe("workouts"); + expect(moduleForMeasurementType("WORKOUT_STRAIN")).toBe("workouts"); + expect(moduleForMeasurementType("CARDIO_LOAD")).toBe("workouts"); + expect(moduleForMeasurementType("ANS_CHARGE")).toBe("recovery"); + expect(moduleForMeasurementType("CARDIO_RECOVERY")).toBe("recovery"); + }); + + it("leaves the core clinical figures unowned on purpose", () => { + // Gating these would hide data behind a toggle the user never associated + // with them. Pinned so a future broad-brush "gate everything" change has + // to argue with a test. + expect(moduleForMeasurementType("WEIGHT")).toBeNull(); + expect(moduleForMeasurementType("PULSE")).toBeNull(); + expect(moduleForMeasurementType("BLOOD_PRESSURE_SYS")).toBeNull(); + expect(moduleForMeasurementType("BODY_MASS_INDEX")).toBeNull(); + }); +}); diff --git a/src/lib/modules/measurement-scope.ts b/src/lib/modules/measurement-scope.ts index 636c0d224..58d4b58ac 100644 --- a/src/lib/modules/measurement-scope.ts +++ b/src/lib/modules/measurement-scope.ts @@ -101,14 +101,38 @@ const METRIC_STATUS_MODULE_OWNERS: Partial> = CARDIO_LOAD: "workouts", ANS_CHARGE: "recovery", CARDIO_RECOVERY: "recovery", + // The HRV fallback type. `resolveRichMetricForUser` swaps HRV to its RMSSD + // fallback for an account that only has ring/strap nightly RMSSD rows, so + // leaving this unowned would have let the fallback path serve exactly the + // recovery data the primary type refuses. Caught by the structural test. + HRV_RMSSD: "recovery", }; /** * Measurement types that resolve over the rich reads but that NO module owns — - * reviewed and deliberately ungated. They are core clinical / mobility figures - * that exist independently of any toggleable module (weight, pulse, blood - * pressure, BMI, body composition, gait, fall count, the walk-test metrics), - * so gating them would hide data the user never opted out of. + * reviewed and deliberately ungated. + * + * These are not an oversight and not a backlog: they are the domains the + * module model has never claimed. The Coach snapshot does not narrow any of + * them either (none of their scope tokens appears in + * {@link MODULE_SCOPED_SOURCES}), so gating them here would make the MCP wire + * STRICTER than the app and the Coach — hiding data behind a toggle the user + * never associated with it. Grouped by why: + * + * - Core clinical figures every account carries: weight, pulse, blood + * pressure, BMI, body temperature. + * - Body composition: fat / lean / muscle / bone / water / visceral. + * - Activity volume: steps, distance, flights, active energy. Owned by no + * module — `workouts` gates workout SESSIONS, not ambient movement. + * - Gait and mobility, incl. the fall / walk-test / stair metrics. + * - Cardio-vascular readings with no module home: SpO2, respiratory rate, + * walking HR, pulse-wave velocity, vascular age, avg/max HR. + * - v1.25 clinical signals: grip strength, pain NRS, waist measures. + * - WRIST_TEMPERATURE and ENERGY_EXPENDITURE_KJ: plausible arguments exist + * for `sleep` and `workouts` respectively, but neither module claims them + * anywhere else in the tree, and guessing an owner would hide a metric + * behind an unrelated toggle. Left unowned deliberately rather than + * assigned on a hunch. * * This set exists so the structural test can distinguish "reviewed as * unscoped" from "nobody has looked at this yet" — a new resolvable metric @@ -116,20 +140,53 @@ const METRIC_STATUS_MODULE_OWNERS: Partial> = */ export const UNSCOPED_REVIEWED_TYPES: ReadonlySet = new Set([ + // Core clinical. "WEIGHT", "PULSE", "BODY_MASS_INDEX", "BLOOD_PRESSURE_SYS", "BLOOD_PRESSURE_DIA", + "BODY_TEMPERATURE", + // Body composition. "BODY_FAT", - "WRIST_TEMPERATURE", + "FAT_MASS", + "FAT_FREE_MASS", + "LEAN_BODY_MASS", + "MUSCLE_MASS", + "BONE_MASS", + "TOTAL_BODY_WATER", + "VISCERAL_FAT", + // Ambient activity volume (not workout sessions). + "ACTIVITY_STEPS", + "WALKING_RUNNING_DISTANCE", + "FLIGHTS_CLIMBED", + "ACTIVE_ENERGY_BURNED", "ENERGY_EXPENDITURE_KJ", + // Gait / mobility. + "WALKING_SPEED", + "WALKING_STEADINESS", + "WALKING_ASYMMETRY", + "WALKING_DOUBLE_SUPPORT", + "WALKING_STEP_LENGTH", "FALL_COUNT", "SIX_MINUTE_WALK_DISTANCE", "STAIR_ASCENT_SPEED", "STAIR_DESCENT_SPEED", + // Cardio-vascular with no module home. + "OXYGEN_SATURATION", + "RESPIRATORY_RATE", + "WALKING_HEART_RATE_AVERAGE", + "PULSE_WAVE_VELOCITY", + "VASCULAR_AGE", "AVERAGE_HEART_RATE", "MAX_HEART_RATE", + // v1.25 clinical signals. + "GRIP_STRENGTH", + "PAIN_NRS", + "WAIST_CIRCUMFERENCE", + "WAIST_TO_HEIGHT", + // Temperature with a plausible but unclaimed owner. + "WRIST_TEMPERATURE", ]); /**