fix: make markdown lint blocking for changed files#271
Conversation
- Modified check_markdown_lint() to track check_mode (changed vs all) - Changed files now return exit 1 (blocking) when violations found - All files fallback remains advisory (exit 0) - Simplified fallback when markdownlint not installed (advisory only) - Added MD031 reminder to AGENTS.md Quality line This prevents markdown lint issues like MD031 (blank lines around code blocks) from being merged, while not requiring cleanup of all pre-existing issues in the codebase.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the quality gates for markdown files by making linting a blocking requirement for any content modified within a pull request. This change ensures that new or updated markdown adheres to established style guidelines, proactively preventing common formatting errors like MD031 (blank lines around code blocks) from being introduced. The system maintains an advisory mode for the existing codebase, allowing for a focused approach on new contributions while providing flexibility for addressing legacy issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively makes markdown linting blocking for changed files while keeping it advisory for the full codebase, which is a great improvement to catch formatting issues early. The changes in linters-local.sh are well-implemented, especially the simplification of the fallback logic for when markdownlint is not installed. I have one suggestion to refactor a small portion of the script to improve readability and reduce a duplicated check.
.agent/scripts/linters-local.sh
Outdated
| if [[ "$check_mode" == "changed" ]]; then | ||
| print_error "Markdown: $violations style issues in changed files (BLOCKING)" | ||
| else | ||
| print_warning "Markdown: $violations style issues found (advisory)" | ||
| fi | ||
| echo "$lint_output" | head -10 | ||
| if [[ $violations -gt 10 ]]; then | ||
| echo "... and $((violations - 10)) more" | ||
| fi | ||
| print_info "Run: markdownlint --fix .agent/**/*.md to auto-fix" | ||
| # Non-blocking for now - many pre-existing issues | ||
| # TODO: Make blocking after fixing existing issues | ||
| print_info "Run: markdownlint --fix <file> to auto-fix" | ||
| # Blocking for changed files, advisory for all files | ||
| if [[ "$check_mode" == "changed" ]]; then | ||
| return 1 | ||
| fi | ||
| return 0 |
There was a problem hiding this comment.
The logic for handling violations can be simplified to improve readability and avoid checking the value of check_mode twice. You can first print the violation details (which are common to both modes) and then have a single if/else block to handle the mode-specific logic of printing the final message and returning the correct exit code.
| if [[ "$check_mode" == "changed" ]]; then | |
| print_error "Markdown: $violations style issues in changed files (BLOCKING)" | |
| else | |
| print_warning "Markdown: $violations style issues found (advisory)" | |
| fi | |
| echo "$lint_output" | head -10 | |
| if [[ $violations -gt 10 ]]; then | |
| echo "... and $((violations - 10)) more" | |
| fi | |
| print_info "Run: markdownlint --fix .agent/**/*.md to auto-fix" | |
| # Non-blocking for now - many pre-existing issues | |
| # TODO: Make blocking after fixing existing issues | |
| print_info "Run: markdownlint --fix <file> to auto-fix" | |
| # Blocking for changed files, advisory for all files | |
| if [[ "$check_mode" == "changed" ]]; then | |
| return 1 | |
| fi | |
| return 0 | |
| echo "$lint_output" | head -10 | |
| if [[ $violations -gt 10 ]]; then | |
| echo "... and $((violations - 10)) more" | |
| fi | |
| print_info "Run: markdownlint --fix <file> to auto-fix" | |
| if [[ "$check_mode" == "changed" ]]; then | |
| print_error "Markdown: $violations style issues in changed files (BLOCKING)" | |
| return 1 | |
| else | |
| print_warning "Markdown: $violations style issues found (advisory)" | |
| return 0 | |
| fi |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Jan 31 23:22:02 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🤖 Augment PR SummarySummary: Makes local Markdown linting block merges when style violations are introduced in changed Markdown files, while keeping full codebase scans advisory. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
.agent/scripts/linters-local.sh
Outdated
| if [[ $violations -gt 0 ]]; then | ||
| print_warning "Markdown: $violations style issues found" | ||
| if [[ "$check_mode" == "changed" ]]; then | ||
| print_error "Markdown: $violations style issues in changed files (BLOCKING)" |
There was a problem hiding this comment.
violations is derived via grep -c "MD[0-9]" || echo "0", which can yield a non-numeric value (e.g., 0 0) when markdownlint emits non-MD errors; that can make this new blocking path misbehave or hide a real markdownlint failure as a clean run. Consider ensuring violations is always a single integer and treating non-rule output from markdownlint as an error case.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
WalkthroughThis PR refactors markdown linting logic to distinguish between blocking and advisory modes, implementing three-stage file selection (uncommitted changes, branch diff, fallback to all tracked .md files). Additionally, quality documentation is updated to include MD031 formatting standards for code blocks. Changes
Sequence DiagramsequenceDiagram
participant Script as linters-local.sh
participant Git as Git Repository
participant FileSystem as File System
participant Linter as markdownlint
participant Output as Output Handler
Script->>Git: Check if uncommitted changes exist
alt Uncommitted files found
Git-->>Script: Return uncommitted .md files
Script->>Script: Set check_mode=blocking
Script->>FileSystem: Select uncommitted files
else No uncommitted files
Script->>Git: Get diff against main branch
alt Branch diff exists
Git-->>Script: Return differing .md files
Script->>Script: Set check_mode=blocking
else No branch diff
Script->>FileSystem: List all tracked .md files
Script->>Script: Set check_mode=advisory
end
end
Script->>Linter: Run markdownlint on selected files
alt markdownlint available
Linter-->>Script: Return violations (parsed count)
Script->>Script: Validate count is numeric
alt Violations found
Script->>Output: Show sample issues
end
else markdownlint unavailable
Script->>Script: Set check_mode=advisory
Script->>Output: Display installation suggestion
end
Script->>Output: Emit mode-aware message
alt check_mode=blocking
Output-->>Script: Return non-zero exit code
else check_mode=advisory
Output-->>Script: Return success (0)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
- Ensure violations count is always a valid integer - Reorder logic to show violations before mode-specific message - Single check_mode conditional for message and return code
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Jan 31 23:25:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agent/scripts/linters-local.sh (1)
398-431:⚠️ Potential issue | 🟠 MajorHandle markdownlint execution errors separately from rule violations.
When markdownlint itself fails (bad config, invalid arguments, syntax errors), the error output won't match the MD[0-9] rule pattern, causing
violation_count=0and the function to return 0 (success)—even in blocking mode. This bypasses the reliability requirement. Capture the tool's exit code and treat execution failures as blocking errors in changed-file mode and advisory warnings in full-scan mode.🔧 Suggested fix
- lint_output=$($markdownlint_cmd $md_files 2>&1) || true + local lint_exit=0 + lint_output=$($markdownlint_cmd $md_files 2>&1) || lint_exit=$? if [[ -n "$lint_output" ]]; then # Count violations - ensure single integer (grep -c can fail, use wc -l as fallback) local violation_count violation_count=$(echo "$lint_output" | grep -c "MD[0-9]" 2>/dev/null) || violation_count=0 # Ensure it's a valid integer if ! [[ "$violation_count" =~ ^[0-9]+$ ]]; then violation_count=0 fi violations=$violation_count if [[ $violations -gt 0 ]]; then # Show violations first (common to both modes) echo "$lint_output" | head -10 if [[ $violations -gt 10 ]]; then echo "... and $((violations - 10)) more" fi print_info "Run: markdownlint --fix <file> to auto-fix" # Mode-specific message and return code if [[ "$check_mode" == "changed" ]]; then print_error "Markdown: $violations style issues in changed files (BLOCKING)" return 1 else print_warning "Markdown: $violations style issues found (advisory)" return 0 fi + elif [[ $lint_exit -ne 0 ]]; then + print_error "Markdown: markdownlint failed (non-rule error)" + if [[ "$check_mode" == "changed" ]]; then + return 1 + else + return 0 + fi fi fi print_success "Markdown: No style issues found"
1 participant
Summary
Makes markdown lint checks blocking for changed files while keeping them advisory for the full codebase. This prevents markdown formatting issues (like MD031 - blank lines around code blocks) from being merged.
Changes
linters-local.sh: Modified
check_markdown_lint()to:check_mode("changed" vs "all")AGENTS.md: Added "blank lines around code blocks (MD031)" to Quality line as a reminder
Why
PR #269 introduced an MD031 violation that was caught by Codacy post-merge, requiring a follow-up fix PR #270. This change ensures such issues are caught during preflight checks.
Testing
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.