[JacobStephens2]

Summary

A Run that fails on Inherited failures with no Base fix taken now says, after its cause, where the checks fail on the Base branch, and offers a Base fix when nobody decided against one. The same lines go on stderr and in the Run notification's body.

 thirdshift: CI red on test, which also fails on main at 362b9ca; fix main first
+thirdshift: Base check: test: https://github.com/acme/widgets/actions/runs/1/job/2
+thirdshift: Retry with: thirdshift https://github.com/acme/widgets/issues/7 base-fix
+thirdshift: Or set: base.fix = true in ~/.thirdshift/config.toml, to allow a Base fix for every Run on this machine
 thirdshift: session log: ~/.thirdshift/logs/acme-widgets-issue-7-….jsonl

 Pull request: https://github.com/acme/widgets/pull/1
 Cause:        CI red on test, which also fails on main at 362b9ca; fix main first
+Base check:   test: https://github.com/acme/widgets/actions/runs/1/job/2
+Retry with:   thirdshift https://github.com/acme/widgets/issues/7 base-fix
+Or set:       base.fix = true in ~/.thirdshift/config.toml, to allow a Base fix for every Run on this machine
 Session log:  …

Which lines appear:

The Run was…	Base check:	Retry with: / Or set:
given neither flag, no base.fix = true	yes	yes
given no-base-fix	yes	no
failed after its one Base fix	no	no
failed on any other cause	no	no

How it works: "nobody decided" is a third thing a Run can ask about a Base fix, and it carries the command to offer.

 enum BaseFixAsk
   Allow
   Forbid
+  Undecided { retry }      # the command that starts the Run again with base-fix

 BaseFix::fix               # only red checks are Inherited failures
   if taken or may not start
+    if no Base fix taken
+      advice = Base check lines (checks_on the Base branch commit)
+             + Retry with / Or set lines, if Undecided
     bail with the cause    # unchanged

 report (stderr)            Body::text (Run notification)
   cause                      Cause:
+  advice lines             + advice lines
   session log                Session log:

Decisions worth a look:

• The retry command is rebuilt from the parsed flags, not copied from argv: thirdshift <Issue URL> [merge|--no-merge] [--email [<address>]|--no-email] [parallel <n>] base-fix. That keeps one renderer for the three places a Run starts from.
• A Ticket's Run offers the Spec run's command. The Spec run passes it down in a hidden --offer-base-fix <command> argument. The Ticket's Run prints the lines on its own stderr (relayed as #21: Retry with: …); the Spec run's summary and notification show the Ticket's cause alone, and do not repeat the lines.
• After an Architect run, the command offered is thirdshift <plan URL> … base-fix, since another Architect run would start a new review rather than take that plan up again.
• child_run::wait skips the advice lines when it reads a failed child Run's cause and session log back, so what a Spec run reads is byte-for-byte what it was.
• If the checks on the Base branch can't be read, the Run says so in a progress line, links none, and still makes the offer.

Docs: README (new section under Failed runs, plus the Base fix and Run notification sections), CONTEXT.md (Inherited failure, Run notification) and thirdshift help.

Seams under test: the thirdshift binary's stderr and the Run notification it sends (end-to-end, against the fake gh and the Resend stand-in); Body::text; args::parse with args::retry_with_base_fix.

Evidence

• Before: the new end-to-end test fails on main's behaviour; stderr ends with only

  thirdshift: CI red on test, which also fails on main at 47416fb; fix main first
  thirdshift: session log: …
  

  After: cargo test passes, all 28 test binaries, with cargo clippy --all-targets clean. New tests:

  tests/base_fix.rs
    a_run_not_asked_about_a_base_fix_links_the_base_branchs_failing_checks_and_offers_one
      # stderr and notification, in order after the cause; failure commit message is the bare cause
    with_no_base_fix_the_base_branchs_failing_checks_are_linked_and_no_base_fix_is_offered
    with_base_fix_in_the_user_config_and_no_base_fix_given_no_base_fix_is_offered
    a_base_branch_check_with_no_url_is_named_alone
    (existing after-a-Base-fix and failed-Base-fix tests now assert no advice lines)
  tests/spec_run.rs
    a_tickets_run_offers_the_spec_runs_command_with_a_base_fix_and_the_spec_run_repeats_none_of_it
    with_no_base_fix_on_a_spec_a_tickets_run_links_the_checks_and_offers_no_base_fix
  tests/architect_run.rs
    without_base_fix_the_dispatched_run_fails_on_an_inherited_failure   # now asserts the plan-URL retry
  src/args.rs, src/notification.rs
    the retry command round-trips through parse; the body lays the lines out after Cause:
  

Merge Danger

Door: two-way

Reverting the commits restores the old output. The hidden --offer-base-fix argument only passes between a thirdshift and the child it starts from its own binary, so no installed version meets it from another.

Blast Radius: output

A Failed run on Inherited failures prints up to three more lines per check set on stderr and in its notification. Anything that scrapes a Failed run's stderr and assumes the cause is the line directly before session log: would now see an advice line there; thirdshift's own reader was updated for that.

Unaddressed findings

The review ran both axes on Sonnet sub-agents, because the default model returned overload errors three times in a row.

Standards

• Data Clumps: goal, email and parallel travel together into retry_with_base_fix. Two callers, and they draw from different types (RunArgs and DispatchArgs plus the Architect run's own email flag), so a shared type would be made for this one function.
• Feature Envy: retry_with_base_fix re-serializes the grammar parse reads. Both live in args.rs, and a test parses the rendered command back to the same asks.
• Primitive Obsession: advice lines are recognised on a child Run's stderr by their label prefix. This is the convention base_fix::REPORT and failed_run::SESSION_LOG already set for what a Spec run reads back.
• Repeated Switches: BaseFixAsk is matched in BaseFix::new and child_run::start_from. Each match does a different job (what the Run does, what its child is told); the usual cost of a three-way enum.
• Hard-coded ~/.thirdshift/config.toml in the Or set: line. No constant for the displayed path exists; the help text and config.rs's docs spell it out the same way.

Spec

• The retry command is rebuilt from the flags rather than copied from argv, so no-email comes back as --no-email. Kept: a child Run's and an Architect run's own argv are not commands a person should retry, and one renderer serves all three.
• The Architect run's retry command and the "could not read the checks" progress line are not in the issue. Kept: a dispatched Run is a Run that fails on Inherited failures, so it needs some command to offer, and a failed read should not be silent.
• An inherited check missing from the Base branch's failed checks on the second read gets no Base check: line, and that read is a second GitHub call. A Base fix issue's body lists the checks the same way, and the gap needs the Base branch's check to change between the two reads; one extra call on a path that is already failing.
• Only one failing Ticket is tested for "must not appear once per Ticket". The Spec run's summary and notification never carry the lines, however many Tickets fail, so there is nothing per-Ticket to repeat.

Closes #271

🤖 Generated with Claude Code