[JacobStephens2]

Summary

What a Run is asked now has one home, src/asks.rs: the flags as the command gave them (Flags), and what they come to with the User config for each kind of run (Asks).

 src/
+├── asks.rs       # Flags as given, Asks resolved, the child Run rule, the retry command
 ├── args.rs       # fills in Flags; RunFlags, DispatchArgs and retry_with_base_fix gone
 ├── config.rs     # holds settings only: default_goal, default_base_fix, default_ask gone
 ├── spec_run.rs   # Parallel and its constructor gone
 ├── run.rs        # run_to_end(issue, asks, started_by, logs_dir): four values, was seven
 └── main.rs       # resolves no ask itself

Asks::of_run(issue, flags, child, config)            a Run on an Issue URL, child or not
Asks::of_architect_plan(plan, flags, config)         a dispatched Architect plan
Asks::of_ready_issue(issue, is_spec, flags, config)  a dispatched Ready issue
flags.notification(config)                           an Architect run's or a Pickup run's own Run notification

• The child Run rule (always a Merge run, no Run notification, the Launch directory left alone, no Base fix unless given one or the command to offer one) is in Asks::of_run and nowhere else.
• A dispatched run's asks say it sends no Run notification of its own.
• parallel ignored for a Ready issue that is not a Spec, in the asks and in the retry command, is in Asks::of_ready_issue.
• The retry command is built inside the module from the flags as given: main.rs no longer keeps a copy of the email flag beside its resolved value.

Seams under test: the asks module's interface (Asks::of_run, Asks::of_architect_plan, Asks::of_ready_issue, Flags::notification, Flags::any_for_dispatched_run), with the retry command read back through args::parse; and the argument parser's args::parse, whose existing unit tests moved onto Flags.

End-to-end tests keep one case per ask per path. Removed, as the unit table covers them:

File	Removed
tests/user_config.rs	7 tests: merge.always and base.fix against the flag, the User config alone, and neither
tests/architect_run.rs	4 tests, and the User config row of the Base fix reaching a Ticket's Run
tests/pickup_run.rs	1 test; merge 12 Runs to 1, Base fix 6 to 1, notification 7 to 1, two skip and pre-flight tables 2 to 1
tests/spec_run.rs	6 tests
tests/run_notification.rs	5 tests
tests/base_fix.rs	1 test

Evidence

• Before: the rules were only reachable by starting the binary, e.g. a Pickup run's merge test ran twelve whole Runs to check a default.
  After: 14 unit tests in asks::tests, in-process:

cargo test --bin thirdshift
test result: ok. 137 passed; 0 failed

• No behaviour change: after the module and its wiring landed (a79857c), and before any end-to-end test was touched, the seven affected end-to-end files passed unedited: architect_run 74, base_fix 21, launch_pull 8, pickup_run 68, run_notification 24, spec_run 80, user_config 20, all passed.
• Full suite after the trimming (2b66883): every test binary passed, 0 failed (architect_run 70, base_fix 20, pickup_run 67, run_notification 19, spec_run 74, user_config 13, the rest unchanged). After the review fixes (the last commit) I re-ran the unit tests and pickup_run, command_surface and user_config, not the whole suite again: all passed.
• cargo fmt --check and cargo clippy --all-targets -- -D warnings are clean.
• prompts/, CONTEXT.md and docs/adr/ are unchanged.

Merge Danger

Door: two-way

A refactor with no change a user sees: same flags, defaults, progress lines, retry command and Run notifications. Reverting the PR restores the old shape.

Blast Radius: contained

Every command that starts a Run goes through the new module, so a mistake in it would reach thirdshift <Issue URL>, architect and pickup alike. After this PR no end-to-end test shows merge.always, spec.parallel or base.fix from the config file changing a Run started on an Issue URL: that is split between the config-parsing unit tests and the asks unit tests, as the issue asks.

Unaddressed findings

Standards

• ArchitectArgs holds flags and plan_only: bool where it held Option<DispatchArgs>, so "no dispatch flags with --plan-only" is no longer in the type. The issue asks for one shared flags type and keeps this refusal as a parsing rule (Decision 4); the check itself now lives on Flags so a new flag can't slip past it.
• asks and args depend on each other, and the retry command's spellings sit apart from the parser's. Decisions 4 and 5 put the words in the parser and the retry command in the asks module; the round-trip test through args::parse is what holds the two together.
• run_to_end takes Asks but does not act on asks.notification. The issue lists the Run notification among the asks and has the Run take them as one value; what started the Run sends the notification, as the doc says.
• tickets_at_once and parallel_asked are the old Parallel pair flattened, and the ask is email in Flags but notification in Asks. Decision 1 says the module holds two types; email is the flag's word and notification what it resolves to, as the existing NotificationAsk already had it.
• of_ready_issue takes a bare is_spec: bool. It is a fact about the issue taken, passed straight from pickup::Taken::is_spec, not an option of the run.

Spec

• Four end-to-end tests were collapsed to one case and renamed rather than left unedited (base_fix_reaches_each_tickets_run_of_the_dispatched_spec_run, merge_reaches_the_dispatched_run, base_fix_reaches_the_dispatched_run, a_pass_that_took_an_issue_sends_one_notification_when_asked), as were two more Pickup run tables after review. Their other rows only varied whether a flag or the User config decides, which the issue says to remove; keeping the loop and the old name around one row would misdescribe the test.

Closes #304

🤖 Generated with Claude Code