feat(worker): report progress for cleanup_task_worktree, get_task_diff, preview_merge_set
Wraps the single-element git/worktree long-runners in ProgressReporter.RunAsync (time-based, ExternalMcpService.ProgressReportInterval) and adds i/n reporting to preview_merge_set's per-task loop, so these MCP calls survive Claude Code's ~300s idle-silence abort instead of leaving the caller with no signal that work is still happening.
This commit is contained in:
@@ -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