infra(ci): the auto-update bot can reach a state where no PR is ever eligible — selection is conditioned on an event only selection causes #17743

Activity

1. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   on Sep 28, 2026
2. github-actions

   added

   infrastructure

   on Sep 28, 2026
3. mrveiss

   added this to the v0.9.5 - CI and guards milestone on Sep 28, 2026
4. mrveiss commented on Sep 28, 2026

   

   mrveiss

   on Sep 28, 2026

   OwnerAuthor

   More actions

   Premise check from the selection block itself: four conditions, and one of them is the "never eligible" state in the title

   Read at origin/main ab05ba3aef, from the code rather than its comments. Present tense.

   1. The stale-bound rescue cannot fire without a computable age — this is the starvation

   STATE=$(check_pr_state "$PR")          # "<count>|<oldest non-terminal startedAt, or empty>"
   COUNT="${STATE%%|*}" ; OLDEST_START="${STATE#*|}"
   if [ "$COUNT" -gt 0 ]; then
     AGE=""
     if [ -n "$OLDEST_START" ]; then … AGE=$((NOW_EPOCH - STARTED_EPOCH)) ; fi
     if [ -n "$AGE" ] && [ "$AGE" -gt "$STALE_BOUND_SECONDS" ]; then
         …eligible (the #13401 wedged-run rescue)
     else
         # "Genuinely still running (or age unknown) — do NOT refresh"
         SKIPPED_BUSY=$((SKIPPED_BUSY + 1)); continue
     fi
   fi

   AGE is computed only from min(.startedAt) over the non-terminal checks. When every non-terminal check carries no startedAt, OLDEST_START is empty, AGE stays empty, [ -n "$AGE" ] is false, and the PR is skipped as busy — on this pass and on every future pass, because nothing else can make it eligible.

   Three real shapes have no startedAt: a Check Run still QUEUED and never started, a legacy Status context in PENDING (the API has no startedAt for those at all — and the --jq filter deliberately includes them via .state == "PENDING"), and a wedged run sitting in status: pending with an empty jobs array.

   That last one is the point: the #13401 escape hatch exists precisely for wedged runs, and a wedged run is the case whose age cannot be computed. The rescue is unreachable by the population it was written for. The branch's own comment says "or age unknown" — the behaviour is deliberate in the code and the consequence is not stated.

   2. The listing is truncated below the PR cap, and the FIFO sort runs after the truncation

   OPEN_PRS=$(gh pr list --repo "$REPO" --base main --state open --json number,headRefName …)

   No --limit, so gh's default applies — gh pr list --help: -L, --limit int Maximum number of items to fetch (default 30). The open-PR cap is 40 (tools/git-hooks/pre-push:128, AUTOBOT_OPEN_PR_CAP:-40}).

   So between 31 and 40 open PRs the listing silently drops the tail, and the FIFO ordering (sort -n, "oldest PR first") is applied to the 30 that survived, not to the queue. Latent today — 19 open PRs — and live the moment the queue passes 30, which the cap permits.

   3. A failed behind_by reads as "not behind"

   BEHIND=$(gh api "repos/$REPO/compare/main...$HEAD_REF" --jq '.behind_by' 2>/dev/null || echo "")
   case "$BEHIND" in ''|0) ;; *) OUTDATED="$OUTDATED $PR" ;; esac

   Empty and zero take the same branch, so a transient API failure removes the PR from the outdated set for that run. The summary admits the conflation and exits successfully: "All PR branches are up to date (or none could be listed)." then exit 0.

   4. And the same collapse points the other way one function later

   check_pr_state ends || echo "0|", so a failed gh pr view yields COUNT=0 → the PR reads as idle and is refreshed. Condition 3 fails closed (a PR drops out), condition 4 fails open (a busy PR is refreshed anyway). Two unknown-states, two opposite defaults, neither distinguishable in the log from a real measurement.

   Verdict

   Premise stands, and the "no PR is ever eligible" state is condition 1 — it is permanent for an affected PR rather than transient, and it disables the mechanism written to rescue exactly that PR. Conditions 2–4 are three further ways a PR leaves the set without anything saying so. Fixing 1 needs an eligibility path that does not depend on an age: e.g. treat a non-terminal check with no startedAt as having started when the run was created, or count consecutive skipped passes and force eligibility past a bound.
5. mrveiss commented on Oct 3, 2026

   

   mrveiss

   on Oct 3, 2026

   OwnerAuthor

   More actions

   Screen (read-only, current origin/main): reachable, and the sequence is shorter than the issue states

   Read against .github/workflows/auto-update-pr-branches.yml on current origin/main. Three facts settle it.

   1. The trigger is push to main and nothing else

   on:
     push:
       branches:

   No schedule:, no workflow_dispatch:. So the queue state is re-evaluated only when something merges — and a human cannot even trigger it manually, which is why the cost tally above records three hand-run update-branch calls rather than one re-run.

   2. The 90-second-bound escape hatch exists, and cannot fire on its own

   STALE_BOUND_SECONDS: '5400' (90 min) at :23, applied at :94: a PR whose oldest non-terminal check is older than the bound is treated as wedged and refreshed anyway (falls through to :103). Otherwise :96-99 increments SKIPPED_BUSY and continues to the next behind PR — no break, confirming #14022's fix is intact.

   But the bound is evaluated inside the push-triggered job. It cannot re-evaluate anything by itself, so it only helps if a later push happens to arrive — which is the thing the fixpoint prevents.

   3. The workflow already detects the state it cannot escape

   :116-117 emits, verbatim:

   ::warning::auto-update-pr-branches: 0 PRs refreshed this push — all N candidate(s) at the head of the queue are still mid-run. Queue is busy, not empty.

   The condition is named and reported, then nothing acts on it. The signal already exists; only the second trigger is missing.

   The concrete sequence, as asked

   1. Every open PR targeting main is behind base, so none is mergeable (the merge gate requires not behind base).
   2. Something merges → one push → workflow fires → every behind PR has a non-terminal check younger than 90 min → all skipped, UPDATED=0, the :117 warning is emitted.
   3. No further push can occur: every PR is behind, so nothing can merge.
   4. Those runs finish. Every PR is now behind and idle — fully eligible.
   5. No trigger exists to notice. The queue stays wedged until a human runs update-branch by hand, per PR.

   This is weaker than the issue's framing, and therefore worse. The issue reads as though the queue must stay all-mid-run. It does not: the PRs only have to be all-mid-run at the single instant of the last push, because after that instant nothing re-evaluates at all. One unlucky moment wedges the queue indefinitely, and the 90-minute bound is irrelevant to that path since it never gets to run again.

   On the remedy

   The proposed second trigger is correct and absent. Two separable pieces, worth splitting by risk:

   • workflow_dispatch: — zero-risk, independent of the design question, and converts step 5 from N manual API calls into one button. Nothing about the fixpoint argument has to be settled first.
   • A time-based trigger (schedule:, or "no merge for N minutes") — the real fix, and the one that needs the N chosen against the 90-minute bound so a scheduled run cannot reset a branch the bound would have protected.

   MAX_BRANCH_UPDATES: '1' at :22 is not implicated: one refresh per evaluation is fine once evaluations are not conditioned on merges.