Deepen Delivery's stage into one module that records both Recovery and the Command record

[JacobStephens2]

Summary

Delivery now owns its stage in one module. Delivery's steps and the Repair loop name each stage they enter through their Outside seam. One table maps each stage to the Stopped at: stage and, where there is one, the Command record stage. The adapters no longer set stages by hand.

 src/delivery.rs              # Route::steps calls outside.stage(...) before each step
+                             # Opening { session: OpeningSession, prompt }: one fact
 src/delivery/
+├── stage.rs                 # Stage enum, the one table, Tracker
 └── repair_loop.rs           # Outside::stage takes the Delivery Stage

Delivery stage	Stopped at:	Command record stage
Implementation	Implementation	none
Spec review	Spec review	none
Security review	Security review	Security review
Push	Push	pull request
Pull request readiness	Pull request readiness	pull request
Repair loop round	Repair	none
Repair session	Repair	Repair
CI watch	CI watching	CI watch
Base fix	Base fix	none
Self-merge	Self-merge	Self-merge

The adapters' work moves into the steps and the loop:

 InWorktree / RunOutside
+  stage(s)  -> tracker.enter(s)          # their only stage code
-  push()    -> stage.set(Push); logs::stage(PullRequest); ...
-  watch()   -> stage.set(Ci);   logs::stage(CiWatch); ...
-  stage(s)  -> match s { SelfMerge | PullRequest => log, _ => {} }
 Repair loop
+  stage(RepairLoopRound) at each round, and before the Repair budget gate
+  stage(RepairSession) before each Repair session
+  stage(Push | CiWatch | BaseFix | SelfMerge) before each of those steps

The Repair budget gate enters a Repair loop round rather than a Repair session. Exhausted repairs start no session, so they still record no "Repair" Command record stage, as on main. A Security hold still reports Stopped at: Security review through Tracker::stop_at_security_review, without recording anything.

The tests go at these seams:

• stage::Tracker (inline tests in src/delivery/stage.rs), with the in-memory Recorded adapter. They check the table, the collapsing of a repeated stage and the Security hold.
• The Repair loop's Outside, through its Scripted double. The double keeps a stages list apart from its call log, so the existing call-sequence assertions are unchanged.
• Delivery's Outside, through its Scripted double, which keeps a stages list the same way.

Evidence

The new inline tests pass. Each one's expected stage comes from the issue's table:

stage::tests
  entering_each_stage_sets_its_recovery_stage_and_records_its_command_record_stage
  a_stage_entered_again_is_recorded_once
  a_security_hold_stops_at_the_security_review_without_recording_it_again
repair_loop::tests
  a_failed_merge_of_the_base_branch_stops_in_a_repair_loop_round
  a_failed_ci_watch_stops_in_the_ci_watch
  a_failed_repair_session_stops_in_the_repair_session
  exhausted_repairs_stop_in_a_repair_loop_round_without_a_session
  a_refused_self_merge_stops_in_the_self_merge
delivery::tests
  a_delivery_enters_each_stage_before_its_step
  a_spec_delivery_opens_in_its_spec_review       (catch-up or session fails -> Spec review)
  each_failed_step_stops_at_its_stage
  a_security_hold_stops_at_the_security_review_without_entering_it_again

• Before: on main, the Repair loop's Scripted double ignored stages (fn stage(&mut self, _: Stage) {}), and no test covered the mapping between the two vocabularies.
• After: cargo test passes in full: all 52 test binaries ok. cargo clippy --all-targets and cargo fmt --check are clean. No integration test changed.

Merge Danger

Door: two-way

This is a behaviour-preserving refactor with no persisted format change. The table reproduces main's pairs, and the Spec reviewer traced every failure point against main.

Blast Radius: status

If a pair were wrong, thirdshift status, the Console and the Stopped at: line of a Failed run's ending would show the wrong stage. It would not change what a Run does.

Review

The review used thirdshift-code-review with main as the fixed point. Neither axis reported a finding that says behaviour is wrong. Fixed after the review:

• Standards: the Opening data clump. kind, stage and catch_up_from_origin are now one fact, OpeningSession.
• Standards: the duplicated table read in Tracker::new.
• Standards: security_hold is renamed stop_at_security_review.
• Standards: delivery::Stage is narrowed to pub(super).
• Spec: the opening's "one fact" is now its own two-variant enum.

Every changed file was read by both reviewers.

Unaddressed findings

Standards

• Tests rebuild the production wiring (InWorktree::stage and RunOutside::stage are untested). Those adapters are the single line self.tracker.enter(stage) over a real worktree and GitHub, which inline tests cannot drive. The issue's "Inline tests cover" list asks for the table at the module's own seam and the stage order at both Scripted doubles. Both are done.
• Locality and Shotgun Surgery: every step pairs outside.stage(X) with its call. The issue's decision "The steps name the stages; the adapters stop" settles this.
• Duplicated stage(SelfMerge) in take_to_goal (src/delivery/repair_loop.rs, outside.stage(Stage::SelfMerge) before outside.merge). The issue requires the call ("The loop calls it before: … the Self-merge"), and its decision "Keep the stage recording idempotent and call it on every entry" covers the repeat.
• Three Stage types (Mysterious Name). The issue's first criterion says the module "owns a Delivery Stage enum". The type is now pub(super), so only src/delivery/ sees it.
• Stage::records() returns an unnamed tuple. It is private to src/delivery/stage.rs, used once in Tracker::enter, and its doc comment names both halves.
• Middle Man: one-line stage adapters. The issue requires them: "InWorktree and RunOutside implement stage once each, by entering the tracker."
• Fold the stopped_at free function into the tracker. Recognising a SecurityHold error stays in src/delivery.rs, beside fail's keep_ready check on the same error types. The tracker offers the one-case method the issue asks for.

Spec

• Delivery tests replay the failure path rather than drive Delivery::deliver. deliver seeds its tracker with Tracker::new(opening.session.stage(), CommandLog), the same OpeningSession::stage the test helper uses, so the two cannot drift apart. Driving the real deliver needs a real worktree and GitHub. The issue's list of inline tests is met.

Closes #865

🤖 Generated with Claude Code

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

Security review outcome

No unaddressed introduced findings.