feat(planning): prevent orphaned subtasks via guards + startup repair
Three coordinated guards close the orphan-creation paths: - CreateChildAsync refuses when the parent is not in a planning phase. - DiscardPlanningAsync now returns a structured DiscardPlanningOutcome and refuses when children are queued or running; callers can opt into auto-dequeuing queued kids via dequeueQueuedChildren=true. Terminal children (Done/Failed/Cancelled) are promoted to top-level instead of becoming orphans when the parent's PlanningPhase is reset. - OrphanRecovery hosted service clears ParentTaskId on any rows whose parent is missing or no longer in a planning phase on worker startup, mirroring the StaleTaskRecovery pattern. UI surfaces the block reason: a confirm dialog offers to dequeue queued children and retry; a running-children block is shown as a hard error asking the user to cancel first. WorkerClient now negotiates the JsonStringEnumConverter so the DiscardPlanningResult enum round-trips correctly over SignalR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -42,7 +42,8 @@ public class ConflictResolutionViewModelTests
|
||||
public Task<List<string>> GetAllTagsAsync() => Task.FromResult(new List<string>());
|
||||
public Task StartPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task ResumePlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<ClaudeDo.Data.Repositories.DiscardPlanningOutcome> DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default)
|
||||
=> Task.FromResult(new ClaudeDo.Data.Repositories.DiscardPlanningOutcome(ClaudeDo.Data.Repositories.DiscardPlanningResult.Discarded, 0, 0));
|
||||
public Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<int> GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0);
|
||||
public Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId) => Task.FromResult<MergeTargetsDto?>(null);
|
||||
|
||||
@@ -72,7 +72,8 @@ public class DetailsIslandPlanningTests : IDisposable
|
||||
public Task<List<string>> GetAllTagsAsync() => Task.FromResult(new List<string>());
|
||||
public Task StartPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task ResumePlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<ClaudeDo.Data.Repositories.DiscardPlanningOutcome> DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default)
|
||||
=> Task.FromResult(new ClaudeDo.Data.Repositories.DiscardPlanningOutcome(ClaudeDo.Data.Repositories.DiscardPlanningResult.Discarded, 0, 0));
|
||||
public Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<int> GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0);
|
||||
public Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId) => Task.FromResult(MergeTargetsResult);
|
||||
|
||||
@@ -39,7 +39,8 @@ public class PlanningDiffViewModelTests
|
||||
public Task<List<string>> GetAllTagsAsync() => Task.FromResult(new List<string>());
|
||||
public Task StartPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task ResumePlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<ClaudeDo.Data.Repositories.DiscardPlanningOutcome> DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default)
|
||||
=> Task.FromResult(new ClaudeDo.Data.Repositories.DiscardPlanningOutcome(ClaudeDo.Data.Repositories.DiscardPlanningResult.Discarded, 0, 0));
|
||||
public Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task<int> GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0);
|
||||
public Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId) => Task.FromResult<MergeTargetsDto?>(null);
|
||||
|
||||
@@ -217,7 +217,7 @@ public sealed class PlanningSessionManagerTests : IDisposable
|
||||
var startCtx = await _sut.StartAsync(parent.Id, CancellationToken.None);
|
||||
Assert.True(Directory.Exists(startCtx.Files.SessionDirectory));
|
||||
|
||||
await _sut.DiscardAsync(parent.Id, CancellationToken.None);
|
||||
await _sut.DiscardAsync(parent.Id, dequeueQueuedChildren: false, CancellationToken.None);
|
||||
|
||||
Assert.False(Directory.Exists(startCtx.Files.SessionDirectory));
|
||||
var loaded = await _tasks.GetByIdAsync(parent.Id);
|
||||
@@ -235,7 +235,7 @@ public sealed class PlanningSessionManagerTests : IDisposable
|
||||
var ctx = await _sut.StartAsync(parent.Id, CancellationToken.None);
|
||||
Assert.True(Directory.Exists(ctx.WorktreePath));
|
||||
|
||||
await _sut.DiscardAsync(parent.Id, CancellationToken.None);
|
||||
await _sut.DiscardAsync(parent.Id, dequeueQueuedChildren: false, CancellationToken.None);
|
||||
|
||||
Assert.False(Directory.Exists(ctx.WorktreePath));
|
||||
// branch deleted
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
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;
|
||||
|
||||
/// <summary>
|
||||
/// Covers the invariant that no task may have <c>ParentTaskId</c> pointing to a
|
||||
/// parent without <c>PlanningPhase.Active|Finalized</c>. Tests the three guard
|
||||
/// rails: <c>CreateChildAsync</c> validation, <c>DiscardPlanningAsync</c>
|
||||
/// gating with the optional dequeue path, and the startup repair sweep.
|
||||
/// </summary>
|
||||
public sealed class TaskRepositoryOrphanGuardTests : IDisposable
|
||||
{
|
||||
private readonly DbFixture _db = new();
|
||||
private readonly ClaudeDoDbContext _ctx;
|
||||
private readonly TaskRepository _tasks;
|
||||
private readonly ListRepository _lists;
|
||||
|
||||
public TaskRepositoryOrphanGuardTests()
|
||||
{
|
||||
_ctx = _db.CreateContext();
|
||||
_tasks = new TaskRepository(_ctx);
|
||||
_lists = new ListRepository(_ctx);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_ctx.Dispose();
|
||||
_db.Dispose();
|
||||
}
|
||||
|
||||
private async Task<string> CreateListAsync()
|
||||
{
|
||||
var id = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = id, Name = "L", CreatedAt = DateTime.UtcNow });
|
||||
return id;
|
||||
}
|
||||
|
||||
private TaskEntity MakeTask(string listId, TaskStatus status = TaskStatus.Idle, PlanningPhase phase = PlanningPhase.None) => new()
|
||||
{
|
||||
Id = Guid.NewGuid().ToString(),
|
||||
ListId = listId,
|
||||
Title = "T",
|
||||
Status = status,
|
||||
PlanningPhase = phase,
|
||||
CreatedAt = DateTime.UtcNow,
|
||||
CommitType = "chore",
|
||||
};
|
||||
|
||||
private async Task<TaskEntity> SeedPlanningParentAsync(string listId)
|
||||
{
|
||||
var parent = MakeTask(listId, status: TaskStatus.Idle, phase: PlanningPhase.Active);
|
||||
await _tasks.AddAsync(parent);
|
||||
return parent;
|
||||
}
|
||||
|
||||
// --- CreateChildAsync validation ---
|
||||
|
||||
[Fact]
|
||||
public async Task CreateChildAsync_Throws_When_Parent_Has_No_Planning_Phase()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = MakeTask(listId, phase: PlanningPhase.None);
|
||||
await _tasks.AddAsync(parent);
|
||||
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => _tasks.CreateChildAsync(parent.Id, "child", null, null, null));
|
||||
Assert.Contains("not in a planning phase", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CreateChildAsync_Succeeds_When_Parent_Is_Active()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
|
||||
var child = await _tasks.CreateChildAsync(parent.Id, "child", null, null, null);
|
||||
Assert.Equal(parent.Id, child.ParentTaskId);
|
||||
Assert.Equal(TaskStatus.Idle, child.Status);
|
||||
}
|
||||
|
||||
// --- DiscardPlanningAsync gating ---
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_NotInPlanning_When_Parent_Phase_Is_None()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var stray = MakeTask(listId, phase: PlanningPhase.None);
|
||||
await _tasks.AddAsync(stray);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(stray.Id, dequeueQueuedChildren: false);
|
||||
Assert.Equal(DiscardPlanningResult.NotInPlanning, outcome.Result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_Succeeds_When_All_Children_Are_Idle()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
await _tasks.CreateChildAsync(parent.Id, "a", null, null, null);
|
||||
await _tasks.CreateChildAsync(parent.Id, "b", null, null, null);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: false);
|
||||
|
||||
Assert.Equal(DiscardPlanningResult.Discarded, outcome.Result);
|
||||
Assert.Equal(0, _ctx.Tasks.AsNoTracking().Count(t => t.ParentTaskId == parent.Id));
|
||||
var reloaded = _ctx.Tasks.AsNoTracking().Single(t => t.Id == parent.Id);
|
||||
Assert.Equal(PlanningPhase.None, reloaded.PlanningPhase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_Blocks_On_Queued_Children_Without_Optin()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
var child = await _tasks.CreateChildAsync(parent.Id, "c", null, null, null);
|
||||
await SetChildStatusAsync(child.Id, TaskStatus.Queued);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: false);
|
||||
|
||||
Assert.Equal(DiscardPlanningResult.BlockedByQueuedChildren, outcome.Result);
|
||||
Assert.Equal(1, outcome.QueuedChildrenCount);
|
||||
// Parent and child are untouched.
|
||||
Assert.Equal(PlanningPhase.Active, _ctx.Tasks.AsNoTracking().Single(t => t.Id == parent.Id).PlanningPhase);
|
||||
Assert.Equal(TaskStatus.Queued, _ctx.Tasks.AsNoTracking().Single(t => t.Id == child.Id).Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_With_Dequeue_Succeeds_And_Drops_Idle_Children()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
var child = await _tasks.CreateChildAsync(parent.Id, "c", null, null, null);
|
||||
await SetChildStatusAsync(child.Id, TaskStatus.Queued);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: true);
|
||||
|
||||
Assert.Equal(DiscardPlanningResult.Discarded, outcome.Result);
|
||||
// Child was dequeued to Idle and then deleted as part of the discard.
|
||||
Assert.False(_ctx.Tasks.AsNoTracking().Any(t => t.Id == child.Id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_Blocks_On_Running_Children_Even_With_Dequeue_Optin()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
var child = await _tasks.CreateChildAsync(parent.Id, "c", null, null, null);
|
||||
await SetChildStatusAsync(child.Id, TaskStatus.Running);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: true);
|
||||
|
||||
Assert.Equal(DiscardPlanningResult.BlockedByRunningChildren, outcome.Result);
|
||||
Assert.Equal(1, outcome.RunningChildrenCount);
|
||||
Assert.Equal(PlanningPhase.Active, _ctx.Tasks.AsNoTracking().Single(t => t.Id == parent.Id).PlanningPhase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DiscardPlanning_Promotes_Terminal_Children_To_Top_Level()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
var done = await _tasks.CreateChildAsync(parent.Id, "done", null, null, null);
|
||||
var failed = await _tasks.CreateChildAsync(parent.Id, "failed", null, null, null);
|
||||
await SetChildStatusAsync(done.Id, TaskStatus.Done);
|
||||
await SetChildStatusAsync(failed.Id, TaskStatus.Failed);
|
||||
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: false);
|
||||
|
||||
Assert.Equal(DiscardPlanningResult.Discarded, outcome.Result);
|
||||
Assert.Null(_ctx.Tasks.AsNoTracking().Single(t => t.Id == done.Id).ParentTaskId);
|
||||
Assert.Null(_ctx.Tasks.AsNoTracking().Single(t => t.Id == failed.Id).ParentTaskId);
|
||||
}
|
||||
|
||||
// --- Repair sweep ---
|
||||
|
||||
[Fact]
|
||||
public async Task Repair_Clears_ParentTaskId_When_Parent_Is_Not_Planning()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
// Parent is plain (not planning), child attached -> orphan by definition.
|
||||
var parent = MakeTask(listId, phase: PlanningPhase.None);
|
||||
await _tasks.AddAsync(parent);
|
||||
var child = MakeTask(listId);
|
||||
child.ParentTaskId = parent.Id;
|
||||
await _tasks.AddAsync(child);
|
||||
|
||||
var repaired = await _tasks.RepairOrphanedChildrenAsync();
|
||||
Assert.Equal(1, repaired);
|
||||
Assert.Null(_ctx.Tasks.AsNoTracking().Single(t => t.Id == child.Id).ParentTaskId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Repair_Leaves_Valid_Children_Untouched()
|
||||
{
|
||||
var listId = await CreateListAsync();
|
||||
var parent = await SeedPlanningParentAsync(listId);
|
||||
var child = await _tasks.CreateChildAsync(parent.Id, "c", null, null, null);
|
||||
|
||||
var repaired = await _tasks.RepairOrphanedChildrenAsync();
|
||||
Assert.Equal(0, repaired);
|
||||
Assert.Equal(parent.Id, _ctx.Tasks.AsNoTracking().Single(t => t.Id == child.Id).ParentTaskId);
|
||||
}
|
||||
|
||||
private async Task SetChildStatusAsync(string id, TaskStatus status)
|
||||
{
|
||||
var t = await _ctx.Tasks.FindAsync(id) ?? throw new InvalidOperationException();
|
||||
t.Status = status;
|
||||
await _ctx.SaveChangesAsync();
|
||||
_ctx.Entry(t).State = Microsoft.EntityFrameworkCore.EntityState.Detached;
|
||||
}
|
||||
}
|
||||
@@ -205,9 +205,9 @@ public sealed class TaskRepositoryPlanningTests : IDisposable
|
||||
var c1 = await _tasks.CreateChildAsync(parent.Id, "c1", null, null, null);
|
||||
var c2 = await _tasks.CreateChildAsync(parent.Id, "c2", null, null, null);
|
||||
|
||||
var ok = await _tasks.DiscardPlanningAsync(parent.Id);
|
||||
var outcome = await _tasks.DiscardPlanningAsync(parent.Id, dequeueQueuedChildren: false);
|
||||
|
||||
Assert.True(ok);
|
||||
Assert.Equal(DiscardPlanningResult.Discarded, outcome.Result);
|
||||
Assert.Null(await _tasks.GetByIdAsync(c1.Id));
|
||||
Assert.Null(await _tasks.GetByIdAsync(c2.Id));
|
||||
|
||||
@@ -226,9 +226,9 @@ public sealed class TaskRepositoryPlanningTests : IDisposable
|
||||
var task = MakeTask(listId);
|
||||
await _tasks.AddAsync(task);
|
||||
|
||||
var ok = await _tasks.DiscardPlanningAsync(task.Id);
|
||||
var outcome = await _tasks.DiscardPlanningAsync(task.Id, dequeueQueuedChildren: false);
|
||||
|
||||
Assert.False(ok);
|
||||
Assert.Equal(DiscardPlanningResult.NotInPlanning, outcome.Result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Ui.Services;
|
||||
using ClaudeDo.Ui.ViewModels.Islands;
|
||||
using CommunityToolkit.Mvvm.Input;
|
||||
@@ -45,7 +46,11 @@ sealed class FakeWorkerClient : IWorkerClient
|
||||
public Task OpenInteractiveTerminalAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task QueuePlanningSubtasksAsync(string parentTaskId, CancellationToken ct = default) => Task.CompletedTask;
|
||||
public Task ResumePlanningSessionAsync(string taskId, CancellationToken ct = default) { ResumePlanningCalls++; return Task.CompletedTask; }
|
||||
public Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default) { DiscardPlanningCalls++; return Task.CompletedTask; }
|
||||
public Task<DiscardPlanningOutcome> DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default)
|
||||
{
|
||||
DiscardPlanningCalls++;
|
||||
return Task.FromResult(new DiscardPlanningOutcome(DiscardPlanningResult.Discarded, 0, 0));
|
||||
}
|
||||
public Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default) { FinalizePlanningCalls++; return Task.CompletedTask; }
|
||||
public Task<int> GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user