[JacobStephens2]

Summary

An Architect run now handles its whole lifecycle in its own module, up to the dispatch of its plan, and is tested in-process through the Pass seam.

 main::architect
   command::start
-  architect::run -> Result<Outcome, FailedRun>
-  match review: PlanReady && !plan_only
-    Asks::of_architect_plan + run::run_to_end
+  architect::run(focus, base, plan_only, …) -> Outcome::{Skipped, Ran { review, dispatched }}
   started.built_with(harness)
   started.finish

architect::run                 # start time, lock (lock skip recorded), preflight checks, LaunchAndGitHub
  run_through(impl Outside)    # gates → skipped(..)
    up_to_the_dispatch
      check_harness → started → pull (if launch.pull) → interrupted?
      review(base, starting line, prompt, conclude)   # new Pass seam call
    step("dispatching the plan …") → dispatch(ArchitectPlan)   # unless plan-only

The Pass seam gets two new calls:

• pull(base)
• review(base, starting, prompt, conclude) -> (Result<T>, Option<Session log>)

In production (LaunchAndGitHub), review does the following, in order:

1. creates the review worktree;
2. hands on the review's starting line;
3. runs Sessions::within, with a clone of the Harness choice, to the session's final message;
4. runs the conclusion inside that session;
5. removes the worktree.

InMemory records Pull and Review in its ordered call list. Its review hands a scripted final message to the conclusion and returns a scripted Session log, or fails. It also records DispatchPlan.

Reviewed::PlanReady no longer carries the Base branch, because run_through already has it.

Evidence

• Before: the unit tests could only reach an Architect run's gates and conclusion. Ordering rules, such as the dispatch's line coming after the label swap and a skipped run making no Harness check, could only be checked by end-to-end tests.
  After: new unit tests through InMemory in src/architect.rs cover each rule the issue lists:

  • a skip for open plans, a waiting idea or a Ready issue makes no Harness check, start, pull or review;
  • a failed Harness check fails the run before its start;
  • the pull is made only when asked, after the start;
  • an interrupt after the gates fails the run as interrupted, with no review;
  • the review's prompt carries the Base branch and the focus;
  • a plan is dispatched on the Base branch after the label swap, unless plan-only;
  • an idea, filed or already filed, dispatches nothing;
  • a failed review dispatches nothing and carries its Session log.

  The three end-to-end tests that only checked one of these rules were removed. progress_lines_cover_the_worktree_the_review_starting_… now also checks that the worktree line comes before the review's starting line, as it did before this change.

• cargo test: every suite passes. cargo clippy --all-targets -- -D warnings is clean.

Merge Danger

Door: two-way

This is a refactor with no change in behaviour, and it reverts cleanly.

Blast Radius: Architect-runs

If something is wrong, it would show up in Architect runs: the order of their progress lines, their failure handling, or their dispatch. Runs and Pickup runs are untouched.

Unaddressed findings

Standards

• architect::run returns Outcome while pickup::run returns Result<Outcome>. Kept as is: an Architect run's failures, preflight included, end as a failed review with a Run notification. Outcome::Ran carries that failure, as Ending::Architect does.
• Outcome::Ran has the same shape as Ending::Architect (Duplicated Code / Middle Man). Kept as is: command would otherwise depend on architect's outcome type, and main maps one to the other in a single line.
• Outside::pull only delegates (Middle Man). It is a seam call that the issue asks for, so the tests can see it.
• plan_only and pull are bools (Primitive Obsession). They mirror the command's flag and launch.pull; an enum would add ceremony for one guard each.
• widgets() and ready_for_review() duplicate test helpers in pickup.rs. These are small fixtures, and sharing them across modules is out of scope here.
• The let Outcome::Ran { review: Err(failed), dispatched: None } else block appears twice in the tests. Those two tests also check failed.interrupted and failed.log, which the reviewed() helper drops.

Spec

• a_focus_goes_into_the_session_prompt overlaps the new unit test. Kept: it also checks that the focus is parsed before or after --plan-only, which only an end-to-end test covers.

Closes #415

🤖 Generated with Claude Code