releases/ClaudeDo
2
0
Fork 0
Code Issues 1 Pull Requests 2 Actions Packages Projects Releases 12 Wiki Activity

feat(ui): single approve action merges the whole unit

Browse Source 
Approve & Merge is now the only review+merge entry. For a parent with
children it drives the unit merge via the worker (conflicts still surface
through the existing PlanningMergeConflict dialog); the separate Merge All
Subtasks button, MergeAllCommand, CanMergeAll plumbing, and the dead
MergeAllPlanningAsync client method are removed. Combined-diff preview and
conflict continue/abort are kept.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
mika kuns
2026-06-09 11:43:04 +02:00
parent 1abb429f12
commit a8b86e25e6
8 changed files with 9 additions and 159 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs
View File

@@ -59,7 +59,6 @@ public interface IWorkerClient : INotifyPropertyChanged
Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId);
Task<IReadOnlyList<SubtaskDiffDto>> GetPlanningAggregateAsync(string planningTaskId);
Task<CombinedDiffResultDto?> BuildPlanningIntegrationBranchAsync(string planningTaskId, string targetBranch);
Task MergeAllPlanningAsync(string planningTaskId, string targetBranch);
Task ContinuePlanningMergeAsync(string planningTaskId);
Task AbortPlanningMergeAsync(string planningTaskId);
Task QueuePlanningSubtasksAsync(string parentTaskId, CancellationToken ct = default);

13
src/ClaudeDo.Ui/Services/WorkerClient.cs
View File

@@ -66,7 +66,7 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC
public event Action<PrimeFiredEvent>? PrimeFired;
public string? LastMergeAllTarget { get; private set; }
public string? LastApproveTarget { get; private set; }
public WorkerClient(string signalRUrl)
{
@@ -412,7 +412,10 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC
}
public Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch)
=> TryInvokeAsync<MergeResultDto>("ApproveReview", taskId, targetBranch);
{
LastApproveTarget = targetBranch;
return TryInvokeAsync<MergeResultDto>("ApproveReview", taskId, targetBranch);
}
public Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch)
=> TryInvokeAsync<MergePreviewDto>("PreviewMerge", taskId, targetBranch);
@@ -486,12 +489,6 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC
public Task<CombinedDiffResultDto?> BuildPlanningIntegrationBranchAsync(string planningTaskId, string targetBranch)
=> TryInvokeAsync<CombinedDiffResultDto>("BuildPlanningIntegrationBranch", planningTaskId, targetBranch);
public async Task MergeAllPlanningAsync(string planningTaskId, string targetBranch)
{
LastMergeAllTarget = targetBranch;
await _hub.InvokeAsync("MergeAllPlanning", planningTaskId, targetBranch);
}
public async Task ContinuePlanningMergeAsync(string planningTaskId)
{
await _hub.InvokeAsync("ContinuePlanningMerge", planningTaskId);

74
src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs
View File

@@ -393,11 +393,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
// Planning merge controls
[ObservableProperty] private ObservableCollection<string> _mergeTargetBranches = new();
[ObservableProperty] private string? _selectedMergeTarget;
[ObservableProperty]
[NotifyCanExecuteChangedFor(nameof(MergeAllCommand))]
private bool _canMergeAll;
[ObservableProperty] private string? _mergeAllDisabledReason;
[ObservableProperty] private string? _mergeAllError;
[ObservableProperty]
[NotifyPropertyChangedFor(nameof(ShowMergePreviewMuted))]
@@ -580,13 +575,11 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
Subtasks.CollectionChanged += (_, _) =>
{
RecomputeCanMergeAll();
ReviewCombinedDiffCommand.NotifyCanExecuteChanged();
};
ChildOutcomes.CollectionChanged += (_, _) =>
{
RecomputeCanMergeAll();
ReviewCombinedDiffCommand.NotifyCanExecuteChanged();
NotifySessionSections();
};
@@ -836,9 +829,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
OnPropertyChanged(nameof(HasChildOutcomes));
MergeTargetBranches.Clear();
SelectedMergeTarget = null;
CanMergeAll = false;
MergeAllDisabledReason = null;
MergeAllError = null;
SessionOutcome = null;
Roadblocks = null;
_claudeBuf.Clear();
@@ -993,7 +983,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
}
}
RecomputeCanMergeAll();
}
catch (OperationCanceledException) { }
catch { /* best-effort */ }
@@ -1090,7 +1079,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
}
}
RecomputeCanMergeAll();
}
catch (OperationCanceledException) { }
catch { /* best-effort */ }
@@ -1115,7 +1103,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
existing.WorktreeState = child.Worktree?.State ?? ClaudeDo.Data.Models.WorktreeState.Active;
}
RecomputeCanMergeAll();
}
catch { /* best-effort */ }
}
@@ -1137,54 +1124,11 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
row.Status = child.Status;
row.RoadblockCount = child.RoadblockCount;
row.WorktreeState = child.Worktree?.State ?? ClaudeDo.Data.Models.WorktreeState.Active;
RecomputeCanMergeAll();
MergeAllCommand.NotifyCanExecuteChanged();
ReviewCombinedDiffCommand.NotifyCanExecuteChanged();
}
catch { /* best-effort */ }
}
internal void RecomputeCanMergeAll()
{
// Improvement parent: merge is allowed once every child is terminal. The
// orchestrator folds the parent's own branch and skips failed/cancelled children.
if (ChildOutcomes.Count > 0)
{
var unfinished = ChildOutcomes.Count(c =>
c.Status != ClaudeDo.Data.Models.TaskStatus.Done
&& c.Status != ClaudeDo.Data.Models.TaskStatus.Failed
&& c.Status != ClaudeDo.Data.Models.TaskStatus.Cancelled);
if (unfinished > 0)
{
CanMergeAll = false;
MergeAllDisabledReason = $"{unfinished} improvement(s) not finished";
return;
}
CanMergeAll = true;
MergeAllDisabledReason = null;
return;
}
var notDone = Subtasks.Count(c => c.Status != ClaudeDo.Data.Models.TaskStatus.Done);
if (notDone > 0)
{
CanMergeAll = false;
MergeAllDisabledReason = $"{notDone} subtask(s) not done";
return;
}
var badWt = Subtasks.FirstOrDefault(c =>
c.WorktreeState == ClaudeDo.Data.Models.WorktreeState.Discarded ||
c.WorktreeState == ClaudeDo.Data.Models.WorktreeState.Kept);
if (badWt is not null)
{
CanMergeAll = false;
MergeAllDisabledReason = "at least one worktree was discarded/kept";
return;
}
CanMergeAll = true;
MergeAllDisabledReason = null;
}
[RelayCommand(CanExecute = nameof(CanReviewDiff))]
private async System.Threading.Tasks.Task ReviewCombinedDiffAsync()
{
@@ -1196,20 +1140,6 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
private bool CanReviewDiff() => (Task?.IsPlanningParent == true && Subtasks.Any()) || HasChildOutcomes;
[RelayCommand(CanExecute = nameof(CanMergeAll))]
private async System.Threading.Tasks.Task MergeAllAsync()
{
MergeAllError = null;
try
{
await _worker.MergeAllPlanningAsync(Task!.Id, SelectedMergeTarget ?? "main");
}
catch (Exception ex)
{
MergeAllError = ex.Message;
}
}
private async System.Threading.Tasks.Task RefreshWorktreeAsync(string taskId)
{
try
@@ -1526,8 +1456,9 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
if (Task is null || !_worker.IsConnected) return;
try
{
var hasChildren = Subtasks.Count > 0 || ChildOutcomes.Count > 0;
var result = await _worker.ApproveReviewAsync(Task.Id, SelectedMergeTarget ?? "");
if (result?.Status == "conflict")
if (!hasChildren && result?.Status == "conflict")
{
if (RequestConflictResolution is not null)
{
@@ -1540,6 +1471,7 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
MergePreviewText = text; MergeIsClean = false; MergeIsConflict = true;
}
}
// hasChildren: conflicts arrive via PlanningMergeConflictEvent → conflict dialog
}
catch { /* stale review action; broadcast reconciles */ }
}

2
src/ClaudeDo.Ui/ViewModels/IslandsShellViewModel.cs
View File

@@ -155,7 +155,7 @@ public sealed partial class IslandsShellViewModel : ViewModelBase, IDisposable
string subtaskTitle = subtaskId;
string worktreePath = System.Environment.CurrentDirectory;
string targetBranch = Worker?.LastMergeAllTarget ?? "main";
string targetBranch = Worker?.LastApproveTarget ?? "main";
try
{

10
src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml
View File

@@ -305,17 +305,7 @@
</Button>
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
Command="{Binding ReviewCombinedDiffCommand}" />
<Button Classes="btn accent" Content="Merge All Subtasks" Margin="0,0,0,8"
Command="{Binding MergeAllCommand}"
IsEnabled="{Binding CanMergeAll}"
ToolTip.Tip="{Binding MergeAllDisabledReason}" />
</WrapPanel>
<TextBlock Text="{Binding MergeAllError}"
Foreground="{DynamicResource BloodBrush}"
TextWrapping="Wrap"
IsVisible="{Binding MergeAllError,
Converter={x:Static ObjectConverters.IsNotNull}}" />
</StackPanel>
</ScrollViewer>

1
tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs
View File

@@ -75,7 +75,6 @@ public abstract class StubWorkerClient : IWorkerClient
=> Task.FromResult<IReadOnlyList<SubtaskDiffDto>>(Array.Empty<SubtaskDiffDto>());
public virtual Task<CombinedDiffResultDto?> BuildPlanningIntegrationBranchAsync(string planningTaskId, string targetBranch)
=> Task.FromResult<CombinedDiffResultDto?>(null);
public virtual Task MergeAllPlanningAsync(string planningTaskId, string targetBranch) => Task.CompletedTask;
public virtual Task ContinuePlanningMergeAsync(string planningTaskId) => Task.CompletedTask;
public virtual Task AbortPlanningMergeAsync(string planningTaskId) => Task.CompletedTask;
public virtual Task QueuePlanningSubtasksAsync(string parentTaskId, CancellationToken ct = default) => Task.CompletedTask;

66
tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs
View File

@@ -78,72 +78,6 @@ public class DetailsIslandPlanningTests : IDisposable
Task.FromException<MergeResultDto?>(new InvalidOperationException("Task is not waiting for review; cannot approve."));
}
private static SubtaskRowViewModel MakeSubtask(TaskStatus status, WorktreeState wt = WorktreeState.Active) =>
new() { Id = Guid.NewGuid().ToString(), Title = "t", Status = status, WorktreeState = wt };
// ── CanMergeAll tests exercising the real VM ─────────────────────────────
[Fact]
public void CanMergeAll_AllChildrenDoneActiveWorktrees_True()
{
var vm = BuildVm(new FakeWorkerClient());
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.RecomputeCanMergeAll();
Assert.True(vm.CanMergeAll);
Assert.Null(vm.MergeAllDisabledReason);
}
[Fact]
public void CanMergeAll_AnyChildNotDone_FalseWithReason()
{
var vm = BuildVm(new FakeWorkerClient());
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.Subtasks.Add(MakeSubtask(TaskStatus.Running, WorktreeState.Active));
vm.RecomputeCanMergeAll();
Assert.False(vm.CanMergeAll);
Assert.NotNull(vm.MergeAllDisabledReason);
Assert.Contains("1 subtask", vm.MergeAllDisabledReason, StringComparison.OrdinalIgnoreCase);
Assert.Contains("not done", vm.MergeAllDisabledReason, StringComparison.OrdinalIgnoreCase);
}
[Fact]
public void CanMergeAll_AnyChildDiscarded_FalseWithReason()
{
var vm = BuildVm(new FakeWorkerClient());
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Discarded));
vm.RecomputeCanMergeAll();
Assert.False(vm.CanMergeAll);
Assert.NotNull(vm.MergeAllDisabledReason);
Assert.True(
vm.MergeAllDisabledReason!.Contains("discarded", StringComparison.OrdinalIgnoreCase) ||
vm.MergeAllDisabledReason.Contains("kept", StringComparison.OrdinalIgnoreCase));
}
[Fact]
public void CanMergeAll_AnyChildKept_FalseWithReason()
{
var vm = BuildVm(new FakeWorkerClient());
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Active));
vm.Subtasks.Add(MakeSubtask(TaskStatus.Done, WorktreeState.Kept));
vm.RecomputeCanMergeAll();
Assert.False(vm.CanMergeAll);
Assert.NotNull(vm.MergeAllDisabledReason);
Assert.True(
vm.MergeAllDisabledReason!.Contains("kept", StringComparison.OrdinalIgnoreCase) ||
vm.MergeAllDisabledReason.Contains("discarded", StringComparison.OrdinalIgnoreCase));
}
// ── Review-action resilience: a failing hub call must not crash the app ───
[Fact]

1
tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs
View File

@@ -81,7 +81,6 @@ sealed class FakeWorkerClient : IWorkerClient
public Task<MergeTargetsDto?> GetMergeTargetsAsync(string taskId) => Task.FromResult<MergeTargetsDto?>(null);
public Task<IReadOnlyList<SubtaskDiffDto>> GetPlanningAggregateAsync(string planningTaskId) => Task.FromResult<IReadOnlyList<SubtaskDiffDto>>(Array.Empty<SubtaskDiffDto>());
public Task<CombinedDiffResultDto?> BuildPlanningIntegrationBranchAsync(string planningTaskId, string targetBranch) => Task.FromResult<CombinedDiffResultDto?>(null);
public Task MergeAllPlanningAsync(string planningTaskId, string targetBranch) => Task.CompletedTask;
public Task ContinuePlanningMergeAsync(string planningTaskId) => Task.CompletedTask;
public Task AbortPlanningMergeAsync(string planningTaskId) => Task.CompletedTask;