Mermaid validation harness: golden hostile-input tests + real parse check

[jbeda]

Summary

Closes the gap where nothing in the repo verifies that generated Mermaid
actually parses. Two layers, per the plan:

1. Go golden tests (internal/render/mermaid/mermaid_test.go +
   internal/render/mermaid/testdata/*.golden.mmd) covering four cases the
   existing strings.Contains-based tests don't:

   • TestERRole_HostileCharacters — a role packing every hostile character
     sanitize handles (backticks, quotes, brackets, a literal newline)
     plus three it does not: <, >, and %%. This pins the current
     (buggy) passthrough behavior on purpose — see Render: sanitize lets Mermaid directives and angle brackets through from role text #29 below — with a comment
     pointing at the issue so a future fix updates this golden deliberately.
   • TestERRole_LongUnbrokenWord — a ~188-char single word with no spaces;
     there's no wrapping logic, so it passes through whole.
   • TestERSelfRow_EntityNamedSelfDoesNotCollide — an entity literally named
     Self with two self-relationships; the self-row attribute names (self,
     self2) are fixed literals, not entity-name-derived, so no collision.
   • TestER_UndeclaredRelationshipTarget — a relationship whose target entity
     has no entry in Entities at all. ER() doesn't validate references
     (that's modelith lint's job), so it renders a well-formed edge without
     panicking.

   Deliberately not duplicated: backtick/bracket/newline/quote/backslash
   sanitization, bounded/malformed cardinality glyph selection, self-row
   numbering/dedup, and reciprocal-edge folding — all already covered by
   TestERLabelSanitizesRole, TestADR_0008_SelfRowCarriesBothCardinalitySides,
   TestADR_0003_BoundedCardinalityRendersNearestGlyph,
   TestERCardinalityNotation, TestERMultipleSelfRelationships,
   TestERSelfRelationshipsDedupe, and the several TestADR_0008_* tests.

2. Real Mermaid parse check (hack/mermaid-check.sh + task mermaid-check

   • a new CI step): extracts every ```mermaid fenced block from
     examples/*.md and docs/05-parking-garage/*.md, plus the four new raw
     .mmd goldens above, and feeds each to
     npx -y @mermaid-js/mermaid-cli@11 (mmdc). Fails with a clear message
     naming the offending file/block on any parse error.

Related issue

References #29 (open, pre-existing bug: sanitize lets <, >, and %%
through unescaped, so a role can inject a Mermaid directive or lose text that
looks like markup). This PR documents the bug via
TestERRole_HostileCharacters's golden and comment; it does not fix it —
out of scope per the task.

Wiring / non-negotiable choices

• task mermaid-check is its own standalone task, not added to
  task check's cmds — explicit product requirement, so task check stays
  green for contributors without node/npm. This diverges from the
  validate-plugin pattern (which IS in check); the task's desc: says so.
• CI's new "Mermaid parse check" step runs the real mmdc binary (unlike
  the plugin-validation step, which substitutes a lighter jq check in CI).
  Pins actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
  with node-version: "24" — reusing the exact SHA already pinned in
  deploy-docs.yml for the same version.

Demonstrated failure, then reverted

Per the task's requirement, I temporarily added a broken fixture
(internal/render/mermaid/testdata/zzz_temp_broken.mmd containing
erDiagram\n A ||--o{ B :::: "broken"\n), ran the check, confirmed it
failed, then removed the fixture. Transcript:

$ bash hack/mermaid-check.sh
::error::mermaid-check: parse failed for internal/render/mermaid/testdata/zzz_temp_broken.mmd
Generating single mermaid chart

Error: Parse error on line 2:
...m    A ||--o{ B :::: "broken"
----------------------^
Expecting 'UNICODE_TEXT', 'STYLE_TEXT', got 'COLON'
...
mermaid-check: FAILED (8 block(s) checked)
EXIT=1

After removing the temp fixture:

$ bash hack/mermaid-check.sh
mermaid-check: OK (7 block(s) checked)
EXIT=0

That fixture is not part of this PR's diff — it was created and deleted
entirely within the local worktree before committing.

Open questions / judgment calls

Since I couldn't ask before finishing, I picked the conservative default in
each case:

1. Extraction script design. Rather than a generic Markdown-fence parser,
   hack/mermaid-check.sh does a simple line-oriented scan for ```mermaid
   / ``` on their own line. This matches every fence actually in the repo
   today (confirmed: none nest fences inside fences). If docs ever start
   nesting code fences, this scan would need to get smarter — noted in the
   script's own comment.
2. Raw .mmd vs re-wrapping in a fence. I fed the golden .mmd files to
   mmdc directly rather than wrapping them in a throwaway fence, since
   they're already exactly what mmdc -i expects and wrapping-then-stripping
   would add ceremony with no benefit. Documented in the script's top comment.
3. docs/05-parking-garage/index.md also matches the docs/05-parking-garage/*.md
   glob and contributes one of the 7 checked blocks (in addition to the
   golden-fixture garage.modelith.md). This seemed like desirable extra
   coverage (that page hand-embeds a diagram too) rather than scope creep, but
   flagging it since the task only explicitly named the golden files.
4. Single commit. The task didn't mandate commit granularity, so both
   layers landed in one commit.

Test plan

• task test — all new and existing tests pass, including the four new
  goldens (eyeballed each by hand against the real mermaid.ER() output
  before committing, per testing.md).
• task check — fully green; does not invoke mermaid-check.
• task mermaid-check — passes against real repo content (7 blocks).
• Demonstrated task mermaid-check's equivalent (hack/mermaid-check.sh)
  failing on injected bad input, then reverted (see transcript above).

[jbeda]

Code review — round 1

Three parallel angles: CLAUDE.md/rules compliance (Sonnet), shallow bug scan of the diff (Sonnet), adversarial run of the real binary + real mmdc against new and hand-built hostile fixtures (Opus).

Blocking

1. hack/mermaid-check.sh can silently "pass" having checked zero blocks. The glob-empty case errors, but nothing asserts count > 0 after processing found files. Two reproduced triggers: an unterminated ```mermaid fence (block silently dropped, never checked) and fence-line format drift (e.g. a trailing space after ```mermaid) — both leave count=0, fail=0, exit 0. This defeats the gate's purpose: a doc that stops closing its fences, or a future reformat, passes CI silently instead of failing loud.

2. TestERRole_HostileCharacters's comment overclaims coverage. It reads as pinning Mermaid directive-injection risk via %%{...}%%, but the fixture only contains bare %%pct, which renders inertly. A real %%{init: {'theme':'forest'}}%% payload was confirmed (via real mmdc render) to parse successfully while silently changing the whole diagram's theme and dropping the label text entirely — a more severe manifestation of issue Render: sanitize lets Mermaid directives and angle brackets through from role text #29 than plain character passthrough. Worth pinning explicitly given Render: sanitize lets Mermaid directives and angle brackets through from role text #29 is already being cited here, and given the task explicitly asked for %% coverage.

Not blocking / recorded as open questions, not required to fix

• hack/mermaid-check.sh's glob (examples/*.md, docs/05-parking-garage/*.md, plus new testdata) doesn't cover hand-authored Mermaid elsewhere in docs/ (e.g. docs/04-reading-the-diagrams.md, 9 blocks, all currently parse). Out of the stated scope (generated Markdown only) — flagging as a possible future broadening, not a defect.
• The new CI step lives inside the existing build-test job rather than a separate one, so a transient npx/registry hiccup can fail the required check. Consistent with the PR's own design (CI runs the real thing); no written rule requires a separate job.

Clean

• Test naming, golden exact-comparison discipline, comment conventions, action-pin SHA reuse from deploy-docs.yml, DCO sign-off, and the task check / mermaid-check split all check out against CLAUDE.md, go-style.md, testing.md, agent-workflow.md, dco.md.
• Self-row numbering doesn't leak across entities; lowercase self-named entity renders fine; entity/relationship-target names are schema-constrained (^[A-Z][A-Za-z0-9]+$) so name-based injection isn't reachable via the CLI path.
• Script's actual parse-failure teeth (a genuine syntax error) were verified to fail loudly (confirmed non-zero exit).

Fixing items 1–2 in a follow-up commit.

[jbeda]

Code review — round 2 (delta only)

Reviewed only fix commit 9f1075c (the two round-1 blockers). Clean: zero-blocks-checked counter is a correct total across all sources (no false-positive risk for a file that legitimately has zero blocks), the unterminated-fence check's state is properly function-local, the extended hostile_role.golden.mmd was hand-traced against sanitize() and matches exactly, and the rewritten comment doesn't overclaim (the test is a string-compare golden with no mmdc invocation, so it correctly claims only to pin source text, not rendered behavior).

No issues found. Stopping the review loop here per .claude/rules/agent-workflow.md (a clean round ends it). Left as a draft per the task's explicit instruction, not flipping to ready.

[jbeda]

Post-review: CI environment fix

After round 2 cleared, the real GitHub Actions run failed on every single Mermaid block (not just bad ones) — GitHub's ubuntu-latest runners restrict unprivileged user namespaces, so Chrome/Puppeteer's sandbox can't launch (No usable sandbox!). This didn't reproduce in local sandbox testing, only on the hosted runner. Fixed in hack/mermaid-check.sh (commit a461a5c) by passing mmdc -p a throwaway Puppeteer config with {"args": ["--no-sandbox"]} — safe here since the script only ever validates repo-authored Mermaid, never untrusted input. Re-confirmed the broken-input repro still fails correctly with the flag in place, and confirmed on a real CI run (https://github.com/stacklok/modelith/actions/runs/30187520068) that build-test now passes. Both required checks are green. Still a draft.

[jbeda]

Added docs/04-reading-the-diagrams.md to the glob (a5a0eb1), closing the first open question.

That page's diagrams are hand-authored rather than generated, which makes them the most important to check, not the least: nothing regenerates them, so render-check can't catch drift and a broken block ships silently. It's also the page that teaches the notation. My original brief said "generated Markdown", which is what scoped it out — my under-specification.

All nine blocks parse today, so this is coverage, not a fix. Checked-block count goes 7 → 16.

Verified locally end to end:

Check	Result
hack/mermaid-check.sh clean run	OK (16 block(s) checked)
Deliberately broken .mmd added	FAILED (8 block(s) checked), non-zero exit, reverted clean
Zero-block / unterminated-fence guards	present, and the count is reported so a silent no-op is visible

The second open question — CI step living in build-test rather than its own job — I'm leaving as-is. A separate job would buy parallelism and a cleaner failure signal, but it also re-does checkout and setup for one script; not worth it at this size.

Remaining before this is ready: the harness pins current sanitize behavior for <, > and %% with a golden. That is deliberate and commented, but it means whoever fixes #29 must update that golden — otherwise the test will look like it's defending the bug.