fix(ci): promote on the push to dev, not on a trigger registered from main (#2822) - #2825
Conversation
…om `main` (Abilityai#2822) `issue-status-on-merge.yml` has produced zero runs since Abilityai#2769 moved it to `pull_request_target` on 2026-09-14. Seventeen PRs have merged into `dev` since, ten of them carrying a closing keyword for eleven issues, and every one of those issues was relabelled by hand. Neither trigger could start a run, for opposite reasons. GitHub reads a `pull_request` workflow from the PR's merge ref, which resolves against `dev` -- and `dev`'s copy no longer declared it. It reads a `pull_request_target` workflow from the repository's DEFAULT branch (`GITHUB_REF` is the default branch, `GITHUB_SHA` its last commit) -- and `main` receives release cuts only, so `main`'s copy still says `pull_request`. The trigger that decides whether a run starts was being read from a branch the PR never touches, which is why the failure was absent rather than red. `push` is read from the ref being pushed, explicitly including workflows that are not merged into the default branch, so a trigger change to this file takes effect on the merge that lands it and cannot go dark until a release cut again. That is Abilityai#2822's fourth acceptance criterion, not merely a way to restore today's promotion. It keeps Abilityai#2767's fix by a safer route: a push to `dev` is a base-repo event whatever the merged PR's origin, so the token carries the declared `permissions:` and a fork author's issue is promoted like anyone else's -- without handing a write token to an event a stranger can start. A push payload carries no pull request, so the merged PR is resolved from the pushed commits with `listPullRequestsAssociatedWithCommit`, which needs `pull-requests: read` and nothing more. Only PRs that merged INTO the pushed branch are read: a commit on `dev` is also in the head of the open `dev` -> `main` release PR, whose body closes every issue in the release. Still no `actions/checkout` and no `run:` step. Closes Abilityai#2822
`listPullRequestsAssociatedWithCommit` documents that for a commit "not present in the default branch" -- which every `dev` commit is until a release cut -- it returns "merged AND open pull requests associated with the commit". Measured on `b8a790b2`: six PRs come back, one merged and five OPEN, and every one of them is `base: dev`, because any branch cut from `dev` after that commit contains it. So the release PR was the smaller half of the story: `merged_at` is what stops one merge promoting the close list of every open PR in flight, and `base.ref` is what excludes the `dev` -> `main` release PR. The comment said only the second; both are now stated with the measurement behind them, and each has its own test.
367a0ad to
04e103f
Compare
|
Rebased onto Kept this branch's header, since it documents the trigger the file now has, and added a line noting that No YAML or job-body changes in the rebase, so what |
…s stale test_2814_workflow_trigger_parity::test_every_accepted_entry_names_a_real_divergence fails once this branch stops declaring pull_request_target on issue-status-on-merge.yml: the ACCEPTED_UNTIL_RELEASE entry recording that divergence no longer matches anything, which is exactly what that guard is for. The entry existed because the promotion was accepted as broken until the next release cut. Moving the trigger to push: branches: [dev] removes the need to wait for a release at all, so the record goes with it.
|
I pruned the entry rather than amending it, because the decision it recorded no longer applies. It read:
The whole point of moving to Discriminating, both runs local on this branch: No other allowlist entry is touched; |
Description
issue-status-on-merge.ymlhas produced zero runs since #2769 moved it frompull_requesttopull_request_targeton 2026-09-14. Not red — absent.Both hypotheses in the issue were tested. The
types: [closed]one is refuted:git show b8a790b2 -- .github/workflows/issue-status-on-merge.ymlchanges exactlyone line in the
on:block,pull_request:→pull_request_target:, withtypes: [closed]andbranches: [dev]untouched context lines either side. Thedefault-branch-registration hypothesis holds.
Why neither trigger could start a run
GitHub reads the two events' definitions from different refs, and after #2769
each of them pointed at a branch whose copy declared the other trigger.
GITHUB_REFper GitHub's docspull_requestrefs/pull/PULL_REQUEST_NUMBER/mergedevpull_request_targetpull_request_targetGITHUB_SHA: "Last commit on default branch")mainpull_requestThe
pull_request_targetsection ofEvents that trigger workflows
says it plainly: "This event runs in the context of the default branch of the
base repository, rather than in the context of the merge commit, as the
pull_requestevent does."mainreceives release cuts only (last cut2026-09-04), so the trigger that decides whether a run starts was being read
from a branch no PR touches.
Worth naming, because it is what made the inference reasonable and because
#2816 concluded the opposite ("the docs describe it only as runs in the context
of the base of the pull request… and the docs are not [the proof]"): both
wordings are in the docs, behind a version switch. In
content/actions/reference/workflows-and-actions/events-that-trigger-workflows.mdthe sentence is
{% ifversion ghes < 3.20 %}base of the pull request{% else %}default branch of the base repository{% endif %}, and thepull_request_targettablehas the matching pair of rows —
GITHUB_REF: "PR base branch" for GHES < 3.20,"Default branch" otherwise. github.com renders the second. So the empirical
record and the current documentation agree; only the GHES-era wording says base
branch, and it is the wording this workflow's own header comment quotes as a
security property. That comment is corrected here.
The fix
pushtodev. ItsGITHUB_REFis the "Updated ref", and the docs areexplicit that
push"includes workflows that are not merged into the defaultbranch" — so a trigger change to this file takes effect on the merge that lands
it and cannot go dark until a release cut again. That is #2822's fourth
acceptance criterion ("switch to a trigger that is evaluated from the PR's base
branch"), not merely a way to restore today's promotion. It also means this PR
is self-verifying: its own squash merge is the first real test.
#2767's fix is kept, by a safer route. A push to
devis a base-repo eventwhatever the merged PR's origin, so the
GITHUB_TOKENcarries exactly thedeclared
permissions:and a fork author's issue is promoted like anyoneelse's — without ever handing a write token to an event a stranger can start.
The
pull_request_targetsafety property is preserved and then made moot.There is still no
actions/checkoutand still norun:step, so no PR code isever fetched or executed; the only untrusted values read are the PR title and
body, and they still reach exactly one regex that extracts
\d+. Nothing isinterpolated into a shell and nothing is
eval'd. On top of that, theescalation surface itself is gone: the workflow no longer runs on an event a
non-collaborator can trigger.
tests/unit/test_2767_issue_status_fork_prs.pystill pins every one of those properties.
Resolving the PR from a push. A push payload carries no pull request, so the
merged PR is looked up from the pushed commits with
github.rest.repos.listPullRequestsAssociatedWithCommit. That endpoint requiresthe
pull_requestspermission atreadand no other — already declared, so thetoken block is unchanged.
Only PRs that merged into the pushed branch are read
(
pr.merged_at && pr.base.ref === base), and both halves of that filter areload-bearing. The endpoint's own description: "Lists the merged pull request
that introduced the commit to the repository. If the commit is not present in
the default branch, it will return merged and open pull requests associated with
the commit." Every
devcommit is "not present in the default branch" until arelease cut, so that second sentence is the normal case here, not an edge one.
Measured against
b8a790b2:Six PRs, one merged and five open — and every one of them
base: dev, becauseany branch cut from
devafterwards contains the commit. Somerged_atis whatstops a single merge promoting the close list of every open PR in flight,
and
base.refis what excludes thedev→mainrelease PR, whose body closesevery issue in the release and which
merged_atstops excluding the moment therelease lands. Each has its own test.
Measured
Last run is 44 seconds before #2769's merge commit
b8a790b2(2026-09-1414:24:22 UTC). Zero
pull_request_targetruns repo-wide, ever is thediscriminator: if
dev's copy were being consulted, thebranches: [dev]filtermatches and runs would exist and merely fail. That also rules out the third
candidate in the issue — the
branches:filter is not the cause.Re-counted at 2026-09-15 15:00 UTC, the window is wider than the issue's six:
17 PRs have merged into
devsinceb8a790b2, 10 of them carrying a closingkeyword naming 11 distinct issues, none of which the workflow promoted —
#2730, #2742, #2349, #2524, #2691, #2692, #2695, #2759, #2794, #2795, #2796.
The control:
push: [dev]fired on every one of those mergesThis is what makes the fix more than an inference. Sixteen other workflows in this
repo already declare
push: branches: [dev], and in the same window, on thesame branch, for the same merges, they ran — while this file ran zero times:
deploy-dev.ymlison: push: branches: [dev]and lives ondevunder exactlythe same release-cut lag as this file. Same branch, same merges, same lag — the
only difference is which ref the trigger is read from. So the chosen trigger is
not a hypothesis about GitHub's behaviour; it is the behaviour this repository
is already relying on for its deploys.
Acceptance criteria
registration ref for each candidate trigger and why the old wording misled.
merge; see What I could not verify.
above: every one already carries
status-in-dev, none is left instatus-in-progress. Nothing to backfill. One note for a maintainer: bug(mcp): schedule validation config is readable but not writable — create/update_agent_schedule omit validation_enabled/prompt/timeout #2759carries a stale
status-readybeside its hand-addedstatus-in-dev. No labelswere touched from this branch.
the reason
pushwas chosen over backporting the file tomain.Relationship to #2814 / #2816
#2814 is the same outage, reported an hour earlier and reaching the same cause.
#2816 deliberately ships the guard rather than the trigger, recording
"wait for the release cut" as an accepted delay; #2822 asks for the trigger fix
itself. The two are complementary and the tests do not overlap — #2816's is a
cross-workflow
dev-vs-mainparity sweep, this one is specific to thisworkflow's trigger and its promotion logic.
They do collide on one line, whichever merges second: #2816 adds an
ACCEPTED_UNTIL_RELEASEentry for("issue-status-on-merge.yml", "pull_request_target"), and this PR removes that trigger. Itstest_every_accepted_entry_names_a_real_divergencethen fails with"this branch no longer declares it". Deleting that one dict entry is the whole
follow-up. Happy to rebase behind #2816 if you would rather land the guard
first — and if #2822 lands first, #2816's remaining six entries and its guard
are unaffected.
Related Issue
Closes #2822
Journey Impact
Journey Impact: none: CI issue-label automation — no promise in tests/journeys/catalog.yaml is exercised by a workflow trigger, and no user-facing surface changes.
Type of Change
Testing
tests/unit/test_2822_issue_status_trigger_registration.py(new, 15 tests) bothpins the trigger statically and — following
test_2533_alembic_head_watch.py'sprecedent — executes the workflow's own
script:under node with stubbedcontext/github/core, because the promotion logic changed shape and asubstring check would not have caught the open-PR case above.
Discriminating run, base =
origin/dev@7b3cf20ain a separate worktree withonly the test file copied in:
The one that passes on both is
test_the_token_still_covers_the_new_call_and_nothing_more— an invariant guard, not a regression assertion. On the base the behavioural
tests fail with
TypeError: Cannot read properties of undefined (reading 'title'),which is precisely what a push payload does to
context.payload.pull_request.Behaviour proven by executing the script: a squash merge promotes the issue its
PR closes (from the body and from the title); an open PR that merely contains
the commit promotes nothing; the
dev→mainrelease PR promotes nothing; aclosed-unmerged PR promotes nothing; every PR in a multi-commit push is
promoted; a push carrying no commits is a no-op rather than a crash; a refused
label write still attempts every issue and still ends
core.setFailed(#2767);a 404 on
status-in-progressis still not a failure.Also run:
The single full-suite failure is
test_2582_portal_uploads.py::test_read_inbox_populates_mime_from_the_extension,which reproduces identically on
origin/devwith none of this diff applied(
1 failed, 32 passedin a clean worktree at7b3cf20a) — pre-existing, anduntouched by anything here. That is why the "all existing tests pass" box above
is left unticked rather than ticked with an asterisk.
actionlintis green on the broken file too — it cannot see this class ofdefect. That is why the guard is a unit test and not a linter rule.
Mutation-checked, so the guards are known to bite rather than assumed to (each
mutation asserts it actually changed the file before the run):
pr.merged_at &&from the PR filter&& pr.base.ref === basepull_request_target: types: [closed]backconst shas = [](kill the commits lookup)The last one fails only once because the
head_commitfallback correctlycovers the single-commit case; the multi-commit test is what catches it.
What I could not verify
after merge, and GitHub exposes no "which events is this workflow registered
for" API —
GET /actions/workflows/265938549returnsstate: activeandnothing about events. What the control above removes is the doubt about the
trigger family:
push: [dev]demonstrably fires on this branch today. Whatis left unverified is only that this particular file, with this
on:block,starts run refactor(ui): unify Health + Ops + Executions into a single Operations nav area #1109 — and that is self-verifying: merging this PR should produce
a
pushrun that moves bug(ci): issue-status-on-merge has not run since its trigger moved to pull_request_target — every merge since 2026-09-14 needed a manual status-in-dev bump #2822 tostatus-in-devwith no human involved,closing AC 2. If no run appears, the diagnosis is wrong and nothing has been
made worse: the promotion is already at zero.
payload.commits[].idandpayload.head_commitagainst the documented pushpayload and the documented commits-API response; it proves the script's logic,
not GitHub's delivery.
pushhonours[skip ci]in a commit message andpull_request_targetdid not. A squashmerge whose title contains
[skip ci]would skip the promotion. No trinitycommit message in the last 3000 on
devcarries[skip ci]or any of itsspellings (checked with
git log --grep), and the alternative — a trigger that onlyworks after a release cut — is what this PR exists to remove.
[skip ci]case or the pre-existingtest_2582failurewas investigated further than stated — neither is caused by this diff, and
chasing either would widen it.
Checklist
Conventional-commit
fix(ci):subject; branchfix/2822-issue-status-on-merge-triggeroffdevandtargeting
dev; three files, +509/−39 againstdev, no unrelated reformatting. Thedocumentation this change requires is AC 1's header comment on the workflow
itself — a bug fix needs no
docs/memory/update under the tiered rules, andnone is included. The script's
core.infolines are unchanged and the newno-commits path logs before returning, so a run that promotes nothing says why.
Screenshots (if applicable)
Not applicable — a workflow trigger change with no UI surface. The observable
artefact is a workflow run, and the first one can only exist after merge.
File, symbol and API references are against
dev@7b3cf20aandmain@646c9314, measured 2026-09-15 ~15:00 UTC. Documentation quotes are fromgithub/docs@main,content/actions/reference/workflows-and-actions/events-that-trigger-workflows.md.