[JacobStephens2]

Summary

Confirmed cause: when Claude Code reaches its background wait ceiling, it stops the session's remaining work and then delivers that stop as an empty, zero-turn result (origin.kind: "task-notification"). That matches #73's 0 turns ending and real Session traces on this machine (2302-…-implement-resume.logs.jsonl). thirdshift treated the empty result as a new turn and cleared the killed review. So the session got no Resume, and the Ticket run stopped at Pull request readiness with a bare no PR found.

 Claude stream: result(N turns) → task killed → task stopped → result(0 turns, task-notification)
-  any root result: clear killed work, replace final message and turn count
+  zero-turn task-notification result: not a turn; killed work kept → Resume
+  late stop for a task a result already settled: not new killed work

 Route::publish
   push
+  stage Pull request readiness
+  open_missing_pr
+    PR exists                          → nothing
+    commits ahead of the Base branch   → "the implement session pushed <branch> without
+                                          opening a pull request; opening it in the session's place"
+                                          open draft: issue title, "Closes #<n>" (+ note)
+    neither                            → fail: "the implement session made no commits and
+                                          opened no pull request"
   write Built-with line
   mark ready → CI → Self-merge as usual

• Prevention: the thirdshift-code-review skill now says to start both reviewers in the foreground, in one message (on Claude Code, run_in_background: false). It also says never to end the turn while a reviewer is still running. Prompts were regenerated.
• Docs: the guide's Pull request readiness step describes the recovery, and its Resume section describes the ceiling. site/index.html and site/how-it-works/ say the same.
• Stopped at the ceiling, in the Command log and cause: once detected, a review stopped at the ceiling takes the existing Resume path. The Command log then says implement: background work was killed as the session ended; resuming it once. If the run later fails, the cause names the review, for example implement session ended with a background task still running (Spec review of issue-7), which was killed, and a later step failed: ….

Evidence

Execution evidence

• Before (on main, at the stream seam): cargo test --bin thirdshift a_review_stopped_at_the_wait_ceiling

  test harness::interpretation::stream::tests::claude::a_review_stopped_at_the_wait_ceiling_stays_killed_past_its_notifications_empty_result ... FAILED
  assertion `left == right` failed
    left: []
   right: ["Spec review of issue-73"]
  

  After: the same test passes. cargo test --bin thirdshift harness::interpretation: test result: ok. 100 passed; 0 failed.

• Before (Ticket run with fakes, with notification_only disabled): cargo test --test background_work a_review_stopped

  thread 'a_review_stopped_at_claudes_wait_ceiling_gets_its_session_resumed' panicked at tests/background_work.rs:191:5:
  thirdshift: no PR found
  test result: FAILED. 0 passed; 1 failed
  

  After: test a_review_stopped_at_claudes_wait_ceiling_gets_its_session_resumed ... ok. All of background_work: 6 passed; 0 failed.

• Recovery (cargo test --test pr_lifecycle an_implement_session): on main, a session that opens no PR ends with no PR found (that was the old expectation in tests/process_tree.rs and tests/codex_sessions.rs). After: 2 passed.

  • With commits, the PR is opened, gets the Built-with line, and is merged into main.
  • With no commits, the run fails with the clearer cause and gh pr create is never called.

• A review finding, run as the reviewer wrote it, failed before the follow-up fix:

  • cargo test --bin thirdshift a_task_stopped_in_the_last_turn_stays_cleared → FAILED.
  • The test passes now and is kept.

• Validation:

  • scripts/affected-tests main: site tests PASS, Console browser tests PASS.
  • Rust: cargo test --bin thirdshift -- --skip sandbox::tests:: → 1897 passed; 0 failed.
  • All integration test files were run. The only failures are sandboxed Trial, Grader and Scoring tests that report sandbox failed to start: socat is not on PATH, plus memory-cap Trial tests that need systemd scope OOM evidence. socat isn't on this machine and none of these tests touch Delivery or the Claude stream.
  • The five suites changed here pass: run_notification, process_tree, codex_sessions, pr_lifecycle, background_work.
  • cargo clippy --all-targets is clean.
  • npm run docs:check --prefix site and cargo test --bin thirdshift prompts_page pass.

Visual evidence

None. The HTML changes add one sentence each inside existing paragraphs of site/index.html and site/how-it-works/index.html, plus regenerated text in site/docs/ and site/prompts/. No markup, styles or layout changed, so no screenshots were taken.

Merge Danger

Door: two-way

Blast Radius: Delivery

• Every Built-in Ticket run's publish step now asks whether a PR exists before readiness. A Ticket run whose PR is missing now gets one opened by thirdshift instead of failing.
• On Claude Code, a stop notification at the ceiling now triggers a Resume.
• A Spec PR delivery is unaffected, because its PR is captured before publish.

Unaddressed findings

Standards

• 1. Duplicated Code with publish_steps: declined.
  • The Custom delivery step opens a draft whenever none exists, as the guide's Custom process section documents.
  • The Built-in recovery is different: it applies only after an implement session that should have opened the PR, refuses when there are no commits, and explains itself in the body.
  • Merging them would change Custom process behaviour, which An implement session that ends with background helpers still running leaves its commits pushed but no PR, failing with 'no PR found' #2231 doesn't ask for.
• 2. open_missing_pr worded for an implement session but also on the Spec PR path: declined.
  • The reviewer found it harmless: the Spec PR is captured before Delivery (src/spec_run.rs, landed), so actionable_head(false) either returns early or fails with no PR found as before.
  • The wording the issue asks for ("the implement session made no commits and opened no pull request") is kept.

Spec

• 3. Run notification on a successful run doesn't mention the ceiling stop: declined, for the Day shift to decide (Should a successful Run notification say a session's background work was stopped and resumed? #2238).
  • AC3 asks only for the Command log and, on failure, the cause. Both are delivered and tested in a_run_that_fails_after_a_review_stopped_at_the_ceiling_names_the_review_in_its_cause and tests/run_notification.rs.
  • Desired behaviour 2 also names the Run notification. A success-time line would need new plumbing from sessions into the Run notification, which goes beyond the acceptance criterion.

Review coverage: the reviewers read every changed file at c2663ce7, except two generated files excluded with regeneration evidence: site/docs/index.html (from docs/guide.md, npm run docs:check --prefix site) and site/prompts/index.html (from skills/, cargo test --bin thirdshift prompts_page). The follow-up commits changed some files after the review:

• Never read by either reviewer: tests/codex_sessions.rs, tests/run_notification.rs, tests/process_tree.rs (expectation updates only).
• Only partly read, in its earlier version: src/delivery/tests.rs.
• Reviewed in their earlier form: the follow-up edits to src/harness/interpretation/stream.rs, src/delivery/test_support.rs, src/skills/tests.rs and skills/thirdshift-code-review/SKILL.md.

Closes #2231

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

Security review outcome

No unaddressed introduced findings.