[JacobStephens2]

Summary

thirdshift architect [<focus>] [--plan-only] starts an Architect run: an Architecture review of the Base branch publishes its top recommendation as a plan, and thirdshift marks the plan ready and dispatches it as thirdshift <plan URL> would.

thirdshift architect [<focus>] [--plan-only] [merge|no-merge] [email|no-email] [parallel <n>] [base-fix|no-base-fix]
  preflight, without an Issue URL        # origin on GitHub, git identity, HEAD attached, Base branch not ahead
  launch pull, if the User config asks
  worktree <repo>-architect, detached at origin/<base>
  Architecture review session            # Factory skills, headless; commits and pushes nothing
  remove the worktree, whatever the outcome
  read the final line
    "Architecture review plan: <url>"
      check: in this repository, open, created during this Architect run, no other unready label
      swap needs-triage for ready-for-agent
      --plan-only   -> print the plan's URL, exit 0
      otherwise     -> Spec run (sub-issues) or Run, with merge / parallel / base-fix
                       the Architect run ends as that run does
    "Architecture review idea: <url>"           -> print it, exit 0, no label changed
    "Architecture review already filed: <url>"  -> print it, exit 0, nothing filed
    no such line, failed or interrupted session, plan fails a check
                                                -> exit 1, cause and session log; plan stays needs-triage
  one Run notification, if asked; the dispatched run sends none

skills/
├── improve-codebase-architecture/   # headless: scan, take the top recommendation, publish plan or idea
├── to-spec/                         # headless: no seam check with a user
├── to-tickets/                      # headless: Tickets with "blocked by" links
└── codebase-design/                 # the architecture vocabulary
src/
├── architect.rs      # the review, the final line, the plan's checks, the label swap
├── args.rs           # Command::Architect; RunFlags reads the flags a Run shares
├── main.rs           # dispatch, exit code, help
├── notification.rs   # ArchitectNotification: the review's outcome, then the dispatched run's
├── worktree.rs       # ReviewWorktree: detached, always removed
├── prompt.rs         # the Architecture review prompt and its three final lines
└── spec_run.rs       # needs-triage and ready-for-agent, one home
prompts/architecture-review.md, site/prompts/   # generated
tests/architect_run.rs                          # 49 end-to-end cases on the fake claude and gh

The last commit addresses the review of the Spec as a whole: one RunFlags in place of two copies of the flag match, ready-for-agent beside needs-triage, a Sender on the prompts page in place of an empty list of units, the README's steps in the order the code takes them, and the README and help saying that a review that finds no deepening opportunity fails the Architect run.

Evidence

• Before: main has no architect command, no Architecture review prompt and no tests/architect_run.rs.
  After: cargo test at fded284: all 28 test targets pass, with 0 failures. tests/architect_run.rs: 49 passed. Unit tests: 94 passed, including the parser tests for architect and the prompts page snapshot, which the label constants and Sender left unchanged. cargo fmt --check and cargo clippy --all-targets are clean.

architect --plan-only, review publishes a Ticket   -> label swapped, URL on stdout, exit 0
architect, plan with sub-issues                    -> Spec run; ends with the Spec PR's URL
architect, single Ticket                           -> Run; ends with its PR's URL
review files an idea / names the covering issue    -> URL on stdout, exit 0, no label changed
review fails, is interrupted, or names no issue    -> exit 1, plan left needs-triage
plan closed / older than the run / other unready label / other repository -> refused
merge, parallel, base-fix, User config defaults    -> reach the dispatched run
email                                              -> exactly one Run notification, for every outcome

Merge Danger

Door: two-way

A new command and new Factory skills; a Run and a Spec run keep their behaviour. Reverting removes the command. Issues an Architect run has already published stay on the tracker.

Blast Radius: contained

The shared code it touches is the argument parser (a Run's flags now go through RunFlags), run::run_to_end, the Run notification body and the worktree helpers. main has since defined in CONTEXT.md, for a later Spec (#250), that an Architect run is skipped while another is running and that plans carry an architect-plan label. This branch does neither: its README says nothing stops a second Architect run.

Unaddressed findings

Standards

• (reviewed, dispatched) matched in both main.rs and notification.rs (Repeated Switches): the two matches make different things, the exit code and stdout in one and the email in the other, and a type for an ended Architect run would move both matches, not remove either.
• DispatchArgs and RunArgs share goal, parallel, base_fix, and main resolves their defaults twice (Data Clump): the parsing is now shared through RunFlags, but a child Run resolves its defaults differently from an Architect run's dispatch, so the two sites are left apart.
• architect::run and run::run repeat the launch steps (Duplicated Code): the shared steps are already functions (check_identity, check_base_branch, pull_base_branch); what is left is a line or two each, and differs on a detached HEAD and a given Base branch.
• architect::Report and Outcome are near twins: one is what the review claimed and the other what thirdshift checked; PlanReady is only true after the label swap.
• launch() returns a tuple and architect::repo() reads origin again for the notification: one extra git config read, before any work, so the notification can name the repository even when preflight later fails.
• A failed review is failed on the Review: line and review failed in the subject: deliberate and documented in the README; the line's label already says it is the review.
• Test helpers repeated from other test files (checks_on_head, the_one_notification, …): each test file in this repo carries its own, and moving them to tests/support is a change across every file, not this Spec's.
• "parent issue" in skills/to-tickets, which CONTEXT.md lists under Avoid: the Factory skills are kept close to the originals so they can be re-synced, and they run in other repositories.

Spec

• The skill takes the highest-ranked Strong candidate no issue covers, even when a non-Strong one ranks above it, where the Spec says "takes the top recommendation": what is implemented is still always Strong, and Headless Factory skills for the Architecture review #229 settled this wording in its own review. Worth a human decision: the stricter reading files an idea and implements nothing in that case.
• With no uncovered Strong candidate, a covered top candidate ends as "already filed" rather than being skipped for the next uncovered idea: Stories 11 and 22 pull against each other here (a skipped candidate can never be "already filed"), and the skill's reading is the one that keeps Story 22 reachable.
• A scan with no candidate at all fails the Architect run: the Spec has no outcome for it and says a missing line is a failed review. Now documented in the README and help; a success outcome for it would be a Spec change.
• parallel <n> on a single-Ticket plan fails after the plan is marked ready: Architect run dispatches its plan as a Spec run or a Run #232 asked for it to be "rejected or reported as for thirdshift <Issue URL>"; it is tested and documented, and the plan is left ready to run without it.
• The fixed <repo>-architect worktree path stops a second concurrent Architect run, and a worktree left by a killed one blocks the next: removing a stale worktree on start would delete a running review's. Documented in the README; the lock is out of scope here and is what Define Architect plan and when an Architect run is skipped #250 defines.
• Dispatch flags rejected with --plan-only: not in the Spec, but Architect run dispatches its plan as a Spec run or a Run #232 and Pass Base fix through an Architect run to its dispatched run #234 asked for it.
• The plan must be in the origin repository: not in the Spec's list of checks, kept so the wrong repository's issue is never relabelled or dispatched.
• Unit tests of the private Report::read: the Spec's "don't test internals" is about the end-to-end seam; the repo unit-tests its parsers the same way.

Closes #27

Tickets

• Headless Factory skills for the Architecture review #229 landed with Headless Factory skills for the Architecture review #238
• thirdshift architect --plan-only: the Architecture review publishes a plan and thirdshift marks it ready #230 landed with thirdshift architect --plan-only: the Architecture review publishes a plan and thirdshift marks it ready #240
• Architect run with no Strong candidate: idea issue or already filed #231 landed with Architect run with no Strong candidate: idea issue or already filed #243
• Architect run dispatches its plan as a Spec run or a Run #232 landed with Architect run dispatches its plan as a Spec run or a Run #244
• One Run notification per Architect run #233 landed with One Run notification per Architect run #245
• Pass Base fix through an Architect run to its dispatched run #234 landed with Pass Base fix through an Architect run to its dispatched run #251