[DX-4969] Handle merge_group events - #1634
Conversation
merge_group events had issues with git history as we didn't fetch proper git history.
|
👋 kalverra, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
🦋 Changeset is good to goLatest commit: 63643d9 We got this. Not sure what this means? Click here to learn what changesets are. |
There was a problem hiding this comment.
Pull request overview
This PR updates the advanced-triggers action to handle merge_group events more reliably by switching to GitHub API-based file change detection (with git-based fallbacks) and improving how missing git refs are fetched when diffs require additional history.
Changes:
- Add
merge_groupchanged-file detection viarepos.compareCommitsand route it throughgetChangedFiles(...). - Replace the prior
git pullfallback with a shallowgit fetchof missing refs (optionally authenticated). - Add Vitest coverage for the new routing and helper behavior, plus a changeset for a patch release.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile metadata for the workspace. |
| actions/advanced-triggers/src/run.ts | Routes merge_group changed-file detection through API with git fallback; threads token through git paths. |
| actions/advanced-triggers/src/github.ts | Adds getChangedFilesForMergeGroup using repos.compareCommits. |
| actions/advanced-triggers/src/git.ts | Replaces git pull fallback with shallow fetch of missing refs; supports authenticated fetch. |
| actions/advanced-triggers/src/tests/run.test.ts | Adds routing tests for merge_group and verifies fallback behavior. |
| actions/advanced-triggers/src/tests/github.test.ts | Adds tests for merge-group file extraction from API responses. |
| actions/advanced-triggers/src/tests/git.test.ts | Adds tests for shallow-fetch fallback behavior in git diff path. |
| actions/advanced-triggers/dist/index.js | Updates the bundled action output to match source changes. |
| .changeset/sweet-horns-bite.md | Declares a patch release for advanced-triggers. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
actions/advanced-triggers/src/run.ts:214
- The merge_group changed-file strategy now uses the GitHub API (compareCommits) with a git fallback, but the action documentation still states that merge_group uses
git diff --name-only(see actions/advanced-triggers/README.md table under “Supported events”). This mismatch can mislead workflow authors troubleshooting behavior and required permissions.
case "merge_group": {
return getChangedFilesForMergeGroupWithFallback(
octokit,
owner,
repo,
event.base,
event.head,
repositoryRoot,
token,
);
}
Switch from octokit.paginate() to a direct REST call for merge group comparisons. The paginated approach masked the API's hard 300-file cap, which returns truncated results without pagination links instead of fetching all pages.
Add a hook to run pnpm build:artifacts when package.json is staged. This ensures build artifacts are generated automatically when the package configuration changes.
merge_groupevents had issues with git history, as we didn't fetch the proper git history. Thegit pullfallback didn't work properly either. Now we fetch proper history with the API.Tested on smartcontractkit/chainlink#23386