feat(claude-do): merge [D3] Worktree- und Diff-MCP-Tools mit Progress (3 Tools in E
ClaudeDo-Task: cc93dd2d-f870-4820-9f69-5388372ee73b
This commit is contained in:
+25
-8
@@ -184,6 +184,12 @@ public sealed record DailyPrepDataDto(
|
||||
[McpServerToolType]
|
||||
public sealed class ExternalMcpService
|
||||
{
|
||||
// Mirrors TaskMergeService.ProgressReportInterval / TaskWaitMcpTools.ProgressReportInterval:
|
||||
// a separate field (not shared) so shrinking one for a test can't race another's tests. Used
|
||||
// by the single-element long-runners here (GetTaskDiff, CleanupTaskWorktree) that wrap a bare
|
||||
// GitService/WorktreeMaintenanceService call with no natural i/n to report instead.
|
||||
internal static TimeSpan ProgressReportInterval = TimeSpan.FromSeconds(30);
|
||||
|
||||
private readonly TaskRepository _tasks;
|
||||
private readonly ListRepository _lists;
|
||||
private readonly QueueService _queue;
|
||||
@@ -897,7 +903,8 @@ public sealed class ExternalMcpService
|
||||
"conflict report already narrowed down which files matter. Omit/empty for every changed file " +
|
||||
"— the default and the only prior behavior. Works in both stat and full-diff mode.")]
|
||||
IReadOnlyList<string>? paths = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
var (repoPath, baseCommit, headCommit) = await LoadDiffRangeAsync(taskId, cancellationToken);
|
||||
@@ -906,13 +913,17 @@ public sealed class ExternalMcpService
|
||||
|
||||
if (stat)
|
||||
{
|
||||
var diffStat = await _git.DiffStatAsync(repoPath, baseCommit, headCommit ?? "HEAD", paths, cancellationToken);
|
||||
var diffStat = await ProgressReporter.RunAsync(
|
||||
_git.DiffStatAsync(repoPath, baseCommit, headCommit ?? "HEAD", paths, cancellationToken),
|
||||
ProgressReportInterval, progress, "computing diff stat");
|
||||
return new TaskDiffDto(diffStat, ParseDiffStatFileNames(diffStat), false, diffStat.Length);
|
||||
}
|
||||
|
||||
var diff = headCommit is null
|
||||
? await _git.GetBranchDiffAsync(repoPath, baseCommit, paths, cancellationToken)
|
||||
: await _git.GetCommitRangeDiffAsync(repoPath, baseCommit, headCommit, paths, cancellationToken);
|
||||
? await ProgressReporter.RunAsync(
|
||||
_git.GetBranchDiffAsync(repoPath, baseCommit, paths, cancellationToken), ProgressReportInterval, progress, "computing diff")
|
||||
: await ProgressReporter.RunAsync(
|
||||
_git.GetCommitRangeDiffAsync(repoPath, baseCommit, headCommit, paths, cancellationToken), ProgressReportInterval, progress, "computing diff");
|
||||
var files = ParseDiffFileNames(diff);
|
||||
|
||||
if (diff.Length <= maxBytes)
|
||||
@@ -1283,8 +1294,9 @@ public sealed class ExternalMcpService
|
||||
var filesByTask = new Dictionary<string, IReadOnlyList<string>>();
|
||||
var numbersByTask = new Dictionary<string, int>();
|
||||
|
||||
foreach (var taskId in taskIds)
|
||||
for (var i = 0; i < taskIds.Count; i++)
|
||||
{
|
||||
var taskId = taskIds[i];
|
||||
try
|
||||
{
|
||||
var (preview, behind, changedFiles, isEmpty, staleFiles, number) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, cancellationToken, progress);
|
||||
@@ -1301,6 +1313,8 @@ public sealed class ExternalMcpService
|
||||
taskId, TaskMergeService.PreviewUnavailable, Array.Empty<string>(), 0, 0, ex.Message,
|
||||
Number: task?.Number));
|
||||
}
|
||||
|
||||
ProgressReporter.ReportItem(progress, "Previewing merges", i + 1, taskIds.Count);
|
||||
}
|
||||
|
||||
var overlaps = filesByTask
|
||||
@@ -1475,7 +1489,8 @@ public sealed class ExternalMcpService
|
||||
[Description("false (default): refuse a worktree with uncommitted changes. true: remove it anyway, losing " +
|
||||
"those changes.")]
|
||||
bool force = false,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
|
||||
@@ -1490,14 +1505,16 @@ public sealed class ExternalMcpService
|
||||
|
||||
if (!force && Directory.Exists(wt.Path))
|
||||
{
|
||||
var isDirty = await _git.HasChangesAsync(wt.Path, cancellationToken);
|
||||
var isDirty = await ProgressReporter.RunAsync(
|
||||
_git.HasChangesAsync(wt.Path, cancellationToken), ProgressReportInterval, progress, "checking worktree for changes");
|
||||
if (isDirty)
|
||||
throw new InvalidOperationException(
|
||||
"Worktree has uncommitted changes. Use force=true to remove anyway (changes will be lost).");
|
||||
}
|
||||
|
||||
var path = wt.Path;
|
||||
var result = await _maintenance.ForceRemoveAsync(taskId, cancellationToken);
|
||||
var result = await ProgressReporter.RunAsync(
|
||||
_maintenance.ForceRemoveAsync(taskId, cancellationToken), ProgressReportInterval, progress, "removing worktree");
|
||||
return new CleanupWorktreeResult(result.Removed, path, result.BranchDeleted, task.Number);
|
||||
}
|
||||
|
||||
|
||||
@@ -52,6 +52,14 @@ internal sealed class ExternalFakeHubContext : IHubContext<WorkerHub>
|
||||
public IGroupManager Groups => throw new NotImplementedException();
|
||||
}
|
||||
|
||||
// A synchronously-collecting IProgress<T>: Progress<T> marshals through the SynchronizationContext
|
||||
// captured at construction, which is unreliable to assert on immediately in a test.
|
||||
file sealed class SyncProgress<T> : IProgress<T>
|
||||
{
|
||||
public readonly List<T> Reports = new();
|
||||
public void Report(T value) => Reports.Add(value);
|
||||
}
|
||||
|
||||
public sealed class ExternalMcpServiceTests : IDisposable
|
||||
{
|
||||
private readonly DbFixture _db = new();
|
||||
@@ -1220,6 +1228,41 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Contains("handled.txt", diff.Content);
|
||||
}
|
||||
|
||||
// get_task_diff has no natural i/n (it's one git process, not a loop over items), so it
|
||||
// reports on a timer like merge_task's verify gate. Shrinking the interval to zero forces a
|
||||
// report on every poll of the still-running git process instead of waiting out a real
|
||||
// interval -- Task.Delay(TimeSpan.Zero) resolves synchronously, so the diff process (a real
|
||||
// subprocess, never already complete when polled) always loses the race at least once.
|
||||
[Fact]
|
||||
public async Task GetTaskDiff_ReportsProgressWhileDiffing()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var original = ExternalMcpService.ProgressReportInterval;
|
||||
ExternalMcpService.ProgressReportInterval = TimeSpan.Zero;
|
||||
try
|
||||
{
|
||||
var (task, list, wt) = await SeedWorktreeAsync();
|
||||
File.WriteAllText(Path.Combine(wt.WorktreePath, "added.txt"), "content");
|
||||
var cfg = new WorkerConfig { WorktreeRootStrategy = "sibling" };
|
||||
var mgr = new WorktreeManager(new GitService(), _db.CreateFactory(), cfg, NullLogger<WorktreeManager>.Instance);
|
||||
await mgr.CommitIfChangedAsync(wt, task, list, CancellationToken.None);
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var progress = new SyncProgress<ProgressNotificationValue>();
|
||||
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, null, CancellationToken.None, progress);
|
||||
|
||||
Assert.Contains("added.txt", diff.Files);
|
||||
Assert.NotEmpty(progress.Reports);
|
||||
Assert.All(progress.Reports, r => Assert.Contains("computing diff", r.Message));
|
||||
}
|
||||
finally
|
||||
{
|
||||
ExternalMcpService.ProgressReportInterval = original;
|
||||
}
|
||||
}
|
||||
|
||||
// ── MergeTask ──────────────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
@@ -1311,6 +1354,33 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.False(Directory.Exists(wt.WorktreePath));
|
||||
}
|
||||
|
||||
// cleanup_task_worktree has no natural i/n either (dirty check + one removal, not a loop), so
|
||||
// it reports on the same timer/zero-interval technique as GetTaskDiff above.
|
||||
[Fact]
|
||||
public async Task CleanupTaskWorktree_ReportsProgressWhileRemoving()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var original = ExternalMcpService.ProgressReportInterval;
|
||||
ExternalMcpService.ProgressReportInterval = TimeSpan.Zero;
|
||||
try
|
||||
{
|
||||
var (task, _, wt) = await SeedWorktreeAsync(TaskStatus.Done);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var progress = new SyncProgress<ProgressNotificationValue>();
|
||||
|
||||
var result = await sut.CleanupTaskWorktree(task.Id, false, CancellationToken.None, progress);
|
||||
|
||||
Assert.True(result.Removed);
|
||||
Assert.False(Directory.Exists(wt.WorktreePath));
|
||||
Assert.NotEmpty(progress.Reports);
|
||||
}
|
||||
finally
|
||||
{
|
||||
ExternalMcpService.ProgressReportInterval = original;
|
||||
}
|
||||
}
|
||||
|
||||
// ── GetTaskConfig ─────────────────────────────────────────────────────────
|
||||
|
||||
private ConfigMcpTools BuildConfigSut() => new(_lists, _tasks, _broadcaster, _db.CreateFactory());
|
||||
@@ -2350,6 +2420,27 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Empty(result.Overlaps);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMergeSet_ReportsProgressPerTask()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var t1 = await SeedTaskAsync(listId, "a");
|
||||
var t2 = await SeedTaskAsync(listId, "b");
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var progress = new SyncProgress<ProgressNotificationValue>();
|
||||
|
||||
// Neither task has a worktree -- each entry fails individually (PreviewUnavailable), but
|
||||
// the loop still reports progress per task since a per-task InvalidOperationException
|
||||
// doesn't abort the batch.
|
||||
await sut.PreviewMergeSet(
|
||||
[t1.Id, t2.Id], "main", cancellationToken: CancellationToken.None, progress: progress);
|
||||
|
||||
Assert.Equal(2, progress.Reports.Count);
|
||||
Assert.Equal("Previewing merges (1/2)", progress.Reports[0].Message);
|
||||
Assert.Equal("Previewing merges (2/2)", progress.Reports[1].Message);
|
||||
Assert.Equal(2, progress.Reports[1].Total);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMergeSet_OneTaskWithoutWorktree_ReportsErrorWithoutAbortingOthers()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user