diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5fe859..6a2822d 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/Taskfile.yml b/Taskfile.yml index 2590476..9446383 100644 --- a/Taskfile.yml +++ b/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: diff --git a/docs/09-local-development.md b/docs/09-local-development.md index 60bd014..7f8b926 100644 --- a/docs/09-local-development.md +++ b/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. |
CI runs a lighter plugin check diff --git a/hack/mermaid-check.sh b/hack/mermaid-check.sh new file mode 100755 index 0000000..68a3303 --- /dev/null +++ b/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)" diff --git a/internal/render/mermaid/mermaid_test.go b/internal/render/mermaid/mermaid_test.go index 73555c9..193b4c4 100644 --- a/internal/render/mermaid/mermaid_test.go +++ b/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} #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") +} diff --git a/internal/render/mermaid/testdata/hostile_role.golden.mmd b/internal/render/mermaid/testdata/hostile_role.golden.mmd new file mode 100644 index 0000000..e9e86b5 --- /dev/null +++ b/internal/render/mermaid/testdata/hostile_role.golden.mmd @@ -0,0 +1,4 @@ +erDiagram + A {} + B {} + A ||--o{ B : "he said 'hi' (bracket) tick {brace} #hash %%pct %%{init: {'theme':'forest'}}%% second line" diff --git a/internal/render/mermaid/testdata/long_word_role.golden.mmd b/internal/render/mermaid/testdata/long_word_role.golden.mmd new file mode 100644 index 0000000..6392f1b --- /dev/null +++ b/internal/render/mermaid/testdata/long_word_role.golden.mmd @@ -0,0 +1,4 @@ +erDiagram + A {} + B {} + A ||--o{ B : "LoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelitLoremipsumdolorsitametconsecteturadipiscingelit" diff --git a/internal/render/mermaid/testdata/self_named_entity.golden.mmd b/internal/render/mermaid/testdata/self_named_entity.golden.mmd new file mode 100644 index 0000000..3a6d496 --- /dev/null +++ b/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" + } diff --git a/internal/render/mermaid/testdata/undeclared_target.golden.mmd b/internal/render/mermaid/testdata/undeclared_target.golden.mmd new file mode 100644 index 0000000..8d05441 --- /dev/null +++ b/internal/render/mermaid/testdata/undeclared_target.golden.mmd @@ -0,0 +1,3 @@ +erDiagram + Node {} + Node ||--o{ Ghost : "haunts"