feat(ui): richer diff viewer + surface child roadblocks on parents
- UnifiedDiffParser detects added/deleted/renamed/binary files; diff modal shows a file list, binary/empty placeholders, and can diff a merged task by commit range after its worktree is gone - DetailsIslandViewModel flags children needing attention (failed, cancelled, awaiting review, or with roadblocks) on the parent - GitService gains worktree head-commit/range support; planning chain, merge orchestration, and session manager tweaks with updated tests - refresh app/installer/worker icons Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
109
tests/ClaudeDo.Ui.Tests/ViewModels/UnifiedDiffParserTests.cs
Normal file
109
tests/ClaudeDo.Ui.Tests/ViewModels/UnifiedDiffParserTests.cs
Normal file
@@ -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);
|
||||
}
|
||||
}
|
||||
@@ -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<InvalidOperationException>(
|
||||
() => _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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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.");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user