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:
@@ -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();
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user