fix(ui): retry the task delta refresh instead of swallowing the error
This commit is contained in:
@@ -161,6 +161,11 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async void OnWorkerTaskUpdated(string taskId)
|
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;
|
var list = _currentList;
|
||||||
if (list is null) return;
|
if (list is null) return;
|
||||||
@@ -176,52 +181,71 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
await using var db = await _dbFactory.CreateDbContextAsync();
|
await ApplyDeltaAsync(taskId, list);
|
||||||
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();
|
|
||||||
}
|
}
|
||||||
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
|
// 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);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user