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
@@ -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