mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 13:22:14 +00:00
test(coach): discovery test pins requireAssistantSurface on every coach insights route
Walk every route.ts under src/app/api/insights/ and require each handler to either invoke requireAssistantSurface("coach") or appear on an explicit allowlist (insightStatus / correlations gates, or non-Coach reads like provider-chain). The walk uncovered two orphan Coach routes — GET/DELETE /api/insights/chat/[id] and POST /api/insights/chat/messages/[id]/feedback — both of which manipulate Coach-only resources but lacked the operator kill-switch. Add the gate to both so a disabled Coach matrix produces a 403 + assistant.disabled.coach envelope through the same path the chat SSE route already uses.
This commit is contained in:
1 parent
1e9bde7b60
commit
1eb0038914
3 files changed
+192
No files matched your search
@@ -0,0 +1,177 @@
|
||||
import { readdirSync, readFileSync, statSync } from "node:fs";
|
||||
import { join, relative, resolve } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
/**
|
||||
* v1.4.38 W-C M6 — Coach API route gate inventory.
|
||||
*
|
||||
* The web surfaces are protected by the `flags.coach` short-circuit in
|
||||
* every Coach-bearing component (see
|
||||
* `src/lib/feature-flags/__tests__/coach-cascade.test.tsx`). The
|
||||
* server-side mirror is `requireAssistantSurface("coach")` in every
|
||||
* Coach-only route handler. Without a discovery test, a future
|
||||
* contributor who lands a new `/api/insights/<foo>/route.ts` whose body
|
||||
* touches the Coach stack would have to remember the gate by hand — a
|
||||
* silent miss leaks the surface even when the operator turned the
|
||||
* Coach matrix off.
|
||||
*
|
||||
* The walk below scans every `route.ts` under `src/app/api/insights/`
|
||||
* and groups each handler into one of three buckets:
|
||||
*
|
||||
* 1. Coach-gated — file imports + invokes
|
||||
* `requireAssistantSurface("coach")`.
|
||||
* 2. Other-gated — file invokes `requireAssistantSurface()` with a
|
||||
* different surface (`insightStatus`,
|
||||
* `correlations`, `briefing`, `healthScoreExplainer`).
|
||||
* 3. Allowlisted — non-Coach Insights routes that don't gate on
|
||||
* the matrix at all (provider chain, settings,
|
||||
* targets, glp1-timeline, feedback). Allowlist
|
||||
* lives alongside the test for easy review.
|
||||
*
|
||||
* Anything that doesn't fit one of the three buckets is an orphan and
|
||||
* fails the test by name so the fix is one search-and-add.
|
||||
*/
|
||||
|
||||
const NON_COACH_GATED_ROUTES: ReadonlyArray<string> = [
|
||||
// Non-Coach assistant surfaces — gated on a sibling sub-flag.
|
||||
"src/app/api/insights/blood-pressure-status/route.ts",
|
||||
"src/app/api/insights/bmi-status/route.ts",
|
||||
"src/app/api/insights/cards/route.ts",
|
||||
"src/app/api/insights/correlations/route.ts",
|
||||
"src/app/api/insights/medication-compliance-status/route.ts",
|
||||
"src/app/api/insights/mood-status/route.ts",
|
||||
"src/app/api/insights/pulse-status/route.ts",
|
||||
"src/app/api/insights/weight-status/route.ts",
|
||||
];
|
||||
|
||||
const NOT_COACH_OWNED_ROUTES: ReadonlyArray<string> = [
|
||||
// Settings + chain inventory — surface-agnostic infra reads. The
|
||||
// settings UI itself is server-rendered and the chain page hides
|
||||
// when no provider is configured; neither one carries assistant
|
||||
// prose. Disabling Coach must not break these reads — the user can
|
||||
// still inspect and reconfigure the chain that gets re-enabled.
|
||||
"src/app/api/insights/provider-chain/route.ts",
|
||||
"src/app/api/insights/settings/route.ts",
|
||||
// Read-only data feeds — `/targets` is a non-assistant Zielwerte
|
||||
// page driven by classifications, not LLM prose; the GLP-1 timeline
|
||||
// is the same data the medication page paints whether the assistant
|
||||
// is on or off. Recommendation feedback (`/feedback`) records a
|
||||
// thumbs-up/-down on a NON-Coach recommendation row that was
|
||||
// produced before the operator flipped the flag; gating it would
|
||||
// mean the operator's mid-flight toggle erases the user's
|
||||
// ability to dispose of stale recommendations.
|
||||
"src/app/api/insights/feedback/route.ts",
|
||||
"src/app/api/insights/glp1-timeline/route.ts",
|
||||
"src/app/api/insights/targets/route.ts",
|
||||
];
|
||||
|
||||
const COACH_GATE_NEEDLE = 'requireAssistantSurface("coach")';
|
||||
const ANY_GATE_NEEDLE = "requireAssistantSurface(";
|
||||
|
||||
/**
|
||||
* Walk every `route.ts` under `src/app/api/insights/` (including
|
||||
* dynamic segments under `[id]`). Returns POSIX-style paths relative
|
||||
* to the repo root.
|
||||
*/
|
||||
function findInsightsRouteFiles(): string[] {
|
||||
// __dirname here is `src/app/api/insights/__tests__/`, so the
|
||||
// insights root is one directory up.
|
||||
const insightsRoot = resolve(__dirname, "..");
|
||||
const repoRoot = resolve(insightsRoot, "..", "..", "..", "..");
|
||||
const hits: string[] = [];
|
||||
|
||||
function walk(dir: string): void {
|
||||
const entries = readdirSync(dir);
|
||||
for (const entry of entries) {
|
||||
const full = join(dir, entry);
|
||||
const stat = statSync(full);
|
||||
if (stat.isDirectory()) {
|
||||
if (entry === "__tests__") continue;
|
||||
if (entry === "node_modules") continue;
|
||||
walk(full);
|
||||
continue;
|
||||
}
|
||||
if (entry !== "route.ts") continue;
|
||||
hits.push(relative(repoRoot, full).split(/[\\/]/).join("/"));
|
||||
}
|
||||
}
|
||||
|
||||
walk(insightsRoot);
|
||||
return hits.sort();
|
||||
}
|
||||
|
||||
describe("Coach API route gate inventory", () => {
|
||||
it("every Coach-bearing insights route imports requireAssistantSurface('coach')", () => {
|
||||
const repoRoot = resolve(__dirname, "..", "..", "..", "..", "..");
|
||||
const routes = findInsightsRouteFiles();
|
||||
expect(routes.length).toBeGreaterThan(0);
|
||||
|
||||
const allowlistOther = new Set(NON_COACH_GATED_ROUTES);
|
||||
const allowlistNone = new Set(NOT_COACH_OWNED_ROUTES);
|
||||
|
||||
const orphans: Array<{ path: string; reason: string }> = [];
|
||||
|
||||
for (const path of routes) {
|
||||
const full = resolve(repoRoot, path);
|
||||
const text = readFileSync(full, "utf8");
|
||||
const hasCoachGate = text.includes(COACH_GATE_NEEDLE);
|
||||
const hasAnyGate = text.includes(ANY_GATE_NEEDLE);
|
||||
|
||||
if (hasCoachGate) continue;
|
||||
|
||||
if (allowlistOther.has(path)) {
|
||||
// Sanity — the file should still gate on SOME assistant
|
||||
// surface. A stale entry on this allowlist is misleading
|
||||
// documentation; flip the failure mode so the orphan is
|
||||
// reported with the right diagnosis.
|
||||
if (!hasAnyGate) {
|
||||
orphans.push({
|
||||
path,
|
||||
reason:
|
||||
"listed in NON_COACH_GATED_ROUTES but has no requireAssistantSurface() call at all",
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (allowlistNone.has(path)) continue;
|
||||
|
||||
orphans.push({
|
||||
path,
|
||||
reason: `missing requireAssistantSurface("coach"); add the gate or move the route onto an allowlist (NON_COACH_GATED_ROUTES / NOT_COACH_OWNED_ROUTES)`,
|
||||
});
|
||||
}
|
||||
|
||||
expect(
|
||||
orphans,
|
||||
[
|
||||
"Coach API route gate inventory found unaccounted-for handler(s):",
|
||||
...orphans.map((o) => ` - ${o.path}: ${o.reason}`),
|
||||
].join("\n"),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("allowlists do not reference deleted route files", () => {
|
||||
const repoRoot = resolve(__dirname, "..", "..", "..", "..", "..");
|
||||
const known = new Set(findInsightsRouteFiles());
|
||||
|
||||
const stale = [...NON_COACH_GATED_ROUTES, ...NOT_COACH_OWNED_ROUTES].filter(
|
||||
(path) => !known.has(path),
|
||||
);
|
||||
|
||||
expect(
|
||||
stale,
|
||||
[
|
||||
"Allowlist entries point to files that no longer exist —",
|
||||
"delete the stale entries from",
|
||||
"`src/app/api/insights/__tests__/coach-route-gate-inventory.test.ts`:",
|
||||
...stale.map((p) => ` - ${p}`),
|
||||
].join("\n"),
|
||||
).toEqual([]);
|
||||
|
||||
// Silence "repoRoot declared and not used" if the loop above
|
||||
// returns empty — reference it explicitly.
|
||||
expect(repoRoot.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
@@ -13,6 +13,7 @@ import type { NextRequest } from "next/server";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import { apiSuccess, apiError } from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { requireAssistantSurface } from "@/lib/feature-flags";
|
||||
|
||||
import {
|
||||
deleteConversation,
|
||||
@@ -25,6 +26,10 @@ interface RouteCtx {
|
||||
|
||||
export const GET = apiHandler(async (_request: NextRequest, ctx: RouteCtx) => {
|
||||
const auth = await requireAuth();
|
||||
// v1.4.38 W-C M6 — operator can hide the Coach surface app-wide;
|
||||
// the conversation reader is part of the Coach stack (encrypted
|
||||
// assistant prose), so a disabled surface must 403 here too.
|
||||
await requireAssistantSurface("coach");
|
||||
const { id } = await ctx.params;
|
||||
if (!id) return apiError("coach.conversation.notFound", 404);
|
||||
|
||||
@@ -44,6 +49,10 @@ export const GET = apiHandler(async (_request: NextRequest, ctx: RouteCtx) => {
|
||||
export const DELETE = apiHandler(
|
||||
async (_request: NextRequest, ctx: RouteCtx) => {
|
||||
const auth = await requireAuth();
|
||||
// v1.4.38 W-C M6 — same gate as GET; a disabled Coach surface
|
||||
// means the user can't reach the conversation list anyway, so the
|
||||
// delete affordance must stay behind the same kill-switch.
|
||||
await requireAssistantSurface("coach");
|
||||
const { id } = await ctx.params;
|
||||
if (!id) return apiError("coach.conversation.notFound", 404);
|
||||
|
||||
|
||||
@@ -41,6 +41,7 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { Prisma } from "@/generated/prisma/client";
|
||||
import { resolveCoachFeedbackAttribution } from "@/lib/ai/feedback-attribution";
|
||||
import { requireAssistantSurface } from "@/lib/feature-flags";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
|
||||
@@ -55,6 +56,11 @@ interface RouteContext {
|
||||
|
||||
async function handlePost(request: NextRequest, ctx: RouteContext) {
|
||||
const { user } = await requireAuth();
|
||||
// v1.4.38 W-C M6 — Coach message feedback is a Coach surface
|
||||
// affordance; gate it on the same operator kill-switch so a
|
||||
// disabled Coach matrix can't be skirted through the feedback
|
||||
// endpoint.
|
||||
await requireAssistantSurface("coach");
|
||||
const { id: messageId } = await ctx.params;
|
||||
|
||||
// Per-user rate limit — same shape as the recommendation feedback
|
||||
|
||||
Reference in new issue
Block a user