refactor(worker): drop ForceSetStatusAsync, route every status write through a guarded transition

Mark-done now fires OnChildTerminalAsync, so ticking a chain child or the last open child
off by hand unblocks the successor and advances the parent. The hub's Mark-as menu and
update_task_status map onto MarkDone/Cancel(retire)/Enqueue/Reset; revert_merge uses the
new ReopenForReviewAsync (Done|WaitingForReview -> WaitingForReview).

docs/explore-notes/external-mcp.md

@@ -109,13 +109,13 @@ Description/Result) in `tasksFull` instead (`tasks` null). Filtering by `created
 happens before the lean/full projection either way.
 
 **`UpdateTaskStatus`** accepts `Idle` / `Queued` / `Cancelled` / `Done` only.
-- `Cancelled` goes through `TaskStateService.CancelAsync(..., allowFromIdle: true)` — the
+- `Cancelled` goes through `TaskStateService.CancelAsync(..., retire: true)` — like the UI's
-  **only** caller that opts into cancelling from `Idle`. `PlanningChainCoordinator` relies on
+  "Mark as cancelled", it opts into cancelling from `Idle`/`Failed`. `PlanningChainCoordinator`
-  `Idle` staying a no-op there by default, because a child parked back to `Idle` mid-chain is
+  relies on `Idle` staying a no-op there by default, because a child parked back to `Idle`
-  a manual opt-out signal.
+  mid-chain is a manual opt-out signal.
-- `Done` goes through `TaskStateService.ForceSetStatusAsync` (the same unconditional write the
+- `Done` goes through `TaskStateService.MarkDoneAsync` (the done toggle, so a chain successor
-  UI's "set status freely" affordance uses) but is **refused** for a task with an active
+  unblocks and the parent advances) but is **refused** for a task with an active worktree,
-  worktree, since that would skip `review_task`'s merge.
+  since that would skip `review_task`'s merge.
 
 **`AddTask`** — always creates the task; also returns `possibleDuplicates` (up to 3, id/title/status
 only, no descriptions) — open (non-terminal) tasks in the *same list* whose normalized title

src/ClaudeDo.Worker/CLAUDE.md

@@ -76,23 +76,29 @@ not conflated.
 Allowed transitions (enforced by `TaskStateService`):
 
 ```
-Idle               → Queued | Running (RunNow) | Cancelled (external update_task_status only, allowFromIdle: true)
+Idle               → Queued | Running (RunNow) | WaitingForReview (interactive submit)
+                   | Cancelled (Mark as / update_task_status, retire: true)
 Queued             → Running | Cancelled | Idle | Failed (OverrideSlotService preflight gap)
 Running            → WaitingForReview (standalone success, no children)
                    | WaitingForChildren (parent with pending children)
                    | Done (planning/improvement child success) | Failed | Cancelled
 WaitingForChildren → WaitingForReview (all children terminal) | Cancelled
 WaitingForReview   → Done (approve) | Queued (reject-rerun, +feedback) | Idle (reject-park) | Cancelled
-Done               → Idle (re-run)
+Done               → Idle (re-run) | WaitingForReview (revert_merge)
-Failed             → Idle | Queued
+Failed             → Idle | Queued | WaitingForReview (interactive submit) | Cancelled (retire)
 Cancelled          → Idle | Queued
 
-any except Running → Done (manual done toggle, SetTaskDone)
+any except Running → Done (manual done toggle, SetTaskDone) | Queued (Enqueue) | Idle (Reset)
 ```
 
+There is no unguarded status write: the hub's "Mark as" menu and `update_task_status` map every
+status onto one of the transitions above (`Running`/`Waiting*` can't be set by hand).
+
 The manual done toggle is the one cross-cutting transition — it ticks a task off from **any**
 status except `Running`, skipping the merge on purpose (a worktree-less list-handler task has
-nothing to merge; a worktree task's branch just stays Active for the worktrees overview).
+nothing to merge; a worktree task's branch just stays Active for the worktrees overview). Like
+every Done transition it fires `OnChildTerminalAsync`, so a chain successor unblocks and a
+`WaitingForChildren` parent advances.
 
 **Unified parent model.** Every parent — planning *or* improvement — flows
 `… → WaitingForChildren → WaitingForReview → Done` via the single `TryAdvanceParentAsync`.

src/ClaudeDo.Worker/External/ExternalMcpService.cs

@@ -594,7 +594,7 @@ public sealed class ExternalMcpService
                 break;
 
             case TaskStatus.Cancelled:
-                var cancelResult = await _state.CancelAsync(taskId, DateTime.UtcNow, cancellationToken, allowFromIdle: true);
+                var cancelResult = await _state.CancelAsync(taskId, DateTime.UtcNow, cancellationToken, retire: true);
                 if (!cancelResult.Ok)
                     throw new InvalidOperationException(cancelResult.Reason ?? "Cannot cancel task.");
                 break;
@@ -611,7 +611,7 @@ public sealed class ExternalMcpService
                             "Task has an active worktree — use review_task to approve and merge instead.");
                 }
 
-                var doneResult = await _state.ForceSetStatusAsync(taskId, TaskStatus.Done, cancellationToken);
+                var doneResult = await _state.MarkDoneAsync(taskId, DateTime.UtcNow, cancellationToken);
                 if (!doneResult.Ok)
                     throw new InvalidOperationException(doneResult.Reason ?? "Cannot set task to Done.");
                 break;

src/ClaudeDo.Worker/Hub/WorkerHub.cs

@@ -693,14 +693,15 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
         if (current.Status == TaskStatus.Running)
             throw new HubException("Cannot change the status of a running task. Cancel it first.");
 
-        // Queueing goes through the gated transition so draft subtasks can't be queued;
+        // Every status routes through its guarded transition; the rest (Running, the Waiting*
-        // Idle goes through ResetToIdleAsync so StartedAt/FinishedAt/Result are cleared;
+        // states) are only reachable through the lifecycle itself.
-        // other statuses keep the unconditional "set status freely" affordance.
         var result = parsed switch
         {
             TaskStatus.Queued => await _state.EnqueueAsync(taskId, Context.ConnectionAborted),
             TaskStatus.Idle => await _state.ResetToIdleAsync(taskId, Context.ConnectionAborted),
-            _ => await _state.ForceSetStatusAsync(taskId, parsed, Context.ConnectionAborted),
+            TaskStatus.Done => await _state.MarkDoneAsync(taskId, DateTime.UtcNow, Context.ConnectionAborted),
+            TaskStatus.Cancelled => await _state.CancelAsync(taskId, DateTime.UtcNow, Context.ConnectionAborted, retire: true),
+            _ => throw new HubException($"status {parsed} cannot be set by hand"),
         };
         if (!result.Ok) throw new HubException(result.Reason ?? "set status failed");
         return new SetTaskStatusResultDto(

src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs

@@ -765,7 +765,7 @@ public sealed class TaskMergeService
             await new WorktreeRepository(ctx).SetStateAsync(taskId, WorktreeState.Kept, ct);
         }
         await _broadcaster.WorktreeUpdated(taskId);
-        await _state.ForceSetStatusAsync(taskId, TaskStatus.WaitingForReview, ct);
+        await _state.ReopenForReviewAsync(taskId, ct);
 
         _logger.LogInformation(
             "Reverted merge of task {TaskId} (merge commit {MergeSha}) via revert commit {RevertSha}",

src/ClaudeDo.Worker/State/Interfaces/ITaskStateService.cs

@@ -11,7 +11,7 @@ public interface ITaskStateService
     Task<TransitionResult> FailAsync(
         string taskId, DateTime finishedAt, string? error, CancellationToken ct,
         string failureReason = "error", int? turnsUsed = null, int? maxTurns = null);
-    Task<TransitionResult> CancelAsync(string taskId, DateTime finishedAt, CancellationToken ct, bool allowFromIdle = false);
+    Task<TransitionResult> CancelAsync(string taskId, DateTime finishedAt, CancellationToken ct, bool retire = false);
     Task<TransitionResult> ResetToIdleAsync(string taskId, CancellationToken ct);
 
     Task<TransitionResult> ApproveReviewAsync(string taskId, CancellationToken ct);
@@ -19,9 +19,9 @@ public interface ITaskStateService
     Task<TransitionResult> RejectToIdleAsync(string taskId, CancellationToken ct);
     Task<TransitionResult> ClearReviewFeedbackAsync(string taskId, CancellationToken ct);
 
-    Task<TransitionResult> ForceSetStatusAsync(string taskId, ClaudeDo.Data.Models.TaskStatus status, CancellationToken ct);
+    Task<TransitionResult> ReopenForReviewAsync(string taskId, CancellationToken ct);
 
-    // Guarded "manual done toggle" affordance (task list checkbox): unlike ForceSetStatusAsync,
+    // Guarded "manual done toggle" affordance (task list checkbox):
     // these check the expected starting status in the same ExecuteUpdateAsync filter so a
     // concurrent picker claim (Idle/Done -> Running) is not silently overwritten.
     Task<TransitionResult> MarkDoneAsync(string taskId, DateTime finishedAt, CancellationToken ct);

src/ClaudeDo.Worker/State/TaskStateService.cs

@@ -297,13 +297,13 @@ public sealed class TaskStateService : ITaskStateService
         return new TransitionResult(true, null);
     }
 
-    // allowFromIdle: only the external update_task_status(Cancelled) path sets this — it lets
+    // retire: only the user-facing "mark cancelled" paths (update_task_status, the row's
-    // an Idle task be retired without deleting it. Every other caller (hub CancelReview,
+    // "Mark as" menu) set this — it lets an Idle or Failed task be retired without deleting it.
-    // PlanningChainCoordinator's chain-walk, batch cancel) relies on Idle staying a no-op here;
+    // Every other caller (hub CancelReview, PlanningChainCoordinator's chain-walk, batch cancel)
-    // PlanningChainCoordinator specifically uses "parked back to Idle" as a deliberate opt-out
+    // relies on Idle staying a no-op here; PlanningChainCoordinator specifically uses "parked
-    // signal, so do not flip this default.
+    // back to Idle" as a deliberate opt-out signal, so do not flip this default.
     public async Task<TransitionResult> CancelAsync(
-        string taskId, DateTime finishedAt, CancellationToken ct, bool allowFromIdle = false)
+        string taskId, DateTime finishedAt, CancellationToken ct, bool retire = false)
     {
         // A unit merge drains its children's worktrees onto the target branch while the parent
         // sits in WaitingForReview for the whole (potentially minutes-long) drain. Cancelling out
@@ -321,7 +321,7 @@ public sealed class TaskStateService : ITaskStateService
                     (t.Status == TaskStatus.Running || t.Status == TaskStatus.Queued
                      || t.Status == TaskStatus.WaitingForReview
                      || t.Status == TaskStatus.WaitingForChildren
-                     || (allowFromIdle && t.Status == TaskStatus.Idle)))
+                     || (retire && (t.Status == TaskStatus.Idle || t.Status == TaskStatus.Failed))))
                 .ExecuteUpdateAsync(s => s
                     .SetProperty(t => t.Status, TaskStatus.Cancelled)
                     .SetProperty(t => t.FinishedAt, finishedAt), ct);
@@ -381,35 +381,26 @@ public sealed class TaskStateService : ITaskStateService
         return new TransitionResult(true, null);
     }
 
-    // Unconditional status write — bypasses transition rules. Used by the UI's
+    /// <summary>Done/WaitingForReview → WaitingForReview after a reverted merge, so the task can be re-approved.</summary>
-    // "set status freely" affordance; intentionally no guards (caller may strand
+    public async Task<TransitionResult> ReopenForReviewAsync(string taskId, CancellationToken ct)
-    // the runner if used while a task is executing). It also bypasses chain/parent
-    // advancement — forcing a chain child or a WaitingForChildren parent's child to a
-    // terminal status here does not unblock its successor or re-check the parent.
-    public async Task<TransitionResult> ForceSetStatusAsync(string taskId, TaskStatus status, CancellationToken ct)
     {
         await using var ctx = await _dbFactory.CreateDbContextAsync(ct);
-        var affected = status == TaskStatus.Done
+        var affected = await ctx.Tasks
-            ? await ctx.Tasks
+            .Where(t => t.Id == taskId &&
-                .Where(t => t.Id == taskId)
+                (t.Status == TaskStatus.Done || t.Status == TaskStatus.WaitingForReview))
-                .ExecuteUpdateAsync(s => s
+            .ExecuteUpdateAsync(s => s.SetProperty(t => t.Status, TaskStatus.WaitingForReview), ct);
-                    .SetProperty(t => t.Status, status)
-                    .SetProperty(t => t.FinishedAt, DateTime.UtcNow), ct)
-            : await ctx.Tasks
-                .Where(t => t.Id == taskId)
-                .ExecuteUpdateAsync(s => s.SetProperty(t => t.Status, status), ct);
 
         if (affected == 0)
-            return new TransitionResult(false, "Task not found.");
+            return new TransitionResult(false, "Task is not Done or waiting for review; cannot reopen.");
 
-        if (status == TaskStatus.Queued) _waker.Wake();
         await NotifyAsync(taskId);
         return new TransitionResult(true, null);
     }
 
     public async Task<TransitionResult> MarkDoneAsync(string taskId, DateTime finishedAt, CancellationToken ct)
     {
-        await using var ctx = await _dbFactory.CreateDbContextAsync(ct);
+        await using (var ctx = await _dbFactory.CreateDbContextAsync(ct))
+        {
             // The done toggle is a manual "tick it off" affordance and works from every status
             // except Running (a live run keeps writing the row, so the picker/runner would race).
             // WaitingForReview is deliberately included: ticking a finished task done skips the
@@ -423,7 +414,9 @@ public sealed class TaskStateService : ITaskStateService
 
             if (affected == 0)
                 return new TransitionResult(false, "Task is running or no longer exists; cannot mark done.");
+        }
 
+        await OnChildTerminalAsync(taskId, TaskStatus.Done);
         await NotifyAsync(taskId);
         return new TransitionResult(true, null);
     }

tests/ClaudeDo.Worker.Tests/Planning/PlanningMergeOrchestratorTests.cs

@@ -895,12 +895,12 @@ file sealed class ApproveObservingTaskStateService : ITaskStateService
     public Task<TransitionResult> SubmitForChildrenAsync(string taskId, DateTime finishedAt, string? result, CancellationToken ct) => _inner.SubmitForChildrenAsync(taskId, finishedAt, result, ct);
     public Task<TransitionResult> FailAsync(string taskId, DateTime finishedAt, string? error, CancellationToken ct, string failureReason = "error", int? turnsUsed = null, int? maxTurns = null)
         => _inner.FailAsync(taskId, finishedAt, error, ct, failureReason, turnsUsed, maxTurns);
-    public Task<TransitionResult> CancelAsync(string taskId, DateTime finishedAt, CancellationToken ct, bool allowFromIdle = false) => _inner.CancelAsync(taskId, finishedAt, ct, allowFromIdle);
+    public Task<TransitionResult> CancelAsync(string taskId, DateTime finishedAt, CancellationToken ct, bool retire = false) => _inner.CancelAsync(taskId, finishedAt, ct, retire);
     public Task<TransitionResult> ResetToIdleAsync(string taskId, CancellationToken ct) => _inner.ResetToIdleAsync(taskId, ct);
     public Task<TransitionResult> RejectToQueueAsync(string taskId, string feedback, CancellationToken ct) => _inner.RejectToQueueAsync(taskId, feedback, ct);
     public Task<TransitionResult> RejectToIdleAsync(string taskId, CancellationToken ct) => _inner.RejectToIdleAsync(taskId, ct);
     public Task<TransitionResult> ClearReviewFeedbackAsync(string taskId, CancellationToken ct) => _inner.ClearReviewFeedbackAsync(taskId, ct);
-    public Task<TransitionResult> ForceSetStatusAsync(string taskId, TaskStatus status, CancellationToken ct) => _inner.ForceSetStatusAsync(taskId, status, ct);
+    public Task<TransitionResult> ReopenForReviewAsync(string taskId, CancellationToken ct) => _inner.ReopenForReviewAsync(taskId, ct);
     public Task<TransitionResult> MarkDoneAsync(string taskId, DateTime finishedAt, CancellationToken ct) => _inner.MarkDoneAsync(taskId, finishedAt, ct);
     public Task<TransitionResult> UnmarkDoneAsync(string taskId, CancellationToken ct) => _inner.UnmarkDoneAsync(taskId, ct);
     public Task<TransitionResult> DequeueToIdleAsync(string taskId, CancellationToken ct) => _inner.DequeueToIdleAsync(taskId, ct);

tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs

@@ -534,6 +534,58 @@ public sealed class TaskStateServiceTests : IDisposable
         Assert.Equal(TaskStatus.Done, await GetStatusAsync(id));
     }
 
+    [Fact]
+    public async Task MarkDoneAsync_OnChainChild_UnblocksSuccessor()
+    {
+        var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized);
+        var c0 = await SeedTaskAsync(TaskStatus.Queued, parentId: parent, sortOrder: 0);
+        var c1 = await SeedTaskAsync(TaskStatus.Queued, parentId: parent, sortOrder: 1, blockedBy: c0);
+
+        await _sut.MarkDoneAsync(c0, DateTime.UtcNow, default);
+
+        Assert.Null((await GetTaskAsync(c1)).BlockedByTaskId);
+    }
+
+    [Fact]
+    public async Task MarkDoneAsync_OnLastOpenChild_AdvancesParentToReview()
+    {
+        var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized);
+        await SeedTaskAsync(TaskStatus.Done, parentId: parent);
+        var open = await SeedTaskAsync(TaskStatus.Failed, parentId: parent);
+
+        await _sut.MarkDoneAsync(open, DateTime.UtcNow, default);
+
+        Assert.Equal(TaskStatus.WaitingForReview, await GetStatusAsync(parent));
+    }
+
+    [Theory]
+    [InlineData(TaskStatus.Done, true)]
+    [InlineData(TaskStatus.WaitingForReview, true)]
+    [InlineData(TaskStatus.Idle, false)]
+    [InlineData(TaskStatus.Running, false)]
+    public async Task ReopenForReviewAsync_OnlyFromDoneOrReview(TaskStatus from, bool ok)
+    {
+        var id = await SeedTaskAsync(from);
+
+        var result = await _sut.ReopenForReviewAsync(id, default);
+
+        Assert.Equal(ok, result.Ok);
+        Assert.Equal(ok ? TaskStatus.WaitingForReview : from, await GetStatusAsync(id));
+    }
+
+    [Theory]
+    [InlineData(TaskStatus.Idle)]
+    [InlineData(TaskStatus.Failed)]
+    public async Task CancelAsync_Retire_AllowsIdleAndFailed(TaskStatus from)
+    {
+        var id = await SeedTaskAsync(from);
+
+        var result = await _sut.CancelAsync(id, DateTime.UtcNow, default, retire: true);
+
+        Assert.True(result.Ok);
+        Assert.Equal(TaskStatus.Cancelled, await GetStatusAsync(id));
+    }
+
     [Fact]
     public async Task UnmarkDoneAsync_FromDone_TransitionsToIdle()
     {