diff --git a/src/ClaudeDo.App/Assets/ClaudeTask.ico b/src/ClaudeDo.App/Assets/ClaudeTask.ico index 1ca3aef..058c5df 100644 Binary files a/src/ClaudeDo.App/Assets/ClaudeTask.ico and b/src/ClaudeDo.App/Assets/ClaudeTask.ico differ diff --git a/src/ClaudeDo.Data/Git/GitService.cs b/src/ClaudeDo.Data/Git/GitService.cs index 7ba2cf9..fc1edd5 100644 --- a/src/ClaudeDo.Data/Git/GitService.cs +++ b/src/ClaudeDo.Data/Git/GitService.cs @@ -124,6 +124,20 @@ public sealed class GitService return await GetDiffAsync(worktreePath, ct); } + /// + /// Diff between two commits, run in any repo that can reach them. Used to view a + /// task's changes after its worktree has been merged away (the commits survive on + /// the target branch even though the worktree directory and branch ref are gone). + /// + public async Task GetCommitRangeDiffAsync(string repoDir, string baseCommit, string headCommit, CancellationToken ct = default) + { + var (exitCode, stdout, stderr) = await RunGitAsync(repoDir, + ["diff", $"{baseCommit}..{headCommit}"], ct); + if (exitCode != 0) + throw new InvalidOperationException($"git diff {baseCommit}..{headCommit} failed (exit {exitCode}): {stderr}"); + return stdout; + } + public async Task DiffStatAsync(string worktreePath, string baseCommit, string headCommit, CancellationToken ct = default) { var (exitCode, stdout, stderr) = await RunGitAsync(worktreePath, diff --git a/src/ClaudeDo.Installer/ClaudeTaskSetup.ico b/src/ClaudeDo.Installer/ClaudeTaskSetup.ico index 87fa5fa..0fa0d35 100644 Binary files a/src/ClaudeDo.Installer/ClaudeTaskSetup.ico and b/src/ClaudeDo.Installer/ClaudeTaskSetup.ico differ diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index 4b8a1b2..d5737d5 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -238,7 +238,10 @@ "diff": { "title": "DIFF", "windowTitle": "Diff", - "merge": "Mergen…" + "merge": "Mergen…", + "filesHeader": "Dateien", + "binary": "Binärdatei — kein Text-Diff", + "empty": "Kein Inhalt" }, "worktree": { "title": "Worktree" diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index aae7ce7..4457637 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -238,7 +238,10 @@ "diff": { "title": "DIFF", "windowTitle": "Diff", - "merge": "Merge…" + "merge": "Merge…", + "filesHeader": "Files", + "binary": "Binary file — no text diff", + "empty": "No content" }, "worktree": { "title": "Worktree" diff --git a/src/ClaudeDo.Ui/Design/IslandStyles.axaml b/src/ClaudeDo.Ui/Design/IslandStyles.axaml index 2dbfe30..d4737d3 100644 --- a/src/ClaudeDo.Ui/Design/IslandStyles.axaml +++ b/src/ClaudeDo.Ui/Design/IslandStyles.axaml @@ -574,6 +574,13 @@ + + + diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index fdefcc9..480d17f 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -165,6 +165,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable { OnPropertyChanged(nameof(HasChildOutcomes)); OnPropertyChanged(nameof(ShowMergeSection)); + NotifyAttention(); // The Session tab is only visible when it has outcomes; if it just // emptied while selected, fall back to Output so the body isn't blank. @@ -354,7 +355,11 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable [ObservableProperty] private string? _worktreePath; [ObservableProperty] private string? _worktreeBaseCommit; + [ObservableProperty] private string? _worktreeHeadCommit; [ObservableProperty] private string? _worktreeStateLabel; + // Repo working dir of the selected task's list — used to diff a merged task's + // commit range after its worktree directory is gone. + private string? _listWorkingDir; [ObservableProperty] private string? _branchLine; [ObservableProperty] private int _turns; [ObservableProperty] private int _tokens; @@ -388,6 +393,27 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable public ObservableCollection ChildOutcomes { get; } = new(); public bool HasChildOutcomes => ChildOutcomes.Count > 0; + // Children that need the user's attention before the parent can be approved: + // failed, cancelled, still awaiting their own review, or that reported roadblocks. + // The parent deliberately stays in WaitingForChildren until these are resolved; + // this surfaces a flag so the roadblock is visible on the parent. + public int ChildrenNeedingAttention => ChildOutcomes.Count(c => + c.Status == ClaudeDo.Data.Models.TaskStatus.Failed + || c.Status == ClaudeDo.Data.Models.TaskStatus.Cancelled + || c.Status == ClaudeDo.Data.Models.TaskStatus.WaitingForReview + || c.RoadblockCount > 0); + public bool HasChildrenNeedingAttention => ChildrenNeedingAttention > 0; + public string ChildrenAttentionText => ChildrenNeedingAttention == 1 + ? "1 child needs attention" + : $"{ChildrenNeedingAttention} children need attention"; + + private void NotifyAttention() + { + OnPropertyChanged(nameof(ChildrenNeedingAttention)); + OnPropertyChanged(nameof(HasChildrenNeedingAttention)); + OnPropertyChanged(nameof(ChildrenAttentionText)); + } + [ObservableProperty] private string _newSubtaskTitle = ""; // Planning merge controls @@ -840,6 +866,8 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable EditableDescription = ""; Model = null; WorktreePath = null; + WorktreeHeadCommit = null; + _listWorkingDir = null; WorktreeStateLabel = null; BranchLine = null; DiffAdditions = 0; @@ -876,6 +904,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable var entity = await ctx.Tasks .AsNoTracking() .Include(t => t.Worktree) + .Include(t => t.List) .FirstOrDefaultAsync(t => t.Id == row.Id, ct); ct.ThrowIfCancellationRequested(); if (entity == null) return; @@ -885,8 +914,10 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable try { EditableDescription = entity.Description ?? ""; } finally { _suppressDescSave = false; } Model = entity.Model; + _listWorkingDir = entity.List?.WorkingDir; WorktreePath = entity.Worktree?.Path; WorktreeBaseCommit = entity.Worktree?.BaseCommit; + WorktreeHeadCommit = entity.Worktree?.HeadCommit; WorktreeStateLabel = entity.Worktree?.State.ToString(); BranchLine = entity.Worktree is { } w ? $"{w.BranchName} \u2190 main" : null; var (add, del) = ParseDiffStat(entity.Worktree?.DiffStat); @@ -914,13 +945,12 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable Subtasks.Add(new SubtaskRowViewModel { Id = s.Id, Title = s.Title, Done = s.Completed }); if (entity.PlanningPhase != ClaudeDo.Data.Models.PlanningPhase.None) - { await LoadPlanningChildrenAsync(row.Id, ct); - } - else - { - await LoadChildOutcomesAsync(row.Id, ct); - } + // Surface every parent's children — planning or improvement — in the + // Session tab with their live status + roadblock count. This is what + // makes the Session tab appear for planning parents and lets a child's + // roadblock register on the parent. + await LoadChildOutcomesAsync(row.Id, ct); if (entity.Worktree != null && entity.PlanningPhase == ClaudeDo.Data.Models.PlanningPhase.None @@ -1125,6 +1155,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable row.RoadblockCount = child.RoadblockCount; row.WorktreeState = child.Worktree?.State ?? ClaudeDo.Data.Models.WorktreeState.Active; ReviewCombinedDiffCommand.NotifyCanExecuteChanged(); + NotifyAttention(); } catch { /* best-effort */ } } @@ -1148,11 +1179,14 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable var entity = await ctx.Tasks .AsNoTracking() .Include(t => t.Worktree) + .Include(t => t.List) .FirstOrDefaultAsync(t => t.Id == taskId); if (entity == null || Task?.Id != taskId) return; + _listWorkingDir = entity.List?.WorkingDir; WorktreePath = entity.Worktree?.Path; WorktreeBaseCommit = entity.Worktree?.BaseCommit; + WorktreeHeadCommit = entity.Worktree?.HeadCommit; WorktreeStateLabel = entity.Worktree?.State.ToString(); BranchLine = entity.Worktree is { } w ? $"{w.BranchName} ← main" : null; AgentState = StatusToStateKey(entity.Status); @@ -1190,21 +1224,53 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable [RelayCommand(CanExecute = nameof(CanOpenDiff))] private async System.Threading.Tasks.Task OpenDiffAsync() { - if (WorktreePath == null || ShowDiffModal == null) return; - var diffVm = new DiffModalViewModel(_services.GetRequiredService()) + if (ShowDiffModal == null) return; + var git = _services.GetRequiredService(); + + // Active worktree on disk → diff the worktree live (and allow merging from it). + var hasLiveWorktree = + WorktreePath != null + && WorktreeStateLabel == "Active" + && System.IO.Directory.Exists(WorktreePath); + + DiffModalViewModel diffVm; + if (hasLiveWorktree) { - WorktreePath = WorktreePath, - BaseRef = WorktreeBaseCommit, - TaskId = Task?.Id, - TaskTitle = Task?.Title ?? "", - ShowMergeModal = ShowMergeModal, - ResolveMergeVm = () => _services.GetRequiredService(), - }; + diffVm = new DiffModalViewModel(git) + { + WorktreePath = WorktreePath!, + BaseRef = WorktreeBaseCommit, + TaskId = Task?.Id, + TaskTitle = Task?.Title ?? "", + ShowMergeModal = ShowMergeModal, + ResolveMergeVm = () => _services.GetRequiredService(), + }; + } + else if (CanDiffMergedRange) + { + // Worktree is gone (merged/discarded) but the commits survive on the + // target branch — diff the captured base..head range in the repo. No + // merge action: the work is already integrated. + diffVm = new DiffModalViewModel(git) + { + WorktreePath = _listWorkingDir!, + BaseRef = WorktreeBaseCommit, + HeadCommit = WorktreeHeadCommit, + FromCommitRange = true, + TaskId = Task?.Id, + TaskTitle = Task?.Title ?? "", + }; + } + else return; + await diffVm.LoadAsync(); await ShowDiffModal(diffVm); } - private bool CanOpenDiff() => WorktreePath != null; + private bool CanDiffMergedRange => + WorktreeBaseCommit != null && WorktreeHeadCommit != null && _listWorkingDir != null; + + private bool CanOpenDiff() => WorktreePath != null || CanDiffMergedRange; [RelayCommand(CanExecute = nameof(CanOpenWorktree))] private void OpenWorktree() @@ -1235,6 +1301,9 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable NotifySessionSections(); } + partial void OnWorktreeHeadCommitChanged(string? value) => + OpenDiffCommand.NotifyCanExecuteChanged(); + partial void OnTaskChanged(TaskRowViewModel? value) => NotifySessionSections(); [RelayCommand] @@ -1473,7 +1542,13 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable } // hasChildren: conflicts arrive via PlanningMergeConflictEvent → conflict dialog } - catch { /* stale review action; broadcast reconciles */ } + catch (Exception ex) + { + // A real failure (e.g. a child still needs attention, so the unit can't + // be approved yet) must not vanish — tell the user why nothing happened. + if (ShowErrorAsync != null) + await ShowErrorAsync(ex.Message); + } } [RelayCommand] diff --git a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs index 31e78af..c25b2c3 100644 --- a/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs @@ -154,21 +154,23 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable if (ShowConflictDialog == null || _dbFactory == null) return; string subtaskTitle = subtaskId; - string worktreePath = System.Environment.CurrentDirectory; + // The conflict lives in the list's working dir (the repo being merged into), + // not the subtask worktree. VS Code must open this folder to show the merge UI. + string repoDirectory = System.Environment.CurrentDirectory; string targetBranch = Worker?.LastApproveTarget ?? "main"; try { await using var ctx = await _dbFactory.CreateDbContextAsync(); var entity = await ctx.Tasks - .Include(t => t.Worktree) + .Include(t => t.List) .AsNoTracking() .FirstOrDefaultAsync(t => t.Id == subtaskId); if (entity != null) { subtaskTitle = entity.Title; - if (entity.Worktree?.Path is { } p) - worktreePath = p; + if (entity.List?.WorkingDir is { } dir && !string.IsNullOrWhiteSpace(dir)) + repoDirectory = dir; } } catch { /* Non-fatal: fall back to subtaskId and cwd */ } @@ -179,7 +181,7 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable subtaskTitle, targetBranch, conflictedFiles, - worktreePath); + repoDirectory); await ShowConflictDialog(vm); } diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/DiffModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/DiffModalViewModel.cs index d8f1129..c92caaf 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/DiffModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/DiffModalViewModel.cs @@ -8,6 +8,8 @@ namespace ClaudeDo.Ui.ViewModels.Modals; public enum DiffLineKind { Add, Del, Ctx, File } +public enum DiffFileStatus { Modified, Added, Deleted, Renamed } + public sealed class DiffLineViewModel { public required DiffLineKind Kind { get; init; } @@ -32,10 +34,27 @@ public sealed class DiffLineViewModel public sealed class DiffFileViewModel { - public required string Path { get; init; } + public required string Path { get; set; } + public string? OldPath { get; set; } + public DiffFileStatus Status { get; set; } = DiffFileStatus.Modified; + public bool IsBinary { get; set; } public int Additions { get; set; } public int Deletions { get; set; } public ObservableCollection Lines { get; } = new(); + + /// Single-letter badge for the file's change kind (A/M/D/R). + public string StatusCode => Status switch + { + DiffFileStatus.Added => "A", + DiffFileStatus.Deleted => "D", + DiffFileStatus.Renamed => "R", + _ => "M", + }; + + public bool HasLines => Lines.Count > 0; + + /// A text file that produced no diff hunks (e.g. a newly added empty file). + public bool IsEmptyContent => !IsBinary && Lines.Count == 0; } public sealed partial class DiffModalViewModel : ViewModelBase @@ -44,6 +63,11 @@ public sealed partial class DiffModalViewModel : ViewModelBase public required string WorktreePath { get; init; } public string? BaseRef { get; init; } + /// When set together with , the diff is computed as + /// BaseRef..HeadCommit inside (used as the repo + /// dir) — lets a merged task's diff be viewed after its worktree is gone. + public string? HeadCommit { get; init; } + public bool FromCommitRange { get; init; } public string? TaskId { get; init; } public string TaskTitle { get; init; } = ""; public Func? ShowMergeModal { get; set; } @@ -77,6 +101,8 @@ public sealed partial class DiffModalViewModel : ViewModelBase var vm = ResolveMergeVm(); await vm.InitializeAsync(TaskId, TaskTitle); await ShowMergeModal(vm); + // The diff is stale once the worktree has been merged away — close it too. + if (vm.Merged) CloseAction?.Invoke(); } public async Task LoadAsync(CancellationToken ct = default) @@ -87,9 +113,11 @@ public sealed partial class DiffModalViewModel : ViewModelBase string raw; try { - raw = BaseRef is not null - ? await _git.GetBranchDiffAsync(WorktreePath, BaseRef, ct) - : await _git.GetDiffAsync(WorktreePath, ct); + raw = FromCommitRange && BaseRef is not null && HeadCommit is not null + ? await _git.GetCommitRangeDiffAsync(WorktreePath, BaseRef, HeadCommit, ct) + : BaseRef is not null + ? await _git.GetBranchDiffAsync(WorktreePath, BaseRef, ct) + : await _git.GetDiffAsync(WorktreePath, ct); } catch (Exception ex) { diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs index 68ae36a..fb3f1a6 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs @@ -28,6 +28,10 @@ public sealed partial class MergeModalViewModel : ViewModelBase public Action? CloseAction { get; set; } + /// True once a merge has succeeded — lets the caller (e.g. the diff window) + /// close itself after this modal closes. + public bool Merged { get; private set; } + public MergeModalViewModel(WorkerClient worker) { _worker = worker; @@ -80,6 +84,7 @@ public sealed partial class MergeModalViewModel : ViewModelBase switch (result.Status) { case "merged": + Merged = true; SuccessMessage = result.ErrorMessage is not null ? $"Merged with warning: {result.ErrorMessage}" : Loc.T("vm.merge.merged"); diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/UnifiedDiffParser.cs b/src/ClaudeDo.Ui/ViewModels/Modals/UnifiedDiffParser.cs index b1e216e..21f6965 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/UnifiedDiffParser.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/UnifiedDiffParser.cs @@ -27,6 +27,36 @@ public static class UnifiedDiffParser if (current == null) continue; + // File-level metadata that carries the change kind. + if (line.StartsWith("new file", StringComparison.Ordinal)) + { + current.Status = DiffFileStatus.Added; + continue; + } + if (line.StartsWith("deleted file", StringComparison.Ordinal)) + { + current.Status = DiffFileStatus.Deleted; + continue; + } + if (line.StartsWith("rename from ", StringComparison.Ordinal)) + { + current.Status = DiffFileStatus.Renamed; + current.OldPath = line["rename from ".Length..]; + continue; + } + if (line.StartsWith("rename to ", StringComparison.Ordinal)) + { + current.Status = DiffFileStatus.Renamed; + current.Path = line["rename to ".Length..]; + continue; + } + if (line.StartsWith("Binary files", StringComparison.Ordinal) || + line.StartsWith("GIT binary patch", StringComparison.Ordinal)) + { + current.IsBinary = true; + continue; + } + if (line.StartsWith("@@ ", StringComparison.Ordinal)) { // e.g. "@@ -10,7 +10,9 @@" @@ -34,13 +64,15 @@ public static class UnifiedDiffParser continue; } - // Skip diff metadata lines + // Skip remaining diff metadata lines if (line.StartsWith("--- ", StringComparison.Ordinal) || line.StartsWith("+++ ", StringComparison.Ordinal) || line.StartsWith("index ", StringComparison.Ordinal) || - line.StartsWith("new file", StringComparison.Ordinal) || - line.StartsWith("deleted file", StringComparison.Ordinal) || - line.StartsWith("Binary ", StringComparison.Ordinal)) + line.StartsWith("old mode", StringComparison.Ordinal) || + line.StartsWith("new mode", StringComparison.Ordinal) || + line.StartsWith("similarity index", StringComparison.Ordinal) || + line.StartsWith("copy from", StringComparison.Ordinal) || + line.StartsWith("copy to", StringComparison.Ordinal)) continue; if (line.StartsWith('+')) diff --git a/src/ClaudeDo.Ui/ViewModels/Planning/ConflictResolutionViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Planning/ConflictResolutionViewModel.cs index 7b3023b..08ea33e 100644 --- a/src/ClaudeDo.Ui/ViewModels/Planning/ConflictResolutionViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Planning/ConflictResolutionViewModel.cs @@ -10,7 +10,10 @@ public sealed partial class ConflictResolutionViewModel : ObservableObject { private readonly IWorkerClient _worker; private readonly string _planningTaskId; - private readonly string _worktreePath; + // The repository directory that is currently mid-merge (the list's working dir), + // NOT the subtask worktree. Opening this folder is what makes VS Code show its + // merge-conflict resolution UI. + private readonly string _repoDirectory; public string SubtaskTitle { get; } public string TargetBranch { get; } @@ -29,11 +32,11 @@ public sealed partial class ConflictResolutionViewModel : ObservableObject string subtaskTitle, string targetBranch, IReadOnlyList conflictedFiles, - string worktreePath) + string repoDirectory) { _worker = worker; _planningTaskId = planningTaskId; - _worktreePath = worktreePath; + _repoDirectory = repoDirectory; SubtaskTitle = subtaskTitle; TargetBranch = targetBranch; ConflictedFiles = conflictedFiles; @@ -44,12 +47,13 @@ public sealed partial class ConflictResolutionViewModel : ObservableObject { try { - var args = string.Join(" ", ConflictedFiles.Select(f => $"\"{f}\"")); + // Open the folder that is mid-merge so VS Code shows the Source Control + // merge-conflict UI for every conflicted file. Opening individual files + // gives only a plain editor with no conflict resolution affordances. Process.Start(new ProcessStartInfo { FileName = "code", - Arguments = args, - WorkingDirectory = _worktreePath, + Arguments = $"\"{_repoDirectory}\"", UseShellExecute = true, }); VsCodeError = null; diff --git a/src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml b/src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml index 0405baf..849ddfb 100644 --- a/src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml +++ b/src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml @@ -313,6 +313,22 @@ + + + + + + + + diff --git a/src/ClaudeDo.Ui/Views/Modals/DiffModalView.axaml b/src/ClaudeDo.Ui/Views/Modals/DiffModalView.axaml index a12606a..7a7cb7f 100644 --- a/src/ClaudeDo.Ui/Views/Modals/DiffModalView.axaml +++ b/src/ClaudeDo.Ui/Views/Modals/DiffModalView.axaml @@ -26,51 +26,100 @@ - - + + - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - + + + + + - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index d3fb60e..4aa86af 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -102,8 +102,10 @@ approve is the single review+merge action. Review transitions live in `TaskState `PlanningSessionManager.FinalizeAsync` is the single path: 1. `_state.FinalizePlanningAsync(parent)` flips parent `PlanningPhase` to `Finalized` and sets `Status` to `WaitingForChildren` (or `WaitingForReview` if the parent has no children). -2. `PlanningChainCoordinator.SetupChainAsync` attaches the `agent` tag to every child, enqueues child[0], and `BlockOn`s child[i] → child[i-1]. -3. The first child is woken automatically; successors unblock as their predecessor reaches a terminal state via `OnChildFinishedAsync`. +2. `PlanningChainCoordinator.SetupChainAsync(parent, enqueue: false)` establishes the blocked-by chain (`BlockOn`s child[i] → child[i-1]) but **leaves children `Idle`** — finalize never auto-queues. Queueing is a deliberate user action: `QueuePlanAsync` (hub `QueuePlanningSubtasksAsync`, the "Queue plan" button) calls `SetupChainAsync(parent, enqueue: true)`, which sets every non-terminal child `Queued` and re-applies the chain. +3. Once queued, the first child is woken automatically; successors unblock as their predecessor reaches a terminal state via `OnChildFinishedAsync`. + +A child that hits a roadblock (fails, or reports `CLAUDEDO_BLOCKED` roadblocks) does **not** advance the parent — the parent stays in `WaitingForChildren` until every child is terminal. The UI surfaces blocked children on the parent's Session tab (`ChildOutcomes` + a "children need attention" band) so the roadblock is visible without forcing a transition. `TaskRepository.FinalizePlanningAsync` no longer exists. The `Mark*Async` repository helpers are `internal` — only `TaskStateService` calls them. diff --git a/src/ClaudeDo.Worker/ClaudeDo.Worker.csproj b/src/ClaudeDo.Worker/ClaudeDo.Worker.csproj index 45c64d3..aac05c3 100644 --- a/src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +++ b/src/ClaudeDo.Worker/ClaudeDo.Worker.csproj @@ -27,6 +27,7 @@ WinExe enable enable + ClaudeTaskWorker.ico diff --git a/src/ClaudeDo.Worker/ClaudeTaskWorker.ico b/src/ClaudeDo.Worker/ClaudeTaskWorker.ico new file mode 100644 index 0000000..27ae8ae Binary files /dev/null and b/src/ClaudeDo.Worker/ClaudeTaskWorker.ico differ diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index 7af24e3..0d46ae1 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -325,7 +325,9 @@ public sealed class TaskMergeService : targetBranch; // MergeAsync transitions the task WaitingForReview -> Done on a successful merge. - return await MergeAsync(taskId, target, removeWorktree: false, $"Merge {wt.BranchName}", ct); + // Remove the worktree on approve (matching the unit-merge path) so merged + // worktrees don't pile up; the merge commit on the target branch is the record. + return await MergeAsync(taskId, target, removeWorktree: true, $"Merge {wt.BranchName}", ct); } private static MergeResult Blocked(string reason) => diff --git a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs index 7e87de8..1275411 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs @@ -19,17 +19,21 @@ public sealed class PlanningChainCoordinator _state = state; } - // Sets up a sequential queue chain over a planning parent's children. - // - First non-terminal child gets Status=Queued, BlockedByTaskId=null. - // - Each subsequent non-terminal child gets Status=Queued + BlockedByTaskId=, + // Sets up a sequential chain over a planning parent's children. + // - First non-terminal child gets BlockedByTaskId=null. + // - Each subsequent non-terminal child gets BlockedByTaskId=, // so the picker skips them until the predecessor finishes. + // - When enqueue is true, each non-terminal child is also set to Status=Queued + // (the user-driven "Queue plan"). When false (finalize), children are left + // Idle and only the blocked-by links are established, so nothing runs until + // the user queues the plan. // - Terminal children (Done/Failed/Cancelled) are left untouched; they are // skipped when computing predecessors so a re-run on a partially executed // chain leaves history alone but still reshapes the tail. // - Running children abort the operation — the chain cannot be reshaped while // one of its members is mid-flight. // Returns the number of children placed in the chain. - internal async Task SetupChainAsync(string parentTaskId, CancellationToken ct = default) + internal async Task SetupChainAsync(string parentTaskId, bool enqueue, CancellationToken ct = default) { await using var ctx = await _dbFactory.CreateDbContextAsync(ct); var parent = await ctx.Tasks.FirstOrDefaultAsync(t => t.Id == parentTaskId, ct) @@ -56,7 +60,8 @@ public sealed class PlanningChainCoordinator var state = _state(); for (int i = 0; i < sequenceable.Count; i++) { - await state.EnqueueAsync(sequenceable[i].Id, ct); + if (enqueue) + await state.EnqueueAsync(sequenceable[i].Id, ct); if (i == 0) await state.UnblockAsync(sequenceable[i].Id, ct); else @@ -81,7 +86,7 @@ public sealed class PlanningChainCoordinator if (phase != PlanningPhase.Finalized) throw new InvalidOperationException("Plan must be finalized before it can be queued."); - return await SetupChainAsync(parentTaskId, ct); + return await SetupChainAsync(parentTaskId, enqueue: true, ct); } public async Task OnChildFinishedAsync( diff --git a/src/ClaudeDo.Worker/Planning/PlanningMcpService.cs b/src/ClaudeDo.Worker/Planning/PlanningMcpService.cs index 7d57df8..c694aee 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningMcpService.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningMcpService.cs @@ -135,7 +135,7 @@ public sealed class PlanningMcpService await BroadcastTaskUpdatedAsync(ctx.ParentTaskId, cancellationToken); } - [McpServerTool, Description("Finalize the planning session, promoting all draft child tasks to queued or manual status.")] + [McpServerTool, Description("Finalize the planning session. Child tasks are left idle and chain-linked (each blocked by its predecessor); they are NOT queued automatically — the user queues the plan from the app when ready. The queueAgentTasks argument is accepted for compatibility but ignored.")] public async Task Finalize( bool queueAgentTasks, CancellationToken cancellationToken) @@ -149,8 +149,10 @@ public sealed class PlanningMcpService var children = await _tasks.GetChildrenAsync(ctx.ParentTaskId, cancellationToken); int count = children.Count; - if (queueAgentTasks && children.Count > 0) - count = await _chain.SetupChainAsync(ctx.ParentTaskId, cancellationToken); + // Establish the blocked-by chain but leave children Idle; queueing is a + // deliberate user action ("Queue plan"), never an automatic finalize step. + if (children.Count > 0) + count = await _chain.SetupChainAsync(ctx.ParentTaskId, enqueue: false, cancellationToken); foreach (var c in children) await BroadcastTaskUpdatedAsync(c.Id, cancellationToken); diff --git a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs index d4c0042..b40196b 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs @@ -199,6 +199,10 @@ public sealed class PlanningMergeOrchestrator parent.FinishedAt = DateTime.UtcNow; await ctx.SaveChangesAsync(ct); + // Surface the Done transition to the UI. Without this the parent row stays + // visibly stuck in WaitingForReview even though the unit merge completed. + await _broadcaster.TaskUpdated(parentTaskId); + // Only planning builds an integration branch via the aggregator; skip cleanup otherwise. if (isPlanning) { diff --git a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs index e89ec95..7a56ca3 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs @@ -209,12 +209,13 @@ public sealed class PlanningSessionManager throw new InvalidOperationException( finalizeResult.Reason ?? $"Could not finalize planning for task {taskId}."); - int count = 0; + // Establish the blocked-by chain but leave children Idle; queueing is a + // deliberate user action ("Queue plan"), never an automatic finalize step. + // queueAgentTasks is accepted for compatibility but no longer auto-queues. var children = await tasks.GetChildrenAsync(taskId, ct); - if (queueAgentTasks && children.Count > 0) - count = await _chain.SetupChainAsync(taskId, ct); - else - count = children.Count; + int count = children.Count; + if (children.Count > 0) + count = await _chain.SetupChainAsync(taskId, enqueue: false, ct); // Best-effort cleanup — don't block finalization on git state. await TryCleanupWorktreeAsync(taskId, lists, settings, ct); diff --git a/src/ClaudeDo.Worker/Refine/RefineRunner.cs b/src/ClaudeDo.Worker/Refine/RefineRunner.cs index 14fdd1d..1439532 100644 --- a/src/ClaudeDo.Worker/Refine/RefineRunner.cs +++ b/src/ClaudeDo.Worker/Refine/RefineRunner.cs @@ -10,7 +10,7 @@ namespace ClaudeDo.Worker.Refine; public sealed class RefineRunner : IRefineRunner { private static readonly TimeSpan RunTimeout = TimeSpan.FromMinutes(5); - private const int MaxTurns = 25; + private const int MaxTurns = 5; private readonly IClaudeProcess _claude; private readonly IDbContextFactory _dbFactory; diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs index 01fb7fd..f8cd25c 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/ConflictResolutionViewModelTests.cs @@ -36,7 +36,7 @@ public class ConflictResolutionViewModelTests subtaskTitle: "My subtask", targetBranch: "main", conflictedFiles: new[] { "src/Foo.cs", "src/Bar.cs" }, - worktreePath: "C:/worktrees/plan-1"); + repoDirectory: "C:/repos/plan-1"); // ------------------------------------------------------------------ tests diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/UnifiedDiffParserTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/UnifiedDiffParserTests.cs new file mode 100644 index 0000000..8a63803 --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/UnifiedDiffParserTests.cs @@ -0,0 +1,109 @@ +using System.Linq; +using ClaudeDo.Ui.ViewModels.Modals; + +namespace ClaudeDo.Ui.Tests.ViewModels; + +public class UnifiedDiffParserTests +{ + [Fact] + public void Modified_file_counts_additions_and_deletions() + { + const string raw = + "diff --git a/src/Foo.cs b/src/Foo.cs\n" + + "index 111..222 100644\n" + + "--- a/src/Foo.cs\n" + + "+++ b/src/Foo.cs\n" + + "@@ -1,3 +1,3 @@\n" + + " ctx\n" + + "-old\n" + + "+new\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.Equal("src/Foo.cs", file.Path); + Assert.Equal(DiffFileStatus.Modified, file.Status); + Assert.Equal("M", file.StatusCode); + Assert.Equal(1, file.Additions); + Assert.Equal(1, file.Deletions); + Assert.True(file.HasLines); + Assert.False(file.IsBinary); + } + + [Fact] + public void New_file_is_marked_added() + { + const string raw = + "diff --git a/New.cs b/New.cs\n" + + "new file mode 100644\n" + + "index 000..abc\n" + + "--- /dev/null\n" + + "+++ b/New.cs\n" + + "@@ -0,0 +1,1 @@\n" + + "+hello\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.Equal(DiffFileStatus.Added, file.Status); + Assert.Equal("A", file.StatusCode); + } + + [Fact] + public void Deleted_file_is_marked_deleted() + { + const string raw = + "diff --git a/Gone.cs b/Gone.cs\n" + + "deleted file mode 100644\n" + + "index abc..000\n" + + "--- a/Gone.cs\n" + + "+++ /dev/null\n" + + "@@ -1,1 +0,0 @@\n" + + "-bye\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.Equal(DiffFileStatus.Deleted, file.Status); + Assert.Equal("D", file.StatusCode); + } + + [Fact] + public void Rename_captures_old_and_new_path() + { + const string raw = + "diff --git a/Old.cs b/New.cs\n" + + "similarity index 100%\n" + + "rename from Old.cs\n" + + "rename to New.cs\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.Equal(DiffFileStatus.Renamed, file.Status); + Assert.Equal("R", file.StatusCode); + Assert.Equal("Old.cs", file.OldPath); + Assert.Equal("New.cs", file.Path); + } + + [Fact] + public void Binary_file_is_flagged_with_no_lines() + { + const string raw = + "diff --git a/img.png b/img.png\n" + + "new file mode 100644\n" + + "index 000..abc\n" + + "Binary files /dev/null and b/img.png differ\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.True(file.IsBinary); + Assert.False(file.HasLines); + Assert.False(file.IsEmptyContent); + } + + [Fact] + public void Empty_new_file_reports_empty_content() + { + const string raw = + "diff --git a/Empty.txt b/Empty.txt\n" + + "new file mode 100644\n" + + "index 000..000\n"; + + var file = Assert.Single(UnifiedDiffParser.Parse(raw)); + Assert.Equal(DiffFileStatus.Added, file.Status); + Assert.False(file.HasLines); + Assert.True(file.IsEmptyContent); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs index d2b7239..bd568e6 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs @@ -75,7 +75,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable { await SeedPlanningFamilyAsync("P", 3); - var count = await _sut.SetupChainAsync("P", default); + var count = await _sut.SetupChainAsync("P", enqueue: true, default); Assert.Equal(3, count); var kids = await GetChildrenAsync("P"); @@ -92,7 +92,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable { await SeedPlanningFamilyAsync("P", 2, childStatus: TaskStatus.Idle); - var count = await _sut.SetupChainAsync("P", default); + var count = await _sut.SetupChainAsync("P", enqueue: true, default); Assert.Equal(2, count); } @@ -101,7 +101,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable public async Task OnChildDone_UnblocksTheSuccessor() { await SeedPlanningFamilyAsync("P", 3); - await _sut.SetupChainAsync("P", default); + await _sut.SetupChainAsync("P", enqueue: true, default); // Mark the head child Done before announcing. await using (var ctx = _factory.CreateDbContext()) @@ -128,7 +128,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable public async Task OnChildFailed_CancelsPendingSuccessors_ChainIsNotWedged() { await SeedPlanningFamilyAsync("P", 3); - await _sut.SetupChainAsync("P", default); + await _sut.SetupChainAsync("P", enqueue: true, default); await using (var ctx = _factory.CreateDbContext()) { @@ -153,7 +153,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable { // Chain: c0 → c1 → c2 → c3. c1 fails mid-chain; c2 and c3 must be cancelled. await SeedPlanningFamilyAsync("P", 4); - await _sut.SetupChainAsync("P", default); + await _sut.SetupChainAsync("P", enqueue: true, default); // Mark c0 Done so c1 was unblocked; c1 ran and failed. await using (var ctx = _factory.CreateDbContext()) @@ -182,7 +182,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable public async Task OnChildDone_LastChild_ReturnsNull() { await SeedPlanningFamilyAsync("P", 2); - await _sut.SetupChainAsync("P", default); + await _sut.SetupChainAsync("P", enqueue: true, default); await using (var ctx = _factory.CreateDbContext()) { @@ -208,7 +208,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable } await Assert.ThrowsAsync( - () => _sut.SetupChainAsync("P", default)); + () => _sut.SetupChainAsync("P", enqueue: true, default)); } [Fact] @@ -230,7 +230,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable await ctx.SaveChangesAsync(); } - var count = await _sut.SetupChainAsync("P", default); + var count = await _sut.SetupChainAsync("P", enqueue: true, default); Assert.Equal(4, count); var kids = await GetChildrenAsync("P"); @@ -257,7 +257,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable await ctx.SaveChangesAsync(); } - var count = await _sut.SetupChainAsync("P", default); + var count = await _sut.SetupChainAsync("P", enqueue: true, default); // Only the two non-terminal tail children get chained. Assert.Equal(2, count); @@ -303,4 +303,21 @@ public sealed class PlanningChainCoordinatorTests : IDisposable var kids = await GetChildrenAsync("P"); Assert.All(kids, k => Assert.Equal(TaskStatus.Idle, k.Status)); } + + [Fact] + public async Task SetupChain_LinkOnly_LeavesChildrenIdle_ButEstablishesChain() + { + // Finalize path: children must stay Idle (nothing auto-queues) but still + // get the blocked-by chain so a later "Queue plan" runs them in order. + await SeedPlanningFamilyAsync("P", 3); + + var count = await _sut.SetupChainAsync("P", enqueue: false, default); + + Assert.Equal(3, count); + var kids = await GetChildrenAsync("P"); + Assert.All(kids, k => Assert.Equal(TaskStatus.Idle, k.Status)); + Assert.Null(kids[0].BlockedByTaskId); + Assert.Equal(kids[0].Id, kids[1].BlockedByTaskId); + Assert.Equal(kids[1].Id, kids[2].BlockedByTaskId); + } } diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningEndToEndTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningEndToEndTests.cs index d99f5f6..eaad850 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningEndToEndTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningEndToEndTests.cs @@ -121,19 +121,20 @@ public sealed class PlanningEndToEndTests : IDisposable Assert.Equal(PlanningPhase.Finalized, reload!.PlanningPhase); var kids = await _tasks.GetChildrenAsync(parent.Id); - // SetupChainAsync auto-attaches agent tag and queues all children; - // the first one is unblocked, the rest are BlockedBy their predecessor. - Assert.Equal(TaskStatus.Queued, kids[0].Status); + // Finalize no longer auto-queues. Children stay Idle and chain-linked + // (head unblocked, rest BlockedBy their predecessor) until the user + // explicitly queues the plan. + Assert.Equal(TaskStatus.Idle, kids[0].Status); Assert.Null(kids[0].BlockedByTaskId); - Assert.Equal(TaskStatus.Queued, kids[1].Status); + Assert.Equal(TaskStatus.Idle, kids[1].Status); Assert.Equal(kids[0].Id, kids[1].BlockedByTaskId); } - // Regression: original bug was "queue never picks up planning tasks". After Finalize - // with queueAgentTasks=true, the first child must be claimable by the queue picker - // automatically — without anyone calling WakeQueue() manually. + // Regression: original bug was "queue never picks up planning tasks". Finalize + // leaves children Idle; queueing the plan (the explicit user gate) must make the + // first child claimable by the picker automatically — without a manual WakeQueue(). [Fact] - public async Task FinalizeAsync_FirstChildIsClaimedByPicker_WithinDeadline() + public async Task QueuePlanAfterFinalize_FirstChildIsClaimedByPicker_WithinDeadline() { var listId = Guid.NewGuid().ToString(); var wd = Path.Combine(Path.GetTempPath(), $"cd_e2e_wd_{Guid.NewGuid():N}"); @@ -160,12 +161,18 @@ public sealed class PlanningEndToEndTests : IDisposable var kidsBefore = await _tasks.GetChildrenAsync(parent.Id); var firstChildId = kidsBefore[0].Id; - var wakesBefore = _built.WakeCount(); await _manager.FinalizeAsync(parent.Id, queueAgentTasks: true, CancellationToken.None); - // The picker should pick the first child immediately. Auto-wake fires inside - // _state.EnqueueAsync; we don't need a manual WakeQueue() for the bug to be fixed. + // Finalize leaves every child Idle — nothing is claimable yet. + var afterFinalize = await _tasks.GetChildrenAsync(parent.Id); + Assert.All(afterFinalize, k => Assert.Equal(TaskStatus.Idle, k.Status)); + + // Queueing the plan is the gate that enqueues + auto-wakes. The picker should + // then pick the first child immediately, without a manual WakeQueue(). + var wakesBefore = _built.WakeCount(); + await _built.Chain.QueuePlanAsync(parent.Id, CancellationToken.None); + var picker = new QueuePicker(_db.CreateFactory()); TaskEntity? claimed = null; @@ -181,6 +188,6 @@ public sealed class PlanningEndToEndTests : IDisposable Assert.Equal(firstChildId, claimed!.Id); Assert.Equal(TaskStatus.Running, claimed.Status); Assert.True(_built.WakeCount() > wakesBefore, - "TaskStateService.EnqueueAsync should auto-wake the queue."); + "QueuePlanAsync → EnqueueAsync should auto-wake the queue."); } } diff --git a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs index 1444249..3b6dbe5 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs @@ -531,6 +531,8 @@ public class TaskMergeServiceTests : IDisposable Assert.Equal(TaskStatus.Done, updated!.Status); var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.Id); Assert.Equal(WorktreeState.Merged, wt!.State); + // Approve removes the worktree so merged worktrees don't pile up. + Assert.False(Directory.Exists(wtCtx.WorktreePath)); } [Fact]