Repository navigation
fix(ci): pr-template-policy.cjs minimatch crash blocks all PR template-format checks - #3701
Conversation
Root cause: e50ad81 introduced `const { minimatch } = require('minimatch')` in scripts/pr-template-policy.cjs, but minimatch is not listed in package.json (neither dependencies nor devDependencies). The .github/workflows/pr-template-format.yml workflow checks out main via actions/checkout and runs the script directly with no `npm ci`/`npm install` step. Because the workflow uses `pull_request_target` + plain checkout (no `ref:`), every PR — including PRs that don't touch this file — invokes main's broken script and the `Pull request template format` check fails with `Cannot find module 'minimatch'`. This blocks every open PR's check. Fix choice: replace minimatch with Node's built-in `path.matchesGlob` (stable since Node 22, required engine is `>=22.0.0`). The only minimatch usage was `minimatch(file, pattern, { matchBase: false, dot: true })` in allPathsAreTooling, against simple glob patterns (`**`, `*`, `*.md`, `requirements*.txt`, etc.) with no extglobs, brace-expansion alternation, or negation. path.matchesGlob handles all required cases including dot files, so no new dependency is needed and no workflow change is required. Verified: all 25 existing tests in tests/pr-template-policy.test.cjs pass, including the tooling carve-out, exempt-marker, and template-detection suites. Direct script invocation with CHANGED_FILES=.github/workflows/... produces the expected `skipped: tooling-paths` carve-out. This unblocks every open PR's `Pull request template format` check.
📝 WalkthroughWalkthroughPR replaces the external ChangesGlob Pattern Matcher Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/pr-template-policy.cjs`:
- Line 3: The code imports path.matchesGlob which exists only in Node >=22.5.0
but package engines.node allows >=22.0.0; update engines.node to ">=22.5.0" in
package configuration OR add a runtime fallback: detect if path.matchesGlob is
undefined and, in that case, require and use minimatch (or a small wrapper
function) to emulate matchesGlob behavior where the code currently calls
matchesGlob; reference the imported symbol matchesGlob and the fallback module
minimatch so you update the import/usage sites to call the fallback wrapper when
matchesGlob is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a422934-b0b3-48f8-90c1-43bdfa3c5b4a
📒 Files selected for processing (1)
scripts/pr-template-policy.cjs
| #!/usr/bin/env node | ||
|
|
||
| const { minimatch } = require('minimatch'); | ||
| const { matchesGlob } = require('path'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json engines.node =="
python - <<'PY'
import json
from pathlib import Path
p = Path("package.json")
if not p.exists():
print("package.json not found")
else:
data = json.loads(p.read_text())
print(data.get("engines", {}).get("node", "engines.node not set"))
PY
echo
echo "== workflows using setup-node / node-version / pr-template-policy =="
rg -n --iglob '.github/workflows/*.yml' --iglob '.github/workflows/*.yaml' \
'actions/setup-node|node-version|scripts/pr-template-policy\.cjs'
echo
echo "== Node docs check for matchesGlob version history =="
curl -s https://nodejs.org/download/release/latest-jod/docs/api/path.html \
| rg -n 'path\.matchesGlob|Added in: v22\.5\.0|Marking the API stable'Repository: gsd-build/get-shit-done
Length of output: 345
🏁 Script executed:
cat -n scripts/pr-template-policy.cjs | head -20Repository: gsd-build/get-shit-done
Length of output: 668
🏁 Script executed:
cat -n scripts/pr-template-policy.cjs | sed -n '140,155p'Repository: gsd-build/get-shit-done
Length of output: 750
🏁 Script executed:
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20Repository: gsd-build/get-shit-done
Length of output: 777
🌐 Web query:
Node.js path.matchesGlob added version release notes
💡 Result:
The path.matchesGlob(path, pattern) method was added to the Node.js path module in version 22.5.0 and 20.17.0 [1]. It allows developers to test whether a given path matches a specific glob pattern [2][1]. Key details regarding its release include: - Contribution: The feature was contributed by Aviv Keller [3]. - Initial Status: Upon introduction, it was marked as experimental (Stability: 1) [4][1]. - Stability Promotion: The API was later marked as stable in Node.js versions 24.8.0 and 22.20.0 [1].
Citations:
- 1: https://nodejs.org/api/path.html
- 2: path: add
matchesGlobmethod nodejs/node#52881 - 3: https://nodejs.org/en/blog/release/v20.17.0
- 4: nodejs/node@57b8b8e18e
🏁 Script executed:
grep -r "node-version\|setup-node" .github/workflows/ --include="*.yml" --include="*.yaml" -A 2 -B 2Repository: gsd-build/get-shit-done
Length of output: 11910
🏁 Script executed:
grep -r "pr-template-policy" . --include="*.yml" --include="*.yaml" --include="*.json" --include="*.js" --include="*.cjs" 2>/dev/nullRepository: gsd-build/get-shit-done
Length of output: 1135
🏁 Script executed:
cat .github/workflows/pr-template-format.ymlRepository: gsd-build/get-shit-done
Length of output: 5303
path.matchesGlob requires Node >=22.5.0, but engines.node declares >=22.0.0.
Line 3 imports an API added in Node 22.5.0. Developers and CI systems respecting engines.node will fail on Node 22.0–22.4. Either update engines.node to >=22.5.0 or add a compatibility fallback to minimatch for unsupported versions.
🧰 Tools
🪛 GitHub Actions: PR Template Format / 0_Pull request template format.txt
[error] 3-3: Node script failed with MODULE_NOT_FOUND: Cannot find module 'minimatch' (require at scripts/pr-template-policy.cjs:3:23).
🪛 GitHub Actions: PR Template Format / Pull request template format
[error] 3-3: Node script failed: Cannot find module 'minimatch' (MODULE_NOT_FOUND). Error occurs at require('minimatch') in scripts/pr-template-policy.cjs.
[error] Workflow step failed with exit code 1 due to unhandled module load error for 'minimatch'.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/pr-template-policy.cjs` at line 3, The code imports path.matchesGlob
which exists only in Node >=22.5.0 but package engines.node allows >=22.0.0;
update engines.node to ">=22.5.0" in package configuration OR add a runtime
fallback: detect if path.matchesGlob is undefined and, in that case, require and
use minimatch (or a small wrapper function) to emulate matchesGlob behavior
where the code currently calls matchesGlob; reference the imported symbol
matchesGlob and the fallback module minimatch so you update the import/usage
sites to call the fallback wrapper when matchesGlob is unavailable.
Fix PR
Linked Issue
Closes #3700
What was broken
scripts/pr-template-policy.cjs:3doesconst { minimatch } = require('minimatch');, butminimatchis not inpackage.json(neitherdependenciesnordevDependencies). The.github/workflows/pr-template-format.ymlworkflow checks out main and runs the script directly with nonpm ci/npm installstep, so every PR'sPull request template formatcheck crashes withMODULE_NOT_FOUND.Because the workflow uses
pull_request_target+actions/checkoutwithoutref:, it always runs main's code — so even PRs that don't touch the policy script fail this check until main is fixed.What this fix does
Replaces
const { minimatch } = require('minimatch')withconst { matchesGlob } = require('path')(Node built-in, stable since Node 22; project'sengines.nodeis>=22.0.0), and rewrites the one call siteminimatch(file, pattern, { matchBase: false, dot: true })asmatchesGlob(file, pattern). The function's public API (allPathsAreTooling(changedFiles, allowlist)) is unchanged. Two-line diff. No new dependency. No workflow change.Root cause
Commit
e50ad812 — fix(3696): pr-template enforcer recognises CI/tooling carve-out (#3697)added arequire('minimatch')toscripts/pr-template-policy.cjswithout addingminimatchtopackage.json, and without adding an install step to.github/workflows/pr-template-format.yml. The PR's checks passed at the time because, withpull_request_target+ plain checkout, the workflow on the introducing PR ran the base branch's (still-working) script — the breakage only surfaced on subsequent PRs once #3697 merged to main.Fix choice and reason
Two valid options:
minimatchtodependencies+ add- run: npm cito the workflow. Heavier — adds a transitive tree and an install step on every PR.path.matchesGlob. No new dep, no workflow change.Picked (B). The single
minimatchcall uses only simple glob patterns (**,*,*.md,requirements*.txt) — no extglobs, no brace-expansion-with-alternation, no negation. Node 22'spath.matchesGlobcovers every required case (including dot-files like.github/**, which is why the original passeddot: true). Verified against every pattern inTOOLING_PATH_ALLOWLIST.Testing
How I verified the fix
node --test tests/pr-template-policy.test.cjs→ 25/25 pass, covering the tooling carve-out paths (.github/workflows/*,docs/,package.json/package-lock.json), exempt-marker handling, default-template rejection, and all three typed templates (fix/enhancement/feature).CHANGED_FILES=".github/workflows/install-smoke.yml" PR_BODY="" AUTHOR_ASSOCIATION="MEMBER" node scripts/pr-template-policy.cjs→{"valid":true,"action":"pass","skipped":"tooling-paths"}(the carve-out PR fix(3696): pr-template enforcer recognises CI/tooling carve-out #3697 was meant to add — still works)gsd-test-summaryon randomized hostplex2→ 11830 passed, 0 failed.Regression test added?
tests/pr-template-policy.test.cjscovers the function behavior thoroughly; the bug is a missing-dep / runtime-load failure that any directrequireof the script catches. All 25 tests already exerciseallPathsAreToolingagainst every allowlist pattern, so they would catch any regression in the glob-matching swap. A separate test for "the file loads" would be duplicate coverage — the existing tests load the module on every run.Platforms tested
Runtimes tested
Checklist
Closes #3700— PR will be auto-closed if missingconfirmed-buglabel — pending maintainer triage (auto-labels arebug+needs-triage)npm testequivalent:node --test tests/pr-template-policy.test.cjs25/25; full docker harness 11830/0).changeset/fragment added if this is a user-facing fix — N/A, this is an internal CI policy script with no user-facing surfaceBreaking changes
None. The function signature and observable behavior of
allPathsAreTooling,evaluatePrTemplate, and every exported helper are identical. The only difference is the engine doing the glob match.Important caveat for the reviewer
This PR's own
Pull request template formatcheck will fail for the exact same reason every other open PR is currently failing — becausepull_request_targetruns main's (still-broken) script regardless of what's on the PR branch. The PR itself is correct; the check needs to be bypassed once for this PR to merge, after which the check will run from this fixed code on main and every subsequent PR will pass cleanly. After merge, please re-run the failing checks on the other open PRs (or push an empty commit) and they'll go green without any further changes.Summary by CodeRabbit