diff --git a/src/ClaudeDo.Worker/Lifecycle/ProgressReporter.cs b/src/ClaudeDo.Worker/Lifecycle/ProgressReporter.cs new file mode 100644 index 00000000..45962e49 --- /dev/null +++ b/src/ClaudeDo.Worker/Lifecycle/ProgressReporter.cs @@ -0,0 +1,47 @@ +using ModelContextProtocol; + +namespace ClaudeDo.Worker.Lifecycle; + +/// +/// Reports MCP/hub progress for a long-running operation so a waiting caller doesn't hit an +/// idle-silence abort (Claude Code's MCP client aborts an MCP call after ~300s of silence). +/// +internal static class ProgressReporter +{ + /// + /// Awaits while reporting progress every . + /// rides the same cadence for non-MCP callers (e.g. the Hub, which + /// turns it into an OperationProgress broadcast). No-op passthrough when both are null. + /// + public static async Task RunAsync( + Task work, TimeSpan interval, IProgress? progress, string message, + Action? onTick = null) + { + if (progress is null && onTick is null) return await work; + + var sw = System.Diagnostics.Stopwatch.StartNew(); + while (true) + { + var finished = await Task.WhenAny(work, Task.Delay(interval)); + if (finished == work) return await work; + progress?.Report(new ProgressNotificationValue { Progress = 0, Message = $"{message} ({sw.Elapsed:mm\\:ss})" }); + onTick?.Invoke(sw.Elapsed); + } + } + + /// + /// Reports progress for one processed item out of a known total, e.g. "cleaning worktrees + /// (3/12)". For callers iterating a batch (cleanup_task_worktree, get_task_diff, + /// preview_merge_set) instead of awaiting one long-running task. + /// + public static void ReportItem( + IProgress? progress, string action, int current, int total) + { + progress?.Report(new ProgressNotificationValue + { + Progress = current, + Total = total, + Message = $"{action} ({current}/{total})", + }); + } +} diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index b29ca7cc..1c273d5b 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -77,7 +77,7 @@ public sealed class TaskMergeService // Mirrors TaskWaitMcpTools.ProgressReportInterval (External/TaskWaitMcpTools.cs): a verify // run can take up to VerifyTimeout, well past Claude Code's ~300s MCP idle-silence abort, so - // RunReportingProgressAsync below reports on this cadence to keep the calling review_task/ + // ProgressReporter.RunAsync reports on this cadence to keep the calling review_task/ // merge_task/preview_merge* call alive. Not readonly -- tests shrink it to observe a report // without waiting 30s. A separate field from TaskWaitMcpTools' own (rather than sharing it) // so shrinking one for a test can't race the other's tests. @@ -146,8 +146,8 @@ public sealed class TaskMergeService VerifyCommandResult result; try { - result = await RunReportingProgressAsync( - _verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct), progress, "verify gate running", onTick); + result = await ProgressReporter.RunAsync( + _verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct), ProgressReportInterval, progress, "verify gate running", onTick); } catch (Exception ex) { @@ -164,29 +164,6 @@ public sealed class TaskMergeService return new MergeResult(StatusVerifyFailed, Array.Empty(), $"{reason}\n{TailOutput(result.Output)}"); } - /// - /// Awaits while reporting MCP progress every - /// so a caller waiting on a long verify run doesn't hit - /// the MCP client's own idle-silence abort. rides the same cadence - /// for non-MCP callers (the Hub, which turns it into an OperationProgress broadcast). No-op - /// passthrough when both are null. - /// - private static async Task RunReportingProgressAsync( - Task work, IProgress? progress, string message, - Action? onTick = null) - { - if (progress is null && onTick is null) return await work; - - var sw = System.Diagnostics.Stopwatch.StartNew(); - while (true) - { - var finished = await Task.WhenAny(work, Task.Delay(ProgressReportInterval)); - if (finished == work) return await work; - progress?.Report(new ProgressNotificationValue { Progress = 0, Message = $"{message} ({sw.Elapsed:mm\\:ss})" }); - onTick?.Invoke(sw.Elapsed); - } - } - private static string TailOutput(string output, int maxChars = 4000) { var trimmed = output.Trim(); @@ -848,8 +825,8 @@ public sealed class TaskMergeService VerifyCommandResult result; try { - result = await RunReportingProgressAsync( - _verify.RunAsync(scratchPath, verifyCommand, VerifyTimeout, ct), progress, "merge preview verify running"); + result = await ProgressReporter.RunAsync( + _verify.RunAsync(scratchPath, verifyCommand, VerifyTimeout, ct), ProgressReportInterval, progress, "merge preview verify running"); } catch (Exception ex) { diff --git a/tests/ClaudeDo.Worker.Tests/Lifecycle/ProgressReporterTests.cs b/tests/ClaudeDo.Worker.Tests/Lifecycle/ProgressReporterTests.cs new file mode 100644 index 00000000..130bf132 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Lifecycle/ProgressReporterTests.cs @@ -0,0 +1,58 @@ +using ClaudeDo.Worker.Lifecycle; +using ModelContextProtocol; +using Xunit; + +namespace ClaudeDo.Worker.Tests.Lifecycle; + +// A synchronously-collecting IProgress: Progress marshals through the SynchronizationContext +// captured at construction, which is unreliable to assert on immediately in a test. +file sealed class SyncProgress : IProgress +{ + public readonly List Reports = new(); + public void Report(T value) => Reports.Add(value); +} + +public sealed class ProgressReporterTests +{ + [Fact] + public async Task RunAsync_NoProgressOrOnTick_ReturnsWorkResultWithoutTicking() + { + var result = await ProgressReporter.RunAsync(Task.FromResult(42), TimeSpan.FromMilliseconds(10), null, "message"); + + Assert.Equal(42, result); + } + + [Fact] + public async Task RunAsync_SlowWork_ReportsProgressOnEachTick() + { + var progress = new SyncProgress(); + var work = Task.Delay(250).ContinueWith(_ => "done"); + + var result = await ProgressReporter.RunAsync(work, TimeSpan.FromMilliseconds(50), progress, "waiting"); + + Assert.Equal("done", result); + Assert.NotEmpty(progress.Reports); + Assert.All(progress.Reports, r => Assert.Contains("waiting", r.Message)); + } + + [Fact] + public void ReportItem_ReportsMessageAndProgressTotalForEachItem() + { + var progress = new SyncProgress(); + + for (var i = 1; i <= 3; i++) + ProgressReporter.ReportItem(progress, "cleaning worktrees", i, 3); + + Assert.Equal(3, progress.Reports.Count); + Assert.Equal("cleaning worktrees (1/3)", progress.Reports[0].Message); + Assert.Equal("cleaning worktrees (2/3)", progress.Reports[1].Message); + Assert.Equal("cleaning worktrees (3/3)", progress.Reports[2].Message); + Assert.Equal(3, progress.Reports[2].Total); + } + + [Fact] + public void ReportItem_NoProgress_DoesNotThrow() + { + ProgressReporter.ReportItem(null, "cleaning worktrees", 1, 3); + } +}