diff --git a/CONTEXT.md b/CONTEXT.md index 793d41780..334d63d9e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -127,7 +127,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. 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. diff --git a/README.md b/README.md index e5f9aeb1c..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, 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-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. @@ -345,7 +345,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: @@ -377,7 +377,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/0010-one-check-re-run-before-a-declined-ci-fix.md b/docs/adr/0010-one-check-re-run-before-a-declined-ci-fix.md new file mode 100644 index 000000000..31bf0af6c --- /dev/null +++ b/docs/adr/0010-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 abbf171ae..273ed3cb6 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; @@ -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 { @@ -49,7 +56,78 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result, + failed: &FailedChecks, +) -> Result> { + let short = short(sha); + 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: {not_jobs}" + )); + return Ok(None); + } + progress::step(format_args!( + "re-running the failed checks on {short}: {}", + check_names(&failed.own) + )); + 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) { + progress::step(format_args!( + "the failed checks on {short} were not 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 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) + .collect(); + 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| previous_attempt.contains(&job.check_run)); + Ok((!still_listed).then_some(())) + })?; + if new_attempt.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)?; @@ -92,11 +170,19 @@ pub fn watch(issue: &IssueUrl, sha: &str, base_commit: Option<&str>) -> Result String { - let names: Vec<&str> = checks.iter().map(|check| check.name.as_str()).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(", ") } +/// 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. pub fn check_list(checks: &[Check]) -> String { checks diff --git a/src/github.rs b/src/github.rs index 5436d3f56..e8a57dd9f 100644 --- a/src/github.rs +++ b/src/github.rs @@ -539,9 +539,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")?; @@ -566,6 +593,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"])), }); } @@ -582,11 +610,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) @@ -645,3 +687,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 1fe55330a..5ca6c450f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -74,6 +74,12 @@ fails, naming the Base fix issue. A Run that finds an open base-fix issue for th branch and checks waits for that one to close 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 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 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 534e155d9..334cc069b 100644 --- a/src/run.rs +++ b/src/run.rs @@ -8,7 +8,7 @@ use anyhow::{Context, Result, anyhow, bail}; use crate::base_fix::{Advice, 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}; @@ -489,16 +489,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)?; @@ -511,18 +515,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 !ci.has_own_failures() => 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")?; @@ -561,7 +574,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)); } } } diff --git a/tests/ci.rs b/tests/ci.rs index 358fbc9dd..f9429af77 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,379 @@ 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"); +} + +/// Assert that the Run ended as a Declined CI fix with no Check re-run asked +/// 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); + 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); + watched.trim().to_string() +} + +#[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)]); + + 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(), + 2, + "before the Repair and after the Check re-run; stderr: {}", + result.stderr + ); + assert_session_log_is(&result, "repair-1"); +} + +#[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)]); + + 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", + &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)]); + + let watched = assert_declined_ci_fix(&scenario, &result, &["900"]); + assert!( + 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 + ); + 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 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(); + 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)]); + + let watched = assert_declined_ci_fix(&scenario, &result, &["900"]); + 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..c6d2673dc 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,97 @@ 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); +} + +/// 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 +/// 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 = is_failure(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 rerun = check.get("rerun").filter(|rerun| rerun.truthy()).cloned(); + let polls = |name: &str| { + let polls = rerun.as_ref().and_then(|rerun| rerun.get(name)); + number(polls.and_then(Json::as_i64).unwrap_or(0)) + }; + 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", + 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) @@ -1185,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(", ")); @@ -1404,8 +1519,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 +1637,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..1d568853a 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 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( + 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 4832b4a19..0bd489ca2 100644 --- a/tests/spec_run.rs +++ b/tests/spec_run.rs @@ -567,6 +567,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 {