fix(pr-e2e): make the eviction retry reachable and kill orphaned central runs - #191
Merged
Merged
Conversation
…ral runs
Two defects, both surfaced by os#367 finishing with no report on 2026-07-30.
1. errexit swallowed every eviction. `run:` steps execute under
`bash -e` (the job log shows `shell: /usr/bin/bash -e {0}`) and the
step's `set -uo pipefail` does NOT clear errexit. The bare
`poll_run; rc=$?` therefore terminated the step the instant poll_run
returned non-zero, so the `rc -eq 2` eviction branch below it was
unreachable dead code. An evicted run died as a bare "Process
completed with exit code 2" — no conclusion written, no retry, and
the gate then reported "unknown" with no report link. Guarding the
call keeps errexit on for everything else while letting the return
value reach $rc.
2. Superseded callers left orphaned central runs. Removing and
re-adding the label starts a second caller; the first is cancelled by
`concurrency: pr-e2e-<PR>` but the central run it already dispatched
keeps going. Nothing polls it, its result is discarded, and it holds
the single env runner for a full ~74-min suite while the real run
queues behind it — os#394 ran twice this way. Cleanup cannot live in
the dying caller: a cancelled job is the least reliable place to run
anything, and a caller killed before its dispatch step published
outputs has no run id to clean up with. The surviving caller now
cancels any still-active central run for the same PR before claiming
a slot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Why
os#367ran the full pipeline and finished with no report and no verdict(run). Root-causing it turned up two
distinct defects — neither of them the queue design itself.
1.
bash -emade the eviction retry unreachableGitHub runs
run:steps underbash -e(the job log literally printsshell: /usr/bin/bash -e {0}), and the step'sset -uo pipefaildoes not clear errexit.So this line:
terminates the step the instant
poll_runreturns non-zero.rc=$?never executes, and theif [ "$rc" -eq 2 ]eviction branch beneath it was dead code from the day it was written.Reproduction, matching the failing run exactly:
What actually happened to os#367:
Central run
30584429876shows the textbook eviction signature —cancelledwith zero jobsever started. It was evicted from the single pending slot when another PR dispatched at 22:15
(GitHub keeps only one pending run per concurrency group). The retry existed to handle precisely
this and could never fire, so
conclusion=was never written and the gate reportedunknownwith no report link.
2. Superseded callers leave orphaned central runs
Removing and re-adding the label starts a second caller.
concurrency: pr-e2e-<PR>cancels thefirst — but the central run it already dispatched keeps running. Nothing polls it, its result
is discarded, and it occupies the single env runner for a full ~74-minute suite while the real
run queues behind it.
os#394ran twice this way today; the zombie had to be cancelled by hand.The existing
if: cancelled()cleanup step cannot fix this: it asks a process being killed toclean up its own child, and a caller killed before its dispatch step published outputs has no run
id to clean up with.
What changed
rc=0; poll_run || rc=$?— the return value reaches$rcinstead of terminating the shell.errexit stays on for everything else.
cancel_stale_siblings(), called at the top of each dispatch attempt: the survivingcaller cancels any still-active central run for this same PR before claiming a slot. Only one
caller per PR is ever alive, so any other active run with this exact title is by definition an
orphan. It returns 0 unconditionally and tolerates a failing
gh, so housekeeping can neverabort the PR check.
Verification
ruby -ryamlparse +bash -non the extracted step — all three repos.poll_runreturning 2 twice: now retries and exits 0 withconclusion=success(was: immediate exit 2).cancel_stale_siblingsagainst a stubbed run list — cancels the orphan, spares our own run,ignores
completedruns, ignores other PRs, and ignoreslms#394when we areos#394.ghregression: step continues, exit 0.Same change is going into
os,lms, andiblai-web-frontendtogether, since all threecallers share this step.
Important
Because
pull_requestruns use the workflow from the merge commit, already-open PRs keeprunning the old step until
mainis merged into their branch. That staleness is what let apre-guard caller evict
os#367in the first place.