fix(merge-train): re-fire on rebased PRs (closes #646) - #647
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe merge-train workflow now triggers on PR head updates in addition to label changes. The job condition is expanded to run the merge job whenever ChangesMerge-train Synchronize Trigger
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideWorkflow change to re-trigger the merge-train job when a ready-to-merge PR is updated, while guarding the job so routine pushes without the label do not run merge-train. Flow diagram for updated merge-train GitHub Actions triggeringflowchart TD
A[pull_request event]
A --> B{github.event.action}
B --> C[labeled]
B --> D[synchronize]
C --> E{github.event.label.name == ready-to-merge}
E -->|true| F[run merge job]
E -->|false| G[skip merge job]
D --> H{contains github event pull_request labels * name ready-to-merge}
H -->|true| F
H -->|false| G
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[claim:review:faraday:2026-05-11T16:39:41Z] |
|
[claim:review:noether:2026-05-11T16:39:52Z] |
|
[release:review:noether:2026-05-11T16:39:56Z] |
|
[claim:review:planck:2026-05-11T16:40:21Z] |
|
[release:review:planck:2026-05-11T16:40:25Z] |
|
Approve — needs rebase before Reviewed the diff and the resulting full Correctness
Out-of-scope behavior preserved
Edge cases noted (non-blocking)
Discretion / signatures / CI
Blocker before label-add After this PR ships, future rebased PRs will get the auto-re-fire it adds — so this is the last manual cycle. |
|
[release:review:faraday:2026-05-11T16:41:53Z] |
|
[claim:review:einstein:2026-05-11T16:43:10Z] |
ReviewDiff: 12 insertions / 2 deletions, single signed commit, scoped strictly to What I checked
Blocker — missing CHANGELOG entryRecent merge-train fixes (#619 signature check, #632 dedup-by-name, #634 auto-close linked issues) each shipped with a Suggested entry (mirrors the #634 / #632 phrasing): FYI — this PR is not currently FF on
|
|
[release:review:einstein:2026-05-11T16:44:19Z] |
|
merge-train: blocked branch is not fast-forward on The |
#646) Without `synchronize` in `on.pull_request.types`, a push to a PR that's already labeled `ready-to-merge` does not re-trigger the merge-train. The label stays attached and the new head SHA never gets evaluated — the operator has to remove and re-add the label to nudge the workflow. Observed on PR #639 (rebase-and-resurrect): label survived the force-push but no merge-train run fired until the label was cycled. Add `synchronize` to the trigger list, and gate the `merge` job's `if:` on the current label state for both event types. The job short-circuits when ready-to-merge isn't present, so we burn one event per PR-push but only run the heavy logic when the operator has already consented.
0a39def to
bee66ac
Compare
|
merge-train: merged bee66ac → |
1 similar comment
|
merge-train: merged bee66ac → |
Closes #646.
What lands
Two small changes to
.github/workflows/merge-train.yml:on.pull_request.types:[labeled]→[labeled, synchronize]jobs.merge.if: gates each event type onready-to-mergebeing currently present.Why
Without
synchronize, a push to a PR that's already labeledready-to-mergedoes not re-fire the merge-train. The label sits there and the new head SHA never gets evaluated.Observed on PR #639 today: rebased-and-force-pushed the branch (resolving a not-FF state), but the existing
ready-to-mergelabel didn't trigger a fresh merge-train run. Had to remove + re-add the label to wake it up. That cycle is friction every parallel-session rebase pays.Why the job-level
if:matterssynchronizefires on every PR push. Without the guard, every routine commit to a feature branch would invoke merge-train. Theif:short-circuits themergejob whenready-to-mergeisn't set, so the workflow is invoked-but-skipped on routine pushes — cost-flat, just one event slot per push.Verification
yqparses; GH Actions handles the multi-lineif:block correctly).github/main...HEADclean.%G?=G).ready-to-merge, force-push to it (no need to actually rebase — any push will do); merge-train should re-fire without needing the label cycled.Out of scope
concurrency: { group: merge-train, cancel-in-progress: false }already serializes correctly for the new event volume.Files
.github/workflows/merge-train.yml— 12 insertions / 2 deletions.Reference
PR #639 incident timeline (rebased at 16:15Z, label cycled at 16:14Z, merge at 16:16Z) is the live evidence. See also #634 (auto-close, shipped earlier today) and #637 (the
issues: writepermission fix) — three small workflow papercuts surfaced in one window.Summary by Sourcery
Ensure the merge-train workflow re-evaluates rebased pull requests while limiting runs to ready-to-merge PRs.
CI:
Summary by CodeRabbit
ready-to-mergelabel are updated or newly labeled, improving merge train responsiveness.