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

.github/workflows/ci.yml

@@ -63,3 +63,12 @@ jobs:
             head -n 1 "$s" | grep -q '^---$' || { echo "::error::$s missing YAML frontmatter"; exit 1; }
           done
           echo "plugin manifest + ${#skills[@]} skill(s) OK"
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
+        with:
+          node-version: "24"
+      - name: Mermaid parse check
+        # Unlike the plugin-validation step above, this runs the real check —
+        # the actual mmdc binary — rather than a lighter substitute, since
+        # task mermaid-check is deliberately excluded from `task check` (it
+        # needs node/npm, which `task check` must not require locally).
+        run: bash hack/mermaid-check.sh

Taskfile.yml

@@ -108,6 +108,15 @@ tasks:
     cmds:
       - claude plugin validate ./plugin --strict
 
+  mermaid-check:
+    desc: >-
+      Parse every emitted/committed Mermaid diagram with the real mermaid-cli
+      (needs npx/node). NOT part of `check` — deliberately, unlike
+      validate-plugin — so `task check` stays green for contributors without
+      node/npm installed; CI runs the real thing instead.
+    cmds:
+      - bash hack/mermaid-check.sh
+
   check:
     desc: CI parity (vet, staticcheck, golangci-lint, test, lint models, render-check) plus the local-only claude-based validate-plugin (CI does a lighter jq check instead)
     cmds:

docs/09-local-development.md

@@ -19,6 +19,8 @@ bottom of this page.
 - **[Task](https://taskfile.dev)** — the task runner (`brew install go-task`).
 - **`jq`** — used by the CI plugin check (`brew install jq`).
 - **The `claude` CLI** — only needed to validate/develop the plugin locally.
+- **Node.js / `npx`** — only needed for `task mermaid-check` (runs
+  `@mermaid-js/mermaid-cli` via `npx`); not required for `task check`.
 
 ## Building the binary
 
@@ -54,7 +56,8 @@ arguments to list every target.
 | `task render` | Re-render the example models to Markdown. |
 | `task render-check` | Verify the committed Markdown is up to date. |
 | `task validate-plugin` | Validate the plugin with `claude plugin validate --strict` (needs the `claude` CLI). |
-| `task check` | CI parity (vet, staticcheck, test, lint-models, render-check) plus `validate-plugin`. |
+| `task mermaid-check` | Parse every emitted/committed Mermaid diagram with the real `mermaid-cli` (needs `npx`/node). Not part of `task check` — see below. |
+| `task check` | CI parity (vet, staticcheck, test, lint-models, render-check) plus `validate-plugin`. Does **not** run `mermaid-check`, so contributors without node/npm still get a green `task check`; CI runs the real Mermaid parse check as its own step instead. |
 
 <details>
 <summary>CI runs a lighter plugin check</summary>

hack/mermaid-check.sh

@@ -0,0 +1,114 @@
+#!/usr/bin/env bash
+# mermaid-check.sh extracts every Mermaid diagram this repo ships and feeds it
+# to the real Mermaid CLI's parser (@mermaid-js/mermaid-cli, "mmdc"), so a
+# syntax error in generated or hand-written Mermaid is caught here instead of
+# shipping a diagram that silently fails to render wherever it's viewed.
+#
+# Two source shapes are extracted, since the repo has both:
+#   - ```mermaid fenced code blocks inside Markdown: examples/*.md and
+#     docs/05-parking-garage/*.md (the renderer's committed golden output),
+#     plus docs/04-reading-the-diagrams.md, whose diagrams are hand-authored.
+#     Hand-authored blocks matter most here: nothing regenerates them, so a
+#     broken one ships silently, and that page is what teaches the notation.
+#   - raw, unfenced .mmd files: internal/render/mermaid/testdata/*.mmd (the
+#     Go golden-test fixtures for the renderer, pinned as bare Mermaid source
+#     with no surrounding fence). These are already exactly what mmdc expects,
+#     so they're fed to it as-is — wrapping them in a throwaway fence just to
+#     strip it back off before checking would be pure ceremony.
+set -euo pipefail
+shopt -s nullglob
+
+root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "$root"
+
+workdir="$(mktemp -d)"
+trap 'rm -rf "$workdir"' EXIT
+
+# GitHub-hosted ubuntu-latest runners disable unprivileged user namespaces
+# (AppArmor), which Chrome's sandbox needs; mmdc's bundled Chromium fails to
+# launch there with "No usable sandbox!". --no-sandbox is the documented
+# workaround (mermaid-cli/puppeteer troubleshooting docs). Safe here only
+# because every source this script feeds mmdc is repo-authored (committed
+# docs/examples or our own renderer's golden fixtures) — never
+# untrusted/fetched content, which is the usual caveat against --no-sandbox.
+puppeteer_config="$workdir/puppeteer-config.json"
+printf '{"args": ["--no-sandbox"]}' >"$puppeteer_config"
+
+fail=0
+count=0
+
+# check_file feeds one Mermaid source file to mmdc and reports failure with
+# the offending label (a path, optionally with a block number) on parse error.
+check_file() {
+  local mmd="$1" label="$2"
+  count=$((count + 1))
+  local out="$workdir/check-$count.svg"
+  local log="$workdir/check-$count.log"
+  if ! npx -y @mermaid-js/mermaid-cli@11 -p "$puppeteer_config" -i "$mmd" -o "$out" >"$log" 2>&1; then
+    echo "::error::mermaid-check: parse failed for $label"
+    cat "$log"
+    fail=1
+  fi
+}
+
+# extract_and_check_md pulls every ```mermaid fenced block out of a Markdown
+# file, writes each to its own temp file, and checks it. A line-oriented scan
+# is enough here: modelith's own fences are exactly ```mermaid / ``` on their
+# own line, and no source under examples/ or docs/05-parking-garage/ nests
+# fences inside fences.
+extract_and_check_md() {
+  local md="$1"
+  local rel="${md#"$root"/}"
+  local block=0
+  local in_block=0
+  local out=""
+  while IFS= read -r line || [ -n "$line" ]; do
+    if [ "$in_block" -eq 0 ]; then
+      if [ "$line" = '```mermaid' ]; then
+        in_block=1
+        block=$((block + 1))
+        out="$workdir/extract-$(basename "$md")-$block.mmd"
+        : >"$out"
+      fi
+      continue
+    fi
+    if [ "$line" = '```' ]; then
+      in_block=0
+      check_file "$out" "$rel block #$block"
+      continue
+    fi
+    printf '%s\n' "$line" >>"$out"
+  done <"$md"
+  if [ "$in_block" -ne 0 ]; then
+    echo "::error::mermaid-check: unterminated \`\`\`mermaid fence in $rel (block #$block never closed before EOF)"
+    fail=1
+  fi
+}
+
+md_files=(examples/*.md docs/04-reading-the-diagrams.md docs/05-parking-garage/*.md)
+mmd_files=(internal/render/mermaid/testdata/*.mmd)
+
+if [ ${#md_files[@]} -eq 0 ] && [ ${#mmd_files[@]} -eq 0 ]; then
+  echo "::error::mermaid-check: no Mermaid sources found to check"
+  exit 1
+fi
+
+for f in "${md_files[@]}"; do
+  extract_and_check_md "$f"
+done
+
+for f in "${mmd_files[@]}"; do
+  check_file "$f" "$f"
+done
+
+if [ "$count" -eq 0 ]; then
+  echo "::error::mermaid-check: found source files but extracted zero Mermaid blocks to check (fence format drift?)"
+  exit 1
+fi
+
+if [ "$fail" -ne 0 ]; then
+  echo "mermaid-check: FAILED ($count block(s) checked)"
+  exit 1
+fi
+
+echo "mermaid-check: OK ($count block(s) checked)"

internal/render/mermaid/mermaid_test.go

@@ -1,6 +1,8 @@
 package mermaid
 
 import (
+	"fmt"
+	"os"
 	"slices"
 	"sort"
 	"strings"
@@ -9,6 +11,45 @@ import (
 	"github.com/stacklok/modelith/internal/model"
 )
 
+// firstDiff reports the first line where want and got differ, so a golden
+// failure points at the change instead of just saying "they differ." Mirrors
+// internal/render/markdown's helper of the same name.
+func firstDiff(want, got string) string {
+	wl := strings.Split(want, "\n")
+	gl := strings.Split(got, "\n")
+	n := len(wl)
+	if len(gl) > n {
+		n = len(gl)
+	}
+	for i := 0; i < n; i++ {
+		var w, g string
+		if i < len(wl) {
+			w = wl[i]
+		}
+		if i < len(gl) {
+			g = gl[i]
+		}
+		if w != g {
+			return fmt.Sprintf("first difference at line %d:\n  want: %q\n  got:  %q", i+1, w, g)
+		}
+	}
+	return "(no line-level difference found)"
+}
+
+// assertGolden renders m and compares it byte-for-byte against the named file
+// under testdata/, failing with the first differing line on mismatch.
+func assertGolden(t *testing.T, m *model.Model, goldenPath string) {
+	t.Helper()
+	want, err := os.ReadFile(goldenPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := ER(m)
+	if got != string(want) {
+		t.Errorf("rendered output does not match %s.\n%s", goldenPath, firstDiff(string(want), got))
+	}
+}
+
 func TestERDeclaresAllEntities(t *testing.T) {
 	m := &model.Model{Entities: map[string]model.Entity{
 		"Alpha": {Definition: "a"},
@@ -558,3 +599,77 @@ func TestERDedupesSemanticallyEqualInverses(t *testing.T) {
 		t.Errorf("expected semantically-equal inverse deduped to 1 edge, got %d:\n%s", n, ER(m))
 	}
 }
+
+// TestERRole_HostileCharacters pins current output for a role that packs
+// every hostile character sanitize knows about (backticks, quotes, brackets, a
+// literal newline) alongside characters it does NOT neutralize: '<', '>', and
+// '%%'. This golden pins the CURRENT (buggy) behavior on purpose, not the
+// desired one: fixing issue #29 must update this golden as part of that fix,
+// not treat the diff as a regression. This test pins only the generated `.mmd`
+// source text produced by ER() — it does not verify how a Mermaid renderer
+// interprets that text.
+//
+// The role also embeds a `%%{init: ...}%%`-shaped substring. Separately, a
+// real render of that shape through mmdc was confirmed to parse successfully
+// while silently changing the whole diagram's theme and dropping this edge's
+// label text — a more severe manifestation of #29 than plain character
+// passthrough into the label. This golden pins that the substring reaches the
+// generated source unescaped; it does not itself exercise or assert the
+// renderer-level effect, which was only checked by hand outside this test.
+func TestERRole_HostileCharacters(t *testing.T) {
+	t.Parallel()
+	role := "he said \"hi\" [bracket] `tick` {brace} <angle> #hash %%pct %%{init: {'theme':'forest'}}%%\nsecond line"
+	m := &model.Model{Entities: map[string]model.Entity{
+		"A": {Definition: "a", Relationships: []model.Relationship{
+			{Entity: "B", Cardinality: "1:n", Role: role, Ownership: "owned"},
+		}},
+		"B": {Definition: "b"},
+	}}
+	assertGolden(t, m, "testdata/hostile_role.golden.mmd")
+}
+
+// TestERRole_LongUnbrokenWord pins current output for a very long role with no
+// spaces to break on. The renderer has no wrapping logic, so it passes the
+// word through whole; this guards that a long label doesn't get truncated,
+// panic, or otherwise misrender.
+func TestERRole_LongUnbrokenWord(t *testing.T) {
+	t.Parallel()
+	longWord := strings.Repeat("Loremipsumdolorsitametconsecteturadipiscingelit", 4) // 188 chars, no spaces
+	m := &model.Model{Entities: map[string]model.Entity{
+		"A": {Definition: "a", Relationships: []model.Relationship{
+			{Entity: "B", Cardinality: "1:n", Role: longWord, Ownership: "owned"},
+		}},
+		"B": {Definition: "b"},
+	}}
+	assertGolden(t, m, "testdata/long_word_role.golden.mmd")
+}
+
+// TestERSelfRow_EntityNamedSelfDoesNotCollide guards an entity literally named
+// "Self": the self-row attribute names are always the fixed literal "self",
+// "self2", … regardless of the entity's own name, so an entity named "Self"
+// must not produce a doubled or otherwise confused row name.
+func TestERSelfRow_EntityNamedSelfDoesNotCollide(t *testing.T) {
+	t.Parallel()
+	m := &model.Model{Entities: map[string]model.Entity{
+		"Self": {Definition: "s", Relationships: []model.Relationship{
+			{Entity: "Self", Cardinality: "0..5:1", Role: "`Mirror`"},
+			{Entity: "Self", Cardinality: "1:2", Role: "`Twin`", Ownership: "owned"},
+		}},
+	}}
+	assertGolden(t, m, "testdata/self_named_entity.golden.mmd")
+}
+
+// TestER_UndeclaredRelationshipTarget pins that ER renders a well-formed edge
+// to a relationship target that has no entry in Entities at all. ER does not
+// validate references — that's modelith lint's job — so it must render
+// without panicking, and the undeclared entity gets no {} block of its own
+// since it never appears in EntityNames().
+func TestER_UndeclaredRelationshipTarget(t *testing.T) {
+	t.Parallel()
+	m := &model.Model{Entities: map[string]model.Entity{
+		"Node": {Definition: "n", Relationships: []model.Relationship{
+			{Entity: "Ghost", Cardinality: "1:n", Role: "haunts", Ownership: "owned"},
+		}},
+	}}
+	assertGolden(t, m, "testdata/undeclared_target.golden.mmd")
+}

internal/render/mermaid/testdata/hostile_role.golden.mmd

@@ -0,0 +1,4 @@
+erDiagram
+    A {}
+    B {}
+    A ||--o{ B : "he said 'hi' (bracket) tick {brace} <angle> #hash %%pct %%{init: {'theme':'forest'}}%% second line"

internal/render/mermaid/testdata/long_word_role.golden.mmd

@@ -0,0 +1,4 @@
+erDiagram
+    A {}
+    B {}
+    A ||--o{ B : "LoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelit"

internal/render/mermaid/testdata/self_named_entity.golden.mmd

@@ -0,0 +1,5 @@
+erDiagram
+    Self {
+        Self self "0..5:1 — Mirror"
+        Self self2 "1:2 owned — Twin"
+    }

internal/render/mermaid/testdata/undeclared_target.golden.mmd

@@ -0,0 +1,3 @@
+erDiagram
+    Node {}
+    Node ||--o{ Ghost : "haunts"