23 KiB
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 prematureRunCreatedbroadcast - Modify:
src/ClaudeDo.Worker/Runner/TaskRunner.cs— emitRunCreatedafter run row insert - Modify:
src/ClaudeDo.Worker/Services/QueueService.cs— add slot-collision guard onRunNow/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 inAddTask, hardenOnTaskUpdated - Modify:
src/ClaudeDo.Ui/ViewModels/TaskDetailViewModel.cs— defer_taskIdassignment 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— addremoveAppDataparameter - Modify:
src/ClaudeDo.Installer/Steps/WriteConfigStep.cs— expand~inUiDbPath - 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:
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
RunCreatedinsideRunOnceAsyncafter row insert
In src/ClaudeDo.Worker/Runner/TaskRunner.cs, find RunOnceAsync. After the runRepo.AddAsync(run, ct); block (~line 256), add:
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
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:
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
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:
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
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:
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:
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:
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
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:
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
Escapecondition and escape newline/tab
In src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs, replace Escape:
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
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) — ensureStartServiceStepis 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 startfrom RegisterServiceStep
Delete the block (~lines 72-77) that runs sc.exe start <service> when ctx.AutoStart == true.
- Step 3: Add
StartServiceStepto 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
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:
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
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.RunAsyncsignature
Read src/ClaudeDo.Installer/Core/UninstallRunner.cs around lines 1-90 to get current signature.
- Step 2: Add
removeAppDataparameter toRunAsync
Change signature to:
public async Task<UninstallResult> RunAsync(bool removeAppData, CancellationToken ct = default)
Guard the deletion:
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
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
CurrentListIdbefore the firstawait
Replace the start of AddTask:
[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
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:
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
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
_tcsat the start ofInitForCreateandInitForEditAsync
In TaskEditorViewModel.cs, at the top of InitForCreate:
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
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
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:
uiCfg.DbPath = Paths.Expand(ctx.UiDbPath);
- Step 2: Build
Run: dotnet build ClaudeDo.slnx
Expected: build succeeds.
- Step 3: Commit
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.Windowsis correct. Stop. No fix needed. -
If Avalonia: replace
using System.Windows;withusing Avalonia;and changeApplication/StartupEventArgs/ExitEventArgsto Avalonia equivalents (Avalonia.Application, lifetimeOnFrameworkInitializationCompleted). -
Step 3: Build
Run: dotnet build ClaudeDo.slnx
Expected: build succeeds.
- Step 4: Commit only if changed
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:
TryScheduleTrampolineDeletePID-less delay (I6 in review, low severity) —ping -n 3is flaky but rarely hitAvailableAgentsbeingList<T>instead ofObservableCollection<T>(U5/info) — currentOnPropertyChangedpattern 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 existingHubBroadcaster.RunCreatedsignature used atTaskRunner.cs:128,219.