fix: unify all scripts to use vendored minimatch, remove minimatch-lite.js#1433
fix: unify all scripts to use vendored minimatch, remove minimatch-lite.js#1433
Conversation
…te.js PR #1426 resolved custom glob removal in bot-comment-dismiss.js but introduced minimatch-lite.js (a copy of the old custom code) used by merge_manager.js and pr-context-graphql.js. This created inconsistency (one script using vendored minimatch, two using custom lite version) and a consumer repo breakage risk since minimatch-lite.js was not in the templates directory or sync manifest. Changes: - merge_manager.js: require('./minimatch-lite.js') → require('minimatch') - pr-context-graphql.js: require('./minimatch-lite.js') → require('minimatch') - Delete .github/scripts/minimatch-lite.js (176-line custom reimplementation) - Update template copies to match All 767 JS tests pass. Addresses verify:compare FAIL from issue #1425.
Automated Status SummaryHead SHA: 5bde019
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopePR #1409 (issue #1407) merged with CONCERNS from both verify:compare providers (openai 78%, anthropic 85%). The core unresolved concern is that custom regex-based glob matching was supposed to be replaced with a standard
Context for AgentRelated Issues/PRsTasksReplace custom glob in bot-comment-dismiss.js
Standardize the vendored minimatch module
Fix consumer repo sync for minimatch
Standardize minimatch options
Acceptance criteria
|
There was a problem hiding this comment.
Pull request overview
This PR completes the “single shared minimatch” cleanup by removing the temporary minimatch-lite.js shim and standardizing all affected scripts (including consumer templates) to import the vendored minimatch module directly.
Changes:
- Switch
merge_manager.jsandpr-context-graphql.js(source + templates) fromrequire('./minimatch-lite.js')torequire('minimatch'). - Delete
.github/scripts/minimatch-lite.jsnow that it has no remaining consumers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
.github/scripts/pr-context-graphql.js |
Uses the shared vendored minimatch module instead of a local shim. |
.github/scripts/merge_manager.js |
Uses the shared vendored minimatch module instead of a local shim. |
.github/scripts/minimatch-lite.js |
Removed (no longer referenced). |
templates/consumer-repo/.github/scripts/pr-context-graphql.js |
Mirrors the source script import change for consumer sync correctness. |
templates/consumer-repo/.github/scripts/merge_manager.js |
Mirrors the source script import change for consumer sync correctness. |
🤖 Keepalive Loop StatusPR #1433 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b487f9ddb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…h) for consumer repos The vendored minimatch requires brace-expansion at runtime, which in turn requires balanced-match. Without these transitive dependencies in the template node_modules, consumer repos would hit MODULE_NOT_FOUND when running reusable-pr-context.yml or merge_manager.js. Changes: - Copy brace-expansion/ and balanced-match/ to templates/consumer-repo/ .github/scripts/node_modules/ - Add both to .github/sync-manifest.yml so they get synced to consumers Addresses review feedback on PR #1433.
Automated Status Summary
Scope
PR #1409 (issue #1407) merged with CONCERNS from both verify:compare providers (openai 78%, anthropic 85%). The core unresolved concern is that custom regex-based glob matching was supposed to be replaced with a standard
minimatchlibrary, but:bot-comment-dismiss.jsstill defines ~140 lines of custom glob functions instead of importingminimatch.minimatchat.github/scripts/node_modules/minimatch/is a0.0.0-localreimplementation, not the real npm package.pr-context-graphql.jsandmerge_manager.jsbothrequire('minimatch'), but the template directory has nonode_modules/minimatch/and the sync manifest does not include it.Context for Agent
Related Issues/PRs
Tasks
Replace custom glob in bot-comment-dismiss.js
escapeRegExp,expandBraces,globToRegExp, andminimatchfunctions (lines 21–163) from.github/scripts/bot-comment-dismiss.jsconst { minimatch } = require('minimatch');import at the top ofbot-comment-dismiss.jsshouldIgnorePathinbot-comment-dismiss.jsto callminimatch(normalized, pattern, { dot: true, nocomment: true, nonegate: true })— matching the options used inmerge_manager.jstemplates/consumer-repo/.github/scripts/bot-comment-dismiss.jsStandardize the vendored minimatch module
.github/scripts/node_modules/minimatch/package.jsonversion from0.0.0-localto10.0.1-vendored(or current real minimatch major) and add a comment in the file explaining it is a vendored subsetnode --test .github/scripts/__tests__/bot-comment-dismiss.test.js && node --test tests/test/dismiss/dismiss-bot-comment.test.jsFix consumer repo sync for minimatch
.github/scripts/node_modules/minimatch/totemplates/consumer-repo/.github/scripts/node_modules/minimatch/.github/scripts/package.jsontotemplates/consumer-repo/.github/scripts/package.jsonnode_modules/minimatch/directory andpackage.jsonto.github/sync-manifest.ymlunder the scripts sectionStandardize minimatch options
bot-comment-dismiss.js,pr-context-graphql.js, andmerge_manager.jsall use consistent minimatch options ({ dot: true, nocomment: true, nonegate: true })node --test .github/scripts/__tests__/*.test.js && node --test tests/test/dismiss/*.test.jsAcceptance criteria
bot-comment-dismiss.jsdoes NOT contain any of:function escapeRegExp,function expandBraces,function globToRegExp, or a localfunction minimatch.Verification:
grep -c "function escapeRegExp\|function expandBraces\|function globToRegExp" .github/scripts/bot-comment-dismiss.jsreturns0.bot-comment-dismiss.jsimports minimatch viarequire('minimatch').Verification:
grep -c "require.*minimatch" .github/scripts/bot-comment-dismiss.jsreturns1.diff .github/scripts/bot-comment-dismiss.js templates/consumer-repo/.github/scripts/bot-comment-dismiss.jsexits 0.node_modules/minimatch/:test -f templates/consumer-repo/.github/scripts/node_modules/minimatch/index.jsexits 0.node --test .github/scripts/__tests__/*.test.js && node --test tests/test/dismiss/*.test.jsexits 0.src/*.{ts,tsx}), character classes (src/[ab].ts), and escaped metacharacters (docs/\\[draft\\].md) continue to pass.Verification: tests in
bot-comment-dismiss.test.jsanddismiss-bot-comment.test.jscover these patterns and pass.Head SHA: bea55c4
Latest Runs: ✅ success — Gate
Required: gate: ✅ success