From c1ab042d0a61735b2f016781a3b1889560f463c0 Mon Sep 17 00:00:00 2001 From: Mika Kuns Date: Wed, 26 Aug 2026 14:53:25 +0200 Subject: [PATCH] fix(ui): block delete on interactive-session tasks, clear ghost selection Delete (row context menu + detail header) now gates on HasInteractiveSession the same way SendToQueue already does, so an open ConPTY session's claude process can't be orphaned by deleting its task out from under it. Row removal (in-UI delete and the delta-refresh path for externally deleted tasks) now clears SelectedTask via SelectFrom(null, "row-removed") when the removed row was selected, so the detail pane no longer keeps showing a deleted task with live buttons. --- .../Islands/DetailsIslandViewModel.cs | 5 +- .../ViewModels/Islands/TaskRowViewModel.cs | 9 ++ .../Islands/TasksIslandViewModel.cs | 15 ++- .../Views/Islands/TaskRowView.axaml.cs | 2 +- .../DetailsIslandDeleteTaskTests.cs | 15 +++ .../ViewModels/TasksIslandDeleteGuardTests.cs | 118 ++++++++++++++++++ 6 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandDeleteGuardTests.cs diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index f33fdf0f..56ce41f2 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -1002,7 +1002,10 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable private void OnBoundTaskPropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e) { if (e.PropertyName == nameof(TaskRowViewModel.HasInteractiveSession)) + { ResetAndRetryCommand.NotifyCanExecuteChanged(); + DeleteTaskCommand.NotifyCanExecuteChanged(); + } } [RelayCommand] @@ -1099,7 +1102,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable CloseDetail?.Invoke(); } - private bool CanDeleteTask() => Task != null && _worker.IsConnected; + private bool CanDeleteTask() => Task != null && _worker.IsConnected && !Task.HasInteractiveSession; [RelayCommand] private async System.Threading.Tasks.Task CommitSubtaskEditAsync(SubtaskRowViewModel? row) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs index 0a38bd0a..87fa0100 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs @@ -219,6 +219,13 @@ public sealed partial class TaskRowViewModel : ViewModelBase } public string? PlanningDisabledReason => PlanningGate().Reason; + // Deleting out from under a live ConPTY session would orphan the claude process in Mission + // Control (it never gets a task-deleted signal) — same rationale as SendToQueueGate. + private (bool Can, string? Reason) DeleteGate() => + HasInteractiveSession ? (false, Loc.T("tasks.reasonInteractiveSession")) : (true, null); + public bool CanDeleteTask => DeleteGate().Can; + public string? DeleteDisabledReason => DeleteGate().Reason; + public bool HasSchedule => ScheduledFor.HasValue; // "Add to My Day" — shown on any task not already in My Day; a Done task has no place in // today's focus list. The mirror of "Remove from My Day" (gated on IsMyDay). @@ -402,6 +409,8 @@ public sealed partial class TaskRowViewModel : ViewModelBase OnPropertyChanged(nameof(StatusChipTooltip)); OnPropertyChanged(nameof(CanSendToQueue)); OnPropertyChanged(nameof(SendToQueueDisabledReason)); + OnPropertyChanged(nameof(CanDeleteTask)); + OnPropertyChanged(nameof(DeleteDisabledReason)); } partial void OnFailureReasonChanged(string? value) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index 1923ee08..6a045f9e 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -311,20 +311,28 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable } var existing = Items.FirstOrDefault(r => r.Id == taskId); + var removed = false; if (entity is null) { - if (existing is not null) Items.Remove(existing); + if (existing is not null) { Items.Remove(existing); removed = true; } } else { var matches = TaskMatchesList(entity, list); if (existing is not null && matches) existing.UpdateFromEntity(entity); - else if (existing is not null) Items.Remove(existing); + else if (existing is not null) { Items.Remove(existing); removed = true; } else if (matches) { LoadForList(list); return; } else return; } + // The row is gone from Items but SelectedTask (and the bound detail pane) would still + // point at it — happens when a task is deleted externally (MCP/another session) while + // selected here, since that path never runs SelectFrom(null, ...) like the in-UI delete + // does. + if (removed && SelectedTask == existing) + SelectFrom(null, "row-removed"); + // Keep the parent's HasQueuedSubtasks flag in sync when a child's status flips. if (entity is not null && !string.IsNullOrEmpty(entity.ParentTaskId)) { @@ -1191,7 +1199,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable [RelayCommand] private async Task DeleteTaskAsync(TaskRowViewModel? row) { - if (row is null || _worker is null) return; + if (row is null || row.HasInteractiveSession || _worker is null) return; if (ConfirmAsync is not null) { var ok = await ConfirmAsync(Loc.T("vm.tasksIsland.deleteTaskConfirm", row.Title)); @@ -1216,6 +1224,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable } Items.Remove(row); + if (SelectedTask == row) SelectFrom(null, "row-removed"); Regroup(); UpdateSubtitle(); TasksChanged?.Invoke(this, EventArgs.Empty); diff --git a/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs b/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs index 09bc4909..cc292ba7 100644 --- a/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs +++ b/src/ClaudeDo.Ui/Views/Islands/TaskRowView.axaml.cs @@ -98,7 +98,7 @@ public partial class TaskRowView : UserControl menu.Items.Add(schedule); menu.Items.Add(new Separator()); - menu.Items.Add(MakeItem("tasks.ctxDeleteTask", OnDeleteTaskClick)); + menu.Items.Add(MakeItem("tasks.ctxDeleteTask", OnDeleteTaskClick, reason: row.DeleteDisabledReason)); menu.Open(border); e.Handled = true; diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs index 45e38d00..198d94f3 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandDeleteTaskTests.cs @@ -110,6 +110,21 @@ public class DetailsIslandDeleteTaskTests : IDisposable Assert.False(vm.DeleteTaskCommand.CanExecute(null)); } + [Fact] + public void DeleteTask_WhenTaskHasInteractiveSession_CommandIsDisabled() + { + var worker = new RecordingWorkerClient(); + var vm = BuildVm(worker); + var row = new TaskRowViewModel { Id = "task-del-5", Status = TaskStatus.Idle }; + vm.Bind(row); + + Assert.True(vm.DeleteTaskCommand.CanExecute(null)); + + row.HasInteractiveSession = true; + + Assert.False(vm.DeleteTaskCommand.CanExecute(null)); + } + [Fact] public async Task DeleteTask_WhenWorkerThrowsInvalidOperationException_SurfacesErrorAndKeepsDetailOpen() { diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandDeleteGuardTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandDeleteGuardTests.cs new file mode 100644 index 00000000..f90ffb69 --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/TasksIslandDeleteGuardTests.cs @@ -0,0 +1,118 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Ui.ViewModels.Islands; +using Microsoft.EntityFrameworkCore; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Ui.Tests.ViewModels; + +// Deleting a row with a live ConPTY session would orphan the claude process in Mission Control +// (WorkerHub.DeleteTask only blocks Status == Running, an interactive session stays Idle) — the +// row command must gate on HasInteractiveSession the same way SendToQueue already does. Separately, +// removing the selected row (in-UI delete or an externally-driven delta removal) must clear +// SelectedTask, otherwise the detail pane keeps showing a deleted task with live buttons. +public class TasksIslandDeleteGuardTests : IDisposable +{ + private readonly string _dbPath; + + public TasksIslandDeleteGuardTests() + { + _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_delete_guard_{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() + .UseSqlite($"Data Source={_dbPath}") + .Options; + return new ClaudeDoDbContext(opts); + } + + private sealed class RecordingWorkerClient : StubWorkerClient + { + public override bool IsConnected => true; + public string? DeletedTaskId { get; private set; } + public override Task<(bool Ok, string? Error)> DeleteTaskAsync(string taskId) + { + DeletedTaskId = taskId; + return Task.FromResult<(bool, string?)>((true, null)); + } + } + + [Fact] + public async Task DeleteTaskCommand_RowHasInteractiveSession_NoOpAndNoHubCall() + { + var worker = new RecordingWorkerClient(); + var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker); + var row = new TaskRowViewModel { Id = "T1", Status = TaskStatus.Idle, HasInteractiveSession = true }; + vm.Items.Add(row); + + await vm.DeleteTaskCommand.ExecuteAsync(row); + + Assert.Null(worker.DeletedTaskId); + Assert.Contains(row, vm.Items); + } + + [Fact] + public async Task DeleteTaskCommand_RemovesSelectedRow_ClearsSelection() + { + var worker = new RecordingWorkerClient(); + var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker); + var row = new TaskRowViewModel { Id = "T2", Status = TaskStatus.Idle }; + vm.Items.Add(row); + vm.SelectFrom(row, "test"); + + await vm.DeleteTaskCommand.ExecuteAsync(row); + + Assert.Equal("T2", worker.DeletedTaskId); + Assert.DoesNotContain(row, vm.Items); + Assert.Null(vm.SelectedTask); + } + + [Fact] + public async Task DeltaRemoval_OfSelectedRow_ClearsSelection() + { + await using (var db = NewContext()) + { + db.Lists.Add(new ListEntity { Id = "L1", Name = "Work", CreatedAt = DateTime.UtcNow }); + db.Tasks.Add(new TaskEntity + { + Number = TestTaskNumbers.Next(), + Id = "T3", ListId = "L1", Title = "Deleted elsewhere", + Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow, SortOrder = 0, + }); + await db.SaveChangesAsync(); + } + + var worker = new RecordingWorkerClient(); + var vm = new TasksIslandViewModel(new TestDbFactory(NewContext), worker); + vm.LoadForList(new ListNavItemViewModel { Id = "user:L1", Name = "Work", Kind = ListKind.User }); + if (vm.LoadTask is { } lt) await lt; + + Assert.True(await vm.SelectByIdAsync("T3")); + Assert.NotNull(vm.SelectedTask); + + await using (var db = NewContext()) + { + db.Tasks.Remove(await db.Tasks.FirstAsync(t => t.Id == "T3")); + await db.SaveChangesAsync(); + } + worker.RaiseTaskUpdated("T3"); + + var deadline = DateTime.UtcNow.AddSeconds(3); + while (DateTime.UtcNow < deadline && vm.Items.Any(r => r.Id == "T3")) + await Task.Delay(25); + + Assert.DoesNotContain(vm.Items, r => r.Id == "T3"); + Assert.Null(vm.SelectedTask); + } +}