fix(worker): Stuck-Running-Fenster in Continue- und Stop-Pfad schließen
TaskRunner.ContinueAsync: Claim, SeedAsync und SetupMcpConfigAsync liefen vor dem try/catch (anders als RunAsync) - warf einer davon nach dem Running-Claim, propagierte die Exception ungefangen bis zu OverrideSlotService.RunContinueInSlotAsync, das nur loggt. Task blieb Running. Fix: derselbe Aufbau wie RunAsync (Claim+Seed+MCP-Setup im try, MarkFailed im catch, mcpToken/mcpConfigPath vor dem try auf null). SetupMcpConfigAsync bekommt zusätzlich einen onTokenRegistered-Callback, damit die äußere mcpToken-Variable den Token sofort nach dem Register sieht - sonst hätte ein Fehler zwischen Register und Rückgabe (z.B. File.WriteAllTextAsync) den Token in der TaskRunTokenRegistry geleakt (betrifft denselben Aufruf in RunAsync mit, daher dort ebenfalls verdrahtet - RunAsync-Struktur selbst unverändert). QueueService.RunInSlotAsync: der Stop-Button (CancelTask) cancelt die Slot-CTS direkt ohne TaskStateService-Schreibzugriff. Traf das die Pre-Dispatch-DB-Reads, loggte der OCE-Catch nur und die vom Picker bereits auf Running geclaimte Task blieb dort für immer hängen. Fix: Status im Catch neu lesen und nur wenn er noch Running ist über TaskStateService.CancelAsync auf Cancelled setzen - ein Ursprung, der bereits selbst einen Terminalstatus geschrieben hat (z.B. CancelReview), wird nicht überschrieben. Kommentar korrigiert. TDD: neue Tests in ContinueAsyncExceptionTests (Seed-/MCP-Setup-Fehler nach Claim -> Failed, kein Token-Leak) und QueueServiceSlotFailureTests (Stop während Pre-Dispatch -> Cancelled statt Running; ein bereits terminal geschriebener Status wird nicht gestompt) vorher rot, jetzt grün. Worker.Tests: 1213/1213 grün, Worker baut in Release.
This commit is contained in:
@@ -9,6 +9,7 @@ using ClaudeDo.Worker.Runner;
|
||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||
using ClaudeDo.Worker.Usage;
|
||||
using Microsoft.Data.Sqlite;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
@@ -180,7 +181,7 @@ public sealed class QueueServiceSlotFailureTests : IDisposable
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_cancelled_slot_run_does_not_mark_the_task_Failed()
|
||||
public async Task A_cancelled_slot_run_is_marked_Cancelled_not_left_Running()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
@@ -203,21 +204,105 @@ public sealed class QueueServiceSlotFailureTests : IDisposable
|
||||
await service.StartAsync(outerCts.Token);
|
||||
waker.Wake();
|
||||
|
||||
// Wait for the slot to be claimed and then released again (RunInSlotAsync's
|
||||
// ContinueWith removes it once the catch block — ours or a stray one — finishes).
|
||||
var deadline = DateTime.UtcNow.AddSeconds(10);
|
||||
while (service.GetActive().Any(a => a.taskId == taskId) && DateTime.UtcNow < deadline)
|
||||
await Task.Delay(25);
|
||||
await Task.Delay(100); // let the fire-and-forget continuation fully settle
|
||||
var reloaded = await PollUntilLeftQueuedAsync(taskId);
|
||||
|
||||
TaskEntity? reloaded;
|
||||
using (var verify = _db.CreateContext())
|
||||
reloaded = await new TaskRepository(verify).GetByIdAsync(taskId);
|
||||
|
||||
// The picker's atomic claim already flipped it to Running; the cancelled slot run must
|
||||
// leave it there rather than flipping it to Failed.
|
||||
Assert.Equal(TaskStatus.Running, reloaded!.Status);
|
||||
Assert.DoesNotContain(hub.Proxy.Calls,
|
||||
// This reproduces a plain Stop (QueueService.CancelTask cancels the slot's CTS
|
||||
// directly, without going through TaskStateService) landing during the pre-dispatch DB
|
||||
// reads — before TaskRunner's own claim/try-catch ever starts. Nothing else writes a
|
||||
// terminal status for that window, so the OCE catch itself must close it.
|
||||
Assert.Equal(TaskStatus.Cancelled, reloaded!.Status);
|
||||
Assert.Contains(hub.Proxy.Calls,
|
||||
c => c.Method == "TaskUpdated" && (string)c.Args[0]! == taskId);
|
||||
}
|
||||
|
||||
// A_cancelled_slot_run_is_marked_Cancelled_not_left_Running above reproduces a plain Stop.
|
||||
// TaskStateService.CancelAsync (the hub's CancelReview action) is a different origin: it
|
||||
// writes Cancelled *before* it cancels the run's CTS via RunCancellationRegistry, so by the
|
||||
// time RunInSlotAsync's OCE catch runs, the task has already left Running. The catch must
|
||||
// recognize that and not blindly stomp whatever terminal status is already there.
|
||||
[Fact]
|
||||
public async Task A_cancelled_slot_run_does_not_stomp_a_status_already_written_terminal()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
ctx.Tasks.Add(new TaskEntity
|
||||
{
|
||||
Id = taskId, ListId = listId, Title = "T", Status = TaskStatus.Queued,
|
||||
CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var outerCts = new CancellationTokenSource();
|
||||
var realPicker = new QueuePicker(_db.CreateFactory());
|
||||
var picker = new ClaimMarkFailedThenCancelPicker(realPicker, outerCts, _db);
|
||||
var (service, hub, waker) = CreateService(picker);
|
||||
|
||||
await service.StartAsync(outerCts.Token);
|
||||
waker.Wake();
|
||||
|
||||
var reloaded = await PollUntilLeftQueuedAsync(taskId);
|
||||
|
||||
// Some other terminal write (simulating CancelAsync/FailAsync having already run) must
|
||||
// survive — the OCE catch must not overwrite it with Cancelled just because it observed
|
||||
// a cancellation.
|
||||
Assert.Equal(TaskStatus.Failed, reloaded!.Status);
|
||||
}
|
||||
|
||||
// Polls until the task leaves its initial Queued status (the claim + dispatch + OCE-catch
|
||||
// chain settling), or gives up at the deadline; then waits a further grace period so the
|
||||
// fire-and-forget continuation (queue-slot cleanup, any terminal-status write) fully lands
|
||||
// before the caller reads the final state.
|
||||
private async Task<TaskEntity?> PollUntilLeftQueuedAsync(string taskId)
|
||||
{
|
||||
TaskEntity? reloaded = null;
|
||||
var deadline = DateTime.UtcNow.AddSeconds(10);
|
||||
while (DateTime.UtcNow < deadline)
|
||||
{
|
||||
using var verify = _db.CreateContext();
|
||||
reloaded = await new TaskRepository(verify).GetByIdAsync(taskId);
|
||||
if (reloaded!.Status != TaskStatus.Queued) break;
|
||||
await Task.Delay(25);
|
||||
}
|
||||
await Task.Delay(150); // let the fire-and-forget continuation fully settle
|
||||
|
||||
using var final = _db.CreateContext();
|
||||
return await new TaskRepository(final).GetByIdAsync(taskId);
|
||||
}
|
||||
|
||||
// Simulates a terminal status already having been written (by TaskStateService, from some
|
||||
// other origin) between the picker's claim and RunInSlotAsync's OCE catch running.
|
||||
private sealed class ClaimMarkFailedThenCancelPicker : IQueuePicker
|
||||
{
|
||||
private readonly IQueuePicker _inner;
|
||||
private readonly CancellationTokenSource _cancelAfterClaim;
|
||||
private readonly DbFixture _db;
|
||||
|
||||
public ClaimMarkFailedThenCancelPicker(IQueuePicker inner, CancellationTokenSource cancelAfterClaim, DbFixture db)
|
||||
{
|
||||
_inner = inner;
|
||||
_cancelAfterClaim = cancelAfterClaim;
|
||||
_db = db;
|
||||
}
|
||||
|
||||
public async Task<TaskEntity?> ClaimNextAsync(DateTime now, CancellationToken ct)
|
||||
{
|
||||
var claimed = await _inner.ClaimNextAsync(now, ct);
|
||||
if (claimed is not null)
|
||||
{
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
await ctx.Tasks.Where(t => t.Id == claimed.Id)
|
||||
.ExecuteUpdateAsync(s => s
|
||||
.SetProperty(t => t.Status, TaskStatus.Failed)
|
||||
.SetProperty(t => t.FinishedAt, DateTime.UtcNow));
|
||||
}
|
||||
_cancelAfterClaim.Cancel();
|
||||
}
|
||||
return claimed;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user