From 43c90eca544ea56c3f19d9488e20a3b407a6143f Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 1 Oct 2026 12:51:09 +0000 Subject: [PATCH 1/5] Re-run failed checks once before a Declined CI fix fails the Run When a CI-fix Repair leaves the head unchanged, ask GitHub to re-run the branch's own failed checks, once per workflow run, and watch CI on that head again from the new attempt on. Green carries on; red again, a check that can't be re-run, or a refused re-run is a Declined CI fix as before. Co-Authored-By: Claude Opus 5.5 --- CONTEXT.md | 6 +- README.md | 6 +- ...e-check-re-run-before-a-declined-ci-fix.md | 19 + src/ci.rs | 72 +++- src/github.rs | 71 +++- src/main.rs | 5 + src/prompts_page.rs | 1 + src/run.rs | 48 ++- tests/ci.rs | 350 +++++++++++++++++- tests/fakes/gh.rs | 132 ++++++- tests/merge_run.rs | 42 +++ tests/spec_run.rs | 37 ++ 12 files changed, 759 insertions(+), 30 deletions(-) create mode 100644 docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md diff --git a/CONTEXT.md b/CONTEXT.md index 75ecdd9ff..c6bb13c0f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -111,7 +111,11 @@ The step at the end of a **Merge run** in which thirdshift itself merges the pul A merge the **Self-merge** tried that failed, where the round of the Repair loop that followed found nothing to fix: the **Base branch** unchanged, no conflict, CI green or absent, the pull request mergeable. The cause is a repository setting or rule, such as merge commits disallowed or a review required. thirdshift never reads GitHub's error text to decide it. The **Merge run** is a **Failed run** that leaves the pull request ready for review. **Declined CI fix**: -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. Going round again would only watch the same red CI, so 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. +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 **Check re-run**; if CI is red again, or there can be no Check 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. + +**Check re-run**: +The one time per head commit 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**. It lets a check that failed for a reason that does not repeat, such as a flaky test, pass. 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**. +_Avoid_: retry, re-trigger, CI re-run **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. diff --git a/README.md b/README.md index 29b60df7d..53e8de43a 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,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 Claude Code session in that worktree with the **Factory skills** in [`skills/`](skills/) loaded, 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. 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, the Run ends as a Failed run, since CI would stay red on the same head. 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, or if GitHub refuses the re-run, which stderr shows. 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-0009](docs/adr/0009-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). 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 `). It never uses GitHub's auto-merge ([ADR-0004](docs/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. 8. Cleans up: removes the worktree, the local Issue branch and the temporary plugin directory, whether the Run succeeded or not. The one exception is a **Failed run** whose work could not be pushed: see below. @@ -344,7 +344,7 @@ A Run is not idempotent: re-running builds on whatever is already on the branch, ## Failed runs -A **Failed run** is one that ends, including by Ctrl-C or a closed terminal, without an open pull request from its Issue branch that targets the Base branch, is mergeable and has passing CI, or, for a Merge run, without that pull request merged. Causes include the session exiting non-zero, no pull request or one with the wrong base, running out of Repairs, CI red only on **Inherited failures** (`CI red on [, …], which also fails on at ; fix first`), a CI-fix Repair that finds nothing on the branch to fix (a **declined CI fix**: the head is unchanged after it, so CI would stay red), a session that still ends while waiting on background work after its Resume, and a Base branch that keeps moving while CI runs, or in a Merge run, an Issue branch that keeps getting Foreign commits. +A **Failed run** is one that ends, including by Ctrl-C or a closed terminal, without an open pull request from its Issue branch that targets the Base branch, is mergeable and has passing CI, or, for a Merge run, without that pull request merged. Causes include the session exiting non-zero, no pull request or one with the wrong base, running out of Repairs, CI red only on **Inherited failures** (`CI red on [, …], which also fails on at ; fix first`), a CI-fix Repair that finds nothing on the branch to fix (a **declined CI fix**: the head is unchanged after it, and its one Check re-run left CI red or could not happen), a session that still ends while waiting on background work after its Resume, and a Base branch that keeps moving while CI runs, or in a Merge run, an Issue branch that keeps getting Foreign commits. A Failed run: @@ -360,7 +360,7 @@ A **Spec** is an issue with sub-issues, its **Tickets**. `thirdshift The Spec PR opens as a draft as soon as the first Ticket lands, titled from the Spec, with `Closes #` and a Tickets checklist: one line per Ticket, ticked once it is done, with its pull request, or saying it is running, failed, blocked or unready. thirdshift rewrites the checklist between its `` markers as each Ticket starts and ends, leaving the rest of the body as it is. Once every Ticket is done, the **Spec review** rewrites the body, and thirdshift puts the checklist back, appending it if the markers are gone, before marking the Spec PR ready. -The Spec review session reviews the whole Spec branch against the Base branch and the Spec. Once it is marked ready, the Spec PR goes through the same step 6 as a Run's pull request, with the Spec as the issue and the Spec branch as the Issue branch: the Base branch is merged in (never rebased), CI watched, and a conflict or red CI handed to a Repair, within the same budgets. Inherited failures are told apart the same way everywhere: the Spec PR's against the Base branch, and a Ticket's against the Spec branch, its Base branch, with the cause shown on the Ticket's line of the checklist. +The Spec review session reviews the whole Spec branch against the Base branch and the Spec. Once it is marked ready, the Spec PR goes through the same step 6 as a Run's pull request, with the Spec as the issue and the Spec branch as the Issue branch: the Base branch is merged in (never rebased), CI watched, and a conflict or red CI handed to a Repair, within the same budgets, with the same Check re-run after a CI-fix Repair that leaves the head unchanged. Inherited failures are told apart the same way everywhere: the Spec PR's against the Base branch, and a Ticket's against the Spec branch, its Base branch, with the cause shown on the Ticket's line of the checklist. Tickets always merge into the Spec branch, whatever the command or the User config says. `merge` (or `--merge`, or `merge.always = true` without `no-merge`) applies to the Spec PR alone: the Spec run ends with the Self-merge of the Spec PR into the Base branch, then deletes the Spec branch on `origin` and closes the Spec if the merge did not. Without it, the Spec run ends with the Spec PR ready for review. A Spec PR that fails from the Spec review on follows the [Failed run](#failed-runs) rules for its pull request, back to draft unless only the Self-merge could not happen (a policy refusal), and the Spec run exits `1`. diff --git a/docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md b/docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md new file mode 100644 index 000000000..31bf0af6c --- /dev/null +++ b/docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md @@ -0,0 +1,19 @@ +# A head a CI-fix Repair left unchanged gets one Check re-run before the Run fails + +A check that fails for a reason that does not repeat, such as a flaky test or a runner or network fault, used to fail the Run: the CI-fix Repair rightly committed nothing, the head was still the commit whose CI had failed, and the Run ended as a **Declined CI fix** although the same commit would pass (#262, where the Failed-run marker commit pushed next went green on the same code). Now, at the point where the Run would fail as a Declined CI fix, thirdshift does a **Check re-run** first: it asks GitHub to re-run the failed jobs of each GitHub Actions workflow run that the branch's own failed checks belong to, once per workflow run, and watches CI on that head again. Green, the Run carries on as for any green head. Red again on a check of the branch's own, the Run is a Failed run with the Declined CI fix cause, with no second Repair and no second re-run for that head. + +The trigger is the head being unchanged after a CI-fix Repair, and nothing else. What the Repair concluded, and the "CI notes" it may have written in the pull request body, are never read: this is a deterministic check, as in ADR 0008. + +## Considered Options + +- **Re-run before any Repair.** Rejected: it adds a full CI cycle to every real failure, which is most of them, to save a Repair session on the rare flaky one. +- **Re-run only when the Repair calls the failure flaky.** Rejected: it leaves the outcome to the agent's judgement and to thirdshift reading its prose, where the unchanged head is a fact thirdshift already checks. +- **Re-run each failed job on its own** (`POST …/actions/jobs//rerun`) rather than a workflow run's failed jobs. Rejected: one request per check, where failed checks usually share a workflow run, for the sake of the case below. +- **More than one re-run per head.** Rejected: a check that fails twice on the same commit is not one a Run should wait out, and each re-run costs a CI cycle. + +## Consequences + +- A Check re-run needs every one of the branch's own failed checks to be a GitHub Actions job, told by the check run's details URL naming the workflow run. With a commit status or another app's check run among them, the head could not go green, so nothing is re-run and the Run fails as before. A re-run GitHub refuses, for example for a missing permission, fails the Run the same way, with the Declined CI fix cause and the refusal in a progress line. +- The watch after the re-run must not read the attempt before it. A re-run job gets a new check run, and GitHub lists only the latest check run of a name, so thirdshift waits up to the grace period for the failed check runs' ids to leave the list, then watches as usual. If they never leave, the Run fails as a Declined CI fix rather than trust what it sees. +- Inherited failures are never re-run on their own account and are handled as before, also after a Check re-run. An Inherited failure that shares a workflow run with one of the branch's own failed checks is re-run along with it, since the request is per workflow run. +- A Check re-run starts no Repair and counts against neither the Repair cap nor the upstream-move budget. A later head, from a Repair that committed or the Base branch merged in again, may get its own. diff --git a/src/ci.rs b/src/ci.rs index 90db2428c..2cbda82d5 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -2,7 +2,7 @@ use anyhow::Result; -use crate::github::{self, Check, CheckState}; +use crate::github::{self, ActionsJob, Check, CheckState}; use crate::issue::IssueUrl; use crate::poll; use crate::progress; @@ -49,7 +49,73 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result, + failed: &FailedChecks, +) -> Result> { + let short = short(sha); + let (jobs, others): (Vec<&Check>, Vec<&Check>) = + failed.own.iter().partition(|check| check.job.is_some()); + if !others.is_empty() { + progress::step(format_args!( + "the failed checks on {short} can't be re-run: not GitHub Actions jobs: {}", + names(&others) + )); + return Ok(None); + } + let jobs: Vec = jobs.iter().filter_map(|check| check.job).collect(); + progress::step(format_args!( + "re-running the failed checks on {short}: {}", + check_names(&failed.own) + )); + let mut workflow_runs: Vec = jobs.iter().map(|job| job.workflow_run).collect(); + workflow_runs.sort_unstable(); + workflow_runs.dedup(); + for workflow_run in workflow_runs { + if let Err(error) = github::rerun_failed_jobs(issue, workflow_run) { + progress::step(format_args!("GitHub refused the re-run: {error:#}")); + return Ok(None); + } + } + let grace = poll::grace_period(); + let appeared = poll::within(grace, || { + let checks = github::checks_on(issue, sha)?; + let stale = checks + .iter() + .filter_map(|check| check.job) + .any(|job| jobs.iter().any(|failed| failed.check_run == job.check_run)); + Ok((!stale).then_some(())) + })?; + if appeared.is_none() { + progress::step(format_args!( + "no re-run appeared on {short} within {}s", + grace.as_secs() + )); + return Ok(None); + } + watch_to_end(issue, sha, base_commit).map(Some) +} + +/// [`watch`], once checks have appeared on `sha`. +fn watch_to_end(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result { + let short = short(sha); let mut reported_pending = None; let checks = poll::until(|| { let checks = github::checks_on(issue, sha)?; @@ -93,6 +159,10 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result String { + names(&checks.iter().collect::>()) +} + +fn names(checks: &[&Check]) -> String { let names: Vec<&str> = checks.iter().map(|check| check.name.as_str()).collect(); names.join(", ") } diff --git a/src/github.rs b/src/github.rs index df941ea13..1d179d21f 100644 --- a/src/github.rs +++ b/src/github.rs @@ -531,9 +531,36 @@ pub struct Check { pub name: String, pub state: CheckState, pub url: Option, + /// The GitHub Actions job it is, if it is one. No other check can be + /// re-run: not a commit status, nor a check run of another app. + pub job: Option, } -/// Every check run and commit status on commit `sha`. +/// A job of a GitHub Actions workflow run, as one of its attempts ran it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ActionsJob { + /// Its check run's id, which a new attempt of the job does not keep. + pub check_run: u64, + /// The workflow run it belongs to. + pub workflow_run: u64, +} + +impl ActionsJob { + /// The job whose check run is `check_run`, if `details_url`, the check + /// run's, is a job's page: `…/actions/runs//job/`. + fn of(check_run: Option, details_url: Option<&str>) -> Option { + let (_, after) = details_url?.split_once("/actions/runs/")?; + let (workflow_run, _) = after.split_once("/job/")?; + Some(ActionsJob { + check_run: check_run?, + workflow_run: workflow_run.parse().ok()?, + }) + } +} + +/// Every check run and commit status on commit `sha`. Of the check runs +/// sharing a name, as the attempts of a re-run job do, only the latest is +/// here. pub fn checks_on(issue: &IssueUrl, sha: &str) -> Result> { let commit = format!("repos/{}/commits/{sha}", issue.repo_slug()); let runs = gh_api_items(&format!("{commit}/check-runs?per_page=100"), "check_runs")?; @@ -558,6 +585,7 @@ pub fn checks_on(issue: &IssueUrl, sha: &str) -> Result> { .context("a check run has no name")? .to_string(), state, + job: ActionsJob::of(run["id"].as_u64(), run["details_url"].as_str()), url: url(&run["details_url"]).or_else(|| url(&run["html_url"])), }); } @@ -574,11 +602,25 @@ pub fn checks_on(issue: &IssueUrl, sha: &str) -> Result> { .to_string(), state, url: url(&status["target_url"]), + job: None, }); } Ok(checks) } +/// 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(issue: &IssueUrl, workflow_run: u64) -> Result<()> { + gh(&[ + "run", + "rerun", + &workflow_run.to_string(), + "--failed", + "--repo", + &issue.repo_slug(), + ]) +} + /// Run `gh `, failing with its stderr if it exits non-zero. fn gh(args: &[&str]) -> Result<()> { gh_stdout(args).map(drop) @@ -637,3 +679,30 @@ fn gh_json(args: &[&str]) -> Result { serde_json::from_slice(&output.stdout) .with_context(|| format!("{command} returned invalid JSON")) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_a_check_run_whose_details_are_a_jobs_page_is_an_actions_job() { + let job = "https://github.com/acme/widgets/actions/runs/900/job/41"; + assert_eq!( + ActionsJob::of(Some(41), Some(job)), + Some(ActionsJob { + check_run: 41, + workflow_run: 900 + }) + ); + for not_a_job in [ + "https://ci.example/test", + "https://github.com/acme/widgets/actions/runs/900", + "https://github.com/acme/widgets/actions/runs/latest/job/41", + "https://github.com/acme/widgets/runs/41", + ] { + assert_eq!(ActionsJob::of(Some(41), Some(not_a_job)), None); + } + assert_eq!(ActionsJob::of(Some(41), None), None); + assert_eq!(ActionsJob::of(None, Some(job)), None); + } +} diff --git a/src/main.rs b/src/main.rs index a7566639e..96cbf7ce3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -72,6 +72,11 @@ open base-fix issue for the same Base branch and checks waits for that one to cl instead of starting another, and a Spec run's Tickets that meet the same Inherited failure share one Base fix. +A failed check that is the branch's own starts a CI-fix Repair. If the Repair leaves the head +commit as it was, thirdshift asks GitHub to re-run those failed checks, once per head, and +watches CI there again: a flaky check then passes and the Run goes on. If CI is red again, +or a failed check can't be re-run (only GitHub Actions jobs can), the Run fails. + On a Spec, an issue with sub-issues, the Run is a Spec run: it takes every Ticket (sub-issue) it can reach, in the order their \"blocked by\" links allow, each merged into the Spec branch. Its Spec PR opens as a draft, with a Tickets checklist, once the first Ticket lands, and is marked ready diff --git a/src/prompts_page.rs b/src/prompts_page.rs index 54c121c80..6fffa5dd1 100644 --- a/src/prompts_page.rs +++ b/src/prompts_page.rs @@ -110,6 +110,7 @@ fn prompts() -> Vec { name: name.to_string(), state: CheckState::Failed, url: Some(CHECK_URL.to_string()), + job: None, }; let failed = FailedChecks { own: vec![check(CHECK)], diff --git a/src/run.rs b/src/run.rs index 58dab0144..85d823c62 100644 --- a/src/run.rs +++ b/src/run.rs @@ -8,7 +8,7 @@ use anyhow::{Context, Result, anyhow, bail}; use crate::base_fix::{BaseFix, BaseFixAsk}; use crate::branch::{self, Selection}; use crate::child_run::Kind; -use crate::ci::{self, Ci}; +use crate::ci::{self, Ci, FailedChecks}; use crate::failed_run::{self, FailedRun, PolicyRefusal}; use crate::git::Git; use crate::github::{self, Mergeable, PullRequest}; @@ -453,16 +453,20 @@ impl RepairLoop<'_> { /// as a Base move, and otherwise once its Base fix has merged, or fails /// naming the checks and the Base branch commit: see [`BaseFix::fix`]. A /// Base fix's own Run sees no Inherited failures. + /// 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, whatever the Repair concluded: see [`ci::rerun`]. CI then + /// green, or red only on Inherited failures, is taken as from any watch. /// Returns the head commit whose CI was last watched and found green or - /// absent. Fails with a Declined CI fix if, after a CI-fix Repair and the - /// Base branch merged again, the head is still the one whose CI failed, - /// and once a Repair beyond `MAX_REPAIRS`, or a round - /// beyond `MAX_UPSTREAM_MOVES`, would be needed. + /// absent. Fails with a Declined CI fix if that Check re-run leaves a + /// check of the branch's own red, or there can be none, and once a Repair + /// beyond `MAX_REPAIRS`, or a round beyond `MAX_UPSTREAM_MOVES`, would be + /// needed. fn run(&mut self, run_session: &mut impl FnMut(&str, &str) -> Result<()>) -> Result { let (issue, worktree, base, pr_url) = (self.issue, self.worktree, self.base, self.pr_url); let branch = worktree.branch(); - // The head whose red CI the last CI-fix Repair was given. - let mut handed_to_repair = None; + // The head the last CI-fix Repair was given, with its failed checks. + let mut handed_to_repair: Option<(String, FailedChecks)> = None; loop { if self.goal == Goal::Merged { self.take_in_foreign_commits(run_session)?; @@ -475,18 +479,27 @@ impl RepairLoop<'_> { } worktree.push()?; let head = worktree.head()?; - if handed_to_repair.as_ref() == Some(&head) { - bail!( - "CI red on {} and the Repair found nothing to fix on the branch", - ci::short(&head) - ); - } let base_commit = worktree.merged_base_commit(base)?; let compared_with = self .base_fix .sees_inherited_failures() .then_some(base_commit.as_str()); - match ci::watch(issue, &head, compared_with)? { + let unchanged = handed_to_repair + .take() + .filter(|(handed, _)| *handed == head); + let ci = match unchanged { + None => ci::watch(issue, &head, compared_with)?, + // A Declined CI fix, unless the head's one Check re-run turns + // the branch's own checks green: it gets no second Repair. + Some((_, failed)) => match ci::rerun(issue, &head, compared_with, &failed)? { + Some(ci) if !has_own_failures(&ci) => ci, + _ => bail!( + "CI red on {} and the Repair found nothing to fix on the branch", + ci::short(&head) + ), + }, + }; + match ci { Ci::Absent | Ci::Passed => { if worktree.base_branch_moved(base)? { self.budgets.count_base_move(base, "while CI ran")?; @@ -525,7 +538,7 @@ impl RepairLoop<'_> { &kind, &prompt::ci_fix_repair(issue, base, branch, pr_url, &failed), )?; - handed_to_repair = Some(head); + handed_to_repair = Some((head, failed)); } } } @@ -599,6 +612,11 @@ impl RepairLoop<'_> { } } +/// Whether `ci` is red on a check that is the branch's own to fix. +fn has_own_failures(ci: &Ci) -> bool { + matches!(ci, Ci::Failed(failed) if !failed.own.is_empty()) +} + /// Mark the PR whose head is `branch` ready for review, failing unless it /// exists, is open and targets `base`. pub fn mark_pr_ready(issue: &IssueUrl, branch: &str, base: &str) -> Result { diff --git a/tests/ci.rs b/tests/ci.rs index 358fbc9dd..c221a9066 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -1,8 +1,9 @@ //! Watching CI: after each push thirdshift waits for checks on the head //! commit, hands red CI to a CI-fix Repair, and gives up after 5 Repairs, or -//! as soon as a CI-fix Repair makes no commit. A red check that also fails on -//! the Base branch commit the head merged in is an Inherited failure, which -//! gets no Repair. +//! once a CI-fix Repair makes no commit and a Check re-run, if the failed +//! checks can have one, leaves CI red. A red check that also fails on the +//! Base branch commit the head merged in is an Inherited failure, which gets +//! no Repair and no Check re-run. mod support; @@ -257,6 +258,349 @@ fn a_second_ci_fix_repair_that_makes_no_commit_is_a_failed_run() { assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); } +/// A check run named `name`, job `job` of the GitHub Actions workflow run +/// `run`, that failed. With a `rerun`, as `{"conclusion": "success"}`, that is +/// what its Check re-run comes to; with `null`, it fails again. +fn actions_failure(name: &str, run: u32, job: u32, rerun: &str) -> String { + format!( + r#"{{"name": "{name}", "conclusion": "failure", + "url": "https://github.com/acme/widgets/actions/runs/{run}/job/{job}", + "rerun": {rerun}}}"# + ) +} + +const RERUN_PASSES: &str = r#"{"conclusion": "success"}"#; + +/// The workflow runs thirdshift asked GitHub to re-run the failed jobs of, in +/// order. +fn rerun_requests(scenario: &Scenario) -> Vec { + scenario + .gh_calls_of("run", "rerun") + .into_iter() + .map(|call| { + assert_eq!(call[3..], ["--failed", "--repo", "acme/widgets"]); + call[2].clone() + }) + .collect() +} + +/// Assert that the Run started exactly one Repair. +fn assert_one_repair(scenario: &Scenario, result: &support::RunResult) { + assert_eq!( + result.stderr.matches("starting Repair").count(), + 1, + "stderr: {}", + result.stderr + ); + assert_eq!( + scenario.claude_calls().len(), + 2, + "the implement session and 1 Repair" + ); +} + +#[test] +fn a_ci_fix_repair_that_makes_no_commit_gets_one_check_re_run_and_green_is_success() { + let scenario = Scenario::new(); + // Two failed checks of one workflow run, and a third of another. + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(&format!( + "[{}, {}, {}]", + actions_failure("test", 900, 1, RERUN_PASSES), + actions_failure("lint", 900, 2, RERUN_PASSES), + actions_failure("docs", 901, 3, RERUN_PASSES) + )) + )); + // The Repair finds the failure flaky and commits nothing. + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, "https://github.com/acme/widgets/pull/1\n"); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), ["900", "901"]); + let head = scenario.origin_git(&["rev-parse", "issue-7"]); + assert!( + result.stderr.contains(&format!( + "thirdshift: re-running the failed checks on {}: test, lint, docs\n", + &head[..7] + )), + "stderr: {}", + result.stderr + ); + assert!( + result + .stderr + .contains(&format!("CI passed on {}", &head[..7])), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], false); + assert_eq!( + scenario.origin_log("issue-7"), + Some(vec![ + "Add feature".to_string(), + "Initial commit".to_string() + ]) + ); + scenario.assert_cleaned_up("issue-7"); +} + +#[test] +fn a_check_re_run_that_fails_again_is_a_failed_run_with_no_second_repair_or_re_run() { + let scenario = Scenario::new(); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(&format!("[{}]", actions_failure("test", 900, 1, "null"))) + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_ne!(result.code, Some(0)); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), ["900"]); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + let cause = declined_ci_fix(watched.trim()); + assert!(result.stderr.contains(&cause), "stderr: {}", result.stderr); + assert_eq!( + result.stderr.matches("CI failed on").count(), + 2, + "before the Repair and after the Check re-run; stderr: {}", + result.stderr + ); + assert_session_log_is(&result, "repair-1"); + assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); + assert_eq!( + scenario.origin_log("issue-7"), + Some(vec![ + format!("thirdshift: failed run ({cause})"), + "Add feature".to_string(), + "Initial commit".to_string(), + ]) + ); +} + +/// Assert that the Run ended as a Declined CI fix with no Check re-run asked +/// for, or with only `requests`. +fn assert_declined_ci_fix(scenario: &Scenario, result: &support::RunResult, requests: &[&str]) { + assert_ne!(result.code, Some(0)); + assert_one_repair(scenario, result); + assert_eq!(rerun_requests(scenario), requests); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + let cause = declined_ci_fix(watched.trim()); + assert_eq!( + scenario.origin_log("issue-7").unwrap()[0], + format!("thirdshift: failed run ({cause})"), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); +} + +#[test] +fn a_failed_commit_status_cant_be_re_run_so_nothing_is() { + let scenario = Scenario::new(); + // The check run could be re-run, but the commit status could not, so the + // head could not go green. + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}gh fake statuses \"$(git rev-parse HEAD)\" '{}'\n", + checks_on_head(&format!( + "[{}]", + actions_failure("test", 900, 1, RERUN_PASSES) + )), + r#"[{"context": "deploy/preview", "state": "error", "url": "https://deploy.example/7"}]"#, + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_declined_ci_fix(&scenario, &result, &[]); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + assert!( + result.stderr.contains(&format!( + "thirdshift: the failed checks on {} can't be re-run: not GitHub Actions jobs: deploy/preview\n", + &watched[..7] + )), + "stderr: {}", + result.stderr + ); +} + +#[test] +fn a_check_re_run_github_refuses_is_a_declined_ci_fix_that_shows_the_refusal() { + let scenario = Scenario::new(); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}gh fake fails 'run rerun'\n", + checks_on_head(&format!( + "[{}]", + actions_failure("test", 900, 1, RERUN_PASSES) + )) + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_declined_ci_fix(&scenario, &result, &["900"]); + assert!( + result.stderr.contains( + "thirdshift: GitHub refused the re-run: gh run rerun 900 --failed --repo acme/widgets failed: HTTP 502: Bad Gateway" + ), + "stderr: {}", + result.stderr + ); + assert_eq!( + result.stderr.matches("CI failed on").count(), + 1, + "stderr: {}", + result.stderr + ); +} + +#[test] +fn a_check_re_run_leaves_inherited_failures_alone() { + let scenario = Scenario::new(); + let inherited = actions_failure("lint", 800, 1, RERUN_PASSES); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}{}", + checks_on_head(&format!( + "[{}, {inherited}]", + actions_failure("test", 900, 2, RERUN_PASSES) + )), + checks_on_base(&format!("[{inherited}]")) + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + // test is green after its Check re-run; lint is as red as on main. + assert_ne!(result.code, Some(0)); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), ["900"]); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + assert!( + result.stderr.contains(&format!( + "thirdshift: re-running the failed checks on {}: test\n", + &watched[..7] + )), + "stderr: {}", + result.stderr + ); + let base_commit = scenario.origin_git(&["rev-parse", "main"]); + let cause = inherited_failure("lint", &base_commit); + assert!(result.stderr.contains(&cause), "stderr: {}", result.stderr); + assert!( + !result.stderr.contains("nothing to fix on the branch"), + "stderr: {}", + result.stderr + ); +} + +#[test] +fn a_ci_fix_repair_that_commits_gets_no_check_re_run() { + let scenario = Scenario::new(); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(&format!( + "[{}]", + actions_failure("test", 900, 1, RERUN_PASSES) + )) + )); + scenario.agent_does_in_session(2, &format!("{}{}", commits_fix(1), checks_on_head(GREEN))); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), Vec::::new()); + assert!( + !result.stderr.contains("re-running"), + "stderr: {}", + result.stderr + ); +} + +#[test] +fn the_attempt_before_a_check_re_run_is_never_taken_for_its_result() { + let scenario = Scenario::new(); + // GitHub goes on listing the failed attempt for a few reads after the + // re-run is asked for, then the new attempt, running, then passed. + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(&format!( + "[{}]", + actions_failure( + "test", + 900, + 1, + r#"{"conclusion": "success", "stale_polls": 3, "pending_polls": 2}"# + ) + )) + )); + scenario.agent_does_in_session(2, "true\n"); + + // The default 300ms grace period holds only about three reads. + let result = scenario.run_with_env( + &[&scenario.issue_url(7)], + &[("THIRDSHIFT_CI_GRACE_MS", "5000")], + ); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), ["900"]); + assert_eq!( + result.stderr.matches("CI failed on").count(), + 1, + "only before the Repair; stderr: {}", + result.stderr + ); + assert!( + result.stderr.contains("1 of 1 checks still running"), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], false); +} + +#[test] +fn a_check_re_run_that_never_appears_is_a_declined_ci_fix() { + let scenario = Scenario::new(); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(&format!( + "[{}]", + actions_failure( + "test", + 900, + 1, + r#"{"conclusion": "success", "stale_polls": 1000}"# + ) + )) + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_declined_ci_fix(&scenario, &result, &["900"]); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + assert!( + result.stderr.contains(&format!( + "thirdshift: no re-run appeared on {} within 0s\n", + &watched[..7] + )), + "stderr: {}", + result.stderr + ); + assert_eq!( + result.stderr.matches("CI failed on").count(), + 1, + "only before the Repair; stderr: {}", + result.stderr + ); +} + /// Bash that pushes `file` with `content` to main from another clone, as if /// someone else's work landed while the agent was busy. fn base_moves_on(file: &str, content: &str) -> String { diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index b1d5a354d..7df7c5c06 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -11,7 +11,10 @@ //! "comments"?: {"": ["", ...]}, //! "prs": [{"number", "url", "head", "base", "state", "isDraft", "title", "body", //! "mergeable"?, "unknown_polls"?}], -//! "checks": {"": [{"name", "conclusion", "url", "pending_polls"?}]}, +//! "checks": {"": [{"id", "name", "conclusion", "url", "pending_polls"?, +//! "rerun"?: {"conclusion", "stale_polls"?, "pending_polls"?}, +//! "stale"?: {"id", "conclusion", "polls"}}]}, +//! "next_check_id"?: , //! "statuses": {"": [{"context", "state", "url"}]}, //! "runs"?: [{"databaseId", "url", "branch", "workflow", "event", "headSha", "status", //! "conclusion", "jobs"?, "hidden_polls"?, "pending_polls"?}], @@ -97,11 +100,22 @@ //! reads. A check run reports as in progress for its first `pending_polls` //! reads, then as completed with its `conclusion`. //! +//! A check run whose `url` is `…/actions/runs//job/` belongs to the +//! GitHub Actions workflow run . `gh run rerun --failed` starts a +//! new attempt of each check run of that failed, on any commit, and +//! fails, as gh does, if there is none. The new attempt has a new `id`, and +//! comes to what the check's `rerun` says: for its first `stale_polls` reads +//! GitHub still lists the attempt before it, failed as it was, then the new +//! one reports as in progress for its `pending_polls` reads, then as +//! completed with its `conclusion`. With no `rerun`, the new attempt fails as +//! the one before it did. +//! //! Tests and fake agents change that state with `gh fake …` (not a real gh //! command): //! //! ```text -//! gh fake checks '' set the check runs on +//! gh fake checks '' set the check runs on , each +//! given the next `id` if it has none //! gh fake statuses '' set the commit statuses on //! gh fake run '' record a workflow run, newer than //! any recorded before it, numbered @@ -180,7 +194,7 @@ fn save(state: &Json) { fs::write(state_path(), state.dump()).unwrap(); } -const BOOLEAN_FLAGS: [&str; 5] = ["draft", "undo", "paginate", "merge", "silent"]; +const BOOLEAN_FLAGS: [&str; 6] = ["draft", "undo", "paginate", "merge", "silent", "failed"]; /// `--flag` values by name; a boolean flag's is `None`. type Flags = BTreeMap>; @@ -977,6 +991,22 @@ fn read_checks(state: &mut Json, sha: &str) -> Vec { .get_mut("checks") .and_then(|checks| checks.get_mut(sha)); for check in checks.map(Json::items_mut).into_iter().flatten() { + let details_url = check.get("url").cloned().unwrap_or(Null); + // The attempt before a re-run, while GitHub still lists it. + if let Some(stale) = check.get_mut("stale").filter(|stale| stale.truthy()) { + let polls = stale.at("polls").as_i64().unwrap(); + if polls > 0 { + stale.set("polls", number(polls - 1)); + runs.push(object([ + ("id", stale.at("id").clone()), + ("name", check.at("name").clone()), + ("status", string("completed")), + ("conclusion", check.at("stale").at("conclusion").clone()), + ("details_url", details_url), + ])); + continue; + } + } let pending_polls = check .get("pending_polls") .and_then(Json::as_i64) @@ -986,6 +1016,7 @@ fn read_checks(state: &mut Json, sha: &str) -> Vec { check.set("pending_polls", number(pending_polls - 1)); } runs.push(object([ + ("id", check.get("id").cloned().unwrap_or(Null)), ("name", check.at("name").clone()), ( "status", @@ -999,13 +1030,95 @@ fn read_checks(state: &mut Json, sha: &str) -> Vec { check.at("conclusion").clone() }, ), - ("details_url", check.get("url").cloned().unwrap_or(Null)), + ("details_url", details_url), ])); } save(state); runs } +/// 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); + let id = next.unwrap_or(1); + state.set("next_check_id", number(id + 1)); + number(id) +} + +/// `gh fake checks `: set the check runs on `sha`, each given the +/// next `id` if it has none. +fn set_checks(state: &mut Json, sha: &str, mut checks: Json) { + for check in checks.items_mut() { + if !check.has("id") { + check.set("id", next_check_id(state)); + } + } + state.entry("checks", object([])).set(sha, checks); +} + +/// `gh run rerun --failed`: start a new attempt of each failed check +/// run of the workflow run ``, which comes to what the check's `rerun` +/// says, or fails as the attempt before it did. With no failed check run of +/// that workflow run, it fails as gh does on a run it can't re-run. +fn run_rerun(state: &mut Json, positional: &[String], flags: &Flags) { + let supported = ["failed", "repo", "R"]; + if !flags.contains_key("failed") || flags.keys().any(|name| !supported.contains(&name.as_str())) + { + die( + &format!("fake gh: unsupported run rerun flags {flags:?}"), + 2, + ); + } + let run = &positional[0]; + let of_run = format!("/actions/runs/{run}/job/"); + let mut checks = state.get("checks").cloned().unwrap_or(object([])); + let Json::Object(commits) = &mut checks else { + panic!("checks is an object") + }; + let mut rerun_any = false; + for check in commits.iter_mut().flat_map(|(_, list)| list.items_mut()) { + let conclusion = check.at("conclusion").clone(); + let failed = !["success", "skipped", "neutral"].contains(&conclusion.str()); + let url = check.get("url").and_then(Json::as_str).unwrap_or(""); + if !failed || !url.contains(&of_run) { + continue; + } + rerun_any = true; + let polls = |name: &str| { + let rerun = check.get("rerun").filter(|rerun| rerun.truthy()); + let polls = rerun + .and_then(|rerun| rerun.get(name)) + .and_then(Json::as_i64); + number(polls.unwrap_or(0)) + }; + let (stale_polls, pending_polls) = (polls("stale_polls"), polls("pending_polls")); + if let Some(rerun) = check.get("rerun").filter(|rerun| rerun.truthy()) { + let comes_to = rerun.at("conclusion").clone(); + check.set("conclusion", comes_to); + } + let before = check.at("id").clone(); + check.set( + "stale", + object([ + ("id", before), + ("conclusion", conclusion), + ("polls", stale_polls), + ]), + ); + check.set("pending_polls", pending_polls); + check.set("rerun", Null); + check.set("id", next_check_id(state)); + } + if !rerun_any { + die( + &format!("run {run} cannot be rerun; its workflow file may be broken"), + 1, + ); + } + state.set("checks", checks); + save(state); +} + /// The `-f`/`-F` fields of a `gh api` call, by name. fn api_fields(args: &[&str]) -> BTreeMap { args.windows(2) @@ -1404,8 +1517,11 @@ fn parse_json(text: &str) -> Json { fn fake_command(state: &mut Json, args: &[&str]) { let times = |text: &str| number(text.parse::().unwrap()); match args { - [kind @ ("checks" | "statuses"), sha, list] => { - state.entry(kind, object([])).set(sha, parse_json(list)); + ["checks", sha, list] => set_checks(state, sha, parse_json(list)), + ["statuses", sha, list] => { + state + .entry("statuses", object([])) + .set(sha, parse_json(list)); } ["run", run] => record_run(state, parse_json(run)), ["on-ci-read", n, script] => state.set( @@ -1519,6 +1635,10 @@ pub fn main(args: Vec) { let (_, flags) = parsed(rest); run_list(&mut state, &flags); } + ["run", "rerun", rest @ ..] => { + let (positional, flags) = parsed(rest); + run_rerun(&mut state, &positional, &flags); + } ["run", "view", rest @ ..] => { let (positional, flags) = parsed(rest); run_view(&state, &positional, &flags); diff --git a/tests/merge_run.rs b/tests/merge_run.rs index 25f8a4520..2f6addef0 100644 --- a/tests/merge_run.rs +++ b/tests/merge_run.rs @@ -371,6 +371,48 @@ fn ctrl_c_before_the_merge_is_a_failed_run() { assert_eq!(scenario.gh_state()["issues"]["7"], "OPEN"); } +#[test] +fn a_check_re_run_that_passes_after_a_ci_fix_repair_made_no_commit_lets_the_merge_run_merge() { + let scenario = Scenario::new(); + // A flaky check: it fails, the Repair commits nothing, and its Check + // re-run passes. + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head( + r#"[{"name": "test", "conclusion": "failure", + "url": "https://github.com/acme/widgets/actions/runs/900/job/1", + "rerun": {"conclusion": "success"}}]"# + ) + )); + scenario.agent_does_in_session(2, "true\n"); + + let result = scenario.run(&["merge", &scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(scenario.claude_calls().len(), 2, "1 Repair"); + assert_eq!( + scenario.gh_calls_of("run", "rerun"), + vec![vec![ + "run", + "rerun", + "900", + "--failed", + "--repo", + "acme/widgets" + ]] + ); + let head = assert_issue_7_merged_into(&scenario, "main"); + assert!( + result.stderr.contains(&format!( + "thirdshift: re-running the failed checks on {}: test\n", + &head[..7] + )), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["state"], "MERGED"); +} + /// Bash that has someone else push `file` with `content` to issue-7 from /// another clone, as a commit with `subject`: a Foreign commit. fn someone_pushes_to_issue_7(file: &str, content: &str, subject: &str) -> String { diff --git a/tests/spec_run.rs b/tests/spec_run.rs index 50cf7976b..e8a1a880c 100644 --- a/tests/spec_run.rs +++ b/tests/spec_run.rs @@ -465,6 +465,43 @@ fn red_ci_on_the_spec_prs_head_gets_a_ci_fix_repair() { ); } +#[test] +fn a_ci_fix_repair_on_the_spec_pr_that_makes_no_commit_gets_a_check_re_run() { + let scenario = linear_spec(); + scenario.agent_does_for_in_session( + SPEC, + 1, + &checks_on_head( + r#"[{"name": "test", "conclusion": "failure", + "url": "https://github.com/acme/widgets/actions/runs/900/job/1", + "rerun": {"conclusion": "success"}}]"#, + ), + ); + scenario.agent_does_for_in_session(SPEC, 2, "true\n"); + + let result = scenario.run(&[&spec_url(&scenario)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!( + spec_prompts(&scenario).len(), + 2, + "the Spec review and 1 Repair" + ); + assert_eq!( + scenario.gh_calls_of("run", "rerun"), + vec![vec![ + "run", + "rerun", + "900", + "--failed", + "--repo", + "acme/widgets" + ]] + ); + assert_contains(&result.stderr, "re-running the failed checks on "); + assert_eq!(spec_pr(&scenario)["isDraft"], false); +} + /// Bash that sets the check runs on `origin/`'s tip, as the worktree /// last fetched it, to `checks`. fn checks_on_origin(branch: &str, checks: &str) -> String { From ea6cdab563be6bb02337615b539ddb31661598d0 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 1 Oct 2026 12:57:14 +0000 Subject: [PATCH 2/5] Address review: wait out co-re-run Inherited failures, tidy names Co-Authored-By: Claude Opus 5.5 --- src/ci.rs | 54 ++++++++++++++++------------- src/main.rs | 5 +-- tests/ci.rs | 85 ++++++++++++++++++++++++++++++---------------- tests/fakes/gh.rs | 15 ++++---- tests/merge_run.rs | 4 +-- 5 files changed, 97 insertions(+), 66 deletions(-) diff --git a/src/ci.rs b/src/ci.rs index 2cbda82d5..013624f2a 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -55,9 +55,9 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result Result> { let short = short(sha); - let (jobs, others): (Vec<&Check>, Vec<&Check>) = - failed.own.iter().partition(|check| check.job.is_some()); - if !others.is_empty() { + let not_jobs = failed.own.iter().filter(|check| check.job.is_none()); + let not_jobs = check_names(not_jobs); + if !not_jobs.is_empty() { progress::step(format_args!( - "the failed checks on {short} can't be re-run: not GitHub Actions jobs: {}", - names(&others) + "the failed checks on {short} can't be re-run: not GitHub Actions jobs: {not_jobs}" )); return Ok(None); } - let jobs: Vec = jobs.iter().filter_map(|check| check.job).collect(); progress::step(format_args!( "re-running the failed checks on {short}: {}", check_names(&failed.own) )); - let mut workflow_runs: Vec = jobs.iter().map(|job| job.workflow_run).collect(); + let mut workflow_runs: Vec = jobs_of(&failed.own).map(|job| job.workflow_run).collect(); workflow_runs.sort_unstable(); workflow_runs.dedup(); - for workflow_run in workflow_runs { - if let Err(error) = github::rerun_failed_jobs(issue, workflow_run) { + for workflow_run in &workflow_runs { + if let Err(error) = github::rerun_failed_jobs(issue, *workflow_run) { progress::step(format_args!("GitHub refused the re-run: {error:#}")); return Ok(None); } } + // 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 before_rerun: Vec = jobs_of(&failed.own) + .chain(jobs_of(&failed.inherited)) + .filter(|job| workflow_runs.contains(&job.workflow_run)) + .map(|job| job.check_run) + .collect(); let grace = poll::grace_period(); - let appeared = poll::within(grace, || { + let new_attempt = poll::within(grace, || { let checks = github::checks_on(issue, sha)?; - let stale = checks - .iter() - .filter_map(|check| check.job) - .any(|job| jobs.iter().any(|failed| failed.check_run == job.check_run)); - Ok((!stale).then_some(())) + let still_listed = jobs_of(&checks).any(|job| before_rerun.contains(&job.check_run)); + Ok((!still_listed).then_some(())) })?; - if appeared.is_none() { + if new_attempt.is_none() { progress::step(format_args!( "no re-run appeared on {short} within {}s", grace.as_secs() @@ -158,13 +160,17 @@ fn watch_to_end(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Resul } /// The names of `checks`, as in "test, lint". -pub fn check_names(checks: &[Check]) -> String { - names(&checks.iter().collect::>()) +pub fn check_names<'a>(checks: impl IntoIterator) -> String { + let names: Vec<&str> = checks + .into_iter() + .map(|check| check.name.as_str()) + .collect(); + names.join(", ") } -fn names(checks: &[&Check]) -> String { - let names: Vec<&str> = checks.iter().map(|check| check.name.as_str()).collect(); - names.join(", ") +/// The GitHub Actions jobs among `checks`. +fn jobs_of(checks: &[Check]) -> impl Iterator + '_ { + checks.iter().filter_map(|check| check.job) } /// `checks` as a list, a line each: its name, and its URL if it has one. diff --git a/src/main.rs b/src/main.rs index 96cbf7ce3..3bda37af9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -74,8 +74,9 @@ share one Base fix. A failed check that is the branch's own starts a CI-fix Repair. If the Repair leaves the head commit as it was, thirdshift asks GitHub to re-run those failed checks, once per head, and -watches CI there again: a flaky check then passes and the Run goes on. If CI is red again, -or a failed check can't be re-run (only GitHub Actions jobs can), the Run fails. +watches CI there again: a check that failed on a flaky test then passes and the Run goes on. +If CI is red again, a failed check can't be re-run (only GitHub Actions jobs can), or GitHub +refuses the re-run, the Run fails. On a Spec, an issue with sub-issues, the Run is a Spec run: it takes every Ticket (sub-issue) it can reach, in the order their \"blocked by\" links allow, each merged into the Spec branch. Its Spec diff --git a/tests/ci.rs b/tests/ci.rs index c221a9066..8a9300cdc 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -348,6 +348,23 @@ fn a_ci_fix_repair_that_makes_no_commit_gets_one_check_re_run_and_green_is_succe scenario.assert_cleaned_up("issue-7"); } +/// Assert that the Run ended as a Declined CI fix with no Check re-run asked +/// for, or with only `requests`. +fn assert_declined_ci_fix(scenario: &Scenario, result: &support::RunResult, requests: &[&str]) { + assert_ne!(result.code, Some(0)); + assert_one_repair(scenario, result); + assert_eq!(rerun_requests(scenario), requests); + let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + let cause = declined_ci_fix(watched.trim()); + assert_eq!( + scenario.origin_log("issue-7").unwrap()[0], + format!("thirdshift: failed run ({cause})"), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); +} + #[test] fn a_check_re_run_that_fails_again_is_a_failed_run_with_no_second_repair_or_re_run() { let scenario = Scenario::new(); @@ -359,9 +376,7 @@ fn a_check_re_run_that_fails_again_is_a_failed_run_with_no_second_repair_or_re_r let result = scenario.run(&[&scenario.issue_url(7)]); - assert_ne!(result.code, Some(0)); - assert_one_repair(&scenario, &result); - assert_eq!(rerun_requests(&scenario), ["900"]); + assert_declined_ci_fix(&scenario, &result, &["900"]); let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); let cause = declined_ci_fix(watched.trim()); assert!(result.stderr.contains(&cause), "stderr: {}", result.stderr); @@ -372,32 +387,6 @@ fn a_check_re_run_that_fails_again_is_a_failed_run_with_no_second_repair_or_re_r result.stderr ); assert_session_log_is(&result, "repair-1"); - assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); - assert_eq!( - scenario.origin_log("issue-7"), - Some(vec![ - format!("thirdshift: failed run ({cause})"), - "Add feature".to_string(), - "Initial commit".to_string(), - ]) - ); -} - -/// Assert that the Run ended as a Declined CI fix with no Check re-run asked -/// for, or with only `requests`. -fn assert_declined_ci_fix(scenario: &Scenario, result: &support::RunResult, requests: &[&str]) { - assert_ne!(result.code, Some(0)); - assert_one_repair(scenario, result); - assert_eq!(rerun_requests(scenario), requests); - let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); - let cause = declined_ci_fix(watched.trim()); - assert_eq!( - scenario.origin_log("issue-7").unwrap()[0], - format!("thirdshift: failed run ({cause})"), - "stderr: {}", - result.stderr - ); - assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); } #[test] @@ -498,6 +487,44 @@ fn a_check_re_run_leaves_inherited_failures_alone() { ); } +#[test] +fn an_inherited_failure_re_run_along_with_the_branchs_own_is_waited_for_too() { + let scenario = Scenario::new(); + // lint shares test's workflow run, so GitHub re-runs it too, and goes on + // listing its failed attempt for a few reads after test's new one. + let inherited = actions_failure( + "lint", + 900, + 1, + r#"{"conclusion": "success", "stale_polls": 3}"#, + ); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}{}", + checks_on_head(&format!( + "[{}, {inherited}]", + actions_failure("test", 900, 2, RERUN_PASSES) + )), + checks_on_base(r#"[{"name": "lint", "conclusion": "failure"}]"#) + )); + scenario.agent_does_in_session(2, "true\n"); + + // The default 300ms grace period holds only about three reads. + let result = scenario.run_with_env( + &[&scenario.issue_url(7)], + &[("THIRDSHIFT_CI_GRACE_MS", "5000")], + ); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_one_repair(&scenario, &result); + assert_eq!(rerun_requests(&scenario), ["900"]); + assert_eq!( + result.stderr.matches("CI failed on").count(), + 1, + "only before the Repair; stderr: {}", + result.stderr + ); +} + #[test] fn a_ci_fix_repair_that_commits_gets_no_check_re_run() { let scenario = Scenario::new(); diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index 7df7c5c06..ad4c78790 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -1084,18 +1084,15 @@ fn run_rerun(state: &mut Json, positional: &[String], flags: &Flags) { continue; } rerun_any = true; + let rerun = check.get("rerun").filter(|rerun| rerun.truthy()).cloned(); let polls = |name: &str| { - let rerun = check.get("rerun").filter(|rerun| rerun.truthy()); - let polls = rerun - .and_then(|rerun| rerun.get(name)) - .and_then(Json::as_i64); - number(polls.unwrap_or(0)) + let polls = rerun.as_ref().and_then(|rerun| rerun.get(name)); + number(polls.and_then(Json::as_i64).unwrap_or(0)) }; - let (stale_polls, pending_polls) = (polls("stale_polls"), polls("pending_polls")); - if let Some(rerun) = check.get("rerun").filter(|rerun| rerun.truthy()) { - let comes_to = rerun.at("conclusion").clone(); - check.set("conclusion", comes_to); + if let Some(rerun) = &rerun { + check.set("conclusion", rerun.at("conclusion").clone()); } + let (stale_polls, pending_polls) = (polls("stale_polls"), polls("pending_polls")); let before = check.at("id").clone(); check.set( "stale", diff --git a/tests/merge_run.rs b/tests/merge_run.rs index 2f6addef0..1d568853a 100644 --- a/tests/merge_run.rs +++ b/tests/merge_run.rs @@ -374,8 +374,8 @@ fn ctrl_c_before_the_merge_is_a_failed_run() { #[test] fn a_check_re_run_that_passes_after_a_ci_fix_repair_made_no_commit_lets_the_merge_run_merge() { let scenario = Scenario::new(); - // A flaky check: it fails, the Repair commits nothing, and its Check - // re-run passes. + // A check with a flaky test: it fails, the Repair commits nothing, and + // its Check re-run passes. scenario.agent_does(&format!( "{AGENT_OPENS_PR}{}", checks_on_head( From 669c9abef54829ff1875df61700ecb55d7f0dcfc Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 1 Oct 2026 13:02:36 +0000 Subject: [PATCH 3/5] thirdshift: failed run (CI red on test-macos, test, which also fails on main at 362b9ca; fix main first) 2026-10-01T13:02:36Z, host thirdshift. Uncommitted work at the time of failure is included in this commit. From fce4227e1ad265f115febae8c33026d26ed2d18d Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 1 Oct 2026 17:37:55 +0000 Subject: [PATCH 4/5] Renumber the Check re-run ADR to 0010, after main's ADR 0009 Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- ...fix.md => 0010-one-check-re-run-before-a-declined-ci-fix.md} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/adr/{0009-one-check-re-run-before-a-declined-ci-fix.md => 0010-one-check-re-run-before-a-declined-ci-fix.md} (100%) diff --git a/README.md b/README.md index 41066f9ae..6f73403fd 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,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 Claude Code session in that worktree with the **Factory skills** in [`skills/`](skills/) loaded, 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. 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, or if GitHub refuses the re-run, which stderr shows. 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-0009](docs/adr/0009-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, or if GitHub refuses the re-run, which stderr shows. 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](docs/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). 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 `). It never uses GitHub's auto-merge ([ADR-0004](docs/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. 8. Cleans up: removes the worktree, the local Issue branch and the temporary plugin directory, whether the Run succeeded or not. The one exception is a **Failed run** whose work could not be pushed: see below. diff --git a/docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md b/docs/adr/0010-one-check-re-run-before-a-declined-ci-fix.md similarity index 100% rename from docs/adr/0009-one-check-re-run-before-a-declined-ci-fix.md rename to docs/adr/0010-one-check-re-run-before-a-declined-ci-fix.md From d075729efc8743e9b0ebfee78e88c93261fc96a2 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Thu, 1 Oct 2026 17:49:36 +0000 Subject: [PATCH 5/5] Address review: name the request that failed, Ci::has_own_failures, shared test helpers Co-Authored-By: Claude Opus 5.5 --- src/ci.rs | 18 ++++++++++++++---- src/run.rs | 7 +------ tests/ci.rs | 29 ++++++++++++++++------------- tests/fakes/gh.rs | 9 +++++++-- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/ci.rs b/src/ci.rs index 3be62e8f5..273ed3cb6 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -17,6 +17,13 @@ pub enum Ci { Failed(FailedChecks), } +impl Ci { + /// Whether it is red on a check that is the branch's own to fix. + pub fn has_own_failures(&self) -> bool { + matches!(self, Ci::Failed(failed) if !failed.own.is_empty()) + } +} + /// The checks that failed on a commit, at least one, split by whether the /// branch is the one to fix them. pub struct FailedChecks { @@ -61,7 +68,8 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result = jobs_of(&failed.own) + let previous_attempt: Vec = jobs_of(&failed.own) .chain(jobs_of(&failed.inherited)) .filter(|job| workflow_runs.contains(&job.workflow_run)) .map(|job| job.check_run) @@ -102,7 +112,7 @@ pub fn rerun( let grace = poll::grace_period(); let new_attempt = poll::within(grace, || { let checks = github::checks_on(issue, sha)?; - let still_listed = jobs_of(&checks).any(|job| before_rerun.contains(&job.check_run)); + let still_listed = jobs_of(&checks).any(|job| previous_attempt.contains(&job.check_run)); Ok((!still_listed).then_some(())) })?; if new_attempt.is_none() { diff --git a/src/run.rs b/src/run.rs index 9533f03df..334cc069b 100644 --- a/src/run.rs +++ b/src/run.rs @@ -528,7 +528,7 @@ impl RepairLoop<'_> { // A Declined CI fix, unless the head's one Check re-run turns // the branch's own checks green: it gets no second Repair. Some((_, failed)) => match ci::rerun(issue, &head, compared_with, &failed)? { - Some(ci) if !has_own_failures(&ci) => ci, + Some(ci) if !ci.has_own_failures() => ci, _ => bail!( "CI red on {} and the Repair found nothing to fix on the branch", ci::short(&head) @@ -648,11 +648,6 @@ impl RepairLoop<'_> { } } -/// Whether `ci` is red on a check that is the branch's own to fix. -fn has_own_failures(ci: &Ci) -> bool { - matches!(ci, Ci::Failed(failed) if !failed.own.is_empty()) -} - /// Mark the PR whose head is `branch` ready for review, failing unless it /// exists, is open and targets `base`. pub fn mark_pr_ready(issue: &IssueUrl, branch: &str, base: &str) -> Result { diff --git a/tests/ci.rs b/tests/ci.rs index 8a9300cdc..f9429af77 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -349,8 +349,12 @@ fn a_ci_fix_repair_that_makes_no_commit_gets_one_check_re_run_and_green_is_succe } /// Assert that the Run ended as a Declined CI fix with no Check re-run asked -/// for, or with only `requests`. -fn assert_declined_ci_fix(scenario: &Scenario, result: &support::RunResult, requests: &[&str]) { +/// for, or with only `requests`. Returns the head whose CI it watched. +fn assert_declined_ci_fix( + scenario: &Scenario, + result: &support::RunResult, + requests: &[&str], +) -> String { assert_ne!(result.code, Some(0)); assert_one_repair(scenario, result); assert_eq!(rerun_requests(scenario), requests); @@ -363,6 +367,7 @@ fn assert_declined_ci_fix(scenario: &Scenario, result: &support::RunResult, requ result.stderr ); assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], true); + watched.trim().to_string() } #[test] @@ -376,9 +381,8 @@ fn a_check_re_run_that_fails_again_is_a_failed_run_with_no_second_repair_or_re_r let result = scenario.run(&[&scenario.issue_url(7)]); - assert_declined_ci_fix(&scenario, &result, &["900"]); - let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); - let cause = declined_ci_fix(watched.trim()); + let watched = assert_declined_ci_fix(&scenario, &result, &["900"]); + let cause = declined_ci_fix(&watched); assert!(result.stderr.contains(&cause), "stderr: {}", result.stderr); assert_eq!( result.stderr.matches("CI failed on").count(), @@ -406,8 +410,7 @@ fn a_failed_commit_status_cant_be_re_run_so_nothing_is() { let result = scenario.run(&[&scenario.issue_url(7)]); - assert_declined_ci_fix(&scenario, &result, &[]); - let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + let watched = assert_declined_ci_fix(&scenario, &result, &[]); assert!( result.stderr.contains(&format!( "thirdshift: the failed checks on {} can't be re-run: not GitHub Actions jobs: deploy/preview\n", @@ -432,11 +435,12 @@ fn a_check_re_run_github_refuses_is_a_declined_ci_fix_that_shows_the_refusal() { let result = scenario.run(&[&scenario.issue_url(7)]); - assert_declined_ci_fix(&scenario, &result, &["900"]); + let watched = assert_declined_ci_fix(&scenario, &result, &["900"]); assert!( - result.stderr.contains( - "thirdshift: GitHub refused the re-run: gh run rerun 900 --failed --repo acme/widgets failed: HTTP 502: Bad Gateway" - ), + result.stderr.contains(&format!( + "thirdshift: the failed checks on {} were not re-run: gh run rerun 900 --failed --repo acme/widgets failed: HTTP 502: Bad Gateway", + &watched[..7] + )), "stderr: {}", result.stderr ); @@ -610,8 +614,7 @@ fn a_check_re_run_that_never_appears_is_a_declined_ci_fix() { let result = scenario.run(&[&scenario.issue_url(7)]); - assert_declined_ci_fix(&scenario, &result, &["900"]); - let watched = scenario.origin_git(&["rev-parse", "issue-7~1"]); + let watched = assert_declined_ci_fix(&scenario, &result, &["900"]); assert!( result.stderr.contains(&format!( "thirdshift: no re-run appeared on {} within 0s\n", diff --git a/tests/fakes/gh.rs b/tests/fakes/gh.rs index ad4c78790..c6d2673dc 100644 --- a/tests/fakes/gh.rs +++ b/tests/fakes/gh.rs @@ -1056,6 +1056,11 @@ fn set_checks(state: &mut Json, sha: &str, mut checks: Json) { state.entry("checks", object([])).set(sha, checks); } +/// Whether a check run or job that came to `conclusion` failed. +fn is_failure(conclusion: &str) -> bool { + !["success", "skipped", "neutral"].contains(&conclusion) +} + /// `gh run rerun --failed`: start a new attempt of each failed check /// run of the workflow run ``, which comes to what the check's `rerun` /// says, or fails as the attempt before it did. With no failed check run of @@ -1078,7 +1083,7 @@ fn run_rerun(state: &mut Json, positional: &[String], flags: &Flags) { let mut rerun_any = false; for check in commits.iter_mut().flat_map(|(_, list)| list.items_mut()) { let conclusion = check.at("conclusion").clone(); - let failed = !["success", "skipped", "neutral"].contains(&conclusion.str()); + let failed = is_failure(conclusion.str()); let url = check.get("url").and_then(Json::as_str).unwrap_or(""); if !failed || !url.contains(&of_run) { continue; @@ -1295,7 +1300,7 @@ fn run_view(state: &Json, positional: &[String], flags: &Flags) { let jobs = run.get("jobs").map(Json::items).unwrap_or_default(); let failed: Vec = jobs .iter() - .filter(|job| !["success", "skipped", "neutral"].contains(&job.at("conclusion").str())) + .filter(|job| is_failure(job.at("conclusion").str())) .map(|job| job.at("name").python()) .collect(); println!("{}", failed.join(", "));