ci(merge-train): dispatch the post-merge workflows after a successful FF (#1423) - #1499
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
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 |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
Reviewer's GuideImplements a derived, script-driven mechanism for dispatching and monitoring post-merge workflows that previously relied on push-to-main events (which are blocked when using GITHUB_TOKEN), adds a heartbeat workflow to detect regressions, and updates CI workflows, merge-train, and tests accordingly. Sequence diagram for post-merge workflow dispatch from merge-trainsequenceDiagram
actor Developer
participant MergeTrain as merge-train.yml
participant GitHub as GitHub
participant Script as push_trigger_workflows.py
participant Workflow as push_main_workflow
Developer->>MergeTrain: Label PR to trigger merge-train
MergeTrain->>GitHub: git push origin "${HEAD_SHA}:refs/heads/main"
Note right of GitHub: push with secrets.GITHUB_TOKEN
GitHub-->>MergeTrain: main fast-forwarded (no push workflows started)
MergeTrain->>Script: python3 scripts/push_trigger_workflows.py --branch main
Script-->>MergeTrain: e2e.yml, codeql.yml, deadcode.yml, eval-calibration.yml, release-drafter.yml, flag-stale-open-prs.yml, zizmor.yml
loop for each workflow in dispatch_list
MergeTrain->>GitHub: gh workflow run <workflow> --ref main --repo "$REPO"
GitHub-->>Workflow: Create workflow_dispatch run on main
end
MergeTrain-->>Developer: Merge completed (dispatch failures only warn)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…files The merge train FF-pushes with `secrets.GITHUB_TOKEN`, and GitHub raises no workflow runs from events made with that token, so every `on: push: branches: [main]` workflow stopped the day the train became the merge path. Re-arming them needs a list of what to dispatch, and a hand-written list reproduces the defect on the next workflow someone adds. This derives it: pure-stdlib text parse (PyYAML is not importable in CI), `branches:` under `on: push:` only, so publish.yml's tag trigger is excluded.
`gh workflow run` returns 422 for a workflow that does not declare the trigger, so without this the merge train's post-merge fan-out would warn and do nothing for five of the seven. release-drafter and flag-stale-open-prs already carry it. paths: filters do not apply to a dispatch, which for e2e and zizmor means a dispatched run always executes — intended, since the point is a post-merge run of main.
GitHub does not start workflow runs from events raised by `secrets.GITHUB_TOKEN` — the documented recursion guard — and `workflow_dispatch` is one of its two exceptions. So the train dispatches what its own push cannot trigger, from the derived enumeration rather than a literal list, after the FF and never from the refusal path. `--ref main` and no inputs: a dispatched run's check-runs attach to the head of the ref it was dispatched on, which is what keeps a post-merge run from reporting against a commit it did not test. Non-fatal by construction — main has already advanced, so a failed dispatch warns rather than reddening a landed merge. Costs `actions: write`; rejected a PAT/App token (a long-lived credential with write access to main, in a repo whose merge model exists because the bot has no key) and a schedule (decouples the run from the commit that caused it).
The #370 issue-opening step was gated on `github.event_name == 'push'`. The post-merge run is now a dispatch, so that guard skipped the step on exactly the runs it exists for — a red e2e on main with no issue and no label. Widened to `!= 'pull_request'`, and the title and body report the ref and event rather than asserting a push to main.
The stop was invisible for 666 commits because 'no run' is an absence and every signal in the system was an absence test. This asserts presence: weekly, for each workflow in the derived enumeration, take the newest run on main by push or workflow_dispatch and compare it against main's head commit. Schedule and pull_request runs are excluded — they are the masking that kept codeql and zizmor looking healthy throughout. Lag is measured against the head commit, not wall-clock, so a quiet fortnight is not a defect. An empty enumeration fails rather than reporting everything healthy.
Twenty mutations, each red: dropping a hatch, dropping actions: write, hoisting the dispatch above the FF push or into fail_and_unlabel, retargeting --ref, passing an input, un-guarding the failure, hard-coding either consumer's list, reverting e2e's surfacing guard, and seven parser mutations. The push:main set is re-derived by a second, deliberately different in-test parse rather than compared against the script's own output.
…in section Says why the FF push triggers nothing, that the list is derived, that a new push:main workflow needs its own workflow_dispatch trigger to be picked up, and where the regression detector lives.
The script is exercised through its CLI, and the two repo-wide gates on test termination caught what the first cut left open: `subprocess.run` with no `timeout=` (#1307's unbounded-blocking-call check) and ten spawning tests on the suite's 5s default, where contention reports as a hang rather than as slowness. Both bounded at 30s, and the enumeration is now memoised so the parametrised cases spawn once rather than once per case.
One entry file under the new per-entry convention (#1475), so it does not touch the shared [Unreleased] block.
`git rev-list --count 3421cd7..github/main` is 942 as of 2026-08-11, not the 666 the issue's AC1 comment recorded on 2026-08-09. Same figure everywhere and dated, so the next reader knows when it was taken.
robotrocketscience
force-pushed
the
ci/issue-1423-dispatch-after-ff
branch
from
August 11, 2026 22:44
77905b7 to
eb7bf67
Compare
robotrocketscience
added
the
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged eb7bf67 → |
5 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1423.
What was wrong
merge-train.ymllands every merge asgit push origin "${HEAD_SHA}:refs/heads/main"usingsecrets.GITHUB_TOKEN. GitHub raises no workflow runs from events made with that token — the documented recursion guard, whose only exceptions areworkflow_dispatchandrepository_dispatch. So the day the merge train became the merge path, everyon: push: branches: [main]workflow in this repository stopped, and nothing said so, because "no run" is an absence and every signal in the system is an absence test.Seven workflows carry that trigger. Six share the same last push-event run —
3421cd73, 2026-07-21T01:30:43Z — andgit rev-list --count 3421cd73..github/mainis 942 today, against the 666 in the AC1 comment and the 541 in the issue body.pushrun on maincodeql.yml3421cd73pull_request,scheduledeadcode.yml3421cd73pull_requeste2e.yml3421cd73pull_request(since #1421)eval-calibration.yml3421cd73pull_requestrelease-drafter.yml3421cd73workflow_dispatchflag-stale-open-prs.yml3421cd73workflow_dispatchmainPR in 942 commitszizmor.ymlb2657537pull_request,schedulezizmoris not evidence for the token guard and is not counted as such: both its arms carry apaths:filter restricted to.github/, so a date ten weeks older is what a path filter produces, not what a recursion guard produces.publish.ymlis unaffected — it triggers onv[0-9]+.[0-9]+.[0-9]+tag pushes, which the train never creates.What changed
The train dispatches what its own push cannot trigger. After the FF push succeeds, a
[6b/6]step runsgh workflow run "$wf" --ref main --repo "$REPO"over the enumerated set. That costsactions: writeand no new secret. The two rejected alternatives are recorded in the workflow itself: a PAT or GitHub App token on the checkout re-armson: pushfor everything at once, but it puts a long-lived credential with write access tomainin a repository whose entire merge model — FF-only, no bot rebase — exists because the bot deliberately holds no signing key; a schedule decouples the run from the commit that caused it, which is the one propertypush: [main]had and the only reason to want it.The list is derived, never written down.
scripts/push_trigger_workflows.pyparsesbranches:underon: push:across.github/workflows/*.ymland prints one filename per line. Both consumers — the train and the heartbeat — read it. A literal list is the original defect wearing a different hat: it silently omits the nextpush:mainworkflow anyone adds. Pure stdlib and parsed as text, becauseyamlis not importable in this repo's CI (the traptests/test_ci_manual_dispatch.pyrecords).--ref main, no inputs, after the push, non-fatal. A dispatched run's check-runs attach to the head of the ref it was dispatched on, which is what makes a post-merge run structurally unable to report against a commit it did not test (#1436/#1451). Dispatching before the push would targetmainat its pre-merge SHA and produce a green post-merge run for a commit that never merged. Andmainhas already advanced by the time the loop runs, so a failed dispatch warns rather than reddening a landed merge — the enumeration is captured into a variable rather than inlined into theforword list, because a failing command substitution in a for-list is not aset -efailure and would have iterated zero workflows in silence.Five workflows gained a bare
workflow_dispatch:.gh workflow runreturns 422 without it, and the loop is non-fatal, so five of seven would have warned and done nothing.paths:filters do not apply to a dispatch, soe2eandzizmornow run in full post-merge; that is intended, since the point is a run ofmain. Concurrency was deliberately left alone: every group here keys the PR arm ongithub.event.pull_request.numberand falls through togithub.ref, so a dispatch ofmaincan never collide with — or cancel — a PR's own in-flight run.e2e's failure surfacing had to move with it. The #370 issue-opening step wasif: github.event_name == 'push'. With the post-merge run arriving as a dispatch, that guard skipped the step on exactly the runs it exists for: a red e2e onmain, no issue, no label, nobody told. It is now!= 'pull_request', and the title and body reportgithub.ref_nameandgithub.event_namerather than asserting a push tomain.A heartbeat asserts presence.
.github/workflows/push-trigger-heartbeat.ymlruns weekly, takes for each enumerated workflow the newest run onmainbypushorworkflow_dispatch, and compares it againstmain's head commit; a lag over 14 days opens or updates one collated issue. Both events, so a future fix that re-armson: pushproperly does not make it cry. Neitherschedulenorpull_request, because those are the masking —codeqlandzizmorboth have a weekly cron that kept their newest run onmainlooking recent through the whole outage. Against the head commit rather than wall-clock, so a genuinely quiet fortnight is not a defect. An empty enumeration exits 1 rather than reporting everything healthy.Mutation check
20 mutations, each applied to the tree, run, and restored from a byte copy (never
git checkout), with__pycache__cleared first. All 20 turntests/test_merge_train_dispatch.pyred.workflow_dispatch:from e2e.ymltest_every_enumerated_workflow_accepts_a_dispatch[e2e.yml]actions: writefrom merge-traintest_merge_train_may_dispatch_at_alltest_the_dispatch_happens_after_the_ff_push--ref "${HEAD_REF}"instead of--ref maintest_the_dispatch_targets_main_and_carries_no_inputs-f sha=xto the dispatchtest_neither_consumer_writes_the_list_down[merge-train.yml]if/::warning::guard so a dispatch failure fails the traintest_a_failed_dispatch_cannot_fail_a_landed_mergefail_and_unlabeltest_the_dispatch_is_not_inside_the_refusal_path== 'push'test_a_dispatched_e2e_failure_still_opens_an_issuescheduletriggertest_the_heartbeat_runs_on_a_schedulescheduleruns tootest_the_heartbeat_ignores_schedule_and_pull_request_runsnowtest_the_heartbeat_measures_lag_against_main_not_wall_clocktest_the_heartbeat_refuses_an_empty_enumerationtest_neither_consumer_writes_the_list_down[push-trigger-heartbeat.yml]tags:asbranches:test_parser_ignores_tag_only_and_branch_ignore_pushesbranches-ignore:push:outside theon:blocktest_parser_does_not_match_a_push_key_outside_the_on_blocktest_parser_matches_the_branch_literallytest_the_enumeration_is_not_empty_and_excludes_tag_only_pushespull_request:branches as push branchespush:mainfalse positivesThree of these survived the first pass and the assertions were tightened until they failed, which is why they are worth naming. M11 was matched by containment —
for ev in push workflow_dispatch schedulecontains the two-event substring — and is now matched exactly. M15 and M18 were behaviour-preserving against realistic fixtures:publish.yml's tags arev[0-9]+…, which no branch match reaches, andrelease/*does not containmain. The fixtures are now the discriminating cases — a tag literally namedmain, and a branch namedmaintenance— not the realistic ones.The push:main set is compared against a second, deliberately different in-test parser rather than against the script's own output, so agreement means something; comparing the script to itself is the tautological guard of #1161.
Tests: 27 in the new file; 165 across the related selection (
test_ci_manual_dispatch,test_merge_train_gate,test_ci_path_filter,test_eval_calibration_gate,test_subprocess_timeout_budget_1307, the three changelog files). Full suite 8258 passed / 7 failed; all seven pre-existing and environmental — six are--archive requires the 'archive' extra(cryptographyis not in this venv; CI syncs--extra archive) and five of those reproduce on a detachedgithub/maincheckout, andtest_doctor_cli_exit_1_when_brokenis a 5s pytest-timeout flake that passes in isolation.Not done, and it needs saying
AC3 and AC5 are not met by this PR and cannot be. Both are observations that only exist after this lands: AC3 wants the run ID of a dispatch fired by a real merge, and AC5 wants the colour of the first e2e run on
mainin 942 commits. Whoever merges this owes both to the issue. The baseline to compare against: this repository has recorded exactly oneworkflow_dispatchrun ever —link-check, 2026-05-19T18:30:35Z, success, human-initiated — so the documentedGITHUB_TOKENexception is documented but not yet demonstrated here. If the step logsdispatched.and no run appears, the guard applies more broadly than documented and this should be reverted in favour of the token substitution.Blast radius of a red first e2e is bounded:
e2eis not a required context, it already runs per-PR since #1421, and merge-train evaluates check-runs on the PR head SHA, so a red post-merge run onmaindoes not block a labelled PR. It will now open anattn:e2e-failureissue naming the ref and SHA, which is the point.Out of scope
pull_requesttrigger, sotest_every_unfiltered_pr_workflow_is_in_the_floordoes not derive it, andtests/test_merge_train_gate.pyis green.zizmorpath-filter question. A dispatched zizmor scans on every merge rather than only on merges touching.github/. That is a deliberate consequence of dispatch ignoringpaths:; teaching the train which filters to honour is a larger change and is not made here.Summary by Sourcery
Dispatch post-merge workflows explicitly from the merge train using a derived list of push-triggered workflows, add monitoring to detect regressions, and adjust workflows and tests to support and validate this behavior.
New Features:
Bug Fixes:
Enhancements:
Tests: