Repository navigation
fix(block): put the block diagram title in the front matter - #117
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR standardizes Mermaid titles as quoted YAML front matter, moves block diagram titles out of diagram bodies, and adds Puppeteer-based rendering validation with self-tests and CI integration. Documentation and generated examples now use the updated format. ChangesMermaid title serialization and output
Browser-based validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant MermaidChecker
participant LocalHTTPServer
participant PuppeteerBrowser
CI->>MermaidChecker: run self-tests and diagram validation
MermaidChecker->>LocalHTTPServer: serve Mermaid assets
MermaidChecker->>PuppeteerBrowser: render each diagram
PuppeteerBrowser-->>MermaidChecker: rendered output or error
MermaidChecker-->>CI: validation status and diagnostics
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/mermaid-check/selftest.mjs (1)
14-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the rendered-title validation path.
No case expects
the title "..." is declared but does not appear in the rendered diagram. The block and kanban cases stop before rendering, and the remaining cases do not declare a title.Add a fixture or a test seam that produces successful SVG output without the declared title. Assert the title-visibility diagnostic and nonzero status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/mermaid-check/selftest.mjs` around lines 14 - 35, Add a self-test case in the cases array that exercises the rendered-title validation path by using a fixture or test seam producing valid SVG without its declared title; assert the `the title "..." is declared but does not appear in the rendered diagram` diagnostic and a nonzero status, while keeping the existing parse-error and valid cases unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mermaid/block/block_diagram.go`:
- Around line 109-110: Update the title emission in the WithTitle flow to quote
trimmedTitle with YAML-safe formatting before adding it to the front matter. Add
regression cases covering titles containing YAML-significant characters, while
preserving the existing trimmed title behavior.
In `@scripts/mermaid-check/check.mjs`:
- Around line 147-151: Update the front-matter title parsing in the loop around
the title match to use Mermaid-compatible YAML semantics instead of the current
regex extraction and unquote handling. Correctly support comments, single-quoted
values, and escaped double-quoted values, reusing the project’s existing YAML
parser or semantics where available, and add tests covering all three cases.
- Around line 203-207: Update the Puppeteer launch configuration in the browser
initialization to remove the "--no-sandbox" argument, while preserving the other
launch options and ensuring the workflow or browser setup supports Chrome’s
sandbox.
---
Nitpick comments:
In `@scripts/mermaid-check/selftest.mjs`:
- Around line 14-35: Add a self-test case in the cases array that exercises the
rendered-title validation path by using a fixture or test seam producing valid
SVG without its declared title; assert the `the title "..." is declared but does
not appear in the rendered diagram` diagnostic and a nonzero status, while
keeping the existing parse-error and valid cases unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d266cf8d-4796-481c-a540-038cec170993
⛔ Files ignored due to path filters (1)
scripts/mermaid-check/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.github/workflows/doc_render.ymlCONTRIBUTING.mdMakefileREADME.mddoc/block/generated.mdmermaid/block/block_diagram.gomermaid/block/block_diagram_test.gomermaid/block/config.gomermaid/block/examples_test.gomermaid/kanban/config.gomermaid/mindmap/config.goscripts/mermaid-check/check.mjsscripts/mermaid-check/package.jsonscripts/mermaid-check/selftest.mjsscripts/mermaid-check/testdata/block-title-statement.mdfixturescripts/mermaid-check/testdata/class-standalone-annotation.mdfixturescripts/mermaid-check/testdata/kanban-title-statement.mdfixturescripts/mermaid-check/testdata/unparsable.mdfixturescripts/mermaid-check/testdata/valid.mdfixture
| const browser = await puppeteer.launch({ | ||
| headless: true, | ||
| executablePath: process.env.PUPPETEER_EXECUTABLE_PATH || undefined, | ||
| args: ["--no-sandbox", "--disable-dev-shm-usage"], | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' .github/workflows/doc_render.yml
rg -n -C 3 -- '--no-sandbox|PUPPETEER_EXECUTABLE_PATH' scripts/mermaid-check/check.mjsRepository: nao1215/markdown
Length of output: 2893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- checker outline ---'
ast-grep outline scripts/mermaid-check/check.mjs
echo '--- checker sections ---'
sed -n '1,280p' scripts/mermaid-check/check.mjs
echo '--- package configuration ---'
cat scripts/mermaid-check/package.json
printf '\n--- package lock browser-related entries ---\n'
rg -n -C 2 'puppeteer|chrome|chromium|sandbox' scripts/mermaid-check/package-lock.json | head -160
echo '--- workflow and sandbox references ---'
rg -n -C 3 'mermaid-check|pull_request|pull_request_target|runs-on|PUPPETEER_EXECUTABLE_PATH|no-sandbox|sandbox|container|permissions' .github scripts README.md docs 2>/dev/null | head -300Repository: nao1215/markdown
Length of output: 25104
🌐 Web query:
site:pptr.dev troubleshooting --no-sandbox trusted content Chrome sandbox Puppeteer
💡 Result:
The --no-sandbox argument in Puppeteer is a flag used to disable Chrome's built-in sandboxing mechanisms [1][2]. Chrome uses multiple layers of sandboxing to isolate the browser process from the host environment, protecting against malicious web content [1][2]. When you use --no-sandbox, this protection is disabled [1][2]. Consequently, if the browser navigates to or interacts with malicious, untrusted, or compromised web content, that content can potentially escape the browser process and access, modify, or execute code on your host system [1][2]. For this reason, the use of --no-sandbox is strongly discouraged and should only be considered if you have absolute trust in every piece of content the browser will open [1][2]. Key considerations: - Troubleshooting: You will typically encounter the No usable sandbox! error if the host environment is not properly configured for sandboxing (often occurring in Linux environments or containers) [1][2]. The recommended solution is to configure the sandbox properly on the host rather than disabling it [1][2]. - Docker/Containers: Running Chrome in a container often requires specific capabilities (e.g., --cap-add=SYS_ADMIN) to enable the sandbox [3]. Disabling the sandbox is a common workaround in these environments but introduces significant security risks [1][2]. - Trusted vs. Untrusted Content: The term trusted in this context refers to your confidence that the content you are automating will not attempt to exploit the browser [1][2]. Do not confuse this with Puppeteer's own event-handling terminology; Puppeteer generates trusted input events (simulated user interactions) by default to help you interact with pages naturally [4], but this is a separate concept from the process-level sandbox that protects your host machine [1][2]. If you must run in restricted environments, prioritize OS-level or container-level security configurations over disabling the internal browser sandbox [1][5].
Citations:
- 1: https://pptr.dev/troubleshooting
- 2: https://pptr.dev/next/troubleshooting
- 3: https://pptr.dev/guides/docker
- 4: https://pptr.dev/faq
- 5: https://pptr.dev/api/puppeteer.connectoptions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all Mermaid workflow files and checkout/install steps ---'
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] || continue
if rg -q 'doc_render|mermaid-check|pull_request|npm ci|actions/checkout' "$f"; then
echo "### $f"
sed -n '1,140p' "$f"
fi
done
echo '--- package lifecycle and local executable references ---'
rg -n -C 3 '"(preinstall|install|postinstall)"|PUPPETEER_EXECUTABLE_PATH|puppeteer\.launch|mermaid\.render|securityLevel' scripts/mermaid-check package*.json .github 2>/dev/null || true
echo '--- repository documentation on CI trust and generated Markdown ---'
rg -n -C 3 'untrusted|pull request|pull-request|generated.*markdown|mermaid|diagram' README.md CONTRIBUTING.md .github scripts doc 2>/dev/null | head -240 || trueRepository: nao1215/markdown
Length of output: 25316
🌐 Web query:
site:docs.github.com GitHub Actions pull_request fork untrusted code secrets read-only GITHUB_TOKEN workflow
💡 Result:
GitHub Actions includes specific security protections for workflows triggered by pull requests from forks to prevent the execution of untrusted code from compromising repository secrets or credentials [1][2][3]. When a workflow is triggered by a pull_request event from a forked repository: 1. Secrets are withheld: Repository and organization secrets are not passed to the workflow runner, preventing untrusted code in the fork from accessing sensitive environment variables [3][4][5]. 2. Read-only GITHUB_TOKEN: The workflow automatically receives a GITHUB_TOKEN with read-only permissions [3][6][7]. This ensures that even if the workflow code is modified within the pull request, it cannot use the token to modify the repository's contents, settings, or other protected resources [2][8]. 3. Trusted Code Execution: The workflow file itself is executed from the merge commit of the pull request [1][9]. Because this commit is controlled by the fork contributor, GitHub applies these restrictive security measures by default [1]. In contrast, the pull_request_target event is designed for scenarios where automation (such as triage or labeling) needs to run on pull requests from forks [1][9]. It runs the workflow file from the base repository's default branch—which is trusted code—rather than the fork's branch [1][9]. Consequently, it can be granted access to secrets and a read/write GITHUB_TOKEN, but developers must exercise extreme caution to ensure the workflow does not inadvertently checkout or execute untrusted code from the pull request, as this could expose the repository to malicious exploits [2][9]. For added security, developers are encouraged to use the permissions key in their workflow files to enforce the principle of least privilege, explicitly setting GITHUB_TOKEN permissions to read-only whenever possible [2][10][7].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 2: https://docs.github.com/en/actions/reference/security/secure-use
- 3: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 4: https://docs.github.com/en/enterprise-cloud@latest/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets
- 5: https://docs.github.com/en/code-security/reference/secret-security/secret-types
- 6: https://docs.github.com/en/enterprise-server@3.2/actions/reference/authentication-in-a-workflow
- 7: https://docs.github.com/en/enterprise-server@3.13/actions/security-guides/automatic-token-authentication
- 8: https://docs.github.com/en/enterprise-cloud@latest/actions/concepts/security/compromised-runners
- 9: https://docs.github.com/en/enterprise-cloud@latest/actions/reference/security/securely-using-pull_request_target
- 10: https://docs.github.com/en/actions/tutorials/authenticate-with-github_token
🌐 Web query:
site:pptr.dev puppeteer Chrome for Testing Linux sandbox GitHub Actions ubuntu-latest no-sandbox
💡 Result:
When running Puppeteer with Chrome for Testing on GitHub Actions (ubuntu-latest), the browser relies on a Linux sandbox for security [1][2]. If you encounter a "No usable sandbox!" error, it is typically because the environment lacks the necessary configuration to support this sandbox [1][2]. The recommended and most secure approach is to ensure the required system dependencies are installed on the runner [1][2][3]. You can install these automatically by running the following command as part of your GitHub Actions workflow: npx puppeteer browsers install chrome --install-deps [4] Using --no-sandbox is discouraged because it disables critical security protections [1][2]. If you absolutely trust the content you are testing and cannot resolve the sandbox issue, you can launch Puppeteer with the --no-sandbox argument: const browser = await puppeteer.launch({ args: ['--no-sandbox'], }); [1][2] Alternatively, you can use the environment variable PUPPETEER_DANGEROUS_NO_SANDBOX if you need to force this behavior across your application [5]. If you are using Docker containers within GitHub Actions, ensure the container has the necessary SYS_ADMIN capability to support sandboxing [6].
Citations:
- 1: https://pptr.dev/next/troubleshooting
- 2: https://pptr.dev/troubleshooting
- 3: https://pptr.dev/guides/system-requirements
- 4: https://pptr.dev/browsers-api
- 5: https://pptr.dev/CHANGELOG
- 6: https://pptr.dev/guides/docker
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- workflow checkout and permissions facts ---'
python3 - <<'PY'
from pathlib import Path
p = Path(".github/workflows/doc_render.yml")
text = p.read_text()
for n, line in enumerate(text.splitlines(), 1):
if n <= 55:
print(f"{n:4}: {line}")
PY
echo '--- source data flow into browser ---'
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/mermaid-check/check.mjs")
lines = p.read_text().splitlines()
for start, end in ((190, 230), (235, 280)):
print(f"--- lines {start}-{end} ---")
for n in range(start, min(end, len(lines)) + 1):
print(f"{n:4}: {lines[n-1]}")
PY
echo '--- browser sandbox configuration in repository ---'
rg -n -C 3 --glob '!scripts/mermaid-check/package-lock.json' \
'no-sandbox|disable-setuid-sandbox|userns|sandbox|SYS_ADMIN|PUPPETEER_EXECUTABLE_PATH|container:' \
.github scripts Dockerfile* docker-compose* 2>/dev/null || trueRepository: nao1215/markdown
Length of output: 6241
Remove --no-sandbox from the Puppeteer launch.
The workflow renders Mermaid blocks from pull requests with securityLevel: "loose". Configure ubuntu-latest or the browser installation to support Chrome’s sandbox.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/mermaid-check/check.mjs` around lines 203 - 207, Update the Puppeteer
launch configuration in the browser initialization to remove the "--no-sandbox"
argument, while preserving the other launch options and ensuring the workflow or
browser setup supports Chrome’s sandbox.
|
Thanks — all three findings were real, and the first one turned out to be the more serious bug of the two in this PR. Quote titles before emitting YAML front matter. Confirmed against mermaid 11.16.1, which is the build GitHub serves. An unquoted scalar is not safe to construct from arbitrary text, and the failures are worse than cosmetic:
The first two are exactly the "Unable to render rich display" failure this PR set out to chase, so this was not a hypothetical. Every builder that emits front matter had it, not just I used an explicit double-quoted scalar with Parse front matter with YAML semantics. Done: Remove The flag itself has to stay. Ubuntu 23.10 and later block the unprivileged user namespaces Chrome's sandbox needs, and |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/frontmatter.go`:
- Around line 26-33: Update quoteYAML to use strconv.Quote(value) instead of the
manual strings.NewReplacer, ensuring YAML-forbidden control characters receive
compatible escape sequences. Remove the obsolete replacer logic and add
regression cases covering the remaining control characters.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19a19ffa-d301-4be7-8a2f-6d83debc6a65
⛔ Files ignored due to path filters (1)
scripts/mermaid-check/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
README.mddoc/block/generated.mddoc/class/generated.mddoc/flowchart/generated.mddoc/gitgraph/generated.mddoc/kanban/generated.mddoc/mindmap/generated.mddoc/requirement/generated.mddoc/state/generated.mdinternal/frontmatter.gointernal/frontmatter_test.gomermaid/block/block_diagram.gomermaid/block/block_diagram_test.gomermaid/block/examples_test.gomermaid/class/class_diagram.gomermaid/class/class_diagram_test.gomermaid/class/examples_test.gomermaid/flowchart/examples_test.gomermaid/flowchart/flowchart.gomermaid/flowchart/flowchart_test.gomermaid/gitgraph/examples_test.gomermaid/gitgraph/git_graph.gomermaid/gitgraph/git_graph_test.gomermaid/kanban/examples_test.gomermaid/kanban/kanban.gomermaid/kanban/kanban_test.gomermaid/mindmap/examples_test.gomermaid/mindmap/mindmap.gomermaid/mindmap/mindmap_test.gomermaid/requirement/examples_test.gomermaid/requirement/requirement_diagram.gomermaid/requirement/requirement_diagram_test.gomermaid/state/examples_test.gomermaid/state/state_diagram.gomermaid/state/state_diagram_test.goscripts/mermaid-check/check.mjsscripts/mermaid-check/package.jsonscripts/mermaid-check/selftest.mjsscripts/mermaid-check/testdata/frontmatter-comment-title.mdfixturescripts/mermaid-check/testdata/frontmatter-unquoted-colon.mdfixture
🚧 Files skipped from review as they are similar to previous changes (7)
- doc/block/generated.md
- mermaid/block/block_diagram.go
- mermaid/block/block_diagram_test.go
- mermaid/block/examples_test.go
- scripts/mermaid-check/package.json
- scripts/mermaid-check/check.mjs
- README.md
|
Escape YAML control characters in I checked both candidates by generating the quoted form in Go and loading it back with
Regression cases added for control characters with a shorthand, control characters without one, and printable non-ASCII, which has to survive unescaped. |
Code Metrics Report
Details | | main (ba85e83) | #117 (aa8717b) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 95.1% | 95.1% | +0.0% |
| Files | 48 | 49 | +1 |
| Lines | 2546 | 2548 | +2 |
+ | Covered | 2423 | 2425 | +2 |
- | Test Execution Time | 7s | 10s | +3s |Code coverage of files in pull request scope (93.8% → 93.8%)
Reported by octocov |
1 participant
Summary
The block diagram builder emitted its title as a
title Xstatement inside the diagram body, butblockhas no title statement in mermaid: the line is read as a row, so the README drew stray blocks labelled "title", "Checkout" and "Architecture" above the real diagram. The title now goes in the front matter, and the render check that was supposed to catch this is extended so the whole class of failure fails CI.Changes
mermaid/block/block_diagram.go: emit the title as front matter (---/title: X/---) ahead of theblockkeyword, the way every other builder here does it.mermaid/block/config.go,mermaid/mindmap/config.go,mermaid/kanban/config.go: document onWithTitlethat mermaid's renderer for these three diagram types keeps the title as metadata and never draws it.README.md,doc/block/generated.md,mermaid/block/*_test.go: the regenerated output and the expectations that pin it.scripts/mermaid-check/check.mjs: draw every committed diagram with the real mermaid renderer in headless Chrome, on top of the existing parse, and report a declared title that never reaches the drawing.scripts/mermaid-check/selftest.mjs,scripts/mermaid-check/testdata/: fixtures that assert the checker still catches each failure it is meant to catch..github/workflows/doc_render.yml,Makefile,CONTRIBUTING.md: run the renderer and its self test, with the puppeteer Chrome download cached.Design Decisions
WithTitleis kept rather than removed. Front matter is valid mermaid and is what the rest of this repository emits, so the option keeps working the day mermaid teaches the block renderer to draw a title, and today it at least stops corrupting the diagram instead of quietly dropping the call on the floor.Parsing was never going to catch this.
title Checkout Architectureis a perfectly valid block row, somermaid.parseaccepted it and the diagram shipped drawing the wrong thing. The check now renders in a browser and compares a declared title against the text that actually lands in the SVG, which is what turned up mindmap and kanban as the same shape of problem. Diagram types whose renderer draws no title at all are listed explicitly, and for those atitlestatement (as opposed to front matter) is reported, because that is the form that becomes content.The fixtures use a
.mdfixtureextension so that files which are broken on purpose cannot be swept into the repository-widegit ls-files '*.md'run.Limitations
Front matter titles on
block,mindmapandkanbandiagrams are still invisible to readers. That is mermaid's renderer, not this package, and the godoc now says so.Summary by CodeRabbit