Skip to content

fix(pr-e2e): make the eviction retry reachable and kill orphaned central runs - #191

Merged
bnsoni merged 1 commit into
mainfrom
fix/pr-e2e-eviction-retry-and-orphan-cleanup
Jul 30, 2026
Merged

fix(pr-e2e): make the eviction retry reachable and kill orphaned central runs#191
bnsoni merged 1 commit into
mainfrom
fix/pr-e2e-eviction-retry-and-orphan-cleanup

Conversation

@bnsoni

@bnsoni bnsoni commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Why

os#367 ran 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 -e made the eviction retry unreachable

GitHub runs run: steps under bash -e (the job log literally prints
shell: /usr/bin/bash -e {0}), and the step's set -uo pipefail does not clear errexit.
So this line:

poll_run; rc=$?

terminates the step the instant poll_run returns non-zero. rc=$? never executes, and the
if [ "$rc" -eq 2 ] eviction branch beneath it was dead code from the day it was written.

Reproduction, matching the failing run exactly:

$ bash -e -c 'set -uo pipefail; poll_run(){ return 2; }
    for a in 1 2 3; do poll_run; rc=$?
      if [ "$rc" -eq 2 ]; then echo "RETRY $a"; continue; fi; break; done
    echo "conclusion=done"'
$ echo $?
2                     # nothing printed, no retry — exactly os#367's "exit code 2"

What actually happened to os#367:

21:41:30  stagger 35s (PR 367)
21:42:14  dispatched run 30584429876 (attempt 1)
22:15:44  ##[error]Process completed with exit code 2

Central run 30584429876 shows the textbook eviction signature — cancelled with zero jobs
ever 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 reported unknown
with 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 the
first — 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#394 ran 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 to
clean 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 $rc instead of terminating the shell.
    errexit stays on for everything else.
  • New cancel_stale_siblings(), called at the top of each dispatch attempt: the surviving
    caller 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 never
    abort the PR check.

Verification

  • ruby -ryaml parse + bash -n on the extracted step — all three repos.
  • Retry loop with a stubbed poll_run returning 2 twice: now retries and exits 0 with
    conclusion=success (was: immediate exit 2).
  • cancel_stale_siblings against a stubbed run list — cancels the orphan, spares our own run,
    ignores completed runs, ignores other PRs, and ignores lms#394 when we are os#394.
  • Failing-gh regression: step continues, exit 0.

Same change is going into os, lms, and iblai-web-frontend together, since all three
callers share this step.

Important

Because pull_request runs use the workflow from the merge commit, already-open PRs keep
running the old step until main is merged into their branch. That staleness is what let a
pre-guard caller evict os#367 in the first place.

…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>
@bnsoni
bnsoni requested a review from mamigot as a code owner July 30, 2026 23:04
@bnsoni
bnsoni merged commit cefd0ab into main Jul 30, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant