[JacobStephens2]

Summary

Any failure after the worktree exists now goes down the Failed run path. A draft PR the agent leaves is marked ready.

 run
   preflight
+  interrupted? → fail "interrupted" (no worktree yet)
   Worktree::create_fresh
   implement
     session::run                     # claude in its own process group
+      on SIGINT/SIGTERM → SIGTERM group, SIGKILL after 10s, "interrupted"
     push
     check PR (exists, open, targets Base branch)
+    draft → gh pr ready
+    interrupted? → "interrupted"
+  on error → failed_run::fail
+    abort unfinished merge
+    git add -A
+    no changes against origin/<base> → skip commit and push
+    commit --allow-empty "thirdshift: failed run (<reason>)" + timestamp, host
+    push
+    open PR → gh pr ready --undo, URL on stdout
+    log path on stderr, exit 1
   Drop: remove worktree, local Issue branch, plugin dir

src/
├── failed_run.rs   # FailedRun, the failure commit, push, draft demotion
├── interrupt.rs    # SIGINT/SIGTERM recorded as a flag, not fatal
├── session.rs      # polls claude, stops its process group on interrupt
└── github.rs       # + mark_ready / convert_to_draft

Tested seams: the thirdshift CLI only (tests/failed_run.rs, tests/fresh_run.rs). Tests run the real binary against real git and fake gh/claude, and SIGINT/SIGTERM are sent to the process.

Evidence

• Before: claude exits 3 with uncommitted work → stderr is only thirdshift: claude exited 3, no failure commit or issue-7 on origin, no log path. A SIGINT during the session → the test waits the full 30s (the session was not stopped). A draft PR from the agent → isDraft stays true.
• After: cargo test → 17 passed (8 failed_run, 9 fresh_run), including:

  claude exits 3            → origin issue-7: "thirdshift: failed run (claude exited 3)", "Add feature", …; wip.txt pushed
  failure commit body       → "<RFC 3339>, host <h>. Uncommitted work at the time of failure is included in this commit."
  no PR                     → "thirdshift: failed run (no PR found)", stdout empty
  PR base develop           → "(PR targets develop, not main)", PR isDraft=true, PR URL on stdout
  agent exits 1, no changes → no issue-7 on origin
  conflicting merge left    → merge aborted, failure commit on top of "Ours"
  SIGINT / SIGTERM          → session stopped in <1s, "(interrupted)", wip.txt pushed
  draft PR from agent       → isDraft=false, exit 0
  every failure             → exit 1, log path on stderr, no worktree, local branch or temp dir left
  

  cargo fmt --check and cargo clippy --all-targets -D warnings are clean.

Merge Danger

Door: two-way

It adds new behaviour on the failure path; the happy path is unchanged except that a draft PR now gets marked ready. It adds signal-hook and libc dependencies.

Blast Radius: contained

A Failed run now pushes an Issue branch and demotes the PR, which is the behaviour the spec asks for. claude now runs in its own process group, so a terminal Ctrl-C reaches it only through thirdshift.

Unaddressed findings

Standards

• Data Clumps (implement/fail share issue, worktree, base, log): two private call sites. A context struct would add ceremony before Continuations and Repairs show what a Run context really needs.
• Primitive Obsession (typed Interrupted error): the reason string is the spec'd output (interrupted). A PR is_open() was added; a full PR state enum is left for when more states matter.
• Inject the interrupt flag instead of a global: signal state is process-global by nature, and the tests drive real signals through the binary seam.
• Repeated interrupt::requested() checks: each guards a different window (before the worktree, during the session, after the session, error translation). Merging them would drop one of those windows.

Spec

• --no-verify on the failure commit: kept on purpose, because a repo hook that rejects the commit would strand the agent's work, which is the one thing the failure path exists to prevent.
• PR URL on stdout on failure: the acceptance criterion's "nothing on stdout unless a PR exists" allows it, and it keeps stdout meaning "the PR for this run" for scripts.
• SIGTERM → SIGKILL escalation and a separate process group: needed to actually "stop the session", including the processes claude spawns.
• "No changes" compares trees, not commits: this matches the spec's "no changes against the Base branch". A commit-then-revert branch has nothing worth pushing.

Closes #5

🤖 Generated with Claude Code