diff --git a/src/__tests__/destructive-control-guard.test.ts b/src/__tests__/destructive-control-guard.test.ts new file mode 100644 index 000000000..053e3211a --- /dev/null +++ b/src/__tests__/destructive-control-guard.test.ts @@ -0,0 +1,815 @@ +/** + * Structural guard on destructive controls: nothing that can take a person's + * data away ships without someone having written down what it takes and what + * gates it. + * + * WHY A REGISTRY RATHER THAN A PATTERN MATCH + * + * The obvious guard — "every `Trash2` trigger must sit next to an + * `AlertDialog`" — fails in both directions, and it fails quietly. It misses + * the control that destroys the most: Settings → AI cleared a person's + * free-text note AND their allergy line from one untargeted tap, through a + * `PUT` carrying two empty strings, with no trash icon involved in the write + * and nothing in the payload spelling "delete". It cries wolf on the reverse: + * a `Trash2` that removes a row from an unsaved draft, a "reset" that only + * touches local state until a separate Save, an undo affordance inside a + * success toast. Both classes are live in this tree. A guard that has to be + * suppressed on half its hits teaches people to suppress it. + * + * So the honest shape is: the REGISTRY below is the source of truth for what + * is destructive, because that judgement is not mechanical. Three detectors + * then make FORGETTING TO REGISTER the failure mode — each is complete for + * its own syntactic shape, and together they cover every way this codebase + * currently issues a destroying request: + * + * D1 a DELETE goes out `apiDelete(…)` / `method: "DELETE"` + * D2 a route whose own name is the act `/api/…/{disconnect,revoke,reset, + * purge,clear,wipe,bulk-delete}` + * D3 a write blanks a stored field a payload literal assigning `""` + * or `null` to a named key + * + * WHAT THIS CANNOT DO — stated plainly, because a guard that oversells itself + * is worse than none: + * + * - It cannot prove the registry is COMPLETE. A destructive control that + * issues no DELETE, hits no act-named route, and blanks no literal field — + * a PUT that overwrites a list with a shorter one, say — is invisible to + * all three detectors and will not be caught by anything here. + * - It cannot prove a registered claim is TRUE. `confirm: ["ConfirmButton"]` + * is checked only to the extent that the named mechanism is present in one + * of the entry's files. A reviewer who waves through a wrong entry defeats + * it, and no test substitutes for that review. + * - `unconfirmed` entries are the escape hatch, and they are deliberately + * expensive: each needs a written reason long enough to be an argument + * rather than a shrug. There is no silent skip. + * + * What it does hold: a new destructive control cannot land unnoticed, a + * removed one cannot leave a lie behind, and the specific regression that + * started this — a destroying payload fired straight from an `onClick` — goes + * red at the call site. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync, globSync } from "node:fs"; +import { join, sep } from "node:path"; + +const SRC = join(process.cwd(), "src"); + +type ConfirmKind = + | "ConfirmButton" + | "ConfirmDialog" + | "DeleteButton" + | "SelectionActionBar" + | "AlertDialog" + | "inline-two-step" + | "typed-phrase"; + +/** + * How bounded the loss is. This is the axis that decides whether a control + * wants a confirmation, an undo, or both, so the registry makes the next + * person answer it rather than leaving it implied. + * + * permanent a column overwritten, or a row hard-deleted. + * Nothing in the product brings it back. + * tombstoned-no-restore soft-deleted in the database, but no restore route + * and no surface a person could reach it from. Gone, + * as far as the person is concerned. + * restorable a restore route exists AND a control reaches it. + */ +type Recovery = "permanent" | "tombstoned-no-restore" | "restorable"; + +interface DestructiveEntry { + /** Path under `src/`, forward slashes. Where the destroying request lives. */ + file: string; + /** Plain words: what a person loses. */ + destroys: string; + /** Files carrying the trigger, when the mutation lives in a hook. */ + triggers?: string[]; + recovery: Recovery; + /** Mechanisms gating the triggers. Empty only alongside `unconfirmed`. */ + confirm: ConfirmKind[]; + /** + * Controls in this entry that deliberately do NOT confirm. Every one needs + * an argument, not a note — the length floor is enforced below. + */ + unconfirmed?: { control: string; reason: string }[]; + /** + * Set when the entry is deliberately kept although no detector sees it — + * the concrete form of "this guard cannot prove the registry is complete". + * The value says which shape slips past, so the blind spot is on the record + * instead of being inferred from an absent row. + */ + undetected?: string; +} + +const REGISTRY: DestructiveEntry[] = [ + // ── Account, credentials, sessions ────────────────────────────────────── + { + file: "components/settings/advanced-section.tsx", + destroys: + "every health record on the account, or the whole account including passkeys and audit history", + recovery: "permanent", + confirm: ["AlertDialog"], + unconfirmed: [ + { + control: "research-mode switch, off position", + reason: + "Turning research mode off drops the stored acknowledgement receipt, which is regenerated by turning it back on and re-acknowledging. Nothing a person authored or measured is involved, so a dialog on a labelled switch would gate a preference rather than a loss.", + }, + ], + }, + { + file: "components/settings/security-sessions-card.tsx", + destroys: "one login session, or every session other than this one", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/trusted-devices-card.tsx", + destroys: "the second-factor memory for one device, or for all of them", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/security-section/passkey-list-section.tsx", + destroys: "a passkey — hard-deleted, the credential cannot be re-derived", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/settings/security-section/security-keys-card.tsx", + destroys: "a WebAuthn second factor", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/settings/api-section.tsx", + destroys: "an API token — revoked, and the secret is never shown again", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/settings/mcp-section.tsx", + destroys: "an MCP client grant or an MCP token", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/settings/sharing-section.tsx", + destroys: "a clinician share link — the URL stays dead once revoked", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/settings/account-section/avatar-section.tsx", + destroys: "the uploaded profile photo, deleted from the server", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + + // ── AI credentials ────────────────────────────────────────────────────── + { + file: "components/settings/ai/codex-provider-form.tsx", + destroys: "the stored Codex sign-in and the cached insight text with it", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/admin/central-codex-section.tsx", + destroys: + "the instance-wide shared AI sign-in — every user relying on it loses AI features", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/admin/ai-server-key-section.tsx", + destroys: + "the operator-wide AI provider key, which is never displayed again once stored", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/ai/ai-consent-card.tsx", + destroys: "the AI consent receipt that gates every AI surface", + recovery: "restorable", + confirm: ["inline-two-step"], + }, + { + file: "components/settings/about-me-section.tsx", + destroys: "the free-text self-context note the Coach and briefing read", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/coach-memory-section.tsx", + destroys: "one remembered Coach fact, or every one of them", + recovery: "tombstoned-no-restore", + confirm: ["ConfirmButton", "AlertDialog"], + }, + { + file: "hooks/use-coach-reminders.ts", + destroys: "a Coach reminder", + triggers: ["components/settings/coach-reminders-section.tsx"], + recovery: "tombstoned-no-restore", + confirm: ["ConfirmButton"], + }, + { + file: "hooks/use-coach-plans.ts", + destroys: "a Coach plan and the record of how it went", + triggers: [ + "app/coach/plans/page.tsx", + "components/insights/coach-panel/plan-proposal-card.tsx", + ], + recovery: "tombstoned-no-restore", + confirm: ["ConfirmButton"], + unconfirmed: [ + { + control: + "decline a PROPOSED plan — app/coach/plans/page.tsx and plan-proposal-card.tsx", + reason: + "Declining is the negative half of an accept/decline pair the assistant just offered; the person has invested nothing in a proposal they never adopted, and the same suggestion re-derives on a later turn. Confirming a refusal would make saying no more expensive than saying yes. The delete of a plan the person actually RAN is confirmed, and that is the one carrying history.", + }, + ], + }, + { + file: "components/insights/coach-panel/use-coach.ts", + destroys: "a whole Coach conversation and every message in it", + triggers: [ + "components/insights/coach-panel/history-rail.tsx", + "app/coach/conversations/page.tsx", + ], + recovery: "permanent", + confirm: [], + unconfirmed: [ + { + control: + "delete conversation — history-rail.tsx and conversations page", + reason: + "This surface deliberately chose undo over confirmation: the delete is held for a few seconds behind an undo toast, and the earlier arm-then-confirm double tap was removed on purpose. Adding a dialog back would give the same act two gates. The pending-delete flush on unmount is a real hole in that promise and is tracked separately; it is a defect in the undo, not an argument for a confirmation.", + }, + ], + }, + { + file: "components/insights/coach-panel/coach-conversation.tsx", + destroys: + "the stored list of pending clarifying questions, and document attachments on a conversation", + recovery: "permanent", + confirm: [], + unconfirmed: [ + { + control: "dismiss questions; detach a document", + reason: + "Neither touches something the person wrote. The questions are assistant-generated suggestions that the extractor regenerates on later turns, and dismissing them IS the affordance. Detaching removes a join row only — the document itself stays in the vault and is two taps from being re-attached.", + }, + ], + }, + + // ── Preferences and layout a person arranged by hand ───────────────────── + { + file: "components/settings/thresholds-editor-section.tsx", + destroys: "the custom range set for one metric, or for every metric", + recovery: "permanent", + confirm: ["ConfirmButton", "ConfirmDialog"], + }, + { + file: "components/targets/target-edit-sheet.tsx", + destroys: + "the custom target range for one metric — both halves at once for blood pressure", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/dashboard-layout-section.tsx", + destroys: + "the dashboard arrangement: tile order, hidden tiles, ring choice", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/insights/insights-edit-mode.tsx", + destroys: "the Insights overview arrangement: section order and visibility", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/environment-section.tsx", + destroys: "a saved travel location", + recovery: "permanent", + confirm: ["ConfirmButton"], + }, + { + file: "components/settings/web-push-card.tsx", + destroys: "the browser push subscription row", + recovery: "restorable", + confirm: [], + unconfirmed: [ + { + control: "unsubscribe from push notifications", + reason: + "This is the off half of a notification channel toggle. Re-subscribing from the same control recreates the row, nothing the person authored is stored in it, and the browser permission survives. Gating it would put a dialog on a switch.", + }, + ], + }, + + // ── Measurements, mood, labs, illness — the undo-and-restore surfaces ──── + { + file: "components/measurements/measurement-list.tsx", + destroys: "one measurement, or every selected measurement", + recovery: "restorable", + confirm: ["DeleteButton", "SelectionActionBar", "AlertDialog"], + }, + { + file: "components/mood/mood-list.tsx", + destroys: "one mood entry, or every selected mood entry", + recovery: "restorable", + confirm: ["DeleteButton", "SelectionActionBar", "AlertDialog"], + }, + { + file: "components/labs/lab-history-list.tsx", + destroys: "one lab reading", + recovery: "restorable", + confirm: ["DeleteButton"], + }, + { + file: "components/illness/use-illness.ts", + destroys: "an illness episode with its day logs and notes", + triggers: ["components/illness/episode-menu.tsx"], + recovery: "restorable", + confirm: ["AlertDialog"], + }, + { + file: "components/labs/biomarker-manager.tsx", + destroys: "a biomarker AND every lab reading recorded against it", + recovery: "permanent", + confirm: ["DeleteButton"], + }, + { + file: "components/labs/lab-biomarker-detail.tsx", + destroys: "a biomarker AND every lab reading recorded against it", + recovery: "permanent", + confirm: ["DeleteButton"], + }, + { + file: "components/custom-metrics/custom-metric-detail.tsx", + destroys: "a custom metric definition", + recovery: "tombstoned-no-restore", + confirm: ["DeleteButton"], + }, + { + file: "components/custom-metrics/custom-metric-history-list.tsx", + destroys: "one custom-metric reading", + recovery: "permanent", + confirm: ["DeleteButton"], + }, + { + file: "components/records/allergy-manager.tsx", + destroys: "one allergy record", + recovery: "tombstoned-no-restore", + confirm: ["DeleteButton"], + }, + { + file: "components/records/family-history-manager.tsx", + destroys: "one family-history entry", + recovery: "tombstoned-no-restore", + confirm: ["DeleteButton"], + }, + { + file: "hooks/use-measurement-reminders.ts", + destroys: "a checkup or measurement reminder with its schedule", + triggers: ["components/measurement-reminders/vorsorge-section.tsx"], + recovery: "tombstoned-no-restore", + confirm: ["AlertDialog"], + }, + + // ── Cycle ─────────────────────────────────────────────────────────────── + { + file: "components/cycle/use-cycle.ts", + destroys: + "a whole logged cycle day — flow, temperature, symptoms, note — or a custom symptom from the catalogue", + triggers: ["components/cycle/log-day-sheet.tsx"], + recovery: "tombstoned-no-restore", + confirm: ["ConfirmButton", "ConfirmDialog"], + }, + + // ── Medications ───────────────────────────────────────────────────────── + { + file: "components/medications/sections/destructive-zone-section.tsx", + destroys: + "every intake event for a medication, or the medication with its schedules and inventory", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/medications/intake-history-editable.tsx", + destroys: "one intake event, or every selected intake event", + triggers: ["components/medications/intake-history-list-v2.tsx"], + recovery: "tombstoned-no-restore", + confirm: ["AlertDialog"], + }, + { + file: "components/medications/dose-history-ledger.tsx", + destroys: "one intake event", + recovery: "tombstoned-no-restore", + confirm: ["AlertDialog"], + }, + { + file: "components/medications/side-effects-section.tsx", + destroys: "one side-effect log entry", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/medications/sections/inventory-section.tsx", + destroys: "one inventory pack record", + recovery: "permanent", + confirm: ["DeleteButton"], + }, + { + file: "components/medications/scheduling/schedule-history-timeline.tsx", + destroys: "one schedule revision, re-segmenting the dosing history", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/medications/use-medication-intake.ts", + destroys: "an intake event recorded seconds ago", + recovery: "tombstoned-no-restore", + confirm: [], + unconfirmed: [ + { + control: "Undo inside the intake success toast", + reason: + "This IS the undo for a create the person made a moment earlier, reached only from that create's own toast. Confirming an undo would ask someone to confirm a correction, which inverts what the affordance is for.", + }, + ], + }, + + // ── Mood tags ─────────────────────────────────────────────────────────── + { + file: "components/mood/manage/archived-tags-card.tsx", + destroys: + "an archived custom mood tag AND every mood entry's link to it, by cascade", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/mood/manage/tag-groups-card.tsx", + destroys: "a custom mood tag group (its member tags are rehomed, not lost)", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + + // ── Documents ─────────────────────────────────────────────────────────── + { + file: "components/documents/document-detail-sheet.tsx", + destroys: "one document in the vault", + recovery: "restorable", + confirm: [], + unconfirmed: [ + { + control: "Delete in the document detail sheet", + reason: + "Documents chose undo over confirmation across every one of its delete paths, and it is the one domain where that choice is fully backed: the row is soft-deleted, a restore route exists, and the success toast carries the undo that calls it. Layering a dialog on top would gate a reversible act on the surface that already handles reversibility best.", + }, + ], + }, + { + file: "components/documents/documents-view.tsx", + destroys: "every selected document, or one document from a focused card", + triggers: [ + "components/documents/document-bulk-bar.tsx", + "components/documents/document-card.tsx", + ], + recovery: "restorable", + confirm: [], + undetected: + 'The bulk route is POST /api/documents/inbound/bulk with the verb in the body ({ action: "delete" }), so no detector sees it: no DELETE goes out, the path names no act, and nothing is blanked. Registered by hand.', + unconfirmed: [ + { + control: "bulk delete, and the Delete/Backspace key on a focused card", + reason: + "Same undo-not-confirmation choice as the detail sheet, and the bulk path raises one aggregate undo for the whole batch. The keyboard path is the weakest link in that argument, since a keystroke needs no aim and the undo lives only in a toast with no trash view behind it; it is recorded here rather than silently accepted, and closing it means giving documents a recovery surface, not a dialog.", + }, + ], + }, + + // ── Admin ─────────────────────────────────────────────────────────────── + { + file: "components/admin/danger-zone-section.tsx", + destroys: "every user's records on the instance", + recovery: "permanent", + confirm: ["typed-phrase"], + }, + { + file: "components/admin/invite-tokens-section.tsx", + destroys: "an unredeemed invite token — the link stays dead", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + { + file: "components/admin/user-management-section.tsx", + destroys: "another user's sessions, or their stored password hash", + recovery: "permanent", + confirm: ["AlertDialog"], + }, + + // ── Integrations ──────────────────────────────────────────────────────── + ...( + [ + ["withings-card.tsx", "Withings"], + ["fitbit-card.tsx", "Fitbit"], + ["whoop-card.tsx", "WHOOP"], + ["google-health-card.tsx", "Google Health"], + ["nightscout-card.tsx", "Nightscout"], + ["oauth-provider-card.tsx", "the generic OAuth providers"], + ] as const + ).map(([f, name]): DestructiveEntry => ({ + file: `components/settings/integrations/${f}`, + destroys: `the stored ${name} credentials — reconnecting means authorising again`, + recovery: "permanent", + confirm: ["AlertDialog"], + })), +]; + +// ── Detectors ───────────────────────────────────────────────────────────── + +/** D1 — a DELETE request goes out. */ +const D1 = /apiDelete[<(]|method:\s*"DELETE"/; +/** D2 — a route path whose own name is the destructive act. */ +const D2 = + /["`]\/api\/[^"`?\s]*(?:disconnect|revoke|reset|purge|clear|wipe|bulk-delete)[^"`?\s]*/; +/** D3 — a write payload that blanks a named field. */ +const D3 = + /(?:\.mutate|\.mutateAsync|apiPut|apiPatch|apiPost)[<(]?\(\s*(?:"[^"]*",\s*)?\{[^{}]*?\b\w+:\s*(?:""|null)/s; + +/** + * Where a destructive CONTROL can live. Route handlers are the other end of + * the wire, not a control, and `src/lib` holds no JSX — its three matches are + * the `apiDelete` helper itself, an audit-path string constant and an OpenAPI + * path literal. Both exclusions are asserted below so neither can be widened + * into a hiding place without this test noticing. + */ +const CONTROL_ROOTS = ["app/", "components/", "hooks/"]; +const EXCLUDED_PREFIXES = ["app/api/", "generated/", "lib/"]; + +function sourceFiles(): string[] { + return globSync("**/*.{ts,tsx}", { cwd: SRC }) + .map((p) => p.split(sep).join("/")) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(SRC, rel), "utf8"); +} + +function isControlFile(rel: string): boolean { + if (EXCLUDED_PREFIXES.some((p) => rel.startsWith(p))) return false; + return CONTROL_ROOTS.some((p) => rel.startsWith(p)); +} + +function detect(rel: string): string[] { + const src = read(rel); + const tags: string[] = []; + if (D1.test(src)) tags.push("D1"); + if (D2.test(src)) tags.push("D2"); + if (D3.test(src)) tags.push("D3"); + return tags; +} + +let detectedCache: string[] | null = null; +function detectedFiles(): string[] { + detectedCache ??= sourceFiles().filter( + (rel) => isControlFile(rel) && detect(rel).length, + ); + return detectedCache; +} + +const CONFIRM_MARKERS: Record = { + ConfirmButton: / [e.file, e])); + +describe("T1 — every destructive control is registered", () => { + it("the registry names every file the detectors flag", () => { + const missing = detectedFiles().filter((rel) => !byFile.has(rel)); + expect( + missing, + "A destructive request is issued from these files and nothing in the " + + "registry accounts for it. Add an entry naming what it destroys, how " + + "recoverable that is, and what gates it — or, if it genuinely does " + + "not destroy anything, say so under `unconfirmed` with the argument.", + ).toEqual([]); + }); + + it("carries no stale entry", () => { + const stale = REGISTRY.filter( + (e) => !e.undetected && !detectedFiles().includes(e.file), + ).map((e) => e.file); + expect( + stale, + "These entries no longer match any detector. If the control was removed, " + + "delete the entry; if it was rewritten into a shape the detectors " + + "cannot see, that is worth knowing before the entry is dropped.", + ).toEqual([]); + }); + + it("declares each file once", () => { + expect(byFile.size).toBe(REGISTRY.length); + }); + + it("pins the detector exclusions so they cannot quietly widen", () => { + // `src/lib` and the route handlers are excluded because a control cannot + // live there. If that stops being true the exclusion is a hiding place. + expect(EXCLUDED_PREFIXES).toEqual(["app/api/", "generated/", "lib/"]); + expect(CONTROL_ROOTS).toEqual(["app/", "components/", "hooks/"]); + }); +}); + +describe("T2 — every registered control names a real gate", () => { + for (const entry of REGISTRY) { + describe(entry.file, () => { + const files = [entry.file, ...(entry.triggers ?? [])]; + + it("has a confirmation mechanism, or an argument for having none", () => { + expect( + entry.confirm.length > 0 || (entry.unconfirmed?.length ?? 0) > 0, + `${entry.file}: neither a confirmation nor a reason for going without one`, + ).toBe(true); + }); + + for (const kind of entry.confirm) { + it(`the claimed ${kind} is actually present`, () => { + const found = files.some((f) => CONFIRM_MARKERS[kind].test(read(f))); + expect( + found, + `${entry.file} claims ${kind}, but no such control renders in ${files.join(" / ")}`, + ).toBe(true); + }); + } + + for (const skip of entry.unconfirmed ?? []) { + it(`the case for leaving "${skip.control}" unconfirmed is written out`, () => { + // A one-liner is a shrug. The floor is set where a sentence that + // actually argues something has to be written. + expect( + skip.reason.trim().length, + `${entry.file} → ${skip.control}: give the reason, not a note`, + ).toBeGreaterThanOrEqual(120); + }); + } + + it("every named trigger file exists", () => { + for (const f of files) expect(() => read(f)).not.toThrow(); + }); + }); + } +}); + +describe("T3 — a destroying payload never fires straight from a handler", () => { + /** + * The regression that started this: `onClick={() => save.mutate({ aboutMe: + * "", allergies: "" })}`. One tap, two fields blanked, no gate. This walks + * every event-handler attribute in every registered file and fails if a + * destroying request sits inline inside one — which is the shape that skips + * the dialog by construction rather than by oversight. + * + * Indirection through a named handler (`onClick={handleDisconnect}`) is out + * of reach here and is covered instead by T2's requirement that the file + * render a confirmation at all. Named, so nobody reads more into this. + */ + /** + * Elements whose handlers ARE the confirmed path — the dialog's own action + * button, and the primitives that own their dialog. A destroying call inside + * one of these is the point, not the bug. Getting this list wrong in the + * other direction is how a guard starts crying wolf, so it is short and + * every member is a confirmation surface by construction. + */ + const CONFIRMED_ELEMENTS = new Set([ + "AlertDialogAction", + "ConfirmButton", + "ConfirmDialog", + "DeleteButton", + "SelectionActionBar", + ]); + /** `onConfirm` is a confirmation callback wherever it appears. */ + const CONFIRMED_ATTRS = /^onConfirm$/; + + /** + * Walk the JSX opening tags and hand back `[element, handlerAttr, body]` for + * every event handler, so the check can tell a raw `onClick` on a `Button` + * from the `onClick` the dialog's action button carries. + */ + function handlers(src: string): { el: string; attr: string; body: string }[] { + const out: { el: string; attr: string; body: string }[] = []; + const TAG = /<([A-Z][\w.]*|[a-z][\w-]*)(?=[\s/>])/g; + for (const tag of src.matchAll(TAG)) { + const el = tag[1]; + // Scan to the end of THIS opening tag, tracking brace depth and strings + // so an attribute expression containing `>` does not end it early. + let i = tag.index + tag[0].length; + let depth = 0; + let quote: string | null = null; + const attrs: { attr: string; body: string }[] = []; + let pendingAttr: string | null = null; + let bodyStart = 0; + for (; i < src.length; i++) { + const c = src[i]; + if (quote) { + if (c === quote) quote = null; + continue; + } + if (depth === 0 && (c === '"' || c === "'" || c === "`")) { + quote = c; + continue; + } + if (c === "{") { + if (depth === 0) { + const before = src.slice(Math.max(0, i - 60), i); + const m = /\b(on[A-Z]\w*)=$/.exec(before); + pendingAttr = m ? m[1] : null; + bodyStart = i; + } + depth++; + continue; + } + if (c === "}") { + depth--; + if (depth === 0 && pendingAttr) { + attrs.push({ + attr: pendingAttr, + body: src.slice(bodyStart, i + 1), + }); + pendingAttr = null; + } + continue; + } + if (depth === 0 && c === ">") break; + } + for (const a of attrs) out.push({ el, ...a }); + } + return out; + } + + // Unconditional, including the entries that argue for having no dialog. The + // whole registry is clean of this shape today, so there is no exemption to + // carve out and no reason to leave a door open for one. A control that + // genuinely should fire without a gate still has somewhere to go: the + // confirmed elements above, or a named handler the file can be read at. + for (const entry of REGISTRY) { + const files = [entry.file, ...(entry.triggers ?? [])].filter((f) => + f.endsWith(".tsx"), + ); + + for (const file of files) { + it(`${file} — no inline destroying handler`, () => { + const offenders = handlers(read(file)) + .filter( + (h) => + !CONFIRMED_ELEMENTS.has(h.el) && !CONFIRMED_ATTRS.test(h.attr), + ) + .filter((h) => D1.test(h.body) || D3.test(h.body)) + .map((h) => `<${h.el} ${h.attr}=${h.body.slice(0, 80)}`); + expect( + offenders, + `${file}: a destroying request fires directly from an event handler. ` + + `Route it through ConfirmButton / ConfirmDialog / DeleteButton, or ` + + `move it behind a named handler the reviewer can read.`, + ).toEqual([]); + }); + } + } +}); + +describe("T4 — the registry answers the recoverability question", () => { + it("every entry classifies what a person can get back", () => { + for (const e of REGISTRY) { + expect( + ["permanent", "tombstoned-no-restore", "restorable"], + e.file, + ).toContain(e.recovery); + expect(e.destroys.trim().length, e.file).toBeGreaterThan(10); + } + }); + + it("nothing permanent is left unconfirmed without an argument", () => { + const bare = REGISTRY.filter( + (e) => + e.recovery === "permanent" && + e.confirm.length === 0 && + (e.unconfirmed?.length ?? 0) === 0, + ).map((e) => e.file); + expect(bare).toEqual([]); + }); +}); diff --git a/src/components/settings/__tests__/about-me-clear-scope.test.tsx b/src/components/settings/__tests__/about-me-clear-scope.test.tsx new file mode 100644 index 000000000..99088eed4 --- /dev/null +++ b/src/components/settings/__tests__/about-me-clear-scope.test.tsx @@ -0,0 +1,166 @@ +/** + * Settings → AI "About me": the Clear control cannot take the allergy line. + * + * The control used to fire `save.mutate({ aboutMe: "", allergies: "" })` from + * a bare `onClick`. One tap wiped a person's free-text note AND their + * allergies — the entry a clinician reads first, stored in a column that is + * overwritten in place, with no tombstone and no restore route. The toast it + * raised said "About-me note cleared", so the copy had been describing the + * intended behaviour all along; the payload was the thing that was wrong. + * + * Two properties are pinned here, and the first matters more than the second: + * + * 1. SCOPE. The request carries `aboutMe` and no `allergies` key at all. + * The PUT leaves an omitted field untouched by contract, so the payload + * cannot carry the allergy line away even if the dialog were removed + * again. A confirmation someone can delete is not a guarantee. + * 2. GATE. Nothing goes out while the dialog is merely on screen. The + * destructive call reaches the control only as `onConfirm`, so a + * dismissed dialog leaves the request unmade — asserted here by + * rendering, checking no request was issued, and only then invoking the + * captured callback. + * + * `@testing-library/react` is not a dependency (SSR-only component tests), so + * the dialog is not literally clicked. `ConfirmButton` is stubbed to capture + * the props the panel hands it; that the real primitive routes `onConfirm` + * through the dialog action alone is proved in + * `src/components/ui/__tests__/confirm-button.test.tsx`. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { I18nProvider } from "@/lib/i18n/context"; +import { queryKeys } from "@/lib/query-keys"; + +const apiPut = vi.fn(async () => ({ + aboutMe: null, + conditions: null, + allergies: "penicillin", + coachFocus: null, + pendingQuestions: [], + updatedAt: "2026-07-26T00:00:00.000Z", + maxChars: 4000, + fieldMaxChars: 500, +})); + +vi.mock("@/lib/api/api-fetch", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, apiPut: (...args: unknown[]) => apiPut(...args) }; +}); + +/** Captured props of every `` the panel rendered. */ +const confirmButtons: Record[] = []; + +vi.mock("@/components/ui/confirm-button", () => ({ + ConfirmButton: (props: Record) => { + confirmButtons.push(props); + return ( + + ); + }, + ConfirmDialog: () => null, +})); + +const { AboutMeSection } = await import("../about-me-section"); + +const STORED = { + aboutMe: "Shift work, training for a half marathon.", + conditions: null, + allergies: "penicillin", + coachFocus: null, + pendingQuestions: [], + updatedAt: "2026-07-26T00:00:00.000Z", + maxChars: 4000, + fieldMaxChars: 500, +}; + +function render(seed: Partial = {}) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: Infinity } }, + }); + client.setQueryData(queryKeys.coachAboutMe(), { ...STORED, ...seed }); + return renderToStaticMarkup( + + + + + , + ); +} + +beforeEach(() => { + confirmButtons.length = 0; + apiPut.mockClear(); +}); + +describe("the Clear control is scoped to the note", () => { + it("routes through the shared confirmation, not a bare button", () => { + const html = render(); + expect(html).toContain('data-slot="settings-about-me-clear"'); + expect(confirmButtons).toHaveLength(1); + expect(typeof confirmButtons[0].onConfirm).toBe("function"); + }); + + it("hands the destroying call to the dialog, not to the control", () => { + render(); + const props = confirmButtons[0]; + // The callback arrives ONLY as `onConfirm`. Anything that would let the + // trigger act — an `onClick`, an `onSelect` — puts the request one tap + // away again, which is the shape this whole change removed. + expect(typeof props.onConfirm).toBe("function"); + for (const key of Object.keys(props)) { + if (key === "onConfirm") continue; + expect(key, "the trigger must carry no handler of its own").not.toMatch( + /^on[A-Z]/, + ); + } + // And rendering it, on its own, asks the server for nothing. + expect(apiPut).not.toHaveBeenCalled(); + }); + + it("omits the allergies field entirely once confirmed", async () => { + render(); + (confirmButtons[0].onConfirm as () => void)(); + await vi.waitFor(() => expect(apiPut).toHaveBeenCalledTimes(1)); + + const [url, payload] = apiPut.mock.calls[0] as [ + string, + Record, + ]; + expect(url).toBe("/api/coach/about-me"); + expect(payload.aboutMe).toBe(""); + // Not "sent as the stored value" — absent. The route leaves an omitted + // field alone, so absence is what protects the allergy line. + expect(Object.keys(payload)).not.toContain("allergies"); + expect(payload.allergies).toBeUndefined(); + }); + + it("says in its dialog that allergies are not touched", () => { + render(); + expect(confirmButtons[0].body).toBe( + "This removes the free-text note only. Your allergies and intolerances stay exactly as they are.", + ); + }); + + it("hides the control when there is no note, even with allergies stored", () => { + const html = render({ aboutMe: null }); + expect(html).not.toContain('data-slot="settings-about-me-clear"'); + expect(confirmButtons).toHaveLength(0); + }); +}); + +describe("clearing allergies stays possible, in the field itself", () => { + it("renders the allergy textarea, editable, carrying the stored value", () => { + // Removing the field from the Clear payload only removes the untargeted + // route. The targeted one has to still be there, or scoping the control + // would have taken a capability away rather than a footgun. + const html = render(); + const field = html.slice( + html.indexOf('data-testid="settings-about-me-allergies"'), + ); + const openTag = field.slice(0, field.indexOf(">")); + expect(openTag).not.toContain("disabled"); + expect(html).toContain("penicillin"); + }); +}); diff --git a/src/components/settings/__tests__/destructive-controls-confirm.test.tsx b/src/components/settings/__tests__/destructive-controls-confirm.test.tsx index 6d5b0d96c..c3c67147f 100644 --- a/src/components/settings/__tests__/destructive-controls-confirm.test.tsx +++ b/src/components/settings/__tests__/destructive-controls-confirm.test.tsx @@ -3,65 +3,189 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; /** - * Destructive settings controls ask first. + * Destructive controls ask first. * * Signing every other device out, and revoking a trusted device, used to fire * on a single tap. Neither is undoable — the other devices simply have to log * in again, and a revoked trusted device asks for a second factor next time — - * while every other destructive surface in the app confirms. On a shared + * while every other destructive surface in the app confirmed. On a shared * screen a mis-tap is a support call. * - * This reads the sources rather than rendering, because what it pins is - * structural: that these particular controls route through `ConfirmButton` - * and never call a revoke mutation straight from an `onClick`. A render test - * would prove the dialog exists in one arrangement; this proves no control on - * these cards can go back to firing directly. + * The sweep that followed found fourteen more of the same shape, and this file + * grew to hold them. It reads the sources rather than rendering, because what + * it pins is structural: that these particular controls route through + * `ConfirmButton` / `ConfirmDialog` and never call a destroying mutation + * straight from an `onClick`. A render test would prove a dialog exists in one + * arrangement; this proves no control on these surfaces can go back to firing + * directly. + * + * It is one leg of three, and none of them stands alone: + * + * - here: each named control routes through the primitive; + * - `src/components/ui/__tests__/confirm-button.test.tsx`: the primitive + * hands the caller's callback to the dialog action and nowhere else, so a + * dismissal cannot act; + * - `src/__tests__/destructive-control-guard.test.ts`: no NEW destructive + * control can arrive without being written down. + * + * Together those give "each guarded control refuses to act on a dismissed + * dialog" without a click-through the SSR-only test setup cannot perform. */ -const CARDS = [ +function source(rel: string) { + return readFileSync(join(process.cwd(), "src", rel), "utf8"); +} + +/** + * Every control that must open a confirmation before it destroys anything, + * keyed by the `slot` it hands the primitive. A slot that disappears from its + * file means the control was renamed, moved, or unwired — all three are worth + * a red test. + */ +const CONTROLS: { file: string; slots: string[] }[] = [ { - file: "security-sessions-card.tsx", + file: "components/settings/security-sessions-card.tsx", slots: ["revoke-session", "sign-out-everywhere"], }, { - file: "trusted-devices-card.tsx", + file: "components/settings/trusted-devices-card.tsx", slots: ["revoke-trusted-device", "revoke-all-trusted-devices"], }, + // The allergy wipe. Scoped to the note as well as gated — see + // `about-me-clear-scope.test.tsx` for the payload half. + { + file: "components/settings/about-me-section.tsx", + slots: ["settings-about-me-clear"], + }, + { + file: "components/settings/ai/codex-provider-form.tsx", + slots: ["settings-codex-disconnect"], + }, + { + file: "components/admin/central-codex-section.tsx", + slots: ["admin-central-codex-disconnect"], + }, + { + file: "components/admin/ai-server-key-section.tsx", + slots: ["admin-ai-server-key-remove"], + }, + { + file: "components/settings/account-section/avatar-section.tsx", + slots: ["settings-avatar-remove"], + }, + { + file: "components/settings/coach-memory-section.tsx", + slots: ["settings-coach-memory-forget"], + }, + { + file: "components/settings/sources-section.tsx", + slots: ["settings-sources-reset"], + }, + { + file: "components/settings/dashboard-layout-section.tsx", + slots: ["settings-dashboard-layout-reset"], + }, + { + file: "components/insights/insights-edit-mode.tsx", + slots: ["insights-edit-reset"], + }, + { + file: "components/targets/target-edit-sheet.tsx", + slots: ["target-edit-reset"], + }, + { file: "app/coach/plans/page.tsx", slots: ["coach-plan-delete"] }, + { + file: "components/cycle/log-day-sheet.tsx", + slots: ["cycle-day-delete", "cycle-custom-symptom-remove"], + }, + // Two controls, one interpolated slot each: the per-metric reset and the + // override switch, which delete the same stored range. + { + file: "components/settings/thresholds-editor-section.tsx", + slots: [ + "settings-thresholds-reset-all", + "settings-thresholds-reset-${metric}", + "settings-thresholds-override-off-${metric}", + ], + }, ]; -function source(file: string) { - return readFileSync( - join(process.cwd(), "src", "components", "settings", file), - "utf8", - ); -} +describe("destructive controls confirm before firing", () => { + for (const control of CONTROLS) { + describe(control.file, () => { + const src = source(control.file); -describe("destructive session controls confirm before firing", () => { - for (const card of CARDS) { - describe(card.file, () => { - const src = source(card.file); - - it("routes every destructive control through ConfirmButton", () => { - expect(src).toContain("ConfirmButton"); - for (const slot of card.slots) { - expect(src).toContain(`slot="${slot}"`); + it("routes every named control through the shared confirmation", () => { + expect(src).toMatch(/ { - // `onConfirm={() => revoke…}` is the confirmed path and is fine; - // `onClick={() => revoke…}` is the unguarded one this forbids. - const unguarded = src.match(/onClick=\{\(\) =>\s*revoke/g) ?? []; - expect(unguarded).toEqual([]); - }); - - it("gives each control its own confirmation body", () => { - // A shared generic body would let one control's dialog describe - // another's consequence — the reason the copy is per-control. - const bodies = src.match(/body=\{t\("([^"]+)"/g) ?? []; - expect(bodies.length).toBe(card.slots.length); - expect(new Set(bodies).size).toBe(card.slots.length); + it("names the consequence in surface-specific copy", () => { + // A dialog that says "Are you sure?" makes the person guess what they + // are agreeing to, so every body is a key belonging to this surface. + // Two controls MAY share one — the per-metric threshold reset and the + // override switch do, because they perform the identical delete — so + // what is pinned is the provenance of the copy, not its cardinality. + const bodies = [...src.matchAll(/\bbody=\{([\s\S]{0,200}?)\}\n/g)].map( + (m) => m[1], + ); + expect( + bodies.length, + `${control.file}: no confirmation body found`, + ).toBeGreaterThan(0); + const keys = bodies.flatMap((b) => + [...b.matchAll(/t\("([^"]+)"/g)].map((m) => m[1]), + ); + expect(keys.length).toBeGreaterThanOrEqual(bodies.length); + for (const key of keys) { + expect(key, `${control.file}: generic dialog copy`).not.toMatch( + /^common\./, + ); + } }); }); } }); + +describe("the threshold override switch deletes behind the same dialog", () => { + /** + * The switch labelled "Custom range" performs the same irreversible delete + * as the Reset button beside it — flipping it off drops the stored range — + * from a control that reads as a display toggle. It was the one destructive + * path in this sweep that no earlier pass named, precisely because it is not + * shaped like a delete. `ConfirmButton` cannot wrap a switch, which is why + * `ConfirmDialog` exists. + */ + const src = source("components/settings/thresholds-editor-section.tsx"); + const handler = src.slice( + src.indexOf("onCheckedChange={(next)"), + src.indexOf("disabled={busy}", src.indexOf("onCheckedChange={(next)")), + ); + + it("does not reset straight from the switch handler", () => { + expect(handler).toContain("setConfirmSwitchOff(true)"); + expect( + handler, + "flipping the switch must open the dialog, not perform the delete", + ).not.toMatch(/onReset\(\)/); + }); + + it("leaves the switch on when the dialog is dismissed", () => { + // `setOverrideMode(false)` lives inside `onConfirm`, not in the handler, + // so a cancelled dialog leaves both the switch and the stored range alone. + const confirmArm = src.slice( + src.indexOf("", src.indexOf("` / `` — the dismissal contract. + * + * Every destructive control this repo guards routes through one of these two, + * so the property "a dismissed dialog does not act" is worth proving ONCE, + * here, rather than re-proving it per consumer. What makes that compositional + * argument sound is that the callers really do route through the primitive, + * which `src/__tests__/destructive-control-guard.test.ts` asserts separately + * (T2: the claimed mechanism renders; T3: no destroying request fires from a + * bare handler). Neither half means much alone. + * + * The proof is structural because it can be. Both components are hook-free, + * so the test calls them as plain functions and walks the element tree they + * return, which is stronger than asserting on markup: it shows WHERE the + * caller's `onConfirm` is attached, not merely that some button exists. + * + * - the trigger carries no `onClick` at all, so tapping it cannot act; + * - `onConfirm` is reachable from exactly one node, the dialog's action; + * - the cancel node never receives it; + * - and the action lives inside `AlertDialogContent`, which Radix mounts + * only while open — so dismissing unmounts the only path to the callback. + * + * WHAT IS NOT PROVEN HERE. The project runs SSR-only component tests + * (`@testing-library/react` is not a dependency), so nothing below clicks + * Cancel. An earlier draft of this file asserted that a closed dialog renders + * no confirm control in the SSR markup; that passed with `open` hard-coded to + * true, because Radix portals do not render server-side at all — it was + * measuring the renderer, not the component, so it is gone rather than left + * green. That the content mounts only while open is Radix's own contract, and + * the rendered dialog is driven by Playwright (`e2e/share-documents.spec.ts` + * exercises one by role). What IS proven here is the wiring: where the + * caller's callback goes, and where it does not. + */ +import { describe, it, expect, vi } from "vitest"; +import { isValidElement, type ReactElement, type ReactNode } from "react"; + +// `ConfirmBody` is invoked as a plain function below so its element tree can +// be walked; that bypasses React's context machinery, so its one hook is +// stubbed. The provider-backed SSR assertions further down use the real one. +vi.mock("@/lib/i18n/context", () => ({ + useTranslations: () => ({ t: (k: string) => k }), +})); + +const { ConfirmButton, ConfirmDialog } = await import("../confirm-button"); + +/** Display name of an element's type, for readable assertions. */ +function nameOf(el: ReactElement): string { + const t = el.type as unknown; + if (typeof t === "string") return t; + if (typeof t === "function") return (t as { name?: string }).name || "anon"; + const obj = t as { displayName?: string; name?: string }; + return obj?.displayName ?? obj?.name ?? "unknown"; +} + +/** Every element in the tree, paired with its resolved name. */ +function walk(node: ReactNode, out: ReactElement[] = []): ReactElement[] { + if (Array.isArray(node)) { + for (const n of node) walk(n, out); + return out; + } + if (!isValidElement(node)) return out; + out.push(node); + const props = node.props as { children?: ReactNode }; + if (props?.children) walk(props.children, out); + return out; +} + +const COPY = { + title: "Clear the note?", + body: "The note goes; your allergies stay.", + confirmLabel: "Clear note", +}; + +describe(" — the trigger cannot act on its own", () => { + const onConfirm = vi.fn(); + const tree = walk( + ConfirmButton({ + label: "Clear", + slot: "demo-clear", + onConfirm, + ...COPY, + }) as ReactElement, + ); + + function find(name: string) { + return tree.filter((el) => nameOf(el) === name); + } + + it("renders exactly one trigger and one dialog body", () => { + expect(find("AlertDialogTrigger")).toHaveLength(1); + expect(find("ConfirmBody")).toHaveLength(1); + }); + + it("the trigger's button has no click handler", () => { + const [trigger] = find("AlertDialogTrigger"); + const [button] = walk( + (trigger.props as { children?: ReactNode }).children, + ).filter((el) => nameOf(el) === "Button"); + expect(button).toBeDefined(); + const props = button.props as Record; + expect( + props.onClick, + "a trigger that can act is a trigger that fires on a mis-tap", + ).toBeUndefined(); + expect(props["data-slot"]).toBe("demo-clear"); + }); + + it("passes onConfirm to the dialog body and nowhere else", () => { + const carriers = tree.filter( + (el) => (el.props as Record).onConfirm === onConfirm, + ); + expect(carriers.map(nameOf)).toEqual(["ConfirmBody"]); + }); + + it("never invokes the callback during render", () => { + expect(onConfirm).not.toHaveBeenCalled(); + }); +}); + +describe("ConfirmBody — the action is the only path to the callback", () => { + const onConfirm = vi.fn(); + // `ConfirmBody` is internal; reach it through the public component's tree. + const body = walk( + ConfirmButton({ + label: "Clear", + slot: "demo-clear", + onConfirm, + ...COPY, + }) as ReactElement, + ).find((el) => nameOf(el) === "ConfirmBody")!; + + const rendered = walk( + (body.type as (p: unknown) => ReactElement)(body.props), + ); + + it("wires the callback to the action node only", () => { + const carriers = rendered.filter( + (el) => (el.props as Record).onClick === onConfirm, + ); + expect(carriers.map(nameOf)).toEqual(["AlertDialogAction"]); + }); + + it("leaves the cancel node without it", () => { + const cancel = rendered.find((el) => nameOf(el) === "AlertDialogCancel")!; + const props = cancel.props as Record; + expect(props.onClick).toBeUndefined(); + expect(props.onConfirm).toBeUndefined(); + }); + + it("keeps the action inside the dialog content Radix only mounts when open", () => { + const content = rendered.find((el) => nameOf(el) === "AlertDialogContent")!; + const inside = walk((content.props as { children?: ReactNode }).children); + expect(inside.map(nameOf)).toContain("AlertDialogAction"); + }); + + it("labels the action with the caller's verb rather than a bare OK", () => { + const action = rendered.find((el) => nameOf(el) === "AlertDialogAction")!; + const children = (action.props as { children: unknown }).children; + expect(JSON.stringify(children)).toContain(COPY.confirmLabel); + }); +}); + +describe(" — the trigger-less half", () => { + const onConfirm = vi.fn(); + const tree = walk( + ConfirmDialog({ + open: false, + onOpenChange: vi.fn(), + onConfirm, + slot: "demo-switch", + ...COPY, + }) as ReactElement, + ); + + it("renders no trigger, so only the caller's own control can open it", () => { + expect(tree.map(nameOf)).not.toContain("AlertDialogTrigger"); + }); + + it("hands the dialog its open state rather than owning it", () => { + const root = tree.find((el) => nameOf(el) === "AlertDialog")!; + expect((root.props as Record).open).toBe(false); + }); + + it("passes onConfirm to the dialog body and nowhere else", () => { + const carriers = tree.filter( + (el) => (el.props as Record).onConfirm === onConfirm, + ); + expect(carriers.map(nameOf)).toEqual(["ConfirmBody"]); + }); +});