From 58741c2bd6896eead4eb1aceb50edb6f85d80413 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 13:16:58 +0200 Subject: [PATCH] fix(worker): advance planning chains and parents on stale-Running recovery RecoverStaleRunningAsync bulk-flipped Running rows to Failed via raw ExecuteUpdate, skipping the chain/parent side effects every other terminal transition triggers. After a worker crash mid-run of a planning/improvement child, the chain successor's BlockedByTaskId was never cleared and a WaitingForChildren parent could wedge forever with no event left to re-check it. Now each recovered task runs the same OnChildTerminalAsync side effects (chain advance + parent advance) as FailAsync, best-effort. --- src/ClaudeDo.Worker/State/TaskStateService.cs | 37 +++++++++++++++---- .../State/TaskStateServiceTests.cs | 19 ++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/ClaudeDo.Worker/State/TaskStateService.cs b/src/ClaudeDo.Worker/State/TaskStateService.cs index c04fe777..a918b15c 100644 --- a/src/ClaudeDo.Worker/State/TaskStateService.cs +++ b/src/ClaudeDo.Worker/State/TaskStateService.cs @@ -325,7 +325,9 @@ public sealed class TaskStateService : ITaskStateService // Unconditional status write — bypasses transition rules. Used by the UI's // "set status freely" affordance; intentionally no guards (caller may strand - // the runner if used while a task is executing). + // 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 ForceSetStatusAsync(string taskId, TaskStatus status, CancellationToken ct) { await using var ctx = await _dbFactory.CreateDbContextAsync(ct); @@ -418,13 +420,32 @@ public sealed class TaskStateService : ITaskStateService { var resultText = "[stale] " + reason; var now = DateTime.UtcNow; - await using var ctx = await _dbFactory.CreateDbContextAsync(ct); - return await ctx.Tasks - .Where(t => t.Status == TaskStatus.Running) - .ExecuteUpdateAsync(s => s - .SetProperty(t => t.Status, TaskStatus.Failed) - .SetProperty(t => t.FinishedAt, now) - .SetProperty(t => t.Result, resultText), ct); + List recoveredIds; + int affected; + await using (var ctx = await _dbFactory.CreateDbContextAsync(ct)) + { + recoveredIds = await ctx.Tasks + .Where(t => t.Status == TaskStatus.Running) + .Select(t => t.Id) + .ToListAsync(ct); + + affected = await ctx.Tasks + .Where(t => t.Status == TaskStatus.Running) + .ExecuteUpdateAsync(s => s + .SetProperty(t => t.Status, TaskStatus.Failed) + .SetProperty(t => t.FinishedAt, now) + .SetProperty(t => t.Result, resultText), ct); + } + + // A recovered task may have been a planning/improvement chain child or the last + // non-terminal child of a WaitingForChildren parent. The bulk flip above skips the + // usual terminal-transition side effects, so without this a crash mid-run would + // leave the chain successor blocked forever and the parent wedged in + // WaitingForChildren with nothing left to re-check it. + foreach (var taskId in recoveredIds) + await OnChildTerminalAsync(taskId, TaskStatus.Failed); + + return affected; } // A subtask is "draft" only while its planning parent has an open (Active) session. diff --git a/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs b/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs index a2ce8a7b..354e61b7 100644 --- a/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs @@ -456,6 +456,25 @@ public sealed class TaskStateServiceTests : IDisposable Assert.StartsWith("[stale] ", t.Result); } + [Fact] + public async Task RecoverStaleRunningAsync_ResolvesBlockedSuccessor_AndAdvancesParent() + { + var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized); + var c0 = await SeedTaskAsync(TaskStatus.Running, parentId: parent, sortOrder: 0); + var c1 = await SeedTaskAsync(TaskStatus.Queued, parentId: parent, sortOrder: 1, blockedBy: c0); + + var count = await _sut.RecoverStaleRunningAsync("worker restart", default); + + Assert.Equal(1, count); + Assert.Equal(TaskStatus.Failed, await GetStatusAsync(c0)); + // c1 was blocked on the crashed task; the chain coordinator resolves it (cancels it, + // since the predecessor didn't finish successfully) instead of leaving it Queued with + // a dangling BlockedByTaskId that the picker would skip forever. + Assert.Equal(TaskStatus.Cancelled, await GetStatusAsync(c1)); + // Both children are now terminal, so the WaitingForChildren parent advances. + Assert.Equal(TaskStatus.WaitingForReview, await GetStatusAsync(parent)); + } + // ─── Child terminal → chain advance ─────────────────────────────────── [Fact]