Merge remote-tracking branch 'origin/main' into feat/onboarding-wizard

# Conflicts:
#	docs/api/openapi.yaml
#	src/lib/openapi/error-codes.ts

src/__tests__/domain-write-support-guard.test.ts

@@ -24,6 +24,16 @@
  * domain outside the vocabulary other than `record` fails the run outright,
  * because the table is keyed on the vocabulary and would silently lack it.
  *
+ * What it deliberately does not hold, stated so nobody reads more into a green
+ * run than it earns: the table is EXISTENTIAL per section. It answers "does
+ * any route in this section accept this need", and within a section the answer
+ * varies by verb — `POST /api/encounters` is WRITE while `POST /api/allergies`
+ * beside it is MANAGE, both under `profile`. So this guard proves the table
+ * and the routes agree about the section, never that one control's own verb is
+ * covered. That is the control's job: it asks the hook for its own verb class,
+ * and a control that read "my section is writable, therefore my create is"
+ * would be wrong in three of the four sections marked writable.
+ *
  * Mutation checks, run:
  *   - set `mind.write` to `true` in the table → "agrees with the routes" red;
  *   - change `requireRecordAuth("manage", "mind")` in `mood-entries/route.ts`

src/__tests__/helpers/source-files.ts

@@ -73,3 +73,70 @@ export function walkSourceFiles(
   }
   return files;
 }
+
+/**
+ * A source file with its comments removed, line comments FIRST.
+ *
+ * The order is the whole reason this is shared rather than re-spelled per
+ * guard. Every private copy in this directory strips block comments first,
+ * with one non-greedy `/\*[\s\S]*?\*\//` pass, and that pass cannot tell a
+ * real block opener from a `/*` that happens to sit inside a line comment.
+ * This tree has one: `src/app/mood/page-client.tsx` explains its gate with
+ * "Every `/api/mood-entries/*` route also enforces the gate", and the `/*` in
+ * that path opened a fake block that swallowed the next 2.2 kB of real code —
+ * the module gate three lines below it included. A guard reading that source
+ * was searching a file with a hole in it and reporting a clean sweep.
+ *
+ * So each line is walked once, left to right: whichever opener comes first
+ * wins, a line comment ends the line, and a block comment carries its state to
+ * the next one. Line-comment detection needs `//` at the start of a line or
+ * after whitespace, so a URL inside a string survives.
+ *
+ * Its honest limit: it is a scanner, not a parser. A `/*` inside a string
+ * literal still opens a block, and a guard that needs more than this needs a
+ * real parse.
+ */
+export function stripComments(source: string): string {
+  const lines: string[] = [];
+  let inBlock = false;
+
+  for (const line of source.split("\n")) {
+    let rest = line;
+    let kept = "";
+
+    while (rest.length > 0) {
+      if (inBlock) {
+        const closes = rest.indexOf("*/");
+        if (closes === -1) break;
+        rest = rest.slice(closes + 2);
+        inBlock = false;
+        continue;
+      }
+
+      const lineComment = rest.search(/(^|\s)\/\//);
+      const blockComment = rest.indexOf("/*");
+
+      if (
+        blockComment !== -1 &&
+        (lineComment === -1 || blockComment < lineComment)
+      ) {
+        kept += rest.slice(0, blockComment);
+        rest = rest.slice(blockComment + 2);
+        inBlock = true;
+        continue;
+      }
+
+      if (lineComment !== -1) {
+        kept += rest.slice(0, rest.indexOf("//", lineComment));
+        break;
+      }
+
+      kept += rest;
+      break;
+    }
+
+    lines.push(kept);
+  }
+
+  return lines.join("\n");
+}

src/__tests__/module-gate-record-scoped.test.ts

@@ -0,0 +1,120 @@
+/**
+ * A module gate decides from the RECORD's map, never from "am I a delegate".
+ *
+ * Every surface that paints a module reads `user?.modules?.X`, and a run of
+ * them used to widen that read with `inSharedRecord || …`. The prefix was
+ * right when it was written: `GET /api/auth/me` published the ACTOR's module
+ * map, so a delegate's own preferences would have closed a page in somebody
+ * else's record. Since the map became the record's, masked to what the grant
+ * opens, the same prefix fails open in the other direction — the navigation
+ * correctly drops the entry while a typed URL still renders the page for a
+ * record whose owner switched the module off, and the routes behind these
+ * pages carry no module gate of their own, so the page works.
+ *
+ * A DISCOVERED set rather than a named one. The first version of this guard
+ * froze the six files one sweep happened to find; three more were carrying the
+ * identical expression at the time, and a named list is structurally unable to
+ * say so. So the sweep walks every component under `src/app` and
+ * `src/components`, and the property is about the SHAPE: no boolean `||` may
+ * join `inSharedRecord` to a module read. The window the matcher searches
+ * cannot cross a `;`, a `{` or a `}`, so it sees one expression at a time and
+ * a delegate check elsewhere in the same file is none of its business.
+ *
+ * A structural check rather than a render, because the thing that regressed is
+ * one `||`: reaching these pages in a unit test would mean standing up the
+ * auth query, the router and the shell, and none of that is where the defect
+ * lives.
+ *
+ * Its limit, stated: it holds the two spellings of the OR. A gate that widened
+ * itself some other way — a ternary, a helper returning the delegate flag —
+ * would pass. The non-vacuity floor below is what keeps a broken walker from
+ * reading as a clean sweep.
+ */
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+
+import { describe, expect, it } from "vitest";
+
+import { stripComments, walkSourceFiles } from "./helpers/source-files";
+
+const ROOT = process.cwd();
+/** Floors pinned below the real counts, so a broken walk throws. */
+const ROOTS: ReadonlyArray<[string, number]> = [
+  ["src/app", 100],
+  ["src/components", 700],
+];
+
+/** The module read every gated surface makes. */
+const MODULE_READ = "user?.modules?.";
+
+/**
+ * The two spellings of the widening, in one expression each.
+ *
+ * `[^;{}]*` is the whole point: it cannot cross a statement end or a block
+ * boundary, so a file that reads the module map in one place and asks about
+ * the delegate in another does not match.
+ */
+const WIDENED = [
+  /inSharedRecord\s*\|\|[^;{}]*user\?\.modules\?\./,
+  /user\?\.modules\?\.[^;{}]*\|\|\s*inSharedRecord/,
+];
+
+/** Every `.tsx` under the roots, with its comment-stripped source. */
+function componentSources(): Array<{ file: string; source: string }> {
+  const out: Array<{ file: string; source: string }> = [];
+  for (const [root, floor] of ROOTS) {
+    const files = walkSourceFiles(join(ROOT, root), {
+      floor,
+      extensions: [".tsx"],
+    });
+    for (const rel of files) {
+      if (rel.includes("__tests__/")) continue;
+      out.push({
+        file: `${root}/${rel}`,
+        source: stripComments(readFileSync(join(ROOT, root, rel), "utf8")),
+      });
+    }
+  }
+  return out;
+}
+
+describe("a module gate reads the record's map alone", () => {
+  const sources = componentSources();
+  const gated = sources.filter(({ source }) => source.includes(MODULE_READ));
+
+  it("discovers a real set of module-gated surfaces", () => {
+    // The empty-match trap: a walker that stopped walking, or a spelling of
+    // the module read that moved, would make every assertion below vacuous.
+    expect(sources.length, "no component was walked at all").toBeGreaterThan(
+      500,
+    );
+    expect(
+      gated.length,
+      "no surface was found reading the module map",
+    ).toBeGreaterThan(20);
+    // Spot checks, so a sweep that found twenty of the wrong files still
+    // fails: these are module-gated pages by construction.
+    const files = new Set(gated.map((entry) => entry.file));
+    for (const file of [
+      "src/app/vaccinations/page.tsx",
+      "src/app/labs/page.tsx",
+      "src/app/mood/page-client.tsx",
+      "src/components/documents/documents-view.tsx",
+    ]) {
+      expect(files, `${file} was not discovered`).toContain(file);
+    }
+  });
+
+  it("finds no gate widened by the delegate flag", () => {
+    const widened = gated
+      .filter(({ source }) => WIDENED.some((pattern) => pattern.test(source)))
+      .map(({ file }) => file);
+
+    expect(
+      widened,
+      `these surfaces widen a module gate for a delegate; the record's masked map already answers for both cases:\n${widened
+        .map((file) => `  ❌ ${file}`)
+        .join("\n")}`,
+    ).toEqual([]);
+  });
+});

src/__tests__/openapi-sharing-denial.test.ts

@@ -45,16 +45,16 @@
  * - Comments are stripped before matching. `.../guardians/route.ts` names both
  *   fence helpers in a docblock explaining why it uses neither, and counting
  *   prose would classify an actor surface as delegable.
- * - It covers the PUBLISHED surface, which is not the whole delegable surface.
- *   116 route modules resolve the fence; 90 of them are in the OpenAPI document
- *   and 26 have never been (mood entries, the record-settings tree, the
- *   medication intake tree, several insight reads). That gap predates this
- *   release and is not closed here — "frozen" means the published contract is
- *   complete and consistent, not that every delegable route is described. What
- *   this guard does guarantee is that one of those 26 cannot be published
- *   without its refusal, because publishing it puts it in scope on the next run.
+ * - It covered the PUBLISHED surface and not the whole delegable one. That was
+ *   the honest limit at v1.37.0, when 116 route modules resolved the fence and
+ *   26 of them were in no OpenAPI document at all. The gap is closed on this
+ *   tree, so the limit is now an assertion rather than a caveat: the last leg
+ *   below recomputes the fenced modules from the route sources and requires
+ *   every one of them to be published. Two counts is one too many, so it names
+ *   no number — a hard-coded figure is what turned the paragraph it replaces
+ *   into a false claim about the contract.
  */
-import { readFileSync, existsSync } from "node:fs";
+import { readdirSync, readFileSync, existsSync } from "node:fs";
 import { join } from "node:path";
 
 import { describe, expect, it } from "vitest";
@@ -63,6 +63,8 @@ import { openApiPaths } from "@/lib/openapi/routes";
 import { SHARING_ACCESS_DENIED_DESCRIPTION } from "@/lib/openapi/routes/shared";
 import { RETIRED_ROUTES } from "@/lib/http/retired-routes";
 
+import { stripComments } from "./helpers/source-files";
+
 /** Published paths that deliberately have no handler behind them. */
 const retiredPaths = new Set(RETIRED_ROUTES.map((route) => route.path));
 
@@ -80,8 +82,40 @@ interface PublishedOperation {
   handlerSource: string;
 }
 
-function stripComments(text: string): string {
-  return text.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|\s)\/\/.*$/gm, "$1");
+/**
+ * Every `route.ts` under `src/app/api` whose handlers resolve the record fence.
+ *
+ * Comments stripped first, for the reason the operation matcher strips them:
+ * one module names both fence helpers in a docblock explaining why it uses
+ * neither.
+ */
+function fencedRouteModules(): string[] {
+  const out: string[] = [];
+  const walk = (dir: string) => {
+    for (const entry of readdirSync(dir, { withFileTypes: true })) {
+      const full = join(dir, entry.name);
+      if (entry.isDirectory()) {
+        walk(full);
+        continue;
+      }
+      if (entry.name !== "route.ts") continue;
+      const source = stripComments(readFileSync(full, "utf8"));
+      if (/require(RecordAuth|GuardianAuth)\s*\(/.test(source)) {
+        out.push(full);
+      }
+    }
+  };
+  walk(join(ROOT, "src/app/api"));
+  return out;
+}
+
+/** `…/src/app/api/labs/[id]/route.ts` → `/api/labs/{id}`. */
+function apiPathFor(moduleFile: string): string {
+  return moduleFile
+    .slice(join(ROOT, "src/app").length)
+    .replaceAll("\\", "/")
+    .replace(/\/route\.ts$/, "")
+    .replace(/\[([^\]]+)\]/g, "{$1}");
 }
 
 function routeModulePath(apiPath: string): string {
@@ -334,6 +368,33 @@ describe("the sharing refusal every delegable path publishes", () => {
     }
   });
 
+  it("publishes every module that resolves the fence", () => {
+    // The reverse direction of the leg above, and the one the docblock used to
+    // apologise for: that one holds every PUBLISHED fenced operation to the
+    // refusal, and says nothing about a fenced module nobody published. A
+    // module missing from the document is a delegable surface a generated
+    // client cannot see and a reader of the contract cannot find, which is
+    // exactly how the mood-entry tree and the record-settings tree stayed
+    // invisible for a release.
+    const modules = fencedRouteModules();
+    expect(
+      modules.length,
+      "no fenced route module was discovered — the matcher found nothing",
+    ).toBeGreaterThan(100);
+
+    const published = new Set(Object.keys(openApiPaths));
+    const missing = modules
+      .map((rel) => apiPathFor(rel))
+      .filter((apiPath) => !published.has(apiPath));
+
+    expect(
+      missing,
+      `these route modules resolve the record fence and are in no published path:\n${missing
+        .map((apiPath) => `  ❌ ${apiPath}`)
+        .join("\n")}`,
+    ).toEqual([]);
+  });
+
   it("carries the refusal into the generated document", () => {
     const spec = readFileSync(join(ROOT, "docs/api/openapi.yaml"), "utf8");
     expect(spec).toContain(SHARING_ACCESS_DENIED_DESCRIPTION.slice(0, 60));

src/__tests__/sharing-legacy-client-contract.test.ts

@@ -161,12 +161,11 @@ describe("what the canonical readers make of the same payload", () => {
       expect(capabilities.sections).toEqual(fixture.sections);
       expect(capabilities.recordKind).toBe(fixture.recordKind);
       expect(capabilities.inSharedRecord).toBe(true);
-      // Resolved server-side and rendered, never re-derived: `canAdd` follows
-      // `canWrite`, and `canManage` follows the published `manageableDomains`
-      // list rather than the level — a delegate's management reach is the
-      // server's answer, section by section.
+      // Resolved server-side and rendered, never re-derived: `canWrite` is the
+      // published level, and both per-section answers follow the published
+      // lists rather than the level — a delegate's reach is the server's
+      // answer, section by section.
       expect(capabilities.canWrite).toBe(fixture.canWrite);
-      expect(capabilities.canAdd).toBe(fixture.canWrite);
       const manageable = active?.manageableDomains ?? [];
       expect(capabilities.canManage).toBe(manageable.length > 0);
       expect(manageable.length > 0).toBe(fixture.level === "manage");