refactor(worker): single parent-advance path for planning + improvement
Collapse TryCompleteParentAsync (planning -> Done) and TryAdvanceImprovementParentAsync (improvement -> WaitingForReview) into one TryAdvanceParentAsync that surfaces any WaitingForChildren parent for review once all children are terminal. Planning parents no longer auto-complete. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -19,7 +19,7 @@ Shared data layer: models, repositories, SQLite infrastructure, and git operatio
|
|||||||
|
|
||||||
All repositories use EF Core LINQ queries via `ClaudeDoDbContext`. The atomic `Queued -> Running` claim lives in the Worker's `QueuePicker` (uses `FromSqlRaw`), not here.
|
All repositories use EF Core LINQ queries via `ClaudeDoDbContext`. The atomic `Queued -> Running` claim lives in the Worker's `QueuePicker` (uses `FromSqlRaw`), not here.
|
||||||
|
|
||||||
- **TaskRepository** — CRUD, planning helpers (`CreateChildAsync`, `SetPlanningStartedAsync`, `DiscardPlanningAsync`, `TryCompleteParentAsync`, `UpdateChildAsync`), `UpdateAgentSettingsAsync` (model / system-prompt / agent-path overrides). Status-mutation primitives `MarkRunningAsync` / `MarkDoneAsync` / `MarkFailedAsync` / `FlipAllRunningToFailedAsync` are `internal` and called only by `TaskStateService` in the worker. `CreateChildAsync` produces children with `Status=Idle, PlanningPhase=None`; once their parent's `PlanningPhase` becomes `Finalized`, the chain coordinator queues them.
|
- **TaskRepository** — CRUD, planning helpers (`CreateChildAsync`, `SetPlanningStartedAsync`, `DiscardPlanningAsync`, `UpdateChildAsync`), `UpdateAgentSettingsAsync` (model / system-prompt / agent-path overrides). Status-mutation primitives `MarkRunningAsync` / `MarkDoneAsync` / `MarkFailedAsync` / `FlipAllRunningToFailedAsync` are `internal` and called only by `TaskStateService` in the worker. `CreateChildAsync` produces children with `Status=Idle, PlanningPhase=None`; once their parent's `PlanningPhase` becomes `Finalized`, the chain coordinator queues them.
|
||||||
- **ListRepository** — CRUD, `GetConfigAsync` / `SetConfigAsync` (upsert) / `DeleteConfigAsync` for `list_config`
|
- **ListRepository** — CRUD, `GetConfigAsync` / `SetConfigAsync` (upsert) / `DeleteConfigAsync` for `list_config`
|
||||||
- **WorktreeRepository** — CRUD, `UpdateHeadAsync`, `SetStateAsync`
|
- **WorktreeRepository** — CRUD, `UpdateHeadAsync`, `SetStateAsync`
|
||||||
- **TaskRunRepository**, **SubtaskRepository**, **AppSettingsRepository**
|
- **TaskRunRepository**, **SubtaskRepository**, **AppSettingsRepository**
|
||||||
|
|||||||
@@ -474,32 +474,5 @@ public sealed class TaskRepository
|
|||||||
return chainIds.Count;
|
return chainIds.Count;
|
||||||
}
|
}
|
||||||
|
|
||||||
public async Task TryCompleteParentAsync(
|
|
||||||
string parentId,
|
|
||||||
CancellationToken ct = default)
|
|
||||||
{
|
|
||||||
var parent = await _context.Tasks.AsNoTracking().FirstOrDefaultAsync(t => t.Id == parentId, ct);
|
|
||||||
if (parent is null || parent.PlanningPhase != PlanningPhase.Finalized) return;
|
|
||||||
|
|
||||||
var children = await _context.Tasks
|
|
||||||
.Where(t => t.ParentTaskId == parentId)
|
|
||||||
.Select(t => t.Status)
|
|
||||||
.ToListAsync(ct);
|
|
||||||
|
|
||||||
if (children.Count == 0) return;
|
|
||||||
|
|
||||||
bool allTerminal = children.All(s => s == TaskStatus.Done || s == TaskStatus.Failed);
|
|
||||||
if (!allTerminal) return;
|
|
||||||
|
|
||||||
bool anyFailed = children.Any(s => s == TaskStatus.Failed);
|
|
||||||
var finalStatus = anyFailed ? TaskStatus.Failed : TaskStatus.Done;
|
|
||||||
var finishedAt = DateTime.UtcNow;
|
|
||||||
await _context.Tasks
|
|
||||||
.Where(t => t.Id == parentId)
|
|
||||||
.ExecuteUpdateAsync(s => s
|
|
||||||
.SetProperty(t => t.Status, finalStatus)
|
|
||||||
.SetProperty(t => t.FinishedAt, finishedAt), ct);
|
|
||||||
}
|
|
||||||
|
|
||||||
#endregion
|
#endregion
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -386,28 +386,18 @@ public sealed class TaskStateService : ITaskStateService
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
await using var ctx = await _dbFactory.CreateDbContextAsync(CancellationToken.None);
|
await TryAdvanceParentAsync(parentId);
|
||||||
await new TaskRepository(ctx).TryCompleteParentAsync(parentId, CancellationToken.None);
|
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
_logger.LogWarning(ex, "TryCompleteParent failed for {ParentId}", parentId);
|
_logger.LogWarning(ex, "TryAdvanceParent failed for {ParentId}", parentId);
|
||||||
}
|
|
||||||
|
|
||||||
try
|
|
||||||
{
|
|
||||||
await TryAdvanceImprovementParentAsync(parentId);
|
|
||||||
}
|
|
||||||
catch (Exception ex)
|
|
||||||
{
|
|
||||||
_logger.LogWarning(ex, "TryAdvanceImprovementParent failed for {ParentId}", parentId);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Improvement parents sit in WaitingForChildren while their suggested children run.
|
// Any parent (planning or improvement) sitting in WaitingForChildren surfaces for review
|
||||||
// Once every child is terminal (Done/Failed/Cancelled) the parent surfaces for review;
|
// once every child is terminal (Done/Failed/Cancelled). A failed or cancelled child does
|
||||||
// a failed or cancelled child does not wedge the parent — it is flagged on the result.
|
// not wedge the parent — it is flagged on the result.
|
||||||
private async Task TryAdvanceImprovementParentAsync(string parentId)
|
private async Task TryAdvanceParentAsync(string parentId)
|
||||||
{
|
{
|
||||||
string? parentResult;
|
string? parentResult;
|
||||||
List<TaskStatus> childStatuses;
|
List<TaskStatus> childStatuses;
|
||||||
|
|||||||
@@ -1,139 +0,0 @@
|
|||||||
using ClaudeDo.Data;
|
|
||||||
using ClaudeDo.Data.Models;
|
|
||||||
using ClaudeDo.Data.Repositories;
|
|
||||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
|
||||||
using Microsoft.EntityFrameworkCore;
|
|
||||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
|
||||||
|
|
||||||
namespace ClaudeDo.Worker.Tests.Repositories;
|
|
||||||
|
|
||||||
public sealed class TaskRepositoryParentCompletionTests : IDisposable
|
|
||||||
{
|
|
||||||
private readonly DbFixture _db = new();
|
|
||||||
private readonly ClaudeDoDbContext _ctx;
|
|
||||||
private readonly TaskRepository _tasks;
|
|
||||||
private readonly ListRepository _lists;
|
|
||||||
|
|
||||||
public TaskRepositoryParentCompletionTests()
|
|
||||||
{
|
|
||||||
_ctx = _db.CreateContext();
|
|
||||||
_tasks = new TaskRepository(_ctx);
|
|
||||||
_lists = new ListRepository(_ctx);
|
|
||||||
}
|
|
||||||
|
|
||||||
public void Dispose() { _ctx.Dispose(); _db.Dispose(); }
|
|
||||||
|
|
||||||
private async Task<string> ListAsync()
|
|
||||||
{
|
|
||||||
var id = Guid.NewGuid().ToString();
|
|
||||||
await _lists.AddAsync(new ListEntity { Id = id, Name = "L", CreatedAt = DateTime.UtcNow });
|
|
||||||
return id;
|
|
||||||
}
|
|
||||||
|
|
||||||
private async Task<TaskEntity> PlannedParentAsync(string listId)
|
|
||||||
{
|
|
||||||
var parent = new TaskEntity
|
|
||||||
{
|
|
||||||
Id = Guid.NewGuid().ToString(),
|
|
||||||
ListId = listId,
|
|
||||||
Title = "p",
|
|
||||||
Status = TaskStatus.Idle,
|
|
||||||
PlanningPhase = PlanningPhase.Finalized,
|
|
||||||
CreatedAt = DateTime.UtcNow,
|
|
||||||
CommitType = "chore",
|
|
||||||
};
|
|
||||||
await _tasks.AddAsync(parent);
|
|
||||||
return parent;
|
|
||||||
}
|
|
||||||
|
|
||||||
private async Task<TaskEntity> ChildAsync(string listId, string parentId, TaskStatus status)
|
|
||||||
{
|
|
||||||
var child = new TaskEntity
|
|
||||||
{
|
|
||||||
Id = Guid.NewGuid().ToString(),
|
|
||||||
ListId = listId,
|
|
||||||
Title = "c",
|
|
||||||
Status = status,
|
|
||||||
CreatedAt = DateTime.UtcNow,
|
|
||||||
CommitType = "chore",
|
|
||||||
ParentTaskId = parentId,
|
|
||||||
};
|
|
||||||
await _tasks.AddAsync(child);
|
|
||||||
return child;
|
|
||||||
}
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task TryCompleteParentAsync_AllChildrenDone_ParentBecomesDone()
|
|
||||||
{
|
|
||||||
var listId = await ListAsync();
|
|
||||||
var parent = await PlannedParentAsync(listId);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
|
|
||||||
await _tasks.TryCompleteParentAsync(parent.Id);
|
|
||||||
|
|
||||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(TaskStatus.Done, loaded!.Status);
|
|
||||||
Assert.NotNull(loaded.FinishedAt);
|
|
||||||
}
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task TryCompleteParentAsync_OneFailedRestDone_ParentBecomesFailed()
|
|
||||||
{
|
|
||||||
var listId = await ListAsync();
|
|
||||||
var parent = await PlannedParentAsync(listId);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Failed);
|
|
||||||
|
|
||||||
await _tasks.TryCompleteParentAsync(parent.Id);
|
|
||||||
|
|
||||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(TaskStatus.Failed, loaded!.Status);
|
|
||||||
Assert.NotNull(loaded.FinishedAt);
|
|
||||||
}
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task TryCompleteParentAsync_OneStillRunning_ParentStaysPlanned()
|
|
||||||
{
|
|
||||||
var listId = await ListAsync();
|
|
||||||
var parent = await PlannedParentAsync(listId);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Running);
|
|
||||||
|
|
||||||
await _tasks.TryCompleteParentAsync(parent.Id);
|
|
||||||
|
|
||||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(TaskStatus.Idle, loaded!.Status);
|
|
||||||
Assert.Equal(PlanningPhase.Finalized, loaded.PlanningPhase);
|
|
||||||
Assert.Null(loaded.FinishedAt);
|
|
||||||
}
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task TryCompleteParentAsync_ChildStillIdle_ParentStaysFinalized()
|
|
||||||
{
|
|
||||||
var listId = await ListAsync();
|
|
||||||
var parent = await PlannedParentAsync(listId);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Idle);
|
|
||||||
|
|
||||||
await _tasks.TryCompleteParentAsync(parent.Id);
|
|
||||||
|
|
||||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(PlanningPhase.Finalized, loaded!.PlanningPhase);
|
|
||||||
}
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task TryCompleteParentAsync_ParentIsNotFinalized_NoChange()
|
|
||||||
{
|
|
||||||
var listId = await ListAsync();
|
|
||||||
var parent = await PlannedParentAsync(listId);
|
|
||||||
await _ctx.Database.ExecuteSqlRawAsync(
|
|
||||||
"UPDATE tasks SET planning_phase = 'active' WHERE id = {0}", parent.Id);
|
|
||||||
await ChildAsync(listId, parent.Id, TaskStatus.Done);
|
|
||||||
|
|
||||||
await _tasks.TryCompleteParentAsync(parent.Id);
|
|
||||||
|
|
||||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(PlanningPhase.Active, loaded!.PlanningPhase);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,75 +0,0 @@
|
|||||||
using ClaudeDo.Data;
|
|
||||||
using ClaudeDo.Data.Models;
|
|
||||||
using ClaudeDo.Data.Repositories;
|
|
||||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
|
||||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
|
||||||
|
|
||||||
namespace ClaudeDo.Worker.Tests.Runner;
|
|
||||||
|
|
||||||
public sealed class TaskRunnerParentCompletionTests : IDisposable
|
|
||||||
{
|
|
||||||
private readonly DbFixture _db = new();
|
|
||||||
private readonly ClaudeDoDbContext _ctx;
|
|
||||||
private readonly TaskRepository _tasks;
|
|
||||||
private readonly ListRepository _lists;
|
|
||||||
|
|
||||||
public TaskRunnerParentCompletionTests()
|
|
||||||
{
|
|
||||||
_ctx = _db.CreateContext();
|
|
||||||
_tasks = new TaskRepository(_ctx);
|
|
||||||
_lists = new ListRepository(_ctx);
|
|
||||||
}
|
|
||||||
|
|
||||||
public void Dispose() { _ctx.Dispose(); _db.Dispose(); }
|
|
||||||
|
|
||||||
[Fact]
|
|
||||||
public async Task ChildMarkedDone_LastOne_ParentFinalized()
|
|
||||||
{
|
|
||||||
var listId = Guid.NewGuid().ToString();
|
|
||||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", CreatedAt = DateTime.UtcNow });
|
|
||||||
|
|
||||||
var parent = new TaskEntity
|
|
||||||
{
|
|
||||||
Id = Guid.NewGuid().ToString(),
|
|
||||||
ListId = listId,
|
|
||||||
Title = "p",
|
|
||||||
Status = TaskStatus.Idle,
|
|
||||||
PlanningPhase = PlanningPhase.Finalized,
|
|
||||||
CreatedAt = DateTime.UtcNow,
|
|
||||||
CommitType = "chore",
|
|
||||||
};
|
|
||||||
await _tasks.AddAsync(parent);
|
|
||||||
|
|
||||||
var c1 = new TaskEntity
|
|
||||||
{
|
|
||||||
Id = Guid.NewGuid().ToString(),
|
|
||||||
ListId = listId,
|
|
||||||
Title = "c1",
|
|
||||||
Status = TaskStatus.Done,
|
|
||||||
CreatedAt = DateTime.UtcNow,
|
|
||||||
CommitType = "chore",
|
|
||||||
ParentTaskId = parent.Id,
|
|
||||||
};
|
|
||||||
var c2 = new TaskEntity
|
|
||||||
{
|
|
||||||
Id = Guid.NewGuid().ToString(),
|
|
||||||
ListId = listId,
|
|
||||||
Title = "c2",
|
|
||||||
Status = TaskStatus.Running,
|
|
||||||
CreatedAt = DateTime.UtcNow,
|
|
||||||
CommitType = "chore",
|
|
||||||
ParentTaskId = parent.Id,
|
|
||||||
};
|
|
||||||
await _tasks.AddAsync(c1);
|
|
||||||
await _tasks.AddAsync(c2);
|
|
||||||
|
|
||||||
// Simulate the runner finishing the second child:
|
|
||||||
await _tasks.MarkDoneAsync(c2.Id, DateTime.UtcNow, "done");
|
|
||||||
if (c2.ParentTaskId is not null)
|
|
||||||
await _tasks.TryCompleteParentAsync(c2.ParentTaskId);
|
|
||||||
|
|
||||||
var parentLoaded = await _tasks.GetByIdAsync(parent.Id);
|
|
||||||
Assert.Equal(TaskStatus.Done, parentLoaded!.Status);
|
|
||||||
Assert.NotNull(parentLoaded.FinishedAt);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -126,4 +126,38 @@ public sealed class WaitingForChildrenLifecycleTests : IDisposable
|
|||||||
var result = await _built.State.EnqueueAsync("kid", default);
|
var result = await _built.State.EnqueueAsync("kid", default);
|
||||||
Assert.False(result.Ok);
|
Assert.False(result.Ok);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task SequentialChildren_parent_only_advances_after_both_are_terminal()
|
||||||
|
{
|
||||||
|
// Arrange: parent WaitingForChildren, child1 Done, child2 blocked by child1 (Running)
|
||||||
|
using (var ctx = _db.CreateContext())
|
||||||
|
{
|
||||||
|
ctx.Lists.Add(new ListEntity { Id = "l1", Name = "L", CreatedAt = DateTime.UtcNow });
|
||||||
|
ctx.Tasks.Add(new TaskEntity { Id = "par", ListId = "l1", Title = "Parent",
|
||||||
|
Status = TaskStatus.WaitingForChildren, CreatedAt = DateTime.UtcNow });
|
||||||
|
ctx.Tasks.Add(new TaskEntity { Id = "c1", ListId = "l1", Title = "Child1",
|
||||||
|
Status = TaskStatus.Running, ParentTaskId = "par", CreatedAt = DateTime.UtcNow });
|
||||||
|
ctx.Tasks.Add(new TaskEntity { Id = "c2", ListId = "l1", Title = "Child2",
|
||||||
|
Status = TaskStatus.Running, ParentTaskId = "par", BlockedByTaskId = "c1",
|
||||||
|
CreatedAt = DateTime.UtcNow });
|
||||||
|
await ctx.SaveChangesAsync();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Act: complete child1 — parent must stay WaitingForChildren because child2 is still running
|
||||||
|
await _built.State.CompleteAsync("c1", DateTime.UtcNow, "c1 ok", default);
|
||||||
|
using (var ctx = _db.CreateContext())
|
||||||
|
{
|
||||||
|
var par = await new TaskRepository(ctx).GetByIdAsync("par");
|
||||||
|
Assert.Equal(TaskStatus.WaitingForChildren, par!.Status);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Act: complete child2 — now all children are terminal; parent must advance
|
||||||
|
await _built.State.CompleteAsync("c2", DateTime.UtcNow, "c2 ok", default);
|
||||||
|
using (var ctx = _db.CreateContext())
|
||||||
|
{
|
||||||
|
var par = await new TaskRepository(ctx).GetByIdAsync("par");
|
||||||
|
Assert.Equal(TaskStatus.WaitingForReview, par!.Status);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user