[JacobStephens2]

Each Pass, an Architect run or a Pickup run, now owns its whole lifecycle in its own module, from its gates to the Spec run or Run it dispatches, generic over the Pass seam. Behaviour is unchanged.

Closes #413

Summary

The entry point's Pass arms shrink to the command's start, the Pass, and the command's finish:

 main::pickup
   command start
-  pickup::run                 # gates, skip, Harness check, start
-  Asks::of_ready_issue
-  run::run_to_end
+  pickup::run                 # lock + preflight, builds LaunchAndGitHub
+    run_through<impl Outside> # gates → skip | step → check_harness → started → dispatch
   command finish

 main::architect
   command start
-  architect::run              # gates … review worktree, Sessions, conclude
-  if PlanReady && !plan_only: step, Asks::of_architect_plan, run::run_to_end
+  architect::run              # start time, lock + preflight, builds LaunchAndGitHub
+    run_through<impl Outside> # gates → skip | check_harness → started → pull?
+                              #   → interrupted? → review(conclude) → step → dispatch unless plan-only
   command finish

The Pass seam (src/pass.rs) grows the calls a Pass makes outside itself beyond GitHub. They are skipped, check_harness, started, pull, review<T> (the conclusion runs inside the session) and dispatch(Dispatch). The production adapter OnGitHub is renamed LaunchAndGitHub. It builds the dispatched run's Asks from the command's flags and the User config, on the checked Harness. InMemory records every call in the one ordered calls list, so a single assertion checks a whole Pass's order.

Evidence

• Before: each ordering rule was testable only end-to-end, through a whole thirdshift pickup or thirdshift architect run against the fake gh, git and claude binaries. Examples: a skipped Pass makes no Harness check, and the dispatch comes after the label swap.
  After: in-process unit tests through InMemory in src/pickup.rs and src/architect.rs, for example:

  pickup(ready 9, harness failing) → Err, calls == [Closed, Open(in-progress), ReadySearch, Step("taking Ready issue #9 …"), HarnessCheck]
  architect(plan reviewed)        → calls end [… Edit{swap}, Step("dispatching the plan …"), DispatchPlan{40, "main"}]
  

  Five end-to-end tests that only checked one such rule are removed. The ones covering the lock, the preflight checks, the Run notification, the Factory skills and the review's worktree, flags reaching the dispatched run, and the dispatched run's ending are kept. cargo test passes (535 unit tests plus the end-to-end suites), and cargo clippy --all-targets is clean.

Merge Danger

Door: two-way

This is an internal refactor with no change to config, labels, logs or outputs. Reverting it is a plain revert.

Blast Radius: Passes

Every cron-driven thirdshift pickup and thirdshift architect goes through the new path. The risk is a subtle drift in the order of stderr lines or log records. The review found one: the review's starting line came before the Factory skills were linked. It is fixed in ea3e380.

Unaddressed findings

Standards

• Outside trait widens to ~14 methods (Divergent Change): skipped. The Spec decides on one seam for both Passes so that a single ordered call list covers a whole Pass.
• The choice of Asks lives in the production adapter, untested by InMemory: skipped. The Spec puts it there: the Pass decides whether and what to dispatch, the adapter decides how, and the Asks rules keep their own tests.
• review<T> generic over one caller (Speculative Generality): skipped. The Spec makes it generic so that the seam module names no Architect type.
• Duplicated lock-and-adapter prologue in pickup::run and architect::run, with the lock skip recorded outside the seam: skipped. The Spec records the lock skip before any adapter exists, and the two prologues differ in their error handling (? against a failed review).
• flags, config and harness travel together (Data Clumps): skipped. They are the adapter's fields and are bundled there once.
• Call::Started(Option<u64>) re-switches on Work: skipped. It is a test-only projection that keeps assertions short.
• The names run_through and up_to_the_dispatch: skipped. <name>_through is the codebase's convention, as the Spec requires.
• pull: bool and plan_only: bool side by side (Primitive Obsession): skipped. They are private fields with doc comments, and the reviewer judged them acceptable.

Spec

• Only 5 of the ~140 end-to-end tests are removed: skipped. The Tickets remove only those that checked a single Pass rule now unit-tested. The rest cover the production adapter and the composition, for example a Model that claude refuses, through the real Harness check.
• review takes an extra starting progress line, not in the Spec's call shape: kept. It lets the line come after the worktree and the Factory skills are ready, as on main.
• InMemory::review returns its scripted Session log even on failure, while the real adapter returns it only if the file exists: kept. It is a scripted fake, and the tests script exactly what they assert.

Tickets

• A Pickup run owns its lifecycle to its dispatch, tested in-process behind the Pass seam #414 landed with A Pickup run owns its lifecycle to its dispatch, tested in-process behind the Pass seam #416
• An Architect run owns its lifecycle to its dispatch, tested in-process behind the Pass seam #415 landed with An Architect run owns its lifecycle to its dispatch, tested in-process behind the Pass seam (#415) #418

🤖 Generated with Claude Code