PORTAL-SHOWMODEL — the public 3D viewer shipped, and nothing could mint a token that reached it

apps/web/src/api/clientPortal.ts

@@ -14,20 +14,27 @@
  *
  *  `services/api/src/aec_api/routers/client_portal.py` groups the same set — 8 of its 9 routes are
  *  exactly these methods, checked rather than assumed. The ninth, `GET /shared/{token}/model.frag`,
- *  has no client method BY DESIGN: it is fetched by the server-rendered share page, not by this SPA.
+ *  still has no client method: it is a whole-file response a recipient opens by URL, not JSON this
+ *  SPA parses. What it DOES need from here is the mint-time opt-in that makes it answer at all.
  *
- *  ### A capability this file cannot currently reach
+ *  ### The opt-in this file could not reach — fixed 2026-09-04
  *
  *  That ninth route serves geometry only to a token minted with `show_model`, which the backend
  *  treats as an opt-in independent of `show_payments` — *"a token may carry payments, or geometry,
- *  or neither, and granting one never implies the other"*. **`createShareToken` below does not send
- *  it**, and the row type `shareTokens` returns has no `show_model` field, so every token this
- *  product mints has it false and that route always 404s for them. The public 3D viewer is dark from
- *  the UI's side.
+ *  or neither, and granting one never implies the other"*. **`createShareToken` did not send it**,
+ *  and the row type `shareTokens` returns had no `show_model` field, so every token this product
+ *  minted had it false and that route 404'd for all of them.
  *
- *  Not fixed here on purpose: this slice's claim is that no behaviour changed, and adding a
- *  parameter would falsify it. Recorded so the next reader of this file finds it at the method
- *  rather than in a backlog.
+ *  Both halves are closed below, and they are separate defects with separate consequences. The
+ *  missing PARAMETER made the capability unreachable. The missing ROW FIELD made it unauditable —
+ *  `_public_row` has always returned `show_model`, the wire carried it, and the type simply dropped
+ *  it, so an owner could not have told a geometry link from a digest link even once one existed.
+ *  R22-PUBLIC-VIEWER's shipped record claims *"the owner's token list shows which links carry
+ *  geometry"*; until this change that was true of the JSON and false of the product.
+ *
+ *  The two flags are passed as separate arguments, never as one "share more" level, because the
+ *  backend's rule is that granting one must never imply the other. A single enum or an ordered
+ *  level would make that rule unexpressible at the call site.
  *
  *  A mixin, so every call site resolves unchanged; `api/surface.test.ts` is what proves it.
  */
@@ -37,17 +44,23 @@ type Ctor<T> = new (...args: any[]) => T;
 
 export function withClientPortal<TBase extends Ctor<HttpCore>>(Base: TBase) {
   return class ClientPortal extends Base {
-  /** `showPayments` is the explicit opt-in for THIS token's digest to carry the payment schedule. */
-  createShareToken(pid: string, label?: string, showPayments?: boolean) {
-    return this.json<{ token: string; label: string | null; share_path: string; revoked: boolean }>(
+  /** Mint a read-only share token. `showPayments` and `showModel` are two INDEPENDENT opt-ins, and
+   * the backend is explicit that granting one never implies the other: `showPayments` lets this
+   * token's digest carry the owner-invoice payment schedule, `showModel` lets it fetch the project's
+   * geometry fragment (`GET /shared/{token}/model.frag` — shapes and placements, never the source
+   * IFC). Both default to false; a token already in somebody's inbox is never widened. */
+  createShareToken(pid: string, label?: string, showPayments?: boolean, showModel?: boolean) {
+    return this.json<{ token: string; label: string | null; share_path: string; revoked: boolean;
+      show_payments: boolean; show_model: boolean }>(
       `/projects/${pid}/share-tokens`,
-      { method: "POST", body: JSON.stringify({ label: label ?? "", show_payments: !!showPayments }) });
+      { method: "POST", body: JSON.stringify({ label: label ?? "", show_payments: !!showPayments,
+        show_model: !!showModel }) });
   }
   /** CLIENT-PORTAL — read-only share tokens for a project-readiness digest. */
   shareTokens(pid: string) {
     type Tok = { token: string; label: string | null; revoked: boolean; created_at: string | null;
       created_by: string | null; view_count: number; last_viewed_at: string | null; share_path: string;
-      show_payments: boolean };
+      show_payments: boolean; show_model: boolean };
     return this.json<{ tokens: Tok[] }>(`/projects/${pid}/share-tokens`);
   }
   revokeShareToken(pid: string, token: string) {

apps/web/src/api/shareTokenGrants.test.ts

@@ -0,0 +1,104 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { ApiClient } from "./client";
+
+/**
+ * A share token's two grants are INDEPENDENT, and the client could only ever send one of them.
+ *
+ * THE DEFECT. `show_model` is the per-token opt-in that lets a share link fetch the project's
+ * geometry — `GET /shared/{token}/model.frag`, gated in `client_portal.model_fragment` on
+ * `not getattr(row, "show_model", False)`. The backend has supported it end to end since the column
+ * shipped: the route reads `body.get("show_model")`, `_public_row` returns it, and
+ * `services/api/test_shared_model.py` already proves the 200-vs-404 pair and that `show_payments`
+ * does not imply it. **`createShareToken` sent only `label` and `show_payments`**, so every token
+ * this product minted had the flag false and that route 404'd for all of them.
+ *
+ * WHY NOTHING CAUGHT IT. Both sides were correct in isolation, which is the whole shape of the bug.
+ * The backend test mints its own tokens with `json={"show_model": True}` — a body the product never
+ * produces — so it passes over a client that cannot ask for the thing it tests. Nothing red, nothing
+ * logged, and a 404 from a route nobody had a working link to is indistinguishable from a project
+ * with no published fragment, which is deliberately the same response.
+ *
+ * The assertions are about the ENCODED BODY, not a mock's arguments, for the reason
+ * `publishBody.test.ts` gives: what reaches the server is the only thing the route can be wrong
+ * about. A test that asserted `createShareToken` was *called* with `true` would have passed against
+ * a method that accepted the argument and dropped it — which is one keystroke from the bug.
+ */
+
+function captureBody(response: unknown = { token: "t", label: null, share_path: "/shared/t/digest",
+  revoked: false, show_payments: false, show_model: false }) {
+  const seen: { url: string; body: string | null }[] = [];
+  vi.stubGlobal("fetch", vi.fn(async (url: string, init?: RequestInit) => {
+    seen.push({ url: String(url), body: (init?.body as string) ?? null });
+    return new Response(JSON.stringify(response), {
+      status: 200, headers: { "content-type": "application/json" },
+    });
+  }));
+  return seen;
+}
+
+const bodyOf = (raw: string | null) => JSON.parse(raw ?? "{}") as Record<string, unknown>;
+
+describe("POST /projects/{pid}/share-tokens — the geometry opt-in reaches the wire", () => {
+  it("sends show_model:true when the caller asks for geometry", async () => {
+    const seen = captureBody();
+    await new ApiClient("http://x").createShareToken("p1", "Owner review", false, true);
+    expect(seen).toHaveLength(1);
+    expect(seen[0]!.url).toContain("/projects/p1/share-tokens");
+    expect(bodyOf(seen[0]!.body)).toEqual(
+      { label: "Owner review", show_payments: false, show_model: true });
+    vi.unstubAllGlobals();
+  });
+
+  it("ALWAYS sends the key, never omits it — an absent key is not the same request", async () => {
+    // `bool(body.get("show_model"))` treats absent and false alike TODAY. This asserts the key is
+    // present anyway: the client must state the grant it is asking for rather than rely on a
+    // server-side default, because that default is the one thing a future backend could change
+    // without touching this file.
+    const seen = captureBody();
+    await new ApiClient("http://x").createShareToken("p1");
+    expect(bodyOf(seen[0]!.body)).toHaveProperty("show_model", false);
+    vi.unstubAllGlobals();
+  });
+
+  it("the two grants are independent in BOTH directions", async () => {
+    // The backend docstring is explicit: "a token may carry payments, or geometry, or neither, and
+    // granting one never implies the other." Asserting only the both-true case would pass on a
+    // client that had collapsed them into one "share more" flag, which is exactly the design error
+    // the wording exists to prevent. So all four corners are checked.
+    const seen = captureBody();
+    const api = new ApiClient("http://x");
+    await api.createShareToken("p1", "a", false, false);
+    await api.createShareToken("p1", "b", true, false);
+    await api.createShareToken("p1", "c", false, true);
+    await api.createShareToken("p1", "d", true, true);
+    expect(seen.map((s) => {
+      const b = bodyOf(s.body);
+      return [b.show_payments, b.show_model];
+    })).toEqual([[false, false], [true, false], [false, true], [true, true]]);
+    vi.unstubAllGlobals();
+  });
+
+  it("the token row carries show_model, so an owner can audit which links grant geometry", async () => {
+    // The SECOND half of the defect, and a separate one: `_public_row` has always returned
+    // `show_model` and the row type dropped it, so the value was on the wire and unreadable.
+    //
+    // STATE THE GRADE. This assertion is NOT what guards that half, and pretending otherwise would
+    // be the more dangerous outcome — a reader would trust vitest to catch a regression it cannot
+    // see. Measured by deleting `show_model` from the `Tok` type: vitest stayed 4/4 GREEN (the
+    // runtime reads whatever the response object holds; a type is not there at runtime), while
+    // `tsc --noEmit` went red in THREE places — this line and the two `masterBuilder.ts` reads.
+    // The typecheck is the gate; this line exists so the field is exercised by a caller at all.
+    captureBody({ tokens: [
+      { token: "aaaaaaaaaa", label: "digest only", revoked: false, created_at: null, created_by: null,
+        view_count: 0, last_viewed_at: null, share_path: "/shared/aaaaaaaaaa/digest",
+        show_payments: false, show_model: false },
+      { token: "bbbbbbbbbb", label: "with model", revoked: false, created_at: null, created_by: null,
+        view_count: 3, last_viewed_at: null, share_path: "/shared/bbbbbbbbbb/digest",
+        show_payments: false, show_model: true },
+    ] });
+    const { tokens } = await new ApiClient("http://x").shareTokens("p1");
+    expect(tokens.map((t) => t.show_model)).toEqual([false, true]);
+    vi.unstubAllGlobals();
+  });
+});

apps/web/src/portal/panels/masterBuilder.test.ts

@@ -0,0 +1,130 @@
+import { describe, expect, it, vi } from "vitest";
+
+import type { PanelContext } from "../panelContext";
+import { renderMasterBuilder } from "./masterBuilder";
+
+/**
+ * The share-link form is the ONLY place this product mints a share token, so it is the only place
+ * the geometry opt-in can be offered — and the only place it can be silently dropped.
+ *
+ * `api/shareTokenGrants.test.ts` proves the client puts `show_model` on the wire. That is a claim
+ * about the METHOD. This is the claim about the USE: a checkbox that exists, renders and is never
+ * read would satisfy the first test completely and leave the capability exactly as unreachable as
+ * it was. This repo has been bitten by that distinction before — a gate that saw a value
+ * destructured and stayed green after the calls to it were deleted — so the assertions here drive
+ * the real DOM: tick the box, click the button, and read what the client was asked for.
+ */
+
+const BRIEF = {
+  project: "Test project", readiness_pct: 50, ready_steps: 4, gap_steps: 4, step_count: 8,
+  grounded_in_place: true, jurisdiction: "Miami-Dade, FL", reframe_prompt: "What is this place for?",
+  place_grounding: { code_family: "IBC", coordinates: null, hemisphere: null, climate_band: null,
+    hazards_to_verify: [] },
+  steps: [], disclaimer: "Not a substitute for licensed judgment.",
+};
+
+const flush = async () => { for (let i = 0; i < 6; i++) await Promise.resolve(); };
+
+function ctx(over: Record<string, unknown> = {}) {
+  const api = {
+    masterBuilderBrief: vi.fn().mockResolvedValue(BRIEF),
+    masterBuilderBriefMdUrl: () => "/md",
+    sharedPageUrl: (t: string) => `/shared/${t}`,
+    shareTokens: vi.fn().mockResolvedValue({ tokens: [] }),
+    createShareToken: vi.fn().mockResolvedValue({ token: "new" }),
+    revokeShareToken: vi.fn().mockResolvedValue({ revoked: true }),
+    ...over,
+  };
+  const c: PanelContext = {
+    root: document.createElement("div"),
+    host: { projectId: () => "p1", api } as unknown as PanelContext["host"],
+    mods: [], activeKey: "__mb__",
+    bar: (title) => { const b = document.createElement("div"); b.textContent = title; return b; },
+    buildNav: () => undefined, renderHome: async () => undefined, openModule: async () => undefined,
+    navigate: () => undefined, hasDest: () => true,
+  };
+  return { c, api };
+}
+
+/** The two opt-in checkboxes, found by their labels rather than by DOM order. */
+function boxes(root: HTMLElement) {
+  const find = (needle: string) => {
+    for (const l of Array.from(root.querySelectorAll("label"))) {
+      if ((l.textContent ?? "").includes(needle)) return l.querySelector("input") as HTMLInputElement;
+    }
+    throw new Error(`no opt-in labelled ${needle} — found: `
+      + Array.from(root.querySelectorAll("label")).map((l) => l.textContent).join(" | "));
+  };
+  return { pay: find("payment schedule"), model: find("3D model") };
+}
+
+const createBtn = (root: HTMLElement) =>
+  Array.from(root.querySelectorAll("button")).find((b) => b.textContent?.includes("Create link"))!;
+
+describe("R22-PUBLIC-VIEWER — the geometry opt-in is offered, and it is READ", () => {
+  it("offers a 3D-model opt-in beside the payments one, both unchecked", async () => {
+    const { c } = ctx();
+    await renderMasterBuilder(c); await flush();
+    const b = boxes(c.root as HTMLElement);
+    expect(b.pay.checked).toBe(false);
+    expect(b.model.checked).toBe(false);
+  });
+
+  it("passes the ticked box through to createShareToken — the wiring, not the widget", async () => {
+    const { c, api } = ctx();
+    await renderMasterBuilder(c); await flush();
+    boxes(c.root as HTMLElement).model.checked = true;
+    createBtn(c.root as HTMLElement).click();
+    await flush();
+    expect(api.createShareToken).toHaveBeenCalledTimes(1);
+    // (pid, label, showPayments, showModel) — the 4th argument is the one that did not exist.
+    expect(api.createShareToken.mock.calls[0]).toEqual(["p1", undefined, false, true]);
+  });
+
+  it("grants stay independent: payments alone never carries geometry", async () => {
+    // The backend refuses to let one imply the other. A UI that quietly sent both when either was
+    // ticked would be a widening the backend's rule exists to forbid, and no backend test could see
+    // it — the request would look like a deliberate double opt-in.
+    const { c, api } = ctx();
+    await renderMasterBuilder(c); await flush();
+    boxes(c.root as HTMLElement).pay.checked = true;
+    createBtn(c.root as HTMLElement).click();
+    await flush();
+    expect(api.createShareToken.mock.calls[0]).toEqual(["p1", undefined, true, false]);
+  });
+
+  it("resets both boxes after a mint, so a grant never carries to the next link", async () => {
+    const { c } = ctx();
+    await renderMasterBuilder(c); await flush();
+    const b = boxes(c.root as HTMLElement);
+    b.model.checked = true; b.pay.checked = true;
+    createBtn(c.root as HTMLElement).click();
+    await flush();
+    expect(b.model.checked).toBe(false);
+    expect(b.pay.checked).toBe(false);
+  });
+
+  it("marks which live links grant geometry, so the opt-in can be audited after minting", async () => {
+    const tok = (over: Record<string, unknown>) => ({
+      token: "0123456789abcdef", label: null, revoked: false, created_at: null, created_by: null,
+      view_count: 0, last_viewed_at: null, share_path: "/s", show_payments: false, show_model: false,
+      ...over,
+    });
+    const { c } = ctx({ shareTokens: vi.fn().mockResolvedValue({ tokens: [
+      tok({ token: "aaaaaaaaaaaaaaaa", label: "digest only" }),
+      tok({ token: "bbbbbbbbbbbbbbbb", label: "with model", show_model: true }),
+    ] }) });
+    await renderMasterBuilder(c); await flush();
+    const links = Array.from((c.root as HTMLElement).querySelectorAll("a"))
+      .filter((a) => (a.textContent ?? "").includes("…"));
+    expect(links).toHaveLength(2);
+    const plain = links.find((a) => a.textContent!.includes("digest only"))!;
+    const withModel = links.find((a) => a.textContent!.includes("with model"))!;
+    expect(withModel.textContent).toContain("🧊");
+    expect(withModel.title).toContain("3D model");
+    // The negative half. Without it the marker could be unconditional and every assertion above
+    // would still pass, which would make the audit trail confidently wrong rather than absent.
+    expect(plain.textContent).not.toContain("🧊");
+    expect(plain.title).not.toContain("3D model");
+  });
+});