Repository navigation
π fix(automerge): recover adopted PRs red from a since-fixed shared-CI flake - #14503
Conversation
β¦flake An in-place adoption red only because of a shared-CI or base-branch flake fixed on the base since (head behind, CI never re-run) was escalated to 085 as "nothing the hive can push". Before that escalation the 074 escape now tries one recovery when the failing check is green on the base tip: update-branch when the head is behind, else re-run the failed workflow jobs; once per (head, base tip) and three times per adoption, recorded on the stage record. An escalation already in place for the head is released. Fixes #14500 Signed-off-by: Andy Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of No conflicting open PRs or same-file overlaps were found. |
|
@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this repository have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
|
merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at 41f12af. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
There was a problem hiding this comment.
Read automerge_adoption_recovery.go in full and traced the wiring against v5. The design holds together; one small comment/behaviour mismatch worth a look, no blocking defect found.
What I verified:
- Escalation genuinely waits.
escapeStalledAdoptiononly callsrecoverStaleRedAdoptionwhenadoptionStalledReasonis non-empty, andadoptionStale(automerge_contributor_adopt.go:1230-1237) returns false whileadoptionCIPending(obs)or whenobs.HeadSHA != rec.AdoptedHead. So after a re-run the stall is suppressed until CI completes, and after an update-branch the new head is no longer the adopted head β in neither case can the 074 escape fire in the next sweep on the pre-recovery red. On the sweep after a failed recovery,AdoptionRecoveryAttempted(head, baseSHA)short-circuits toskipped:already-attemptedand the escape proceeds as before, which is the intended steady state. - Candidate gate is conservative.
adoptionRecoveryCandidatedefers toSharedCIIncident,BaseRedChecks,ForkGuardChecks(all populated at automerge_stage_labels.go:417-419), hold labels, blocking/needs-human verdicts and the workflow-path refusal, and requiresclassifyCIReason == ciReasonRed.baseClearsFailurerefuses onbase-red:<check>and on an unknown base (base-checks-unknown), so a red base can never trigger an update-branch. - Bounds.
MarkAdoptionRecoveryis idempotent per(head, base)pair and incrementsAdoptRecoveryCount, capped at 3 byadoptionRecoveryMaxAttempts; both reset inMarkYieldedandClearAdoption. Update-branch errors reuseclassifyUpdateBranchError(automerge_sweep.go:2263-2285) and the up-to-date case falls through to the re-run rather than consuming a second attempt.MarkHivePushedHeadis already gated onAdoptedInPlace()(store.go:954), so the merge commit is attributed to the hive, not the author.
The one thing to reconcile (low): the doc comment at automerge_adoption_recovery.go:172-173 says API failures are "treated as a skip β¦ without consuming the attempt", but store.MarkAdoptionRecovery runs at :216, before the action, so update-branch-failed (:242), list-runs-failed (:265) and rerun-refused (:281) do consume the (head, base) attempt and the lifetime count. That matches the store comment at store.go:1060 ("recorded before the API call β¦ so a refused call is not retried each sweep") and is the safer choice for a 403, but a transient ListRepositoryWorkflowRuns error will spend one of the three attempts and let escalation proceed on the next sweep. Either fix the comment at :172 or move the mark to after the compare/base-check reads for the transient cases β your call; just flagging the two comments disagree.
β hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Fixes #14500
Problem
Adopted contributor PRs #14430, #14440, #14456 (
stage/074-adopted-by-hive, in-place on the fork branch) sat red only because ofTestPollTmuxOutputForAgent_WiresThrashBreakerβ the shared-CI flake in #14406, fixed onv5by #14415. Their heads were 15β22 commits behindv5and never ran CI again, so the 074 escape read the stale red asnothing the hive can pushand escalated them to085-needs-decision. An update-branch or a re-run would have made them green.Fix
escapeStalledAdoptionnow runs a recovery step before escalating a red adopted head (automerge_adoption_recovery.go):required-check-failing:<name>is completed green on the base tip; when the base does not run that check (PR-only workflows) or no name is known, the base tip must have completed checks and none red.executeUpdateBranchEffect(GitHub update-branch; works on a maintainer-editable fork, errors classified as before). Head current (or GitHub says up to date) βRerunFailedJobsByIDon the latest failed run per workflow.(head, base tip)(Record.AdoptRecoveryAttempt) andadoptionRecoveryMaxAttempts = 3per adoption (AdoptRecoveryCount), both reset with the adoption. Marked before the API call, like every adoption path.085happens only after an attempt (or a skip) and the head is still red. API errors are logged and skipped without consuming the attempt.One structured line per decision:
msg="adopted PR recovery" reason=update-branch|rerun-failed-jobs|skipped:<why>.Tests
Table-driven
TestAdoptedStaleRedRecoveryover the adopt harness (update-branch, rerun, base-red, base-unknown, listing failure, fork-not-editable, up-to-date fallthrough, no failed runs), plus the one-attempt-then-escalate / release-on-base-move sequence, the lifetime budget, and unit tables for the candidate predicate,baseClearsFailureand the reason parser. No sleeps.Docs
src/docs/hive-merge.md,src/docs/labels-and-control-signals.md; changelog fragmentchangelog.d/fixed-14500-adopted-stale-red-recovery.md.