From 53e26f0340664900fd733cfc301d29815c71d3d5 Mon Sep 17 00:00:00 2001 From: Mika Kuns Date: Mon, 17 Aug 2026 08:56:32 +0200 Subject: [PATCH] feat(worker): report per-item progress on all 7 batch_* MCP tools Each batch tool now sends an i/n progress ping via ProgressReporter.ReportItem after processing every item, so a waiting agent doesn't see the MCP client's 300s idle-silence abort while the worker keeps looping. Covers BatchGetTasks, BatchAddTasks, BatchUpdateTaskStatus, BatchCancelTasks, BatchDeleteTasks, BatchSetMyDay, and BatchCleanupTaskWorktrees (the slowest of the seven, since it does git work per task). --- src/ClaudeDo.Worker/External/BatchMcpTools.cs | 67 ++++++---- src/ClaudeDo.Worker/External/McpToolDocs.cs | 9 ++ .../External/BatchMcpToolsTests.cs | 123 ++++++++++++++++++ 3 files changed, 177 insertions(+), 22 deletions(-) diff --git a/src/ClaudeDo.Worker/External/BatchMcpTools.cs b/src/ClaudeDo.Worker/External/BatchMcpTools.cs index 9392b4d9..4d864718 100644 --- a/src/ClaudeDo.Worker/External/BatchMcpTools.cs +++ b/src/ClaudeDo.Worker/External/BatchMcpTools.cs @@ -1,6 +1,8 @@ using System.ComponentModel; using System.Text.Json; using ClaudeDo.Worker.Git; +using ClaudeDo.Worker.Lifecycle; +using ModelContextProtocol; using ModelContextProtocol.Server; namespace ClaudeDo.Worker.External; @@ -92,7 +94,8 @@ public sealed class BatchMcpTools "descriptionMaxChars (default 1500 — was unlimited); check *Truncated/*FullLength before assuming " + "you got the whole text. Use `fields` to fetch only what you need (e.g. just roadblockText) instead " + "of raising the cap. The whole response is still capped — if it's too big even so, lower " + - "descriptionMaxChars, narrow `fields`, or split taskIds into a smaller batch." + McpToolDocs.MaxBatch)] + "descriptionMaxChars, narrow `fields`, or split taskIds into a smaller batch." + + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchGetTasks( string[] taskIds, [Description("If true, return the full task (incl. Description/Result) in `taskFull`; if false " + @@ -103,7 +106,8 @@ public sealed class BatchMcpTools [Description("Restrict `taskFull` to these field names (e.g. ['title','status','roadblockText']) " + "instead of returning all of them; unknown names are rejected. Ignored when " + "includeDescription=false.")] string[]? fields = null, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default, + IProgress? progress = null) { EnsureWithinCap(taskIds, nameof(taskIds)); if (descriptionMaxChars < 0) @@ -111,8 +115,9 @@ public sealed class BatchMcpTools ValidateFields(fields); var results = new List(taskIds.Length); - foreach (var id in taskIds) + for (var i = 0; i < taskIds.Length; i++) { + var id = taskIds[i]; try { if (includeDescription) @@ -135,6 +140,7 @@ public sealed class BatchMcpTools { results.Add(new BatchGetTaskResult(id, false, null, null, ex.Message)); } + ProgressReporter.ReportItem(progress, "Fetching tasks", i + 1, taskIds.Length); } var responseLength = JsonSerializer.Serialize(results).Length; @@ -204,13 +210,14 @@ public sealed class BatchMcpTools "Create many tasks in one list at once — use instead of repeated add_task calls when seeding a list. " + "Every item is still created even if it looks like a duplicate; possibleDuplicates is a non-blocking " + "heads-up (up to 3 similar open tasks in the list) worth mentioning to the caller, not an error." + - McpToolDocs.LeanTaskRef + McpToolDocs.MaxBatch)] + McpToolDocs.LeanTaskRef + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchAddTasks( string listId, BatchAddTaskInput[] tasks, string? createdBy = null, [Description("If true, enqueue every created task immediately instead of leaving it Idle.")] bool queueImmediately = false, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default, + IProgress? progress = null) { EnsureWithinCap(tasks, nameof(tasks)); @@ -230,6 +237,7 @@ public sealed class BatchMcpTools { results.Add(new BatchAddTaskResult(i, item.Title, false, null, null, ex.Message)); } + ProgressReporter.ReportItem(progress, "Adding tasks", i + 1, tasks.Length); } return results; } @@ -240,17 +248,19 @@ public sealed class BatchMcpTools "baseDirty on a 'Queued' item means that task's list has uncommitted changes in its working dir right " + "now — a new worktree forks from the last commit and won't include them; non-blocking, but worth " + "checking before assuming a fresh worktree starts from what's on disk." + - McpToolDocs.MaxBatch)] + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchUpdateTaskStatus( string[] taskIds, [Description("One of 'Idle', 'Queued', 'Cancelled', or 'Done'.")] string status, - CancellationToken cancellationToken) + CancellationToken cancellationToken, + IProgress? progress = null) { EnsureWithinCap(taskIds, nameof(taskIds)); var results = new List(taskIds.Length); - foreach (var id in taskIds) + for (var i = 0; i < taskIds.Length; i++) { + var id = taskIds[i]; try { var task = await _svc.UpdateTaskStatus(id, status, cancellationToken); @@ -261,21 +271,24 @@ public sealed class BatchMcpTools { results.Add(new BatchTaskResult(id, false, ex.Message)); } + ProgressReporter.ReportItem(progress, "Updating task status", i + 1, taskIds.Length); } return results; } [McpServerTool, Description( "Cancel many running tasks at once — use to bulk-stop tasks instead of calling cancel_task per id. " + - "ok=true with cancelled=false just means the task wasn't running." + McpToolDocs.MaxBatch)] + "ok=true with cancelled=false just means the task wasn't running." + + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchCancelTasks( - string[] taskIds, CancellationToken cancellationToken) + string[] taskIds, CancellationToken cancellationToken, IProgress? progress = null) { EnsureWithinCap(taskIds, nameof(taskIds)); var results = new List(taskIds.Length); - foreach (var id in taskIds) + for (var i = 0; i < taskIds.Length; i++) { + var id = taskIds[i]; try { var r = await _svc.CancelTask(id, cancellationToken); @@ -286,33 +299,35 @@ public sealed class BatchMcpTools { results.Add(new BatchCancelResult(id, false, false, ex.Message)); } + ProgressReporter.ReportItem(progress, "Cancelling tasks", i + 1, taskIds.Length); } return results; } [McpServerTool, Description( "Delete many tasks at once — use for bulk cleanup instead of calling delete_task per id." + - McpToolDocs.NotWhileRunning + McpToolDocs.MaxBatch)] + McpToolDocs.NotWhileRunning + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchDeleteTasks( - string[] taskIds, CancellationToken cancellationToken) + string[] taskIds, CancellationToken cancellationToken, IProgress? progress = null) { EnsureWithinCap(taskIds, nameof(taskIds)); return await RunPerTaskAsync(taskIds, - async (id, ct) => (await _svc.DeleteTask(id, ct)).Number, cancellationToken); + async (id, ct) => (await _svc.DeleteTask(id, ct)).Number, cancellationToken, progress, "Deleting tasks"); } [McpServerTool, Description( "Set or clear MyDay (daily prep) for many tasks at once — use instead of calling set_my_day per task. " + "Still cap-guarded: items that would exceed DailyPrepMaxTasks open MyDay tasks fail individually " + - "(ok=false) without blocking the rest." + McpToolDocs.MaxBatch)] + "(ok=false) without blocking the rest." + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchSetMyDay( - BatchSetMyDayInput[] items, CancellationToken cancellationToken) + BatchSetMyDayInput[] items, CancellationToken cancellationToken, IProgress? progress = null) { EnsureWithinCap(items, nameof(items)); var results = new List(items.Length); - foreach (var item in items) + for (var i = 0; i < items.Length; i++) { + var item = items[i]; try { var task = await _svc.SetMyDay(item.TaskId, item.IsMyDay, item.SortOrder, cancellationToken); @@ -323,24 +338,28 @@ public sealed class BatchMcpTools { results.Add(new BatchTaskResult(item.TaskId, false, ex.Message)); } + ProgressReporter.ReportItem(progress, "Setting My Day", i + 1, items.Length); } return results; } [McpServerTool, Description( "Remove the worktrees (directory + git branch) of many tasks at once — use for bulk cleanup instead " + - "of calling cleanup_task_worktree per id." + McpToolDocs.NotWhileRunning + McpToolDocs.MaxBatch)] + "of calling cleanup_task_worktree per id." + + McpToolDocs.NotWhileRunning + McpToolDocs.MaxBatch + McpToolDocs.BatchProgressHint)] public async Task> BatchCleanupTaskWorktrees( string[] taskIds, [Description("If true, also remove a dirty worktree, losing uncommitted changes; a Running task " + "is still refused either way.")] bool force = false, - CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default, + IProgress? progress = null) { EnsureWithinCap(taskIds, nameof(taskIds)); var results = new List(taskIds.Length); - foreach (var id in taskIds) + for (var i = 0; i < taskIds.Length; i++) { + var id = taskIds[i]; try { var r = await _svc.CleanupTaskWorktree(id, force, cancellationToken); @@ -351,16 +370,19 @@ public sealed class BatchMcpTools { results.Add(new BatchCleanupResult(id, false, false, false, ex.Message)); } + ProgressReporter.ReportItem(progress, "Cleaning up worktrees", i + 1, taskIds.Length); } return results; } private static async Task> RunPerTaskAsync( - string[] taskIds, Func> op, CancellationToken cancellationToken) + string[] taskIds, Func> op, CancellationToken cancellationToken, + IProgress? progress, string progressAction) { var results = new List(taskIds.Length); - foreach (var id in taskIds) + for (var i = 0; i < taskIds.Length; i++) { + var id = taskIds[i]; try { var number = await op(id, cancellationToken); @@ -371,6 +393,7 @@ public sealed class BatchMcpTools { results.Add(new BatchTaskResult(id, false, ex.Message)); } + ProgressReporter.ReportItem(progress, progressAction, i + 1, taskIds.Length); } return results; } diff --git a/src/ClaudeDo.Worker/External/McpToolDocs.cs b/src/ClaudeDo.Worker/External/McpToolDocs.cs index c0b029a9..30ce9ec3 100644 --- a/src/ClaudeDo.Worker/External/McpToolDocs.cs +++ b/src/ClaudeDo.Worker/External/McpToolDocs.cs @@ -56,4 +56,13 @@ internal static class McpToolDocs " Sends MCP progress pings periodically while a merge or verify gate runs, so a long call survives the " + "calling client's own idle-silence abort (Claude Code defaults to killing an MCP call after ~300s of " + "silence) -- this is not guaranteed by every possible MCP client."; + + /// + /// Batch tools loop per item and send one MCP progress ping per item (i/n) instead of only at + /// the end -- same idle-abort rationale as , worded for a batch loop. + /// + public const string BatchProgressHint = + " Sends an MCP progress ping after each item is processed (i/n), so a long batch survives the calling " + + "client's own idle-silence abort (Claude Code defaults to killing an MCP call after ~300s of silence) -- " + + "this is not guaranteed by every possible MCP client."; } diff --git a/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs index 66490135..2df57df6 100644 --- a/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs @@ -14,10 +14,19 @@ using ClaudeDo.Worker.Tests.Infrastructure; using ClaudeDo.Worker.Usage; using ClaudeDo.Worker.Worktrees; using Microsoft.Extensions.Logging.Abstractions; +using ModelContextProtocol; using TaskStatus = ClaudeDo.Data.Models.TaskStatus; namespace ClaudeDo.Worker.Tests.External; +// 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 BatchMcpToolsTests : IDisposable { private readonly DbFixture _db = new(); @@ -498,4 +507,118 @@ public sealed class BatchMcpToolsTests : IDisposable () => sut.BatchGetTasks(ids, cancellationToken: CancellationToken.None)); Assert.Contains("max", ex.Message, StringComparison.OrdinalIgnoreCase); } + + [Fact] + public async Task BatchGetTasks_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var a = await SeedTaskAsync(listId); + var b = await SeedTaskAsync(listId); + var c = await SeedTaskAsync(listId); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchGetTasks(new[] { a.Id, b.Id, c.Id }, cancellationToken: CancellationToken.None, progress: progress); + + Assert.Equal(3, progress.Reports.Count); + Assert.Equal(3, progress.Reports[2].Total); + } + + [Fact] + public async Task BatchAddTasks_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchAddTasks(listId, new[] + { + new BatchAddTaskInput("a"), + new BatchAddTaskInput("b"), + }, cancellationToken: CancellationToken.None, progress: progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } + + [Fact] + public async Task BatchUpdateTaskStatus_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle); + var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchUpdateTaskStatus(new[] { t1.Id, t2.Id }, "Queued", CancellationToken.None, progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } + + [Fact] + public async Task BatchCancelTasks_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle); + var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchCancelTasks(new[] { t1.Id, t2.Id }, CancellationToken.None, progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } + + [Fact] + public async Task BatchDeleteTasks_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle); + var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchDeleteTasks(new[] { t1.Id, t2.Id }, CancellationToken.None, progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } + + [Fact] + public async Task BatchSetMyDay_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle); + var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle); + var sut = BuildSut(); + var progress = new SyncProgress(); + + await sut.BatchSetMyDay(new[] + { + new BatchSetMyDayInput(t1.Id, true), + new BatchSetMyDayInput(t2.Id, true), + }, CancellationToken.None, progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } + + [Fact] + public async Task BatchCleanupTaskWorktrees_ReportsProgressPerItem() + { + var listId = await SeedListAsync(); + var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle); + var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle); + var sut = BuildSut(); + var progress = new SyncProgress(); + + // Neither task has a worktree -- each item fails individually (ok=false), but the batch + // still reports progress per item since the loop doesn't abort on a per-item error. + await sut.BatchCleanupTaskWorktrees(new[] { t1.Id, t2.Id }, cancellationToken: CancellationToken.None, progress: progress); + + Assert.Equal(2, progress.Reports.Count); + Assert.Equal(2, progress.Reports[1].Total); + } }