Skip to content

fix(whiteboard): stop malformed agent-drawn elements from crashing the board - #857

Open
lukebrevoort wants to merge 1 commit into
selfcontained:mainfrom
lukebrevoort-mytra:agt_6d01e0fae2b9/agent-fae2b9
Open

fix(whiteboard): stop malformed agent-drawn elements from crashing the board#857
lukebrevoort wants to merge 1 commit into
selfcontained:mainfrom
lukebrevoort-mytra:agt_6d01e0fae2b9/agent-fae2b9

Conversation

@lukebrevoort

@lukebrevoort lukebrevoort commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Overview

Ran into a bit of a nasty bug when playing with the whiteboard today. Seems like agents that are further along on context (degrading on performance) can occasionally make some large mistakes! This one particular agent bricked the whole whiteboard for its entire session and crashed it which was nasty work.

I was getting this on the page before a refresh brought me back to a spinning wheel of death lol. Right after a sent an agent to work on a fix and communicate the issue with another dispatch agent. This is what it came up with!

Error!
Cannot read properties of undefined (reading 'length')
TypeError: Cannot read properties of undefined (reading 'length')
at Ns (http://127.0.0.1:6767/assets/percentages-BXMCSKIN-35O5HK7p.js:41:17494)
at http://127.0.0.1:6767/assets/percentages-BXMCSKIN-35O5HK7p.js:38:23339
at Array.reduce ()
at pu (http://127.0.0.1:6767/assets/percentages-BXMCSKIN-35O5HK7p.js:38:23298)
at http://127.0.0.1:6767/assets/percentages-BXMCSKIN-35O5HK7p.js:159:66624
at http://127.0.0.1:6767/assets/percentages-BXMCSKIN-35O5HK7p.js:159:67055
at Ck (http://127.0.0.1:6767/assets/index-DUPrbybF.js:41:24296)
at Mp (http://127.0.0.1:6767/assets/index-DUPrbybF.js:41:42451)
at Cve (http://127.0.0.1:6767/assets/index-DUPrbybF.js:41:41273)
at Dd (http://127.0.0.1:6767/assets/index-DUPrbybF.js:41:40312)
💿 Hey developer 👋

You can provide a way better UX than this when your app throws errors by providing your own ErrorBoundary or errorElement prop on your route.


AGENT

What broke

The whiteboard tab crashed with Unexpected Application Error! Cannot read properties of undefined (reading 'length'), and in the other entry path hung forever on Excalidraw's "Loading scene…" spinner.

Both symptoms are one bug, and it's data, not rendering code. The FeedbackKit agent (agt_c711844f5fdb) generated a flow diagram through the whiteboard_update MCP tool and emitted 6 arrows — s1-s2-arrow, s2-s3-arrow, … — carrying only {id, type, x, y, width, height, strokeColor} and no points array.

Excalidraw 0.18.1's restoreElements() runs isInvisiblySmallElement() on each raw element:

if (isLinearElement(element) || isFreeDrawElement(element)) {
  return element.points.length < 2;   // unguarded
}

A points-less arrow throws there. In the render path React Router catches it and blanks the route; inside Excalidraw's async initializeScene the same throw becomes an unhandled rejection so isLoading never flips false — hence the permanent spinner.

The server let this through because whiteboard_update validated only that id and type are strings and persisted the rest verbatim.

Why this fix

restoreElement() — three lines later in the same function — already contains the exact fallback for this case:

!Array.isArray(element.points) || element.points.length < 2
  ? [pointFrom(0, 0), pointFrom(element.width, element.height)]
  : element.points

So the library already defends against points-less arrows; it just runs the unguarded check first. This is an ordering bug upstream. sanitizeElements() applies that identical fallback before the data reaches the editor, so there is no behavioral divergence to keep in sync — and if the upstream ordering is ever fixed, this becomes a harmless no-op rather than a conflicting workaround.

It's wired into loadWhiteboard and saveWhiteboard, the single choke point for every path (REST route, MCP handlers, merge-on-conflict). Sanitizing on read as well as write means:

  • bad data can no longer be persisted, and
  • the 8 boards already holding it render immediately and heal on their next write — no data migration, no writes to the production DB.

Also included

  • Error boundary around the whiteboard tab. A single unrenderable board taking down the entire app is disproportionate; it now degrades to a message.
  • Corrected MCP cheat sheet. This was a cause, not just stale docs — it never marked points required and said "the editor auto-heals many issues… Don't over-validate." I messaged the agent that wrote the bad data; it confirmed that line is why it omitted points.

Verification

Copied the real 100-element board out of production (read-only) into an isolated dev DB to reproduce exactly, then ran the actual Excalidraw library against both versions in-browser:

input result
raw agent data throws Cannot read properties of undefined (reading 'length')
server-sanitized restores all 100 elements

Then live in the UI: board renders, survives a tab round-trip, and a PUT of a points-less arrow lands in the DB with points.

  • apps/server/test/whiteboard.test.ts — 41 passing (11 new, covering the derived-points fallback, malformed/short points, non-finite geometry, and the merge path healing already-stored elements)
  • e2e/whiteboard.spec.ts — new regression test driving the full agent-writes-bad-arrow → tab-renders path
  • Full E2E suite: 176 passed, 0 failed
  • pnpm run check, pnpm run lint:web, pnpm run build:web all clean

Out of scope

  • The upstream ordering bug in Excalidraw's restoreElements() is not patched here — worth an issue against excalidraw.
  • Existing production rows are left untouched; they self-heal via the read path.
  • Pre-existing repo issue, not addressed: root-level files can't resolve pg, @types/node, or eslint under pnpm 11's stricter hoisting (breaks E2E collection and the pre-commit hook). I worked around it locally with symlinks; the real fix is declaring those in the root package.json.

Reviewer notes

  • sanitizeElements spreads unknown fields through untouched — it normalizes geometry and points only, so bindings, boundElements, styling, and custom fields survive round-tripping. There's a test pinning that.
  • It runs on every board read. Boards cap at 20k elements and reads are infrequent, so the cost is negligible, but that's the tradeoff being made for the self-healing property.
  • freedraw is included in the sanitized types: restoreElement() passes element.points straight through for it with no fallback, so it would throw later even if the earlier check were fixed.

…e board

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 <noreply@anthropic.com>
@lukebrevoort
lukebrevoort marked this pull request as ready for review July 30, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants