[JacobStephens2]

Summary

A Spec PR closed, merged or retargeted while its Spec run is working through Tickets now stops the Ticket loop. The Spec run used to log a warning and keep going.

 run_ready_tickets
+  ensure_spec_pr_unchanged          # before each Ticket starts
   start_ticket(kind: Ticket { spec_branch,
+                              spec_pr: captured number + Base branch })
   show(checklist)
-    closed Spec PR → warn_checklist, carry on
+    closed/merged/retargeted → SpecPrChanged (other write failures still warn)
+  keep_cause: a SpecPrChanged replaces an earlier error as the cause

 child Ticket run (and the Base fix it starts into the Spec branch)
   PullRequest::merge
     ensure_ready_and_mergeable
+    ensure_spec_pr_allows_merge     # Spec PR still open, from the Spec branch, into the chosen Base branch
     merge request

 Recovery
+  spec_pr_changed → no Recovery / Launch / Continuing lines;
+                    "Before retrying: … no Recovery command resumes issue-20 into main"

Cause line: the Spec PR <url> was closed under this Spec run (or merged, or retargeted from <base> to <other>).

Confirmed cause. PullRequest::show_checklist routed every checklist failure through warn_checklist, including the identity gate's PR … is closed, not open. So after the Day shift closed the Spec PR, run_ready_tickets kept starting the Tickets that became startable. Their child Runs Self-merged into the abandoned Spec branch, because nothing in a child Run looked at the Spec PR. The Spec run failed only when the next landing's gate rejected the PR. Its Recovery then printed the command that continues the Spec branch (ADR-0002).

GLOSSARY (Spec PR, Recovery), docs/guide.md (Failed spec run section) and the regenerated site/docs/, site/how-it-works/ and docs/code-navigation.md describe the new behaviour.

Evidence

Reproduction: cargo test --test spec_run issue_1471. Spec #20 has Tickets #21, then #22 and #23, both blocked by #21, run with parallel 1. #21 lands and opens the Spec PR; #22's session closes the Spec PR and fails.

• Before (at f842c85): Headless sessions leave smoke-test files in /tmp #23 was started anyway, Self-merged and closed. The Spec run then failed with the generic gate error and printed Recovery: thirdshift https://github.com/acme/widgets/issues/20 --no-merge parallel 1 ….

  thirdshift: could not update the Spec PR's Tickets checklist: PR https://github.com/acme/widgets/pull/2 is closed, not open
  thirdshift: starting #23
  thirdshift: #23: merging the PR into issue-20
  thirdshift: #23 landed
  ---- issue_1471_a_spec_pr_closed_mid_run_stops_dispatch_and_ticket_merges FAILED
    left: ["21", "22", "23"]   right: ["21", "22"]
  

  After: issue_1471_a_spec_pr_closed_mid_run_starts_no_further_ticket ... ok. Only Numbered Issue branches after a merged or closed PR #21 and CI watching, CI-fix Repair and the Repair cap #22 have sessions, Headless sessions leave smoke-test files in /tmp #23 has no PR and stays open, and stderr ends the Spec PR https://github.com/acme/widgets/pull/2 was closed under this Spec run with no thirdshift: Recovery: line.

• Before (with the child wiring removed): a Ticket already running when its session closes the Spec PR Self-merged anyway.

  issue_1471_a_ticket_running_when_the_spec_pr_closes_is_not_self_merged FAILED
    assertion `left != right` failed  left: "MERGED"  right: "MERGED"
  

  After: it passes. CI watching, CI-fix Repair and the Repair cap #22's PR is not merged, issue CI watching, CI-fix Repair and the Repair cap #22 stays open, and both CI watching, CI-fix Repair and the Repair cap #22 and the Spec run say the Spec PR was closed under this Spec run.

• Before (with the Base fix wiring removed): issue_1471_a_base_fix_into_the_spec_branch_is_not_self_merged_once_the_spec_pr_closes FAILED, because the Base fix Headless sessions leave smoke-test files in /tmp #23 into issue-20 was MERGED. After: it passes.

• Before: spec_run::tests::a_spec_pr_closed_after_an_earlier_error_is_still_the_cause panicked with could not wait for #21. After: it passes. Further unit tests:

  • pull_request::tests::a_spec_pr_changed_under_accounting_fails_the_checklist_and_the_ticket_start_check
  • pull_request::tests::a_tickets_self_merge_waits_on_its_spec_pr_still_open_into_the_spec_runs_base
  • recovery::tests::a_spec_pr_changed_under_the_spec_run_leaves_out_every_command_line
  • the child_run hidden-argument round trip, now with --spec-pr

• Full run: cargo test --no-fail-fast -- --skip sandbox::tests: 3092 passed and 21 failed. All 21 are in tests/trial.rs (15), tests/scoring.rs (3), tests/bug_origins.rs (2) and tests/experiments.rs (1), and the same tests fail at the base commit f842c85 on this host. Most report sandbox failed to start: socat is not on PATH; there were no failures unique to this branch. npm test --prefix site: 51 passed. cargo clippy --all-targets: clean. scripts/affected-tests main ran the same suite. Its only failures were the 22 sandbox::tests, which need socat, and this host doesn't have it (sandbox failed to start: socat is not on PATH). None of them touches this change.

Merge Danger

Door: two-way

Revert restores the old warn-and-continue behaviour. No stored data or GitHub state changes shape. The new --spec-pr hidden argument passes only between a parent and a child of the same binary.

Blast Radius: Spec runs

A Spec run now fails sooner, without a Recovery command, when its Spec PR is closed, merged or retargeted. A transient failure to read the Spec PR before a Ticket starts only warns. A Ticket's Self-merge, by contrast, is held by any failure to read its Spec PR, and that Ticket ends as a Failed ticket run with its PR drafted. Two checklist-test expectations changed to the new cause wording, and successful_ticket_completion_with_an_invalid_spec_pr_starts_no_review now expects #22 to fail rather than land.

Unaddressed findings

Standards

• ADR-0006 does not record the new stop (note). Declined. ADR-0006 decides that Tickets merge into a Spec branch, and this change does not contradict that. The stop is recorded where behaviour lives: the GLOSSARY Spec PR and Recovery entries and the guide's Failed spec run section. Whether in-flight Tickets should finish or be abandoned is an open policy question in Stop a Spec run when its Spec PR is closed or retargeted mid-Run #1469 (ready-for-human), and an ADR should follow that decision.
• Duplicated Code with validate (the part that still stands). Declined. validate's Delivery messages and AlreadyMerged are Delivery's contract, which spec_delivery_rejects_late_retargeting_and_keeps_the_original_identity asserts (PR targets develop, not main). Accounting and the Self-merge check share one classifier, SpecPrChange::of, so the duplication left is between two scopes with different outcomes.
• SpecPr lives in src/child_run.rs. Declined. It is the payload of Kind::Ticket / Kind::BaseFix and of their hidden --spec-pr argument, which src/child_run.rs alone writes and reads. pull_request.rs consumes it as it consumes Choice. The code-navigation half of this finding is fixed.
• Test fake counts Spec PR checks. Declined. Counting checks rather than starts is what lets a_spec_pr_closed_after_an_earlier_error_is_still_the_cause close the Spec PR after an earlier error, a placement that counting starts cannot express. Each use comments the call order.
• Earlier-error test as proposed. Fixed, but with my own test. As written, the reviewer's test would close the Spec PR at the first check, before the read error, so it would pass without exercising the finding. a_spec_pr_closed_after_an_earlier_error_is_still_the_cause closes it after a wait error, failed before the fix (could not wait for #21) and passes after.

Spec

• A Ticket started before the first landing opened the Spec PR still Self-merges after the PR closes. Declined: the Day shift must decide, and the open issue Stop a Spec run when its Spec PR is closed or retargeted mid-Run #1469 covers it. I ran the reviewer's test (issue_1471_a_ticket_started_before_the_spec_pr_opened_is_not_self_merged_after_it_closes, Tickets Numbered Issue branches after a merged or closed PR #21 and CI watching, CI-fix Repair and the Repair cap #22 with parallel 2): it fails, because CI watching, CI-fix Repair and the Repair cap #22's PR is MERGED. Such a Ticket has no Spec PR to check, and it cannot learn of one after it starts. Fixing that needs one of three decisions:

  • open the Spec PR before the first Ticket starts, which contradicts GLOSSARY's "It opens as a draft once the first Ticket lands";
  • add a parent-to-child channel to Tickets already running;
  • define a rule that tells a Spec PR closed before the run (which resume_spec deliberately lets a rerun replace) from one closed during it.

  Stop a Spec run when its Spec PR is closed or retargeted mid-Run #1469 lists "Whether a Ticket run that is already in flight finishes or gets abandoned" as a policy call for the Day shift. The guide states the limit: "A Ticket started before the first landing opened the Spec PR has nothing to check and Self-merges as before."

Security review outcome

No unaddressed introduced findings.

Review coverage

Both review axes read every file in their changed-file lists. site/docs/index.html is generated output, intentionally not read: it was regenerated from docs/guide.md, node generate-docs.mjs --check exits 0, and both reviewers inspected its one-paragraph diff. Two files changed after the review's follow-up pass and no reviewer read them: src/delivery.rs (the Base fix wiring line) and docs/code-navigation.md (the Spec PR row). Later hunks in already-reviewed files (src/base_fix.rs, src/child_run.rs, src/pull_request.rs, src/run.rs, src/spec_run.rs, tests/spec_run.rs, GLOSSARY.md, docs/guide.md) were not re-reviewed either.

Closes #1471

Built with claude 2.1.312 · claude-opus-5-5 · high