Cover the module gates on the MCP export reads

One suite per gate: the rich reads omit with an honest module_disabled
reason, the correlations reader omits and touches no channel, the
doctor-visit resource and prompt refuse without assembling the record,
and the clinician share collapses to its documents-only state. Each
asserts the operator availability path alongside the user toggle, and
that the gate short-circuits before the engine runs rather than building
the answer and withholding it.

The structural test pins that every metric the rich reads can resolve has
either an owning module or a written review decision, and is explicit in
its own comment about what it cannot prove — it does not show that any
read calls the gate. Writing it surfaced an unowned HRV_RMSSD, the
fallback type an RMSSD-only account resolves to, which would have served
recovery data the primary type refuses.
This commit is contained in:
Marc-André Bombeck committed 2026-07-19 03:18:25 +02:00
1 parent fa2b9531f7
commit bb41b1673e
11 files changed
+822 -5

No files matched your search

@@ -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<typeof import("@/lib/modules/gate")>();
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<string, boolean> = {}) {
const out: Record<string, boolean> = {};
for (const key of MODULE_KEYS) out[key] = true;
return { ...out, ...overrides } as Record<string, boolean>;
}
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");
});
});
@@ -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");
});
});
@@ -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) },
@@ -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();
});
});
@@ -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<typeof collectDoctorReportData>
>,
);
});
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");
});
});
+3
View File
@@ -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(),
+3
View File
@@ -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() },
@@ -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<typeof import("@/lib/rollups/measurement-read-wmy")>();
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<typeof readBestGranularityRollups>
>,
);
vi.mocked(buildCoachReadStrip).mockResolvedValue({
baseline: { median: 100, low: 90, high: 110 },
learning: false,
latest: { value: 105, at: new Date().toISOString() },
} as unknown as Awaited<ReturnType<typeof buildCoachReadStrip>>);
});
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");
});
});
+3
View File
@@ -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(),
@@ -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<string> {
const out = new Set<string>();
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();
});
});
+62 -5
View File
@@ -101,14 +101,38 @@ const METRIC_STATUS_MODULE_OWNERS: Partial<Record<MeasurementType, ModuleKey>> =
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<Record<MeasurementType, ModuleKey>> =
*/
export const UNSCOPED_REVIEWED_TYPES: ReadonlySet<MeasurementType> =
new Set<MeasurementType>([
// 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",
]);
/**