Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

fix(ci): pr-template-policy.cjs minimatch crash blocks all PR template-format checks - #3701

Merged
trek-e merged 1 commit into
mainfrom
fix/pr-template-policy-minimatch
May 18, 2026
Merged

trek-e merged 1 commit into
mainfrom
fix/pr-template-policy-minimatch

Conversation

@trek-e

@trek-e trek-e commented May 18, 2026 •

Copy link
Copy Markdown
Contributor

Fix PR

Using the typed fix template.


Linked Issue

Closes #3700

The linked issue has the bug + needs-triage labels (auto-applied via .github/ISSUE_TEMPLATE/bug_report.yml). Maintainer should add confirmed-bug before merging if that gate is enforced; the bug is trivially reproducible (every open PR's Pull request template format check is red with Cannot find module 'minimatch').


What was broken

scripts/pr-template-policy.cjs:3 does const { minimatch } = require('minimatch');, but minimatch is not in package.json (neither dependencies nor devDependencies). The .github/workflows/pr-template-format.yml workflow checks out main and runs the script directly with no npm ci / npm install step, so every PR's Pull request template format check crashes with MODULE_NOT_FOUND.

Because the workflow uses pull_request_target + actions/checkout without ref:, 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') with const { matchesGlob } = require('path') (Node built-in, stable since Node 22; project's engines.node is >=22.0.0), and rewrites the one call site minimatch(file, pattern, { matchBase: false, dot: true }) as matchesGlob(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 a require('minimatch') to scripts/pr-template-policy.cjs without adding minimatch to package.json, and without adding an install step to .github/workflows/pr-template-format.yml. The PR's checks passed at the time because, with pull_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:

  • (A) Add minimatch to dependencies + add - run: npm ci to the workflow. Heavier — adds a transitive tree and an install step on every PR.
  • (B) Use Node's built-in path.matchesGlob. No new dep, no workflow change.

Picked (B). The single minimatch call uses only simple glob patterns (**, *, *.md, requirements*.txt) — no extglobs, no brace-expansion-with-alternation, no negation. Node 22's path.matchesGlob covers every required case (including dot-files like .github/**, which is why the original passed dot: true). Verified against every pattern in TOOLING_PATH_ALLOWLIST.

Testing

How I verified the fix

  • Ran the full existing suite: 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).
  • Direct script invocation reproducing the workflow's env:
    • 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)
    • Mixed (tooling + source) changed-files → carve-out correctly does NOT trigger, falls through to template enforcement.
    • Full docker test harness: gsd-test-summary on randomized host plex2 → 11830 passed, 0 failed.

Regression test added?

  • Yes — added a test that would have caught this bug
  • No — explain why: the existing tests/pr-template-policy.test.cjs covers the function behavior thoroughly; the bug is a missing-dep / runtime-load failure that any direct require of the script catches. All 25 tests already exercise allPathsAreTooling against 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

  • macOS (local node 26 + docker harness)
  • Windows (including backslash path handling)
  • Linux (docker harness)
  • N/A (not platform-specific)

Runtimes tested

  • Claude Code
  • Gemini CLI
  • OpenCode
  • Other: ___
  • N/A (not runtime-specific — repo CI policy script)

Checklist

  • Issue linked above with Closes #3700 — PR will be auto-closed if missing
  • Linked issue has the confirmed-bug label — pending maintainer triage (auto-labels are bug + needs-triage)
  • Fix is scoped to the reported bug — no unrelated changes included (2-line diff)
  • Regression test added (or explained why not) — see Testing section
  • All existing tests pass (npm test equivalent: node --test tests/pr-template-policy.test.cjs 25/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 surface
  • No unnecessary dependencies added (the fix removes a missing dependency rather than adding one)

Breaking 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 format check will fail for the exact same reason every other open PR is currently failing — because pull_request_target runs 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

  • Chores
    • Updated internal development script to use native Node.js utilities for file matching, improving consistency with platform-standard tooling.

Review Change Stack

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.
@trek-e
trek-e requested a review from glittercowboy as a code owner May 18, 2026 16:37
@coderabbitai

coderabbitai Bot commented May 18, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

PR replaces the external minimatch dependency with Node's built-in path.matchesGlob in the PR-template policy script. The import at line 3 is updated, and the allPathsAreTooling() function now calls matchesGlob() instead of minimatch() to evaluate allowlist patterns.

Changes

Glob Pattern Matcher Update

Layer / File(s) Summary
Dependency replacement in PR-template policy
scripts/pr-template-policy.cjs
Import changed from minimatch to matchesGlob (from path module), and allPathsAreTooling() now evaluates allowlist glob patterns using matchesGlob(file, pattern) instead of minimatch(file, pattern, { matchBase: false, dot: true }).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

size/S, no-changelog

Suggested reviewers

  • glittercowboy

Poem

🐰 A rabbit hops through Node's own path,
No external packages to do the math!
matchesGlob now handles the game,
Built-in and speedy—a better way, the same! 🌟

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing a minimatch crash in pr-template-policy.cjs that blocks PR template-format checks.
Description check ✅ Passed The PR uses the correct Fix template with all required sections: linked issue, what was broken, what the fix does, root cause analysis, fix rationale, comprehensive testing details, and regression test justification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr-template-policy-minimatch

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e50ad81 and 2555819.

📒 Files selected for processing (1)
  • scripts/pr-template-policy.cjs

#!/usr/bin/env node

const { minimatch } = require('minimatch');
const { matchesGlob } = require('path');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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 -20

Repository: 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 -20

Repository: 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:


🏁 Script executed:

grep -r "node-version\|setup-node" .github/workflows/ --include="*.yml" --include="*.yaml" -A 2 -B 2

Repository: 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/null

Repository: gsd-build/get-shit-done

Length of output: 1135


🏁 Script executed:

cat .github/workflows/pr-template-format.yml

Repository: 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.

@trek-e
trek-e merged commit 1645bb5 into main May 18, 2026
17 of 20 checks passed
@github-actions
github-actions Bot deleted the fix/pr-template-policy-minimatch branch May 18, 2026 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(ci): pr-template-policy.cjs crashes — missing minimatch dep blocks every PR's template-format check

1 participant