diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index 9d483be7..6e4dfc48 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -50,7 +50,7 @@ subfolder within their area; the namespace stays the area namespace. ## Architecture - **Program.cs** — loads config, inits schema, registers DI, configures SignalR on `/hub`, binds to `127.0.0.1:47821` -- **TaskStateService** — the **only** component that writes `Status`, `PlanningPhase`, `BlockedByTaskId`, and the `task_dependencies` edges. All transitions return a `TransitionResult` (no exceptions on invalid moves). Wakes the queue and advances the planning chain on child terminal transitions. Every transition broadcasts via a private `NotifyAsync(taskId)` — never call `_broadcaster.TaskUpdated` directly from this class, `NotifyAsync` also drives `TicketStatusSync.SyncAsync` (see Tickets below) and a direct call skips that transition's ticket write-back. `SetDependsOnAsync` replaces the task's whole dependency **set** (empty list clears it) and rejects a self-reference, an unknown dependency id, or any link that would create a cycle — a BFS over the whole `task_dependencies` edge set from every proposed predecessor, so a multi-edge loop (A→B→C→A) is caught too, and one bad edge leaves the entire set unwritten. +- **TaskStateService** — the **only** component that writes `Status`, `PlanningPhase`, `BlockedByTaskId`, and the `task_dependencies` edges. All transitions return a `TransitionResult` (no exceptions on invalid moves). Wakes the queue and advances the planning chain on child terminal transitions. `SubmitForChildrenAsync` (Running → WaitingForChildren) also chains and immediately queues a normal parent's own Idle/Queued children via `PlanningChainCoordinator.SetupChainAsync(enqueue: true)` — unlike a planning parent (`PlanningPhase != None`), whose children stay untouched here and wait for the user's explicit "Queue plan" (`QueuePlanAsync`); a child already `Running` makes `SetupChainAsync` throw, which is caught and logged rather than failing the parent's own transition. Every transition broadcasts via a private `NotifyAsync(taskId)` — never call `_broadcaster.TaskUpdated` directly from this class, `NotifyAsync` also drives `TicketStatusSync.SyncAsync` (see Tickets below) and a direct call skips that transition's ticket write-back. `SetDependsOnAsync` replaces the task's whole dependency **set** (empty list clears it) and rejects a self-reference, an unknown dependency id, or any link that would create a cycle — a BFS over the whole `task_dependencies` edge set from every proposed predecessor, so a multi-edge loop (A→B→C→A) is caught too, and one bad edge leaves the entire set unwritten. - **IQueueWaker / IQueuePicker / QueueService** — waker is a singleton `SemaphoreSlim`; picker performs the atomic `Queued → Running` claim filtered by `BlockedByTaskId IS NULL`, `is_manual = 0`, schedule, and "no `task_dependencies` edge whose predecessor isn't `done`" (doubly negated on purpose: zero edges stays eligible, all edges must be done, and an edge pointing at a vanished task keeps blocking); QueueService is a thin `BackgroundService` looping on the waker, dispatching via `TaskRunner`. Per tick it also applies the usage throttle and gate → [usage-monitoring](../../docs/explore-notes/usage-monitoring.md). - **RunCancellationRegistry** — taskId → running-run CTS. Lets `TaskStateService.CancelAsync` kill a cancelled task's process without a DI cycle. - **OverrideSlotService** — owns `RunNow` / `ContinueTask`; goes through `TaskStateService.StartRunningAsync` (caller-driven, serialized by slot lock). diff --git a/src/ClaudeDo.Worker/Runner/TaskRunner.cs b/src/ClaudeDo.Worker/Runner/TaskRunner.cs index 0033485a..ca14055e 100644 --- a/src/ClaudeDo.Worker/Runner/TaskRunner.cs +++ b/src/ClaudeDo.Worker/Runner/TaskRunner.cs @@ -553,8 +553,6 @@ public sealed class TaskRunner var transition = await _state.SubmitForChildrenAsync(task.Id, finishedAt, reviewResult, CancellationToken.None); if (!transition.Ok) _logger.LogWarning("SubmitForChildrenAsync rejected for task {TaskId}: {Reason}", task.Id, transition.Reason); - foreach (var child in pendingChildren) - await _state.EnqueueAsync(child.Id, CancellationToken.None); await _broadcaster.WorkerLog( $"Finished #{task.Number} \"{task.Title}\" (waiting on {pendingChildren.Count} improvement(s))", WorkerLogLevel.Success, DateTime.UtcNow); diff --git a/src/ClaudeDo.Worker/State/TaskStateService.cs b/src/ClaudeDo.Worker/State/TaskStateService.cs index fd2e621a..e465d642 100644 --- a/src/ClaudeDo.Worker/State/TaskStateService.cs +++ b/src/ClaudeDo.Worker/State/TaskStateService.cs @@ -159,16 +159,40 @@ public sealed class TaskStateService : ITaskStateService public async Task SubmitForChildrenAsync(string taskId, DateTime finishedAt, string? result, CancellationToken ct) { - await using var ctx = await _dbFactory.CreateDbContextAsync(ct); - var affected = await ctx.Tasks - .Where(t => t.Id == taskId && t.Status == TaskStatus.Running) - .ExecuteUpdateAsync(s => s - .SetProperty(t => t.Status, TaskStatus.WaitingForChildren) - .SetProperty(t => t.FinishedAt, finishedAt) - .SetProperty(t => t.Result, result), ct); + PlanningPhase planningPhase; + await using (var ctx = await _dbFactory.CreateDbContextAsync(ct)) + { + var affected = await ctx.Tasks + .Where(t => t.Id == taskId && t.Status == TaskStatus.Running) + .ExecuteUpdateAsync(s => s + .SetProperty(t => t.Status, TaskStatus.WaitingForChildren) + .SetProperty(t => t.FinishedAt, finishedAt) + .SetProperty(t => t.Result, result), ct); - if (affected == 0) - return new TransitionResult(false, "Task not running; cannot submit for children."); + if (affected == 0) + return new TransitionResult(false, "Task not running; cannot submit for children."); + + planningPhase = await ctx.Tasks.AsNoTracking() + .Where(t => t.Id == taskId) + .Select(t => t.PlanningPhase) + .FirstAsync(ct); + } + + // A normal (non-planning) parent queues its own children as a sequential chain right + // here; a planning parent's children wait for the user's explicit "Queue plan" action + // (PlanningChainCoordinator.QueuePlanAsync), so leave those untouched. A child that's + // already Running makes SetupChainAsync throw — log it rather than fail this transition. + if (planningPhase == PlanningPhase.None) + { + try + { + await _chain.SetupChainAsync(taskId, enqueue: true, ct); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "SetupChainAsync failed for parent {TaskId}", taskId); + } + } await NotifyAsync(taskId); return new TransitionResult(true, null); diff --git a/tests/ClaudeDo.Worker.Tests/WaitingForChildrenLifecycleTests.cs b/tests/ClaudeDo.Worker.Tests/WaitingForChildrenLifecycleTests.cs index fb8f1ed5..ff7c54a6 100644 --- a/tests/ClaudeDo.Worker.Tests/WaitingForChildrenLifecycleTests.cs +++ b/tests/ClaudeDo.Worker.Tests/WaitingForChildrenLifecycleTests.cs @@ -54,6 +54,81 @@ public sealed class WaitingForChildrenLifecycleTests : IDisposable Assert.False(second.Ok); } + private async Task SeedRunningParentWithChildrenAsync( + PlanningPhase parentPhase, params TaskStatus[] childStatuses) + { + using var ctx = _db.CreateContext(); + if (!ctx.Lists.Any()) + ctx.Lists.Add(new ListEntity { Id = "l1", Name = "L", CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = "p1", ListId = "l1", Title = "Parent", Number = ++_numberSeed, + Status = TaskStatus.Running, PlanningPhase = parentPhase, CreatedAt = DateTime.UtcNow }); + int i = 0; + foreach (var cs in childStatuses) + ctx.Tasks.Add(new TaskEntity { Id = $"c{i++}", ListId = "l1", Title = "Child", Number = ++_numberSeed, + Status = cs, ParentTaskId = "p1", CreatedAt = DateTime.UtcNow }); + await ctx.SaveChangesAsync(); + return "p1"; + } + + [Fact] + public async Task SubmitForChildren_NormalParent_ChainsIdleChildren() + { + var id = await SeedRunningParentWithChildrenAsync( + PlanningPhase.None, TaskStatus.Idle, TaskStatus.Idle, TaskStatus.Idle); + + var result = await _built.State.SubmitForChildrenAsync(id, DateTime.UtcNow, "ran ok", default); + Assert.True(result.Ok); + + using var ctx = _db.CreateContext(); + var repo = new TaskRepository(ctx); + var c0 = await repo.GetByIdAsync("c0"); + var c1 = await repo.GetByIdAsync("c1"); + var c2 = await repo.GetByIdAsync("c2"); + + Assert.Equal(TaskStatus.Queued, c0!.Status); + Assert.Null(c0.BlockedByTaskId); + + Assert.Equal(TaskStatus.Queued, c1!.Status); + Assert.Equal("c0", c1.BlockedByTaskId); + + Assert.Equal(TaskStatus.Queued, c2!.Status); + Assert.Equal("c1", c2.BlockedByTaskId); + } + + [Fact] + public async Task SubmitForChildren_PlanningParent_DoesNotChainChildren() + { + var id = await SeedRunningParentWithChildrenAsync(PlanningPhase.Active, TaskStatus.Idle, TaskStatus.Idle); + + var result = await _built.State.SubmitForChildrenAsync(id, DateTime.UtcNow, "ran ok", default); + Assert.True(result.Ok); + + using var ctx = _db.CreateContext(); + var repo = new TaskRepository(ctx); + var c0 = await repo.GetByIdAsync("c0"); + var c1 = await repo.GetByIdAsync("c1"); + + Assert.Equal(TaskStatus.Idle, c0!.Status); + Assert.Null(c0.BlockedByTaskId); + Assert.Equal(TaskStatus.Idle, c1!.Status); + Assert.Null(c1.BlockedByTaskId); + } + + [Fact] + public async Task SubmitForChildren_RunningChild_DoesNotFailParentTransition() + { + var id = await SeedRunningParentWithChildrenAsync(PlanningPhase.None, TaskStatus.Running, TaskStatus.Idle); + + var result = await _built.State.SubmitForChildrenAsync(id, DateTime.UtcNow, "ran ok", default); + Assert.True(result.Ok); + + using var ctx = _db.CreateContext(); + var repo = new TaskRepository(ctx); + Assert.Equal(TaskStatus.WaitingForChildren, (await repo.GetByIdAsync(id))!.Status); + // Reshape aborted (a running child cannot be reshaped) — the untouched sibling stays Idle. + Assert.Equal(TaskStatus.Idle, (await repo.GetByIdAsync("c1"))!.Status); + } + private async Task SeedParentWithChildrenAsync(string parentStatus, params TaskStatus[] childStatuses) { using var ctx = _db.CreateContext();