fix(docs): cover contributor docs in MDX check; catch autolinks#1151
Conversation
Closes #1148. The MDX safety check in tools/check-docs-mdx only scoped docs/user/ and docs/integrator/. Since PR #1143 added docs/contributor/ to the Fern sidebar, contributor docs are now MDX-rendered but were not covered by the safety check. docs/contributor/validator.md slipped through with an autolink (`<https://...>`) that breaks the MDX parser on the `://` sequence. Changes: - Expand `SEARCH_DIRS` in `tools/check-docs-mdx` to include `docs/contributor/`. - Add a fourth check: autolinks (`<https://...>` / `<http://...>`) outside fenced and inline code. MDX tries to parse them as JSX tags and chokes on the `://`. The fix is markdown link syntax `[text](url)`. - Replace the offending autolink in `docs/contributor/validator.md:461` with a `[Chainsaw assert reference](url)` markdown link. The 13 other `<word>` patterns the issue lists in validator.md (e.g., `<phase>`, `<name>`) are inside inline code spans, which MDX treats as literal `<code>` content — they are not parser-breakers and intentionally remain (the placeholder convention is shared across docs, including ones already in the user/integrator scope). The new autolink check skips inline-code spans for the same reason. Verified: - `bash tools/check-docs-mdx` — green - `make lint` — green - Synthetic `<https://...>` test file flagged correctly
📝 WalkthroughWalkthroughThis PR extends the MDX validation script (tools/check-docs-mdx) to detect and flag autolinks (http://... / https://...) as MDX safety violations, consolidates bare-brace and HTML-comment checks into a single fenced-code-aware awk pass that strips inline code spans before matching, and adds autolink → markdown conversion guidance to the common-fixes help text. It also clarifies the Chainsaw health-check documentation in docs/contributor/validator.md to require an existence guard before phase assertions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tools/check-docs-mdx`:
- Around line 61-79: The HTML comment check and related prints still use the
original $0 instead of the stripped variable line, causing false positives for
inline code; update the rule matching and error output to use the 'line'
variable (e.g. change the '/<!--/' test to 'line ~ /<!--/' and use 'line' in the
corresponding printf) and similarly ensure any other checks in this block that
should operate on the stripped content (like the autolink and printf calls) use
'line' for matching and printing instead of $0.
- Around line 66-68: The current AWK pattern using line ~ /[^\\]{/ misses a bare
"{" at column 1; update the condition (the one currently written as line ~
/[^\\]{/) to match either start-of-line or a non-backslash before "{" (for
example use a regexp like (^|[^\\]){ ) so lines beginning with "{" are caught as
well as cases with a preceding non-escape character; keep the surrounding printf
and message unchanged.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: f8788f7d-a047-4121-95b6-4f584e87edd9
📒 Files selected for processing (2)
docs/contributor/validator.mdtools/check-docs-mdx
Coverage Report ✅
Coverage BadgeNo Go source files changed in this PR. |
|
LGTM — scope expansion, shared inline-code stripping, and the new autolink check all look correct. |
Both fixes were pre-existing bugs that the refactor preserved; the
review caught them as part of the new check pass:
- HTML comment rule was matching `$0` instead of the stripped `line`,
which would false-positive on inline-code like `` `<!-- note -->` ``.
Now matches `line ~ /<!--/` consistently with the bare-{ and
autolink rules.
- Bare `{` regex `[^\\]{` required a preceding character, so `{foo}`
in column 1 slipped through. Changed to `(^|[^\\])\{` so the
start-of-line case is caught.
Verified with a synthetic test file under `docs/contributor/`:
- bare HTML comment outside backticks → flagged
- inline-code HTML comment → not flagged
- bare `{` at column 1 → flagged
- inline-code `{foo}` → not flagged
`make lint` green.
|
Addressed both CodeRabbit findings in 38bd0e0:
Both were pre-existing bugs that survived the refactor; the new check pass surfaced them. Verified with a synthetic test file: bare HTML comment and bare |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/check-docs-mdx (1)
20-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate Makefile help text to include the autolinks MDX check.
tools/check-docs-mdxdocuments Check 4 for autolinks, but themake helpdescription forcheck-docs-mdx(Makefile line 140) omits “autolinks”, listing only void elements, bare braces, and HTML comments.📝 Suggested Makefile update
check-docs-mdx: ## Checks docs/ markdown for MDX compatibility (void elements, bare braces, HTML comments, autolinks)🤖 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 `@tools/check-docs-mdx` around lines 20 - 21, Update the Makefile help text for the check-docs-mdx target so it mentions the autolinks MDX check that tools/check-docs-mdx implements; edit the help string for the Makefile target named check-docs-mdx (the target's comment/description used by the help task) to include "autolinks" alongside the existing "void elements, bare braces, HTML comments" so the help line reads like: "Checks docs/ markdown for MDX compatibility (void elements, bare braces, HTML comments, autolinks)".
🤖 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.
Outside diff comments:
In `@tools/check-docs-mdx`:
- Around line 20-21: Update the Makefile help text for the check-docs-mdx target
so it mentions the autolinks MDX check that tools/check-docs-mdx implements;
edit the help string for the Makefile target named check-docs-mdx (the target's
comment/description used by the help task) to include "autolinks" alongside the
existing "void elements, bare braces, HTML comments" so the help line reads
like: "Checks docs/ markdown for MDX compatibility (void elements, bare braces,
HTML comments, autolinks)".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 707b9cd2-5811-40af-937f-3ba767c1545e
📒 Files selected for processing (1)
tools/check-docs-mdx
Summary
Closes #1148.
Expand `tools/check-docs-mdx` to scope `docs/contributor/` and add a new check for markdown autolinks (`https://...` / `http://...`) that break the MDX parser on the `://`. Fix the one offending autolink in `docs/contributor/validator.md:461` that slipped through when #1143 added contributor docs to the Fern sidebar.
Motivation/Context
PR #1143 added `docs/contributor/` to the Fern sidebar, so contributor docs are now MDX-rendered. But the safety check in `tools/check-docs-mdx` only scoped `docs/user/` and `docs/integrator/`. `validator.md:461` had a `https://kyverno.github.io/...` autolink that broke MDX parsing with:
```
Failed to parse ../docs/contributor/validator.md: Unexpected character `/` (U+002F) before local name
```
Fixes: #1148
Type of Change
Bug fix (tooling + docs).
Implementation Notes
Three changes:
On the issue's listed 14 `` patterns: the other 13 (``, ``, ``, ``) are all inside inline code spans (`` ` ``), which MDX treats as literal `
` content. They are not parser-breakers — the same placeholder convention is used in `docs/user/cli-reference.md` and `docs/integrator/automation.md`, both of which are MDX-rendered and pass. The new check follows the issue's own criterion — "outside code fences/inline code" — so it strips inline code spans before scanning, matching the existing braces/comments checks.Testing
Risk Assessment
Low. Tightens a CI check and fixes one doc line; no runtime impact.
Checklist