From d094a21e099d692bebf1a3c1cf574bf11c153775 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Mon, 18 May 2026 16:02:15 +0200 Subject: [PATCH] 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) --- .../Repositories/DiscardPlanningOutcome.cs | 18 ++ .../Repositories/TaskRepository.cs | 81 ++++++- src/ClaudeDo.Ui/Services/IWorkerClient.cs | 3 +- src/ClaudeDo.Ui/Services/WorkerClient.cs | 14 +- .../Islands/TasksIslandViewModel.cs | 44 +++- .../Views/Islands/TasksIslandView.axaml.cs | 47 ++++ src/ClaudeDo.Worker/Hub/WorkerHub.cs | 11 +- .../Lifecycle/OrphanRecovery.cs | 38 +++ .../Planning/PlanningSessionManager.cs | 12 +- src/ClaudeDo.Worker/Program.cs | 1 + .../ConflictResolutionViewModelTests.cs | 3 +- .../ViewModels/DetailsIslandPlanningTests.cs | 3 +- .../ViewModels/PlanningDiffViewModelTests.cs | 3 +- .../Planning/PlanningSessionManagerTests.cs | 4 +- .../TaskRepositoryOrphanGuardTests.cs | 216 ++++++++++++++++++ .../TaskRepositoryPlanningTests.cs | 8 +- .../UiVm/TasksIslandViewModelPlanningTests.cs | 7 +- 17 files changed, 481 insertions(+), 32 deletions(-) create mode 100644 src/ClaudeDo.Data/Repositories/DiscardPlanningOutcome.cs create mode 100644 src/ClaudeDo.Worker/Lifecycle/OrphanRecovery.cs create mode 100644 tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryOrphanGuardTests.cs diff --git a/src/ClaudeDo.Data/Repositories/DiscardPlanningOutcome.cs b/src/ClaudeDo.Data/Repositories/DiscardPlanningOutcome.cs new file mode 100644 index 0000000..1e84810 --- /dev/null +++ b/src/ClaudeDo.Data/Repositories/DiscardPlanningOutcome.cs @@ -0,0 +1,18 @@ +namespace ClaudeDo.Data.Repositories; + +public enum DiscardPlanningResult +{ + /// Planning state cleared, children handled. + Discarded, + /// Parent not found or not in PlanningPhase.Active. + NotInPlanning, + /// At least one child is Queued and the caller did not opt in to auto-dequeue. + BlockedByQueuedChildren, + /// At least one child is Running; user must cancel it before discarding. + BlockedByRunningChildren, +} + +public readonly record struct DiscardPlanningOutcome( + DiscardPlanningResult Result, + int QueuedChildrenCount, + int RunningChildrenCount); diff --git a/src/ClaudeDo.Data/Repositories/TaskRepository.cs b/src/ClaudeDo.Data/Repositories/TaskRepository.cs index a6d9d68..23bcf22 100644 --- a/src/ClaudeDo.Data/Repositories/TaskRepository.cs +++ b/src/ClaudeDo.Data/Repositories/TaskRepository.cs @@ -258,9 +258,15 @@ public sealed class TaskRepository string? commitType, CancellationToken ct = default) { - var parent = await _context.Tasks.FirstOrDefaultAsync(t => t.Id == parentId, ct); + // AsNoTracking: SetPlanningStartedAsync mutates via ExecuteUpdate which + // bypasses the change tracker; a tracked Find would return stale data. + var parent = await _context.Tasks.AsNoTracking() + .FirstOrDefaultAsync(t => t.Id == parentId, ct); if (parent is null) throw new InvalidOperationException($"Parent task {parentId} not found."); + if (parent.PlanningPhase == PlanningPhase.None) + throw new InvalidOperationException( + $"Parent task {parentId} is not in a planning phase; cannot attach children."); var maxSort = await _context.Tasks .Where(t => t.ListId == parent.ListId) @@ -401,8 +407,9 @@ public sealed class TaskRepository .FirstOrDefaultAsync(t => t.PlanningSessionToken == token, ct); } - public async Task DiscardPlanningAsync( + public async Task DiscardPlanningAsync( string parentId, + bool dequeueQueuedChildren, CancellationToken ct = default) { using var tx = await _context.Database.BeginTransactionAsync(ct); @@ -413,10 +420,54 @@ public sealed class TaskRepository if (parent is null || parent.PlanningPhase != PlanningPhase.Active) { await tx.RollbackAsync(ct); - return false; + return new DiscardPlanningOutcome(DiscardPlanningResult.NotInPlanning, 0, 0); } - // Children created during the planning session are Status=Idle, PlanningPhase=None. + var children = await _context.Tasks + .Where(t => t.ParentTaskId == parentId) + .Select(t => new { t.Id, t.Status }) + .ToListAsync(ct); + + var runningCount = children.Count(c => c.Status == TaskStatus.Running); + if (runningCount > 0) + { + await tx.RollbackAsync(ct); + return new DiscardPlanningOutcome(DiscardPlanningResult.BlockedByRunningChildren, 0, runningCount); + } + + var queuedIds = children.Where(c => c.Status == TaskStatus.Queued).Select(c => c.Id).ToList(); + if (queuedIds.Count > 0) + { + if (!dequeueQueuedChildren) + { + await tx.RollbackAsync(ct); + return new DiscardPlanningOutcome(DiscardPlanningResult.BlockedByQueuedChildren, queuedIds.Count, 0); + } + + await _context.Tasks + .Where(t => queuedIds.Contains(t.Id)) + .ExecuteUpdateAsync(s => s + .SetProperty(t => t.Status, TaskStatus.Idle) + .SetProperty(t => t.BlockedByTaskId, (string?)null), ct); + } + + // Terminal children (Done/Failed/Cancelled) survive the discard but cannot remain + // attached: their parent's PlanningPhase is about to be reset to None, which would + // make them orphans. Promote them to top-level. + var terminalIds = children + .Where(c => c.Status == TaskStatus.Done + || c.Status == TaskStatus.Failed + || c.Status == TaskStatus.Cancelled) + .Select(c => c.Id) + .ToList(); + if (terminalIds.Count > 0) + { + await _context.Tasks + .Where(t => terminalIds.Contains(t.Id)) + .ExecuteUpdateAsync(s => s.SetProperty(t => t.ParentTaskId, (string?)null), ct); + } + + // Idle children created during this planning session are dropped. await _context.Tasks .Where(t => t.ParentTaskId == parentId && t.Status == TaskStatus.Idle @@ -433,7 +484,27 @@ public sealed class TaskRepository .SetProperty(t => t.PlanningFinalizedAt, (DateTime?)null), ct); await tx.CommitAsync(ct); - return true; + return new DiscardPlanningOutcome(DiscardPlanningResult.Discarded, queuedIds.Count, 0); + } + + /// + /// Clears ParentTaskId on rows whose parent is missing or no longer in a + /// planning phase. Returns the number of rows repaired. Idempotent. + /// + internal async Task RepairOrphanedChildrenAsync(CancellationToken ct = default) + { + var orphanIds = await _context.Tasks + .Where(t => t.ParentTaskId != null) + .Where(t => !_context.Tasks.Any(p => + p.Id == t.ParentTaskId && p.PlanningPhase != PlanningPhase.None)) + .Select(t => t.Id) + .ToListAsync(ct); + + if (orphanIds.Count == 0) return 0; + + return await _context.Tasks + .Where(t => orphanIds.Contains(t.Id)) + .ExecuteUpdateAsync(s => s.SetProperty(t => t.ParentTaskId, (string?)null), ct); } public async Task TryCompleteParentAsync( diff --git a/src/ClaudeDo.Ui/Services/IWorkerClient.cs b/src/ClaudeDo.Ui/Services/IWorkerClient.cs index 1a618b6..f9522c6 100644 --- a/src/ClaudeDo.Ui/Services/IWorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/IWorkerClient.cs @@ -1,5 +1,6 @@ using System.ComponentModel; using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; using TaskStatus = ClaudeDo.Data.Models.TaskStatus; namespace ClaudeDo.Ui.Services; @@ -34,7 +35,7 @@ public interface IWorkerClient : INotifyPropertyChanged Task StartPlanningSessionAsync(string taskId, CancellationToken ct = default); Task OpenInteractiveTerminalAsync(string taskId, CancellationToken ct = default); Task ResumePlanningSessionAsync(string taskId, CancellationToken ct = default); - Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default); + Task DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default); Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default); Task GetPendingDraftCountAsync(string taskId, CancellationToken ct = default); Task GetMergeTargetsAsync(string taskId); diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index 7baf9a9..6bd6d35 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -1,9 +1,11 @@ using System.Collections.ObjectModel; using Avalonia.Threading; using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; using CommunityToolkit.Mvvm.ComponentModel; using Microsoft.AspNetCore.SignalR; using Microsoft.AspNetCore.SignalR.Client; +using Microsoft.Extensions.DependencyInjection; namespace ClaudeDo.Ui.Services; @@ -64,6 +66,10 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC _hub = new HubConnectionBuilder() .WithUrl(signalRUrl) .WithAutomaticReconnect(new IndefiniteRetryPolicy()) + .AddJsonProtocol(options => + { + options.PayloadSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter()); + }) .Build(); _hub.Reconnected += async _ => @@ -436,8 +442,8 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC public async Task OpenInteractiveTerminalAsync(string taskId, CancellationToken ct = default) => await _hub.InvokeAsync("OpenInteractiveTerminalAsync", taskId, ct); - public async Task DiscardPlanningSessionAsync(string taskId, CancellationToken ct = default) - => await _hub.InvokeAsync("DiscardPlanningSessionAsync", taskId, ct); + public async Task DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false, CancellationToken ct = default) + => await _hub.InvokeAsync("DiscardPlanningSessionAsync", taskId, dequeueQueuedChildren, ct); public async Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true, CancellationToken ct = default) => await _hub.InvokeAsync("FinalizePlanningSessionAsync", taskId, queueAgentTasks, ct); @@ -496,8 +502,8 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC => await StartPlanningSessionAsync(taskId, ct); async Task IWorkerClient.ResumePlanningSessionAsync(string taskId, CancellationToken ct) => await ResumePlanningSessionAsync(taskId, ct); - async Task IWorkerClient.DiscardPlanningSessionAsync(string taskId, CancellationToken ct) - => await DiscardPlanningSessionAsync(taskId, ct); + async Task IWorkerClient.DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren, CancellationToken ct) + => await DiscardPlanningSessionAsync(taskId, dequeueQueuedChildren, ct); async Task IWorkerClient.FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks, CancellationToken ct) => await FinalizePlanningSessionAsync(taskId, queueAgentTasks, ct); async Task IWorkerClient.GetPendingDraftCountAsync(string taskId, CancellationToken ct) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index f0187d8..8360657 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -5,6 +5,7 @@ using CommunityToolkit.Mvvm.Input; using ClaudeDo.Data; using ClaudeDo.Data.Filtering; using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; using ClaudeDo.Ui.Services; using ClaudeDo.Ui.ViewModels.Modals; using Microsoft.EntityFrameworkCore; @@ -647,7 +648,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase await _worker.FinalizePlanningSessionAsync(row.Id); break; case UnfinishedPlanningModalResult.Discard: - await _worker.DiscardPlanningSessionAsync(row.Id); + await TryDiscardPlanningWithRetryAsync(row.Id); break; case UnfinishedPlanningModalResult.Cancel: default: @@ -660,11 +661,46 @@ public sealed partial class TasksIslandViewModel : ViewModelBase [RelayCommand] private async Task DiscardPlanningSessionAsync(TaskRowViewModel? row) { - if (row is null) return; - try { await _worker!.DiscardPlanningSessionAsync(row.Id); } - catch { } + if (row is null || _worker is null) return; + await TryDiscardPlanningWithRetryAsync(row.Id); } + /// + /// Calls discard, and if it is blocked because children are queued, prompts the + /// user to dequeue them and retries. Running children are surfaced as a hard + /// block — the user must cancel them first. + /// + private async Task TryDiscardPlanningWithRetryAsync(string taskId) + { + if (_worker is null) return; + DiscardPlanningOutcome outcome; + try { outcome = await _worker.DiscardPlanningSessionAsync(taskId); } + catch { return; } + + if (outcome.Result == DiscardPlanningResult.BlockedByQueuedChildren) + { + if (ConfirmAsync is null) return; + var ok = await ConfirmAsync( + $"{outcome.QueuedChildrenCount} child task(s) are queued.\n" + + "Dequeue them and discard the planning session?"); + if (!ok) return; + try { await _worker.DiscardPlanningSessionAsync(taskId, dequeueQueuedChildren: true); } + catch { } + } + else if (outcome.Result == DiscardPlanningResult.BlockedByRunningChildren) + { + if (ConfirmAsync is null) return; + await ConfirmAsync( + $"{outcome.RunningChildrenCount} child task(s) are still running.\n" + + "Cancel them first, then try again."); + } + } + + /// + /// Wired by the view via . Returns true when the user confirms. + /// + public Func>? ConfirmAsync { get; set; } + [RelayCommand] private async Task QueuePlanningSubtasksAsync(TaskRowViewModel? row) { diff --git a/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml.cs b/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml.cs index 882efa1..a0f30aa 100644 --- a/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml.cs +++ b/src/ClaudeDo.Ui/Views/Islands/TasksIslandView.axaml.cs @@ -2,6 +2,8 @@ using Avalonia; using Avalonia.Controls; using Avalonia.Input; using Avalonia.Interactivity; +using Avalonia.Layout; +using Avalonia.Media; using Avalonia.VisualTree; using ClaudeDo.Ui.ViewModels.Islands; using ClaudeDo.Ui.ViewModels.Modals; @@ -33,10 +35,55 @@ public partial class TasksIslandView : UserControl await modal.ShowDialog(owner); // ShowDialog completes once the window is closed (CloseAction or OS close). }; + vm.ConfirmAsync = ShowConfirmAsync; } }; } + private async System.Threading.Tasks.Task ShowConfirmAsync(string message) + { + var owner = TopLevel.GetTopLevel(this) as Window; + if (owner is null) return false; + + var tcs = new TaskCompletionSource(); + var cancel = new Button { Content = "Cancel", MinWidth = 90 }; + var confirm = new Button { Content = "Confirm", MinWidth = 90, Classes = { "danger" } }; + + var dialog = new Window + { + Title = "Confirm", + Width = 380, + SizeToContent = SizeToContent.Height, + CanResize = false, + WindowStartupLocation = WindowStartupLocation.CenterOwner, + ShowInTaskbar = false, + Background = this.FindResource("SurfaceBrush") as IBrush, + Content = new StackPanel + { + Margin = new Thickness(20), + Spacing = 16, + Children = + { + new TextBlock { Text = message, TextWrapping = TextWrapping.Wrap }, + new StackPanel + { + Orientation = Orientation.Horizontal, + HorizontalAlignment = HorizontalAlignment.Right, + Spacing = 8, + Children = { cancel, confirm }, + }, + }, + }, + }; + + cancel.Click += (_, _) => { tcs.TrySetResult(false); dialog.Close(); }; + confirm.Click += (_, _) => { tcs.TrySetResult(true); dialog.Close(); }; + dialog.Closed += (_, _) => tcs.TrySetResult(false); + + _ = dialog.ShowDialog(owner); + return await tcs.Task; + } + private async void OnTunnelPointerPressed(object? sender, PointerPressedEventArgs e) { if (DataContext is not TasksIslandViewModel vm) return; diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 3e60811..1f360c7 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -388,7 +388,8 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub } catch (PlanningLaunchException) { - await _planning.DiscardAsync(taskId, Context.ConnectionAborted); + // Launch failed before any children could be created; force-cleanup is safe. + await _planning.DiscardAsync(taskId, dequeueQueuedChildren: true, Context.ConnectionAborted); throw; } await Clients.All.SendAsync("TaskUpdated", taskId); @@ -408,10 +409,12 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub await _launcher.LaunchInteractiveAsync(ctx, Context.ConnectionAborted); } - public async Task DiscardPlanningSessionAsync(string taskId) + public async Task DiscardPlanningSessionAsync(string taskId, bool dequeueQueuedChildren = false) { - await _planning.DiscardAsync(taskId, Context.ConnectionAborted); - await Clients.All.SendAsync("TaskUpdated", taskId); + var outcome = await _planning.DiscardAsync(taskId, dequeueQueuedChildren, Context.ConnectionAborted); + if (outcome.Result == DiscardPlanningResult.Discarded) + await Clients.All.SendAsync("TaskUpdated", taskId); + return outcome; } public async Task FinalizePlanningSessionAsync(string taskId, bool queueAgentTasks = true) diff --git a/src/ClaudeDo.Worker/Lifecycle/OrphanRecovery.cs b/src/ClaudeDo.Worker/Lifecycle/OrphanRecovery.cs new file mode 100644 index 0000000..068a5d8 --- /dev/null +++ b/src/ClaudeDo.Worker/Lifecycle/OrphanRecovery.cs @@ -0,0 +1,38 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Repositories; +using Microsoft.EntityFrameworkCore; + +namespace ClaudeDo.Worker.Lifecycle; + +/// +/// Startup-only sweep: clears ParentTaskId on rows whose parent is missing or +/// no longer in a planning phase. These rows would otherwise be invisible in the UI +/// (the parent doesn't render as a planning header) and cannot reach a terminal state +/// through the chain coordinator. Promoting them to top-level restores both. +/// +public sealed class OrphanRecovery : IHostedService +{ + private readonly IDbContextFactory _dbFactory; + private readonly ILogger _logger; + + public OrphanRecovery( + IDbContextFactory dbFactory, + ILogger logger) + { + _dbFactory = dbFactory; + _logger = logger; + } + + public async Task StartAsync(CancellationToken cancellationToken) + { + await using var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken); + var repo = new TaskRepository(ctx); + var repaired = await repo.RepairOrphanedChildrenAsync(cancellationToken); + if (repaired > 0) + _logger.LogWarning("Orphan recovery: promoted {Count} orphaned child task(s) to top-level", repaired); + else + _logger.LogInformation("Orphan recovery: no orphans found"); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; +} diff --git a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs index 37dbafc..2ed496f 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs @@ -236,12 +236,17 @@ public sealed class PlanningSessionManager return children.Count(c => c.Status == TaskStatus.Idle); } - public async Task DiscardAsync(string taskId, CancellationToken ct) + public async Task DiscardAsync( + string taskId, + bool dequeueQueuedChildren, + CancellationToken ct) { var (tasks, lists, settings, ctx) = CreateRepos(); await using var __ = ctx; - var ok = await tasks.DiscardPlanningAsync(taskId, ct); + var outcome = await tasks.DiscardPlanningAsync(taskId, dequeueQueuedChildren, ct); + if (outcome.Result != DiscardPlanningResult.Discarded) + return outcome; await TryCleanupWorktreeAsync(taskId, lists, settings, ct); @@ -251,8 +256,7 @@ public sealed class PlanningSessionManager try { Directory.Delete(sessionDir, recursive: true); } catch { } } - if (!ok) - throw new InvalidOperationException($"Task {taskId} was not in Planning state; nothing to discard."); + return outcome; } public async Task ResumeAsync(string taskId, CancellationToken ct) diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index d89ab79..39fa7e2 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -27,6 +27,7 @@ builder.Services.AddDbContextFactory(opt => builder.Services.AddSingleton(cfg); builder.Services.AddHostedService(); +builder.Services.AddHostedService(); builder.Services.AddSignalR().AddJsonProtocol(options => { options.PayloadSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter()); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs index cea22c2..59cf26d 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs @@ -42,7 +42,8 @@ public class ConflictResolutionViewModelTests public Task> GetAllTagsAsync() => Task.FromResult(new List()); 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 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 GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0); public Task GetMergeTargetsAsync(string taskId) => Task.FromResult(null); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs index a729852..5b79337 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs @@ -72,7 +72,8 @@ public class DetailsIslandPlanningTests : IDisposable public Task> GetAllTagsAsync() => Task.FromResult(new List()); 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 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 GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0); public Task GetMergeTargetsAsync(string taskId) => Task.FromResult(MergeTargetsResult); diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/PlanningDiffViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/PlanningDiffViewModelTests.cs index 7160de3..0b054eb 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/PlanningDiffViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/PlanningDiffViewModelTests.cs @@ -39,7 +39,8 @@ public class PlanningDiffViewModelTests public Task> GetAllTagsAsync() => Task.FromResult(new List()); 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 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 GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0); public Task GetMergeTargetsAsync(string taskId) => Task.FromResult(null); diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs index ab377fd..0f1c901 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs @@ -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 diff --git a/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryOrphanGuardTests.cs b/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryOrphanGuardTests.cs new file mode 100644 index 0000000..9fe4d92 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryOrphanGuardTests.cs @@ -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; + +/// +/// Covers the invariant that no task may have ParentTaskId pointing to a +/// parent without PlanningPhase.Active|Finalized. Tests the three guard +/// rails: CreateChildAsync validation, DiscardPlanningAsync +/// gating with the optional dequeue path, and the startup repair sweep. +/// +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 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 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( + () => _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; + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryPlanningTests.cs b/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryPlanningTests.cs index dcabc98..ddd33f8 100644 --- a/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryPlanningTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryPlanningTests.cs @@ -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] diff --git a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs index 457f5a5..14b2e2c 100644 --- a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs +++ b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs @@ -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 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 GetPendingDraftCountAsync(string taskId, CancellationToken ct = default) => Task.FromResult(0);