fix(ui): Reactivität — Description-Save-Race, UsageMonitor-Leak, Listen-Live-Refresh
Drei unabhängige Reactivity-Bugs aus dem Polish-Audit 2026-08-20: 1. Description-Autosave überschrieb den falschen Task, weil SaveDescriptionAsync Task.Id/EditableDescription erst nach dem 400ms-Debounce las statt Row+Wert an der Aufrufstelle zu capturen (wie SaveTitleAsync es schon tat). Bind() cancelt jetzt zusätzlich einen laufenden Title-/Description-Save der vorherigen Row. 2. UsageMonitorModalViewModel abonnierte UsageUpdatedEvent erst nach dem Erst-Load-Await — schloss man das Modal währenddessen, lief das Unsubscribe in Close() ins Leere und die VM hing für immer am WorkerClient. Ein _isClosed-Flag wird jetzt nach dem Await geprüft, bevor abonniert wird. 3. Per MCP erstellte Listen blieben unsichtbar: RefreshRowAsync hatte keinen Add-Zweig für unbekannte Ids und verglich zudem die falsche Id-Form (der Worker broadcastet die rohe DB-Id, nie die "user:"-prefixte Row-Id). Ein Reconnect lud zudem nur Counts statt der vollen Listen neu.
This commit is contained in:
mika kuns
@@ -0,0 +1,140 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Ui.Services;
|
||||
using ClaudeDo.Ui.ViewModels.Islands;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Ui.Tests.ViewModels;
|
||||
|
||||
// Polish-Audit 2026-08-20 #1: typing in task A, then binding to task B inside the 400 ms debounce
|
||||
// window used to write A's text into B's row, because SaveDescriptionAsync/SaveTitleAsync read
|
||||
// `Task`/`EditableDescription` after the delay instead of capturing them at the call site.
|
||||
public class DetailsIslandDescriptionSaveRaceTests : IDisposable
|
||||
{
|
||||
private readonly string _dbPath;
|
||||
|
||||
public DetailsIslandDescriptionSaveRaceTests()
|
||||
{
|
||||
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_desc_race_test_{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);
|
||||
}
|
||||
|
||||
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
|
||||
{
|
||||
private readonly Func<ClaudeDoDbContext> _create;
|
||||
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
|
||||
public ClaudeDoDbContext CreateDbContext() => _create();
|
||||
}
|
||||
|
||||
private sealed class NullServiceProvider : IServiceProvider
|
||||
{
|
||||
public object? GetService(Type serviceType) => null;
|
||||
}
|
||||
|
||||
private sealed class StubNotesApi : ClaudeDo.Ui.Services.Interfaces.INotesApi
|
||||
{
|
||||
public Task<List<DailyNoteDto>> ListAsync(DateOnly day) =>
|
||||
Task.FromResult(new List<DailyNoteDto>());
|
||||
public Task<DailyNoteDto?> AddAsync(DateOnly day, string text) =>
|
||||
Task.FromResult<DailyNoteDto?>(null);
|
||||
public Task UpdateAsync(string id, string text) => Task.CompletedTask;
|
||||
public Task DeleteAsync(string id) => Task.CompletedTask;
|
||||
}
|
||||
|
||||
private sealed class FakeWorker : StubWorkerClient
|
||||
{
|
||||
public override bool IsConnected => true;
|
||||
}
|
||||
|
||||
private async Task SeedTaskAsync(string id, string title, string? description)
|
||||
{
|
||||
await using var db = NewContext();
|
||||
if (!await db.Lists.AnyAsync(l => l.Id == "L1"))
|
||||
db.Lists.Add(new ListEntity { Id = "L1", Name = "Work", CreatedAt = DateTime.UtcNow });
|
||||
db.Tasks.Add(new TaskEntity
|
||||
{
|
||||
Number = TestTaskNumbers.Next(),
|
||||
Id = id, ListId = "L1", Title = title, Description = description,
|
||||
Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
private DetailsIslandViewModel BuildVm() =>
|
||||
new(new TestDbFactory(NewContext), new FakeWorker(), new NullServiceProvider(), new StubNotesApi(),
|
||||
new ClaudeDo.Ui.Services.MergeCoordinator());
|
||||
|
||||
private async Task<TaskEntity> ReadTaskAsync(string id)
|
||||
{
|
||||
await using var db = NewContext();
|
||||
return await db.Tasks.FirstAsync(t => t.Id == id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task TypingInA_ThenTaskSwappedUnderneath_StillWritesOntoA_NeverB()
|
||||
{
|
||||
// Reproduces the exact split the audit called out: `Bind()` assigns `Task` synchronously,
|
||||
// while `EditableDescription` is only reset once `BindAsync`'s DB round trip completes —
|
||||
// so for a window, `Task` already points at B while `EditableDescription` still holds A's
|
||||
// freshly typed text. Setting `Task` directly (bypassing `Bind()`'s own DB round trip and
|
||||
// its now-added save cancellation) isolates that exact window deterministically, instead of
|
||||
// racing the real 400 ms debounce against however long a real bind happens to take.
|
||||
await SeedTaskAsync("A", "Task A", "original A");
|
||||
await SeedTaskAsync("B", "Task B", "original B");
|
||||
|
||||
var vm = BuildVm();
|
||||
vm.Bind(new TaskRowViewModel { Id = "A", Title = "Task A" });
|
||||
await Task.Delay(50);
|
||||
|
||||
vm.EditableDescription = "typed into A";
|
||||
// Simulate the mid-debounce moment where Task already points at B but EditableDescription
|
||||
// has not been reset yet — without going through Bind() (which would cancel the pending
|
||||
// save and dispose this exact race).
|
||||
vm.Task = new TaskRowViewModel { Id = "B", Title = "Task B" };
|
||||
|
||||
await Task.Delay(600); // past the debounce window
|
||||
|
||||
var a = await ReadTaskAsync("A");
|
||||
var b = await ReadTaskAsync("B");
|
||||
Assert.Equal("typed into A", a.Description);
|
||||
Assert.Equal("original B", b.Description);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Bind_CancelsPendingTitleAndDescriptionSaves_ForThePreviousTask()
|
||||
{
|
||||
await SeedTaskAsync("A", "Task A", "original A");
|
||||
await SeedTaskAsync("B", "Task B", "original B");
|
||||
|
||||
var vm = BuildVm();
|
||||
vm.Bind(new TaskRowViewModel { Id = "A", Title = "Task A" });
|
||||
await Task.Delay(50);
|
||||
|
||||
vm.EditableTitle = "typed title A";
|
||||
vm.EditableDescription = "typed desc A";
|
||||
vm.Bind(new TaskRowViewModel { Id = "B", Title = "Task B" });
|
||||
|
||||
await Task.Delay(600); // past the debounce window
|
||||
|
||||
var a = await ReadTaskAsync("A");
|
||||
Assert.Equal("Task A", a.Title);
|
||||
Assert.Equal("original A", a.Description);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,125 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Localization;
|
||||
using ClaudeDo.Ui.Localization;
|
||||
using ClaudeDo.Ui.Services;
|
||||
using ClaudeDo.Ui.ViewModels.Islands;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
|
||||
namespace ClaudeDo.Ui.Tests.ViewModels;
|
||||
|
||||
// Polish-Audit 2026-08-20 #3: a list created by a running MCP session broadcasts ListUpdated with
|
||||
// the raw list id, which RefreshRowAsync had no add-branch for — the list stayed invisible until
|
||||
// the next app restart. And a reconnect only refreshed counts, missing offline list changes.
|
||||
public class ListsIslandListUpdatedTests : IDisposable
|
||||
{
|
||||
private readonly string _dbPath;
|
||||
|
||||
public ListsIslandListUpdatedTests()
|
||||
{
|
||||
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_lists_updated_test_{Guid.NewGuid():N}.db");
|
||||
using var ctx = NewContext();
|
||||
ctx.Database.EnsureCreated();
|
||||
|
||||
var dir = AppContext.BaseDirectory;
|
||||
while (dir is not null && !Directory.Exists(Path.Combine(dir, "src", "ClaudeDo.Localization", "locales")))
|
||||
dir = Path.GetDirectoryName(dir);
|
||||
Loc.Current = new Localizer(
|
||||
LocaleStore.Load(Path.Combine(dir!, "src", "ClaudeDo.Localization", "locales")), "en");
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
|
||||
{
|
||||
private readonly Func<ClaudeDoDbContext> _create;
|
||||
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
|
||||
public ClaudeDoDbContext CreateDbContext() => _create();
|
||||
}
|
||||
|
||||
private sealed class FakeWorker : StubWorkerClient
|
||||
{
|
||||
public override bool IsConnected => true;
|
||||
}
|
||||
|
||||
private async Task SeedListAsync(string id, string name, int sortOrder)
|
||||
{
|
||||
await using var db = NewContext();
|
||||
db.Lists.Add(new ListEntity { Id = id, Name = name, SortOrder = sortOrder, CreatedAt = DateTime.UtcNow });
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListUpdated_ForAnIdNotYetLoaded_AddsItToUserLists()
|
||||
{
|
||||
await SeedListAsync("existing", "Existing", sortOrder: 0);
|
||||
|
||||
var worker = new FakeWorker();
|
||||
var vm = new ListsIslandViewModel(new TestDbFactory(NewContext), worker: worker);
|
||||
await vm.LoadAsync();
|
||||
Assert.Single(vm.UserLists);
|
||||
|
||||
// A running MCP session creates the list after our initial load — the worker broadcasts
|
||||
// the raw db id, never the "user:" prefixed row id.
|
||||
await SeedListAsync("new-from-mcp", "Created via MCP", sortOrder: 1);
|
||||
worker.RaiseListUpdated("new-from-mcp");
|
||||
await Task.Delay(100);
|
||||
|
||||
Assert.Equal(2, vm.UserLists.Count);
|
||||
Assert.Contains(vm.UserLists, r => r.Id == "user:new-from-mcp" && r.Name == "Created via MCP");
|
||||
// Inserted at its DB sort position (after "existing"), not just appended — same relative
|
||||
// order as if it had been present since LoadAsync.
|
||||
Assert.Equal("user:existing", vm.UserLists[0].Id);
|
||||
Assert.Equal("user:new-from-mcp", vm.UserLists[1].Id);
|
||||
Assert.Contains(vm.Items, r => r.Id == "user:new-from-mcp");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListUpdated_ForAnIdMissingFromTheDbToo_DoesNothingAndDoesNotThrow()
|
||||
{
|
||||
var worker = new FakeWorker();
|
||||
var vm = new ListsIslandViewModel(new TestDbFactory(NewContext), worker: worker);
|
||||
await vm.LoadAsync();
|
||||
var countBefore = vm.UserLists.Count;
|
||||
|
||||
worker.RaiseListUpdated("ghost-id");
|
||||
await Task.Delay(100);
|
||||
|
||||
Assert.Equal(countBefore, vm.UserLists.Count);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ConnectionRestored_FullyReloadsLists_AndKeepsSelectionIfStillPresent()
|
||||
{
|
||||
await SeedListAsync("keep-me", "Keep Me", sortOrder: 0);
|
||||
|
||||
var worker = new FakeWorker();
|
||||
var vm = new ListsIslandViewModel(new TestDbFactory(NewContext), worker: worker);
|
||||
await vm.LoadAsync();
|
||||
var toSelect = vm.UserLists.Single(r => r.Id == "user:keep-me");
|
||||
vm.SelectedList = toSelect;
|
||||
|
||||
// A list created while offline — the reconnect must pick it up, unlike a bare count refresh.
|
||||
await SeedListAsync("came-in-offline", "Came In Offline", sortOrder: 1);
|
||||
|
||||
worker.RaiseConnectionRestored();
|
||||
await Task.Delay(150);
|
||||
|
||||
Assert.Contains(vm.UserLists, r => r.Id == "user:came-in-offline");
|
||||
Assert.NotNull(vm.SelectedList);
|
||||
Assert.Equal("user:keep-me", vm.SelectedList!.Id);
|
||||
}
|
||||
}
|
||||
@@ -155,6 +155,24 @@ public class UsageMonitorModalViewModelTests
|
||||
Assert.False(vm.IsBusy);
|
||||
}
|
||||
|
||||
// ── Leak on early close (Polish-Audit 2026-08-20 #2) ────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task LoadAsync_ClosedBeforeSnapshotArrives_NeverSubscribesToUsageUpdated()
|
||||
{
|
||||
var worker = new FakeWorker { SnapshotGate = new TaskCompletionSource<UsageSnapshotDto?>() };
|
||||
var vm = new UsageMonitorModalViewModel(worker);
|
||||
|
||||
var load = vm.LoadAsync();
|
||||
vm.CloseCommand.Execute(null);
|
||||
worker.SnapshotGate.SetResult(Snapshot(new[] { Limit("session") }));
|
||||
await load;
|
||||
|
||||
worker.RaiseUsageUpdated(Snapshot(new[] { Limit("session"), Limit("weekly_all") }));
|
||||
|
||||
Assert.Null(vm.Snapshot);
|
||||
}
|
||||
|
||||
// ── Manual refresh ──────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user