From d84607f79681864077e93ce78b250258369dbf67 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 13:29:44 +0200 Subject: [PATCH] fix(ui): gate queueing on an open interactive ConPTY session A task-based ConPTY session leaves the row Idle in the DB (sessions never touch status), so nothing stopped the queue picker from claiming it too: CanSendToQueue ignored HasInteractiveSession, and both TasksIslandViewModel. SendToQueueAsync and MissionControlViewModel.EnqueueTaskAsync (drag-to-queue) wrote Status=Queued straight via EF, bypassing TaskStateService entirely and its manual/draft-child guards. That let an autonomous claude process spawn in the same worktree a user was hand-editing in the ConPTY pane. Add !HasInteractiveSession to CanSendToQueue, and route both UI enqueue paths through IWorkerClient.SetTaskStatusAsync (worker hub -> TaskStateService. EnqueueAsync) instead of raw EF writes. The interactive-session gate itself stays in the UI: the worker has no notion of a UI-hosted ConPTY pane. --- src/ClaudeDo.Localization/locales/de.json | 2 +- src/ClaudeDo.Localization/locales/en.json | 2 +- .../ViewModels/Islands/TaskRowViewModel.cs | 9 +++- .../Islands/TasksIslandViewModel.cs | 20 +++----- .../ViewModels/MissionControlViewModel.cs | 20 +++----- .../MissionControlViewModelTests.cs | 46 ++++++++++++++++++- .../UiVm/TaskRowViewModelPlanningTests.cs | 15 ++++++ .../UiVm/TasksIslandViewModelPlanningTests.cs | 30 +++++++++++- 8 files changed, 111 insertions(+), 33 deletions(-) diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 3c73a3da..2be0add9 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -645,7 +645,7 @@ "taskStatus": { "idle": "Leerlauf", "queued": "In Warteschlange", "running": "Läuft", "waitingForReview": "Wartet auf Prüfung", "waitingForChildren": "Wartet auf Teilaufgaben", "done": "Fertig", "failed": "Fehlgeschlagen", "cancelled": "Abgebrochen", "parked": "Geparkt", "interactive": "Interaktiv" }, "planningBadge": { "active": "PLANUNG", "finalized": "GEPLANT" }, "taskRow": { "createdPrefix": "Erstellt {0}", "stepsText": "{0}/{1} Schritte" }, - "tasksIsland": { "completedHeader": "ABGESCHLOSSEN", "completedHeaderCount": "ABGESCHLOSSEN · {0}", "planningOpenFailed": "Planungssitzung konnte nicht geöffnet werden: {0}", "planningResumeFailed": "Planungssitzung konnte nicht fortgesetzt werden: {0}", "approveFailed": "Genehmigen & Mergen fehlgeschlagen: {0}", "moveRunningRejected": "Ein laufender Task kann nicht in eine andere Liste verschoben werden.", "moveWorktreeRejected": "Verschieben nicht möglich — dieser Task hat einen aktiven Worktree, der auf sein aktuelles Repo zeigt.", "moveRepoConfirm": "Unterschiedliche Repos — {0} → {1}. Task trotzdem verschieben?", "moveConfirmUnavailable": "Verschieben nicht möglich — der Bestätigungsdialog ist nicht verfügbar.", "quickClaudeNoWorkingDir": "Für diese Liste ist kein Arbeitsverzeichnis konfiguriert.", "quickClaudeDirMissing": "Arbeitsverzeichnis existiert nicht mehr: {0}" }, + "tasksIsland": { "completedHeader": "ABGESCHLOSSEN", "completedHeaderCount": "ABGESCHLOSSEN · {0}", "planningOpenFailed": "Planungssitzung konnte nicht geöffnet werden: {0}", "planningResumeFailed": "Planungssitzung konnte nicht fortgesetzt werden: {0}", "approveFailed": "Genehmigen & Mergen fehlgeschlagen: {0}", "sendToQueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", "moveRunningRejected": "Ein laufender Task kann nicht in eine andere Liste verschoben werden.", "moveWorktreeRejected": "Verschieben nicht möglich — dieser Task hat einen aktiven Worktree, der auf sein aktuelles Repo zeigt.", "moveRepoConfirm": "Unterschiedliche Repos — {0} → {1}. Task trotzdem verschieben?", "moveConfirmUnavailable": "Verschieben nicht möglich — der Bestätigungsdialog ist nicht verfügbar.", "quickClaudeNoWorkingDir": "Für diese Liste ist kein Arbeitsverzeichnis konfiguriert.", "quickClaudeDirMissing": "Arbeitsverzeichnis existiert nicht mehr: {0}" }, "diff": { "loadFailed": "Diff konnte nicht geladen werden: {0}", "noChanges": "Keine Änderungen anzuzeigen.", "unavailable": "Diff nicht mehr verfügbar — Commit-Bereich unvollständig." }, "planningDiff": { "hubError": "Kombinierte Vorschau konnte nicht erstellt werden (Hub-Fehler).", "conflict": "Kombinierte Vorschau nicht möglich: Teilaufgabe {0} steht im Konflikt mit einer früheren Teilaufgabe ({1} Dateien).", "buildFailed": "Kombinierte Vorschau konnte nicht erstellt werden: {0}" }, "merge": { "commitMessage": "Merge-Aufgabe: {0}", "workerOfflineBranches": "Worker offline — Branches können nicht aufgelistet werden.", "loadBranchesFailed": "Branches konnten nicht geladen werden: {0}", "merged": "Zusammengeführt.", "conflict": "Merge-Konflikt — Ziel-Branch wiederhergestellt. Manuell oder über Fortsetzen lösen, dann erneut versuchen.", "blocked": "Blockiert: {0}", "verifyFailed": "Merge ist gelandet, aber das Verify-Kommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf Erledigt gesetzt.", "unknownStatus": "Unbekannter Status: {0}", "mergeFailed": "Merge fehlgeschlagen: {0}" }, diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index e2e0a6d2..d5a8e983 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -645,7 +645,7 @@ "taskStatus": { "idle": "Idle", "queued": "Queued", "running": "Running", "waitingForReview": "Waiting for Review", "waitingForChildren": "Waiting for Subtasks", "done": "Done", "failed": "Failed", "cancelled": "Cancelled", "parked": "Parked", "interactive": "Interactive" }, "planningBadge": { "active": "PLANNING", "finalized": "PLANNED" }, "taskRow": { "createdPrefix": "Created {0}", "stepsText": "{0}/{1} steps" }, - "tasksIsland": { "completedHeader": "COMPLETED", "completedHeaderCount": "COMPLETED · {0}", "planningOpenFailed": "Couldn't open planning session: {0}", "planningResumeFailed": "Couldn't resume planning session: {0}", "approveFailed": "Approve & merge failed: {0}", "moveRunningRejected": "Can't move a running task to another list.", "moveWorktreeRejected": "Can't move — this task has an active worktree pointing at its current repo.", "moveRepoConfirm": "Different repos — {0} → {1}. Move the task anyway?", "moveConfirmUnavailable": "Can't move — the confirmation dialog isn't available.", "quickClaudeNoWorkingDir": "This list has no working directory configured.", "quickClaudeDirMissing": "Working directory no longer exists: {0}" }, + "tasksIsland": { "completedHeader": "COMPLETED", "completedHeaderCount": "COMPLETED · {0}", "planningOpenFailed": "Couldn't open planning session: {0}", "planningResumeFailed": "Couldn't resume planning session: {0}", "approveFailed": "Approve & merge failed: {0}", "sendToQueueFailed": "Send to queue failed: {0}", "moveRunningRejected": "Can't move a running task to another list.", "moveWorktreeRejected": "Can't move — this task has an active worktree pointing at its current repo.", "moveRepoConfirm": "Different repos — {0} → {1}. Move the task anyway?", "moveConfirmUnavailable": "Can't move — the confirmation dialog isn't available.", "quickClaudeNoWorkingDir": "This list has no working directory configured.", "quickClaudeDirMissing": "Working directory no longer exists: {0}" }, "diff": { "loadFailed": "Failed to load diff: {0}", "noChanges": "No changes to show.", "unavailable": "Diff no longer available — commit range incomplete." }, "planningDiff": { "hubError": "Could not build combined preview (hub error).", "conflict": "Cannot build combined preview: subtask {0} conflicts with an earlier subtask ({1} files).", "buildFailed": "Could not build combined preview: {0}" }, "merge": { "commitMessage": "Merge task: {0}", "workerOfflineBranches": "Worker offline — cannot list branches.", "loadBranchesFailed": "Failed to load branches: {0}", "merged": "Merged.", "conflict": "Merge conflict — target branch restored. Resolve manually or via Continue, then retry.", "blocked": "Blocked: {0}", "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "unknownStatus": "Unknown status: {0}", "mergeFailed": "Merge failed: {0}" }, diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs index 671059b6..3e3f0e66 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs @@ -99,11 +99,15 @@ public sealed partial class TaskRowViewModel : ViewModelBase public bool CanRemoveFromQueue => IsQueued || HasQueuedSubtasks; // "Send to queue" is the single queue entry. On a finalized planning parent it queues the // plan (children) via CanQueuePlan; an Active (not-yet-finalized) planning parent is hidden — - // it must be finalized first. + // it must be finalized first. The worker never sees a UI-hosted ConPTY session (it never + // touches task status), so this gate has to live here: queueing a task the user is actively + // hand-editing in an interactive pane would spawn an autonomous run racing it in the same + // worktree. public bool CanSendToQueue => !IsRunning && !IsQueued && !IsWaitingForReview && !HasQueuedSubtasks && (!IsChild || ParentFinalized) && PlanningPhase != PlanningPhase.Active - && !IsManual; + && !IsManual + && !HasInteractiveSession; // Parent-level "send plan to queue" — only once the plan is finalized (children Planned). // Drives the routing inside SendToQueue, not a separate menu entry. public bool CanQueuePlan => !IsChild && HasPlanningChildren @@ -242,6 +246,7 @@ public sealed partial class TaskRowViewModel : ViewModelBase OnPropertyChanged(nameof(StatusLabel)); OnPropertyChanged(nameof(ShowStatusChip)); OnPropertyChanged(nameof(InteractiveChipTooltip)); + OnPropertyChanged(nameof(CanSendToQueue)); } partial void OnHasQueuedSubtasksChanged(bool value) diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs index f8915ce8..42035039 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs @@ -833,26 +833,18 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable [RelayCommand] private async Task SendToQueueAsync(TaskRowViewModel? row) { - if (row is null || row.IsRunning) return; + if (row is null || row.IsRunning || row.HasInteractiveSession || _worker is null) return; // A finalized planning parent queues its plan (children sequentially), not itself. if (row.CanQueuePlan) { await QueuePlanningSubtasksAsync(row); return; } - await using var db = await _dbFactory.CreateDbContextAsync(); - var entity = await db.Tasks.FirstOrDefaultAsync(t => t.Id == row.Id); - if (entity is null) return; - entity.Status = TaskStatus.Queued; - await db.SaveChangesAsync(); - row.Status = TaskStatus.Queued; - if (_worker is not null) - { - try { await _worker.WakeQueueAsync(); } catch { } - } - Regroup(); - UpdateSubtitle(); - TasksChanged?.Invoke(this, EventArgs.Empty); + // Goes through the worker hub (TaskStateService.EnqueueAsync) rather than a raw EF write + // so the manual/draft-child guards apply here too; the row refreshes from the resulting + // TaskUpdated broadcast. + try { await _worker.SetTaskStatusAsync(row.Id, TaskStatus.Queued); } + catch (Exception ex) { ErrorReported?.Invoke(Loc.T("vm.tasksIsland.sendToQueueFailed", ex.Message)); } } [RelayCommand] diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs index c25d2e7a..187b0d7b 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs @@ -115,22 +115,16 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable catch { /* best-effort queue refresh */ } } - // Drop-to-queue: a task dragged from the main app onto Mission Control gets queued. + // Drop-to-queue: a task dragged from the main app onto Mission Control gets queued. Goes + // through the worker hub (TaskStateService.EnqueueAsync) rather than a raw EF write so the + // manual/draft-child guards apply here too. The interactive-session check has to live here + // rather than on the worker side: the worker never touches task status for a UI-hosted ConPTY + // session, so it has no way to know one is open — only Mission Control's own pane list does. public async System.Threading.Tasks.Task EnqueueTaskAsync(string taskId) { if (string.IsNullOrEmpty(taskId)) return; - try - { - await using var db = await _dbFactory.CreateDbContextAsync(); - var entity = await db.Tasks.FirstOrDefaultAsync(t => t.Id == taskId); - if (entity is null - || entity.Status == ClaudeDo.Data.Models.TaskStatus.Running - || entity.Status == ClaudeDo.Data.Models.TaskStatus.Queued) - return; - entity.Status = ClaudeDo.Data.Models.TaskStatus.Queued; - await db.SaveChangesAsync(); - await _worker.WakeQueueAsync(); - } + if (ConPtySessions.Any(s => s.TaskId == taskId)) return; + try { await _worker.SetTaskStatusAsync(taskId, ClaudeDo.Data.Models.TaskStatus.Queued); } catch { /* best-effort enqueue */ } await RefreshQueueAsync(); } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs index 148a5155..c38442f2 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs @@ -190,6 +190,27 @@ public class MissionControlViewModelTests : IDisposable await db.SaveChangesAsync(); } + // Mirrors TaskStateService.EnqueueAsync writing to the same DB so the test can verify the + // resulting task state without the real worker process — EnqueueTaskAsync now delegates the + // actual status write to the worker hub instead of doing a raw EF write itself. + private sealed class QueueingWorkerClient : StubWorkerClient + { + private readonly Func _newContext; + public List QueuedTaskIds { get; } = new(); + + public QueueingWorkerClient(Func newContext) => _newContext = newContext; + + public override async Task SetTaskStatusAsync(string taskId, TaskStatus status) + { + QueuedTaskIds.Add(taskId); + await using var db = _newContext(); + var entity = await db.Tasks.FirstOrDefaultAsync(t => t.Id == taskId); + if (entity is null) return; + entity.Status = status; + await db.SaveChangesAsync(); + } + } + [Fact] public async Task EnqueueTaskAsync_SetsTaskQueued_AndShowsInStrip() { @@ -200,11 +221,12 @@ public class MissionControlViewModelTests : IDisposable await db.SaveChangesAsync(); } - var worker = new FakeWorker(); + var worker = new QueueingWorkerClient(NewContext); using var vm = BuildVm(worker); await vm.EnqueueTaskAsync("idleTask"); + Assert.Equal(new[] { "idleTask" }, worker.QueuedTaskIds); Assert.True(vm.HasQueued); Assert.Contains(vm.Queued, q => q.Id == "idleTask"); @@ -213,6 +235,28 @@ public class MissionControlViewModelTests : IDisposable Assert.Equal(TaskStatus.Queued, entity.Status); } + [Fact] + public async Task EnqueueTaskAsync_TaskHasOpenConPtySession_DoesNotQueue() + { + await using (var db = NewContext()) + { + db.Lists.Add(new ListEntity { Id = "L1", Name = "Work", CreatedAt = DateTime.UtcNow }); + db.Tasks.Add(new TaskEntity { Id = "t1", ListId = "L1", Title = "Do the thing", Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow, SortOrder = 0 }); + await db.SaveChangesAsync(); + } + + var worker = new QueueingWorkerClient(NewContext); + using var vm = BuildVm(worker); + await vm.OpenConPtySessionAsync("t1"); + + await vm.EnqueueTaskAsync("t1"); + + Assert.Empty(worker.QueuedTaskIds); + await using var verify = NewContext(); + var entity = await verify.Tasks.FirstAsync(t => t.Id == "t1"); + Assert.Equal(TaskStatus.Idle, entity.Status); + } + private sealed class ThrowingLaunchSpecWorker : StubWorkerClient { public override Task GetInteractiveLaunchSpecAsync(string taskId, CancellationToken ct = default) diff --git a/tests/ClaudeDo.Worker.Tests/UiVm/TaskRowViewModelPlanningTests.cs b/tests/ClaudeDo.Worker.Tests/UiVm/TaskRowViewModelPlanningTests.cs index 52a65211..6238209a 100644 --- a/tests/ClaudeDo.Worker.Tests/UiVm/TaskRowViewModelPlanningTests.cs +++ b/tests/ClaudeDo.Worker.Tests/UiVm/TaskRowViewModelPlanningTests.cs @@ -87,6 +87,21 @@ public class TaskRowViewModelPlanningTests Assert.True(vm.CanSendToQueue); } + [Fact] + public void OpenInteractiveSession_CannotSendToQueue() + { + // A hand-driven ConPTY session is still Idle in the DB (sessions never touch status), so + // this has to be gated on the UI-only HasInteractiveSession flag, not on Status. + var vm = MakeRow(TaskStatus.Idle); + Assert.True(vm.CanSendToQueue); + + vm.HasInteractiveSession = true; + Assert.False(vm.CanSendToQueue); + + vm.HasInteractiveSession = false; + Assert.True(vm.CanSendToQueue); + } + [Fact] public void FinalizedParentWithChildren_CanQueuePlan() { diff --git a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs index 735ea2d4..dbcf5e92 100644 --- a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs +++ b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs @@ -20,6 +20,7 @@ sealed class FakeWorkerClient : IWorkerClient public int DiscardPlanningCalls { get; private set; } public int FinalizePlanningCalls { get; private set; } public int WakeQueueCalls { get; private set; } + public List<(string TaskId, TaskStatus Status)> SetTaskStatusCalls { get; } = new(); public bool IsConnected => false; public bool IsReconnecting => false; @@ -58,7 +59,11 @@ sealed class FakeWorkerClient : IWorkerClient public Task> InstallSessionSkillAsync(string url) => Task.FromResult(new List()); public Task UpdateSessionSkillAsync(string sourceUrl) => Task.CompletedTask; public Task RemoveSessionSkillAsync(string sourceUrl) => Task.CompletedTask; - public Task SetTaskStatusAsync(string taskId, TaskStatus status) => Task.CompletedTask; + public Task SetTaskStatusAsync(string taskId, TaskStatus status) + { + SetTaskStatusCalls.Add((taskId, status)); + return Task.CompletedTask; + } public Task ApproveReviewAsync(string taskId, string targetBranch) => Task.FromResult(null); public Task PreviewMergeAsync(string taskId, string targetBranch) => Task.FromResult(null); public Task MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty(), null)); @@ -299,6 +304,29 @@ public class TasksIslandViewModelPlanningTests Assert.True(child.ParentInView); Assert.True(child.ShowAsChild); } + + [Fact] + public async Task SendToQueueAsync_RoutesThroughWorkerHub_NotRawEf() + { + var row = MakeRow("t1", TaskStatus.Idle); + var (vm, worker) = VmFactory.Create([row]); + + await ((IAsyncRelayCommand)vm.SendToQueueCommand).ExecuteAsync(row); + + Assert.Equal(("t1", TaskStatus.Queued), Assert.Single(worker.SetTaskStatusCalls)); + } + + [Fact] + public async Task SendToQueueAsync_TaskHasOpenInteractiveSession_DoesNotQueue() + { + var row = MakeRow("t1", TaskStatus.Idle); + row.HasInteractiveSession = true; + var (vm, worker) = VmFactory.Create([row]); + + await ((IAsyncRelayCommand)vm.SendToQueueCommand).ExecuteAsync(row); + + Assert.Empty(worker.SetTaskStatusCalls); + } } // ── My Day add / remove (real DB) ─────────────────────────────────────────────