Skip to content

fix(docs): cover contributor docs in MDX check; catch autolinks#1151

Merged
mchmarny merged 2 commits into
mainfrom
fix/mdx-check-contributor-scope
Jun 2, 2026
Merged

fix(docs): cover contributor docs in MDX check; catch autolinks#1151
mchmarny merged 2 commits into
mainfrom
fix/mdx-check-contributor-scope

Conversation

@mchmarny

@mchmarny mchmarny commented Jun 2, 2026

Copy link
Copy Markdown
Member

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:

  1. `SEARCH_DIRS` in `tools/check-docs-mdx` now includes `docs/contributor/`.
  2. New Check 4 flags autolinks outside fenced code and inline code. The actual MDX-breaker class — once that fires, the `://` confuses the JSX tag parser.
  3. `docs/contributor/validator.md:461` autolink rewritten as a markdown link.

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

  • `make lint` passes
  • New check verified to catch the target pattern
  • Existing checks still pass on user/integrator docs
  • Commit signed

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
@mchmarny mchmarny requested a review from a team as a code owner June 2, 2026 18:39
@mchmarny mchmarny self-assigned this Jun 2, 2026
@github-actions github-actions Bot added the size/S label Jun 2, 2026
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This 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

documentation

Suggested reviewers

  • njhensley
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and specifically addresses the two main changes: expanding MDX check to contributor docs and catching autolinks that break parsing.
Description check ✅ Passed The description clearly explains the changeset, including motivation (PR #1143 added contributor docs but check didn't cover them), the specific implementation (three changes), and comprehensive testing and risk assessment.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mdx-check-contributor-scope

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between def5025 and 309d151.

📒 Files selected for processing (2)
  • docs/contributor/validator.md
  • tools/check-docs-mdx

Comment thread tools/check-docs-mdx
Comment thread tools/check-docs-mdx Outdated
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Coverage Report ✅

Metric Value
Coverage 76.3%
Threshold 75%
Status Pass
Coverage Badge
![Coverage](https://img.shields.io/badge/coverage-76.3%25-green)

No Go source files changed in this PR.

@pdmack

pdmack commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

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.
@mchmarny

mchmarny commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Addressed both CodeRabbit findings in 38bd0e0:

  • HTML comment check on $0 instead of line (tools/check-docs-mdx:79) — fixed by matching line ~ /<!--/ so the rule is consistent with the bare-{ and autolink checks and no longer false-positives on inline-code `<!-- note -->`.
  • Bare { regex missed column 1 (tools/check-docs-mdx:68) — changed [^\\\\]{ to (^|[^\\\\])\\{ so {foo} at the start of a line is caught.

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 { at col 1 both flag, while inline-code variants of each are correctly skipped. make lint green.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Update Makefile help text to include the autolinks MDX check.

tools/check-docs-mdx documents Check 4 for autolinks, but the make help description for check-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

📥 Commits

Reviewing files that changed from the base of the PR and between 309d151 and 38bd0e0.

📒 Files selected for processing (1)
  • tools/check-docs-mdx

@mchmarny mchmarny enabled auto-merge (squash) June 2, 2026 19:37
@mchmarny mchmarny merged commit c606d17 into main Jun 2, 2026
33 checks passed
@mchmarny mchmarny deleted the fix/mdx-check-contributor-scope branch June 2, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MDX safety check does not cover contributor docs

3 participants