fix(webapp): a broken mermaid fence says which line, and why (BEA-135)

[ssowonny]

TL;DR

• A broken mermaid fence used to be a dead end: "Couldn't render this diagram." and nothing else. Now it prints the line number, the offending source, the caret and the expected tokens.
• Nothing new computes this — mermaid already threw it, and a bare catch { dropped it on the floor one line from where it was needed.
• The message is inserted as text, never HTML — it quotes the author's source verbatim into a string the viewer mounts through dangerouslySetInnerHTML.
• Lands on the viewer and the /s/ share page from one helper, but the CSS had to go in two places (the share shell never loads the app's stylesheet).
• Known gap: the line number is the diagram's, not the file's, and nothing here makes the error visible to the agent that wrote the fence — both explicitly out of scope in the issue.

flowchart LR
    E["mermaid throws:<br/>line, source, caret,<br/>expected tokens"]
    E -->|"before: catch {}"| D["discarded<br/>one faint line"]
    E -->|"after: catch (err)"| K["printed as textContent<br/>under the note"]

Loading

The one thing that can't break

renderMermaid returns a string that FileView.tsx mounts through dangerouslySetInnerHTML. The parse error embeds the author's source verbatim, so the insertion is textContent and the DOM serialization at the end of the helper escapes it. An innerHTML here would be a stored-XSS path through any teammate's markdown.

The e2e proves it, and the seeded fence had to change for the proof to mean anything: the parser's window is 20 characters of past input, so the spec's <img src=x onerror=alert(1)> (28 chars) can never appear whole in the message — and a tag the parser truncated leaves no start tag for an innerHTML bug to mount, so the test would pass either way. The seed now carries <img onerror=x>, which fits, and the test asserts both halves: the literal characters are shown, and #content img is 0.

Screenshots

Hub viewer (diagram.md in the seeded wiki project):

before	after

The /s/<token> share page for the same file:

before	after

The share shell has its own inline CSS and its own dark block, so that got driven too:

What changed

File	Change
frontend/src/lib/mermaid.ts	catch (err); the message goes in a sibling div.mermaid-err-detail via textContent, capped at 2000 chars
frontend/src/style.css	.mermaid-err-detail — monospace, white-space: pre, max-height: 12em, overflow: auto
webapp/shares.go	the same rule in the share shell's inline CSS, plus its dark override
webapp/e2e_serve_test.go	the seeded broken fence now carries a complete <img onerror=x> tag
frontend/e2e/browse.spec.ts	diagnostics asserted on both surfaces
webapp/static/	rebuilt (second commit)

white-space: pre is load-bearing — the caret marker only lines up in a monospace, non-wrapping box. max-height + overflow is what keeps a pathological expected-token list inside its own box instead of pushing the document sideways; the 2000-char slice is the same cap on the DOM side.

The diagnostic sits in a sibling of .mermaid-err rather than inside it, which is what keeps the two existing exact-text assertions (browse.spec.ts for the viewer and the share page) green with no edit.

Deviations from the reviewed plan

Two, both because the plan's assertions couldn't hold against the real parser:

1. The XSS payload is shorter than specced. <img src=x onerror=alert(1)> never appears whole in the message (20-char window), and the plan's toContainText("<img src=x onerror=alert(1)>") would have failed. Worse, asserting the truncated form would have made the test pass even with an innerHTML bug. Now <img onerror=x>.
2. The note's bottom margin moved, -.9em 0 1.3em → -.9em 0 .4em (and -8px 0 20px → -8px 0 4px on the share page), so the detail block sits against the note instead of a paragraph gap away. The plan put a negative top margin on the detail instead; same result, one fewer negative margin.

Not done, per the plan: file-relative line numbers, agent-visible errors, and any change to the chunk-blocked path.

Reviewer note on the diff size

The second commit renames 118 files under static/assets/. Rollup re-hashes the whole mermaid chunk graph when its importer changes; the contents are byte-identical and only the embedded import filenames differ. check-dist.sh demands it, so it can't be left out — but it can be read as one commit and skipped.

Acceptance

• go build ./..., go vet ./... — clean
• go test ./... — all packages pass (internal/webapp 209s)
• npm run e2e — 170 passed, 1 pre-existing skip
• npm test (frontend unit) — 76 pass
• frontend/check-dist.sh — internal/webapp/static is fresh
• UI driven for real on both surfaces, light and dark, plus the computed styles behind the acceptance criteria (white-space: pre, the detail box scrolling while documentElement does not)

Architecture diagrams unchanged: no types or relationships moved — mermaid.ts's exported surface is the same, and .mermaid-err-detail is a div, not a seam.

Closes BEA-135.

Build session

cd $(git worktree list | grep bea-135-ph-scan-bug-broken-mermaid-block-fails-with-no-line-number | awk '{print $1}') && claude --resume db69bf9d-9315-49b3-a5ca-ec20d7a8597b

(only works on the machine the build ran on)