A child merge that came back blocked/verify_failed/untracked_collision during a parent/children unit merge used to vanish: DrainAsync only logged it server-side, PlanningMergeAborted carried no reason, and ApproveReview/review_task always reported StatusMerged for a task with children regardless of the real outcome, so a failed unit merge left the parent stuck with no visible error. - PlanningMergeOrchestrator.StartAsync/ContinueAsync/DrainAsync now return a PlanningMergeResult(Status, Reason) instead of void, and PlanningMergeAborted carries that reason to the UI. - WorkerHub.ApproveReview and ExternalMcpService.ReviewTask's approve branch propagate the real status/reason for a parent with children instead of hardcoding "merged" (or masking a non-conflict failure as "conflict"). - StartAsync now requires the parent to already be WaitingForReview for improvement parents too, not just planning ones, so a stale caller can no longer trigger a partial child merge. - HasActiveMerge now also covers the window between the last child merging and FinalizeParentDoneAsync completing, closing a gap where a concurrent Cancel could race the parent's own approve-to-Done transition. - IslandsShellViewModel.OnPlanningMergeAborted flashes the reason via FlashFooterError instead of only clearing the external-merge banner.
128 lines
4.4 KiB
C#
128 lines
4.4 KiB
C#
using System;
|
|
using System.Threading.Tasks;
|
|
using ClaudeDo.Data.Models;
|
|
using ClaudeDo.Ui.ViewModels;
|
|
using Xunit;
|
|
|
|
namespace ClaudeDo.Ui.Tests;
|
|
|
|
// Covers the MCP-driven-merge popup fix: a unit-merge conflict started by an MCP session
|
|
// (review_task/continue_merge) must not auto-open the in-app resolver — two parties editing the
|
|
// same shared checkout at once is how the original bug manifested. The UI-driven path (a human
|
|
// clicking Approve) keeps opening the resolver as before.
|
|
public class IslandsShellViewModelExternalMergeTests
|
|
{
|
|
[Fact]
|
|
public void ExternallyDriven_ShowsBanner()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", new[] { "file.txt" }, externallyDriven: true);
|
|
|
|
Assert.True(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void UiDriven_DoesNotShowExternalBanner()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", new[] { "file.txt" }, externallyDriven: false);
|
|
|
|
Assert.False(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void Aborted_ClearsBanner()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", Array.Empty<string>(), externallyDriven: true);
|
|
Assert.True(vm.IsExternalMergeBannerVisible);
|
|
|
|
vm.OnPlanningMergeAborted("plan1");
|
|
|
|
Assert.False(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
// Covers the unit-merge-error-propagation fix: a real merge failure (blocked/verify_failed/
|
|
// untracked_collision) reaches the UI as a reason on PlanningMergeAborted, and must surface
|
|
// in the footer strip rather than silently vanishing once the banner is cleared.
|
|
[Fact]
|
|
public void Aborted_WithReason_FlashesFooterError()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
|
|
vm.OnPlanningMergeAborted("plan1", "child merge blocked: unrelated histories");
|
|
|
|
Assert.Equal(WorkerLogLevel.Error, vm.WorkerLogLevel);
|
|
Assert.Contains("child merge blocked: unrelated histories", vm.WorkerLogText);
|
|
Assert.True(vm.IsWorkerLogVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void Aborted_WithoutReason_DoesNotFlashFooterError()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
|
|
vm.OnPlanningMergeAborted("plan1", null);
|
|
|
|
Assert.False(vm.IsWorkerLogVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void Completed_ClearsBanner()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", Array.Empty<string>(), externallyDriven: true);
|
|
|
|
vm.OnPlanningMergeCompleted("plan1");
|
|
|
|
Assert.False(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void UnrelatedPlanningTaskEnding_LeavesBannerVisible()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", Array.Empty<string>(), externallyDriven: true);
|
|
|
|
vm.OnPlanningMergeCompleted("some-other-plan");
|
|
|
|
Assert.True(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public void LaterConflictOnSamePlanningTask_UpdatesInPlaceWithoutDuplicating()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
vm.OnPlanningMergeConflict("plan1", "subA", Array.Empty<string>(), externallyDriven: true);
|
|
vm.OnPlanningMergeConflict("plan1", "subB", Array.Empty<string>(), externallyDriven: true);
|
|
Assert.True(vm.IsExternalMergeBannerVisible);
|
|
|
|
// A single terminal event for that planning task clears it entirely — proves the second
|
|
// conflict updated the same entry rather than piling up a second one.
|
|
vm.OnPlanningMergeCompleted("plan1");
|
|
|
|
Assert.False(vm.IsExternalMergeBannerVisible);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task OpenExternalMergeConflictCommand_NoPendingConflict_DoesNotThrow()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
|
|
await vm.OpenExternalMergeConflictCommand.ExecuteAsync(null);
|
|
}
|
|
|
|
[Fact]
|
|
public async Task OpenExternalMergeConflictCommand_NoDialogsWired_DoesNotThrow()
|
|
{
|
|
var vm = new IslandsShellViewModel();
|
|
vm.OnPlanningMergeConflict("plan1", "sub1", Array.Empty<string>(), externallyDriven: true);
|
|
|
|
// Dialogs/ConflictResolverFactory aren't wired in the test-only ctor — a deliberate
|
|
// click must degrade gracefully rather than throw, same as the existing UI-driven path.
|
|
await vm.OpenExternalMergeConflictCommand.ExecuteAsync(null);
|
|
}
|
|
}
|