From 76817f339adb87ad1f250e9d866dc959bfcb9948 Mon Sep 17 00:00:00 2001 From: Luke Brevoort Date: Thu, 30 Jul 2026 11:06:28 -0700 Subject: [PATCH 1/2] fix(whiteboard): stop malformed agent-drawn elements from crashing the board MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent drew a flow diagram via the whiteboard_update MCP tool and emitted arrows with no `points` array. Excalidraw's restoreElements() calls isInvisiblySmallElement() — which reads `element.points.length` unguarded — before restoreElement() applies its own [[0,0],[width,height]] fallback, so the scene threw "Cannot read properties of undefined (reading 'length')" and took down the whole route. Add sanitizeElements() and apply it in loadWhiteboard and saveWhiteboard, so malformed geometry can neither be persisted nor served. It applies the same fallback Excalidraw itself would, just early enough to matter. Sanitizing on read means boards already holding bad elements render immediately and heal on their next write, with no data migration. Also wrap the whiteboard tab in an error boundary so an unrenderable scene degrades to a message instead of blanking the app, and correct the MCP tool cheat sheet, which listed `points` as optional and told agents not to over-validate. Co-Authored-By: Claude Fable 5 --- .../server/src/shared/mcp/whiteboard-tools.ts | 5 +- apps/server/src/shared/whiteboard-store.ts | 12 +- apps/server/src/shared/whiteboard.ts | 58 ++++++ apps/server/test/whiteboard.test.ts | 176 +++++++++++++++++- .../src/components/app/whiteboard-pane.tsx | 52 +++++- e2e/whiteboard.spec.ts | 48 +++++ 6 files changed, 336 insertions(+), 15 deletions(-) diff --git a/apps/server/src/shared/mcp/whiteboard-tools.ts b/apps/server/src/shared/mcp/whiteboard-tools.ts index 36e4e6ee..dccd6161 100644 --- a/apps/server/src/shared/mcp/whiteboard-tools.ts +++ b/apps/server/src/shared/mcp/whiteboard-tools.ts @@ -145,7 +145,7 @@ Text font families: 1=Virgil (handwritten), 3=Cascadia (monospace), 5=Excalifont } \`\`\` -**Points:** Array of [x, y] offsets relative to the element's x, y. First point is always [0, 0]. Add intermediate points for bends. +**Points (required):** Array of at least two [x, y] offsets relative to the element's x, y. First point is always [0, 0]. Add intermediate points for bends. Arrows, lines, and freedraw elements must always carry a \`points\` array — omit it and the element cannot be rendered. **Arrowheads:** \`startArrowhead\` and \`endArrowhead\` can be: null, "arrow", "bar", "dot", "triangle", "diamond". @@ -191,8 +191,9 @@ Children are assigned to frames by setting their \`frameId\` to the frame's id. ### Important notes -- The editor auto-heals many issues (null fields, missing indices). Don't over-validate. +- The editor auto-heals many cosmetic issues (null fields, missing indices). Don't over-validate those. - Always provide \`id\`, \`type\`, \`x\`, \`y\` at minimum. Width and height default to 0 if omitted. +- For \`arrow\`, \`line\`, and \`freedraw\`, \`points\` is mandatory — it is not auto-healed. - Use readable, descriptive ids — you'll reference them in bindings and future updates. - Elements are merged by id: sending an element with an existing id replaces it entirely. `; diff --git a/apps/server/src/shared/whiteboard-store.ts b/apps/server/src/shared/whiteboard-store.ts index 742bb70d..a127d1f0 100644 --- a/apps/server/src/shared/whiteboard-store.ts +++ b/apps/server/src/shared/whiteboard-store.ts @@ -1,5 +1,7 @@ import type { Pool } from "pg"; +import { sanitizeElements } from "./whiteboard.js"; + export const WHITEBOARD_SNAPSHOT_FILENAME = "whiteboard.png"; export const MAX_ELEMENTS = 20_000; @@ -21,7 +23,12 @@ export async function loadWhiteboard( "SELECT scene, version, updated_by, updated_at FROM whiteboards WHERE agent_id = $1", [agentId] ); - return result.rows[0] ?? null; + const row = result.rows[0]; + if (!row) return null; + // Sanitize on read too, so boards already holding malformed elements render + // instead of crashing, and heal on their next write. + const elements = Array.isArray(row.scene?.elements) ? row.scene.elements : []; + return { ...row, scene: { elements: sanitizeElements(elements) } }; } export function isValidScene(scene: unknown): scene is { elements: unknown[] } { @@ -40,6 +47,7 @@ export async function saveWhiteboard( baseVersion: number, updatedBy: "user" | "agent" ): Promise<{ version: number } | null> { + const clean = { elements: sanitizeElements(scene.elements) }; const result = await pool.query<{ version: string }>( `INSERT INTO whiteboards (agent_id, scene, version, updated_by) VALUES ($1, $2::jsonb, 1, $3) @@ -50,7 +58,7 @@ export async function saveWhiteboard( updated_at = NOW() WHERE whiteboards.version = $4 RETURNING version`, - [agentId, JSON.stringify(scene), updatedBy, baseVersion] + [agentId, JSON.stringify(clean), updatedBy, baseVersion] ); if (result.rows.length === 0) { return null; diff --git a/apps/server/src/shared/whiteboard.ts b/apps/server/src/shared/whiteboard.ts index c59a9bc5..4cc4785b 100644 --- a/apps/server/src/shared/whiteboard.ts +++ b/apps/server/src/shared/whiteboard.ts @@ -35,6 +35,64 @@ export type WhiteboardUpdateResult = { elements: SimplifiedElement[]; }; +// Excalidraw's restoreElements() calls isInvisiblySmallElement() — which reads +// `element.points.length` unguarded — before restoreElement() applies its own +// defaults. A points-less arrow therefore throws and takes down the whole view, +// so we normalize these fields before the data ever reaches the editor. +const POINTS_REQUIRED_TYPES = new Set(["arrow", "line", "draw", "freedraw"]); + +function finite(value: unknown): number { + return typeof value === "number" && Number.isFinite(value) ? value : 0; +} + +function isPoint(value: unknown): boolean { + return ( + Array.isArray(value) && + value.length >= 2 && + Number.isFinite(value[0]) && + Number.isFinite(value[1]) + ); +} + +export function sanitizeElement(raw: unknown): RawElement | null { + if (typeof raw !== "object" || raw === null) return null; + const el = raw as RawElement; + if (typeof el.id !== "string" || typeof el.type !== "string") return null; + + const width = finite(el.width); + const height = finite(el.height); + const out: RawElement = { + ...el, + x: finite(el.x), + y: finite(el.y), + width, + height, + }; + + if (POINTS_REQUIRED_TYPES.has(el.type)) { + const points = Array.isArray(el.points) ? el.points.filter(isPoint) : []; + // Same fallback restoreElement() would apply, just early enough to matter. + out.points = + points.length >= 2 + ? points + : [ + [0, 0], + [width, height], + ]; + } + + return out; +} + +export function sanitizeElements(elements: unknown[]): unknown[] { + const out: RawElement[] = []; + for (const raw of elements) { + const el = sanitizeElement(raw); + if (el) out.push(el); + } + return out; +} + function num(value: unknown): number { return typeof value === "number" && Number.isFinite(value) ? Math.round(value) diff --git a/apps/server/test/whiteboard.test.ts b/apps/server/test/whiteboard.test.ts index c8cc3e04..01482737 100644 --- a/apps/server/test/whiteboard.test.ts +++ b/apps/server/test/whiteboard.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from "vitest"; -import { simplifyElements } from "../src/shared/whiteboard.js"; +import { + sanitizeElements, + simplifyElements, +} from "../src/shared/whiteboard.js"; import { isValidScene, MAX_ELEMENTS } from "../src/shared/whiteboard-store.js"; // ── mergeElements is not exported, so we test it indirectly through @@ -134,6 +137,122 @@ describe("simplifyElements", () => { }); }); +// ── sanitizeElements ── + +describe("sanitizeElements", () => { + type El = Record; + + it("gives a points-less arrow points derived from width/height", () => { + // Regression: Excalidraw's isInvisiblySmallElement() reads + // `element.points.length` before restoreElement() supplies a default. + const [el] = sanitizeElements([ + { + id: "s1-s2-arrow", + type: "arrow", + x: -20, + y: 875, + width: 50, + height: 2, + }, + ]) as El[]; + expect(el.points).toEqual([ + [0, 0], + [50, 2], + ]); + }); + + it("supplies points for line and freedraw too", () => { + for (const type of ["line", "freedraw"]) { + const [el] = sanitizeElements([ + { id: `x-${type}`, type, x: 0, y: 0, width: 10, height: 20 }, + ]) as El[]; + expect(el.points).toEqual([ + [0, 0], + [10, 20], + ]); + } + }); + + it("preserves valid points", () => { + const points = [ + [0, 0], + [30, 10], + [60, 0], + ]; + const [el] = sanitizeElements([ + { id: "a", type: "arrow", x: 0, y: 0, width: 60, height: 0, points }, + ]) as El[]; + expect(el.points).toEqual(points); + }); + + it("replaces malformed or too-short points arrays", () => { + const [short] = sanitizeElements([ + { + id: "a", + type: "arrow", + x: 0, + y: 0, + width: 5, + height: 5, + points: [[0, 0]], + }, + ]) as El[]; + expect(short.points).toEqual([ + [0, 0], + [5, 5], + ]); + + const [junk] = sanitizeElements([ + { + id: "b", + type: "arrow", + x: 0, + y: 0, + width: 5, + height: 5, + points: ["nope", null, [1]], + }, + ]) as El[]; + expect(junk.points).toEqual([ + [0, 0], + [5, 5], + ]); + }); + + it("does not add points to shape elements", () => { + const [el] = sanitizeElements([rect("a")]) as El[]; + expect(el.points).toBeUndefined(); + }); + + it("coerces non-finite geometry to 0", () => { + const [el] = sanitizeElements([ + { id: "a", type: "rectangle", x: "10", y: null, width: NaN, height: 5 }, + ]) as El[]; + expect(el).toMatchObject({ x: 0, y: 0, width: 0, height: 5 }); + }); + + it("drops non-objects and elements missing id or type", () => { + const result = sanitizeElements([ + null, + 42, + "str", + { type: "rectangle" }, + { id: "no-type" }, + rect("ok"), + ]); + expect(result).toHaveLength(1); + expect((result[0] as El).id).toBe("ok"); + }); + + it("keeps unrelated fields untouched", () => { + const [el] = sanitizeElements([ + { ...rect("a"), strokeColor: "#e03131", boundElements: [{ id: "t" }] }, + ]) as El[]; + expect(el.strokeColor).toBe("#e03131"); + expect(el.boundElements).toEqual([{ id: "t" }]); + }); +}); + // ── isValidScene ── describe("isValidScene", () => { @@ -363,6 +482,61 @@ describe("createWhiteboardHandlers", () => { ).rejects.toThrow("full"); }); + it("persists arrows with points even when the agent omits them", async () => { + const deps = createMockDeps(); + const handlers = createWhiteboardHandlers(deps); + const pool = deps.pool as unknown as { query: ReturnType }; + + mockEmptyLoad(pool); + mockSaveReturn(pool, 1); + + await handlers.updateWhiteboard( + "agt_test", + [{ id: "a1", type: "arrow", x: 0, y: 0, width: 50, height: 2 }], + [] + ); + + const saveCall = pool.query.mock.calls[1]; + const scene = JSON.parse(saveCall[1][1] as string); + expect(scene.elements[0].points).toEqual([ + [0, 0], + [50, 2], + ]); + }); + + it("heals malformed elements already stored on the board", async () => { + const deps = createMockDeps(); + const handlers = createWhiteboardHandlers(deps); + const pool = deps.pool as unknown as { query: ReturnType }; + + mockLoadReturn( + pool, + { + elements: [ + { id: "old", type: "arrow", x: 0, y: 0, width: 9, height: 0 }, + ], + }, + 1 + ); + mockSaveReturn(pool, 2); + + const result = await handlers.updateWhiteboard( + "agt_test", + [rect("b")], + [] + ); + expect(result.elementCount).toBe(2); + + const scene = JSON.parse(pool.query.mock.calls[1][1][1] as string); + const healed = scene.elements.find( + (e: Record) => e.id === "old" + ); + expect(healed.points).toEqual([ + [0, 0], + [9, 0], + ]); + }); + it("ignores incoming elements without required fields", async () => { const deps = createMockDeps(); const handlers = createWhiteboardHandlers(deps); diff --git a/apps/web/src/components/app/whiteboard-pane.tsx b/apps/web/src/components/app/whiteboard-pane.tsx index 3996cc16..d9589385 100644 --- a/apps/web/src/components/app/whiteboard-pane.tsx +++ b/apps/web/src/components/app/whiteboard-pane.tsx @@ -1,4 +1,5 @@ -import { lazy, Suspense, useEffect, useState } from "react"; +import { Component, lazy, Suspense, useEffect, useState } from "react"; +import type { ErrorInfo, ReactNode } from "react"; import { useAtom } from "jotai"; import { whiteboardAgentDrewAtomFamily } from "@/lib/store"; @@ -6,6 +7,35 @@ import { cn } from "@/lib/utils"; const WhiteboardTab = lazy(() => import("@/components/app/whiteboard-tab")); +// A scene the editor can't restore must not escape to the router's error +// boundary, which would blank the entire app instead of just this tab. +class WhiteboardErrorBoundary extends Component< + { children: ReactNode }, + { failed: boolean } +> { + state = { failed: false }; + + static getDerivedStateFromError(): { failed: boolean } { + return { failed: true }; + } + + componentDidCatch(error: Error, info: ErrorInfo): void { + console.error("Whiteboard failed to render", error, info); + } + + render(): ReactNode { + if (this.state.failed) { + return ( +
+ This whiteboard could not be rendered. Reload to try again — if it + keeps failing, ask the agent to clear the board. +
+ ); + } + return this.props.children; + } +} + type WhiteboardPaneProps = { agentId: string | null; active: boolean; @@ -30,15 +60,17 @@ export function WhiteboardPane({ if (!opened || !agentId) return null; return (
- - Loading whiteboard… -
- } - > - - + + + Loading whiteboard… + + } + > + + + ); } diff --git a/e2e/whiteboard.spec.ts b/e2e/whiteboard.spec.ts index 8ef4fdb0..6b36fbf6 100644 --- a/e2e/whiteboard.spec.ts +++ b/e2e/whiteboard.spec.ts @@ -142,6 +142,54 @@ test.describe("Whiteboard", () => { expect(conflictData.version).toBe(1); }); + test("whiteboard renders when an agent writes an arrow without points", async ({ + page, + request, + }) => { + const agent = await createAgentViaAPI(request, { + name: `e2e-agent-wb-arrow-${Date.now()}`, + }); + + // An arrow with no `points` used to crash the whole route: Excalidraw's + // isInvisiblySmallElement() reads `element.points.length` unguarded. + await callMcpTool(request, agent.id, "whiteboard_update", { + elements: [ + { id: "s1", type: "rectangle", x: 0, y: 0, width: 160, height: 70 }, + { id: "s2", type: "rectangle", x: 300, y: 0, width: 160, height: 70 }, + { + id: "s1-s2-arrow", + type: "arrow", + x: 170, + y: 35, + width: 120, + height: 0, + }, + ], + }); + + const getRes = await request.get(`/api/v1/agents/${agent.id}/whiteboard`, { + headers: AUTH_HEADER, + }); + const { scene } = (await getRes.json()) as { + scene: { elements: Array<{ id: string; points?: unknown }> }; + }; + const arrow = scene.elements.find((e) => e.id === "s1-s2-arrow"); + expect(arrow?.points).toEqual([ + [0, 0], + [120, 0], + ]); + + await loadApp(page); + await clickAgentRow(page, agent.id); + await page.getByTestId("center-tab-whiteboard").click(); + await page.waitForURL(/\/agents\/[^/]+\/whiteboard/); + + await expect(page.getByTestId("whiteboard-canvas")).toBeVisible(); + await expect(page.locator(".excalidraw canvas").first()).toBeVisible(); + await expect(page.getByText("Unexpected Application Error")).toHaveCount(0); + await expect(page.getByText("could not be rendered")).toHaveCount(0); + }); + test("whiteboard MCP tool: agent can update and read whiteboard", async ({ request, }) => { From 3b2b28e7a1b264d11d45dc0a72f28f66f635b8a0 Mon Sep 17 00:00:00 2001 From: Luke Brevoort Date: Thu, 13 Aug 2026 11:18:50 -0600 Subject: [PATCH 2/2] fix(whiteboard): align freedraw pressures with points --- apps/server/src/shared/whiteboard.ts | 28 ++++++-- apps/server/test/whiteboard.test.ts | 104 +++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/apps/server/src/shared/whiteboard.ts b/apps/server/src/shared/whiteboard.ts index 4cc4785b..9cf35856 100644 --- a/apps/server/src/shared/whiteboard.ts +++ b/apps/server/src/shared/whiteboard.ts @@ -72,13 +72,27 @@ export function sanitizeElement(raw: unknown): RawElement | null { if (POINTS_REQUIRED_TYPES.has(el.type)) { const points = Array.isArray(el.points) ? el.points.filter(isPoint) : []; // Same fallback restoreElement() would apply, just early enough to matter. - out.points = - points.length >= 2 - ? points - : [ - [0, 0], - [width, height], - ]; + const usedFallback = points.length < 2; + out.points = usedFallback + ? [ + [0, 0], + [width, height], + ] + : points; + + // The renderer indexes pressures[i] per point when simulatePressure is + // falsy, so a missing or short pressures array throws the same way + // missing points did. Keep it aligned with points, defaulting to 0.5 as + // Excalidraw's own restore does. + if (el.type === "freedraw" && el.simulatePressure !== true) { + const pressures = Array.isArray(el.pressures) ? el.pressures : []; + out.pressures = (out.points as unknown[]).map((_, i) => { + const pressure = usedFallback ? undefined : pressures[i]; + return typeof pressure === "number" && Number.isFinite(pressure) + ? pressure + : 0.5; + }); + } } return out; diff --git a/apps/server/test/whiteboard.test.ts b/apps/server/test/whiteboard.test.ts index 01482737..aa9b625d 100644 --- a/apps/server/test/whiteboard.test.ts +++ b/apps/server/test/whiteboard.test.ts @@ -251,6 +251,110 @@ describe("sanitizeElements", () => { expect(el.strokeColor).toBe("#e03131"); expect(el.boundElements).toEqual([{ id: "t" }]); }); + + it("aligns freedraw pressures with points when simulatePressure is falsy", () => { + // The renderer indexes pressures[i] per point when simulatePressure is + // falsy — a freedraw with points but no pressures array throws. + const [missing] = sanitizeElements([ + { + id: "f1", + type: "freedraw", + x: 0, + y: 0, + width: 10, + height: 20, + points: [ + [0, 0], + [5, 5], + [10, 20], + ], + }, + ]) as El[]; + expect(missing.pressures).toEqual([0.5, 0.5, 0.5]); + + const [short] = sanitizeElements([ + { + id: "f2", + type: "freedraw", + x: 0, + y: 0, + width: 10, + height: 20, + points: [ + [0, 0], + [5, 5], + [10, 20], + ], + simulatePressure: false, + pressures: [0.25, "bad"], + }, + ]) as El[]; + expect(short.pressures).toEqual([0.25, 0.5, 0.5]); + }); + + it("supplies pressures alongside fallback freedraw points", () => { + const [el] = sanitizeElements([ + { + id: "f3", + type: "freedraw", + x: 0, + y: 0, + width: 10, + height: 20, + simulatePressure: false, + pressures: [0.9, 0.8, 0.7], + }, + ]) as El[]; + expect(el.points).toEqual([ + [0, 0], + [10, 20], + ]); + // Stale pressures from dropped points are not carried over. + expect(el.pressures).toEqual([0.5, 0.5]); + }); + + it("leaves pressures alone when simulatePressure is true", () => { + const [el] = sanitizeElements([ + { + id: "f4", + type: "freedraw", + x: 0, + y: 0, + width: 10, + height: 20, + points: [ + [0, 0], + [10, 20], + ], + simulatePressure: true, + pressures: [], + }, + ]) as El[]; + expect(el.pressures).toEqual([]); + }); + + it("preserves real pen pressures for well-formed freedraw", () => { + const points = [ + [0, 0], + [5, 5], + [10, 20], + ]; + const [el] = sanitizeElements([ + { + id: "f5", + type: "freedraw", + x: 0, + y: 0, + width: 10, + height: 20, + points, + simulatePressure: false, + pressures: [0.1, 0.2, 0.3], + }, + ]) as El[]; + expect(el.points).toEqual(points); + expect(el.pressures).toEqual([0.1, 0.2, 0.3]); + }); }); // ── isValidScene ──