Run markdownlint-cli2 as a CLI instead of a blocked action - #10628
Run markdownlint-cli2 as a CLI instead of a blocked action#10628Jakub Jareš (nohwnd) wants to merge 3 commits into
Conversation
The microsoft organization's allowed-actions list only permits DavidAnson/markdownlint-cli2-action at @v18. Every bump past that is rejected before a job is created, so the run ends in startup_failure with no job, no check run and no logs. The workflow has produced 435 consecutive startup_failures since the action was bumped to v24.1.0, which means markdown has been unlinted on every pull request since 2026-07-25. Call the CLI from a run step with a pinned version. A run step is not subject to the allow-list, so a version bump can no longer silently disable the gate, and Dependabot no longer has an action reference to propose versions for. Fix the 5 markdown violations this exposes, and disable MD060 (table-column-style), a cosmetic pipe-padding rule added in markdownlint v0.41 that no table in the repository follows. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Restores Markdown linting by replacing the blocked action with the CLI.
Changes:
- Runs pinned
markdownlint-cli2vianpx. - Disables the repository-wide MD060 rule.
- Fixes RFC formatting and link violations.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/markdownlint.yml |
Invokes the lint CLI directly. |
.markdownlint-cli2.jsonc |
Disables MD060. |
docs/RFCs/016-JUnit-Report.md |
Fixes code-span formatting. |
docs/RFCs/017-TestHost-Launcher.md |
Adds required heading spacing. |
docs/RFCs/020-Resource-Lock-Attribute.md |
Corrects a heading fragment. |
docs/RFCs/021-Per-Test-Temporary-Directory.md |
Corrects list formatting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Review Summary
This is a low-risk PR touching CI workflow configuration and documentation only. No source code, public API, or test changes.
Findings
| # | Dimension | Verdict |
|---|---|---|
| 1–6 | Correctness, Concurrency, Performance, API, Compat, IPC | N/A |
| 7 | Localization | N/A |
| 8–10 | Testing, Security, Cross-TFM | N/A |
| 11 | Documentation | ✅ (one minor nit below) |
| 12 | Scope | ✅ Single concern |
| 13–22 | Remaining dimensions | N/A |
Details
CI workflow (markdownlint.yml): Replacing the action with npx markdownlint-cli2@0.23.2 avoids the allow-list issue. Pinning to a specific version is good practice. ✅
.markdownlint-cli2.jsonc: Disabling MD060 with rationale is fine. ✅
020-Resource-Lock-Attribute.md anchor fix: Confirmed the target heading is ## Guidance (granularity), which GitHub slugifies to #guidance-granularity. The old anchor #granularity-guidance was broken; this fix is correct. ✅
016-JUnit-Report.md: Minor — see inline comment. The separator in .NET exception rendering is typically : (colon + space), so removing the space from the documented description may be slightly inaccurate. Low severity.
021-Per-Test-Temporary-Directory.md: Line rewrap only, no semantic change. ✅
017-TestHost-Launcher.md: Blank line after heading for markdownlint compliance. ✅
Overall: clean PR, no blocking issues.
npx pinned only the top-level package, so markdownlint itself was resolved fresh on every run. That is how MD060 arrived unannounced. eng/markdownlint pins the whole graph and the workflow installs it with npm ci. Also restore the colon-space separator description in the JUnit RFC, which lost its trailing space when it was rewritten to satisfy MD038. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Fixed the permissions instead, there are more actions that try to keep running but don't finish. |
The lockfile was generated on a machine whose global npmrc points at an internal package proxy, so all 86 entries recorded that proxy's Azure DevOps URLs. The proxy also strips the upstream sha512 from its metadata, so npm fell back to sha1 for every package. That bound a public repository's CI, and every external contributor's npm ci, to an internal mirror, and left the integrity check resting on sha1 rather than sha512. Rewrite all 86 entries to registry.npmjs.org with sha512. Versions and the resolved graph are unchanged; each tarball was verified against the sha1 npm had recorded before being rehashed, and npm ci re-verifies the new hashes against npmjs on the runner. Add eng/markdownlint/.npmrc pinning the registry so regenerating the lockfile on a proxied machine fails instead of silently reintroducing this. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Superseded — closing. The maintainer fixed this at the source by allowing #10636 carries the part that was still missing — That leaves this PR with nothing worth keeping. The lockfile in Worth recording for anyone who hits
History for the record: #10163 bumped v18 to v24 and broke it, #10196 reverted to v18 inside an unrelated change and it worked again, #10214 re-bumped and broke it for 435 consecutive runs. #10581 can go ahead now that the action is allowed. 🤖 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- eng/markdownlint/package-lock.json: Generated file
Suppressed comments (2)
.github/workflows/markdownlint.yml:14
- The workflow does not run when
.npmrcchanges, even though that file controls the registry used by the newnpm cistep. A bad or redirected registry change could therefore merge without validating that the lint toolchain still installs; include it in the path filter with the manifest and lockfile.
- "eng/markdownlint/package.json"
- "eng/markdownlint/package-lock.json"
eng/markdownlint/.npmrc:8
- This last sentence is inaccurate: a project-level
.npmrcoverrides a global registry setting, so a machine configured for an internal proxy will normally regenerate successfully against npmjs rather than fail. The protection is that the proxy is not used; describe that behavior directly.
# here means such a machine fails loudly instead.
.github/workflows/markdownlint.ymlhas ended instartup_failureon 435 consecutive runs since 2026-07-25, with zero successes. No job is created, so there is no check run andruns/<id>/logsreturns 404 — the failure is invisible everywhere except the run page itself. Markdown has been unlinted on every pull request for over three weeks.The error
The startup error is only on the run page (run 31674560655), which is why nobody had it written down. Verbatim:
The organization's allowed-actions list permits this action at
@v18and nothing else. That matches the history exactly: #10163 bumped v18 → v24 and broke it, #10196 reverted to v18 as a side effect of an unrelated MSBuildCache change and it worked again, and #10214 re-bumped to v24.1.0 and broke it for good.The fix
Call the CLI from a
run:step instead of using the action. Arun:step is not subject to the allow-list, so the gate cannot be disabled again by a version bump, and Dependabot no longer has an action reference to propose versions for — pinning back to@v18would have left both of those failure modes in place.The CLI is installed from a committed lockfile in
eng/markdownlintrather than resolved at run time.npx markdownlint-cli2@0.23.2pins only the top-level package: the run log showed it pullingmarkdownlint v0.41.1transitively, which is exactly where the MD060 surprise below came from.npm ciagainst the lockfile pins the whole graph — all 86 packages resolved fromregistry.npmjs.orgwith sha512integrity— so an upstream release cannot add a rule that fails the lint on files nobody touched. It lives undereng/because the config already ignoreseng/**/*.md, which keepsnode_modulesout of the lint. Dependabot updates it weekly.This also means #10581 is unnecessary: it moves a broken 24.1.0 to an equally broken 24.2.0.
Lint fixes
Enabling the gate exposes 278 issues, 273 of which are
MD060/table-column-style— a cosmetic pipe-padding rule added in markdownlint v0.41 that no table in the repository follows. It is disabled in.markdownlint-cli2.jsoncalongside the existingMD013opt-out rather than reformatting 273 table cells across 6 RFCs. The 5 genuine violations (MD004,MD022,MD032,MD038,MD051— including a link fragment that pointed at a heading that does not exist) are fixed.The lockfile was first generated on a machine whose global npmrc points at an internal package proxy, so every entry recorded that proxy''s Azure DevOps URL, and because the proxy strips upstream sha512 from its metadata npm fell back to sha1 for all 86 packages. That would have bound this repository''s CI, and every external contributor''s
npm ci, to an internal mirror with the integrity check resting on sha1. All 86 entries are rewritten toregistry.npmjs.orgwith sha512; versions and the resolved graph are unchanged.eng/markdownlint/.npmrcpins the registry so regenerating on a proxied machine fails loudly instead of silently reintroducing it.Verified: the Markdownlint run on this PR installs from the lockfile against npmjs (
added 86 packages, and audited 87 packages, which re-checks every sha512) and reports0 issues in 0 filesacross 86 files;python .github/scripts/check_action_pins.pyexits 0 across 2272 references.🤖