fix(worker,ui): route details-pane task delete through worker to advance blocked parents

Deleting a child from the details pane hard-deleted straight from the UI
process via TaskRepository, bypassing TaskStateService.TryAdvanceParentAsync
entirely. Deleting the last child of a WaitingForChildren parent that way
left it wedged forever. WorkerHub.DeleteTask now mirrors the MCP delete_task
tool (running-task guard, FK-friendly error, advance-parent call), and the
UI goes through it.

TryAdvanceParentAsync also short-circuited when zero children remained,
treating "no children left" as "nothing to evaluate" instead of "all done" -
removed the early return so an empty child list (vacuously) counts as all
terminal.
This commit is contained in:
mika kuns
2026-08-06 13:25:49 +02:00
parent 0d1e3b9a6f
commit 9e2a15e421
11 changed files with 324 additions and 12 deletions
@@ -50,6 +50,9 @@ How a task moves Queued → Running → terminal, across `src/ClaudeDo.Worker`
- **Done** CompleteAsync (Running → Done) — top-level success.
- **WaitingForReview** SubmitForReviewAsync (Running → WaitingForReview) — review gate.
- **WaitingForChildren** SubmitForChildrenAsync (Running → WaitingForChildren) — blocks on children.
Advances to WaitingForReview via `TryAdvanceParentAsync` once every remaining child is
terminal (Done/Failed/Cancelled) — including zero children left, e.g. after the last child
is deleted (`WorkerHub.DeleteTask` / `ExternalMcpService.DeleteTask` both call it).
- **Failed** FailAsync (Running/Queued → Failed).
- **Cancelled** CancelAsync (Running/Queued/WaitingForReview/WaitingForChildren → Cancelled).
@@ -57,6 +57,10 @@ public interface IWorkerClient : INotifyPropertyChanged
Task<PendingQuestionDto?> GetPendingQuestionAsync(string taskId);
Task ResetTaskAsync(string taskId);
Task CancelTaskAsync(string taskId);
/// <summary>Deletes a task via the worker (mirrors the MCP delete_task tool), so a deleted
/// child correctly advances a WaitingForChildren parent. Returns (false, message) instead of
/// throwing when the task has children or is running, preserving the FK-error UX.</summary>
Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId);
Task<List<AgentInfo>> GetAgentsAsync();
Task RefreshAgentsAsync();
Task<SeedResultDto?> RestoreDefaultAgentsAsync();
+13
View File
@@ -327,6 +327,19 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC
await _hub.InvokeAsync("CancelTask", taskId);
}
public async Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId)
{
try
{
await _hub.InvokeAsync("DeleteTask", taskId, CancellationToken.None);
return (true, null);
}
catch (HubException ex)
{
return (false, ex.Message);
}
}
public async Task WakeQueueAsync()
{
await _hub.InvokeAsync("WakeQueue");
@@ -946,18 +946,14 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
var ok = await ConfirmAsync($"Delete \"{row.Title}\"? This cannot be undone.");
if (!ok) return;
}
try
{
await using var ctx = _dbFactory.CreateDbContext();
var repo = new TaskRepository(ctx);
await repo.DeleteAsync(row.Id);
}
catch (Microsoft.EntityFrameworkCore.DbUpdateException ex) when (
ex.Message.Contains("FOREIGN KEY", StringComparison.OrdinalIgnoreCase)
|| ex.InnerException?.Message.Contains("FOREIGN KEY", StringComparison.OrdinalIgnoreCase) == true)
// Routed through the worker (mirrors the MCP delete_task tool) so a deleted child
// correctly advances a WaitingForChildren parent — a direct-repo delete from here used
// to bypass TaskStateService.TryAdvanceParentAsync and could wedge the parent forever.
var (deleted, error) = await _worker.DeleteTaskAsync(row.Id);
if (!deleted)
{
if (ShowErrorAsync != null)
await ShowErrorAsync("This task has child tasks. Discard the planning session or delete child tasks first.");
await ShowErrorAsync(error ?? "Delete failed.");
return;
}
if (DeleteFromList != null)
+32
View File
@@ -380,6 +380,38 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
public bool CancelTask(string taskId) => _queue.CancelTask(taskId);
// Mirrors ExternalMcpService.DeleteTask so a UI-initiated delete gets the same
// TryAdvanceParentAsync side effect — a direct-repo delete from the details pane
// used to skip it, permanently wedging a WaitingForChildren parent whose last
// child was deleted from there.
public async Task DeleteTask(string taskId, CancellationToken cancellationToken)
{
await using var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken);
var repo = new TaskRepository(ctx);
var task = await repo.GetByIdAsync(taskId, cancellationToken)
?? throw new HubException("task not found");
if (task.Status == TaskStatus.Running)
throw new HubException("Cannot delete a running task. Cancel it first.");
try
{
await repo.DeleteAsync(taskId, cancellationToken);
}
// TaskRepository.DeleteAsync uses ExecuteDeleteAsync, which bypasses SaveChanges and
// surfaces provider errors directly as SqliteException rather than DbUpdateException.
catch (Exception ex) when (
(ex is Microsoft.Data.Sqlite.SqliteException || ex.InnerException is Microsoft.Data.Sqlite.SqliteException)
&& (ex.Message.Contains("FOREIGN KEY", StringComparison.OrdinalIgnoreCase)
|| ex.InnerException?.Message.Contains("FOREIGN KEY", StringComparison.OrdinalIgnoreCase) == true))
{
throw new HubException("This task has child tasks. Discard the planning session or delete child tasks first.");
}
if (task.ParentTaskId is not null)
await _state.TryAdvanceParentAsync(task.ParentTaskId);
await _broadcaster.TaskUpdated(taskId);
}
public void WakeQueue() => _waker.Wake();
public async Task<List<AgentInfo>> GetAgents() => await _agentService.ScanAsync();
@@ -505,8 +505,8 @@ public sealed class TaskStateService : ITaskStateService
.Select(t => t.Status)
.ToListAsync(CancellationToken.None);
}
if (childStatuses.Count == 0) return;
// No early-out on an empty list: zero children left (e.g. the last one was just
// deleted) counts as "all terminal" — .All() on an empty sequence is vacuously true.
bool allTerminal = childStatuses.All(s =>
s == TaskStatus.Done || s == TaskStatus.Failed || s == TaskStatus.Cancelled);
if (!allTerminal) return;
@@ -79,6 +79,7 @@ public abstract class StubWorkerClient : IWorkerClient
public virtual Task<PendingQuestionDto?> GetPendingQuestionAsync(string taskId) => Task.FromResult(PendingQuestion);
public virtual Task ResetTaskAsync(string taskId) => Task.CompletedTask;
public virtual Task CancelTaskAsync(string taskId) => Task.CompletedTask;
public virtual Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId) => Task.FromResult<(bool, string?)>((true, null));
public virtual Task<List<AgentInfo>> GetAgentsAsync() => Task.FromResult(new List<AgentInfo>());
public virtual Task RefreshAgentsAsync() => Task.CompletedTask;
public virtual Task<SeedResultDto?> RestoreDefaultAgentsAsync() => Task.FromResult<SeedResultDto?>(null);
@@ -0,0 +1,117 @@
using ClaudeDo.Data;
using ClaudeDo.Data.Models;
using ClaudeDo.Ui.Services;
using ClaudeDo.Ui.ViewModels.Islands;
using Microsoft.EntityFrameworkCore;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
namespace ClaudeDo.Ui.Tests.ViewModels;
public class DetailsIslandDeleteTaskTests : IDisposable
{
private readonly string _dbPath;
public DetailsIslandDeleteTaskTests()
{
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_delete_task_test_{Guid.NewGuid():N}.db");
using var ctx = NewContext();
ctx.Database.EnsureCreated();
}
public void Dispose()
{
try { File.Delete(_dbPath); } catch { }
try { File.Delete(_dbPath + "-wal"); } catch { }
try { File.Delete(_dbPath + "-shm"); } catch { }
}
private ClaudeDoDbContext NewContext()
{
var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
.UseSqlite($"Data Source={_dbPath}")
.Options;
return new ClaudeDoDbContext(opts);
}
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
{
private readonly Func<ClaudeDoDbContext> _create;
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
public ClaudeDoDbContext CreateDbContext() => _create();
}
private sealed class NullServiceProvider : IServiceProvider
{
public object? GetService(Type serviceType) => null;
}
private sealed class StubNotesApi : ClaudeDo.Ui.Services.Interfaces.INotesApi
{
public Task<List<DailyNoteDto>> ListAsync(DateOnly day) =>
Task.FromResult(new List<DailyNoteDto>());
public Task<DailyNoteDto?> AddAsync(DateOnly day, string text) =>
Task.FromResult<DailyNoteDto?>(null);
public Task UpdateAsync(string id, string text) => Task.CompletedTask;
public Task DeleteAsync(string id) => Task.CompletedTask;
}
private sealed class RecordingWorkerClient : StubWorkerClient
{
public override bool IsConnected => true;
public (bool Ok, string? Error) Result { get; set; } = (true, null);
public string? DeletedTaskId { get; private set; }
public override Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId)
{
DeletedTaskId = taskId;
return Task.FromResult(Result);
}
}
private DetailsIslandViewModel BuildVm(RecordingWorkerClient worker)
{
var factory = new TestDbFactory(NewContext);
return new DetailsIslandViewModel(factory, worker, new NullServiceProvider(), new StubNotesApi(), new ClaudeDo.Ui.Services.MergeCoordinator());
}
[Fact]
public async Task DeleteTask_Succeeds_RemovesFromListAndClosesDetail()
{
var worker = new RecordingWorkerClient { Result = (true, null) };
var vm = BuildVm(worker);
vm.Bind(new TaskRowViewModel { Id = "task-del-1", Status = TaskStatus.Idle });
TaskRowViewModel? removed = null;
vm.DeleteFromList = row => { removed = row; return Task.CompletedTask; };
var closed = false;
vm.CloseDetail = () => closed = true;
await vm.DeleteTaskCommand.ExecuteAsync(null);
Assert.Equal("task-del-1", worker.DeletedTaskId);
Assert.Equal("task-del-1", removed?.Id);
Assert.True(closed);
}
[Fact]
public async Task DeleteTask_WhenTaskHasChildren_SurfacesErrorAndKeepsDetailOpen()
{
const string fkMessage = "This task has child tasks. Discard the planning session or delete child tasks first.";
var worker = new RecordingWorkerClient { Result = (false, fkMessage) };
var vm = BuildVm(worker);
vm.Bind(new TaskRowViewModel { Id = "task-del-2", Status = TaskStatus.WaitingForChildren });
var deleteFromListCalled = false;
vm.DeleteFromList = _ => { deleteFromListCalled = true; return Task.CompletedTask; };
var closed = false;
vm.CloseDetail = () => closed = true;
string? reportedError = null;
vm.ShowErrorAsync = msg => { reportedError = msg; return Task.CompletedTask; };
await vm.DeleteTaskCommand.ExecuteAsync(null);
Assert.Equal(fkMessage, reportedError);
Assert.False(deleteFromListCalled);
Assert.False(closed);
}
}
@@ -0,0 +1,107 @@
using ClaudeDo.Data.Models;
using ClaudeDo.Data.Repositories;
using ClaudeDo.Worker.Hub;
using ClaudeDo.Worker.Tests.Infrastructure;
using Microsoft.AspNetCore.SignalR;
using Microsoft.EntityFrameworkCore;
using Xunit;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
namespace ClaudeDo.Worker.Tests.Hub;
public sealed class DeleteTaskHubTests : IDisposable
{
private readonly DbFixture _db = new();
public void Dispose() => _db.Dispose();
private WorkerHub CreateHub()
{
var factory = _db.CreateFactory();
var built = TaskStateServiceBuilder.Build(factory);
var broadcaster = new HubBroadcaster(new CapturingHubContext());
var hub = new WorkerHub(
null!, null!, null!, null!, broadcaster, factory,
null!, null!, null!, null!, null!, null!, null!, null!, null!, null!,
built.State,
null!, null!,
null!, new ClaudeDo.Worker.Online.OnlineInboxConfig(), new ClaudeDo.Worker.Online.OnlineTokenStore(),
new ClaudeDo.Worker.Runner.PendingQuestionRegistry(), null!);
hub.Clients = new FakeHubCallerClients(new RecordingClientProxy());
hub.Context = new FakeHubCallerContext();
return hub;
}
private async Task<(string ParentId, string ChildId)> SeedParentWithChildAsync(TaskStatus childStatus)
{
using var ctx = _db.CreateContext();
var listId = Guid.NewGuid().ToString();
await new ListRepository(ctx).AddAsync(new ListEntity
{
Id = listId, Name = "L", CreatedAt = DateTime.UtcNow,
});
var repo = new TaskRepository(ctx);
var parentId = Guid.NewGuid().ToString();
await repo.AddAsync(new TaskEntity
{
Id = parentId, ListId = listId, Title = "Parent",
Status = TaskStatus.WaitingForChildren, PlanningPhase = PlanningPhase.Finalized,
CreatedAt = DateTime.UtcNow, CommitType = "feat",
});
var childId = Guid.NewGuid().ToString();
await repo.AddAsync(new TaskEntity
{
Id = childId, ListId = listId, Title = "Child", ParentTaskId = parentId,
Status = childStatus, CreatedAt = DateTime.UtcNow, CommitType = "feat",
});
return (parentId, childId);
}
[Fact]
public async Task DeleteTask_LastChildOfWaitingForChildrenParent_AdvancesParentToWaitingForReview()
{
var (parentId, childId) = await SeedParentWithChildAsync(TaskStatus.Idle);
var hub = CreateHub();
await hub.DeleteTask(childId, default);
await using var ctx = _db.CreateContext();
Assert.Null(await ctx.Tasks.FindAsync(childId));
var parent = await ctx.Tasks.FindAsync(parentId);
Assert.Equal(TaskStatus.WaitingForReview, parent!.Status);
}
[Fact]
public async Task DeleteTask_RunningTask_Throws_AndDoesNotDelete()
{
var (_, childId) = await SeedParentWithChildAsync(TaskStatus.Running);
var hub = CreateHub();
await Assert.ThrowsAsync<HubException>(() => hub.DeleteTask(childId, default));
await using var ctx = _db.CreateContext();
Assert.NotNull(await ctx.Tasks.FindAsync(childId));
}
[Fact]
public async Task DeleteTask_TaskWithChildren_ThrowsFriendlyForeignKeyMessage()
{
var (parentId, _) = await SeedParentWithChildAsync(TaskStatus.Idle);
var hub = CreateHub();
var ex = await Assert.ThrowsAsync<HubException>(() => hub.DeleteTask(parentId, default));
Assert.Contains("child tasks", ex.Message, StringComparison.OrdinalIgnoreCase);
}
[Fact]
public async Task DeleteTask_Missing_Throws()
{
var hub = CreateHub();
await Assert.ThrowsAsync<HubException>(() => hub.DeleteTask("does-not-exist", default));
}
}
@@ -479,4 +479,42 @@ public sealed class TaskStateServiceTests : IDisposable
Assert.Equal(TaskStatus.Queued, t2.Status);
Assert.Equal(c1, t2.BlockedByTaskId);
}
// ─── TryAdvanceParentAsync ────────────────────────────────────────────
[Fact]
public async Task TryAdvanceParentAsync_ZeroChildren_AdvancesToWaitingForReview()
{
// Mirrors deleting the last remaining child: no terminal transition fires for it,
// so the caller invokes TryAdvanceParentAsync directly with zero children left.
var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized);
await _sut.TryAdvanceParentAsync(parent);
Assert.Equal(TaskStatus.WaitingForReview, await GetStatusAsync(parent));
}
[Fact]
public async Task TryAdvanceParentAsync_AllChildrenTerminal_AdvancesToWaitingForReview()
{
var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized);
await SeedTaskAsync(TaskStatus.Done, parentId: parent);
await SeedTaskAsync(TaskStatus.Done, parentId: parent);
await _sut.TryAdvanceParentAsync(parent);
Assert.Equal(TaskStatus.WaitingForReview, await GetStatusAsync(parent));
}
[Fact]
public async Task TryAdvanceParentAsync_SomeChildrenStillRunning_DoesNotAdvance()
{
var parent = await SeedTaskAsync(TaskStatus.WaitingForChildren, phase: PlanningPhase.Finalized);
await SeedTaskAsync(TaskStatus.Done, parentId: parent);
await SeedTaskAsync(TaskStatus.Running, parentId: parent);
await _sut.TryAdvanceParentAsync(parent);
Assert.Equal(TaskStatus.WaitingForChildren, await GetStatusAsync(parent));
}
}
@@ -47,6 +47,7 @@ sealed class FakeWorkerClient : IWorkerClient
public Task<PendingQuestionDto?> GetPendingQuestionAsync(string taskId) => Task.FromResult<PendingQuestionDto?>(null);
public Task ResetTaskAsync(string taskId) => Task.CompletedTask;
public Task CancelTaskAsync(string taskId) => Task.CompletedTask;
public Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId) => Task.FromResult<(bool, string?)>((true, null));
public Task<List<AgentInfo>> GetAgentsAsync() => Task.FromResult(new List<AgentInfo>());
public Task RefreshAgentsAsync() => Task.CompletedTask;
public Task<SeedResultDto?> RestoreDefaultAgentsAsync() => Task.FromResult<SeedResultDto?>(null);