From 672f9b79ba7054fb25bebd301a989ace660dd326 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Wed, 21 Oct 2026 05:54:48 -0400 Subject: [PATCH 1/2] Re-run a job that lost its runner instead of starting a CI-fix Repair A GitHub Actions job GitHub ended as failed with no failed step and a step not completed is an Abandoned job. When every one of the branch's own failures on a head is one, the Repair loop re-runs them once per head for the Delivery, with no Repair, and watches CI again behind the Check re-run's guard. Mixed failures, a refused or unseen re-run, or a second abandonment take the CI-fix Repair as before. ADR 0020 records why this is not the re-run before every Repair that ADR 0010 rejected. Closes #908 Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 6 +- ...andoned-job-is-re-run-before-any-repair.md | 19 ++ docs/guide.md | 2 +- site/docs/index.html | 2 +- src/ci.rs | 319 +++++++++++++++++- src/delivery.rs | 11 + src/delivery/repair_loop.rs | 226 ++++++++++++- src/github.rs | 35 +- tests/ci.rs | 45 +++ tests/fakes/gh.rs | 70 +++- 10 files changed, 715 insertions(+), 20 deletions(-) create mode 100644 docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md diff --git a/GLOSSARY.md b/GLOSSARY.md index 96dc7a3e..c84a2675 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -186,9 +186,13 @@ A **Self-merge** blocked by a required review, or a merge it tried that failed w A CI-fix **Repair** that found nothing on the branch to fix, for example because the failure is not the branch's but was not recognised as an **Inherited failure**, since the **Base branch** commit had no finished result for that check: once the Repair ends and the **Base branch** is merged in again, the head of the **Issue branch** is still the commit whose CI just failed. That head gets its one **Check re-run** for the **Delivery**; if its own checks are red again, or they cannot be re-run, the **Run** is a **Failed run** with the cause `CI red on and the Repair found nothing to fix on the branch`, rather than spending its remaining Repairs. Own checks found red on that head in any later round, including after a refused **Self-merge**, fail with the same cause without another CI-fix Repair or Check re-run. Inherited-only failures keep their usual handling, and a different head may receive its own CI-fix Repair and Check re-run. **Check re-run**: -The one time per full head commit across one **Delivery** that thirdshift asks GitHub to re-run the branch's own failed checks and watches CI on that head again: when a CI-fix **Repair** has left the head unchanged, and before the **Run** fails as a **Declined CI fix**. The allowance is consumed before the request and stays consumed through green CI, **Inherited failures**, a **Base fix**, upstream moves, later rounds and refused **Self-merge** reconciliation, even after visiting another head and returning. Later watches still obtain fresh CI facts; own checks red on a consumed head mean a Declined CI fix without another CI-fix Repair. A different head has its own allowance, and a separately started **Continuation** begins a new Delivery with fresh allowances. It lets a check that failed for a reason that does not repeat, such as a flaky test, pass. If GitHub refuses a request, the outcome comes from fresh checks on that same head: green continues the Run, pending is watched, and its own checks still red mean a Declined CI fix. Checks that cannot be read or disappear after known failures cannot establish success. What the Repair concluded is never read. It needs every one of those checks to be a GitHub Actions job, starts no Repair and counts against no budget, and never re-runs a check for being an Inherited failure. +The one time per full head commit across one **Delivery** that thirdshift asks GitHub to re-run the branch's own failed checks and watches CI on that head again: when a CI-fix **Repair** has left the head unchanged, and before the **Run** fails as a **Declined CI fix**. The allowance is consumed before the request and stays consumed through green CI, **Inherited failures**, a **Base fix**, upstream moves, later rounds and refused **Self-merge** reconciliation, even after visiting another head and returning. Later watches still obtain fresh CI facts; own checks red on a consumed head mean a Declined CI fix without another CI-fix Repair. A different head has its own allowance, and a separately started **Continuation** begins a new Delivery with fresh allowances. It lets a check that failed for a reason that does not repeat, such as a flaky test, pass. If GitHub refuses a request, the outcome comes from fresh checks on that same head: green continues the Run, pending is watched, and its own checks still red mean a Declined CI fix. Checks that cannot be read or disappear after known failures cannot establish success. What the Repair concluded is never read. It needs every one of those checks to be a GitHub Actions job, starts no Repair and counts against no budget, and never re-runs a check for being an Inherited failure. It is separate from the re-run of **Abandoned jobs**, which comes before any Repair. _Avoid_: retry, re-trigger, CI re-run +**Abandoned job**: +A GitHub Actions job among the branch's own failed checks that GitHub ended as a failure while none of its steps failed and at least one had not completed, as when its runner is lost: its code never ran to completion. It is told from the job's and its steps' fields alone, never annotation or log text. When every one of the branch's own failed checks on a head is one, thirdshift re-runs them with no **Repair**, once per full head commit across one **Delivery**, and watches CI on that head again; red again, or a re-run refused or never seen, the head gets a CI-fix Repair as for any red CI. It comes before, and is separate from, that head's **Check re-run**, which a head may also get after the Repair, and like it counts against no budget. Mixed with ordinary failures of the branch's own, it is handed to the CI-fix Repair with them. +_Avoid_: lost runner, infrastructure failure + **Inherited failure**: A failed check on the head of the **Issue branch** that also failed, under the same check name, on the **Base branch** commit that head last merged in. It is not the branch's to fix: no **Repair** is started for it, and a **Run** whose only red checks are Inherited failures is a **Failed run** with the cause `CI red on , which also fails on at ; fix first`. A check with no finished result on that Base branch commit is not one. When asked to, by a flag or the **User config**, the Run starts a **Base fix** before failing. A Run that fails on one with no Base fix taken links each check where it fails on the Base branch, after its cause, and, if neither a flag nor the User config decided against a Base fix, offers one: the command that starts the Run again with one allowed, and the User config setting that allows one for every Run. _Avoid_: base-red check, flaky check, shared failure diff --git a/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md b/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md new file mode 100644 index 00000000..59edfe24 --- /dev/null +++ b/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md @@ -0,0 +1,19 @@ +# A job that lost its runner is re-run before any Repair + +A GitHub Actions job whose runner GitHub lost ends as `failure` with a step still in progress and none failed. Its code never ran to completion, so a CI-fix **Repair** given it has nothing to fix: one did nothing for 81 s and 420,084 input tokens on Spec PR #842, spending one of the Delivery's 5 Repairs (#908). Now such a job is an **Abandoned job**. When every one of the branch's own failed checks on a head is one, thirdshift asks GitHub to re-run the failed jobs of their workflow runs, with no Repair, and watches CI on that head again, once per head for the complete Delivery. Green, the Run carries on as for any green head; red, it carries on as for any red CI. + +What makes a job abandoned is read from the jobs API's job and step fields alone: the job's conclusion is `failure`, no step's is `failure`, and at least one step is not completed. Annotation text such as "The hosted runner lost communication with the server" and job logs are never read, as with ADR 0008 and ADR 0010. + +## Considered Options + +- **Re-run before any Repair**, the option ADR 0010 rejected. Still rejected: that adds a CI cycle to every real failure to save a Repair on the rare flaky one, and whether a failure is flaky is a judgement. An Abandoned job is not a judgement. GitHub reports that the code never finished, so there is nothing for a Repair to read, and the extra CI cycle is spent only where a Repair could do nothing. +- **Fold it into the Check re-run.** Rejected: the Check re-run follows a Repair that left the head unchanged, so it cannot save that Repair. The two stay separate, each with its own allowance per head, and a head may get both, in that order. +- **Read the annotation or the log for the runner-lost message.** Rejected: it is prose GitHub may reword, where the step states are fields. +- **Re-run abandoned jobs mixed with ordinary failures.** Rejected: the ordinary failures need a CI-fix Repair anyway, and the Repair's new head gets fresh CI. + +## Consequences + +- Only the branch's own failed checks that are GitHub Actions jobs concluded `failure` have their steps read, once per watch that ends red. Inherited failures, commit statuses and other apps' check runs are never read or re-run for this. A failed read of a job fails the watch, as a failed read of checks does. +- The request and the watch after it are the Check re-run's: one request per workflow run, and the watch waits up to the grace period for the old check run ids to leave the listing before it trusts what it sees. If a request is refused, or no new attempt appears in time, the red CI first watched is handled as before, with a CI-fix Repair. +- A head whose re-run came to an Abandoned job again, or any later watch of that head, gets a CI-fix Repair as before, and after it, if the head is unchanged, its Check re-run. A head whose Check re-run has been consumed gets no Abandoned job re-run. +- The re-run starts no Repair and counts against neither the Repair cap nor the upstream-move budget. The Command log names it: ` lost its runner on ; re-running it`. diff --git a/docs/guide.md b/docs/guide.md index 386854b4..1e89a190 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -37,7 +37,7 @@ From a clone of the issue's repository, `thirdshift `: 3. Creates a git worktree next to your clone, named `-`, so your own checkout is never touched. 4. Runs a headless agent session in that worktree on the **Harness**, Claude Code unless you [choose another](#harness-model-and-effort), with the **Factory skills** in [`skills/`](../skills) linked into its `.claude/skills/`, or `.agents/skills/` on the other Harnesses, as `thirdshift-` and kept out of git by a `.git/info/exclude` entry ([ADR-0012](adr/0012-factory-skills-linked-into-the-worktree-codex-unsandboxed.md)), started with one of the **Session prompts** in [`prompts/`](../prompts). The agent implements the issue, reviews its work against the Base branch, addresses the **Standards findings** and **Spec findings** it agrees with, and opens a ready-for-review pull request that lists every **Unaddressed finding** with a reason and closes the issue. With a [Security review](#security-review) asked for, a second session then reviews the change for vulnerabilities. 5. Takes over deterministically: pushes the Issue branch, skipping your repo's git hooks since the session runs the tests and CI gates the pull request, checks through `gh` that the pull request exists, is open and targets the Base branch, and marks it ready for review (`gh pr ready`) if the agent left it as a draft. -6. Keeps the pull request mergeable and green: merges the Base branch in and watches CI, starting a **Repair** session for a merge conflict or failing checks, at most 5 per Run. A failed check that also failed, under the same name, on the Base branch commit the head last merged in is an **Inherited failure**, not the branch's to fix: the CI-fix Repair is given only the branch's own failures, with the Inherited failures listed as not to fix. If every failed check is an Inherited failure, no Repair starts: the Run merges the Base branch again if it has moved since, and otherwise ends as a [Failed run](#failed-runs) that says to fix the Base branch first, unless it was given `base-fix` or the [User config](#user-config) sets `base.fix`, when it starts a [Base fix](#base-fix) first. A check that is pending, passed or missing on that Base branch commit is the branch's own, and thirdshift never triggers or waits for the Base branch's CI. If a CI-fix Repair leaves the head unchanged, with no commit of its own and no Base branch move to merge, thirdshift does a **Check re-run**, once for that head: it asks GitHub to re-run the branch's own failed checks (`gh run rerun --failed`, once per workflow run), says so on stderr as `re-running the failed checks on : [, …]`, and watches CI on that head again, from the new attempt on. A check that failed for a reason that does not repeat, such as a flaky test or a runner fault, then passes, and the Run goes on as for any green head. If CI is red again, the Run ends as a Failed run, with no second Repair or re-run for that head. It ends the same way, with nothing re-run, if one of those checks can't be re-run (a commit status, or a check run that is not a GitHub Actions job), since the head could not go green. When GitHub refuses a re-run, stderr shows the refusal and thirdshift reads all checks on that head again: a Repair may already have re-run them. Green continues the Run, pending checks are watched, and the branch's own checks still red fail as before, without another Repair or request for that head. Checks that cannot be read or disappear after known failures cannot establish green CI. Any earlier successful workflow request still waits for its new attempt before the whole head is assessed. What the Repair concluded is never read, Inherited failures are never re-run on their own account, and a Check re-run is no Repair, so it counts against no budget ([ADR-0010](adr/0010-one-check-re-run-before-a-declined-ci-fix.md)). If the Base branch moves while CI runs, it merges it again and goes round, at most 5 times per Run. In a Merge run, each round also takes in **Foreign commits** first: see [Foreign commits in a Merge run](#foreign-commits-in-a-merge-run). +6. Keeps the pull request mergeable and green: merges the Base branch in and watches CI, starting a **Repair** session for a merge conflict or failing checks, at most 5 per Run. A failed check that also failed, under the same name, on the Base branch commit the head last merged in is an **Inherited failure**, not the branch's to fix: the CI-fix Repair is given only the branch's own failures, with the Inherited failures listed as not to fix. If every failed check is an Inherited failure, no Repair starts: the Run merges the Base branch again if it has moved since, and otherwise ends as a [Failed run](#failed-runs) that says to fix the Base branch first, unless it was given `base-fix` or the [User config](#user-config) sets `base.fix`, when it starts a [Base fix](#base-fix) first. A check that is pending, passed or missing on that Base branch commit is the branch's own, and thirdshift never triggers or waits for the Base branch's CI. If a CI-fix Repair leaves the head unchanged, with no commit of its own and no Base branch move to merge, thirdshift does a **Check re-run**, once for that head: it asks GitHub to re-run the branch's own failed checks (`gh run rerun --failed`, once per workflow run), says so on stderr as `re-running the failed checks on : [, …]`, and watches CI on that head again, from the new attempt on. A check that failed for a reason that does not repeat, such as a flaky test or a runner fault, then passes, and the Run goes on as for any green head. If CI is red again, the Run ends as a Failed run, with no second Repair or re-run for that head. It ends the same way, with nothing re-run, if one of those checks can't be re-run (a commit status, or a check run that is not a GitHub Actions job), since the head could not go green. When GitHub refuses a re-run, stderr shows the refusal and thirdshift reads all checks on that head again: a Repair may already have re-run them. Green continues the Run, pending checks are watched, and the branch's own checks still red fail as before, without another Repair or request for that head. Checks that cannot be read or disappear after known failures cannot establish green CI. Any earlier successful workflow request still waits for its new attempt before the whole head is assessed. What the Repair concluded is never read, Inherited failures are never re-run on their own account, and a Check re-run is no Repair, so it counts against no budget ([ADR-0010](adr/0010-one-check-re-run-before-a-declined-ci-fix.md)). Before any Repair, a head whose own failed checks are all **Abandoned jobs**, GitHub Actions jobs GitHub ended as failed with no failed step and a step not completed, as when a runner is lost, has them re-run once, saying ` lost its runner on ; re-running it`, and CI watched again the same way: green goes on, and red again, or a refused or unseen re-run, gets the CI-fix Repair as before. This too is no Repair and counts against no budget ([ADR-0020](adr/0020-an-abandoned-job-is-re-run-before-any-repair.md)). If the Base branch moves while CI runs, it merges it again and goes round, at most 5 times per Run. In a Merge run, each round also takes in **Foreign commits** first: see [Foreign commits in a Merge run](#foreign-commits-in-a-merge-run). 7. In a **Merge run** (`thirdshift merge `), does the **Self-merge**: once the pull request is open, ready for review, mergeable and green, thirdshift merges it into the Base branch with a merge commit, on exactly the head commit whose CI it watched (`gh pr merge --merge --match-head-commit `). Just before each attempt, it reads the PR's `reviewDecision`: `REVIEW_REQUIRED` or `CHANGES_REQUESTED` ends the Run as **review required**, a policy refusal, without attempting the merge or taking another Repair round. An empty or `APPROVED` decision, or a failed read, keeps the existing merge behavior. It never uses GitHub's auto-merge ([ADR-0004](adr/0004-self-merge-by-thirdshift-not-github-auto-merge.md)). A merge that fails goes back round step 6, within the same budgets, and is tried again on the new green head. If that round finds nothing to fix, the refusal is a **policy refusal**, such as merge commits being disallowed or a review being required. After the merge, thirdshift deletes the Issue branch on `origin`, and closes the issue if it is still open, with the comment `Closed by #, merged into by a thirdshift Merge run.` GitHub closes it on its own only for a merge into the repository's default branch, and then thirdshift leaves it alone. Once the issue is closed, either way, thirdshift takes `in-progress` off it, [removing the Claim](#when-the-claim-ends). 8. Cleans up: removes the worktree, the local Issue branch and the temporary directory the Factory skills were written out to, whether the Run succeeded or not. The one exception is a **Failed run** whose work could not be pushed: see below. diff --git a/site/docs/index.html b/site/docs/index.html index 3d416945..70cce94a 100644 --- a/site/docs/index.html +++ b/site/docs/index.html @@ -88,7 +88,7 @@

  • Takes over deterministically: pushes the Issue branch, skipping your repo's git hooks since the session runs the tests and CI gates the pull request, checks through gh that the pull request exists, is open and targets the Base branch, and marks it ready for review (gh pr ready) if the agent left it as a draft.

    • -

  • Keeps the pull request mergeable and green: merges the Base branch in and watches CI, starting a Repair session for a merge conflict or failing checks, at most 5 per Run. A failed check that also failed, under the same name, on the Base branch commit the head last merged in is an Inherited failure, not the branch's to fix: the CI-fix Repair is given only the branch's own failures, with the Inherited failures listed as not to fix. If every failed check is an Inherited failure, no Repair starts: the Run merges the Base branch again if it has moved since, and otherwise ends as a Failed run that says to fix the Base branch first, unless it was given base-fix or the User config sets base.fix, when it starts a Base fix first. A check that is pending, passed or missing on that Base branch commit is the branch's own, and thirdshift never triggers or waits for the Base branch's CI. If a CI-fix Repair leaves the head unchanged, with no commit of its own and no Base branch move to merge, thirdshift does a Check re-run, once for that head: it asks GitHub to re-run the branch's own failed checks (gh run rerun <run-id> --failed, once per workflow run), says so on stderr as re-running the failed checks on <short sha>: <check>[, <check>…], and watches CI on that head again, from the new attempt on. A check that failed for a reason that does not repeat, such as a flaky test or a runner fault, then passes, and the Run goes on as for any green head. If CI is red again, the Run ends as a Failed run, with no second Repair or re-run for that head. It ends the same way, with nothing re-run, if one of those checks can't be re-run (a commit status, or a check run that is not a GitHub Actions job), since the head could not go green. When GitHub refuses a re-run, stderr shows the refusal and thirdshift reads all checks on that head again: a Repair may already have re-run them. Green continues the Run, pending checks are watched, and the branch's own checks still red fail as before, without another Repair or request for that head. Checks that cannot be read or disappear after known failures cannot establish green CI. Any earlier successful workflow request still waits for its new attempt before the whole head is assessed. What the Repair concluded is never read, Inherited failures are never re-run on their own account, and a Check re-run is no Repair, so it counts against no budget (ADR-0010). If the Base branch moves while CI runs, it merges it again and goes round, at most 5 times per Run. In a Merge run, each round also takes in Foreign commits first: see Foreign commits in a Merge run.

    +

  • Keeps the pull request mergeable and green: merges the Base branch in and watches CI, starting a Repair session for a merge conflict or failing checks, at most 5 per Run. A failed check that also failed, under the same name, on the Base branch commit the head last merged in is an Inherited failure, not the branch's to fix: the CI-fix Repair is given only the branch's own failures, with the Inherited failures listed as not to fix. If every failed check is an Inherited failure, no Repair starts: the Run merges the Base branch again if it has moved since, and otherwise ends as a Failed run that says to fix the Base branch first, unless it was given base-fix or the User config sets base.fix, when it starts a Base fix first. A check that is pending, passed or missing on that Base branch commit is the branch's own, and thirdshift never triggers or waits for the Base branch's CI. If a CI-fix Repair leaves the head unchanged, with no commit of its own and no Base branch move to merge, thirdshift does a Check re-run, once for that head: it asks GitHub to re-run the branch's own failed checks (gh run rerun <run-id> --failed, once per workflow run), says so on stderr as re-running the failed checks on <short sha>: <check>[, <check>…], and watches CI on that head again, from the new attempt on. A check that failed for a reason that does not repeat, such as a flaky test or a runner fault, then passes, and the Run goes on as for any green head. If CI is red again, the Run ends as a Failed run, with no second Repair or re-run for that head. It ends the same way, with nothing re-run, if one of those checks can't be re-run (a commit status, or a check run that is not a GitHub Actions job), since the head could not go green. When GitHub refuses a re-run, stderr shows the refusal and thirdshift reads all checks on that head again: a Repair may already have re-run them. Green continues the Run, pending checks are watched, and the branch's own checks still red fail as before, without another Repair or request for that head. Checks that cannot be read or disappear after known failures cannot establish green CI. Any earlier successful workflow request still waits for its new attempt before the whole head is assessed. What the Repair concluded is never read, Inherited failures are never re-run on their own account, and a Check re-run is no Repair, so it counts against no budget (ADR-0010). Before any Repair, a head whose own failed checks are all Abandoned jobs, GitHub Actions jobs GitHub ended as failed with no failed step and a step not completed, as when a runner is lost, has them re-run once, saying <check> lost its runner on <short sha>; re-running it, and CI watched again the same way: green goes on, and red again, or a refused or unseen re-run, gets the CI-fix Repair as before. This too is no Repair and counts against no budget (ADR-0020). If the Base branch moves while CI runs, it merges it again and goes round, at most 5 times per Run. In a Merge run, each round also takes in Foreign commits first: see Foreign commits in a Merge run.

  • In a Merge run (thirdshift merge <Issue URL>), does the Self-merge: once the pull request is open, ready for review, mergeable and green, thirdshift merges it into the Base branch with a merge commit, on exactly the head commit whose CI it watched (gh pr merge --merge --match-head-commit <sha>). Just before each attempt, it reads the PR's reviewDecision: REVIEW_REQUIRED or CHANGES_REQUESTED ends the Run as review required, a policy refusal, without attempting the merge or taking another Repair round. An empty or APPROVED decision, or a failed read, keeps the existing merge behavior. It never uses GitHub's auto-merge (ADR-0004). A merge that fails goes back round step 6, within the same budgets, and is tried again on the new green head. If that round finds nothing to fix, the refusal is a policy refusal, such as merge commits being disallowed or a review being required. After the merge, thirdshift deletes the Issue branch on origin, and closes the issue if it is still open, with the comment Closed by #<pr>, merged into <base> by a thirdshift Merge run. GitHub closes it on its own only for a merge into the repository's default branch, and then thirdshift leaves it alone. Once the issue is closed, either way, thirdshift takes in-progress off it, removing the Claim.

    • diff --git a/src/ci.rs b/src/ci.rs index 72f3ca8f..a3e3db7c 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -9,7 +9,7 @@ use std::time::{Duration, Instant}; use anyhow::{Context, Result, ensure}; -use crate::github::{ActionsJob, Check, Conclusion, GitHub}; +use crate::github::{ActionsJob, Check, Conclusion, GitHub, JobRun}; use crate::issue::IssueUrl; use crate::poll; use crate::progress; @@ -31,6 +31,18 @@ impl Ci { } } +impl FailedChecks { + /// Whether the branch's own failures, at least one, are all Abandoned + /// jobs, whatever the Inherited failures are. + pub fn only_abandoned_jobs_own(&self) -> bool { + !self.own.is_empty() + && self + .own + .iter() + .all(|check| check.job.is_some_and(|job| job.abandoned)) + } +} + /// The checks that failed on a commit, at least one, split by whether the /// branch is the one to fix them. pub struct FailedChecks { @@ -92,6 +104,27 @@ pub fn rerun( ) } +/// The re-run of `sha`'s Abandoned jobs, when every one of the branch's +/// own failures in `failed` is one: ask GitHub to re-run the failed jobs of +/// their workflow runs, then watch CI on `sha` from the new attempt on, with +/// the same guard against the attempt before as the Check re-run's. `None` +/// if a request was refused or no new attempt appeared within the grace +/// period, each told in a progress line, so that red CI is taken as it was. +pub fn rerun_abandoned( + issue: &IssueUrl, + sha: &str, + base_commit: Option<&str>, + failed: &FailedChecks, + actionable: &mut dyn FnMut() -> Result<()>, +) -> Result> { + rerun_abandoned_through( + &mut OnGitHub { issue, actionable }, + sha, + base_commit, + failed, + ) +} + /// What the CI watch reads of GitHub and asks of it, how it waits, and /// where its progress lines go. trait Outside { @@ -99,6 +132,8 @@ trait Outside { type Deadline; /// The checks and statuses on the commit `sha`. fn checks_on(&mut self, sha: &str) -> Result>; + /// The GitHub Actions job whose check run is `check_run`, with its steps. + fn job_run(&mut self, check_run: u64) -> Result; /// Ask GitHub to re-run the failed jobs of the workflow run /// `workflow_run`. fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()>; @@ -133,6 +168,13 @@ impl Outside for OnGitHub<'_> { Ok(checks) } + fn job_run(&mut self, check_run: u64) -> Result { + (self.actionable)()?; + let job_run = GitHub::new().job_run(self.issue, check_run)?; + (self.actionable)()?; + Ok(job_run) + } + fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()> { (self.actionable)()?; GitHub::new().rerun_failed_jobs(self.issue, workflow_run) @@ -234,6 +276,44 @@ fn rerun_through( "re-running the failed checks on {short}: {}", check_names(&failed.own) )); + let (rerun_checks, _) = request_reruns(outside, sha, failed)?; + if rerun_checks.requested.is_empty() { + return watch_to_end(outside, sha, base_commit, Some(&rerun_checks)).map(Some); + } + watch_new_attempts(outside, sha, base_commit, &rerun_checks) +} + +/// [`rerun_abandoned`], through `outside`. +fn rerun_abandoned_through( + outside: &mut impl Outside, + sha: &str, + base_commit: Option<&str>, + failed: &FailedChecks, +) -> Result> { + let (lost, them) = match failed.own.len() { + 1 => ("its runner", "it"), + _ => ("their runners", "them"), + }; + outside.step(format!( + "{} lost {lost} on {}; re-running {them}", + check_names(&failed.own), + short(sha) + )); + match request_reruns(outside, sha, failed)? { + (rerun_checks, true) => watch_new_attempts(outside, sha, base_commit, &rerun_checks), + (_, false) => Ok(None), + } +} + +/// Ask GitHub to re-run the failed jobs of each workflow run the branch's +/// own failures in `failed`, all GitHub Actions jobs, belong to, stopping at +/// the first refusal, which is reported. Returns the checks whose new +/// attempts the watch must see, and whether every request succeeded. +fn request_reruns<'a>( + outside: &mut impl Outside, + sha: &str, + failed: &'a FailedChecks, +) -> Result<(RerunChecks<'a>, bool)> { let mut workflow_runs: Vec = jobs_of(&failed.own).map(|job| job.workflow_run).collect(); workflow_runs.sort_unstable(); workflow_runs.dedup(); @@ -247,12 +327,14 @@ fn rerun_through( for workflow_run in &workflow_runs { if let Err(error) = outside.rerun_failed_jobs(*workflow_run) { outside.step(format!( - "the failed checks on {short} were not re-run: {error:#}" + "the failed checks on {} were not re-run: {error:#}", + short(sha) )); break; } requested.push(*workflow_run); } + let all_requested = requested.len() == workflow_runs.len(); // GitHub re-runs every failed job of a workflow run, so an Inherited // failure that shares one with the branch's own gets a new attempt too. let rerun_checks = RerunChecks { @@ -260,9 +342,18 @@ fn rerun_through( previous_attempt, requested, }; - if rerun_checks.requested.is_empty() { - return watch_to_end(outside, sha, base_commit, Some(&rerun_checks)).map(Some); - } + Ok((rerun_checks, all_requested)) +} + +/// Wait up to the grace period for the new attempts of `rerun_checks` to be +/// listed on `sha`, then watch CI there to the end: `None`, told in a +/// progress line, if none appeared. +fn watch_new_attempts( + outside: &mut impl Outside, + sha: &str, + base_commit: Option<&str>, + rerun_checks: &RerunChecks<'_>, +) -> Result> { let (grace, deadline) = outside.start_grace(); let new_attempt = within(outside, deadline, |outside| { let checks = read_checks(outside, sha)?; @@ -270,12 +361,13 @@ fn rerun_through( })?; if new_attempt.is_none() { outside.step(format!( - "no re-run appeared on {short} within {}s", + "no re-run appeared on {} within {}s", + short(sha), grace.as_secs() )); return Ok(None); } - watch_to_end(outside, sha, base_commit, Some(&rerun_checks)).map(Some) + watch_to_end(outside, sha, base_commit, Some(rerun_checks)).map(Some) } /// Known failed checks that must stay listed during a Check re-run. Those @@ -357,7 +449,16 @@ fn watch_to_end( Some(base_commit) => read_checks(outside, base_commit)?, None => Vec::new(), }; - Ok(Ci::Failed(split(failed, on_base))) + let mut failed = split(failed, on_base); + // Only the branch's own failures can be re-run for losing their runner. + for check in &mut failed.own { + if let Some(job) = &mut check.job + && check.conclusion == Conclusion::Failure + { + job.abandoned = is_abandoned(&outside.job_run(job.check_run)?); + } + } + Ok(Ci::Failed(failed)) } /// Ask `probe` every poll interval of `outside` until it answers `Some`, and @@ -472,6 +573,15 @@ fn reading(check: &Check) -> Reading { } } +/// Whether `job` is an Abandoned job: GitHub ended it as a failure while +/// none of its steps failed and one had not completed, as when its runner is +/// lost. A fact GitHub reports, so no annotation or log text is read. +fn is_abandoned(job: &JobRun) -> bool { + job.conclusion == Conclusion::Failure + && !job.steps.contains(&Conclusion::Failure) + && job.steps.contains(&Conclusion::Unfinished) +} + fn is_failed(check: &Check) -> bool { reading(check) == Reading::Failed } @@ -490,7 +600,7 @@ mod in_memory { use anyhow::{Result, bail}; use super::Outside; - use crate::github::Check; + use crate::github::{Check, Conclusion, JobRun}; /// GitHub in memory: for each commit, a script of the check listings /// its reads take in turn, the last one repeating; the workflow runs @@ -505,6 +615,9 @@ mod in_memory { interrupted_on: Option, grace_pauses: usize, pauses: usize, + job_runs: Vec<(u64, JobRun)>, + /// The check run of each job read, in order. + pub job_reads: Vec, /// The commit of each read, in order. pub reads: Vec, /// The workflow run of each re-run request, refused ones included. @@ -525,6 +638,8 @@ mod in_memory { interrupted_on: None, grace_pauses: 3, pauses: 0, + job_runs: Vec::new(), + job_reads: Vec::new(), reads: Vec::new(), reruns: Vec::new(), steps: Vec::new(), @@ -550,6 +665,13 @@ mod in_memory { self } + /// Report the job whose check run is `check_run` as `job_run`. A job + /// not scripted failed on a failed step. + pub fn job_run(mut self, check_run: u64, job_run: JobRun) -> Self { + self.job_runs.push((check_run, job_run)); + self + } + /// Refuse a re-run of `workflow_run`. pub fn refusing(mut self, workflow_run: u64) -> Self { self.refused.push(workflow_run); @@ -595,6 +717,18 @@ mod in_memory { Ok(listing.cloned().unwrap_or_default()) } + fn job_run(&mut self, check_run: u64) -> Result { + self.job_reads.push(check_run); + let scripted = self.job_runs.iter().find(|(id, _)| *id == check_run); + Ok(scripted.map_or_else( + || JobRun { + conclusion: Conclusion::Failure, + steps: vec![Conclusion::Success, Conclusion::Failure], + }, + |(_, job_run)| job_run.clone(), + )) + } + fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()> { self.reruns.push(workflow_run); if self.refused.contains(&workflow_run) { @@ -670,6 +804,7 @@ mod tests { job: Some(ActionsJob { check_run, workflow_run, + abandoned: false, }), ..check } @@ -1647,6 +1782,172 @@ mod tests { ); } + /// A job run as GitHub ends one whose runner was lost. + fn lost_runner() -> JobRun { + JobRun { + conclusion: Conclusion::Failure, + steps: vec![Conclusion::Success, Conclusion::Unfinished], + } + } + + /// The names of the Abandoned jobs among `checks`. + fn abandoned(checks: &[Check]) -> Vec<&str> { + checks + .iter() + .filter(|check| check.job.is_some_and(|job| job.abandoned)) + .map(|check| check.name.as_str()) + .collect() + } + + #[test] + fn a_watch_reads_the_steps_of_failed_jobs_alone_and_marks_the_abandoned_ones() { + let mut github = InMemory::new() + .listing( + HEAD, + vec![vec![ + job(red("wsl"), 1, 900), + job(red("test"), 2, 900), + job( + check("lint", Conclusion::Cancelled, "https://head/lint"), + 3, + 901, + ), + job(green("build"), 4, 900), + red("status"), + ]], + ) + .job_run(1, lost_runner()); + + let failed = failed_of(watch_through(&mut github, HEAD, None).unwrap()); + + assert_eq!(abandoned(&failed.own), ["wsl"]); + assert_eq!(github.job_reads, [1, 2]); + assert!(!failed.only_abandoned_jobs_own()); + } + + #[test] + fn own_failures_are_only_abandoned_jobs_when_each_is_one_whatever_is_inherited() { + let mut github = InMemory::new() + .listing( + HEAD, + vec![vec![job(red("wsl"), 1, 900), job(red("lint"), 2, 901)]], + ) + .listing(BASE, vec![vec![red("lint")]]) + .job_run(1, lost_runner()); + + let failed = failed_of(watch_through(&mut github, HEAD, Some(BASE)).unwrap()); + + assert!(failed.only_abandoned_jobs_own()); + assert_eq!(github.job_reads, [1]); + assert!(!failed_on_watch(vec![red("lint")], vec![red("lint")]).only_abandoned_jobs_own()); + } + + /// `check` as the Abandoned job whose check run is `check_run`, of the + /// workflow run `workflow_run`. + fn abandoned_job(check: Check, check_run: u64, workflow_run: u64) -> Check { + let mut check = job(check, check_run, workflow_run); + check.job.as_mut().unwrap().abandoned = true; + check + } + + #[test] + fn abandoned_jobs_are_re_run_and_watched_from_their_new_attempt() { + let old = abandoned_job(red("custom-release-smoke / wsl"), 1, 900); + let failed = FailedChecks { + own: vec![old.clone()], + inherited: vec![], + on_base: vec![], + }; + let new = job(green("custom-release-smoke / wsl"), 2, 900); + let mut github = InMemory::new().listing( + HEAD, + // GitHub lists the old attempt a moment after the request. + vec![vec![old.clone()], vec![old], vec![new]], + ); + + let ci = rerun_abandoned_through(&mut github, HEAD, Some(BASE), &failed).unwrap(); + + assert!(matches!(ci, Some(Ci::Passed))); + assert_eq!(github.reruns, [900]); + assert_eq!( + github.steps, + [ + "custom-release-smoke / wsl lost its runner on abcdef0; re-running it", + "CI passed on abcdef0", + ] + ); + } + + #[test] + fn a_refused_or_unseen_re_run_of_abandoned_jobs_leaves_nothing_to_watch() { + let old = abandoned_job(red("wsl"), 1, 900); + let failed = FailedChecks { + own: vec![old.clone(), abandoned_job(red("mac"), 3, 901)], + inherited: vec![], + on_base: vec![], + }; + let mut github = InMemory::new() + .listing(HEAD, vec![failed.own.clone()]) + .refusing(901); + + let ci = rerun_abandoned_through(&mut github, HEAD, None, &failed).unwrap(); + + assert!(ci.is_none()); + assert_eq!(github.reruns, [900, 901]); + assert_eq!(github.reads_of(HEAD), 1); + assert_eq!( + github.steps, + [ + "wsl, mac lost their runners on abcdef0; re-running them", + "the failed checks on abcdef0 were not re-run: \ + gh: HTTP 403: workflow run 901 cannot be re-run", + ] + ); + + let mut github = InMemory::new().listing(HEAD, vec![vec![old]]); + let failed = FailedChecks { + own: failed.own[..1].to_vec(), + inherited: vec![], + on_base: vec![], + }; + assert!( + rerun_abandoned_through(&mut github, HEAD, None, &failed) + .unwrap() + .is_none() + ); + assert_eq!( + github.steps.last().map(String::as_str), + Some("no re-run appeared on abcdef0 within 60s") + ); + } + + #[test] + fn a_failed_job_with_no_failed_step_and_a_step_not_completed_is_abandoned() { + let job = |conclusion: Conclusion, steps: &[Conclusion]| JobRun { + conclusion, + steps: steps.to_vec(), + }; + use Conclusion::{Failure, Skipped, Success, Unfinished}; + + // As run 39469875996 lost its runner: set-up done, a step still in + // progress, the rest pending. + assert!(is_abandoned(&job( + Failure, + &[Success, Unfinished, Unfinished, Unfinished] + ))); + assert!(!is_abandoned(&job(Failure, &[Success, Failure, Skipped]))); + assert!(!is_abandoned(&job( + Failure, + &[Success, Failure, Unfinished] + ))); + assert!(!is_abandoned(&job(Failure, &[Success, Success]))); + assert!(!is_abandoned(&job(Failure, &[]))); + assert!(!is_abandoned(&job( + Conclusion::Cancelled, + &[Success, Unfinished] + ))); + } + #[test] fn an_interrupt_during_a_pause_fails_the_watch() { let mut github = InMemory::new() diff --git a/src/delivery.rs b/src/delivery.rs index b4dda75b..48a56444 100644 --- a/src/delivery.rs +++ b/src/delivery.rs @@ -711,6 +711,17 @@ impl repair_loop::Outside for RunOutside<'_> { }) } + fn rerun_abandoned( + &mut self, + head: &str, + base_commit: Option<&str>, + failed: &FailedChecks, + ) -> Result> { + ci::rerun_abandoned(self.issue, head, base_commit, failed, &mut || { + self.pull_request.ensure_actionable(true) + }) + } + fn repair(&mut self, kind: Kind, repair: Repair) -> Result<()> { let (issue, base, pr_url) = (self.issue, self.base, self.pr_url); let branch = self.worktree.branch(); diff --git a/src/delivery/repair_loop.rs b/src/delivery/repair_loop.rs index 11adc0f3..26841fed 100644 --- a/src/delivery/repair_loop.rs +++ b/src/delivery/repair_loop.rs @@ -79,6 +79,14 @@ pub(super) trait Outside { base_commit: Option<&str>, failed: &FailedChecks, ) -> Result>; + /// The re-run of `head`'s Abandoned jobs, every one of the branch's own + /// failures in `failed`: `None` if it was refused or never appeared. + fn rerun_abandoned( + &mut self, + head: &str, + base_commit: Option<&str>, + failed: &FailedChecks, + ) -> Result>; /// Start the Repair session of `kind`, as in `Kind::Repair(1)`, for /// `repair`. @@ -225,13 +233,15 @@ enum Round { NothingToFix, } -/// Check re-run decisions for one complete Delivery, including rounds after -/// a refused Self-merge. CI observation and attempt reconciliation stay with -/// `Outside`; the pending Repair facts and consumed full head IDs stay here. +/// Check re-run and Abandoned job re-run decisions for one complete +/// Delivery, including rounds after a refused Self-merge. CI observation and +/// attempt reconciliation stay with `Outside`; the pending Repair facts and +/// the full head IDs whose allowances are consumed stay here. #[derive(Default)] struct CheckReruns { handed_to_repair: Option<(String, FailedChecks)>, consumed: HashSet, + abandoned_rerun: HashSet, } impl CheckReruns { @@ -243,7 +253,8 @@ impl CheckReruns { /// Assess fresh CI against this round's selected Base commit. An /// unchanged Repair head consumes its allowance before any re-run - /// request. Later watches remain allowed, but own failures on a consumed + /// request; any other head is watched, its Abandoned jobs re-run if + /// they are its only own failures. Later watches remain allowed, but own failures on a consumed /// head decline the CI fix before another Repair can start. fn assess( &mut self, @@ -260,7 +271,10 @@ impl CheckReruns { Some((_, failed)) if self.consumed.insert(head.to_string()) => { outside.rerun(head, compared_with, &failed)? } - _ => Some(outside.watch(head, compared_with)?), + _ => { + let watched = outside.watch(head, compared_with)?; + Some(self.past_abandoned_jobs(outside, head, compared_with, watched)?) + } }; match ci { Some(ci) if !(self.consumed.contains(head) && ci.has_own_failures()) => Ok(ci), @@ -272,6 +286,32 @@ impl CheckReruns { } } +impl CheckReruns { + /// `ci`, a watch of `head`, or, if it is red only on Abandoned jobs of + /// the branch's own and `head` has had neither their re-run nor its + /// Check re-run, what their re-run came to. A refused or unseen re-run + /// leaves `ci` as it was. + fn past_abandoned_jobs( + &mut self, + outside: &mut impl Outside, + head: &str, + compared_with: Option<&str>, + ci: Ci, + ) -> Result { + let Ci::Failed(failed) = &ci else { + return Ok(ci); + }; + if !failed.only_abandoned_jobs_own() + || self.consumed.contains(head) + || !self.abandoned_rerun.insert(head.to_string()) + { + return Ok(ci); + } + let rerun = outside.rerun_abandoned(head, compared_with, failed)?; + Ok(rerun.unwrap_or(ci)) + } +} + /// Keeps the PR mergeable and its CI green, within the Delivery's budgets. struct RepairLoop<'a, O> { outside: &'a mut O, @@ -323,6 +363,10 @@ impl RepairLoop<'_, O> { /// as a Base move, and otherwise once its Base fix has returned, or fails /// with the Base fix's cause. A Run that sees no Inherited failures /// compares with no Base branch commit. + /// A head whose own failures are all Abandoned jobs first has them + /// re-run, once per head for the complete Delivery, before any CI-fix + /// Repair; their re-run's CI is taken as from any watch, and a refused or + /// unseen re-run leaves the red CI first watched to its CI-fix Repair. /// If, after a CI-fix Repair and the Base branch merged again, the head /// is still the one whose CI failed, it gets its Check re-run instead of /// a watch, once per head for the complete Delivery, whatever the Repair @@ -471,7 +515,7 @@ mod tests { use anyhow::anyhow; use super::*; - use crate::github::{Check, Conclusion}; + use crate::github::{ActionsJob, Check, Conclusion}; /// What the Repair loop did outside itself, in the order it did it. #[derive(Debug, Clone, PartialEq, Eq)] @@ -487,6 +531,8 @@ mod tests { Watch(String, Option), /// Gave this head its Check re-run of these checks of its own. Rerun(String, Option, Vec), + /// Re-ran these Abandoned jobs of this head's own. + RerunAbandoned(String, Option, Vec), /// Started the Repair session of this kind for this Repair. Repair(Kind, Started), /// Took the Base fix for these Inherited failures, at this Base @@ -520,6 +566,9 @@ mod tests { /// Red on these checks of the branch's own, and these Inherited /// failures. Failed(&'static [&'static str], &'static [&'static str]), + /// Red on these Abandoned jobs of the branch's own, and these other + /// checks of its own. + Lost(&'static [&'static str], &'static [&'static str]), } impl Is { @@ -536,6 +585,17 @@ mod tests { }) .collect() }; + let abandoned = |names: &[&str]| -> Vec { + let mut checks = checks(names); + for check in &mut checks { + check.job = Some(ActionsJob { + check_run: 1, + workflow_run: 900, + abandoned: true, + }); + } + checks + }; match self { Is::Absent => Ci::Absent, Is::Passed => Ci::Passed, @@ -544,6 +604,11 @@ mod tests { inherited: checks(inherited), on_base: checks(inherited), }), + Is::Lost(lost, own) => Ci::Failed(FailedChecks { + own: [abandoned(lost), checks(own)].concat(), + inherited: Vec::new(), + on_base: Vec::new(), + }), } } } @@ -569,6 +634,7 @@ mod tests { new_commits: VecDeque, watches: VecDeque, reruns: VecDeque>, + abandoned_reruns: VecDeque>, /// For each Base fix, the cause it fails with, if it does. base_fixes: VecDeque>, /// For each merge, the error it fails with, if it does. @@ -713,6 +779,25 @@ mod tests { Ok(is.map(|is| is.ci())) } + fn rerun_abandoned( + &mut self, + head: &str, + base_commit: Option<&str>, + failed: &FailedChecks, + ) -> Result> { + self.did.push(Did::RerunAbandoned( + head.to_string(), + base_commit.map(String::from), + names(&failed.own), + )); + let is = self + .script + .abandoned_reruns + .pop_front() + .expect("no re-run of Abandoned jobs scripted"); + Ok(is.map(|is| is.ci())) + } + fn repair(&mut self, kind: Kind, repair: Repair) -> Result<()> { let started = match repair { Repair::Conflict(upstream) => Started::Conflict(upstream), @@ -1343,6 +1428,135 @@ mod tests { } } + /// The re-run of `h1`'s Abandoned job `wsl`, compared with `b1`. + fn rerun_abandoned_wsl() -> Did { + Did::RerunAbandoned( + "h1".to_string(), + Some("b1".to_string()), + vec!["wsl".to_string()], + ) + } + + #[test] + fn an_abandoned_job_alone_is_re_run_with_no_repair_and_green_reaches_the_goal() { + let (delivered, did) = deliver( + Goal::Merged, + Script { + heads: script(["h1"]), + watches: script([Is::Lost(&["wsl"], &[])]), + abandoned_reruns: script([Some(Is::Passed)]), + merges: script([None]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [&round("h1")[..], &[rerun_abandoned_wsl()], &merge("h1"),].concat() + ); + } + + #[test] + fn an_abandoned_job_abandoned_again_or_not_re_run_gets_the_ci_fix_repair_then_its_check_re_run() + { + for rerun in [Some(Is::Lost(&["wsl"], &[])), None] { + let (delivered, did) = deliver( + Goal::Merged, + Script { + heads: script(["h1"]), + watches: script([Is::Lost(&["wsl"], &[])]), + abandoned_reruns: script([rerun]), + reruns: script([Some(Is::Passed)]), + merges: script([None]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [ + &round("h1")[..], + &[ + rerun_abandoned_wsl(), + line("CI red; starting Repair 1 of 5"), + repair(1, ci_fix(&["wsl"], &[])), + Did::MergeBase, + Did::Push, + Did::Rerun( + "h1".to_string(), + Some("b1".to_string()), + vec!["wsl".to_string()], + ), + ], + &merge("h1"), + ] + .concat() + ); + } + } + + #[test] + fn a_head_whose_abandoned_jobs_were_re_run_gets_no_second_re_run_after_a_refused_merge() { + let (delivered, did) = deliver( + Goal::Merged, + Script { + heads: script(["h1", "h1", "h2"]), + watches: script([Is::Lost(&["wsl"], &[]), Is::Lost(&["wsl"], &[]), Is::Passed]), + abandoned_reruns: script([Some(Is::Passed)]), + merges: script([Some("merge refused"), None]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [ + &round("h1")[..], + &[rerun_abandoned_wsl()], + &merge("h1"), + &[line("the merge failed: merge refused")], + &round("h1"), + &[ + line("CI red; starting Repair 1 of 5"), + repair(1, ci_fix(&["wsl"], &[])), + ], + &round("h2"), + &merge("h2"), + ] + .concat() + ); + } + + #[test] + fn abandoned_jobs_beside_ordinary_own_failures_get_the_ci_fix_repair_as_any_red_ci() { + let (delivered, did) = deliver( + Goal::ReadyForReview, + Script { + heads: script(["h1", "h2"]), + watches: script([Is::Lost(&["wsl"], &["test"]), Is::Passed]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [ + &round("h1")[..], + &[ + line("CI red; starting Repair 1 of 5"), + repair(1, ci_fix(&["wsl", "test"], &[])), + ], + &round("h2"), + &[Did::CheckPr], + ] + .concat() + ); + } + /// What the loop does up to and including the Inherited failures line /// for `test` on `h1`. fn up_to_inherited_failures() -> Vec { diff --git a/src/github.rs b/src/github.rs index 8821c480..822b7178 100644 --- a/src/github.rs +++ b/src/github.rs @@ -833,6 +833,25 @@ impl GitHub { Ok(checks) } + /// The GitHub Actions job whose check run is `check_run`, as the jobs + /// API reports it: its conclusion and each of its steps'. + pub fn job_run(&self, issue: &IssueUrl, check_run: u64) -> Result { + let path = format!("repos/{}/actions/jobs/{check_run}", issue.repo_slug()); + let job = self.gh_json(&["api", &path])?; + let conclusion = |value: &Value| match value["status"].as_str() { + Some("completed") => Conclusion::of_check_run(value["conclusion"].as_str()), + _ => Conclusion::Unfinished, + }; + let steps = job["steps"] + .as_array() + .map(Vec::as_slice) + .unwrap_or_default(); + Ok(JobRun { + conclusion: conclusion(&job), + steps: steps.iter().map(conclusion).collect(), + }) + } + /// Ask GitHub to re-run the failed jobs of the GitHub Actions workflow run /// `workflow_run`, as a new attempt of it. pub fn rerun_failed_jobs(&self, issue: &IssueUrl, workflow_run: u64) -> Result<()> { @@ -1383,6 +1402,18 @@ pub struct ActionsJob { pub check_run: u64, /// The workflow run it belongs to. pub workflow_run: u64, + /// Whether it is an Abandoned job, which only the CI watch tells, and + /// only of a job that failed: false until then. + pub abandoned: bool, +} + +/// A GitHub Actions job as the jobs API reports it, with no annotation or +/// log text: its conclusion and each of its steps', in order, a step not +/// completed being [`Conclusion::Unfinished`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct JobRun { + pub conclusion: Conclusion, + pub steps: Vec, } impl ActionsJob { @@ -1394,6 +1425,7 @@ impl ActionsJob { Some(ActionsJob { check_run: check_run?, workflow_run: workflow_run.parse().ok()?, + abandoned: false, }) } } @@ -1409,7 +1441,8 @@ mod tests { ActionsJob::of(Some(41), Some(job)), Some(ActionsJob { check_run: 41, - workflow_run: 900 + workflow_run: 900, + abandoned: false, }) ); for not_a_job in [ diff --git a/tests/ci.rs b/tests/ci.rs index 247998e7..f3b7d1fb 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -271,6 +271,51 @@ fn assert_one_repair(scenario: &Scenario, result: &support::RunResult) { ); } +#[test] +fn a_job_that_lost_its_runner_is_re_run_with_no_repair_and_green_is_success() { + let scenario = Scenario::new(); + // As GitHub ends a job whose runner was lost: failed, with a step still + // in progress and none failed. + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head( + r#"[{"name": "smoke / wsl", "conclusion": "failure", + "url": "https://github.com/acme/widgets/actions/runs/900/job/1", + "steps": [{"status": "completed", "conclusion": "success"}, + {"status": "in_progress", "conclusion": null}, + {"status": "queued", "conclusion": null}], + "rerun": {"conclusion": "success", "stale_polls": 1}}]"# + ) + )); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(scenario.claude_calls().len(), 1, "the implement session"); + assert!( + !result.stderr.contains("starting Repair"), + "{}", + result.stderr + ); + assert_eq!(rerun_requests(&scenario), ["900"]); + let head = scenario.origin_git(&["rev-parse", "issue-7"]); + assert!( + result.stderr.contains(&format!( + "thirdshift: smoke / wsl lost its runner on {}; re-running it\n", + &head[..7] + )), + "stderr: {}", + result.stderr + ); + assert!( + result + .stderr + .contains(&format!("CI passed on {}", &head[..7])), + "stderr: {}", + result.stderr + ); +} + #[test] fn a_ci_fix_repair_that_makes_no_commit_gets_one_check_re_run_and_green_is_success() { let scenario = Scenario::new(); diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index ca67ce01..4ba9ab36 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -20,7 +20,9 @@ //! "prs": [{"number", "url", "head", "base", "state", "isDraft", "title", "body", //! "mergeable"?, "unknown_polls"?, "reviewDecision"?}], //! "checks": {"": [{"id", "name", "conclusion", "url", "pending_polls"?, -//! "rerun"?: {"conclusion", "stale_polls"?, "pending_polls"?}, +//! "steps"?: [{"status", "conclusion"}], +//! "rerun"?: {"conclusion", "stale_polls"?, "pending_polls"?, +//! "steps"?}, //! "stale"?: {"id", "conclusion", "polls"}}]}, //! "next_check_id"?: , //! "statuses": {"": [{"context", "state", "url"}]}, @@ -155,6 +157,11 @@ //! completed with its `conclusion`. With no `rerun`, the new attempt fails as //! the one before it did. //! +//! `gh api repos//actions/jobs/` answers with the job whose check +//! run has that `id`: its status and conclusion, as the check-runs endpoint +//! last reported them, and its `steps`, else one completed step that came to +//! the check's conclusion. A new attempt takes its `rerun`'s `steps`, if any. +//! //! Tests and fake agents change that state with `gh fake …` (not a real gh //! command): //! @@ -1401,6 +1408,13 @@ fn api(state: &mut Json, positional: &[String], flags: &Flags) { ); return; } + if let Some((repo, rest)) = repo_prefix(path) + && let Some(id) = rest.strip_prefix("actions/jobs/") + { + check_repo_is(state, Some(repo)); + println!("{}", actions_job(state, id)); + return; + } let Some((repo, sha, endpoint)) = commit_endpoint(path) else { die(&format!("fake gh: unsupported api path {path}"), 2) }; @@ -1537,6 +1551,58 @@ fn read_checks(state: &mut Json, sha: &str) -> Vec { runs } +/// The GitHub Actions job whose check run's `id` is `id`, with its steps. +fn actions_job(state: &Json, id: &str) -> Json { + let checks = state.get("checks").cloned().unwrap_or(object([])); + let Json::Object(commits) = &checks else { + panic!("checks is an object") + }; + let has_id = |run: Option<&Json>| { + run.and_then(|run| run.get("id")) + .is_some_and(|run_id| run_id.python() == id) + }; + let checks: Vec<&Json> = commits.iter().flat_map(|(_, list)| list.items()).collect(); + // The attempt before a re-run failed on a failed step. + if let Some(check) = checks.iter().find(|check| has_id(check.get("stale"))) { + let conclusion = check.at("stale").at("conclusion").clone(); + let step = object([ + ("status", string("completed")), + ("conclusion", conclusion.clone()), + ]); + return object([ + ("id", check.at("stale").at("id").clone()), + ("status", string("completed")), + ("conclusion", conclusion), + ("steps", Array(vec![step])), + ]); + } + let Some(check) = checks.into_iter().find(|check| has_id(Some(check))) else { + die(&format!("gh: Not Found (HTTP 404): no job {id}"), 1) + }; + let pending = check + .get("pending_polls") + .and_then(Json::as_i64) + .is_some_and(|polls| polls > 0); + let (status, conclusion) = if pending { + (string("in_progress"), Null) + } else { + (string("completed"), check.at("conclusion").clone()) + }; + let steps = match check.get("steps").filter(|steps| steps.truthy()) { + Some(steps) => steps.clone(), + None => Array(vec![object([ + ("status", status.clone()), + ("conclusion", conclusion.clone()), + ])]), + }; + object([ + ("id", check.at("id").clone()), + ("status", status), + ("conclusion", conclusion), + ("steps", steps), + ]) +} + /// The next check run `id`, each one higher than the last. fn next_check_id(state: &mut Json) -> Json { let next = state.get("next_check_id").and_then(Json::as_i64); @@ -1597,6 +1663,8 @@ fn run_rerun(state: &mut Json, positional: &[String], flags: &Flags) { if let Some(rerun) = &rerun { check.set("conclusion", rerun.at("conclusion").clone()); } + let steps = rerun.as_ref().and_then(|rerun| rerun.get("steps")).cloned(); + check.set("steps", steps.unwrap_or(Null)); let (stale_polls, pending_polls) = (polls("stale_polls"), polls("pending_polls")); let before = check.at("id").clone(); check.set( From 0fe106a7e46ddd16c2335adb48006300ef790f01 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Wed, 21 Oct 2026 06:02:58 -0400 Subject: [PATCH 2/2] Address review: read job steps only when they can matter, and an unreadable job is not abandoned Steps are read only while every own failure may yet be an Abandoned job, stopping at the first that is not, and a failed read is told and taken as not abandoned, so red CI still gets its CI-fix Repair rather than failing the Run. Names now use the glossary's Abandoned job, the re-run request says whether every workflow run was requested, and the two per-head allowances are documented. Co-Authored-By: Claude Opus 5.5 --- ...andoned-job-is-re-run-before-any-repair.md | 4 +- src/ci.rs | 196 ++++++++++++------ src/delivery/repair_loop.rs | 39 ++-- tests/ci.rs | 2 +- 4 files changed, 157 insertions(+), 84 deletions(-) diff --git a/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md b/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md index 59edfe24..a746b86f 100644 --- a/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md +++ b/docs/adr/0020-an-abandoned-job-is-re-run-before-any-repair.md @@ -1,4 +1,4 @@ -# A job that lost its runner is re-run before any Repair +# An Abandoned job is re-run before any Repair A GitHub Actions job whose runner GitHub lost ends as `failure` with a step still in progress and none failed. Its code never ran to completion, so a CI-fix **Repair** given it has nothing to fix: one did nothing for 81 s and 420,084 input tokens on Spec PR #842, spending one of the Delivery's 5 Repairs (#908). Now such a job is an **Abandoned job**. When every one of the branch's own failed checks on a head is one, thirdshift asks GitHub to re-run the failed jobs of their workflow runs, with no Repair, and watches CI on that head again, once per head for the complete Delivery. Green, the Run carries on as for any green head; red, it carries on as for any red CI. @@ -13,7 +13,7 @@ What makes a job abandoned is read from the jobs API's job and step fields alone ## Consequences -- Only the branch's own failed checks that are GitHub Actions jobs concluded `failure` have their steps read, once per watch that ends red. Inherited failures, commit statuses and other apps' check runs are never read or re-run for this. A failed read of a job fails the watch, as a failed read of checks does. +- Steps are read once per watch that ends red, and only when every one of the branch's own failed checks is a GitHub Actions job concluded `failure`, stopping at the first that is not abandoned, since only all of them being abandoned changes anything. Inherited failures, commit statuses and other apps' check runs are never read or re-run for this. A job whose steps cannot be read is not abandoned: the Run says so and carries on as for any red CI, rather than failing where it would have started a Repair. - The request and the watch after it are the Check re-run's: one request per workflow run, and the watch waits up to the grace period for the old check run ids to leave the listing before it trusts what it sees. If a request is refused, or no new attempt appears in time, the red CI first watched is handled as before, with a CI-fix Repair. - A head whose re-run came to an Abandoned job again, or any later watch of that head, gets a CI-fix Repair as before, and after it, if the head is unchanged, its Check re-run. A head whose Check re-run has been consumed gets no Abandoned job re-run. - The re-run starts no Repair and counts against neither the Repair cap nor the upstream-move budget. The Command log names it: ` lost its runner on ; re-running it`. diff --git a/src/ci.rs b/src/ci.rs index a3e3db7c..94d80744 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -12,6 +12,7 @@ use anyhow::{Context, Result, ensure}; use crate::github::{ActionsJob, Check, Conclusion, GitHub, JobRun}; use crate::issue::IssueUrl; use crate::poll; +use crate::process::Interruption; use crate::progress; /// How CI on a commit ended. @@ -31,18 +32,6 @@ impl Ci { } } -impl FailedChecks { - /// Whether the branch's own failures, at least one, are all Abandoned - /// jobs, whatever the Inherited failures are. - pub fn only_abandoned_jobs_own(&self) -> bool { - !self.own.is_empty() - && self - .own - .iter() - .all(|check| check.job.is_some_and(|job| job.abandoned)) - } -} - /// The checks that failed on a commit, at least one, split by whether the /// branch is the one to fix them. pub struct FailedChecks { @@ -58,6 +47,18 @@ pub struct FailedChecks { pub on_base: Vec, } +impl FailedChecks { + /// Whether the branch's own failures, at least one, are all Abandoned + /// jobs, whatever the Inherited failures are. + pub fn own_failures_all_abandoned(&self) -> bool { + !self.own.is_empty() + && self + .own + .iter() + .all(|check| check.job.is_some_and(|job| job.abandoned)) + } +} + /// Wait up to the grace period for any check or status on `sha`, then watch /// them until they have all finished. Checks that failed are compared, by /// name, with the checks on `base_commit` as they stand: the Base branch's CI @@ -132,8 +133,10 @@ trait Outside { type Deadline; /// The checks and statuses on the commit `sha`. fn checks_on(&mut self, sha: &str) -> Result>; - /// The GitHub Actions job whose check run is `check_run`, with its steps. - fn job_run(&mut self, check_run: u64) -> Result; + /// The GitHub Actions job whose check run is `check_run`, with its + /// steps, or why it could not be read. Fails only if the Run is not to + /// go on. + fn job_run(&mut self, check_run: u64) -> Result>; /// Ask GitHub to re-run the failed jobs of the workflow run /// `workflow_run`. fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()>; @@ -168,9 +171,10 @@ impl Outside for OnGitHub<'_> { Ok(checks) } - fn job_run(&mut self, check_run: u64) -> Result { + fn job_run(&mut self, check_run: u64) -> Result> { (self.actionable)()?; - let job_run = GitHub::new().job_run(self.issue, check_run)?; + let job_run = GitHub::new().job_run(self.issue, check_run); + Interruption::Ordinary.check()?; (self.actionable)()?; Ok(job_run) } @@ -276,7 +280,7 @@ fn rerun_through( "re-running the failed checks on {short}: {}", check_names(&failed.own) )); - let (rerun_checks, _) = request_reruns(outside, sha, failed)?; + let rerun_checks = request_reruns(outside, sha, failed)?; if rerun_checks.requested.is_empty() { return watch_to_end(outside, sha, base_commit, Some(&rerun_checks)).map(Some); } @@ -299,21 +303,22 @@ fn rerun_abandoned_through( check_names(&failed.own), short(sha) )); - match request_reruns(outside, sha, failed)? { - (rerun_checks, true) => watch_new_attempts(outside, sha, base_commit, &rerun_checks), - (_, false) => Ok(None), + let rerun_checks = request_reruns(outside, sha, failed)?; + if !rerun_checks.all_requested { + return Ok(None); } + watch_new_attempts(outside, sha, base_commit, &rerun_checks) } /// Ask GitHub to re-run the failed jobs of each workflow run the branch's /// own failures in `failed`, all GitHub Actions jobs, belong to, stopping at /// the first refusal, which is reported. Returns the checks whose new -/// attempts the watch must see, and whether every request succeeded. +/// attempts the watch must see. fn request_reruns<'a>( outside: &mut impl Outside, sha: &str, failed: &'a FailedChecks, -) -> Result<(RerunChecks<'a>, bool)> { +) -> Result> { let mut workflow_runs: Vec = jobs_of(&failed.own).map(|job| job.workflow_run).collect(); workflow_runs.sort_unstable(); workflow_runs.dedup(); @@ -334,15 +339,14 @@ fn request_reruns<'a>( } requested.push(*workflow_run); } - let all_requested = requested.len() == workflow_runs.len(); // GitHub re-runs every failed job of a workflow run, so an Inherited // failure that shares one with the branch's own gets a new attempt too. - let rerun_checks = RerunChecks { + Ok(RerunChecks { checks: failed.own.iter().chain(&failed.inherited).collect(), previous_attempt, + all_requested: requested.len() == workflow_runs.len(), requested, - }; - Ok((rerun_checks, all_requested)) + }) } /// Wait up to the grace period for the new attempts of `rerun_checks` to be @@ -370,12 +374,15 @@ fn watch_new_attempts( watch_to_end(outside, sha, base_commit, Some(rerun_checks)).map(Some) } -/// Known failed checks that must stay listed during a Check re-run. Those -/// in successfully requested workflow runs must also have new attempts. +/// Known failed checks that must stay listed during a Check re-run or a +/// re-run of Abandoned jobs. Those in successfully requested workflow runs +/// must also have new attempts. struct RerunChecks<'a> { checks: Vec<&'a Check>, previous_attempt: Vec, requested: Vec, + /// Whether every workflow run asked for was requested, none refused. + all_requested: bool, } impl RerunChecks<'_> { @@ -450,15 +457,39 @@ fn watch_to_end( None => Vec::new(), }; let mut failed = split(failed, on_base); - // Only the branch's own failures can be re-run for losing their runner. - for check in &mut failed.own { - if let Some(job) = &mut check.job - && check.conclusion == Conclusion::Failure - { - job.abandoned = is_abandoned(&outside.job_run(job.check_run)?); + mark_abandoned_jobs(outside, sha, &mut failed.own)?; + Ok(Ci::Failed(failed)) +} + +/// Mark which of `own`, the branch's own failures on `sha`, are Abandoned +/// jobs. Only every one being abandoned changes anything, so nothing is read +/// unless each is a GitHub Actions job concluded `failure`, and reading stops +/// at the first that is not abandoned. A job whose steps cannot be read is +/// not one, told in a progress line, so that red CI is taken as it was. +fn mark_abandoned_jobs(outside: &mut impl Outside, sha: &str, own: &mut [Check]) -> Result<()> { + let may_all_be_abandoned = own + .iter() + .all(|check| check.job.is_some() && check.conclusion == Conclusion::Failure); + if !may_all_be_abandoned { + return Ok(()); + } + for check in own { + let Some(job) = &mut check.job else { + unreachable!("every own failure is a job") + }; + match outside.job_run(job.check_run)? { + Ok(job_run) => job.abandoned = is_abandoned(&job_run), + Err(error) => outside.step(format!( + "couldn't read the steps of {} on {}: {error:#}", + check.name, + short(sha) + )), + } + if !job.abandoned { + break; } } - Ok(Ci::Failed(failed)) + Ok(()) } /// Ask `probe` every poll interval of `outside` until it answers `Some`, and @@ -597,7 +628,7 @@ fn count(checks: &[Check], wanted: Reading) -> usize { mod in_memory { use std::time::Duration; - use anyhow::{Result, bail}; + use anyhow::{Result, anyhow, bail}; use super::Outside; use crate::github::{Check, Conclusion, JobRun}; @@ -616,6 +647,7 @@ mod in_memory { grace_pauses: usize, pauses: usize, job_runs: Vec<(u64, JobRun)>, + failing_job_reads: Vec, /// The check run of each job read, in order. pub job_reads: Vec, /// The commit of each read, in order. @@ -639,6 +671,7 @@ mod in_memory { grace_pauses: 3, pauses: 0, job_runs: Vec::new(), + failing_job_reads: Vec::new(), job_reads: Vec::new(), reads: Vec::new(), reruns: Vec::new(), @@ -672,6 +705,12 @@ mod in_memory { self } + /// Fail each read of the job whose check run is `check_run`. + pub fn job_read_failing(mut self, check_run: u64) -> Self { + self.failing_job_reads.push(check_run); + self + } + /// Refuse a re-run of `workflow_run`. pub fn refusing(mut self, workflow_run: u64) -> Self { self.refused.push(workflow_run); @@ -717,16 +756,19 @@ mod in_memory { Ok(listing.cloned().unwrap_or_default()) } - fn job_run(&mut self, check_run: u64) -> Result { + fn job_run(&mut self, check_run: u64) -> Result> { self.job_reads.push(check_run); + if self.failing_job_reads.contains(&check_run) { + return Ok(Err(anyhow!("gh: HTTP 403: Resource not accessible"))); + } let scripted = self.job_runs.iter().find(|(id, _)| *id == check_run); - Ok(scripted.map_or_else( + Ok(Ok(scripted.map_or_else( || JobRun { conclusion: Conclusion::Failure, steps: vec![Conclusion::Success, Conclusion::Failure], }, |(_, job_run)| job_run.clone(), - )) + ))) } fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()> { @@ -1782,47 +1824,71 @@ mod tests { ); } - /// A job run as GitHub ends one whose runner was lost. - fn lost_runner() -> JobRun { + /// An Abandoned job's run, as GitHub ends one whose runner was lost. + fn abandoned_job_run() -> JobRun { JobRun { conclusion: Conclusion::Failure, steps: vec![Conclusion::Success, Conclusion::Unfinished], } } - /// The names of the Abandoned jobs among `checks`. - fn abandoned(checks: &[Check]) -> Vec<&str> { - checks - .iter() - .filter(|check| check.job.is_some_and(|job| job.abandoned)) - .map(|check| check.name.as_str()) - .collect() - } - #[test] - fn a_watch_reads_the_steps_of_failed_jobs_alone_and_marks_the_abandoned_ones() { + fn a_watch_reads_steps_only_while_every_own_failure_may_yet_be_an_abandoned_job() { + let lost = job(red("wsl"), 1, 900); let mut github = InMemory::new() .listing( HEAD, vec![vec![ - job(red("wsl"), 1, 900), + lost.clone(), job(red("test"), 2, 900), - job( - check("lint", Conclusion::Cancelled, "https://head/lint"), - 3, - 901, - ), + job(red("docs"), 3, 901), job(green("build"), 4, 900), - red("status"), ]], ) - .job_run(1, lost_runner()); + .job_run(1, abandoned_job_run()); let failed = failed_of(watch_through(&mut github, HEAD, None).unwrap()); - assert_eq!(abandoned(&failed.own), ["wsl"]); + // `test` failed on a step, so `docs` can change nothing. assert_eq!(github.job_reads, [1, 2]); - assert!(!failed.only_abandoned_jobs_own()); + assert!(!failed.own_failures_all_abandoned()); + + // A failure that is no job concluded `failure` can't be re-run so. + for other in [ + red("status"), + job( + check("lint", Conclusion::Cancelled, "https://head/lint"), + 3, + 901, + ), + ] { + let mut github = InMemory::new() + .listing(HEAD, vec![vec![lost.clone(), other]]) + .job_run(1, abandoned_job_run()); + + let failed = failed_of(watch_through(&mut github, HEAD, None).unwrap()); + + assert!(github.job_reads.is_empty()); + assert!(!failed.own_failures_all_abandoned()); + } + } + + #[test] + fn a_job_whose_steps_cannot_be_read_is_not_abandoned_and_ci_stays_red() { + let mut github = InMemory::new() + .listing(HEAD, vec![vec![job(red("wsl"), 1, 900)]]) + .job_read_failing(1); + + let failed = failed_of(watch_through(&mut github, HEAD, None).unwrap()); + + assert_eq!(listed(&failed.own), ["wsl: https://head/wsl"]); + assert!(!failed.own_failures_all_abandoned()); + assert_eq!( + github.steps.last().map(String::as_str), + Some( + "couldn't read the steps of wsl on abcdef0: gh: HTTP 403: Resource not accessible" + ) + ); } #[test] @@ -1833,13 +1899,15 @@ mod tests { vec![vec![job(red("wsl"), 1, 900), job(red("lint"), 2, 901)]], ) .listing(BASE, vec![vec![red("lint")]]) - .job_run(1, lost_runner()); + .job_run(1, abandoned_job_run()); let failed = failed_of(watch_through(&mut github, HEAD, Some(BASE)).unwrap()); - assert!(failed.only_abandoned_jobs_own()); + assert!(failed.own_failures_all_abandoned()); assert_eq!(github.job_reads, [1]); - assert!(!failed_on_watch(vec![red("lint")], vec![red("lint")]).only_abandoned_jobs_own()); + assert!( + !failed_on_watch(vec![red("lint")], vec![red("lint")]).own_failures_all_abandoned() + ); } /// `check` as the Abandoned job whose check run is `check_run`, of the diff --git a/src/delivery/repair_loop.rs b/src/delivery/repair_loop.rs index 26841fed..965bc7f4 100644 --- a/src/delivery/repair_loop.rs +++ b/src/delivery/repair_loop.rs @@ -240,8 +240,10 @@ enum Round { #[derive(Default)] struct CheckReruns { handed_to_repair: Option<(String, FailedChecks)>, + /// The heads whose Check re-run allowance is consumed. consumed: HashSet, - abandoned_rerun: HashSet, + /// The heads whose Abandoned jobs have had their one re-run. + abandoned_jobs_rerun: HashSet, } impl CheckReruns { @@ -254,8 +256,9 @@ impl CheckReruns { /// Assess fresh CI against this round's selected Base commit. An /// unchanged Repair head consumes its allowance before any re-run /// request; any other head is watched, its Abandoned jobs re-run if - /// they are its only own failures. Later watches remain allowed, but own failures on a consumed - /// head decline the CI fix before another Repair can start. + /// they are its only own failures. Later watches remain allowed, but own + /// failures on a consumed head decline the CI fix before another Repair + /// can start. fn assess( &mut self, outside: &mut impl Outside, @@ -273,7 +276,7 @@ impl CheckReruns { } _ => { let watched = outside.watch(head, compared_with)?; - Some(self.past_abandoned_jobs(outside, head, compared_with, watched)?) + Some(self.rerun_abandoned_jobs_once(outside, head, compared_with, watched)?) } }; match ci { @@ -284,14 +287,12 @@ impl CheckReruns { ), } } -} -impl CheckReruns { /// `ci`, a watch of `head`, or, if it is red only on Abandoned jobs of /// the branch's own and `head` has had neither their re-run nor its /// Check re-run, what their re-run came to. A refused or unseen re-run /// leaves `ci` as it was. - fn past_abandoned_jobs( + fn rerun_abandoned_jobs_once( &mut self, outside: &mut impl Outside, head: &str, @@ -301,9 +302,9 @@ impl CheckReruns { let Ci::Failed(failed) = &ci else { return Ok(ci); }; - if !failed.only_abandoned_jobs_own() + if !failed.own_failures_all_abandoned() || self.consumed.contains(head) - || !self.abandoned_rerun.insert(head.to_string()) + || !self.abandoned_jobs_rerun.insert(head.to_string()) { return Ok(ci); } @@ -568,7 +569,7 @@ mod tests { Failed(&'static [&'static str], &'static [&'static str]), /// Red on these Abandoned jobs of the branch's own, and these other /// checks of its own. - Lost(&'static [&'static str], &'static [&'static str]), + Abandoned(&'static [&'static str], &'static [&'static str]), } impl Is { @@ -604,8 +605,8 @@ mod tests { inherited: checks(inherited), on_base: checks(inherited), }), - Is::Lost(lost, own) => Ci::Failed(FailedChecks { - own: [abandoned(lost), checks(own)].concat(), + Is::Abandoned(jobs, own) => Ci::Failed(FailedChecks { + own: [abandoned(jobs), checks(own)].concat(), inherited: Vec::new(), on_base: Vec::new(), }), @@ -1443,7 +1444,7 @@ mod tests { Goal::Merged, Script { heads: script(["h1"]), - watches: script([Is::Lost(&["wsl"], &[])]), + watches: script([Is::Abandoned(&["wsl"], &[])]), abandoned_reruns: script([Some(Is::Passed)]), merges: script([None]), ..Script::default() @@ -1460,12 +1461,12 @@ mod tests { #[test] fn an_abandoned_job_abandoned_again_or_not_re_run_gets_the_ci_fix_repair_then_its_check_re_run() { - for rerun in [Some(Is::Lost(&["wsl"], &[])), None] { + for rerun in [Some(Is::Abandoned(&["wsl"], &[])), None] { let (delivered, did) = deliver( Goal::Merged, Script { heads: script(["h1"]), - watches: script([Is::Lost(&["wsl"], &[])]), + watches: script([Is::Abandoned(&["wsl"], &[])]), abandoned_reruns: script([rerun]), reruns: script([Some(Is::Passed)]), merges: script([None]), @@ -1503,7 +1504,11 @@ mod tests { Goal::Merged, Script { heads: script(["h1", "h1", "h2"]), - watches: script([Is::Lost(&["wsl"], &[]), Is::Lost(&["wsl"], &[]), Is::Passed]), + watches: script([ + Is::Abandoned(&["wsl"], &[]), + Is::Abandoned(&["wsl"], &[]), + Is::Passed, + ]), abandoned_reruns: script([Some(Is::Passed)]), merges: script([Some("merge refused"), None]), ..Script::default() @@ -1536,7 +1541,7 @@ mod tests { Goal::ReadyForReview, Script { heads: script(["h1", "h2"]), - watches: script([Is::Lost(&["wsl"], &["test"]), Is::Passed]), + watches: script([Is::Abandoned(&["wsl"], &["test"]), Is::Passed]), ..Script::default() }, ); diff --git a/tests/ci.rs b/tests/ci.rs index f3b7d1fb..70fd8e4b 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -272,7 +272,7 @@ fn assert_one_repair(scenario: &Scenario, result: &support::RunResult) { } #[test] -fn a_job_that_lost_its_runner_is_re_run_with_no_repair_and_green_is_success() { +fn an_abandoned_job_is_re_run_with_no_repair_and_green_is_success() { let scenario = Scenario::new(); // As GitHub ends a job whose runner was lost: failed, with a step still // in progress and none failed.