feat: add markdownlint validation hook and git pre-commit#71
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeff-roche The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThis PR introduces markdown linting infrastructure by adding a new lint script with multiple execution modes, configuring it as both a Git pre-commit hook and Claude Code stop hook, and documenting the required local setup step. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
106afab to
f77b090
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
scripts/lint-markdown.sh (1)
27-31: Guardjqparsing so malformed stdin doesn’t abort the hook path.With
set -e, Line 28 can terminate the script if stdin is not valid JSON, preventing the fallbackCWDlogic on Line 30-32.Suggested hardening
- if command -v jq &>/dev/null; then - CWD=$(echo "$INPUT" | jq -r '.cwd // empty') - fi + if command -v jq &>/dev/null; then + CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty' 2>/dev/null || true) + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-markdown.sh` around lines 27 - 31, The script currently runs jq on $INPUT and with set -e a JSON parse error can abort the script and skip the fallback CWD logic; change the block that sets CWD from INPUT (the command using jq and the variables INPUT and CWD) so jq failures are tolerated — e.g., run jq in a guarded conditional or append a safe fallback (use a conditional echo "$INPUT" | jq ... and check jq's exit status, or use the command substitution with "|| true") so a malformed stdin does not cause the script to exit and the existing fallback CWD assignment is still executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.json:
- Around line 27-52: The JSON contains duplicate "Stop" keys so the second block
overrides the first; fix .claude/settings.json by merging the two Stop hook
entries into a single "Stop" array (or remove the unwanted duplicate) and
preserve both hook objects (the one running ".claude/hooks/lint-markdown.sh" and
the one running "scripts/lint-markdown.sh") so neither hook is lost; ensure the
final file has only one "Stop" key containing both hook objects.
In `@docs/ai-best-practices/building-ai-tools/agents.md`:
- Around line 101-109: Update example usages of the sub-agent type by replacing
the underscored value general_purpose with the hyphenated standard
general-purpose wherever it appears in examples (e.g., the Agent blocks using
subagent_type=general_purpose and any other occurrences such as the example
referenced around line 173); specifically edit the subagent_type field in those
Agent examples to subagent_type=general-purpose to match the sub-agent type
table and ensure consistent, valid invocations.
In `@docs/ai-best-practices/building-ai-tools/hooks.md`:
- Line 72: The example for the SessionStart matcher uses an escaped alternation
`"startup\|resume"` which is inconsistent with the rest of the document and
incorrect for standard regex; update the matcher example for SessionStart to use
an unescaped pipe `"startup|resume"` so it matches the config example at line 29
and other matcher examples, ensuring consistency across the hooks.md
documentation.
In `@docs/ai-best-practices/building-ai-tools/mcp-servers.md`:
- Around line 61-66: Update the deprecated table entry that currently lists the
transport as "SSE": replace the transport name "SSE" with "Streamable HTTP" and
change its description from "Remote services with server-sent events" to
indicate "Remote services supporting HTTP streaming (Streamable HTTP) — use only
when stdio is not possible" (ensure the example/notes reference Remote MCP
endpoints and match the later usage of "Streamable HTTP" found in the document).
Locate the table row containing the "SSE" token and update the transport label
and description text to align with the current MCP specification and the
language used in lines referencing "Streamable HTTP".
In `@docs/ai-best-practices/README.md`:
- Around line 139-140: Replace the compound-adjective uses "Open source tools"
and "open source licenses" in the two lines with the hyphenated form
"open-source tools/models" and "open-source licenses" respectively so the
phrases read "open-source tools that are not models..." and "Red Hat prefers
models under genuine open-source licenses (e.g., Apache-2.0 Granite and Mistral
models)". Ensure both instances are updated for consistency.
In `@docs/ai-best-practices/security.md`:
- Line 35: Update the phrasing in the sentence that currently reads "**Check
upstream policies** before contributing AI-assisted code to open source
projects." to hyphenate the adjectival form "open-source" so it reads
"...open-source projects"; locate and edit the exact string "open source
projects" in the docs content (the bolded sentence starting with "Check upstream
policies") and replace it with "open-source projects" for consistency with the
docs.
In `@docs/ai-best-practices/tool-guides/local-agents.md`:
- Line 34: Change the phrase "open source" to the compound adjective
"open-source" in the three occurrences noted (the header line containing "**Red
Hat prefers models under genuine open source licenses:**" and the similar lines
at the other two occurrences) so the policy text uses "open-source" consistently
as a hyphenated modifier.
In `@scripts/lint-markdown.sh`:
- Around line 51-63: Change the script shebang to #!/usr/bin/bash and replace
the space-delimited FILES_TO_LINT string with a proper Bash array (use
FILES_TO_LINT=() and append with FILES_TO_LINT+=("$file") inside the while
loop); check the array emptiness with [ "${`#FILES_TO_LINT`[@]}" -eq 0 ] and run
markdownlint using the array expansion "${FILES_TO_LINT[@]}" so filenames with
spaces or leading '-' are safe, capturing output into LINT_OUTPUT and
propagating the exit status correctly instead of unconditionally exiting 0.
---
Nitpick comments:
In `@scripts/lint-markdown.sh`:
- Around line 27-31: The script currently runs jq on $INPUT and with set -e a
JSON parse error can abort the script and skip the fallback CWD logic; change
the block that sets CWD from INPUT (the command using jq and the variables INPUT
and CWD) so jq failures are tolerated — e.g., run jq in a guarded conditional or
append a safe fallback (use a conditional echo "$INPUT" | jq ... and check jq's
exit status, or use the command substitution with "|| true") so a malformed
stdin does not cause the script to exit and the existing fallback CWD assignment
is still executed.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 6adec4ff-f421-4c52-a2ed-e3bc27f0a058
📒 Files selected for processing (23)
.claude/settings.json.githooks/pre-commitdocs/ai-best-practices/README.mddocs/ai-best-practices/building-ai-tools/README.mddocs/ai-best-practices/building-ai-tools/agents.mddocs/ai-best-practices/building-ai-tools/guardrails.mddocs/ai-best-practices/building-ai-tools/hooks.mddocs/ai-best-practices/building-ai-tools/mcp-servers.mddocs/ai-best-practices/building-ai-tools/plugins.mddocs/ai-best-practices/building-ai-tools/skills.mddocs/ai-best-practices/building-ai-tools/structured-outputs.mddocs/ai-best-practices/security.mddocs/ai-best-practices/tool-guides/claude-code.mddocs/ai-best-practices/tool-guides/coderabbit.mddocs/ai-best-practices/tool-guides/context-management.mddocs/ai-best-practices/tool-guides/cursor.mddocs/ai-best-practices/tool-guides/local-agents.mddocs/ai-best-practices/tool-guides/vscode.mddocs/proposals/README.mddocs/proposals/TEMPLATE.mddocs/proposals/ec2-watchman.mddocs/proposals/slack-bot-eddie.mdscripts/lint-markdown.sh
209088e to
07cf0b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/lint-markdown.sh (1)
67-67:⚠️ Potential issue | 🟠 MajorUse
--before file arguments to avoid option-parsing edge cases.Line 67 should pass
--before"${FILES_TO_LINT[@]}"so filenames beginning with-are always treated as paths, not CLI flags.Suggested fix
-LINT_OUTPUT=$(npx markdownlint-cli "${FILES_TO_LINT[@]}" 2>&1) || LINT_EXIT=$? +LINT_OUTPUT=$(npx markdownlint-cli -- "${FILES_TO_LINT[@]}" 2>&1) || LINT_EXIT=$?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-markdown.sh` at line 67, The command invoking markdownlint-cli should pass a standalone "--" before the file list so paths beginning with "-" are not treated as options; update the invocation that sets LINT_OUTPUT (the line using npx markdownlint-cli and FILES_TO_LINT) to include "--" immediately before "${FILES_TO_LINT[@]}" and preserve the existing redirection and LINT_EXIT handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/lint-markdown.sh`:
- Around line 16-18: The script currently exits 0 when required tooling (npx or
markdownlint-cli) is missing which allows commits to bypass linting; update the
checks around the npx and markdownlint-cli availability (the blocks referencing
"if ! command -v npx" and the markdownlint-cli check later) so that when running
in pre-commit mode (detect via a PRE_COMMIT or similar env var) the script exits
non‑zero (e.g., exit 1) to block the commit, while retaining the existing
non-blocking behavior (exit 0) only when not in pre-commit mode.
---
Duplicate comments:
In `@scripts/lint-markdown.sh`:
- Line 67: The command invoking markdownlint-cli should pass a standalone "--"
before the file list so paths beginning with "-" are not treated as options;
update the invocation that sets LINT_OUTPUT (the line using npx markdownlint-cli
and FILES_TO_LINT) to include "--" immediately before "${FILES_TO_LINT[@]}" and
preserve the existing redirection and LINT_EXIT handling.
🪄 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: CHILL
Plan: Pro Plus
Run ID: cb9ae4ed-cd56-4654-921a-a79eb00bbcc4
📒 Files selected for processing (4)
.claude/settings.json.githooks/pre-commitCONTRIBUTING.mdscripts/lint-markdown.sh
✅ Files skipped from review due to trivial changes (3)
- .githooks/pre-commit
- CONTRIBUTING.md
- .claude/settings.json
|
/assign Can this get trimmed down just to the markdown lint and pre-commit hook? As discussed, let's put I can lgtm after the PR scope is trimmed. |
Yup! I based off the wrong branch.. let me fix that |
Single script (scripts/lint-markdown.sh) serves both contexts: - Claude Code Stop hook: reports lint errors via hookSpecificOutput - Git pre-commit hook: blocks commits with markdown lint errors Install git hook: git config core.hooksPath .githooks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
07cf0b1 to
e4f4deb
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/lint-markdown.sh (2)
67-67:⚠️ Potential issue | 🟡 MinorAdd
--before file arguments when invoking markdownlint-cli.Line 67 passes filenames directly; a filename starting with
-can be interpreted as an option.Suggested fix
-LINT_OUTPUT=$(npx markdownlint-cli "${FILES_TO_LINT[@]}" 2>&1) || LINT_EXIT=$? +LINT_OUTPUT=$(npx markdownlint-cli -- "${FILES_TO_LINT[@]}" 2>&1) || LINT_EXIT=$?#!/bin/bash set -euo pipefail # Verify the markdownlint invocation safely terminates option parsing. rg -n -C2 'markdownlint-cli' scripts/lint-markdown.shExpected result: invocation includes
markdownlint-cli -- "${FILES_TO_LINT[@]}".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-markdown.sh` at line 67, The markdownlint invocation in scripts/lint-markdown.sh (the command that assigns LINT_OUTPUT using npx markdownlint-cli and the FILES_TO_LINT array) should stop option parsing for filenames that begin with "-" by inserting a "--" before the file arguments; update the command that sets LINT_OUTPUT (and any similar invocations) to call npx markdownlint-cli -- "${FILES_TO_LINT[@]}", preserving the redirection and exit capture behavior (LINT_EXIT) and keeping the variable names LINT_OUTPUT and FILES_TO_LINT intact.
16-18:⚠️ Potential issue | 🟠 MajorFail closed in pre-commit mode when lint tooling is unavailable (Severity: medium).
Line 16–18 and Line 60–63 currently
exit 0even with--pre-commit. That allows commits to proceed without required markdown validation.Suggested fix
if ! command -v npx &>/dev/null; then + if [ "$PRE_COMMIT" = true ]; then + echo "npx is required for markdownlint pre-commit checks." >&2 + exit 1 + fi exit 0 fi @@ if ! npx --yes markdownlint-cli --version &>/dev/null; then + if [ "$PRE_COMMIT" = true ]; then + echo "markdownlint-cli is not available; cannot validate staged markdown." >&2 + exit 1 + fi exit 0 fi#!/bin/bash set -euo pipefail # Verify that tool-unavailable branches fail in pre-commit mode. rg -n -C3 'command -v npx|markdownlint-cli --version|PRE_COMMIT|exit 1|exit 0' scripts/lint-markdown.shExpected result: each tooling check block should include a
PRE_COMMIT=truepath that exits non-zero.As per coding guidelines, “Getting Started” requires the repo pre-commit hook that runs
markdownlinton staged.mdfiles and blocks commits when markdown lint errors are present.Also applies to: 60-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lint-markdown.sh` around lines 16 - 18, The script currently returns exit 0 when tooling (e.g., npx or markdownlint) is missing, which lets commits pass; update the tooling-check blocks (the `command -v npx` check and the `markdownlint-cli --version` block around lines ~60–63) to fail non‑zero when running in pre-commit mode by checking the PRE_COMMIT env var (e.g., if [ "${PRE_COMMIT:-}" = "true" ] then exit 1 else exit 0), so that when PRE_COMMIT=true the script exits 1 on missing tooling; reference the `command -v npx` and `PRE_COMMIT` symbols to locate and change those checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/lint-markdown.sh`:
- Line 67: The markdownlint invocation in scripts/lint-markdown.sh (the command
that assigns LINT_OUTPUT using npx markdownlint-cli and the FILES_TO_LINT array)
should stop option parsing for filenames that begin with "-" by inserting a "--"
before the file arguments; update the command that sets LINT_OUTPUT (and any
similar invocations) to call npx markdownlint-cli -- "${FILES_TO_LINT[@]}",
preserving the redirection and exit capture behavior (LINT_EXIT) and keeping the
variable names LINT_OUTPUT and FILES_TO_LINT intact.
- Around line 16-18: The script currently returns exit 0 when tooling (e.g., npx
or markdownlint) is missing, which lets commits pass; update the tooling-check
blocks (the `command -v npx` check and the `markdownlint-cli --version` block
around lines ~60–63) to fail non‑zero when running in pre-commit mode by
checking the PRE_COMMIT env var (e.g., if [ "${PRE_COMMIT:-}" = "true" ] then
exit 1 else exit 0), so that when PRE_COMMIT=true the script exits 1 on missing
tooling; reference the `command -v npx` and `PRE_COMMIT` symbols to locate and
change those checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4dcf6f96-52d4-46d0-a93f-731e86516f6b
📒 Files selected for processing (4)
.claude/settings.json.githooks/pre-commitCONTRIBUTING.mdscripts/lint-markdown.sh
✅ Files skipped from review due to trivial changes (3)
- .claude/settings.json
- .githooks/pre-commit
- CONTRIBUTING.md
|
/lgtm |
Summary
scripts/lint-markdown.sh— a single script that validates modified.mdfiles with markdownlinthookSpecificOutputso Claude can fix them.githooks/pre-commitwrapper that calls the shared scriptCONTRIBUTING.mdsection documenting how to enable the shared git hooksSetup
Install the git hook:
The Claude Code hook is configured automatically via
.claude/settings.json.How it works
.mdfiles viagit diffandgit ls-filesnpx markdownlint-clionly on changed files (not the whole repo).mdfiles are modified or whennpxis unavailableTest plan
.mdfile with a lint error, verify Claude Code reports it on Stop.mdfile with a lint error, verifygit commitis blocked.mdfile, verify no interference.mdfiles are changedSummary by CodeRabbit
Release Notes
New Features
Chores