fix(ci): make the markdownlint job lint files again (#4526) - #4584
SurefireStudios wants to merge 3 commits into
Conversation
The globs were written inside a YAML block scalar, so the quotes around `'**/*.md'` were passed through to markdownlint-cli2 verbatim. It looked for paths beginning with a literal quote, matched nothing, and exited 0. The job has reported success without reading a file since it was added. Unquoting the glob alone turns the job red: it then finds 116 files with 277 violations. So this also clears every one of them. Most were mechanical (blank lines around fences, lists and headings) and came from markdownlint's own --fix. Three groups needed judgment: - Identifiers that markdown reads as emphasis. `__init__.py`, `__SPECKIT_COMMAND_*__` and friends were being rendered as bold in CHANGELOG.md. Running --fix over them rewrites the text itself (`**init**.py`), so they are wrapped in code spans instead, matching how the same tokens are already written elsewhere in the repo. - Fenced blocks without a language. Tagged from their actual content: `markdown` for the blocks that are markdown output, `text` for directory trees, commit messages and log excerpts. - The pillar headings on the docs landing page. They must stay at h3 because main.css styles `.pillar-card h3`, so MD001 is suppressed there with a comment saying why. It is the only suppression added. No prose changed. Verified against markdownlint-cli2 0.23.2, the version the pinned action actually runs. Assisted-by: Claude (model: claude-opus-5, autonomous)
mnriem
added
the
triage-nice-to-have
There was a problem hiding this comment.
🟡 Changes recommended
The workflow fix lacks a regression test preventing the silent zero-match glob from returning.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 32/33 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
| with: | ||
| globs: | | ||
| '**/*.md' | ||
| **/*.md |
|
Thanks for identifying and investigating the zero-file lint problem. The fix needs a narrower scope: Markdown linting is intended for documentation only, not every Markdown file in the repository. Commands, skills, prompt templates, agent instructions such as Please rescope the lint configuration to an explicit documentation-only allowlist and remove the formatting edits outside that scope. Retain only the documentation corrections needed for those targets, then verify that the job checks the intended files. Drafted for @mnriem with assistance from GitHub Copilot (model: GPT-6 Astra; interactive comment drafting). |
mnriem
added
author-needs-rescope
Addresses review feedback: markdown linting is for documentation, so commands, skills, prompt templates, agent instructions (AGENTS.md) and .github/ content must stay outside the lint target set. The globs input is now an explicit documentation allowlist held in a job-level DOC_GLOBS variable: docs/**, the root project docs, and the top-level docs under integrations/, presets/ and workflows/. It matches 57 files. The formatting edits made outside that scope in the first pass are reverted, taking this from 33 changed files to 16 -- the workflow plus the documentation corrections the allowlist actually covers. Adds a guard step that expands the same DOC_GLOBS and fails when it matches nothing. That is the regression test for github#4526: the original '**/*.md' was quoted inside a YAML block scalar, so the quotes became part of the pattern, it matched zero files, and the job passed while linting nothing. Verified the guard reports 57 for the allowlist and 0 (job fails) for the original quoted glob. Also merges main and resolves the docs/index.md conflict in favour of main's rewritten hero paragraph; the MD049 fix this branch made there no longer applies to that text.
|
Thanks — rescoped as asked. The branch now carries the workflow fix plus documentation corrections only, down from 33 changed files to 16 (+111/−61). Explicit documentation-only allowlist. The That matches 57 files, 0 errors. Everything you named is outside it: Formatting edits outside that scope are reverted. All 17 out-of-scope files are byte-identical to Verifying the job checks the intended files. Added a guard step that expands the same This is the regression test for the underlying bug and for the point the Copilot reviewer raised. The root cause was that Also merged One judgement call worth confirming: nested preset READMEs ( |
The check summed matches across every glob and only failed at zero, so one stale entry among several still passed while leaving that part of the documentation unlinted -- the same silent narrowing as github#4526, just partial. Each glob is now checked on its own and every empty one is reported. The unmatched globs are collected in an array rather than a string: nullglob is on for this step, so re-expanding an unquoted list of globs that match nothing erases the list before it can be printed.
|
One more push: the glob check now fails per glob instead of on the total. As written it summed matches across every glob and only failed at zero, so one stale entry among several would still pass while leaving that part of the docs unlinted — the same silent narrowing as #4526, just partial. A list with a typo'd (It collects the unmatched globs in an array rather than a string, because Body updated to match the rescoped branch. |
Description
Fixes #4526.
.github/workflows/lint.ymlpassedglobs: '**/*.md' !extensions/**/*.mdwith the quotes inside the YAML block scalar, so markdownlint-cli2 received them as literal characters and matched nothing.The Lint run on
mainlogsFinding: '**/*.md' ...thenLinting: 0 files— the job has been green because it lints nothing.Scope
Per @mnriem's review, the globs are now an explicit documentation allowlist rather than everything in the repository.
Commands, skills, prompt templates,
AGENTS.mdand.github/content stay out: they are inputs to coding agents rather than prose, and markdownlint's fixes rewrite their syntax.That matters concretely — an unattended
--fixpass turned__init__.pyinto**init**.pyand__SPECKIT_COMMAND_*__into**SPECKIT_COMMAND***_, because markdownlint reads__as bold where we mean an identifier.The allowlist lives in a job-level
DOC_GLOBSvariable and coversdocs/**, the root project docs, and the top-level docs underintegrations/,presets/andworkflows/— 57 files.The formatting edits outside that set are reverted, taking this from 33 changed files to 16: the workflow plus the documentation corrections the allowlist actually covers.
The regression guard
A glob matching nothing makes markdownlint-cli2 exit 0, so the job passes having checked no files — which is the whole of #4526.
A guard step expands
DOC_GLOBSand fails if any glob matches nothing, naming it.It checks each glob rather than the total, because one stale entry among several still leaves that part of the documentation unlinted.
One suppression
docs/index.mdpillar headings stay at h3 with an MD001 disable, becausedocs/template/public/main.cssstyles.pillar-card h3including the per-card accent colours, so promoting them would break the landing page.Note
@prateekraawat1commented "take" on the duplicate #4527; I had already built this before noticing, so happy to close in their favour.Testing
uv run specify --helpuv sync && uv run pytestValidated with markdownlint-cli2 0.23.2, the version the pinned action bundles — I had first checked against 0.18.1, which would have passed locally and failed in CI.
The allowlist reports
Linting: 57 files/Summary: 0 issues, and the guard independently counts the same 57.Reverting the documentation corrections reproduces 24 issues across the 9 files they touch, so every retained edit is load-bearing rather than churn.
Guard behaviour, checked directly: the real list passes at 57;
'**/*.md'— the original #4526 glob — reports 0 and fails; and a list with one stale entry among several still matches 40 files but fails and names the stale glob.specify --helpand the sample-project path aren't exercised, because this changes markdown content and one workflow file, and touches no CLI code.AI Disclosure
Per CONTRIBUTING and AGENTS.md: made with AI assistance (Claude), working from my direction. Every claim above was verified by running the command named rather than asserted — including the token-level diff that caught the
__init__.pyrewrite, and the guard's failure cases.