-
Notifications
You must be signed in to change notification settings - Fork 86
Refactor sync scripts with shared library and bot authentication #3799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Major changes: - Extract shared utilities into .github/scripts/lib/ module: - cache.py: PRCache for parallel prefetching (8x faster) - github_api.py: Shared GitHub API functions - github_client.py: Session setup with retry logic - labels.py: Label management - membership.py: Org membership and trigger comment checks - pr_comments.py: Exclusion comment generation - file_utils.py: File processing helpers - yaml_utils.py: YAML manipulation - constants.py: Shared constants and defaults - Add write_session support for bot authentication: - Labels and comments now posted as github-actions[bot] - Separate read (PAT) and write (github.token) sessions - No fallback - tokens are required - Add bulk PR handling: - Skip PRs with >10 rules - Apply exclusion label and explanatory comment - Configurable via MAX_RULES_PER_PR - Add organization membership filtering: - Filter by org membership (configurable) - Support trigger comments to override - Exclusion labels and comments for non-members - Workflow improvements: - Checkout scripts from main branch for consistency - Use github.token for write operations - Immediate rule deletion for closed PRs (0 day delay) - Relocate scripts to .github/scripts/ directory Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
0e147df to
a4a57d9
Compare
Replace REST API reads with GraphQL to reduce API calls by ~95%: - Add lib/graphql_client.py with bulk PR query - Add lib/pr_data.py with PRData dataclass - Remove lib/cache.py (no longer needed) - Remove lib/membership.py (replaced by authorAssociation) - Remove lib/github_api.py (replaced by GraphQL) API call reduction: 500-600+ calls → ~32 calls per run Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
With GraphQL, the built-in GITHUB_TOKEN works for all operations: - GraphQL queries for bulk PR data - REST API for file contents - REST API for label/comment writes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove SYNC_TOKEN PAT and GITHUB_WRITE_TOKEN - now using single built-in github.token for all GitHub API operations (GraphQL + REST). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the sync scripts by extracting shared functionality into a modular library structure under .github/scripts/lib/, adds GraphQL-based bulk PR fetching for significant performance improvements, and introduces bot authentication support for labels and comments. The scripts are also relocated from scripts/ to .github/scripts/ for better organization.
Changes:
- Extracted common utilities into 10 reusable library modules in
.github/scripts/lib/ - Implemented GraphQL-based bulk data fetching via
PRCacheto reduce API calls from 500-600+ to 1-2 per run - Added organization membership filtering and PR comment exclusion handling with automatic label management
- Created two new specialized sync scripts:
sync_test_rules.pyandsync_shared_samples.py
Reviewed changes
Copilot reviewed 16 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/sync_detection_rules.py |
Deleted monolithic 1080-line script |
.github/scripts/sync_test_rules.py |
New test-rules sync script using shared library and GraphQL |
.github/scripts/sync_shared_samples.py |
New shared-samples sync script with Sublime API integration |
.github/scripts/lib/*.py |
10 new library modules for shared functionality (constants, GraphQL, REST API, PR data, labels, comments, YAML/file utils) |
.github/workflows/sync-test-rules.yml |
Updated to use new script path and github.token |
.github/workflows/sync-shared-samples.yml |
New workflow for shared-samples branch syncing |
.github/workflows/rule-validate.yml |
Updated script paths from scripts/ to .github/scripts/ |
.github/scripts/mql_format.py |
Relocated from scripts/ (no code changes) |
.github/scripts/check_invisible_chars.py |
Relocated from scripts/ (no code changes) |
.github/scripts/generate_rule_ids/* |
Relocated from scripts/generate-rule-ids/ (no code changes) |
Comments suppressed due to low confidence (1)
.github/workflows/sync-test-rules.yml:55
- Corrected spelling of 'labls' to 'labels' on line 55.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unused marker variable in pr_comments.py - Move change_type_map to module level in graphql_client.py - Use datetime.fromisoformat for cleaner ISO 8601 parsing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
.github/scripts/lib/module for better code organization and reusabilityPRCachefor ~8x faster sync executionwrite_sessionsupport so labels and comments are posted asgithub-actions[bot]instead of the PAT userscripts/to.github/scripts/directoryNew shared library modules
cache.pyPRCachefor parallel prefetching of labels, files, and contentgithub_api.pyget_pull_requests,get_files_for_pull_request)github_client.pylabels.pyhas_label,apply_label,remove_label)membership.pypr_comments.pyfile_utils.pyyaml_utils.pyconstants.pyTest plan