Skip to content

fix: resolve markdownlint-cli2 directly instead of via npx (Windows) - #3459

Open
eleshar wants to merge 2 commits into
developfrom
fix/lint-md-staged-windows-npx
Open

eleshar wants to merge 2 commits into
developfrom
fix/lint-md-staged-windows-npx

Conversation

@eleshar

@eleshar eleshar commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Bugfix Pull Request

This repository enforces changelog, release, and label automation for all PRs and issues.
See the organisation-wide Automation Governance & Release Strategy for required rules.

Linked issues

Fixes #3455

Context

  • Severity/Impact: Medium — blocks every commit touching a markdown file on Windows unless bypassed with --no-verify (which skips the lint gate entirely, not just this bug).
  • Affected versions/environments: Windows only (confirmed not to affect the GitHub Actions Linux runners). scripts/validation/lint-md-staged.cjs, invoked by the .lintstagedrc.cjs pre-commit hook.

Reproduction

  • Steps: 1) Stage a change to any .md file. 2) git commit.
  • Expected vs Actual: Expected the commit to proceed (or fail on real lint violations). Actual: Failed to run markdownlint-cli2 --fix: spawnSync npx ENOENT on every attempt, regardless of the file's actual content.

Root Cause

scripts/validation/lint-md-staged.cjs called spawnSync("npx", [...]) with no shell: true. On Windows, npx is a .cmd shim, and Node's child_process.spawnSync does not resolve .cmd/.bat PATH shims without either shell: true or an explicit .cmd extension — a well-documented Node-on-Windows behaviour, not a PATH configuration issue. Confirmed by running npx markdownlint-cli2 --fix <file> directly in a shell — it resolves and runs fine; only the spawnSync call inside the script failed.

Fix Summary

Resolves the locally-installed markdownlint-cli2 binary directly instead of going through npx: reads the package's own package.json bin field (via fs, since require.resolve("markdownlint-cli2/package.json") is rejected by the package's exports field even though the file exists on disk) to locate its actual script, then invokes that directly with process.execPath. This avoids the shell-shim problem at the source — deterministic (the exact locally-installed version, no npm registry resolution) and needs no shell layer on any platform, rather than patching around it with shell: true (which would reintroduce shell-quoting risk for file paths with spaces on Windows).

Verification

  • Tests added/updated to cover the bug — no existing test suite covers this script's npx invocation directly (it's invoked as a git hook, not imported as a module); verified via direct invocation and via a real git commit exercising the actual pre-commit hook path (see below).
  • Manual verification steps: ran node scripts/validation/lint-md-staged.cjs <file> directly — no more ENOENT, linter runs and reports correctly. Also verified end-to-end: staged a real CHANGELOG.md change and ran git commit — the pre-commit hook completed successfully for the first time on this Windows machine, without --no-verify.
  • Negative/edge cases checked: confirmed markdownlint-cli2's own CLI output (violation counts, summary) is unchanged from the npx-based invocation — only the process-spawning mechanism changed, not the tool's behaviour.

Risk & Rollback

  • Risk level: Low — changes how one local tool is invoked, not what it does. No shell interpretation in either the old or new invocation (both pass arguments as an array, no shell: true), so no new injection surface.
  • Rollback plan: revert this commit.

Changelog

Fixed

  • Windows Markdown Lint Commits Fixed — Committing markdown changes on Windows no longer fails; the linter is now resolved directly instead of through a command that Windows could not locate. (chore: Update branch validation metrics #3457)

Checklist (Global DoD / PR)

  • All AC met and demonstrated
  • Tests added/updated (unit/E2E as appropriate) — no existing unit test suite for this script; verified via direct invocation and a real end-to-end commit instead (see Verification above).
  • Accessibility checklist completed (where relevant): N/A — no user-facing UI, Node.js tooling script.
  • Docs/readme/changelog updated (if user-facing) — changelog entry above.
  • Security checklist completed (where relevant): N/A — no untrusted input, no shell interpretation added or removed, /security-review run with no findings.
  • Code/design reviews approved — pending human review.
  • CI green; linked issues closed; release notes prepared (if shipping) — pending CI.

🤖 Generated with Claude Code

scripts/validation/lint-md-staged.cjs called spawnSync("npx", [...])
with no shell: true, which fails with ENOENT on Windows -- npx is a
.cmd shim, and Node's spawnSync doesn't resolve .cmd/.bat PATH shims
without a shell. This blocked every commit touching a markdown file
on Windows unless bypassed with --no-verify.

Reads markdownlint-cli2's own package.json "bin" field via fs (not
require.resolve, since the package's "exports" field rejects that
subpath even though the file exists) to locate its actual script,
then invokes that directly via process.execPath. Deterministic (exact
locally-installed version, no npm registry resolution) and needs no
shell layer on any platform.

Fixes #3455
@eleshar
eleshar requested a review from ashleyshaw as a code owner September 22, 2026 12:18
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 50 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: lightspeedwp/.github/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: c5a92ac1-621b-4dae-8e2c-4b1595bbb2c4

📥 Commits

Reviewing files that changed from the base of the PR and between cca9d08 and 3043b08.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • scripts/validation/lint-md-staged.cjs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📋 Changelog Quality Validation

Summary

Metric Count
✅ Passing 79
⚠️ Warnings 0
❌ Failing 8
🆕 New failures in this PR 0
📦 Pre-existing failures 8

Status

Validation PASSED - No new failures introduced by this PR.
Note: 8 pre-existing failure(s) remain in the Unreleased section.

No action required.

@github-actions

Copy link
Copy Markdown
Contributor

PR Template Routing

Branch Type: fix
Scope: lint-md-staged-windows-npx
Template: pr_bug.md
Labels Applied: type:bug,area:ci

This PR was automatically routed based on the branch naming strategy.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: pre-commit markdown lint hook fails on Windows (spawnSync npx ENOENT)

1 participant