fix: persist the delete-branch opt-in across a daemon restart

[Sma1lboy]

Direction

Bugs / correctness — found by code review of the daemon-owned persisted state machines (orchestrator index store codec).

Problem

Task deletion is a durable, daemon-owned background state machine (TaskDeletionState): prepare() persists a deletion block to tasks.json (phase, force, deleteBranch, requestedAt), then a separate begin() → finish() run does the physical cleanup. The whole point of persisting it is to survive a daemon restart in the queued/running window (and the error-retry path keeps it on disk indefinitely).

But the load-path coercer coerceDeletion in store-codec.ts reconstructed every field except deleteBranch. So:

1. User deletes a task with the branch-cleanup opt-in → deletion.deleteBranch: true is written to disk.
2. Daemon restarts before finish() runs → the store reloads, and deleteBranch is silently dropped (reads back undefined).
3. finish() computes deleteBranch: task.deletion.deleteBranch === true → false, and removes the worktree while keeping the branch the user explicitly asked to delete — no error, no signal.

Every other optional deletion/task field is round-tripped precisely to avoid this class of "silently forgotten on every daemon restart" bug; deleteBranch was the one gap.

Fix

One line in coerceDeletion — round-trip deleteBranch when it's a boolean, alongside the existing error passthrough:

...(typeof v.deleteBranch === "boolean" ? { deleteBranch: v.deleteBranch } : {}),

Verification

• Added a test in test/orchestrator/task-deletion.test.ts that persists a deletion with deleteBranch: true, reloads through a fresh TaskIndexStore + Orchestrator, and asserts the opt-in survives and finish() calls worktrees.remove(..., { deleteBranch: true }). Confirmed it fails without the fix (reads back undefined → deleteBranch: false) and passes with it.
• bun run typecheck and bun run lint — both green.
• test/orchestrator/task-deletion.test.ts — all 8 tests pass.

Note: the full bun run test run has one unrelated pre-existing failure — test/state/layout-migration.test.ts > … partial failure and retries — which fails on a clean origin/main checkout too, because it injects a failure via a read-only directory that the container's root user bypasses. Not related to this change.

Follow-ups (deliberately out of scope)

A separate code-review pass surfaced a second, unrelated one-line correctness bug worth its own PR: hasLiveEngineTab (src/cli/api/tab-snapshot.ts) counts a scratch task's shell tab-1 as a live engine tab because the snapshot-free floor fires unconditionally, making a scratch shell sitting at a prompt report running: true to a coordinating agent. Left for a focused follow-up to keep this PR to one slice.

---

Generated by Claude Code

[claude]

Claude finished @Sma1lboy's task in 2m 2s —— View job

---

Reviewing PR #488

• Gather context (diff, AGENTS.md/CLAUDE.md, changed files)
• Run pr-review-toolkit agents (code-reviewer, silent-failure-hunter, pr-test-analyzer, type-design-analyzer, comment-analyzer, code-simplifier)
• Check house rules (attribution, changeset bump, file-size cap, regression test, no implementation-mirroring tests)
• Synthesize and post consolidated findings

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
kobe-landing	Ignored		Aug 17, 2026 4:28pm