fix(worker): close status-force bypasses on Running tasks

update_task_status Idle/Done, WorkerHub.SetTaskStatus, and the UI's Mark
Done/Cancelled menu items could all force a Running task's status without
going through TaskStateService's guards, leaving its CLI process untracked
and its worktree writes silently dropped. Idle now routes through the
already-guarded TaskStateService.ResetToIdleAsync instead of the repo's
unconditional ResetToManualAsync; Done and SetTaskStatus explicitly reject
Running; the context menu disables both entries with a tooltip while
running. TaskRunner also now logs a warning whenever a terminal transition
(SubmitForChildren/SubmitForReview/Complete/Fail) is rejected instead of
silently discarding the run result. DeleteTask's MCP path gets the same
friendly foreign-key message WorkerHub.DeleteTask already had for a task
with children.
This commit is contained in:
Mika Kuns
2026-08-26 15:01:13 +02:00
parent c4928b4def
commit 299c7b749c
7 changed files with 183 additions and 11 deletions
@@ -171,6 +171,13 @@ public sealed partial class TaskRowViewModel : ViewModelBase
IsRunning ? (true, null) : (false, Loc.T("tasks.reasonNotRunning"));
public string? CancelDisabledReason => CancelGate().Reason;
// Gates "Mark Done"/"Mark Cancelled" in the context menu — forcing either status while the
// task is actively running would strand the CLI process untracked (WorkerHub.SetTaskStatus
// rejects it server-side too; this is the UI-visible half of that guard).
private (bool Can, string? Reason) MarkAsGate() =>
IsRunning ? (false, Loc.T("tasks.reasonAlreadyRunning")) : (true, null);
public string? MarkAsDisabledReason => MarkAsGate().Reason;
// "Open quick session" has no precondition today (see A1's file header) — kept as a
// placeholder so the menu wiring is uniform; always null until a real gate exists.
public string? QuickSessionDisabledReason => null;
@@ -318,6 +325,7 @@ public sealed partial class TaskRowViewModel : ViewModelBase
OnPropertyChanged(nameof(CancelDisabledReason));
OnPropertyChanged(nameof(RefineDisabledReason));
OnPropertyChanged(nameof(PlanningDisabledReason));
OnPropertyChanged(nameof(MarkAsDisabledReason));
}
partial void OnParentTaskIdChanged(string? value)
@@ -67,8 +67,8 @@ public partial class TaskRowView : UserControl
// "Mark as" — exclusive-choice hiding inside is unchanged; the header is never empty.
var markAs = new MenuItem { Header = Loc.T("tasks.ctxMarkAs"), DataContext = row };
markAs.Items.Add(MakeItem("tasks.ctxMarkDone", OnSetStatusClick, tag: "Done"));
markAs.Items.Add(MakeItem("tasks.ctxMarkCancelled", OnSetStatusClick, tag: "Cancelled"));
markAs.Items.Add(MakeItem("tasks.ctxMarkDone", OnSetStatusClick, tag: "Done", reason: row.MarkAsDisabledReason));
markAs.Items.Add(MakeItem("tasks.ctxMarkCancelled", OnSetStatusClick, tag: "Cancelled", reason: row.MarkAsDisabledReason));
markAs.Items.Add(new Separator());
markAs.Items.Add(MakeItem("tasks.ctxMarkManual", OnToggleManualClick, !row.IsManual));
markAs.Items.Add(MakeItem("tasks.ctxMarkClaudeTask", OnToggleManualClick, row.IsManual));
+23 -4
View File
@@ -592,8 +592,9 @@ public sealed class ExternalMcpService
string taskId,
[Description("'Idle' (reset to editable), 'Queued' (enqueue for execution), 'Cancelled' (retire without " +
"deleting; can be reset to Idle later) or 'Done' (mark complete; refused if the task has an " +
"active worktree — use review_task to approve and merge that worktree instead). No other " +
"value is settable externally.")]
"active worktree — use review_task to approve and merge that worktree instead). 'Idle' and " +
"'Done' are both refused while the task is Running — cancel it first. No other value is " +
"settable externally.")]
string status,
CancellationToken cancellationToken)
{
@@ -609,8 +610,9 @@ public sealed class ExternalMcpService
switch (target)
{
case TaskStatus.Idle:
await _tasks.ResetToManualAsync(taskId, cancellationToken);
await _broadcaster.TaskUpdated(taskId);
var idleResult = await _state.ResetToIdleAsync(taskId, cancellationToken);
if (!idleResult.Ok)
throw new InvalidOperationException(idleResult.Reason ?? "Cannot reset task to Idle.");
break;
case TaskStatus.Queued:
@@ -627,6 +629,9 @@ public sealed class ExternalMcpService
break;
case TaskStatus.Done:
if (task.Status == TaskStatus.Running)
throw new InvalidOperationException("Cannot mark a running task Done — cancel it first.");
await using (var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken))
{
var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(taskId, cancellationToken);
@@ -870,7 +875,21 @@ public sealed class ExternalMcpService
if (task.Status == TaskStatus.Running)
throw new InvalidOperationException("Cannot delete a running task. Cancel it first.");
try
{
await _tasks.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 InvalidOperationException(
"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);
+7
View File
@@ -586,6 +586,13 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
{
if (!Enum.TryParse<TaskStatus>(status, ignoreCase: true, out var parsed))
throw new HubException($"unknown status: {status}");
await using var ctx = await _dbFactory.CreateDbContextAsync(Context.ConnectionAborted);
var current = await new TaskRepository(ctx).GetByIdAsync(taskId, Context.ConnectionAborted)
?? throw new HubException("task not found");
if (current.Status == TaskStatus.Running)
throw new HubException("Cannot change the status of a running task. Cancel it first.");
// Queueing goes through the gated transition so draft subtasks can't be queued;
// other statuses keep the unconditional "set status freely" affordance.
var result = parsed == TaskStatus.Queued
+12 -4
View File
@@ -550,7 +550,9 @@ public sealed class TaskRunner
if (isStandalone && pendingChildren.Count > 0)
{
await _state.SubmitForChildrenAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
var transition = await _state.SubmitForChildrenAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
if (!transition.Ok)
_logger.LogWarning("SubmitForChildrenAsync rejected for task {TaskId}: {Reason}", task.Id, transition.Reason);
foreach (var child in pendingChildren)
await _state.EnqueueAsync(child.Id, CancellationToken.None);
await _broadcaster.WorkerLog(
@@ -560,13 +562,17 @@ public sealed class TaskRunner
}
else if (isStandalone)
{
await _state.SubmitForReviewAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
var transition = await _state.SubmitForReviewAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
if (!transition.Ok)
_logger.LogWarning("SubmitForReviewAsync rejected for task {TaskId}: {Reason}", task.Id, transition.Reason);
await _broadcaster.WorkerLog($"Finished #{task.Number} \"{task.Title}\" (waiting for review)", WorkerLogLevel.Success, DateTime.UtcNow);
await _broadcaster.TaskFinished(slot, task.Id, "waiting_for_review", finishedAt);
}
else
{
await _state.CompleteAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
var transition = await _state.CompleteAsync(task.Id, finishedAt, reviewResult, CancellationToken.None);
if (!transition.Ok)
_logger.LogWarning("CompleteAsync rejected for task {TaskId}: {Reason}", task.Id, transition.Reason);
await _broadcaster.WorkerLog($"Finished #{task.Number} \"{task.Title}\" (done)", WorkerLogLevel.Success, DateTime.UtcNow);
await _broadcaster.TaskFinished(slot, task.Id, "done", finishedAt);
}
@@ -583,8 +589,10 @@ public sealed class TaskRunner
try
{
var finishedAt = DateTime.UtcNow;
await _state.FailAsync(taskId, finishedAt, error, CancellationToken.None,
var transition = await _state.FailAsync(taskId, finishedAt, error, CancellationToken.None,
failureReason, turnCount > 0 ? turnCount : null, maxTurns);
if (!transition.Ok)
_logger.LogWarning("FailAsync rejected for task {TaskId}: {Reason}", taskId, transition.Reason);
await _broadcaster.WorkerLog($"Finished #{taskNumber} \"{taskTitle}\" (failed)", WorkerLogLevel.Error, DateTime.UtcNow);
await _broadcaster.TaskFinished(slot, taskId, "failed", finishedAt);
_logger.LogWarning("Task {TaskId} failed (turns={Turns}): {Error}", taskId, turnCount, error);
@@ -900,6 +900,21 @@ public sealed class ExternalMcpServiceTests : IDisposable
sut.DeleteTask(task.Id, CancellationToken.None));
}
[Fact]
public async Task DeleteTask_WithChildren_ThrowsUnderstandableMessage()
{
var listId = await SeedListAsync();
var parent = await SeedTaskAsync(listId, status: TaskStatus.WaitingForChildren);
await SeedTaskAsync(listId, status: TaskStatus.Idle, parentId: parent.Id);
var sut = BuildSut(CreateQueue());
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
sut.DeleteTask(parent.Id, CancellationToken.None));
Assert.Contains("child task", ex.Message, StringComparison.OrdinalIgnoreCase);
Assert.NotNull(await _tasks.GetByIdAsync(parent.Id));
}
[Fact]
public async Task DeleteTask_NotFound_Throws()
{
@@ -981,6 +996,35 @@ public sealed class ExternalMcpServiceTests : IDisposable
Assert.Equal(TaskStatus.WaitingForReview, loaded!.Status);
}
[Fact]
public async Task UpdateTaskStatus_Idle_OnRunningTask_Throws()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId, status: TaskStatus.Running);
var sut = BuildSut(CreateQueue());
await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.UpdateTaskStatus(task.Id, "Idle", CancellationToken.None));
var loaded = await _tasks.GetByIdAsync(task.Id);
Assert.Equal(TaskStatus.Running, loaded!.Status);
}
[Fact]
public async Task UpdateTaskStatus_Done_OnRunningTask_Throws()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId, status: TaskStatus.Running);
var sut = BuildSut(CreateQueue());
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.UpdateTaskStatus(task.Id, "Done", CancellationToken.None));
Assert.Contains("running", ex.Message, StringComparison.OrdinalIgnoreCase);
var loaded = await _tasks.GetByIdAsync(task.Id);
Assert.Equal(TaskStatus.Running, loaded!.Status);
}
private ExternalMcpService NewService() => BuildSut(CreateQueue());
private async Task<string> SeedIdleTask(string title = "t")
@@ -0,0 +1,86 @@
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 SetTaskStatusHubTests : 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> SeedTaskAsync(TaskStatus status)
{
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 taskId = Guid.NewGuid().ToString();
await new TaskRepository(ctx).AddAsync(new TaskEntity
{
Id = taskId, ListId = listId, Title = "T",
Status = status, CreatedAt = DateTime.UtcNow, CommitType = "feat",
});
return taskId;
}
[Fact]
public async Task SetTaskStatus_RunningTask_Throws_AndLeavesStatusUnchanged()
{
var taskId = await SeedTaskAsync(TaskStatus.Running);
var hub = CreateHub();
await Assert.ThrowsAsync<HubException>(() => hub.SetTaskStatus(taskId, "Done"));
await using var ctx = _db.CreateContext();
var task = await ctx.Tasks.FindAsync(taskId);
Assert.Equal(TaskStatus.Running, task!.Status);
}
[Fact]
public async Task SetTaskStatus_IdleTask_StillSucceeds()
{
var taskId = await SeedTaskAsync(TaskStatus.Idle);
var hub = CreateHub();
await hub.SetTaskStatus(taskId, "Cancelled");
await using var ctx = _db.CreateContext();
var task = await ctx.Tasks.FindAsync(taskId);
Assert.Equal(TaskStatus.Cancelled, task!.Status);
}
[Fact]
public async Task SetTaskStatus_Missing_Throws()
{
var hub = CreateHub();
await Assert.ThrowsAsync<HubException>(() => hub.SetTaskStatus("does-not-exist", "Done"));
}
}