[JacobStephens2]

Closes #247

Summary

Only one Architect run per repository runs at a time on a machine. One started while another is still running, its dispatched Spec run or Run included, is skipped: exit 0, one progress line, nothing else.

 thirdshift architect
   arguments, User config
   Run notification's checks
   preflight checks (origin, identity, HEAD, Base branch)
+  try the repository's lock, without waiting
+    held by another process
+      -> Outcome::Skipped(AlreadyRunning)
+         stderr: an Architect run is already running on <owner>/<repo>
+         stdout: nothing, exit 0
+         Run notification, if asked: subject "... Architect run: skipped", body "Skipped: <reason>"
+    taken
+      -> held until the process exits, through the dispatched Spec run or Run
   launch pull
   review worktree
+    a leftover <repo>-architect worktree of a killed run is removed first
   Architecture review, plan checks, label swap
   dispatch

• The lock is an OS advisory lock on ~/.thirdshift/architect-locks/<owner>/<repo>.lock, lower-cased as GitHub compares names. The file is never deleted and means nothing unless locked; the operating system releases the lock when the process ends for any reason.
• architect::Outcome is now Skipped(Skipped) or Reviewed(Reviewed); Reviewed holds the three endings a review had before. The open-plan Ticket can add its skip as a second Skipped variant.
• thirdshift help, the README (new "One at a time" section, replacing the "start one at a time" advice) and the Run notification glossary entry describe the rule.

Seam under test: the thirdshift binary through the architect_run end-to-end tests, with the fake claude and gh. The harness gains Scenario::run_until, which returns a HeldRun the test can signal, kill or finish, and run_and_signal is now built on it.

Evidence

• Before: with the first Architect run held mid-review, the second failed on the worktree:

  a_second_architect_run_on_the_repository_is_skipped_while_the_first_is_mid_review ... FAILED
  thirdshift: git worktree add --detach .../work/widgets-architect origin/main failed
  fatal: '.../work/widgets-architect' already exists      (exit 1)
  

  and after a kill -KILL of the first, a new one failed the same way.

  After: cargo test passes, all targets, 0 failed. The new end-to-end cases in tests/architect_run.rs (56 pass, 7 new):

  a_second_architect_run_on_the_repository_is_skipped_while_the_first_is_mid_review
    exit 0, stderr is exactly the skip line, stdout empty, no session,
    no launch pull, GitHub state and gh calls unchanged
  a_second_architect_run_is_skipped_while_the_firsts_dispatched_run_or_spec_run_is_still_going
  once_the_first_architect_run_has_ended_a_new_one_runs
  once_the_first_architect_run_is_killed_a_new_one_runs_with_nothing_to_clean_up
    the second starts while the killed run's session is still going
  an_architect_run_on_a_different_repository_at_the_same_time_is_not_skipped
  a_skipped_architect_run_sends_one_notification_with_skipped_in_its_subject_and_why_in_its_body
  the_notifications_checks_come_before_the_skip_so_a_skip_never_goes_unnotified
  

  The three concurrency cases were run five more times in a row without a failure. cargo clippy --all-targets and cargo fmt --check are clean.

Unaddressed findings

Review was /thirdshift:code-review against issue-237. Addressed: the lock function now returns the held file (S1, in part), reviewed renamed where it can hold a skip (S2), one Outcome::name() for the notification's outcome word and one match less (S3), the worktree module no longer narrates the Architect run's rule (S4), an assert_skipped_as_already_running helper (S6), and the kill case no longer releases the orphaned session before the next run starts (C3).

Standards

• S1, rest: the lock is still leaked with std::mem::forget, not carried to main as a guard. The Spec says it is held until the process exits; leaking it at the one place that takes it says exactly that, and carrying a guard out through Result<Outcome, FailedRun> would bend that return type for no behaviour.
• S1: try_run_lock repeats the open-and-lock shape of Git::lock. Two short call sites that differ in the parts that matter (try, not wait; under ~/.thirdshift, not the git directory); a shared helper would be mostly parameters.
• S5: Repo::slug beside IssueUrl::repo_slug. repo.repo_slug() would stutter; only its doc comment was corrected.

Spec

• B1: removing a leftover review worktree was not asked for in so many words. Kept: without it "after the first run … is killed, a new Architect run … runs normally, with no cleanup by the user" does not hold, as the Before above shows.
• B1, corner: a killed run's agent session can outlive it, so the next Architect run may review while that orphan is still going. The lock is the process's by design ("the operating system releases it when the process ends for any reason"); stopping orphaned sessions is outside this Ticket.
• B2: the lock key is lower-cased. Kept: GitHub names are case-insensitive, and two spellings of one origin must not get two locks.
• B3: a unit test of the skipped body. Kept: it sits with the existing body tests in notification.rs, and the end-to-end case covers the same line.
• C1: "no worktree" is asserted by directory listing, which alone would miss a remove-and-recreate. The same test asserts stderr is exactly the skip line and that no session started, which do catch it.
• C2: a skipped run still runs the preflight git ls-remote and git fetch. That is Nightly Architect runs: a cron recipe, and an Architect run that is safe to schedule #237's order: preflight checks, then the lock.

Merge Danger

Door: two-way

Reverting restores the old behaviour; the lock files left under ~/.thirdshift/architect-locks/ are inert.

Blast Radius: Architect-runs

Every thirdshift architect now takes the lock, and fails before any work if ~/.thirdshift can't be written or the filesystem refuses advisory locks. A second Architect run that used to fail on the worktree now exits 0. A directory at <repo>-architect that is a registered worktree of the clone is force-removed at the start of a review. Runs and Spec runs on an Issue URL are untouched.

🤖 Generated with Claude Code