From 12360b6180c8a54e16cfedba266621a8fbd109de Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Wed, 21 Oct 2026 03:55:01 -0400 Subject: [PATCH 1/3] Leave a ready-for-review PR on green CI when the moved Base branch merges cleanly A Delivery left ready for review no longer merges a moved Base branch in again after green or absent CI, unless GitHub reports the PR not mergeable. The move is not counted against the upstream-move budget. Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 2 +- site/how-it-works/index.html | 2 +- src/delivery.rs | 4 + src/delivery/repair_loop.rs | 156 +++++++++++++++++++++++++++++++++-- src/pull_request.rs | 58 +++++++++++-- 5 files changed, 204 insertions(+), 18 deletions(-) diff --git a/GLOSSARY.md b/GLOSSARY.md index 37a1c3c19..b11ae0aaa 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -218,7 +218,7 @@ What a **Run**, or a **Spec run** for its **Spec PR**, does from its worktree on If a Delivery gate observes that its pull request has already merged into the expected Base branch, and its merged head includes all local work (the exact local head, with no uncommitted changes), the Run ends as merged, even when it originally asked only for ready for review. Delivery stops further publication, PR edits and Self-merge work. A merged PR that leaves local work out still fails and retains that work; a merged state alone never establishes completion. **Repair loop**: -The rounds a **Delivery** goes through until the pull request's head is mergeable with the **Base branch** and its CI is green or absent. Each round merges the Base branch in (and, in a **Merge run**, any **Foreign commits**), pushes, watches CI, and starts a **Repair** for a conflict or for the branch's own red checks. Repairs and upstream moves have fixed budgets. +The rounds a **Delivery** goes through until the pull request's head is mergeable with the **Base branch** and its CI is green or absent. Each round merges the Base branch in (and, in a **Merge run**, any **Foreign commits**), pushes, watches CI, and starts a **Repair** for a conflict or for the branch's own red checks. When the Base branch moved while green or absent CI ran, a Merge run goes round again to merge it in. A Delivery that leaves the pull request ready for review goes round again only if GitHub reports the pull request not mergeable; otherwise it stops on that head, and the move is not counted. Repairs and upstream moves have fixed budgets. **Foreign commit**: A commit that appears on the **Issue branch** during a **Run** and was made neither by one of that Run's sessions nor by thirdshift itself. Commits already on the branch when a **Continuation** starts are not Foreign commits. A **Merge run** merges one only after a **Repair** has reviewed its resulting tree, unless that tree is identical to the Run's own head before reconciliation and is already covered by its existing review. A tree-identical batch still counts as an upstream move, but uses no Review Repair. diff --git a/site/how-it-works/index.html b/site/how-it-works/index.html index a384f2d50..627c47851 100644 --- a/site/how-it-works/index.html +++ b/site/how-it-works/index.html @@ -126,7 +126,7 @@

How thirdshift works

Conflict or the branch's own CI failure?Start a Repair to resolve or fix it.Then return to merging the Base branch ↑
-
Base branch moved during CI?Reconcile the new upstream state.Return to merging the Base branch ↑
+
Base branch moved during CI?A Self-merge reconciles the new upstream state. A pull request left for review stops on green CI if it still merges cleanly.Return to merging the Base branch ↑

Repair and upstream-move budgets keep the loop bounded. If a CI-fix Repair leaves the same failing head, that head gets one Check re-run; if its own checks stay red, the Run fails rather than spending more Repairs on unchanged code.

diff --git a/src/delivery.rs b/src/delivery.rs index 2cbbe4f20..b4dda75b5 100644 --- a/src/delivery.rs +++ b/src/delivery.rs @@ -743,6 +743,10 @@ impl repair_loop::Outside for RunOutside<'_> { .fix(outside, self.pr_url, self.base, base_commit, failed) } + fn pr_mergeable(&mut self) -> Result { + self.pull_request.is_mergeable() + } + fn ensure_pr_ready_and_mergeable(&mut self) -> Result<()> { self.pull_request.ensure_ready_and_mergeable() } diff --git a/src/delivery/repair_loop.rs b/src/delivery/repair_loop.rs index dc4e4cf4c..168038c99 100644 --- a/src/delivery/repair_loop.rs +++ b/src/delivery/repair_loop.rs @@ -92,6 +92,9 @@ pub(super) trait Outside { /// go round again. fn base_fix(&mut self, base_commit: &str, failed: &FailedChecks) -> Result<()>; + /// Whether GitHub reports the open, ready pull request mergeable with the + /// Base branch. Mergeability it has not worked out in time is not. + fn pr_mergeable(&mut self) -> Result; /// Fail unless the pull request is open, ready for review and mergeable. fn ensure_pr_ready_and_mergeable(&mut self) -> Result<()>; /// Merge the pull request at `head`. @@ -162,7 +165,9 @@ const MAX_REPAIRS: usize = 5; /// CI ran or since a merge was tried, or, in a Merge run, because Foreign /// commits arrived, whether or not the merge that follows needs a Repair. A /// clean merge of the Base branch uses no Repair, so without this a busy Base -/// branch could keep a Delivery going forever. +/// branch could keep a Delivery going forever. A Base branch that moved while +/// green or absent CI ran is not counted for a Delivery left ready for review +/// whose pull request still merges cleanly, since that Delivery stops there. const MAX_UPSTREAM_MOVES: usize = 5; /// What a Delivery has spent of its Repair and upstream-move budgets, @@ -288,6 +293,13 @@ impl RepairLoop<'_, O> { self.outside.repair(kind, repair) } + /// Whether GitHub reports the pull request mergeable, read in the + /// readiness stage. + fn pr_merges_cleanly(&mut self) -> Result { + self.outside.stage(Stage::PullRequestReadiness); + self.outside.pr_mergeable() + } + /// Count a round taken because the Base branch moved `when`. fn count_base_move(&mut self, when: &str) -> Result<()> { let line = self.budgets.count_base_move(self.base, when)?; @@ -299,8 +311,11 @@ impl RepairLoop<'_, O> { /// Base branch (never rebase), push, and watch CI on the head commit, /// starting a Repair session for a conflict or red /// CI and then going round again, since the Base branch may have moved - /// meanwhile. Green or absent CI also goes round again if the Base branch - /// moved while CI ran, or, in a Merge run, if Foreign commits arrived. + /// meanwhile. Green or absent CI also goes round again, in a Merge run, if + /// the Base branch moved while CI ran or Foreign commits arrived. Left + /// ready for review, it goes round again for a Base branch that moved only + /// if GitHub reports the pull request not mergeable, and otherwise + /// returns that head without counting the move. /// A red check that also fails on the Base branch commit the head last /// merged in is an Inherited failure, which no Repair is started for: the /// CI-fix Repair is given the branch's own failures, and when there are @@ -348,6 +363,20 @@ impl RepairLoop<'_, O> { match ci { Ci::Absent | Ci::Passed => { if self.outside.base_branch_moved()? { + // Green CI on a head that merges cleanly is enough + // for review; only a Self-merge needs the moved Base + // branch merged in and watched. + if self.goal == Goal::ReadyForReview { + if self.pr_merges_cleanly()? { + self.outside.progress(format!( + "origin/{base} moved while CI ran; leaving the PR for \ + review on {}, which merges cleanly", + ci::short(&head) + )); + return Ok(head); + } + self.outside.stage(Stage::RepairLoopRound); + } self.count_base_move("while CI ran")?; } else if self.goal == Goal::ReadyForReview || self.outside.new_commits_on_origin()?.commits().is_empty() @@ -463,6 +492,8 @@ mod tests { /// Took the Base fix for these Inherited failures, at this Base /// branch commit. BaseFix(String, Vec), + /// Read whether GitHub reports the PR mergeable. + ReadMergeable, /// Checked the PR is open, ready and mergeable. CheckPr, /// Merged the PR at this head. @@ -519,8 +550,8 @@ mod tests { /// What the outside world answers, call by call. A head or Base branch /// commit stays the last one given; a merge is clean, the Base branch - /// unmoved, origin without new commits and a Base fix returns, unless - /// scripted otherwise. + /// unmoved, the PR mergeable, origin without new commits and a Base fix + /// returns, unless scripted otherwise. #[derive(Default)] struct Script { heads: VecDeque<&'static str>, @@ -532,6 +563,9 @@ mod tests { tree_matches: VecDeque>, merge_completion_error: Option<&'static str>, base_moved: VecDeque, + /// For each read of the PR's mergeability, whether GitHub reports it + /// mergeable. + mergeable: VecDeque, new_commits: VecDeque, watches: VecDeque, reruns: VecDeque>, @@ -709,6 +743,11 @@ mod tests { } } + fn pr_mergeable(&mut self) -> Result { + self.did.push(Did::ReadMergeable); + Ok(self.script.mergeable.pop_front().unwrap_or(true)) + } + fn ensure_pr_ready_and_mergeable(&mut self) -> Result<()> { self.did.push(Did::CheckPr); Ok(()) @@ -1045,6 +1084,8 @@ mod tests { heads: script(["h1"]), base_commits: script(["b1", "b2", "b3"]), base_moved: script([true]), + // The PR conflicts with the moved Base branch. + mergeable: script([false]), watches: script([Is::Failed(&["test"], &[]), Is::Failed(&["test"], &[])]), reruns: script([Some(Is::Passed)]), ..Script::default() @@ -1075,6 +1116,8 @@ mod tests { heads: script(["h1", "h1", "h2", "h1"]), base_commits: script(["b1", "b2", "b3", "b4"]), base_moved: script([true, false]), + // The PR conflicts with the moved Base branch. + mergeable: script([false]), watches: script([ Is::Failed(&["test"], &[]), Is::Failed(&[], &["lint"]), @@ -1112,6 +1155,8 @@ mod tests { heads: script([first, first, second, second]), base_commits: script(["b1", "b2", "b3", "b4"]), base_moved: script([true, false]), + // The PR conflicts with the moved Base branch. + mergeable: script([false]), watches: script([Is::Failed(&["test"], &[]), Is::Failed(&["lint"], &[])]), reruns: script([Some(Is::Passed), Some(Is::Passed)]), ..Script::default() @@ -1172,6 +1217,8 @@ mod tests { heads: script(["h1"]), base_commits: script(["b1", "b2", "rewound-base", "b3"]), base_moved: script([true, false]), + // The PR conflicts with the moved Base branch. + mergeable: script([false]), watches: script([ Is::Failed(&["test"], &[]), Is::Failed(&[], &["lint"]), @@ -1411,13 +1458,14 @@ mod tests { } #[test] - fn the_base_branch_moving_while_ci_ran_goes_round_again_as_a_base_move() { + fn in_a_merge_run_the_base_branch_moving_while_ci_ran_goes_round_again_as_a_base_move() { let (delivered, did) = deliver( - Goal::ReadyForReview, + Goal::Merged, Script { heads: script(["h1", "h2"]), base_moved: script([true]), watches: script([Is::Passed, Is::Passed]), + merges: script([None]), ..Script::default() }, ); @@ -1429,6 +1477,72 @@ mod tests { &round("h1")[..], &[line("origin/main moved while CI ran; merging it again")], &round("h2"), + &merge("h2"), + ] + .concat() + ); + } + + #[test] + fn a_run_left_ready_for_review_ends_on_green_ci_when_the_moved_base_branch_merges_cleanly() { + for is in [Is::Passed, Is::Absent] { + let (delivered, did) = deliver( + Goal::ReadyForReview, + Script { + heads: script(["h1234567890", "h2"]), + base_moved: script([true]), + watches: script([is]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [ + &[Did::MergeBase, Did::Push, watch("h1234567890")][..], + &[ + Did::ReadMergeable, + line( + "origin/main moved while CI ran; leaving the PR for review on \ + h123456, which merges cleanly" + ), + Did::CheckPr, + ], + ] + .concat() + ); + } + } + + #[test] + fn a_run_left_ready_for_review_goes_round_again_as_a_base_move_if_the_pr_does_not_merge() { + let (delivered, did) = deliver( + Goal::ReadyForReview, + Script { + heads: script(["h1", "h2"]), + base_moved: script([true]), + base_conflicts: script([false, true]), + mergeable: script([false]), + watches: script([Is::Passed, Is::Passed]), + ..Script::default() + }, + ); + + delivered.unwrap(); + assert_eq!( + did, + [ + &round("h1")[..], + &[ + Did::ReadMergeable, + line("origin/main moved while CI ran; merging it again"), + Did::MergeBase, + line("conflict; starting Repair 1 of 5"), + repair(1, Started::Conflict(Upstream::BaseBranch)), + Did::CheckMerged("origin/main"), + ], + &round("h2"), &[Did::CheckPr], ] .concat() @@ -2121,6 +2235,34 @@ mod tests { ); } + #[test] + fn a_run_left_ready_for_review_reads_mergeability_in_readiness_and_counts_a_conflict_in_a_round() + { + let (delivered, _, stages) = deliver_with_stages( + Goal::ReadyForReview, + Script { + heads: script(["h1"]), + base_moved: script([true; MAX_UPSTREAM_MOVES + 1]), + mergeable: script([false; MAX_UPSTREAM_MOVES + 1]), + watches: script((0..=MAX_UPSTREAM_MOVES).map(|_| Is::Passed)), + ..Script::default() + }, + ); + + assert_eq!( + cause(delivered), + "origin/main kept moving: merged it again 5 times" + ); + assert_eq!( + stages[stages.len() - 3..], + [ + Stage::CiWatch, + Stage::PullRequestReadiness, + Stage::RepairLoopRound + ] + ); + } + #[test] fn a_failed_ci_watch_stops_in_the_ci_watch() { assert_eq!( diff --git a/src/pull_request.rs b/src/pull_request.rs index e860b39be..cd046dc0e 100644 --- a/src/pull_request.rs +++ b/src/pull_request.rs @@ -283,21 +283,33 @@ impl PullRequest { /// coherent numbered observation. Waiting remains interruptible. pub fn ensure_ready_and_mergeable(&mut self) -> Result<()> { progress::step("checking the PR is open, ready and mergeable"); + let (mergeable, url) = self.ready_mergeability()?; + match mergeable { + Mergeable::Yes => Ok(()), + Mergeable::No => bail!("PR {url} is not mergeable"), + Mergeable::Unknown => { + bail!("GitHub has not worked out whether PR {url} is mergeable") + } + } + } + + /// Whether GitHub reports this open, ready PR mergeable, as + /// [`Self::ensure_ready_and_mergeable`] polls it. Mergeability GitHub has + /// not worked out within the grace period is not mergeable. + pub fn is_mergeable(&mut self) -> Result { + Ok(self.ready_mergeability()?.0 == Mergeable::Yes) + } + + /// The mergeability GitHub reports for this open, ready PR, and its URL, + /// polled while unknown until the grace period ends. + fn ready_mergeability(&mut self) -> Result<(Mergeable, String)> { let deadline = std::time::Instant::now() + self.adapter.grace_period(); let mut waiting_line = poll::WaitingLine::new(); loop { let snapshot = self.observe(false)?.context("no PR found")?; self.validate(&snapshot, true)?; match snapshot.mergeable { - Mergeable::Yes => return Ok(()), - Mergeable::No => bail!("PR {} is not mergeable", snapshot.pr.url), - Mergeable::Unknown if std::time::Instant::now() >= deadline => { - bail!( - "GitHub has not worked out whether PR {} is mergeable", - snapshot.pr.url - ); - } - Mergeable::Unknown => { + Mergeable::Unknown if std::time::Instant::now() < deadline => { if waiting_line.should_report(()) { progress::step(format!( "waiting for GitHub to calculate mergeability of PR {}", @@ -306,6 +318,7 @@ impl PullRequest { } self.adapter.pause()?; } + mergeable => return Ok((mergeable, snapshot.pr.url)), } } } @@ -1433,6 +1446,33 @@ mod tests { } } + #[test] + fn only_a_pr_github_reports_mergeable_is_mergeable_and_only_an_unready_one_fails() { + let mut conflicting = snapshot(); + conflicting.mergeable = Mergeable::No; + let mut unknown = snapshot(); + unknown.mergeable = Mergeable::Unknown; + let mut draft = snapshot(); + draft.pr.is_draft = true; + for (answer, mergeable) in [ + (snapshot(), Some(true)), + (conflicting, Some(false)), + (unknown, Some(false)), + (draft, None), + ] { + let (mut pr, _) = owner(World { + observations: VecDeque::from([ + Ok(Some(snapshot())), + Ok(Some(snapshot())), + Ok(Some(answer)), + ]), + ..World::default() + }); + pr.mark_ready(None).unwrap(); + assert_eq!(pr.is_mergeable().ok(), mergeable); + } + } + #[test] fn disappearance_or_transport_failure_after_capture_never_rediscovers_by_branch() { for missing in [Ok(None), Err("gone")] { From a630b86fad72aa2fdd91e2bcfcb3c419e5e88da3 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Wed, 21 Oct 2026 04:11:46 -0400 Subject: [PATCH 2/3] Address review: say Unknown mergeability goes round, name the PR snapshot, cover the ready-for-review path end to end Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 2 +- src/delivery/repair_loop.rs | 8 ++--- src/pull_request.rs | 15 +++++----- tests/mergeable.rs | 60 ++++++++++++++++++++++++++++++------- 4 files changed, 63 insertions(+), 22 deletions(-) diff --git a/GLOSSARY.md b/GLOSSARY.md index b11ae0aaa..96dc7a3e6 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -218,7 +218,7 @@ What a **Run**, or a **Spec run** for its **Spec PR**, does from its worktree on If a Delivery gate observes that its pull request has already merged into the expected Base branch, and its merged head includes all local work (the exact local head, with no uncommitted changes), the Run ends as merged, even when it originally asked only for ready for review. Delivery stops further publication, PR edits and Self-merge work. A merged PR that leaves local work out still fails and retains that work; a merged state alone never establishes completion. **Repair loop**: -The rounds a **Delivery** goes through until the pull request's head is mergeable with the **Base branch** and its CI is green or absent. Each round merges the Base branch in (and, in a **Merge run**, any **Foreign commits**), pushes, watches CI, and starts a **Repair** for a conflict or for the branch's own red checks. When the Base branch moved while green or absent CI ran, a Merge run goes round again to merge it in. A Delivery that leaves the pull request ready for review goes round again only if GitHub reports the pull request not mergeable; otherwise it stops on that head, and the move is not counted. Repairs and upstream moves have fixed budgets. +The rounds a **Delivery** goes through until the pull request's head is mergeable with the **Base branch** and its CI is green or absent. Each round merges the Base branch in (and, in a **Merge run**, any **Foreign commits**), pushes, watches CI, and starts a **Repair** for a conflict or for the branch's own red checks. When the Base branch moved while green or absent CI ran, a Merge run goes round again to merge it in. A Delivery that leaves the pull request ready for review stops on that head if GitHub reports the pull request mergeable, and the move is not counted; otherwise it goes round again. Repairs and upstream moves have fixed budgets. **Foreign commit**: A commit that appears on the **Issue branch** during a **Run** and was made neither by one of that Run's sessions nor by thirdshift itself. Commits already on the branch when a **Continuation** starts are not Foreign commits. A **Merge run** merges one only after a **Repair** has reviewed its resulting tree, unless that tree is identical to the Run's own head before reconciliation and is already covered by its existing review. A tree-identical batch still counts as an upstream move, but uses no Review Repair. diff --git a/src/delivery/repair_loop.rs b/src/delivery/repair_loop.rs index 168038c99..11adc0f3c 100644 --- a/src/delivery/repair_loop.rs +++ b/src/delivery/repair_loop.rs @@ -313,9 +313,9 @@ impl RepairLoop<'_, O> { /// CI and then going round again, since the Base branch may have moved /// meanwhile. Green or absent CI also goes round again, in a Merge run, if /// the Base branch moved while CI ran or Foreign commits arrived. Left - /// ready for review, it goes round again for a Base branch that moved only - /// if GitHub reports the pull request not mergeable, and otherwise - /// returns that head without counting the move. + /// ready for review, it returns that head without counting the move if + /// GitHub reports the pull request mergeable, and otherwise goes round + /// again. /// A red check that also fails on the Base branch commit the head last /// merged in is an Inherited failure, which no Repair is started for: the /// CI-fix Repair is given the branch's own failures, and when there are @@ -2236,7 +2236,7 @@ mod tests { } #[test] - fn a_run_left_ready_for_review_reads_mergeability_in_readiness_and_counts_a_conflict_in_a_round() + fn a_run_left_ready_for_review_reads_mergeability_in_readiness_and_counts_an_unmergeable_pr_in_a_round() { let (delivered, _, stages) = deliver_with_stages( Goal::ReadyForReview, diff --git a/src/pull_request.rs b/src/pull_request.rs index cd046dc0e..c9ed6b727 100644 --- a/src/pull_request.rs +++ b/src/pull_request.rs @@ -283,8 +283,9 @@ impl PullRequest { /// coherent numbered observation. Waiting remains interruptible. pub fn ensure_ready_and_mergeable(&mut self) -> Result<()> { progress::step("checking the PR is open, ready and mergeable"); - let (mergeable, url) = self.ready_mergeability()?; - match mergeable { + let snapshot = self.settled_ready_snapshot()?; + let url = &snapshot.pr.url; + match snapshot.mergeable { Mergeable::Yes => Ok(()), Mergeable::No => bail!("PR {url} is not mergeable"), Mergeable::Unknown => { @@ -297,12 +298,12 @@ impl PullRequest { /// [`Self::ensure_ready_and_mergeable`] polls it. Mergeability GitHub has /// not worked out within the grace period is not mergeable. pub fn is_mergeable(&mut self) -> Result { - Ok(self.ready_mergeability()?.0 == Mergeable::Yes) + Ok(self.settled_ready_snapshot()?.mergeable == Mergeable::Yes) } - /// The mergeability GitHub reports for this open, ready PR, and its URL, - /// polled while unknown until the grace period ends. - fn ready_mergeability(&mut self) -> Result<(Mergeable, String)> { + /// An observation of this open, ready PR, polled while its mergeability + /// is unknown until the grace period ends. + fn settled_ready_snapshot(&mut self) -> Result { let deadline = std::time::Instant::now() + self.adapter.grace_period(); let mut waiting_line = poll::WaitingLine::new(); loop { @@ -318,7 +319,7 @@ impl PullRequest { } self.adapter.pause()?; } - mergeable => return Ok((mergeable, snapshot.pr.url)), + _ => return Ok(snapshot), } } } diff --git a/tests/mergeable.rs b/tests/mergeable.rs index f47ddaf36..24d6812b6 100644 --- a/tests/mergeable.rs +++ b/tests/mergeable.rs @@ -1,7 +1,8 @@ //! Keeping the PR mergeable: once the PR is confirmed, thirdshift merges the //! Base branch into the Issue branch and pushes, handing a conflicting merge to -//! a conflict Repair session first. If the Base branch moves while CI runs, it -//! merges it again, at most five times. +//! a conflict Repair session first. If the Base branch moves while CI runs, a +//! Merge run merges it again, at most five times; a PR left ready for review +//! that still merges cleanly is left on the head whose CI passed. mod support; @@ -253,25 +254,64 @@ fn base_moves_during_ci(times: usize, file: &str, content: &str) -> String { ) } -#[test] -fn merges_a_base_branch_that_moved_while_ci_passed() { - let scenario = Scenario::new(); - // Green CI on the agent's commit; the merge commit after it has none. - scenario.agent_does(&format!( +/// The agent opens the PR with green CI on its commit, and main moves while +/// that CI is read. +fn green_ci_while_the_base_branch_moves() -> String { + format!( "{AGENT_OPENS_PR}\ gh fake checks \"$(git rev-parse HEAD)\" '[{{\"name\": \"test\", \"conclusion\": \"success\"}}]'\n\ {}", base_moves_during_ci(1, "other.txt", "other") - )); + ) +} + +#[test] +fn a_merge_run_merges_a_base_branch_that_moved_while_ci_passed() { + let scenario = Scenario::new(); + // Green CI on the agent's commit; the merge commit after it has none. + scenario.agent_does(&green_ci_while_the_base_branch_moves()); + + let result = scenario.run(&[&scenario.issue_url(7), "merge"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert!( + result.stderr.contains("CI passed on") + && result + .stderr + .contains("origin/main moved while CI ran; merging it again"), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.claude_calls().len(), 1); + // The Issue branch is gone once merged; main has the merge of main in. + assert!( + scenario + .origin_git(&["log", "--format=%s", "main"]) + .contains("Merge remote-tracking branch 'origin/main' into issue-7\n") + ); +} + +#[test] +fn a_pr_left_for_review_stays_on_green_ci_when_the_moved_base_branch_merges_cleanly() { + let scenario = Scenario::new(); + scenario.agent_does(&green_ci_while_the_base_branch_moves()); let result = scenario.run(&[&scenario.issue_url(7)]); assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + let head = scenario.origin_git(&["rev-parse", "--short=7", "issue-7"]); assert!( - result.stderr.contains("CI passed on"), + result.stderr.contains(&format!( + "origin/main moved while CI ran; leaving the PR for review on {}, which merges cleanly", + head.trim() + )), "stderr: {}", result.stderr ); + assert!(!result.stderr.contains("merging it again")); assert_eq!(scenario.claude_calls().len(), 1); - assert_merged_main_into_issue_7(&scenario); + assert_eq!( + scenario.origin_git(&["log", "--first-parent", "--format=%s", "issue-7"]), + "Add feature\nInitial commit\n" + ); } From 97a5056fec60e3fd53c57190b46e84e6acc4ce8e Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Wed, 21 Oct 2026 04:51:52 -0400 Subject: [PATCH 3/3] Name every mergeability state in the settled snapshot, say where the base-move branch returns Co-Authored-By: Claude Opus 5.5 --- site/how-it-works/index.html | 2 +- src/pull_request.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/site/how-it-works/index.html b/site/how-it-works/index.html index 627c47851..2a1998562 100644 --- a/site/how-it-works/index.html +++ b/site/how-it-works/index.html @@ -126,7 +126,7 @@

How thirdshift works

Conflict or the branch's own CI failure?Start a Repair to resolve or fix it.Then return to merging the Base branch ↑
-
Base branch moved during CI?A Self-merge reconciles the new upstream state. A pull request left for review stops on green CI if it still merges cleanly.Return to merging the Base branch ↑
+
Base branch moved during CI?A Self-merge reconciles the new upstream state. A pull request left for review stops on green CI if it still merges cleanly.Otherwise return to merging the Base branch ↑

Repair and upstream-move budgets keep the loop bounded. If a CI-fix Repair leaves the same failing head, that head gets one Check re-run; if its own checks stay red, the Run fails rather than spending more Repairs on unchanged code.

diff --git a/src/pull_request.rs b/src/pull_request.rs index c9ed6b727..752282dae 100644 --- a/src/pull_request.rs +++ b/src/pull_request.rs @@ -319,7 +319,7 @@ impl PullRequest { } self.adapter.pause()?; } - _ => return Ok(snapshot), + Mergeable::Yes | Mergeable::No | Mergeable::Unknown => return Ok(snapshot), } } }