fix(ui): retry the task delta refresh instead of swallowing the error

2026-08-07 09:16:21 +02:00
parent f62dbb9239
commit cba7d01c31
2 changed files with 194 additions and 45 deletions
@@ -161,6 +161,11 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
    }

    private async void OnWorkerTaskUpdated(string taskId)
+        => await RefreshTaskFromWorkerAsync(taskId);
+
+    // Awaitable so tests can drive it deterministically. One retry, then a full reload:
+    // a swallowed exception here used to leave the row on a stale status permanently.
+    internal async Task RefreshTaskFromWorkerAsync(string taskId)
    {
        var list = _currentList;
        if (list is null) return;
@@ -176,52 +181,71 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable

        try
        {
-            await using var db = await _dbFactory.CreateDbContextAsync();
-            var entity = await db.Tasks
-                .Include(t => t.List)
-                .Include(t => t.Worktree)
-                .FirstOrDefaultAsync(t => t.Id == taskId);
-
-            // A parent transition (finalize/discard) broadcasts only the parent's id, but it
-            // changes its children's derived state — finalize flips them Draft→Planned, discard
-            // deletes them. The delta path below only touches the parent row and never recomputes
-            // the child-derived flags (ParentFinalized, HasPlanningChildren) nor drops deleted
-            // children, so reconcile the whole list when the updated task is (or owns) a subtree.
-            if (entity is not null &&
-                (entity.PlanningPhase != PlanningPhase.None || Items.Any(r => r.ParentTaskId == entity.Id)))
-            {
-                LoadForList(list);
-                return;
-            }
-
-            var existing = Items.FirstOrDefault(r => r.Id == taskId);
-
-            if (entity is null)
-            {
-                if (existing is not null) Items.Remove(existing);
-            }
-            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 (matches)                     { LoadForList(list); return; }
-                else                                  return;
-            }
-
-            // Keep the parent's HasQueuedSubtasks flag in sync when a child's status flips.
-            if (entity is not null && !string.IsNullOrEmpty(entity.ParentTaskId))
-            {
-                var parent = Items.FirstOrDefault(r => r.Id == entity.ParentTaskId);
-                if (parent is not null)
-                    parent.HasQueuedSubtasks = Items.Any(r =>
-                        r.ParentTaskId == parent.Id && (r.IsQueued || r.IsWaiting));
-            }
-
-            Regroup();
-            UpdateSubtitle();
+            await ApplyDeltaAsync(taskId, list);
        }
-        catch { }
+        catch (Exception first)
+        {
+            System.Diagnostics.Debug.WriteLine(
+                $"TasksIsland: delta refresh for {taskId} failed ({first.Message}); retrying");
+            try
+            {
+                await ApplyDeltaAsync(taskId, list);
+            }
+            catch (Exception second)
+            {
+                System.Diagnostics.Debug.WriteLine(
+                    $"TasksIsland: delta retry for {taskId} failed ({second.Message}); full reload");
+                LoadForList(list);
+            }
+        }
+    }
+
+    private async Task ApplyDeltaAsync(string taskId, ListNavItemViewModel list)
+    {
+        await using var db = await _dbFactory.CreateDbContextAsync();
+        var entity = await db.Tasks
+            .Include(t => t.List)
+            .Include(t => t.Worktree)
+            .FirstOrDefaultAsync(t => t.Id == taskId);
+
+        // A parent transition (finalize/discard) broadcasts only the parent's id, but it
+        // changes its children's derived state — finalize flips them Draft→Planned, discard
+        // deletes them. The delta path below only touches the parent row and never recomputes
+        // the child-derived flags (ParentFinalized, HasPlanningChildren) nor drops deleted
+        // children, so reconcile the whole list when the updated task is (or owns) a subtree.
+        if (entity is not null &&
+            (entity.PlanningPhase != PlanningPhase.None || Items.Any(r => r.ParentTaskId == entity.Id)))
+        {
+            LoadForList(list);
+            return;
+        }
+
+        var existing = Items.FirstOrDefault(r => r.Id == taskId);
+
+        if (entity is null)
+        {
+            if (existing is not null) Items.Remove(existing);
+        }
+        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 (matches)                     { LoadForList(list); return; }
+            else                                  return;
+        }
+
+        // Keep the parent's HasQueuedSubtasks flag in sync when a child's status flips.
+        if (entity is not null && !string.IsNullOrEmpty(entity.ParentTaskId))
+        {
+            var parent = Items.FirstOrDefault(r => r.Id == entity.ParentTaskId);
+            if (parent is not null)
+                parent.HasQueuedSubtasks = Items.Any(r =>
+                    r.ParentTaskId == parent.Id && (r.IsQueued || r.IsWaiting));
+        }
+
+        Regroup();
+        UpdateSubtitle();
    }

    // NOTE: virtual:queued/virtual:running cannot be decided by a single entity — a Planning
@@ -0,0 +1,125 @@
+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;
+
+// The delta path in OnWorkerTaskUpdated used to be wrapped in a blank `catch { }`. A single
+// transient DB error therefore left the row on its old status forever — the "task stuck on
+// Queued although it is running" bug. It must retry, and fall back to a full reload.
+public class TasksIslandDeltaResilienceTests : IDisposable
+{
+    private readonly string _dbPath;
+
+    public TasksIslandDeltaResilienceTests()
+    {
+        _dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_ui_delta_{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<ClaudeDoDbContext>()
+            .UseSqlite($"Data Source={_dbPath}")
+            .Options;
+        return new ClaudeDoDbContext(opts);
+    }
+
+    // Throws on the first N CreateDbContext calls, then behaves normally.
+    private sealed class FlakyDbFactory : IDbContextFactory<ClaudeDoDbContext>
+    {
+        private readonly Func<ClaudeDoDbContext> _create;
+        private int _failuresLeft;
+        public int CreateCalls { get; private set; }
+
+        public FlakyDbFactory(Func<ClaudeDoDbContext> create, int failuresLeft)
+        {
+            _create = create;
+            _failuresLeft = failuresLeft;
+        }
+
+        public ClaudeDoDbContext CreateDbContext()
+        {
+            CreateCalls++;
+            if (_failuresLeft > 0)
+            {
+                _failuresLeft--;
+                throw new InvalidOperationException("simulated transient DB failure");
+            }
+            return _create();
+        }
+
+        public void FailNext() => _failuresLeft++;
+    }
+
+    private sealed class FakeWorker : StubWorkerClient
+    {
+    }
+
+    // A user list's nav id is prefixed — see TasksIslandRegroupTests.UserList.
+    private static ListNavItemViewModel UserList(string listEntityId, string name) =>
+        new() { Id = $"user:{listEntityId}", Kind = ListKind.User, Name = name };
+
+    // LoadForList is void and fires a background task; this is the wait idiom the other
+    // TasksIsland test files use.
+    private static async Task LoadAndWaitAsync(TasksIslandViewModel vm, ListNavItemViewModel list)
+    {
+        vm.LoadForList(list);
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (DateTime.UtcNow < deadline)
+        {
+            await Task.Delay(25);
+            if (vm.Items.Count > 0) break;
+        }
+        await Task.Delay(50);
+    }
+
+    private async Task SeedAsync()
+    {
+        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 = "Task one",
+            Status = TaskStatus.Queued, CreatedAt = DateTime.UtcNow, SortOrder = 0,
+        });
+        await db.SaveChangesAsync();
+    }
+
+    [Fact]
+    public async Task Delta_refresh_retries_after_a_transient_failure_and_still_applies_the_new_status()
+    {
+        await SeedAsync();
+
+        var flaky = new FlakyDbFactory(NewContext, failuresLeft: 0);
+        var vm = new TasksIslandViewModel(flaky, new FakeWorker());
+        var list = UserList("L1", "Work");
+
+        await LoadAndWaitAsync(vm, list);
+        Assert.Equal(TaskStatus.Queued, vm.Items.Single(r => r.Id == "T1").Status);
+
+        // Worker flips the task to Running.
+        await using (var db = NewContext())
+        {
+            var t = await db.Tasks.FirstAsync(x => x.Id == "T1");
+            t.Status = TaskStatus.Running;
+            await db.SaveChangesAsync();
+        }
+
+        // The next delta read fails once; the retry must still land the new status.
+        flaky.FailNext();
+        await vm.RefreshTaskFromWorkerAsync("T1");
+
+        Assert.Equal(TaskStatus.Running, vm.Items.Single(r => r.Id == "T1").Status);
+    }
+}