diff --git a/src/ClaudeDo.Data/TaskNumberAllocator.cs b/src/ClaudeDo.Data/TaskNumberAllocator.cs index b51edf7c..2e54f940 100644 --- a/src/ClaudeDo.Data/TaskNumberAllocator.cs +++ b/src/ClaudeDo.Data/TaskNumberAllocator.cs @@ -1,4 +1,5 @@ using ClaudeDo.Data.Models; +using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; namespace ClaudeDo.Data; @@ -29,6 +30,16 @@ public static class TaskNumberAllocator RETURNING * """, AppSettingsEntity.SingletonId).AsNoTracking().ToListAsync(ct); + if (settings.Count == 0) + { + // The singleton row is missing (e.g. a corrupted/hand-edited DB) -- recreate it, + // as its own separate insert/commit, and retry the UPDATE above rather than + // indexing into an empty result. Doesn't count against MaxAttempts: it's not a + // number collision. + await EnsureSingletonRowAsync(context, ct); + continue; + } + entity.Number = settings[0].NextTaskNumber - 1; context.Tasks.Add(entity); @@ -37,10 +48,40 @@ public static class TaskNumberAllocator await context.SaveChangesAsync(ct); return; } - catch (DbUpdateException) when (attempt < MaxAttempts) + catch (DbUpdateException ex) { context.Entry(entity).State = EntityState.Detached; + if (attempt < MaxAttempts && IsNumberCollision(ex)) + continue; + throw; } } } + + private static async Task EnsureSingletonRowAsync(ClaudeDoDbContext context, CancellationToken ct) + { + var row = new AppSettingsEntity { Id = AppSettingsEntity.SingletonId }; + context.AppSettings.Add(row); + try + { + await context.SaveChangesAsync(ct); + } + catch (DbUpdateException) + { + // A concurrent process already inserted the singleton -- fine, the next UPDATE + // attempt above will find it. + } + finally + { + context.Entry(row).State = EntityState.Detached; + } + } + + // Distinguishes the expected unique-index collision on idx_tasks_number (worth retrying with + // a fresh number) from any other DbUpdateException -- an FK violation, a NOT NULL violation, + // etc. -- which must surface immediately instead of silently burning up to MaxAttempts numbers + // on a failure a retry can never fix. + private static bool IsNumberCollision(DbUpdateException ex) => + ex.InnerException is SqliteException { SqliteErrorCode: 19 } sqliteEx + && sqliteEx.Message.Contains("tasks.number", StringComparison.Ordinal); } diff --git a/src/ClaudeDo.Worker/External/TaskWaitMcpTools.cs b/src/ClaudeDo.Worker/External/TaskWaitMcpTools.cs index 7079672a..4f2b4ce7 100644 --- a/src/ClaudeDo.Worker/External/TaskWaitMcpTools.cs +++ b/src/ClaudeDo.Worker/External/TaskWaitMcpTools.cs @@ -109,7 +109,19 @@ public sealed class TaskWaitMcpTools var tasks = new TaskRepository(ctx); var resolved = new string[ids.Length]; for (var i = 0; i < ids.Length; i++) - resolved[i] = await TaskIdResolver.ResolveAsync(tasks, ids[i], ct); + { + try + { + resolved[i] = await TaskIdResolver.ResolveAsync(tasks, ids[i], ct); + } + catch (InvalidOperationException) + { + // A #/bare-number id with no matching task -- keep the original, + // unresolvable id so CheckOnceAsync's lookup misses and reports "NotFound", + // per this tool's documented contract, instead of failing the whole call. + resolved[i] = ids[i]; + } + } return resolved; } diff --git a/tests/ClaudeDo.Data.Tests/TaskNumberAllocatorTests.cs b/tests/ClaudeDo.Data.Tests/TaskNumberAllocatorTests.cs index 93be3071..fc341443 100644 --- a/tests/ClaudeDo.Data.Tests/TaskNumberAllocatorTests.cs +++ b/tests/ClaudeDo.Data.Tests/TaskNumberAllocatorTests.cs @@ -131,6 +131,35 @@ public sealed class TaskNumberAllocatorTests : IDisposable Assert.All(numbers, n => Assert.True(n > 0)); } + [Fact] + public async Task AddAsync_recreates_a_missing_app_settings_row_instead_of_crashing() + { + await SeedListAsync(); + await _ctx.Database.ExecuteSqlRawAsync("DELETE FROM app_settings"); + + var task = NewTask("l1"); + await new TaskRepository(_ctx).AddAsync(task); + + Assert.True(task.Number > 0); + } + + [Fact] + public async Task AddAsync_noncollision_DbUpdateException_surfaces_immediately_without_burning_numbers() + { + await SeedListAsync(); + var badTask = NewTask("no-such-list"); + + await Assert.ThrowsAsync(() => new TaskRepository(_ctx).AddAsync(badTask)); + + // The failed attempt above should have burned exactly one number (attempt #1, no + // collision-driven retries) -- confirm the next successful insert isn't several numbers + // further along. + var goodTask = NewTask("l1"); + await new TaskRepository(_ctx).AddAsync(goodTask); + + Assert.Equal(2, goodTask.Number); + } + [Fact] public async Task GetByNumberAsync_finds_the_task_and_returns_null_for_unknown_numbers() { diff --git a/tests/ClaudeDo.Worker.Tests/External/TaskWaitMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/TaskWaitMcpToolsTests.cs index 80d400f2..846d7955 100644 --- a/tests/ClaudeDo.Worker.Tests/External/TaskWaitMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/TaskWaitMcpToolsTests.cs @@ -253,6 +253,52 @@ public sealed class TaskWaitMcpToolsTests : IDisposable Assert.True(sw.Elapsed >= TimeSpan.FromMilliseconds(900), $"took {sw.Elapsed}"); } + [Fact] + public async Task WaitForTaskChange_UnknownNumberHashForm_ReturnsImmediatelyAsNotFound() + { + var sut = BuildSut(); + var sw = Stopwatch.StartNew(); + + var result = await sut.WaitForTaskChange(["#999999"], timeoutSeconds: 30, cancellationToken: CancellationToken.None); + + sw.Stop(); + Assert.False(result.TimedOut); + Assert.Equal("NotFound", Assert.Single(result.Changed).Status); + Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}"); + } + + [Fact] + public async Task WaitForTaskChange_UnknownBareNumber_ReturnsImmediatelyAsNotFound() + { + var sut = BuildSut(); + var sw = Stopwatch.StartNew(); + + var result = await sut.WaitForTaskChange(["999999"], timeoutSeconds: 30, cancellationToken: CancellationToken.None); + + sw.Stop(); + Assert.False(result.TimedOut); + Assert.Equal("NotFound", Assert.Single(result.Changed).Status); + Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}"); + } + + [Fact] + public async Task WaitForTaskChange_MixOfValidAndUnknownNumber_ReportsBothCorrectly() + { + var task = await SeedTaskAsync(TaskStatus.WaitingForReview); + var sut = BuildSut(); + var sw = Stopwatch.StartNew(); + + var result = await sut.WaitForTaskChange( + [$"#{task.Number}", "#999999"], timeoutSeconds: 30, cancellationToken: CancellationToken.None); + + sw.Stop(); + Assert.False(result.TimedOut); + Assert.Equal(2, result.Changed.Count); + Assert.Contains(result.Changed, c => c.TaskId == task.Id && c.Status == "WaitingForReview"); + Assert.Contains(result.Changed, c => c.TaskId == "#999999" && c.Status == "NotFound"); + Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}"); + } + [Fact] public void MaxTimeoutSeconds_StaysComfortablyUnderMcpToolTimeout() {