fix(mcp): exempt the reviewed discovery allowlist from the mcp:false veto

The signal registry's `mcp:false` flag vetoes a metric-status resolution
so an unreviewed non-exposed signal cannot leak through the id/display-name
path. The metric-status discovery allowlist is a deliberate, human-named
exposure — co-equal to `mcp:true` — so its ids must stay resolvable, or
compare_metric / get_metric_baseline would advertise a metric in discovery
that then refuses to resolve. Exempt the allowlist from the veto; the guard
still fires for genuinely-unreviewed mcp:false ids (workout HR stats).
This commit is contained in:
Marc-André Bombeck committed 2026-07-18 08:06:42 +02:00
1 parent a70c98d2b4
commit d2e36e2546
2 files changed
+55 -11

No files matched your search

+28 -9
View File
@@ -133,16 +133,35 @@ describe("resolveRichMetric", () => {
// v1.30.4 (C2) — the signal registry's `surfaces.mcp` flag is documented as
// the SINGLE source of truth for MCP exposure, but `resolveRichMetric`'s
// metric-status-registry path (exact id / display-name match) used to
// resolve an id regardless of that flag. `CARDIO_RECOVERY` /
// `WRIST_TEMPERATURE` / `SLEEP_SCORE` are all valid `METRIC_STATUS_IDS`
// entries but carry `mcp:false` in the signal registry — pin that they
// now stay unreachable, closing the contract gap (no PHI actually leaked
// through it, but a future `mcp:false` signal without this guard would).
// resolve an id regardless of that flag. `AVERAGE_HEART_RATE` /
// `MAX_HEART_RATE` are valid `METRIC_STATUS_IDS` entries but carry
// `mcp:false` in the signal registry AND are NOT on the reviewed
// metric-status discovery allowlist — pin that they stay unreachable,
// closing the contract gap (no PHI actually leaked through it, but a future
// `mcp:false` signal without this guard would). The reviewed discovery ids
// (`CARDIO_RECOVERY` / `WRIST_TEMPERATURE` / `SLEEP_SCORE` / …) are the one
// deliberate exemption and stay resolvable — asserted below.
it("honours the signal registry's mcp:false as a hard veto over a metric-status hit", () => {
expect(resolveRichMetric("CARDIO_RECOVERY")).toBeNull();
expect(resolveRichMetric("cardio recovery")).toBeNull();
expect(resolveRichMetric("WRIST_TEMPERATURE")).toBeNull();
expect(resolveRichMetric("SLEEP_SCORE")).toBeNull();
expect(resolveRichMetric("AVERAGE_HEART_RATE")).toBeNull();
expect(resolveRichMetric("MAX_HEART_RATE")).toBeNull();
expect(resolveRichMetric("max heart rate")).toBeNull();
});
// v1.30.4 (coverage review) — the reviewed metric-status discovery allowlist
// is a deliberate MCP exposure, co-equal to a signal `mcp:true`. A
// `mcp:false` on the same signal must NOT strip these ids' resolvability, or
// `compare_metric` / `get_metric_baseline` would advertise a metric in
// discovery that then refuses to resolve.
it("exempts the reviewed metric-status discovery allowlist from the mcp:false veto", () => {
expect(resolveRichMetric("CARDIO_RECOVERY")?.measurementType).toBe(
"CARDIO_RECOVERY",
);
expect(resolveRichMetric("WRIST_TEMPERATURE")?.measurementType).toBe(
"WRIST_TEMPERATURE",
);
expect(resolveRichMetric("SLEEP_SCORE")?.measurementType).toBe(
"SLEEP_SCORE",
);
});
it("still resolves a metric-status id the signal registry does NOT mark mcp:false", () => {
+27 -2
View File
@@ -214,9 +214,19 @@ function fromRegistry(id: string): RichMetric | null {
// the SAME id in the signal registry as a hard veto over a metric-status
// hit; an id absent from the signal registry (most of `METRIC_STATUS_IDS`)
// is unaffected — metric-status stays authoritative when the two don't
// disagree.
// disagree. The one exemption is the reviewed metric-status discovery
// allowlist (`REVIEWED_DISCOVERY_IDS`): those ids are a deliberate,
// human-named MCP exposure — co-equal to a signal `mcp:true` — so a
// `mcp:false` on the same signal must NOT strip their resolvability, or
// `compare_metric` / `get_metric_baseline` would advertise a metric in
// discovery that then refuses to resolve.
const sig = getSignal(id);
if (sig && sig.kind === "measurement" && sig.surfaces.mcp === false) {
if (
sig &&
sig.kind === "measurement" &&
sig.surfaces.mcp === false &&
!REVIEWED_DISCOVERY_IDS.has(id)
) {
return null;
}
const metric: RichMetric = {
@@ -400,6 +410,21 @@ const METRIC_STATUS_DISCOVERY_IDS = [
"ENERGY_EXPENDITURE_KJ",
] as const satisfies readonly MetricStatusMetricId[];
/**
* The reviewed metric-status discovery allowlist as a membership Set. Each
* member is a DELIBERATE, human-named MCP exposure (v1.30.4 coverage review) —
* co-equal to a signal-registry `mcp:true`, just carried in the metric-status
* layer instead of the signal layer. `fromRegistry`'s `mcp:false` veto exempts
* these ids so a reviewed metric stays resolvable (`compare_metric` /
* `get_metric_baseline` / `detect_changepoints`); the veto still fires for any
* UNREVIEWED `mcp:false` signal that happens to carry a metric-status entry
* (the two mental-health screeners and every `ENV_*` signal are absent from
* this list and have no metric-status entry, so they stay unreachable).
*/
const REVIEWED_DISCOVERY_IDS: ReadonlySet<string> = new Set(
METRIC_STATUS_DISCOVERY_IDS,
);
/**
* The metric-status-only ids `list_metrics` / the inventory resource /
* `search` can now discover: the registry id (also the `metric:` id `fetch`