[JacobStephens2]

Summary

When the Base branch moves while green or absent CI runs, a Delivery that leaves its pull request ready for review now stops on that head if GitHub reports the PR mergeable. The move is not counted against the upstream-move budget. A Self-merge (Goal::Merged) is unchanged.

 RepairLoop::rounds, on Ci::Absent | Ci::Passed
   if base_branch_moved
+    if goal is ReadyForReview
+      if pr_mergeable                    # GitHub's report, polled while Unknown
+        log "origin/main moved while CI ran; leaving the PR for review on <short head>, which merges cleanly"
+        return head                      # post-rounds readiness check still runs
+      (Unknown or not mergeable: go round as before)
     count_base_move "while CI ran"       # merge again, push, watch

• Outside::pr_mergeable is the new seam method. RunOutside backs it with PullRequest::is_mergeable.
• is_mergeable shares settled_ready_snapshot, the readiness and Unknown-polling read, with ensure_ready_and_mergeable.
• The Repair loop glossary entry, the rounds comment, the MAX_UPSTREAM_MOVES comment and the how-it-works page now say which goal goes round again on a Base move.

Evidence

• Before: tests/mergeable.rs ran a ready-for-review Delivery with green CI and main moving during CI. It expected issue-7 to gain Merge remote-tracking branch 'origin/main' into issue-7 and a second CI watch.
  After: the new code fails that expectation, because nothing further is merged. The test is now split in two:
  • a_pr_left_for_review_stays_on_green_ci_when_the_moved_base_branch_merges_cleanly asserts:
    • the new log line with the real short head;
    • no merging it again;
    • issue-7 history unchanged (Add feature, Initial commit).
  • a_merge_run_merges_a_base_branch_that_moved_while_ci_passed keeps the old behaviour for a Merge run.
• Unit tests in src/delivery/repair_loop.rs:
  • ready for review, green or absent CI, Base moved, PR mergeable: the sequence is MergeBase, Push, Watch, ReadMergeable, <log line>, CheckPr, with no further merge, push or watch;
  • the same, but the PR is not mergeable: it logs merging it again, merges again, runs a conflict Repair and goes round;
  • a Merge run with the Base moved: round again, then the merge, as before;
  • budget exhaustion after unmergeable reads stops in a RepairLoopRound, not in PullRequestReadiness.
• pull_request::tests::only_a_pr_github_reports_mergeable_is_mergeable_and_only_an_unready_one_fails: Yes → true, No → false, Unknown → false, draft → error.
• Four existing Check re-run tests now script the PR as not mergeable after their green re-run, so they still exercise going round on a Base move.
• cargo fmt --check and cargo clippy --all-targets -- -D warnings are clean. The full cargo test run on a630b86 exited 0. The last commit, 97a5056, was checked with cargo test --bin thirdshift pull_request (36 passed) and clippy.

Merge Danger

Door: two-way

A revert restores the old loop. No state or data is migrated.

Blast Radius: Delivery

This affects ready-for-review Runs and Spec runs. They now finish on a head that may not include the latest main, relying on GitHub's mergeability report. The post-rounds ensure_pr_ready_and_mergeable check is unchanged. Self-merge is not touched.

Unaddressed findings

Standards

• Repeated Switches: the arm branches on Goal::ReadyForReview twice. Declined. The two checks answer different questions: a moved Base branch, and Foreign commits (the else if in RepairLoop::rounds, src/delivery/repair_loop.rs). One match self.goal would restructure the Merge run path, and the spec says "Goal::Merged (Self-merge) is unchanged by this issue."
• Two Outside methods and two GitHub reads for one fact (pr_mergeable and ensure_pr_ready_and_mergeable). Declined. The spec requires "The existing readiness and mergeability check after the rounds stays." Folding them together would move that gate into the loop.
• The Merge run integration test checks main's log with .contains, which is weaker than the old parent check. Declined. The Issue branch is deleted after the Self-merge, so assert_merged_main_into_issue_7 cannot run (it failed with unknown revision issue-7). The test also asserts the merging it again line, and the reviewer judged this acceptable.
• No integration test drives the ready-for-review, not-mergeable path. Declined. a_run_left_ready_for_review_goes_round_again_as_a_base_move_if_the_pr_does_not_merge covers it in-process, as the acceptance criteria ask ("An in-process Repair loop test").
• --short=7 couples the test to ci::short's length. No change. The reviewer raised no defect, and a mismatch fails loudly.
• RunOutside::pr_mergeable is a Middle Man. No change. The reviewer judged it acceptable, matching the neighbouring adapters.

Spec

• "no upstream move counted" is proven only indirectly. Declined. The proof is the absence of count_base_move's line: RepairLoop::count_base_move writes the Budgets::count_base_move line every time it counts. The stage test also shows the counting path exhausting the budget only on unmergeable reads.
• Unknown mergeability costs up to one grace period per check. No change. Treating Unknown as not mergeable keeps today's behaviour, per "provided the pull request is mergeable … as GitHub reports it."
• GitHub may briefly report a stale MERGEABLE. No change. The spec accepts "as GitHub reports it", and the post-rounds check re-reads it.
• The glossary's "a Merge run goes round again" leaves out a Self-merging Spec run. No change. The Repair loop entry already uses "in a Merge run" this way, so this is not new.

All of the following were addressed: the Unknown-mergeability wording in the glossary and the rounds comment, the test name, the (Mergeable, String) tuple, the wildcard Mergeable arm, and the how-it-works link text.

Security review outcome

No unaddressed introduced findings.

Review coverage

Both reviewers read every changed file at a630b86. After the reviews, 97a5056 changed two lines they did not see: the explicit Mergeable::Yes | Mergeable::No | Mergeable::Unknown arm in src/pull_request.rs (a Standards suggestion) and the link text in site/how-it-works/index.html. No generated output was excluded.

Closes #906

🤖 Generated with Claude Code

Built with claude · claude-opus-5-5 · medium