fix(ci): unblock pre-PR markdown lint baseline on main (#1837)#2133
Conversation
…o siblings The pre-PR runner failed Markdown Linting on a pristine main. Regenerated Copilot CLI skills under src/copilot-cli/skills/** carried 403 MD040/MD041/MD036 violations while their source counterparts under .claude/skills/** were excluded from lint scope. That asymmetry trained contributors to ignore baseline failures (Issue #1837). Root cause: build/scripts/generate_skills.py copies .claude/skills/ verbatim into src/copilot-cli/skills/ (its module docstring line 2: "Generate Copilot CLI skill artifacts from .claude/skills/ (REQ-003-001)", mode directory-copy). The two trees hold the same plugin-class content, but only the .claude side was lint-excluded. Changes: - Exclude src/copilot-cli/skills/** the same way .claude/skills/** is excluded (PR #331 treats skills as third-party plugins). Removes 403 violations. - Set MD024 siblings_only true so docs/installation.md can reuse "Claude Code" and "GitHub Copilot CLI" sub-headings under distinct parent sections, which is intentional and readable. Verified empirically against markdownlint v0.40.0: with siblings_only true, two "### Foo" under different "##" parents pass; with the default false the second is flagged. Add tests/validation/test_markdownlint_config.py to pin both invariants so a future config edit cannot silently drop them and reintroduce the baseline failure. Five tests, all passing. Fixes #1837 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three authored markdown files carried baseline markdownlint violations that pre_pr.py surfaced on a pristine main (Issue #1837): - README.md: two standalone bold platform labels ("Claude Code", "GitHub Copilot CLI") in the Quick Install section triggered MD036 (emphasis-as-heading). Added the trailing colon to match the six sibling labels already using the "**Claude Code:**" style, which markdownlint does not flag. - evals/README.md and evals/reviewer-asymmetry-spike/README.md: the Layout directory-tree fences were bare (MD040). Tagged them as text. Fixes #1837 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three agent prompts carried bare code fences (MD040) that pre_pr.py surfaced on a pristine main (Issue #1837): - spec-generator: the 3-tier-output and EARS-syntax illustrative fences were bare. Tagged both as text in the two install copies that carry the file (.claude/agents/ and src/claude/); the two stay byte-identical so install parity holds. - roadmap and milestone-planner: the RICE/KANO and dependency-graph fences in the .claude/agents/ self-host copies were bare. Resynced both from the canonical src/claude/ copies, which already carried the text-fence fix (and, for roadmap, the comma-for-em-dash fix in the description). Only those two lines changed; the rest of each file is unchanged. Fixes #1837 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR Validation ReportCaution ❌ Status: FAIL Description Validation
PR Standards
QA Validation
|
There was a problem hiding this comment.
Code Review
This pull request adds language identifiers to fenced code blocks across several Markdown files, updates the markdownlint configuration to scope duplicate heading checks to siblings and exclude Copilot CLI skills, and introduces a regression test suite to verify these linting rules. The reviewer feedback points out that several modified files in .claude/agents/ and src/claude/ are either generated or hand-maintained copies that must be updated via their corresponding templates in templates/agents/ to ensure synchronization and prevent changes from being overwritten.
Spec-to-Implementation ValidationCaution ❌ Final Verdict: FAIL What is Spec Validation?This validation ensures your implementation matches the specifications:
Validation Summary
Spec References
Requirements Traceability DetailsVERDICT: CRITICAL_FAIL Implementation Completeness DetailsVERDICT: CRITICAL_FAIL Run Details
Powered by AI Spec Validator workflow |
AI Quality Gate ReviewWarning WalkthroughThis PR was reviewed by six AI agents in parallel, analyzing different aspects of the changes:
Review Summary
💡 Quick Access: Click on individual agent jobs (e.g., "🔒 security Review", "🧪 qa Review") in the workflow run to see detailed findings and step summaries. Action Required@rjmurillo, this PR has findings that need your attention:
Please review the agent findings above and push fixes or reply with justification. Security Review DetailsVERDICT: CRITICAL_FAIL QA Review DetailsVERDICT: CRITICAL_FAIL Analyst Review DetailsVERDICT: CRITICAL_FAIL Architect Review DetailsVERDICT: CRITICAL_FAIL DevOps Review DetailsVERDICT: CRITICAL_FAIL Roadmap Review DetailsVERDICT: CRITICAL_FAIL Run Details
Powered by AI Quality Gate workflow |
There was a problem hiding this comment.
Pull request overview
This PR unblocks the pre-PR validation baseline on main by fixing Markdownlint failures caused by generated Copilot CLI skill copies, and by pinning the key lint configuration invariants with a regression test.
Changes:
- Updated
.markdownlint-cli2.yamlto (a) excludesrc/copilot-cli/skills/**like.claude/skills/**, and (b) configureMD024withsiblings_only: trueto allow repeated sub-headings under different parent sections. - Fixed remaining MD040/MD036 violations by tagging previously untyped fenced blocks as
textand tightening standalone bold labels inREADME.md. - Added
tests/validation/test_markdownlint_config.pyto prevent config drift that would reintroduce baseline lint failures.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
.markdownlint-cli2.yaml |
Excludes Copilot CLI skill copies from lint scope, configures MD024 siblings_only to avoid false positives in installation docs headings. |
tests/validation/test_markdownlint_config.py |
Adds regression tests that pin the lint exclusion and MD024 behavior, plus guards against disabling MD040/MD041. |
README.md |
Fixes MD036 by making standalone bold platform labels end with a colon. |
evals/README.md |
Fixes MD040 by adding a text language tag to the layout fence. |
evals/reviewer-asymmetry-spike/README.md |
Fixes MD040 by adding a text language tag to the layout fence. |
src/claude/spec-generator.md |
Fixes MD040 by tagging previously untyped fences as text. |
.claude/agents/spec-generator.md |
Mirrors the text-tagged fences fix for the Claude Code agent copy. |
.claude/agents/roadmap.md |
Fixes MD040 by tagging an untyped fence as text, and removes a prohibited dash character in the description line. |
.claude/agents/milestone-planner.md |
Fixes MD040 by tagging the dependency-graph fence as text. |
The branch-wide em/en-dash scan in pre_pr.py (validate_dash_prohibition) checks every *.md file changed on the branch, not just newly added ones. Once the markdown fixes for Issue #1837 put these files into the branch diff, three pre-existing U+2014/U+2013 occurrences started failing the gate: - README.md:140: the roadmap agent description quoted in the install table used an em-dash ("product—strategic"). Replaced with a comma to match the canonical src/claude/roadmap.md wording. - evals/reviewer-asymmetry-spike/README.md: the Acceptance bullet wrapped onto a continuation line that began with an en-dash. Merged the continuation into the bullet so it reads as one sentence with no leading dash. - spec-generator (both install copies): the intro and the 3-tier-output fence used U+2014 em-dashes as flow connectors. Replaced " -- " connectors with " -> " ASCII arrows. The two copies stay byte-identical so install parity holds. .claude/rules/universal.md MUST NOT entry 5 prohibits these characters. Fixes #1837 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: rjmurillo <6811113+rjmurillo@users.noreply.github.com>
Co-authored-by: rjmurillo <6811113+rjmurillo@users.noreply.github.com>
Resolved and pushed. Merge conflicts are fixed in commit |
Summary
Fixes the baseline failures the pre-PR runner reported on a pristine
main. On a clean checkout,.venv/bin/python scripts/validation/pre_pr.pywent from 2 failed / 9 passed to 1 failed / 10 passed. The remaining failure (merge-resolver agent drift) is a separate content/governance decision and is left for a follow-up; see "Out of Scope" below.Baseline reproduction (clean main, HEAD 1970ad1)
The issue body listed 6 failures, but 5 of those (the PowerShell validators) are already handled on current
main: 4 expunged validators report[SKIP]viaMissingScriptSkip, and agent drift runs through the Python port. The two real failures were:Background
403 of the 420 violations were under
src/copilot-cli/skills/**. That tree is a whole-tree copy of.claude/skills/produced bybuild/scripts/generate_skills.py(its module docstring, line 2: "Generate Copilot CLI skill artifacts from .claude/skills/ (REQ-003-001)", modedirectory-copy)..claude/skills/**is lint-excluded (third-party plugins, PR #331); the copy was not. That asymmetry is exactly what the issue describes, and it trains contributors to ignore baseline failures.Changes
.markdownlint-cli2.yaml:src/copilot-cli/skills/**the same way.claude/skills/**is excluded. Removes 403 violations.MD024: siblings_only: trueso the installation guide can reuse the "Claude Code" / "GitHub Copilot CLI" sub-headings under distinct parent sections (Installation Methods / Uninstallation / Troubleshooting). Verified empirically against markdownlint v0.40.0.README.md: two standalone bold platform labels triggered MD036; added the trailing colon to match the sibling**Claude Code:**style.spec-generator(both install copies, kept byte-identical): tagged the 3-tier-output and EARS-syntax fences astext..claude/agents/roadmap.md,.claude/agents/milestone-planner.md: resynced from the canonicalsrc/claude/copies, which already carried thetext-fence fix (and, for roadmap, a comma-for-em-dash fix in the description).tests/validation/test_markdownlint_config.py(5 tests) pins the exclusion and the MD024 setting so a future config edit cannot silently reintroduce the baseline failure.Verification
npx markdownlint-cli2 "**/*.md"exits 0 (zero violations);--fixproduces no unintended changes to copilot/skill files..venv/bin/python scripts/validation/pre_pr.py: 1 failed / 10 passed (was 2 failed / 9 passed). Markdown Linting now PASSES..venv/bin/python -m pytest tests/validation/test_markdownlint_config.py tests/validation/test_pre_pr.py tests/build_scripts/test_install_parity.py: 47 passed..venv/bin/python build/generate_agents.py --validate: PASSED. Install Parity: OK.Out of Scope: merge-resolver agent drift
The Agent Drift Detection failure is genuine and pre-existing, independent of this change.
src/claude/merge-resolver.mdis a 289-line hand-authored Claude doc (16 sections);src/vs-code-agents/merge-resolver.agent.mdis a 137-line template-generated doc (6 sections).generate_agents.py --validatePASSES, so the generated side is correct; the drift is that the Claude copy was authored independently of the shared template. Reconciling them is a content/governance call (Ask First perAGENTS.md): either rewritetemplates/agents/merge-resolver.shared.mdto match the rich Claude content (changes 4 generated files, needs architect plus agent-template owners), or quarantine merge-resolver from the drift gate via a governance ADR. Out of scope for this lint-baseline fix.Fixes #1837
🤖 Generated with Claude Code