diff --git a/changelog.d/fixed-14500-adopted-stale-red-recovery.md b/changelog.d/fixed-14500-adopted-stale-red-recovery.md new file mode 100644 index 0000000000..1c0cff3dbc --- /dev/null +++ b/changelog.d/fixed-14500-adopted-stale-red-recovery.md @@ -0,0 +1 @@ +- Adopted contributor PRs that were red only because of a shared-CI or base-branch flake fixed on the base since — their heads 15–22 commits behind `v5` with CI never re-run (hivecommons/hive#14430, #14440, #14456 after #14406 was fixed by #14415) — were escalated to `stage/085-needs-decision` as "nothing the hive can push". The 074 escape now tries one recovery first when the head's failing check is green on the base tip: it merges the base into the head (GitHub update-branch, also on a maintainer-editable fork) when the head is behind, or re-runs the failed workflow jobs when it is current, at most once per (head, base tip) and three times per adoption, recorded on the stage record; an escalation already in place for that head is released, and 085 waits until the attempt was made and the head is still red. One `adopted PR recovery` log line names the outcome (`update-branch`, `rerun-failed-jobs`, `skipped:`) (hivecommons/hive#14500). diff --git a/src/docs/hive-merge.md b/src/docs/hive-merge.md index f4d09994ca..458142f70d 100644 --- a/src/docs/hive-merge.md +++ b/src/docs/hive-merge.md @@ -580,8 +580,18 @@ re-homes immediately (`reason=push-rehome`). A pushable `needs-rebase` or conflicting branch first gets the normal fixer window; if its head remains unchanged past the 074 dwell, the same sweep supersedes it (`reason=rebase-rehome`) rather than parking at 085. Previously escalated -adoptions are re-evaluated for these recoveries too (#13975). An adoption with nothing the hive can -push past the dwell goes to `stage/085-needs-decision` as +adoptions are re-evaluated for these recoveries too (#13975). A red adopted +head whose failing check is green on the base branch tip — the shared-CI +flake or red base it fell to was fixed since, but the head never ran CI +again — is recovered before anyone is asked: the sweep merges the base into +the head when it is behind (GitHub update-branch, which works on a fork the +maintainers may edit) or re-runs the failed workflow jobs when it is current, +at most once per (head, base tip) and three times per adoption, logged as +`adopted PR recovery` with `reason=update-branch`, `rerun-failed-jobs` or +`skipped:`; an escalation already in place for that head is released +(hivecommons/hive#14500). An adoption with nothing the hive can +push past the dwell — after that recovery was tried and the head is still +red — goes to `stage/085-needs-decision` as `adoption-stalled:` with one comment naming the blocker, so the dashboard shows a person's item, not a silent 074; "waiting on maintainer lgtm" is only ever the state at 080+ (hivecommons/hive#12858). The dwell runs diff --git a/src/docs/labels-and-control-signals.md b/src/docs/labels-and-control-signals.md index 349a8152c2..a0b682882e 100644 --- a/src/docs/labels-and-control-signals.md +++ b/src/docs/labels-and-control-signals.md @@ -884,6 +884,11 @@ When a clanker-authored contributor PR goes red the hive does not just relabel i not done. The sweep re-evaluates existing 074 records every pass, so a record adopted before this rule existed converts too (hivecommons/hive#12858). + - Stale-red recovery (no re-home): an in-place head red only from a + since-fixed base/shared-CI flake — its failing check is green on the + base tip — is updated from the base when behind, or has its failed + jobs re-run when current, once per (head, base tip), before the dwell + escalates it to 085 (hivecommons/hive#14500). A PR carrying `hold`, `do-not-merge` or `do-not-merge/hold` is never adopted (a human parked it; one log line, the stage stays 072). A PR whose diff trips the contributor lane's diff fences — it touches diff --git a/src/pkg/github/automerge/automerge_adoption_recovery.go b/src/pkg/github/automerge/automerge_adoption_recovery.go new file mode 100644 index 0000000000..46eaa89f54 --- /dev/null +++ b/src/pkg/github/automerge/automerge_adoption_recovery.go @@ -0,0 +1,328 @@ +package automerge + +import ( + "context" + "sort" + "strings" + + gh "github.com/google/go-github/v72/github" + hgithub "github.com/hivecommons/hive/pkg/github" + "github.com/hivecommons/hive/pkg/stagelabels" +) + +// Stale-red recovery for adopted PRs (hivecommons/hive#14500). +// +// An in-place adoption can sit red for a reason nobody owns: the check that +// failed was a shared-CI flake or a red base branch, fixed on the base since, +// while the adopted head never ran CI again. Before this step the 074 escape +// read that as "nothing the hive can push" and handed the PR to a person at +// 085 — observed on hivecommons/hive#14430, #14440 and #14456 after the +// #14406 flake was fixed by #14415. The fix was a branch update or a re-run +// away. +// +// When the dwell would escalate a red adopted head whose failing check is +// green on the base branch tip, the hive first tries one recovery: if the +// head is behind the base, update-branch (a merge commit, no human commit +// rewritten; GitHub allows it on a fork when maintainers may edit); if it is +// current, re-run the failed workflow jobs for the head. One attempt per +// (head, base tip) pair and a small lifetime budget per adoption are recorded +// on the stage record, so the step never loops. Escalation to 085 waits +// until an attempt was made and the head is still red. + +const ( + adoptionRecoveryUpdateBranch = "update-branch" + adoptionRecoveryRerun = "rerun-failed-jobs" + adoptionRecoverySkipPrefix = "skipped:" + // adoptionRecoveryClaimReason is the update-branch claim reason recorded + // when the hive refreshes a stale-red adopted head. + adoptionRecoveryClaimReason = "adopted-recovery" + // adoptionRecoveryMaxAttempts bounds recoveries over the life of one + // adoption: a base that keeps moving under a genuinely red head gets a + // few refreshes, then the 074 escape runs as before. + adoptionRecoveryMaxAttempts = 3 +) + +// adoptionRecovery is what recoverStaleRedAdoption did for one sweep. +type adoptionRecovery struct { + // Attempted means an update-branch or re-run was issued: the escalation + // waits for CI on the result. + Attempted bool + // Reason is the structured outcome: update-branch, rerun-failed-jobs or + // skipped:. + Reason string +} + +func adoptionRecoverySkipped(why string) adoptionRecovery { + return adoptionRecovery{Reason: adoptionRecoverySkipPrefix + why} +} + +// adoptionFailingCheck names the check the lane reason blames, or "". +func adoptionFailingCheck(reason string) string { + for _, prefix := range []string{"required-check-failing:", "check-failing:"} { + if name, ok := strings.CutPrefix(reason, prefix); ok { + return strings.TrimSpace(name) + } + } + return "" +} + +// adoptionRecoveryCandidate reports a hive-owned in-place adoption whose +// only blocker is red CI on the adopted head, with no open shared-CI +// incident, red base, fork guard, hold label or review blocker to defer to, +// and no workflow-path refusal pinning it to a person. +func adoptionRecoveryCandidate(obs stageObservation, rec stagelabels.Record) bool { + if !rec.AdoptedInPlace() || rec.AdoptedBy > 0 || obs.HeadSHA == "" || obs.HeadSHA != rec.AdoptedHead { + return false + } + if rec.AdoptionEscalated(obs.HeadSHA) && rec.AdoptEscalatedReason == adoptionWorkflowRefusedReason { + return false + } + if adoptionBlocked(obs.Labels) != "" || verdictBlocks(obs.Detail) || verdictNeedsHuman(obs.Detail) { + return false + } + if obs.SharedCIIncident > 0 || len(obs.BaseRedChecks) > 0 || len(obs.ForkGuardChecks) > 0 { + return false + } + reason := nonEmpty(obs.Reason, obs.LaneReason) + if strings.HasPrefix(reason, stageReasonBaseBranchRed) { + return false + } + return classifyCIReason(reason) == ciReasonRed +} + +// baseCheckConclusionIsRed mirrors the CI gate's rollup reading of a +// completed check-run conclusion. +func baseCheckConclusionIsRed(conclusion string) bool { + switch conclusion { + case "failure", "timed_out", "cancelled", "action_required", "startup_failure", "stale": + return true + } + return false +} + +// baseCheckState is the latest completed check run per name on a ref. +type baseCheckState struct { + completed map[string]bool // name → present + red map[string]bool // name → red +} + +// baseChecks lists the latest completed check run per name on ref. +func (c *Engine) baseChecks(ctx context.Context, owner, repo, ref string) (baseCheckState, error) { + ctx = hgithub.WithRESTCaller(ctx, "hive:adopted_recovery_base_checks") + latest := map[string]*gh.CheckRun{} + opts := &gh.ListCheckRunsOptions{ListOptions: gh.ListOptions{PerPage: 100}} + for { + runs, resp, err := c.gh.Checks.ListCheckRunsForRef(ctx, owner, repo, ref, opts) + if err != nil { + return baseCheckState{}, err + } + if runs != nil { + for _, cr := range runs.CheckRuns { + if cr == nil || cr.GetName() == "" { + continue + } + if prev := latest[cr.GetName()]; prev == nil || cr.GetID() > prev.GetID() { + latest[cr.GetName()] = cr + } + } + } + if resp == nil || resp.NextPage == 0 { + break + } + opts.Page = resp.NextPage + } + st := baseCheckState{completed: map[string]bool{}, red: map[string]bool{}} + for name, cr := range latest { + if cr.GetStatus() != "completed" { + continue + } + st.completed[name] = true + if baseCheckConclusionIsRed(cr.GetConclusion()) { + st.red[name] = true + } + } + return st, nil +} + +// baseClearsFailure reports whether the base tip's checks say the head's +// failure is not the PR's: the blamed check completed green on the base, or +// — when the base does not run that check (PR-only workflows) or the +// reason names none — the base tip has completed checks and none is red. +// skip names why not. +func baseClearsFailure(st baseCheckState, failing string) (ok bool, skip string) { + if failing != "" && st.completed[failing] { + if st.red[failing] { + return false, "base-red:" + failing + } + return true, "" + } + if len(st.completed) == 0 { + return false, "base-checks-unknown" + } + if len(st.red) > 0 { + return false, "base-red" + } + return true, "" +} + +// recoverStaleRedAdoption runs where the 074 escape would otherwise +// escalate a red adopted head. It issues at most one update-branch or +// re-run per (head, base tip) and reports whether one was issued this +// sweep; a skipped recovery leaves the escalation to proceed as before. +// API failures are logged and treated as a skip for this sweep without +// consuming the attempt, so a transient error is retried next sweep. +func (c *Engine) recoverStaleRedAdoption(ctx context.Context, store *stagelabels.Store, key string, obs stageObservation, rec stagelabels.Record) adoptionRecovery { + if !adoptionRecoveryCandidate(obs, rec) { + return adoptionRecoverySkipped("not-a-candidate") + } + logArgs := func(extra ...any) []any { + return append([]any{"repo", obs.Display, "pr", obs.Number, "head", obs.HeadSHA, "base", obs.BaseRef, "lane_reason", nonEmpty(obs.Reason, obs.LaneReason)}, extra...) + } + if rec.AdoptRecoveryCount >= adoptionRecoveryMaxAttempts { + out := adoptionRecoverySkipped("attempts-exhausted") + c.info("adopted PR recovery", logArgs("reason", out.Reason, "attempts", rec.AdoptRecoveryCount)...) + return out + } + baseRef := strings.TrimSpace(obs.BaseRef) + if baseRef == "" { + return adoptionRecoverySkipped("no-base-ref") + } + restCtx := hgithub.WithRESTCaller(ctx, "hive:adopted_recovery_compare") + cmp, _, err := c.gh.Repositories.CompareCommits(restCtx, obs.Owner, obs.Repo, baseRef, obs.HeadSHA, &gh.ListOptions{PerPage: 1}) + if err != nil { + c.warn("adopted PR recovery", logArgs("reason", adoptionRecoverySkipPrefix+"compare-failed", "error", err)...) + return adoptionRecoverySkipped("compare-failed") + } + baseSHA := cmp.GetBaseCommit().GetSHA() + if baseSHA == "" { + return adoptionRecoverySkipped("base-sha-unknown") + } + if rec.AdoptionRecoveryAttempted(obs.HeadSHA, baseSHA) { + // Steady state after an attempt that left the head red: the escape + // proceeds; no log, this repeats every sweep until the head moves. + return adoptionRecoverySkipped("already-attempted") + } + st, err := c.baseChecks(ctx, obs.Owner, obs.Repo, baseSHA) + if err != nil { + c.warn("adopted PR recovery", logArgs("reason", adoptionRecoverySkipPrefix+"base-checks-failed", "error", err)...) + return adoptionRecoverySkipped("base-checks-failed") + } + failing := adoptionFailingCheck(nonEmpty(obs.Reason, obs.LaneReason)) + if ok, skip := baseClearsFailure(st, failing); !ok { + out := adoptionRecoverySkipped(skip) + c.info("adopted PR recovery", logArgs("reason", out.Reason, "base_sha", baseSHA, "failing_check", failing)...) + return out + } + store.MarkAdoptionRecovery(key, obs.HeadSHA, baseSHA) + behind := cmp.GetBehindBy() + if behind > 0 { + out, done := c.recoverByUpdateBranch(ctx, store, key, obs, logArgs("base_sha", baseSHA, "behind_by", behind, "failing_check", failing)) + if done { + return out + } + } + return c.recoverByRerun(ctx, obs, logArgs("base_sha", baseSHA, "behind_by", behind, "failing_check", failing)) +} + +// recoverByUpdateBranch merges the base into the adopted head. done is +// false only when GitHub says the head is already current, in which case +// the caller falls through to a re-run. +func (c *Engine) recoverByUpdateBranch(ctx context.Context, store *stagelabels.Store, key string, obs stageObservation, logArgs []any) (adoptionRecovery, bool) { + fork := obs.HeadRepo != "" && !strings.EqualFold(obs.HeadRepo, obs.Owner+"/"+obs.Repo) + if err := c.executeUpdateBranchEffect(ctx, obs.Owner, obs.Repo, obs.Number, obs.HeadSHA, adoptionRecoveryClaimReason); err != nil { + var out adoptionRecovery + switch classifyUpdateBranchError(err, fork) { + case updateBranchUpToDate: + return adoptionRecovery{}, false + case updateBranchForkNotEditable: + out = adoptionRecoverySkipped("fork-not-editable") + case updateBranchConflict: + out = adoptionRecoverySkipped("conflict") + default: + out = adoptionRecoverySkipped("update-branch-failed") + } + c.warn("adopted PR recovery", append(logArgs, "reason", out.Reason, "error", err)...) + return out, true + } + c.info("adopted PR recovery", append(logArgs, "reason", adoptionRecoveryUpdateBranch)...) + // The merge commit is the hive's; when GitHub already shows it, record + // the push so the attribution check does not read it as the author's. + if pr, _, err := c.gh.PullRequests.Get(hgithub.WithRESTCaller(ctx, "hive:adopted_recovery_head"), obs.Owner, obs.Repo, obs.Number); err == nil { + if head := pr.GetHead().GetSHA(); head != "" && head != obs.HeadSHA { + store.MarkHivePushedHead(key, head) + } + } + return adoptionRecovery{Attempted: true, Reason: adoptionRecoveryUpdateBranch}, true +} + +// recoverByRerun re-runs the failed jobs of the latest failed run of every +// workflow on the adopted head. +func (c *Engine) recoverByRerun(ctx context.Context, obs stageObservation, logArgs []any) adoptionRecovery { + ctx = hgithub.WithRESTCaller(ctx, "hive:adopted_recovery_rerun") + runs, err := c.latestFailedWorkflowRuns(ctx, obs.Owner, obs.Repo, obs.HeadSHA) + if err != nil { + c.warn("adopted PR recovery", append(logArgs, "reason", adoptionRecoverySkipPrefix+"list-runs-failed", "error", err)...) + return adoptionRecoverySkipped("list-runs-failed") + } + if len(runs) == 0 { + out := adoptionRecoverySkipped("no-failed-runs") + c.info("adopted PR recovery", append(logArgs, "reason", out.Reason)...) + return out + } + var rerun []int64 + for _, run := range runs { + if _, err := c.gh.Actions.RerunFailedJobsByID(ctx, obs.Owner, obs.Repo, run.GetID()); err != nil { + c.warn("adopted PR recovery: re-run refused", append(logArgs, "run", run.GetID(), "error", err)...) + continue + } + rerun = append(rerun, run.GetID()) + } + if len(rerun) == 0 { + out := adoptionRecoverySkipped("rerun-refused") + c.warn("adopted PR recovery", append(logArgs, "reason", out.Reason, "runs", len(runs))...) + return out + } + c.info("adopted PR recovery", append(logArgs, "reason", adoptionRecoveryRerun, "runs", rerun)...) + return adoptionRecovery{Attempted: true, Reason: adoptionRecoveryRerun} +} + +// latestFailedWorkflowRuns returns, per workflow, the newest run for sha +// that completed as failure or timed_out. +func (c *Engine) latestFailedWorkflowRuns(ctx context.Context, owner, repo, sha string) ([]*gh.WorkflowRun, error) { + opts := &gh.ListWorkflowRunsOptions{HeadSHA: sha, ListOptions: gh.ListOptions{PerPage: 100}} + latest := map[int64]*gh.WorkflowRun{} + for { + runs, resp, err := c.gh.Actions.ListRepositoryWorkflowRuns(ctx, owner, repo, opts) + if err != nil { + return nil, err + } + if runs != nil { + for _, run := range runs.WorkflowRuns { + if run == nil || run.GetHeadSHA() != sha { + continue + } + cur, ok := latest[run.GetWorkflowID()] + if !ok || run.GetRunNumber() > cur.GetRunNumber() || + (run.GetRunNumber() == cur.GetRunNumber() && run.GetRunAttempt() > cur.GetRunAttempt()) { + latest[run.GetWorkflowID()] = run + } + } + } + if resp == nil || resp.NextPage == 0 { + break + } + opts.ListOptions.Page = resp.NextPage + } + var out []*gh.WorkflowRun + for _, run := range latest { + if run.GetStatus() != "completed" { + continue + } + switch run.GetConclusion() { + case "failure", "timed_out": + out = append(out, run) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].GetID() < out[j].GetID() }) + return out, nil +} diff --git a/src/pkg/github/automerge/automerge_adoption_recovery_test.go b/src/pkg/github/automerge/automerge_adoption_recovery_test.go new file mode 100644 index 0000000000..a1bd1495b6 --- /dev/null +++ b/src/pkg/github/automerge/automerge_adoption_recovery_test.go @@ -0,0 +1,337 @@ +package automerge + +import ( + "net/http" + "testing" + "time" + + "github.com/hivecommons/hive/pkg/stagelabels" +) + +// Stale-red recovery for adopted PRs (hivecommons/hive#14500). + +const recoveryReason = "required-check-failing:coverage" + +// recoveryHarness is an in-place adoption of PR 7 at head "aaa" whose only +// blocker is the red `coverage` check, with the 074 dwell already elapsed. +func recoveryHarness(t *testing.T) (*adoptHarness, string, func()) { + t.Helper() + p := adoptPolicy() + p.AdoptInPlace = true + h, done := newAdoptHarness(t, ContributorNudgeOutcome{Mode: "comment"}, p) + key := adoptInPlaceAt(h, "aaa", recoveryReason) + h.adopt.compareBase = "base1" + h.clock.advance(adoptedDwell + time.Minute) + return h, key, done +} + +func TestAdoptedStaleRedRecovery(t *testing.T) { + cases := []struct { + name string + setup func(a *adoptAPI) + wantUpdates int + wantReruns int + wantStage stagelabels.Stage + wantAttempt string + }{ + { + name: "behind with the check green on base updates the branch and holds 074", + setup: func(a *adoptAPI) { + a.compareBehind = 15 + a.baseChecks = map[string]string{"coverage": "success", "build": "success"} + }, + wantUpdates: 1, wantStage: stagelabels.AdoptedByHive, wantAttempt: "aaa@base1", + }, + { + name: "current head with the check green on base re-runs the failed jobs", + setup: func(a *adoptAPI) { + a.baseChecks = map[string]string{"coverage": "success"} + a.failedRuns = []int64{41, 42} + }, + wantReruns: 2, wantStage: stagelabels.AdoptedByHive, wantAttempt: "aaa@base1", + }, + { + name: "base does not run the check but is fully green: still recoverable", + setup: func(a *adoptAPI) { + a.compareBehind = 3 + a.baseChecks = map[string]string{"build": "success"} + }, + wantUpdates: 1, wantStage: stagelabels.AdoptedByHive, wantAttempt: "aaa@base1", + }, + { + name: "check still red on base: nothing to recover, escalate as before", + setup: func(a *adoptAPI) { + a.compareBehind = 15 + a.baseChecks = map[string]string{"coverage": "failure"} + }, + wantStage: stagelabels.NeedsDecision, + }, + { + name: "base checks unknown: escalate as before", + setup: func(a *adoptAPI) { + a.compareBehind = 15 + a.baseChecks = map[string]string{"coverage": "pending"} + }, + wantStage: stagelabels.NeedsDecision, + }, + { + name: "base check listing fails: escalate, attempt not consumed", + setup: func(a *adoptAPI) { + a.compareBehind = 15 + a.failBaseChecks = true + }, + wantStage: stagelabels.NeedsDecision, + }, + { + name: "fork head not editable by the app: escalate", + setup: func(a *adoptAPI) { + a.compareBehind = 15 + a.baseChecks = map[string]string{"coverage": "success"} + a.updateBranchStatus = http.StatusUnprocessableEntity + a.updateBranchMessage = "maintainer edits are not allowed" + }, + wantUpdates: 1, wantStage: stagelabels.NeedsDecision, wantAttempt: "aaa@base1", + }, + { + name: "update-branch says up to date: fall through to the re-run", + setup: func(a *adoptAPI) { + a.compareBehind = 1 + a.baseChecks = map[string]string{"coverage": "success"} + a.updateBranchStatus = http.StatusUnprocessableEntity + a.updateBranchMessage = "There are no new commits on the base branch." + a.failedRuns = []int64{41} + }, + wantUpdates: 1, wantReruns: 1, wantStage: stagelabels.AdoptedByHive, wantAttempt: "aaa@base1", + }, + { + name: "current head with no failed runs to re-run: escalate", + setup: func(a *adoptAPI) { + a.baseChecks = map[string]string{"coverage": "success"} + }, + wantStage: stagelabels.NeedsDecision, wantAttempt: "aaa@base1", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h, key, done := recoveryHarness(t) + defer done() + tc.setup(h.adopt) + + res := h.observe(adoptPR(7, "aaa", adoptedLabels...), recoveryReason) + if got := len(h.adopt.branchUpdates); got != tc.wantUpdates { + t.Fatalf("branch updates=%v, want %d", h.adopt.branchUpdates, tc.wantUpdates) + } + if got := len(h.adopt.reruns); got != tc.wantReruns { + t.Fatalf("reruns=%v, want %d", h.adopt.reruns, tc.wantReruns) + } + if len(h.adopt.pulls) != 0 { + t.Fatalf("red CI is never a re-home: %v", h.adopt.pulls) + } + rec, _ := h.e.stageStoreOrShared().Get(key) + if rec.Stage != tc.wantStage { + t.Fatalf("stage=%v, want %v (record %+v)", rec.Stage, tc.wantStage, rec) + } + if rec.AdoptRecoveryAttempt != tc.wantAttempt { + t.Fatalf("recovery attempt=%q, want %q", rec.AdoptRecoveryAttempt, tc.wantAttempt) + } + escalated := tc.wantStage == stagelabels.NeedsDecision + if notes := stalledComments(h.api); (len(notes) == 1) != escalated || (res.Comments == 1) != escalated { + t.Fatalf("stalled notes=%v res=%+v, escalated=%v", notes, res, escalated) + } + if escalated != rec.AdoptionEscalated("aaa") || !rec.AdoptedInPlace() { + t.Fatalf("record after sweep: %+v", rec) + } + for _, u := range h.adopt.branchUpdates { + if u != "7@aaa" { + t.Fatalf("update-branch must name the evaluated head: %v", h.adopt.branchUpdates) + } + } + }) + } +} + +// The same head stays red after the re-run: one attempt per (head, base), +// then the escalation runs; a new hive head starts over. +func TestAdoptedStaleRedRecoveryEscalatesAfterOneAttempt(t *testing.T) { + h, key, done := recoveryHarness(t) + defer done() + h.adopt.baseChecks = map[string]string{"coverage": "success"} + h.adopt.failedRuns = []int64{41} + + h.observe(adoptPR(7, "aaa", adoptedLabels...), recoveryReason) + if len(h.adopt.reruns) != 1 || len(stalledComments(h.api)) != 0 { + t.Fatalf("first sweep: reruns=%v comments=%v", h.adopt.reruns, h.api.comments) + } + + // Re-run finished, still red, same base: no second attempt, escalate. + h.api.reset() + h.clock.advance(time.Hour) + res := h.observe(adoptPR(7, "aaa", adoptedLabels...), recoveryReason) + if len(h.adopt.reruns) != 1 || len(h.adopt.branchUpdates) != 0 { + t.Fatalf("recovery repeated: reruns=%v updates=%v", h.adopt.reruns, h.adopt.branchUpdates) + } + if notes := stalledComments(h.api); len(notes) != 1 || res.Comments != 1 { + t.Fatalf("escalation after the attempt: notes=%v res=%+v", notes, res) + } + rec, _ := h.e.stageStoreOrShared().Get(key) + if rec.Stage != stagelabels.NeedsDecision || !rec.AdoptionEscalated("aaa") || rec.AdoptRecoveryCount != 1 { + t.Fatalf("record: %+v", rec) + } + + // Base moved while escalated: one more attempt releases the escalation. + h.api.reset() + h.adopt.compareBase = "base2" + h.adopt.compareBehind = 4 + h.clock.advance(time.Hour) + h.observe(adoptPR(7, "aaa", stagelabels.NeedsDecision.Label(), "role/contributor", "author/clanker"), recoveryReason) + if len(h.adopt.branchUpdates) != 1 { + t.Fatalf("base moved: updates=%v", h.adopt.branchUpdates) + } + rec, _ = h.e.stageStoreOrShared().Get(key) + if rec.Stage != stagelabels.AdoptedByHive || rec.AdoptionEscalated("aaa") || rec.AdoptRecoveryAttempt != "aaa@base2" || rec.AdoptRecoveryCount != 2 { + t.Fatalf("record after release: %+v", rec) + } + if len(stalledComments(h.api)) != 0 { + t.Fatalf("release must not note: %v", h.api.comments) + } + + // The update landed as a hive head: 074 with a fresh clock. + h.adopt.setCommitAuthor("bbb", testHiveAppBotLogin) + h.api.reset() + h.clock.advance(time.Minute) + h.observe(adoptPR(7, "bbb", adoptedLabels...), "check-pending") + rec, _ = h.e.stageStoreOrShared().Get(key) + if rec.Stage != stagelabels.AdoptedByHive || rec.AdoptedHead != "bbb" || !rec.AdoptedHeadSince().Equal(h.clock.at) { + t.Fatalf("new hive head: %+v", rec) + } +} + +// The lifetime budget: a base that keeps moving under a red head gets +// adoptionRecoveryMaxAttempts refreshes, then the escape runs as before. +func TestAdoptedStaleRedRecoveryBudget(t *testing.T) { + h, key, done := recoveryHarness(t) + defer done() + h.adopt.baseChecks = map[string]string{"coverage": "success"} + h.adopt.compareBehind = 2 + for i := 0; i < adoptionRecoveryMaxAttempts; i++ { + h.adopt.compareBase = "base" + string(rune('1'+i)) + h.observe(adoptPR(7, "aaa", adoptedLabels...), recoveryReason) + h.clock.advance(time.Hour) + } + if len(h.adopt.branchUpdates) != adoptionRecoveryMaxAttempts || len(stalledComments(h.api)) != 0 { + t.Fatalf("within budget: updates=%v comments=%v", h.adopt.branchUpdates, h.api.comments) + } + h.adopt.compareBase = "base9" + h.observe(adoptPR(7, "aaa", adoptedLabels...), recoveryReason) + if len(h.adopt.branchUpdates) != adoptionRecoveryMaxAttempts { + t.Fatalf("budget not enforced: %v", h.adopt.branchUpdates) + } + rec, _ := h.e.stageStoreOrShared().Get(key) + if rec.Stage != stagelabels.NeedsDecision || len(stalledComments(h.api)) != 1 { + t.Fatalf("exhausted budget must escalate: %+v comments=%v", rec, h.api.comments) + } +} + +func TestAdoptionRecoveryCandidate(t *testing.T) { + rec := stagelabels.Record{AdoptedHead: "aaa", AdoptMode: stagelabels.AdoptModeInPlace} + base := func() stageObservation { + return stageObservation{HeadSHA: "aaa", LaneReason: recoveryReason} + } + cases := []struct { + name string + obs func() stageObservation + rec func() stagelabels.Record + want bool + }{ + {"red in-place adoption", base, func() stagelabels.Record { return rec }, true}, + {"check-failing reason", func() stageObservation { o := base(); o.LaneReason = "check-failing:unit"; return o }, func() stagelabels.Record { return rec }, true}, + {"pending CI", func() stageObservation { o := base(); o.LaneReason = "check-pending"; return o }, func() stagelabels.Record { return rec }, false}, + {"green with no verdict", func() stageObservation { o := base(); o.LaneReason = contributorLaneReasonNoVerdict; return o }, func() stagelabels.Record { return rec }, false}, + {"open shared-CI incident", func() stageObservation { o := base(); o.SharedCIIncident = 10528; return o }, func() stagelabels.Record { return rec }, false}, + {"base red", func() stageObservation { o := base(); o.BaseRedChecks = []string{"coverage"}; return o }, func() stagelabels.Record { return rec }, false}, + {"base-red reason", func() stageObservation { o := base(); o.Reason = stageReasonBaseBranchRed + "coverage"; return o }, func() stagelabels.Record { return rec }, false}, + {"fork guard", func() stageObservation { o := base(); o.ForkGuardChecks = []string{"coverage"}; return o }, func() stagelabels.Record { return rec }, false}, + {"hold label", func() stageObservation { o := base(); o.Labels = []string{"do-not-merge/hold"}; return o }, func() stagelabels.Record { return rec }, false}, + {"blocking verdict", func() stageObservation { + o := base() + o.Detail.VerdictKnown = true + o.Detail.Verdict = ContributorLaneVerdict{Configured: true, Verdict: "changes_requested"} + return o + }, func() stagelabels.Record { return rec }, false}, + {"head moved", func() stageObservation { o := base(); o.HeadSHA = "bbb"; return o }, func() stagelabels.Record { return rec }, false}, + {"supersession", base, func() stagelabels.Record { + r := rec + r.AdoptMode = stagelabels.AdoptModeSupersede + r.AdoptedBy = 90 + return r + }, false}, + {"workflow-path refusal", base, func() stagelabels.Record { + r := rec + r.AdoptEscalatedHead, r.AdoptEscalatedReason = "aaa", adoptionWorkflowRefusedReason + return r + }, false}, + {"stalled escalation", base, func() stagelabels.Record { + r := rec + r.AdoptEscalatedHead, r.AdoptEscalatedReason = "aaa", stagelabels.LaneReasonAdoptionStalledPrefix+recoveryReason + return r + }, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := adoptionRecoveryCandidate(tc.obs(), tc.rec()); got != tc.want { + t.Fatalf("candidate=%v, want %v", got, tc.want) + } + }) + } +} + +func TestBaseClearsFailure(t *testing.T) { + st := func(red, green []string) baseCheckState { + s := baseCheckState{completed: map[string]bool{}, red: map[string]bool{}} + for _, n := range red { + s.completed[n], s.red[n] = true, true + } + for _, n := range green { + s.completed[n] = true + } + return s + } + cases := []struct { + name string + st baseCheckState + failing string + wantOK bool + wantSkip string + }{ + {"named check green", st(nil, []string{"coverage"}), "coverage", true, ""}, + {"named check green while another is red", st([]string{"docs"}, []string{"coverage"}), "coverage", true, ""}, + {"no name, nothing completed", st(nil, nil), "", false, "base-checks-unknown"}, + {"named check red on base", st([]string{"coverage"}, nil), "coverage", false, "base-red:coverage"}, + {"check absent, base green", st(nil, []string{"build"}), "coverage", true, ""}, + {"check absent, base red", st([]string{"build"}, nil), "coverage", false, "base-red"}, + {"no name, base green", st(nil, []string{"build"}), "", true, ""}, + {"nothing completed on base", st(nil, nil), "coverage", false, "base-checks-unknown"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ok, skip := baseClearsFailure(tc.st, tc.failing) + if ok != tc.wantOK || skip != tc.wantSkip { + t.Fatalf("got (%v, %q), want (%v, %q)", ok, skip, tc.wantOK, tc.wantSkip) + } + }) + } +} + +func TestAdoptionFailingCheck(t *testing.T) { + for in, want := range map[string]string{ + "required-check-failing:coverage": "coverage", + "check-failing:unit": "unit", + "check-runs": "", + "status-failure": "", + "": "", + } { + if got := adoptionFailingCheck(in); got != want { + t.Errorf("adoptionFailingCheck(%q)=%q, want %q", in, got, want) + } + } +} diff --git a/src/pkg/github/automerge/automerge_contributor_adopt.go b/src/pkg/github/automerge/automerge_contributor_adopt.go index 126ba16fd8..a014602a94 100644 --- a/src/pkg/github/automerge/automerge_contributor_adopt.go +++ b/src/pkg/github/automerge/automerge_contributor_adopt.go @@ -1324,6 +1324,19 @@ func (c *Engine) escapeStalledAdoption(ctx context.Context, store *stagelabels.S if rehomeReason == "" && rec.AdoptedInPlace() && obs.HeadSHA == rec.AdoptedHead && !adoptedHeadDCORed(obs, rec) && adoptionBlocked(obs.Labels) == "" && !verdictNeedsHuman(obs.Detail) && c.pushAccessLost(ctx, obs) { rehomeReason = AdoptReasonPushRehome } + if rehomeReason == "" && adoptionStalledReason(obs, rec, policy, now) != "" { + // Before a red head is handed to a person, try the recovery a + // since-fixed base or shared-CI flake needs (hivecommons/hive#14500): + // an attempt holds the escalation while CI runs on the result, and + // releases one already in place for the same head. + if recovery := c.recoverStaleRedAdoption(ctx, store, key, obs, rec); recovery.Attempted { + if rec.AdoptionEscalated(obs.HeadSHA) { + store.ClearAdoptionEscalation(key) + c.info("stage labels: adoption escalation released for recovery; back to adopted-by-hive", "repo", obs.Display, "pr", obs.Number, "head", obs.HeadSHA, "was", rec.AdoptEscalatedReason, "recovery", recovery.Reason) + } + return adoptionEscape{}, 0, nil + } + } if rec.AdoptionEscalated(obs.HeadSHA) { // A workflow-path refusal is a verdict on the head itself: it holds // until the head moves, whatever the re-home or stall reasons say. diff --git a/src/pkg/github/automerge/automerge_contributor_adopt_test.go b/src/pkg/github/automerge/automerge_contributor_adopt_test.go index d0655cd89c..83491b25e2 100644 --- a/src/pkg/github/automerge/automerge_contributor_adopt_test.go +++ b/src/pkg/github/automerge/automerge_contributor_adopt_test.go @@ -55,6 +55,22 @@ type adoptAPI struct { onPermission func() // failEditStatus, when non-zero, is the status PATCH pulls/n answers. failEditStatus int + // Stale-red recovery fixtures (hivecommons/hive#14500): compareBehind + // and compareBase are what GET compare/base...head reports; baseChecks + // is name → conclusion served by GET commits//check-runs; + // failedRuns are the workflow runs GET actions/runs?head_sha= lists; + // updateBranchStatus, when non-zero, is the status PUT update-branch + // answers with updateBranchMessage. The recorder slices count calls. + compareBehind int + compareBase string + baseChecks map[string]string + failBaseChecks bool + failedRuns []int64 + updateBranchStatus int + updateBranchMessage string + branchUpdates []string + reruns []int64 + baseCheckReads int } type adoptCommit struct { @@ -182,7 +198,58 @@ func (a *adoptAPI) handle(w http.ResponseWriter, r *http.Request) bool { for _, f := range a.compareFiles { files = append(files, map[string]string{"filename": f}) } - _ = json.NewEncoder(w).Encode(map[string]any{"files": files}) + payload := map[string]any{"files": files, "behind_by": a.compareBehind} + if a.compareBase != "" { + payload["base_commit"] = map[string]any{"sha": a.compareBase} + } + _ = json.NewEncoder(w).Encode(payload) + return true + case r.Method == http.MethodGet && strings.HasPrefix(path, "/repos/acme/widget/commits/") && strings.HasSuffix(path, "/check-runs"): + a.baseCheckReads++ + if a.failBaseChecks { + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(map[string]string{"message": "scripted failure"}) + return true + } + runs := []map[string]any{} + id := int64(1) + for name, conclusion := range a.baseChecks { + run := map[string]any{"id": id, "name": name, "status": "completed", "conclusion": conclusion} + if conclusion == "pending" { + run["status"] = "in_progress" + delete(run, "conclusion") + } + runs = append(runs, run) + id++ + } + _ = json.NewEncoder(w).Encode(map[string]any{"total_count": len(runs), "check_runs": runs}) + return true + case r.Method == http.MethodPut && strings.HasPrefix(path, "/repos/acme/widget/pulls/") && strings.HasSuffix(path, "/update-branch"): + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + expected, _ := body["expected_head_sha"].(string) + a.branchUpdates = append(a.branchUpdates, strings.TrimSuffix(strings.TrimPrefix(path, "/repos/acme/widget/pulls/"), "/update-branch")+"@"+expected) + if a.updateBranchStatus != 0 { + w.WriteHeader(a.updateBranchStatus) + _ = json.NewEncoder(w).Encode(map[string]string{"message": nonEmpty(a.updateBranchMessage, "scripted refusal")}) + return true + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"message":"Updating pull request branch."}`)) + return true + case r.Method == http.MethodGet && path == "/repos/acme/widget/actions/runs": + sha := r.URL.Query().Get("head_sha") + runs := []map[string]any{} + for _, id := range a.failedRuns { + runs = append(runs, map[string]any{"id": id, "workflow_id": id, "run_number": 1, "run_attempt": 1, "head_sha": sha, "status": "completed", "conclusion": "failure"}) + } + _ = json.NewEncoder(w).Encode(map[string]any{"total_count": len(runs), "workflow_runs": runs}) + return true + case r.Method == http.MethodPost && strings.HasPrefix(path, "/repos/acme/widget/actions/runs/") && strings.HasSuffix(path, "/rerun-failed-jobs"): + var id int + _, _ = sscanInt(strings.TrimSuffix(strings.TrimPrefix(path, "/repos/acme/widget/actions/runs/"), "/rerun-failed-jobs"), &id) + a.reruns = append(a.reruns, int64(id)) + w.WriteHeader(http.StatusCreated) return true case r.Method == http.MethodGet && strings.HasPrefix(path, "/repos/acme/widget/commits/"): sha := strings.TrimPrefix(path, "/repos/acme/widget/commits/") diff --git a/src/pkg/stagelabels/store.go b/src/pkg/stagelabels/store.go index eb8b0c8567..b0af939481 100644 --- a/src/pkg/stagelabels/store.go +++ b/src/pkg/stagelabels/store.go @@ -174,6 +174,14 @@ type Record struct { // for; it survives ClearAdoptionEscalation so a blocker that flaps on // one head is noted once. AdoptEscalationNotedHead string `json:"adopt_escalation_noted_head,omitempty"` + // AdoptRecoveryAttempt is the "@" pair the hive last + // tried to recover a stale-red in-place adoption on (update-branch or a + // re-run of the failed jobs, hivecommons/hive#14500); one attempt per + // pair. AdoptRecoveryCount counts attempts across heads for the + // adoption so a base that keeps moving under a red head cannot churn + // update-branch commits forever; both reset with the adoption. + AdoptRecoveryAttempt string `json:"adopt_recovery_attempt,omitempty"` + AdoptRecoveryCount int `json:"adopt_recovery_count,omitempty"` // MergeCommitNotedHead is the head the `merge-method/merge` note was // posted for, so the lane explains the merge method once per head. MergeCommitNotedHead string `json:"merge_commit_noted_head,omitempty"` @@ -873,6 +881,15 @@ func (r Record) AdoptionEscalationNoted(head string) bool { return head != "" && r.AdoptEscalationNotedHead == head } +// AdoptionRecoveryKey is the "@" pair AdoptRecoveryAttempt stores. +func AdoptionRecoveryKey(head, base string) string { return head + "@" + base } + +// AdoptionRecoveryAttempted reports whether the stale-red recovery already +// ran for head against this base tip. +func (r Record) AdoptionRecoveryAttempted(head, base string) bool { + return head != "" && base != "" && r.AdoptRecoveryAttempt == AdoptionRecoveryKey(head, base) +} + // MergeCommitNoted reports whether the merge-method note was posted for head. func (r Record) MergeCommitNoted(head string) bool { return head != "" && r.MergeCommitNotedHead == head @@ -976,6 +993,7 @@ func (s *Store) MarkYielded(key, head string) { r.AdoptedHead, r.AdoptedBy, r.AdoptedAt, r.AdoptCheckedAt = "", 0, time.Time{}, time.Time{} r.AdoptMode, r.AdoptReason, r.AdoptDiffFence = "", "", "" r.AdoptedHeadAt, r.AdoptEscalatedHead, r.AdoptEscalatedReason, r.AdoptEscalationNotedHead = time.Time{}, "", "", "" + r.AdoptRecoveryAttempt, r.AdoptRecoveryCount = "", 0 r.YieldedHead = head s.items[key] = r s.dirty = true @@ -1038,6 +1056,26 @@ func (s *Store) MarkAdoptionEscalated(key, head, reason string) { s.dirty = true } +// MarkAdoptionRecovery records one stale-red recovery attempt for head +// against base (hivecommons/hive#14500). Recorded before the API call, as +// every adoption path does, so a refused call is not retried each sweep. +func (s *Store) MarkAdoptionRecovery(key, head, base string) { + s.mu.Lock() + defer s.mu.Unlock() + r, ok := s.items[key] + if !ok || head == "" || base == "" { + return + } + pair := AdoptionRecoveryKey(head, base) + if r.AdoptRecoveryAttempt == pair { + return + } + r.AdoptRecoveryAttempt = pair + r.AdoptRecoveryCount++ + s.items[key] = r + s.dirty = true +} + // MarkMergeCommitNoted records that the merge-method note was posted for head. func (s *Store) MarkMergeCommitNoted(key, head string) { s.mu.Lock() @@ -1077,6 +1115,7 @@ func (s *Store) ClearAdoption(key string) { r.AdoptedHead, r.AdoptedBy, r.AdoptedAt, r.AdoptCheckedAt = "", 0, time.Time{}, time.Time{} r.AdoptMode, r.AdoptReason, r.AdoptDiffFence = "", "", "" r.AdoptedHeadAt, r.AdoptEscalatedHead, r.AdoptEscalatedReason, r.AdoptEscalationNotedHead = time.Time{}, "", "", "" + r.AdoptRecoveryAttempt, r.AdoptRecoveryCount = "", 0 r.SupersededBy, r.SupersededAt, r.SupersededOpen, r.SupersederMergedNotedHead = 0, time.Time{}, false, "" s.items[key] = r s.dirty = true