[JacobStephens2]

Summary

The Repair loop moves into its own private submodule of the Delivery module. It reaches the outside only through one internal seam, Outside. Its rules are now tested in-process against a scripted adapter.

 src/
-├── delivery.rs              # Delivery + Repair loop wired to git, gh, claude
+├── delivery.rs              # Delivery, Self-merge after-steps, production adapter RunOutside
+└── delivery/
+    └── repair_loop.rs       # rounds, Foreign commits, merge-and-retry, budgets, seam, unit tests

Delivery::deliver
  opening session, push, mark PR ready
  repair_loop::take_to_goal(&mut RunOutside, base, goal)
    rounds / take_in_foreign_commits / round_after_failed_merge
      Outside: merges, push, head, CI watch / Check re-run,
               repair(kind, Repair::{Conflict(upstream), CiFix(failed), Review{from}}),
               Base fix, PR check, merge, interrupt, progress line
  if Goal::Merged: after_merge (delete branch, close issue)

• The loop names each Repair by kind. RunOutside builds the same prompts as before through the prompt module.
• The budgets return their progress lines, and all of the loop's lines go out through the seam one at a time. Lines written by the CI watch, the Check re-run, the worktree, the Base fix and the sessions stay with them.
• Users see no change: the same calls in the same order, the same lines and the same causes. The Delivery's public interface is unchanged.

Evidence

• Before: apart from the budget arithmetic, the loop's rules could only be checked end to end, against a git origin, the fake gh and the fake claude.
  After: 25 unit tests in delivery::repair_loop drive take_to_goal against a scripted adapter. They assert on the result, the recorded effects in order and the lines, for example:

  script: heads [h1], watch Passed ×2, merge fails "refused by a ruleset"
  expect: Err is PolicyRefusal
          [merge base, push, watch h1, check PR, "merging the PR into main", merge h1,
           "the merge failed: refused by a ruleset",
           merge base, push, watch h1, check PR]
  

  I mutation-checked them: dropping the budget comparison in the Policy refusal rule, or flipping the ReadyForReview goal check, makes 3 tests fail.

• I deleted 23 end-to-end tests that only checked a loop decision (CI, Merge run and mergeable suites). Each production adapter call keeps at least one end-to-end test:

  • each worktree operation, a real conflict and a Foreign commit fetch;
  • the CI watch and the gh run rerun Check re-run;
  • conflict Repairs against the Base branch and against the Issue branch, a CI-fix Repair and a review Repair;
  • the Base fix;
  • the PR check: closed, draft, unmergeable and unknown;
  • the merge of exactly the watched head, a Policy refusal, and Ctrl-C before the merge.

  The Spec run and Base fix suites are unchanged.

• cargo test: 880 passed, 0 failed. cargo clippy --all-targets and cargo fmt --check are clean.

Merge Danger

Door: two-way

A refactor plus test deletions; a revert restores both.

Blast Radius: internal

Every Run and Spec run goes through this loop. A behaviour slip would show up as a different Repair order or different budgets, but the logic is a line-for-line port.

Unaddressed findings

Standards

• The three kept Budgets unit tests test past the interface ("replace, don't layer"). Skipped: the issue says "The three existing budget tests stay, adjusted to the returned lines."
• The Outside seam is wide, with 19 methods; bundle some. Skipped: the issue lists exactly what the seam carries, and bundling would hide the order of a round that the tests check.
• Outside / RunOutside are mysterious names. Skipped: the seam is the loop's "outside effects", as the issue's title puts it, and the doc comments say what each one holds.
• next_repair returns a positional (String, String). Skipped: it has one private caller, and the doc comment names both parts.
• pending(Merge) is Feature Envy and belongs on Merge in the worktree. Skipped: it maps worktree types for the seam only, and moving it would widen the worktree's interface for one adapter.
• Pure queries take &mut self. Skipped: the issue decided "The seam's methods take &mut self."

Spec

• The scripted adapter doesn't record reads (head, base moved, new commits). Skipped: the issue asks for "the effects the adapter recorded (in order)". The reads show through the effects and lines that follow them.
• The kept budget tests assert on Budgets fields. Skipped: the issue keeps those three tests explicitly. The interface tests never assert on internal state.

Closes #372

🤖 Generated with Claude Code