[JacobStephens2]

Summary

A Pickup run asked for a Run notification now makes the notification's checks before any other work, and sends the one the Spec run or Run it dispatched would have sent by hand.

 pickup
   User config
+  Run notification's checks: an address and a Resend API key   # exit 1 on every pass, one that would be skipped included
   preflight checks, the lock                                   # skipped: no notification
   search for the lowest-numbered Ready issue                   # skipped: no notification
-  Run notification's checks, and the issue's title read from GitHub
+  whether the issue is a Spec, then "taking Ready issue #<n> ..."
+  the checked notification takes the issue and its title from the listing
   dispatch: a Spec run or a Run, which sends none of its own
   send the one notification, after the outcome is printed

src/
├── notification.rs   # Checked: the checks every notification shares; PickupNotification waits for the issue taken
├── pickup.rs         # Taken carries the issue's title; the Spec check comes before the "taking" line
└── main.rs           # pickup: checks first, a skip returns before anything is sent; help text
README.md             # "One Run notification for the issue taken" under Pickup runs; Run notifications points to it
CONTEXT.md            # Run notification: the dispatched run sends none of its own
tests/pickup_run.rs   # end-to-end cases with the Resend stand-in

Seam under test, the one the Spec names: the real thirdshift binary in a scenario against the fake gh, the fake claude and the Resend stand-in.

Evidence

• Before: on issue-212, with the two tests written first:

  a_pass_that_would_be_skipped_for_having_no_ready_issue_still_stops_on_the_notifications_checks ... FAILED
    stderr: thirdshift: no Ready issue on acme/widgets      (exit 0, wanted 1)
  asked_for_a_notification_with_no_address_known_the_pass_stops_before_any_work ... FAILED
    gh calls before the check: issue list, pr list, api graphql   (wanted none)
  

  After: cargo test --test pickup_run: 43 passed; 0 failed. Full suite, cargo test: all 30 test binaries ok, 0 failed. cargo clippy --all-targets and cargo fmt --check clean.

• Each acceptance criterion and the test that covers it:

  Criterion	Test in tests/pickup_run.rs
  One notification for each ending	the_notification_of_a_run_left_ready_for_review_is_that_runs, ..._of_a_merge_run_says_it_merged, ..._of_a_failed_run_has_its_pull_request_and_its_cause, ..._of_an_interrupted_run_says_it_was_interrupted
  A Run's subject and body, or a Spec run's with a line per Ticket	the four above, the_notification_of_a_spec_run_is_that_spec_runs_with_a_line_per_ticket, the_notification_of_a_failed_spec_run_has_each_tickets_outcome
  email <address> over email.to; no-email over email.always	a_pass_that_took_an_issue_sends_one_notification_when_the_command_or_the_user_config_asks
  The dispatched run sends none of its own	the same table's email.always rows and the Spec run test: exactly one request
  A skipped pass sends none	a_pass_skipped_for_having_no_ready_issue_sends_no_notification, a_pass_skipped_for_the_lock_sends_no_notification
  No address or no key exits 1 before any label change or session	asked_for_a_notification_with_no_address_known_..., ..._with_no_resend_api_key_..., the two a_pass_that_would_be_skipped_..._still_stops_on_the_notifications_checks, the_notifications_checks_come_before_the_checks_on_the_clone
  A failed send is a warning: line	a_notification_that_cant_be_sent_is_a_warning_that_changes_neither_the_exit_code_nor_stdout

Unaddressed findings

Standards

• resend_env and the_one_notification are copies of the ones in tests/architect_run.rs and tests/spec_run.rs. Skipped: each test file keeps its own Resend helpers today (KEY and ACCEPTED too), and moving them to tests/support/resend.rs means editing two test files this Ticket doesn't otherwise touch.
• The no-Ready-issue checks test re-spells assert_stopped_by_preflight. Skipped: that helper also asserts Numbered Issue branches after a merged or closed PR #7 is still ready-for-agent, and this scenario has no Numbered Issue branches after a merged or closed PR #7.
• Issue and title travel together through ListedIssue, Taken and of_taken. Skipped: two fields, and passing Taken whole would make the notification depend on the Pickup run's module.
• One rule is restated in the help, the README, CONTEXT.md and doc comments. Skipped: that is how this repo documents every command, and the help test pins the wording.
• "Before any other work" comes after the User config is read and the interrupt handler is installed. Skipped: an Architect run has the same order and the same wording, and the Spec puts the User config first.
• "As it would by hand" is loose: by hand the title can be missing, here it always comes from the listing. Skipped: the subject is the same whenever GitHub answers, and the listing saves a call that could fail.

Spec

• No test that a pass skipped for the Claim limit sends none. Not possible yet: the Claim limit is The Claim limit, and taking in-progress off closed issues #284, which is not on issue-212. Every skip returns from pickup before anything is sent, so a new skip reason sends none without further work; The Claim limit, and taking in-progress off closed issues #284 should add the case to its own tests.
• The body gives how long the whole pass took, the search included, not the dispatched run alone. Skipped: the search takes seconds, an Architect run's notification times the whole run the same way, and the README says so.

Addressed from the review: one Checked core shared by the three notifications in place of a third copy of the checks; stale doc comments on RunNotification; Taken.title documented; the reused notification name in pickup; the lock tests' shared setup; a test that the notification's checks precede the clone's; and the Spec check moved ahead of the taking Ready issue line, so a pass that fails there has not said it took an issue.

Merge Danger

Door: two-way

Reverting restores the checks after the take. No stored state, label or config key changes.

Blast Radius: Pickup-runs

A scheduled thirdshift pickup with email.always set but no address or Resend key now exits 1 on every pass, where before it exited 0 on passes that were skipped. That is the Ticket's intent, and it shows in the cron log. Runs and Architect runs go through the refactored Checked core; their notification tests pass unchanged.

Closes #285

🤖 Generated with Claude Code