test: tighten coach-disable discovery, drop forwardRef sentinel, plug fake-timer leak

- target-card.tsx added to coach-user-disable fixture (per-card CTA hides)
- grep-discovery test mirrors coach-cascade for user.disableCoach +
  useDisableCoach — new sites must be added to the allowlist explicitly
- textarea forwardRef Symbol assertion deleted (React-internal API)
- vi.useRealTimers() moved to afterEach in medications/intake and
  dashboard/summary route tests; a failed assertion can no longer
  leak fake timers across suites

Closes L2 + L12 + L13 + H2 in v1.4.48 backlog.
This commit is contained in:
Marc-André Bombeck committed 2026-05-23 10:22:12 +02:00
1 parent ae57e3d335
commit 59e2e8299f
4 files changed
+223 -18

No files matched your search

@@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { NextRequest } from "next/server";
vi.mock("@/lib/db", () => ({
@@ -94,6 +94,14 @@ beforeEach(() => {
} as never);
});
// v1.4.48 L13 — restore real timers no matter how a test exits. A failed
// assertion inside an `it` that opted into fake timers would otherwise
// leak the fake clock into the next test in the file (or, worse, the
// next file in the same vitest worker).
afterEach(() => {
vi.useRealTimers();
});
describe("GET /api/dashboard/summary", () => {
it("returns 401 when unauthenticated", async () => {
vi.mocked(getSession).mockResolvedValue(null);
@@ -653,7 +661,7 @@ describe("GET /api/dashboard/summary", () => {
data: { compliance: { scheduledToday: number; takenToday: number } };
};
expect(body.data.compliance.scheduledToday).toBe(1);
vi.useRealTimers();
// v1.4.48 L13 — suite-level `afterEach` restores real timers.
});
it("computes intake compliance for today", async () => {
@@ -1,4 +1,4 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { NextRequest } from "next/server";
vi.mock("@/lib/db", () => ({
@@ -103,6 +103,14 @@ beforeEach(() => {
vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never);
});
// v1.4.48 L13 — restore real timers no matter how a test exits. A failed
// assertion inside an `it` that opted into fake timers would otherwise
// leak the fake clock into the next test in the file (or, worse, the
// next file in the same vitest worker).
afterEach(() => {
vi.useRealTimers();
});
describe("GET /api/medications/intake", () => {
it("returns 401 when unauthenticated", async () => {
vi.mocked(getSession).mockResolvedValue(null);
@@ -262,7 +270,7 @@ describe("GET /api/medications/intake", () => {
);
expect(res.status).toBe(200);
expect(prisma.medicationIntakeEvent.createMany).not.toHaveBeenCalled();
vi.useRealTimers();
// v1.4.48 L13 — suite-level `afterEach` restores real timers.
});
it("returns compliance buckets for the last N days", async () => {
@@ -63,20 +63,6 @@ describe("<Textarea>", () => {
expect(html).toContain("sm:text-sm");
});
it("is wrapped in React.forwardRef so refs reach the <textarea>", () => {
// React tags forwardRef components with a sentinel symbol on
// `$$typeof`. Verifying the sentinel keeps the assertion
// dependency-free (no testing-library / DOM env needed) while
// still catching a regression that strips the wrapper.
const forwardRefSymbol = Symbol.for("react.forward_ref");
const componentMeta = Textarea as unknown as {
$$typeof: symbol;
render?: unknown;
};
expect(componentMeta.$$typeof).toBe(forwardRefSymbol);
expect(typeof componentMeta.render).toBe("function");
});
it("forwards rows / placeholder / maxLength to the underlying element", () => {
const html = renderToStaticMarkup(
<Textarea
@@ -1,3 +1,6 @@
import { readdirSync, readFileSync, statSync } from "node:fs";
import { join, relative, resolve } from "node:path";
import React from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
@@ -16,6 +19,9 @@ import { LayoutCoachFab } from "@/components/insights/layout-coach-fab";
import { LayoutCoachMount } from "@/components/insights/layout-coach-mount";
import { SuggestedPrompts } from "@/components/insights/suggested-prompts";
import { HeroStrip } from "@/components/insights/hero-strip";
import { TargetCard, type TargetCardData } from "@/components/targets/target-card";
import { useFeatureFlags } from "@/hooks/use-feature-flags";
import { useDisableCoach } from "@/hooks/use-disable-coach";
/**
* v1.4.47 W3 — per-user "Hide Coach" opt-out invariant.
@@ -171,8 +177,61 @@ const DISABLE_COACH_SURFACES: DisableCoachSurface[] = [
),
proofWhenVisible: 'data-slot="insights-hero-strip-action-coach"',
},
{
// v1.4.48 L2 — `<TargetCard>` carries the per-card "Ask the coach"
// CTA on the targets page. The card itself only consults
// `flags.coach`; the per-user disable gate is computed by the parent
// (`src/app/targets/page.tsx`) which does
// `aiEnabled = flags.coach && !user?.disableCoach && providerActive`.
// The driver below mirrors that wiring so the negative assertion
// (CTA absent when `disableCoach` is true) pins the chain end-to-end
// and trips a regression in either the card-level gate or the
// page-level computation.
name: "TargetCard per-card 'Ask the coach' CTA",
mount: () => <TargetCardDriver />,
proofWhenVisible: 'data-slot="target-coach-cta"',
},
];
/**
* Inline driver that mirrors the `src/app/targets/page.tsx` wiring of
* `aiEnabled` to the per-card CTA. Reads the same hooks the page does,
* threads the resolved boolean into `<TargetCard>`. Keeps the user-
* disable fixture honest about the cascade — a future contributor who
* "simplifies" the page's `coachEnabled` computation and drops the
* `user?.disableCoach` branch fails this surface's negative assertion.
*/
const TARGET_CARD_FIXTURE: TargetCardData = {
type: "WEIGHT",
label: "Weight",
current: 78.2,
average30: 79.1,
trend: "down",
unit: "kg",
range: { min: 60, max: 80 },
classification: { category: "Normal", color: "#50fa7b" },
source: "WHO BMI",
daysInRange7d: 5,
daysLogged7d: 7,
lastMetGoalAt: "2026-05-10",
streakDays: 4,
insufficientData: false,
consistency7d: ["in", "in", "near", "in", "out", "in", "in"],
};
function TargetCardDriver() {
const flags = useFeatureFlags();
const disableCoach = useDisableCoach();
const aiEnabled = flags.coach && !disableCoach;
return (
<TargetCard
target={TARGET_CARD_FIXTURE}
aiEnabled={aiEnabled}
onAskCoach={() => undefined}
/>
);
}
describe("Coach per-user disableCoach invariant", () => {
beforeEach(() => {
vi.spyOn(console, "error").mockImplementation(() => undefined);
@@ -236,4 +295,148 @@ describe("Coach per-user disableCoach invariant", () => {
).toHaveBeenCalled();
}
});
// ──────────────────────────────────────────────────────────────────
// v1.4.48 H2 — grep-based discovery of every `useDisableCoach` /
// `user.disableCoach` call site in the codebase.
//
// Mirrors the `flags.coach` discovery walk in `coach-cascade.test.tsx`:
// the surface fixture above pins the surfaces this file mounts
// directly. Cross-cut gates on sub-pages (`/targets` page parent
// computation, Settings → Insights toggle, `/api/auth/me` response
// shape, the disable-coach PATCH route) sit OUTSIDE the fixture and
// are owned by sibling invariants or by route-level tests.
//
// The walk below scans `src/` for every `useDisableCoach` /
// `user.disableCoach` / `user?.disableCoach` occurrence and requires
// each path to appear in the explicit `KNOWN_DISABLE_COACH_GATE_SITES`
// allowlist. Add a new entry every time you intentionally read the
// per-user disable flag in a new path; the failure message names the
// unaccounted-for paths so the fix is one search-and-add.
// ──────────────────────────────────────────────────────────────────
/**
* Every file in `src/` that legitimately reads `useDisableCoach` or
* `user.disableCoach`. Lives next to the disable-cascade fixture so a
* contributor extending the fixture sees the allowlist in the same
* review.
*
* Paths are POSIX-style relative to the repo root.
*/
const KNOWN_DISABLE_COACH_GATE_SITES: ReadonlyArray<string> = [
// Coach-bearing surfaces mounted directly by the disable-cascade
// fixture above; each one calls `useDisableCoach()` and short-
// circuits to `null` when the per-user flag is on.
"src/components/insights/coach-launch-button.tsx",
"src/components/insights/hero-strip.tsx",
"src/components/insights/layout-coach-fab.tsx",
"src/components/insights/layout-coach-mount.tsx",
"src/components/insights/suggested-prompts.tsx",
// The hook itself + its `useAuth`-backed reader.
"src/hooks/use-disable-coach.ts",
// Cross-cut gates owned by sibling invariants / route tests.
// - `/api/auth/me` ships the flag in the session payload so the
// client hooks can read it (`useAuth → useDisableCoach`). The
// sibling `/api/auth/me/disable-coach` PATCH endpoint that
// writes the flag does NOT read `user.disableCoach` (it reads
// `body.disableCoach`) and is intentionally not listed here.
"src/app/api/auth/me/route.ts",
// - The targets page's parent-level cascade: computes
// `coachEnabled = flags.coach && !user?.disableCoach` and threads
// `aiEnabled` down to `<TargetCard>`. The disable-cascade fixture
// covers the end-to-end chain via `<TargetCardDriver>`.
"src/app/targets/page.tsx",
// - Settings → Insights toggle reads + writes the flag.
"src/components/settings/ai-section.tsx",
];
/**
* Walk every TS / TSX file under `src/`, excluding test trees + the
* Prisma client output, and collect the relative paths that
* legitimately reference `useDisableCoach` or `user.disableCoach`.
* Any literal occurrence on a non-comment line counts so a
* contributor who paraphrases the read still trips this test.
*/
function findDisableCoachSites(): string[] {
const srcRoot = resolve(__dirname, "..", "..", "..");
const repoRoot = resolve(srcRoot, "..");
const hits: string[] = [];
function isRealGateLine(line: string): boolean {
// Trim once so leading whitespace doesn't fool the comment check.
const trimmed = line.trim();
if (trimmed.startsWith("//")) return false;
if (trimmed.startsWith("*")) return false;
// `useDisableCoach` covers both the hook import + the hook call.
if (line.includes("useDisableCoach")) return true;
// `user.disableCoach` / `user?.disableCoach` — the canonical
// per-user gate read on the auth user object. Strict match on
// `user` so we don't grab unrelated `disableCoach` strings (route
// body keys, i18n message ids, …).
if (/\buser\??\.disableCoach\b/.test(line)) return true;
return false;
}
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 === "node_modules" || entry === ".next") continue;
if (entry === "__tests__") continue;
// Skip the Prisma client output — the generator embeds the
// entire `prisma/schema.prisma` source as a JSON-escaped
// `inlineSchema` string and any schema doc-comment that
// mentions `disableCoach` would otherwise leak through here.
if (entry === "generated") continue;
walk(full);
continue;
}
if (!/\.(tsx|ts)$/.test(entry)) continue;
if (/\.test\.(tsx|ts)$/.test(entry)) continue;
const text = readFileSync(full, "utf8");
if (text.split("\n").some(isRealGateLine)) {
hits.push(relative(repoRoot, full).split(/[\\/]/).join("/"));
}
}
}
walk(srcRoot);
return hits.sort();
}
it("grep-based discovery pins every `useDisableCoach` / `user.disableCoach` call site", () => {
const discovered = findDisableCoachSites();
const allowlist = new Set(KNOWN_DISABLE_COACH_GATE_SITES);
const orphans = discovered.filter((path) => !allowlist.has(path));
expect(
orphans,
[
"Found `useDisableCoach` / `user.disableCoach` in file(s) not on",
"the KNOWN_DISABLE_COACH_GATE_SITES allowlist",
"(src/lib/feature-flags/__tests__/coach-user-disable.test.tsx).",
"Add a fixture entry (or a sibling invariant) for the new gate,",
"then add the path to the allowlist. Orphans:",
...orphans.map((p) => ` - ${p}`),
].join("\n"),
).toEqual([]);
// Sanity check the other direction — an allowlist entry that no
// longer references the gate is dead documentation and should be
// pruned in the same PR that removes the read.
const discoveredSet = new Set(discovered);
const stale = KNOWN_DISABLE_COACH_GATE_SITES.filter(
(path) => !discoveredSet.has(path),
);
expect(
stale,
[
"KNOWN_DISABLE_COACH_GATE_SITES references file(s) that no longer",
"use `useDisableCoach` / `user.disableCoach`. Remove the stale",
"allowlist entry:",
...stale.map((p) => ` - ${p}`),
].join("\n"),
).toEqual([]);
});
});