From 816f247d90f068061318868b264d051dfd8890c6 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 23 Jul 2026 17:09:55 +0200 Subject: [PATCH] fix(worker): keep planning-chain cascade moving past an Idle middle link OnChildFinishedAsync ignored CancelAsync's result. If a chain successor sat in a non-cancellable state (e.g. parked to Idle out of band) when its predecessor failed/was cancelled, CancelAsync was a silent no-op and the cascade stopped there, leaving the rest of the chain Queued+blocked forever. Now it walks past any link CancelAsync can't touch and keeps cancelling downstream. --- .../Planning/PlanningChainCoordinator.cs | 42 +++++++++++++------ .../Planning/PlanningChainCoordinatorTests.cs | 31 ++++++++++++++ 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs index 12754112..cf1f19f8 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs @@ -92,14 +92,7 @@ public sealed class PlanningChainCoordinator public async Task OnChildFinishedAsync( string childTaskId, TaskStatus finalStatus, CancellationToken ct = default) { - await using var ctx = await _dbFactory.CreateDbContextAsync(ct); - // The successor is whichever sibling explicitly blocks on this child. - var nextId = await ctx.Tasks - .AsNoTracking() - .Where(t => t.BlockedByTaskId == childTaskId) - .OrderBy(t => t.SortOrder).ThenBy(t => t.CreatedAt) - .Select(t => t.Id) - .FirstOrDefaultAsync(ct); + var nextId = await FindSuccessorAsync(childTaskId, ct); if (nextId is null) return null; if (finalStatus == TaskStatus.Done) @@ -108,10 +101,33 @@ public sealed class PlanningChainCoordinator return nextId; } - // Child failed or was cancelled: cancel the immediate successor so the chain - // is not left wedged. CancelAsync triggers OnChildTerminalAsync → OnChildFinishedAsync - // for that successor, cascading cancellation through the rest of the chain. - await _state().CancelAsync(nextId, DateTime.UtcNow, ct); - return null; + // Child failed or was cancelled: cancel the immediate successor so the chain is + // not left wedged. If it's cancellable, CancelAsync's own OnChildTerminalAsync + // callback recurses into this method for its successor, cascading cancellation + // through the rest of the chain. If it's not (e.g. it was parked back to Idle + // out of band), CancelAsync is a no-op and nothing will ever call back for it — + // keep walking the chain ourselves so the tail isn't left wedged forever. + var predecessorId = nextId; + while (true) + { + var result = await _state().CancelAsync(predecessorId, DateTime.UtcNow, ct); + if (result.Ok) return null; + + var following = await FindSuccessorAsync(predecessorId, ct); + if (following is null) return null; + predecessorId = following; + } + } + + // The successor is whichever sibling explicitly blocks on this task. + private async Task FindSuccessorAsync(string taskId, CancellationToken ct) + { + await using var ctx = await _dbFactory.CreateDbContextAsync(ct); + return await ctx.Tasks + .AsNoTracking() + .Where(t => t.BlockedByTaskId == taskId) + .OrderBy(t => t.SortOrder).ThenBy(t => t.CreatedAt) + .Select(t => t.Id) + .FirstOrDefaultAsync(ct); } } diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs index bd568e64..dbde5531 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs @@ -178,6 +178,37 @@ public sealed class PlanningChainCoordinatorTests : IDisposable Assert.Equal(TaskStatus.Cancelled, kids[3].Status); } + [Fact] + public async Task OnChildFailed_MidChain_IdleSuccessor_DoesNotWedgeTail() + { + // Chain: c0 → c1 → c2 → c3. c1 is parked back to Idle (e.g. a manual reset) + // while still wired into the chain (BlockedByTaskId = c0). c0 then fails: + // CancelAsync(c1) is a no-op because Idle isn't a cancellable state, so the + // cascade must keep walking past c1 instead of leaving c2/c3 wedged forever. + await SeedPlanningFamilyAsync("P", 4); + await _sut.SetupChainAsync("P", enqueue: true, default); + + await using (var ctx = _factory.CreateDbContext()) + { + var c0 = await ctx.Tasks.FirstAsync(t => t.Id == "P-c0"); + c0.Status = TaskStatus.Failed; + var c1 = await ctx.Tasks.FirstAsync(t => t.Id == "P-c1"); + c1.Status = TaskStatus.Idle; + await ctx.SaveChangesAsync(); + } + + var advanced = await _sut.OnChildFinishedAsync("P-c0", TaskStatus.Failed, default); + + Assert.Null(advanced); + var kids = await GetChildrenAsync("P"); + Assert.Equal(TaskStatus.Failed, kids[0].Status); + // c1 was already parked Idle; the coordinator must not resurrect it. + Assert.Equal(TaskStatus.Idle, kids[1].Status); + // c2/c3 must not stay wedged as Queued+blocked forever. + Assert.Equal(TaskStatus.Cancelled, kids[2].Status); + Assert.Equal(TaskStatus.Cancelled, kids[3].Status); + } + [Fact] public async Task OnChildDone_LastChild_ReturnsNull() {