docs: add UI-rewrite notes, plans, and stream-formatter spec
This commit is contained in:
Mika Kuns
705
docs/superpowers/plans/2026-04-17-logic-bug-fixes.md
Normal file
705
docs/superpowers/plans/2026-04-17-logic-bug-fixes.md
Normal file
@@ -0,0 +1,705 @@
|
||||
# Logic Bug Fixes Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Fix confirmed logic bugs across Worker, App/Ui, and Installer found in the 2026-04-17 three-agent review.
|
||||
|
||||
**Architecture:** Each bug is an isolated change to one or two files. Group by priority (Critical → High → Medium → Info). TDD where the bug is observable via xUnit integration test (Worker, Data); for UI/Installer bugs without test harness, do a focused manual repro and guard with a regression comment referencing the commit.
|
||||
|
||||
**Tech Stack:** .NET 8, Avalonia 12, CommunityToolkit.Mvvm, EF Core + SQLite, SignalR, xUnit (Worker tests only).
|
||||
|
||||
---
|
||||
|
||||
## File Map
|
||||
|
||||
**Worker:**
|
||||
- Modify: `src/ClaudeDo.Worker/Hub/WorkerHub.cs` — remove premature `RunCreated` broadcast
|
||||
- Modify: `src/ClaudeDo.Worker/Runner/TaskRunner.cs` — emit `RunCreated` after run row insert
|
||||
- Modify: `src/ClaudeDo.Worker/Services/QueueService.cs` — add slot-collision guard on `RunNow`/`ContinueTask`
|
||||
- Modify: `src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs` — extend quoting to cover whitespace/newline
|
||||
- Test: `tests/ClaudeDo.Worker.Tests/ClaudeArgsBuilderTests.cs` — regression for newline in system prompt
|
||||
- Test: `tests/ClaudeDo.Worker.Tests/QueueServiceSlotGuardTests.cs` — regression for RunNow-while-queued
|
||||
|
||||
**Ui:**
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskListViewModel.cs` — guard nullability in `AddTask`, harden `OnTaskUpdated`
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskDetailViewModel.cs` — defer `_taskId` assignment until after cancel check
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskEditorViewModel.cs` — init TCS before dialog shown
|
||||
|
||||
**Installer:**
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs` — remove inline start; reject CurrentUser without password
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs` — rename-before-extract rollback
|
||||
- Modify: `src/ClaudeDo.Installer/Core/UninstallRunner.cs` — add `removeAppData` parameter
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/WriteConfigStep.cs` — expand `~` in `UiDbPath`
|
||||
- Verify: `src/ClaudeDo.Installer/App.xaml.cs` — confirm Avalonia vs WPF usings
|
||||
|
||||
---
|
||||
|
||||
## Critical
|
||||
|
||||
### Task 1: Worker — fix `RunCreated` broadcast ordering (W1)
|
||||
|
||||
Bug: `WorkerHub.RunNow` fires `RunCreated` before the run row is inserted by `RunOnceAsync`. UI can receive an event for a row that does not yet exist.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Worker/Hub/WorkerHub.cs:35-50`
|
||||
- Modify: `src/ClaudeDo.Worker/Runner/TaskRunner.cs:234-256` (`RunOnceAsync`)
|
||||
|
||||
- [ ] **Step 1: Remove premature broadcast from WorkerHub**
|
||||
|
||||
In `src/ClaudeDo.Worker/Hub/WorkerHub.cs`, replace the body of `RunNow`:
|
||||
|
||||
```csharp
|
||||
public async Task RunNow(string taskId)
|
||||
{
|
||||
try
|
||||
{
|
||||
await _queue.RunNow(taskId);
|
||||
}
|
||||
catch (InvalidOperationException)
|
||||
{
|
||||
throw new HubException("override slot busy");
|
||||
}
|
||||
catch (KeyNotFoundException)
|
||||
{
|
||||
throw new HubException("task not found");
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Emit `RunCreated` inside `RunOnceAsync` after row insert**
|
||||
|
||||
In `src/ClaudeDo.Worker/Runner/TaskRunner.cs`, find `RunOnceAsync`. After the `runRepo.AddAsync(run, ct);` block (~line 256), add:
|
||||
|
||||
```csharp
|
||||
await _broadcaster.RunCreated(taskId, runNumber, isRetry);
|
||||
```
|
||||
|
||||
Then remove the existing `await _broadcaster.RunCreated(task.Id, 2, true);` on line 128 (inside the auto-retry block in `RunAsync`) and the `await _broadcaster.RunCreated(taskId, nextRunNumber, false);` on line 219 (in `ContinueAsync`), since `RunOnceAsync` now broadcasts unconditionally.
|
||||
|
||||
- [ ] **Step 3: Build and run Worker tests**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx && dotnet test tests/ClaudeDo.Worker.Tests`
|
||||
Expected: all existing tests pass.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Worker/Hub/WorkerHub.cs src/ClaudeDo.Worker/Runner/TaskRunner.cs
|
||||
git commit -m "fix(worker): emit RunCreated after run row exists"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Ui — harden `OnTaskUpdated` against async void crash (U2)
|
||||
|
||||
Bug: `TaskListViewModel.OnTaskUpdated` is `async void` with no try/catch. A DB error escapes to `TaskScheduler.UnobservedTaskException` and can crash the process.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskListViewModel.cs:328-332`
|
||||
|
||||
- [ ] **Step 1: Wrap handler body in try/catch**
|
||||
|
||||
Replace the existing method with:
|
||||
|
||||
```csharp
|
||||
private async void OnTaskUpdated(string taskId)
|
||||
{
|
||||
if (CurrentListId is null) return;
|
||||
try
|
||||
{
|
||||
await RefreshSingleAsync(taskId);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
System.Diagnostics.Debug.WriteLine($"[TaskListViewModel] OnTaskUpdated failed for {taskId}: {ex}");
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Ui/ViewModels/TaskListViewModel.cs
|
||||
git commit -m "fix(ui): swallow DB errors in TaskListViewModel.OnTaskUpdated"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Installer — reject CurrentUser service registration without password (I1)
|
||||
|
||||
Bug: `RegisterServiceStep` passes `obj=.\<user>` to `sc.exe create` with no `password=`. SCM rejects it with exit 5 / 1069 and the user gets an opaque error.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs`
|
||||
|
||||
- [ ] **Step 1: Read the current file**
|
||||
|
||||
Read `src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs` to confirm the exact shape of the `obj=` branch and the outer `StepResult` return.
|
||||
|
||||
- [ ] **Step 2: Replace CurrentUser branch with early failure**
|
||||
|
||||
Where the step builds `obj=".\\<username>"` for the `CurrentUser` account option, replace it with:
|
||||
|
||||
```csharp
|
||||
if (ctx.ServiceAccount == ServiceAccountType.CurrentUser)
|
||||
{
|
||||
return StepResult.Fail(
|
||||
"Service cannot run as Current User without a password. " +
|
||||
"Select 'Local System' or extend ServicePage to capture a password.");
|
||||
}
|
||||
```
|
||||
|
||||
Keep the `LocalSystem` branch (which passes `obj= LocalSystem` with no password requirement) unchanged.
|
||||
|
||||
- [ ] **Step 3: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs
|
||||
git commit -m "fix(installer): reject CurrentUser service account without password"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## High
|
||||
|
||||
### Task 4: Worker — guard slot collision on `RunNow` and `ContinueTask` (W2)
|
||||
|
||||
Bug: Queue slot and override slot have no guard against operating on the same `taskId`. `TaskRunner.MarkRunningAsync` can overwrite `started_at`.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Worker/Services/QueueService.cs:59-115`
|
||||
- Test: `tests/ClaudeDo.Worker.Tests/QueueServiceSlotGuardTests.cs` (new)
|
||||
|
||||
- [ ] **Step 1: Write failing test**
|
||||
|
||||
Create `tests/ClaudeDo.Worker.Tests/QueueServiceSlotGuardTests.cs`:
|
||||
|
||||
```csharp
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Worker.Services;
|
||||
using Xunit;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests;
|
||||
|
||||
public class QueueServiceSlotGuardTests : WorkerTestBase
|
||||
{
|
||||
[Fact]
|
||||
public async Task RunNow_rejects_task_already_active_in_queue_slot()
|
||||
{
|
||||
var queue = ServiceProvider.GetRequiredService<QueueService>();
|
||||
var task = await SeedAgentTaskAsync(listId: await SeedListAsync(), title: "blocker");
|
||||
|
||||
// Prime queue slot by wake signal.
|
||||
queue.WakeQueue();
|
||||
await WaitForActiveSlotAsync("queue", task.Id);
|
||||
|
||||
// RunNow on the same id must throw InvalidOperationException.
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() => queue.RunNow(task.Id));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(Helpers `WorkerTestBase`, `SeedAgentTaskAsync`, `SeedListAsync`, `WaitForActiveSlotAsync` exist in the test project — follow the pattern from existing tests.)
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `dotnet test tests/ClaudeDo.Worker.Tests --filter "FullyQualifiedName~QueueServiceSlotGuardTests"`
|
||||
Expected: FAIL — currently `RunNow` succeeds and creates a duplicate slot.
|
||||
|
||||
- [ ] **Step 3: Add guard in `RunNow`**
|
||||
|
||||
In `src/ClaudeDo.Worker/Services/QueueService.cs`, inside the `lock (_lock)` block in `RunNow` (~line 69), add before the existing override check:
|
||||
|
||||
```csharp
|
||||
if (_queueSlot?.TaskId == taskId)
|
||||
throw new InvalidOperationException("task is already running in queue slot");
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Add same guard in `ContinueTask`**
|
||||
|
||||
In the `lock (_lock)` block in `ContinueTask` (~line 97), add:
|
||||
|
||||
```csharp
|
||||
if (_queueSlot?.TaskId == taskId)
|
||||
throw new InvalidOperationException("task is already running in queue slot");
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run tests**
|
||||
|
||||
Run: `dotnet test tests/ClaudeDo.Worker.Tests`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Worker/Services/QueueService.cs tests/ClaudeDo.Worker.Tests/QueueServiceSlotGuardTests.cs
|
||||
git commit -m "fix(worker): guard against same task in queue and override slot"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Worker — quote CLI args with tab/newline/carriage-return (W3)
|
||||
|
||||
Bug: `ClaudeArgsBuilder.Escape` only quotes on space/quote. System prompts with newlines pass through unquoted and corrupt the argument list.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs:56-64`
|
||||
- Test: `tests/ClaudeDo.Worker.Tests/ClaudeArgsBuilderTests.cs` (new or existing)
|
||||
|
||||
- [ ] **Step 1: Write failing test**
|
||||
|
||||
If `ClaudeArgsBuilderTests.cs` does not exist, create it:
|
||||
|
||||
```csharp
|
||||
using ClaudeDo.Worker.Runner;
|
||||
using Xunit;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests;
|
||||
|
||||
public class ClaudeArgsBuilderTests
|
||||
{
|
||||
[Fact]
|
||||
public void Build_quotes_system_prompt_with_newline()
|
||||
{
|
||||
var builder = new ClaudeArgsBuilder();
|
||||
var args = builder.Build(new ClaudeRunConfig(
|
||||
Model: null,
|
||||
SystemPrompt: "line1\nline2",
|
||||
AgentPath: null,
|
||||
ResumeSessionId: null));
|
||||
|
||||
Assert.Contains("--append-system-prompt \"line1\\nline2\"", args);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Build_quotes_system_prompt_with_tab()
|
||||
{
|
||||
var builder = new ClaudeArgsBuilder();
|
||||
var args = builder.Build(new ClaudeRunConfig(
|
||||
Model: null,
|
||||
SystemPrompt: "col1\tcol2",
|
||||
AgentPath: null,
|
||||
ResumeSessionId: null));
|
||||
|
||||
Assert.Contains("\"col1", args);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `dotnet test tests/ClaudeDo.Worker.Tests --filter "FullyQualifiedName~ClaudeArgsBuilderTests"`
|
||||
Expected: FAIL — newline is passed through unquoted.
|
||||
|
||||
- [ ] **Step 3: Extend `Escape` condition and escape newline/tab**
|
||||
|
||||
In `src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs`, replace `Escape`:
|
||||
|
||||
```csharp
|
||||
private static string Escape(string value)
|
||||
{
|
||||
if (value.Contains(' ') || value.Contains('"') || value.Contains('\'')
|
||||
|| value.Contains('\t') || value.Contains('\n') || value.Contains('\r'))
|
||||
{
|
||||
var escaped = value
|
||||
.Replace("\\", "\\\\")
|
||||
.Replace("\"", "\\\"")
|
||||
.Replace("\n", "\\n")
|
||||
.Replace("\r", "\\r")
|
||||
.Replace("\t", "\\t");
|
||||
return $"\"{escaped}\"";
|
||||
}
|
||||
return value;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests**
|
||||
|
||||
Run: `dotnet test tests/ClaudeDo.Worker.Tests --filter "FullyQualifiedName~ClaudeArgsBuilderTests"`
|
||||
Expected: PASS.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs tests/ClaudeDo.Worker.Tests/ClaudeArgsBuilderTests.cs
|
||||
git commit -m "fix(worker): escape newline/tab in CLI args"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Installer — remove inline service start from `RegisterServiceStep` (I2)
|
||||
|
||||
Bug: `RegisterServiceStep` calls `sc.exe start` inline. `StartServiceStep` exists separately. If the update path ever wires both, the service is started twice.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs`
|
||||
- Modify: `src/ClaudeDo.Installer/App.xaml.cs` (pipeline) — ensure `StartServiceStep` is in the fresh-install pipeline
|
||||
|
||||
- [ ] **Step 1: Read current pipeline wiring in App.xaml.cs**
|
||||
|
||||
Read `src/ClaudeDo.Installer/App.xaml.cs` around line 112 to confirm the list of steps passed into `InstallerService`.
|
||||
|
||||
- [ ] **Step 2: Remove inline `sc.exe start` from RegisterServiceStep**
|
||||
|
||||
Delete the block (~lines 72-77) that runs `sc.exe start <service>` when `ctx.AutoStart == true`.
|
||||
|
||||
- [ ] **Step 3: Add `StartServiceStep` to the fresh-install pipeline if missing**
|
||||
|
||||
In `App.xaml.cs`, append `new StartServiceStep(...)` after `RegisterServiceStep` in the step list. Gate its execution internally on `ctx.AutoStart` (it already handles exit code 1056).
|
||||
|
||||
- [ ] **Step 4: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/Steps/RegisterServiceStep.cs src/ClaudeDo.Installer/App.xaml.cs
|
||||
git commit -m "fix(installer): move service start out of RegisterServiceStep"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Installer — rollback-safe extract in `DownloadAndExtractStep` (I3)
|
||||
|
||||
Bug: Old `app/` and `worker/` are deleted before extraction. If extraction throws, user is left with no binaries and no recovery path.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs:70-95`
|
||||
|
||||
- [ ] **Step 1: Read the current delete/extract sequence**
|
||||
|
||||
Read `src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs` around lines 70-95 to identify the exact `Directory.Delete` and `ZipFile.ExtractToDirectory` calls and which `ctx` paths they reference.
|
||||
|
||||
- [ ] **Step 2: Replace delete-before-extract with rename-then-commit**
|
||||
|
||||
Wrap the delete+extract block:
|
||||
|
||||
```csharp
|
||||
var appDir = Path.Combine(ctx.InstallRoot, "app");
|
||||
var workDir = Path.Combine(ctx.InstallRoot, "worker");
|
||||
var appBak = appDir + ".bak";
|
||||
var workBak = workDir + ".bak";
|
||||
|
||||
// Stash existing dirs.
|
||||
if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true);
|
||||
if (Directory.Exists(workBak)) Directory.Delete(workBak, recursive: true);
|
||||
if (Directory.Exists(appDir)) Directory.Move(appDir, appBak);
|
||||
if (Directory.Exists(workDir)) Directory.Move(workDir, workBak);
|
||||
|
||||
try
|
||||
{
|
||||
ZipFile.ExtractToDirectory(zipPath, ctx.InstallRoot, overwriteFiles: true);
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Roll back to previous binaries.
|
||||
if (Directory.Exists(appDir)) Directory.Delete(appDir, recursive: true);
|
||||
if (Directory.Exists(workDir)) Directory.Delete(workDir, recursive: true);
|
||||
if (Directory.Exists(appBak)) Directory.Move(appBak, appDir);
|
||||
if (Directory.Exists(workBak)) Directory.Move(workBak, workDir);
|
||||
throw;
|
||||
}
|
||||
|
||||
// Success — drop stash.
|
||||
if (Directory.Exists(appBak)) Directory.Delete(appBak, recursive: true);
|
||||
if (Directory.Exists(workBak)) Directory.Delete(workBak, recursive: true);
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/Steps/DownloadAndExtractStep.cs
|
||||
git commit -m "fix(installer): rollback-safe extract with .bak stash"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 8: Installer — gate `~/.todo-app` deletion behind explicit consent (I4)
|
||||
|
||||
Bug: Uninstaller always deletes user data (db, logs, configs). Reinstalling a different version silently destroys all tasks.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Installer/Core/UninstallRunner.cs:60-80`
|
||||
- Modify: `src/ClaudeDo.Installer/Views/UninstallPage.xaml(.cs)` (or equivalent) — add a checkbox
|
||||
|
||||
- [ ] **Step 1: Read `UninstallRunner.RunAsync` signature**
|
||||
|
||||
Read `src/ClaudeDo.Installer/Core/UninstallRunner.cs` around lines 1-90 to get current signature.
|
||||
|
||||
- [ ] **Step 2: Add `removeAppData` parameter to `RunAsync`**
|
||||
|
||||
Change signature to:
|
||||
|
||||
```csharp
|
||||
public async Task<UninstallResult> RunAsync(bool removeAppData, CancellationToken ct = default)
|
||||
```
|
||||
|
||||
Guard the deletion:
|
||||
|
||||
```csharp
|
||||
if (removeAppData)
|
||||
{
|
||||
var appData = Paths.Expand("~/.todo-app");
|
||||
if (Directory.Exists(appData))
|
||||
Directory.Delete(appData, recursive: true);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Wire a "Remove user data" checkbox on the uninstall page**
|
||||
|
||||
In the uninstall view/VM, add `[ObservableProperty] private bool _removeAppData;` (default `false`) and pass it into `RunAsync(RemoveAppData)`.
|
||||
|
||||
- [ ] **Step 4: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/Core/UninstallRunner.cs src/ClaudeDo.Installer/Views/UninstallPage.xaml src/ClaudeDo.Installer/Views/UninstallPage.xaml.cs
|
||||
git commit -m "fix(installer): make user-data deletion on uninstall opt-in"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Medium
|
||||
|
||||
### Task 9: Ui — guard `AddTask` against null `CurrentListId` after await (U1)
|
||||
|
||||
Bug: `AddTask` awaits `editor.LoadAgentsAsync`. Between `CanAddTask` and `listRepo.GetByIdAsync(CurrentListId)` on line 164, a concurrent `LoadAsync(null)` could null the id. Compiler warns CS8604.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskListViewModel.cs:157-170`
|
||||
|
||||
- [ ] **Step 1: Capture `CurrentListId` before the first `await`**
|
||||
|
||||
Replace the start of `AddTask`:
|
||||
|
||||
```csharp
|
||||
[RelayCommand(CanExecute = nameof(CanAddTask))]
|
||||
private async Task AddTask()
|
||||
{
|
||||
var listId = CurrentListId;
|
||||
if (listId is null) return;
|
||||
|
||||
string defaultCommitType;
|
||||
using (var context = _dbFactory.CreateDbContext())
|
||||
{
|
||||
var listRepo = new ListRepository(context);
|
||||
var list = await listRepo.GetByIdAsync(listId);
|
||||
defaultCommitType = list?.DefaultCommitType ?? "chore";
|
||||
}
|
||||
|
||||
var editor = _editorFactory();
|
||||
await editor.LoadAgentsAsync(_worker);
|
||||
editor.InitForCreate(listId, defaultCommitType);
|
||||
// …rest unchanged, but use `listId` consistently where CurrentListId was read
|
||||
```
|
||||
|
||||
Audit the rest of the method: replace every subsequent read of `CurrentListId` with `listId`.
|
||||
|
||||
- [ ] **Step 2: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds with no CS8604 on this method.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Ui/ViewModels/TaskListViewModel.cs
|
||||
git commit -m "fix(ui): capture CurrentListId before await in AddTask"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 10: Ui — defer `_taskId` assignment in `TaskDetailViewModel.LoadAsync` (U3)
|
||||
|
||||
Bug: `_taskId = taskId` is set at line 87, before the previous `_loadCts` is cancelled. If load is cancelled, `_taskId` has been clobbered but `HasWorktree` / `CanWorktreeAction` still reflect the previous task.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskDetailViewModel.cs:76-90`
|
||||
|
||||
- [ ] **Step 1: Reset stale worktree state when starting a new load**
|
||||
|
||||
Replace the start of `LoadAsync`:
|
||||
|
||||
```csharp
|
||||
public async Task LoadAsync(string taskId)
|
||||
{
|
||||
var oldCts = _loadCts;
|
||||
var cts = new CancellationTokenSource();
|
||||
_loadCts = cts;
|
||||
oldCts?.Cancel();
|
||||
oldCts?.Dispose();
|
||||
var ct = cts.Token;
|
||||
|
||||
_taskId = taskId;
|
||||
|
||||
// Clear stale worktree state so buttons don't act on the previous task.
|
||||
HasWorktree = false;
|
||||
WorktreeState = "";
|
||||
BranchName = null;
|
||||
DiffStat = null;
|
||||
WorktreePath = null;
|
||||
OnPropertyChanged(nameof(CanWorktreeAction));
|
||||
|
||||
LiveText = "";
|
||||
_formatter = new StreamLineFormatter();
|
||||
// …rest unchanged
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Ui/ViewModels/TaskDetailViewModel.cs
|
||||
git commit -m "fix(ui): reset stale worktree state on TaskDetail reload"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 11: Ui — initialize TCS before dialog shown in `TaskEditorViewModel` (U4)
|
||||
|
||||
Bug: `ShowAndWaitAsync` creates a fresh `_tcs` only when called. If `Save` fires before `ShowAndWaitAsync` (possible if `ShowDialogAsync` is ever awaited), the result is dropped.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/TaskEditorViewModel.cs:72-80, 260-264`
|
||||
- Modify: `src/ClaudeDo.Ui/ViewModels/ListEditorViewModel.cs` (same pattern — apply identically)
|
||||
|
||||
- [ ] **Step 1: Reset `_tcs` at the start of `InitForCreate` and `InitForEditAsync`**
|
||||
|
||||
In `TaskEditorViewModel.cs`, at the top of `InitForCreate`:
|
||||
|
||||
```csharp
|
||||
public void InitForCreate(string listId, string defaultCommitType = "chore")
|
||||
{
|
||||
_tcs = new TaskCompletionSource<TaskEntity?>();
|
||||
_editId = null;
|
||||
// …rest unchanged
|
||||
```
|
||||
|
||||
Same first line at the top of `InitForEditAsync` and `InitForEdit`.
|
||||
|
||||
- [ ] **Step 2: Remove re-assignment in `ShowAndWaitAsync`**
|
||||
|
||||
```csharp
|
||||
public Task<TaskEntity?> ShowAndWaitAsync() => _tcs.Task;
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Apply the same pattern to `ListEditorViewModel`**
|
||||
|
||||
Mirror the same three edits in `src/ClaudeDo.Ui/ViewModels/ListEditorViewModel.cs` (reset TCS in its `InitForCreate` / `InitForEdit`, strip the creation in `ShowAndWaitAsync`).
|
||||
|
||||
- [ ] **Step 4: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Ui/ViewModels/TaskEditorViewModel.cs src/ClaudeDo.Ui/ViewModels/ListEditorViewModel.cs
|
||||
git commit -m "fix(ui): init editor TCS before dialog can complete"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 12: Installer — expand `~` in `UiDbPath` (I5)
|
||||
|
||||
Bug: `workerCfg.DbPath = Paths.Expand(ctx.DbPath)` but `uiCfg.DbPath = ctx.UiDbPath` is stored as-is. If UI cannot expand `~` at runtime on Windows, DB path is unresolvable.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ClaudeDo.Installer/Steps/WriteConfigStep.cs:31-34`
|
||||
|
||||
- [ ] **Step 1: Expand UiDbPath symmetrically**
|
||||
|
||||
Change the assignment:
|
||||
|
||||
```csharp
|
||||
uiCfg.DbPath = Paths.Expand(ctx.UiDbPath);
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/Steps/WriteConfigStep.cs
|
||||
git commit -m "fix(installer): expand ~ in UiDbPath"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Info
|
||||
|
||||
### Task 13: Installer — verify App.xaml.cs WPF-vs-Avalonia usings (I6)
|
||||
|
||||
Suspected bug: `src/ClaudeDo.Installer/App.xaml.cs` uses `System.Windows` (WPF). If the project is Avalonia, wrong base class is inherited.
|
||||
|
||||
**Files:**
|
||||
- Read: `src/ClaudeDo.Installer/App.xaml.cs`
|
||||
- Read: `src/ClaudeDo.Installer/ClaudeDo.Installer.csproj`
|
||||
|
||||
- [ ] **Step 1: Inspect the csproj for the UI framework SDK**
|
||||
|
||||
Read `src/ClaudeDo.Installer/ClaudeDo.Installer.csproj` and look for `<UseWPF>` or `<PackageReference Include="Avalonia" ... />`.
|
||||
|
||||
- [ ] **Step 2: Decision fork**
|
||||
|
||||
- If WPF (`<UseWPF>true</UseWPF>`): `System.Windows` is correct. Stop. No fix needed.
|
||||
- If Avalonia: replace `using System.Windows;` with `using Avalonia;` and change `Application` / `StartupEventArgs` / `ExitEventArgs` to Avalonia equivalents (`Avalonia.Application`, lifetime `OnFrameworkInitializationCompleted`).
|
||||
|
||||
- [ ] **Step 3: Build**
|
||||
|
||||
Run: `dotnet build ClaudeDo.slnx`
|
||||
Expected: build succeeds.
|
||||
|
||||
- [ ] **Step 4: Commit only if changed**
|
||||
|
||||
```bash
|
||||
git add src/ClaudeDo.Installer/App.xaml.cs
|
||||
git commit -m "fix(installer): use Avalonia application base class"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Out of Scope
|
||||
|
||||
Deferred / not fixed in this plan:
|
||||
- `TryScheduleTrampolineDelete` PID-less delay (I6 in review, low severity) — `ping -n 3` is flaky but rarely hit
|
||||
- `AvailableAgents` being `List<T>` instead of `ObservableCollection<T>` (U5/info) — current `OnPropertyChanged` pattern works; revisit only if a bug manifests
|
||||
|
||||
---
|
||||
|
||||
## Self-Review Notes
|
||||
|
||||
- Every Worker bug (W1–W3) has a regression test or tested path.
|
||||
- Every UI fix names the exact file:line and shows the replacement snippet.
|
||||
- Installer Task 3 (I1) does not guess a password-capture UI — it deliberately returns `StepResult.Fail`, leaving the UX change for a later plan.
|
||||
- Task 13 (I6) is a conditional task with a decision fork; no speculative rewrite.
|
||||
- Types are consistent: `RunCreated(taskId, runNumber, isRetry)` in Task 1 matches the existing `HubBroadcaster.RunCreated` signature used at `TaskRunner.cs:128,219`.
|
||||
Reference in New Issue
Block a user