Skip to content

fix: add is_directory support to scripts sync section#1435

Merged
stranske merged 1 commit intomainfrom
fix/sync-directory-handling
Feb 10, 2026
Merged

fix: add is_directory support to scripts sync section#1435
stranske merged 1 commit intomainfrom
fix/sync-directory-handling

Conversation

@stranske
Copy link
Copy Markdown
Owner

@stranske stranske commented Feb 10, 2026

Source: Issue #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 minimatch library, but:

  1. bot-comment-dismiss.js still defines ~140 lines of custom glob functions instead of importing minimatch.
  2. The vendored minimatch at .github/scripts/node_modules/minimatch/ is a 0.0.0-local reimplementation, not the real npm package.
  3. Consumer repos will break on sync — template files pr-context-graphql.js and merge_manager.js both require('minimatch'), but the template directory has no node_modules/minimatch/ and the sync manifest does not include it.

Context for Agent

Related Issues/PRs

Context for Agent

Related Issues/PRs

Tasks

Replace custom glob in bot-comment-dismiss.js

  • Remove the inline escapeRegExp, expandBraces, globToRegExp, and minimatch functions (lines 21–163) from .github/scripts/bot-comment-dismiss.js
  • Add const { minimatch } = require('minimatch'); import at the top of bot-comment-dismiss.js
  • Update shouldIgnorePath in bot-comment-dismiss.js to call minimatch(normalized, pattern, { dot: true, nocomment: true, nonegate: true }) — matching the options used in merge_manager.js
  • Apply the identical changes to templates/consumer-repo/.github/scripts/bot-comment-dismiss.js

Standardize the vendored minimatch module

  • Replace .github/scripts/node_modules/minimatch/package.json version from 0.0.0-local to 10.0.1-vendored (or current real minimatch major) and add a comment in the file explaining it is a vendored subset
  • Verify existing tests pass with the vendored module: node --test .github/scripts/__tests__/bot-comment-dismiss.test.js && node --test tests/test/dismiss/dismiss-bot-comment.test.js

Fix consumer repo sync for minimatch

  • Copy .github/scripts/node_modules/minimatch/ to templates/consumer-repo/.github/scripts/node_modules/minimatch/
  • Copy .github/scripts/package.json to templates/consumer-repo/.github/scripts/package.json
  • Add the node_modules/minimatch/ directory and package.json to .github/sync-manifest.yml under the scripts section
  • Verify the sync manifest health check passes

Standardize minimatch options

  • Ensure bot-comment-dismiss.js, pr-context-graphql.js, and merge_manager.js all use consistent minimatch options ({ dot: true, nocomment: true, nonegate: true })
  • Verify all JS tests still pass: node --test .github/scripts/__tests__/*.test.js && node --test tests/test/dismiss/*.test.js

Acceptance criteria

  • bot-comment-dismiss.js does NOT contain any of: function escapeRegExp, function expandBraces, function globToRegExp, or a local function minimatch.
    Verification: grep -c "function escapeRegExp\|function expandBraces\|function globToRegExp" .github/scripts/bot-comment-dismiss.js returns 0.

  • bot-comment-dismiss.js imports minimatch via require('minimatch').
    Verification: grep -c "require.*minimatch" .github/scripts/bot-comment-dismiss.js returns 1.

  • Template copy matches: diff .github/scripts/bot-comment-dismiss.js templates/consumer-repo/.github/scripts/bot-comment-dismiss.js exits 0.

  • Consumer template has node_modules/minimatch/: test -f templates/consumer-repo/.github/scripts/node_modules/minimatch/index.js exits 0.

  • All existing JS tests pass: node --test .github/scripts/__tests__/*.test.js && node --test tests/test/dismiss/*.test.js exits 0.

  • Glob matching behavior is unchanged: brace expansion (src/*.{ts,tsx}), character classes (src/[ab].ts), and escaped metacharacters (docs/\\[draft\\].md) continue to pass.
    Verification: tests in bot-comment-dismiss.test.js and dismiss-bot-comment.test.js cover these patterns and pass.

  • Head SHA: bea55c4

  • Latest Runs: ✅ success — Gate

  • Required: gate: ✅ success

  • | Workflow / Job | Result | Logs |

  • |----------------|--------|------|

  • | Agents PR meta manager | ❔ in progress | View run |

  • | CI Autofix Loop | ✅ success | View run |

  • | Gate | ✅ success | View run |

  • | Health 40 Sweep | ✅ success | View run |

  • | Health 44 Gate Branch Protection | ❔ in progress | View run |

  • | Health 45 Agents Guard | ✅ success | View run |

  • | Health 50 Security Scan | ✅ success | View run |

  • | Health 72 Template Sync | ✅ success | View run |

  • | Health 73 Template Completeness | ✅ success | View run |

  • | Maint 52 Validate Workflows | ✅ success | View run |

  • | PR 11 - Minimal invariant CI | ✅ success | View run |

  • | Selftest CI | ✅ success | View run |

  • | Validate Sync Manifest | ✅ success | View run |

Head SHA: d50c21d
Latest Runs: ❔ in progress — Gate
Required: gate: ❔ in progress

Workflow / Job Result Logs
Agents Auto-Pilot ⏭️ skipped View run
Agents Bot Comment Handler ✅ success View run
Agents Keepalive Loop ✅ success View run
Agents PR meta manager ❔ in progress View run
Agents Verifier ✅ success View run
Auto-label Dependabot PRs ⏭️ skipped View run
CI Autofix Loop ✅ success View run
Copilot code review ❔ in progress View run
Create Issue from Verification (DEPRECATED) ⏭️ skipped View run
Create Issue from Verification (Enhanced) ⏭️ skipped View run
Create New PR from Verification ⏭️ skipped View run
Gate ❔ in progress View run
Health 40 Sweep ✅ success View run
Health 44 Gate Branch Protection ✅ success View run
Health 45 Agents Guard ✅ success View run
Health 50 Security Scan ✅ success View run
Health 73 Template Completeness ✅ success View run
Maint 52 Validate Workflows ✅ success View run
PR 11 - Minimal invariant CI ✅ success View run
Selftest CI ✅ success View run
Validate Sync Manifest ✅ success View run

The sync workflow's scripts processing loop always called sync_file(),
which uses shutil.copy2() and fails on directories with IsADirectoryError.

This broke sync for all 7 consumer repos when minimatch/, brace-expansion/,
and balanced-match/ were added to the sync manifest as script entries.

Changes:
- Add is_directory check to scripts processing loop (matches actions and
  copilot_config sections)
- Add is_directory: true to minimatch/, brace-expansion/, balanced-match/
  entries in sync manifest

Fixes #1432
Copilot AI review requested due to automatic review settings February 10, 2026 07:44
@stranske stranske added the bug Something isn't working label Feb 10, 2026
@stranske-keepalive
Copy link
Copy Markdown
Contributor

stranske-keepalive bot commented Feb 10, 2026

🤖 Keepalive Loop Status

PR #1435 | Agent: Codex | Iteration 0/5

Current State

Metric Value
Iteration progress [----------] 0/5
Action wait (missing-agent-label)
Disposition skipped (transient)
Gate success
Tasks 0/36 complete
Timeout 45 min (default)
Timeout usage 3m elapsed (9%, 42m remaining)
Keepalive ❌ disabled
Autofix ❌ disabled

🔍 Failure Classification

| Error type | infrastructure |
| Error category | resource |
| Suggested recovery | Confirm the referenced resource exists (repo, PR, branch, workflow, or file). |

@stranske-keepalive
Copy link
Copy Markdown
Contributor

stranske-keepalive bot commented Feb 10, 2026

Automated Status Summary

Head SHA: 3bfd08d
Latest Runs: ⏳ pending — Gate
Required contexts: Gate / gate, Health 45 Agents Guard / guard
Required: core tests (3.11): ⏳ pending, core tests (3.12): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 93.12%
Baseline 85.00%
Delta +8.12%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
src/cli_parser.py 81.8% 4
src/percentile_calculator.py 95.0% 1
src/aggregator.py 95.0% 2
src/__init__.py 100.0% 0
src/ndjson_parser.py 100.0% 0

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

@stranske stranske temporarily deployed to agent-high-privilege February 10, 2026 07:45 — with GitHub Actions Inactive
@stranske stranske merged commit 8c779dd into main Feb 10, 2026
149 of 154 checks passed
@stranske stranske deleted the fix/sync-directory-handling branch February 10, 2026 07:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes consumer repo sync failures caused by adding vendored node_modules/ directories to the scripts sync manifest, by teaching the sync workflow to copy directories in the scripts section.

Changes:

  • Update maint-68-sync-consumer-repos.yml to support is_directory for scripts manifest entries and use sync_directory() when set.
  • Update .github/sync-manifest.yml to mark vendored minimatch/, brace-expansion/, and balanced-match/ entries as directories.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/maint-68-sync-consumer-repos.yml Adds directory-aware syncing for scripts entries to avoid shutil.copy2() on directories.
.github/sync-manifest.yml Marks vendored node_modules/*/ entries with is_directory: true so the sync workflow copies them correctly.

stranske added a commit that referenced this pull request Feb 10, 2026
The check_consumer_sync_drift.py script crashed with IsADirectoryError
when encountering directory entries (minimatch/, brace-expansion/,
balanced-match/) in the sync manifest.

Extract file comparison into _check_file() helper and add directory
handling that recursively compares all files within directory entries,
matching the fix applied to the sync workflow in PR #1435.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants