From ca6eb62b5cdb8783c4f7d1959ea7bfc298f0fa3d Mon Sep 17 00:00:00 2001 From: mika kuns Date: Mon, 10 Aug 2026 14:19:16 +0200 Subject: [PATCH] feat(mcp): truncate/select batch_get_tasks payload, expose roadblockText MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit batch_get_tasks with includeDescription=true had no size guard: 8 tasks' full Description/Result serialized to a single 51k-char line, blowing past the tool-result token budget and forcing a file/PowerShell workaround to read it back. Roadblock text was also only reachable via a second get_task call per task, since roadblockCount (in the lean ref) has no text sibling. - descriptionMaxChars (default 1500, was unlimited) truncates Description and Result independently, flagged via *Truncated/*FullLength so a caller never silently works off a cut string. - fields narrows taskFull to just the named properties, including the new roadblockText (the tail of Result after TaskRunner's roadblock marker) — reachable without pulling in the rest of Result/Description. - taskFull is now BatchTaskDetailDto, a batch-only shape decoupled from TaskDto so get_task's own contract is untouched. - The whole response is capped (MaxResponseChars); over that, the call throws naming which parameter (descriptionMaxChars/fields/taskIds) to adjust instead of shipping an oversized payload. 10 tasks with 5000-char descriptions (default settings) serialize to ~13.8k chars, comfortably under the 25k-char cap. --- docs/explore-notes/external-mcp.md | 15 +- src/ClaudeDo.Worker/External/BatchMcpTools.cs | 122 +++++++++++++- .../External/BatchMcpToolsTests.cs | 150 ++++++++++++++++++ 3 files changed, 280 insertions(+), 7 deletions(-) diff --git a/docs/explore-notes/external-mcp.md b/docs/explore-notes/external-mcp.md index 9795f50d..76c1a243 100644 --- a/docs/explore-notes/external-mcp.md +++ b/docs/explore-notes/external-mcp.md @@ -1,8 +1,8 @@ # External MCP tool surface > **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative. -> Last verified against commit `20bce9b` (2026-08-06). -> Drift check: `git log --oneline 20bce9b..HEAD -- src/ClaudeDo.Worker/External` +> Last verified against commit `6a2a19c` (2026-08-10). +> Drift check: `git log --oneline 6a2a19c..HEAD -- src/ClaudeDo.Worker/External` > Stable structure only (no line numbers). See docs/explore-notes/README.md. Covers `src/ClaudeDo.Worker/External/` — the always-on MCP tools ClaudeDo exposes to general @@ -145,7 +145,16 @@ directory is shared with other concurrent sessions. single-task. Every tool returns a per-item result array (`{ id/index, ok, error?, … }`) — a failing item never aborts the rest — and rejects batches over **100 items**. `BatchGetTasks` mirrors `ListTasks`'s `includeDescription` flag (default `false`): a found item's -`BatchGetTaskResult` carries `task` (lean) or `taskFull` (full), never both. +`BatchGetTaskResult` carries `task` (lean `TaskRefDto`) or `taskFull`, never both. `taskFull` is +`BatchTaskDetailDto` — a batch-only shape, **not** `TaskDto` (`get_task` is untouched) — whose +Description/Result are cut to `descriptionMaxChars` (default 1500, was unlimited) with +`*Truncated`/`*FullLength` flagging it, and which `fields` (an optional name allow-list, e.g. +`['title','roadblockText']`) can narrow further; unrequested fields come back `null`. +`roadblockText` pulls just the bullet lines after `TaskRunner.ComposeReviewResult`'s roadblock +marker out of `Result`, without needing the rest of it. The whole per-call response is also +capped (`BatchMcpTools.MaxResponseChars`) — over that, the call throws naming which parameter to +adjust instead of shipping an oversized payload (the incident that prompted this: 8 tasks' +full Description/Result serialized to a single 51k-char line). **`GetTaskLog`** — latest run's log, tail-capped at 256 KB. diff --git a/src/ClaudeDo.Worker/External/BatchMcpTools.cs b/src/ClaudeDo.Worker/External/BatchMcpTools.cs index 14ccb54e..9938ed08 100644 --- a/src/ClaudeDo.Worker/External/BatchMcpTools.cs +++ b/src/ClaudeDo.Worker/External/BatchMcpTools.cs @@ -1,4 +1,5 @@ using System.ComponentModel; +using System.Text.Json; using ModelContextProtocol.Server; namespace ClaudeDo.Worker.External; @@ -14,8 +15,36 @@ public sealed record BatchSetMyDayInput( // task is populated when found and includeDescription=false (the default, lean reference); // taskFull is populated when found and includeDescription=true (full task incl. -// Description/Result). Both are null when found=false. -public sealed record BatchGetTaskResult(string Id, bool Found, TaskRefDto? Task, TaskDto? TaskFull, string? Error); +// Description/Result, possibly narrowed by `fields` and/or truncated). Both are null when +// found=false. +public sealed record BatchGetTaskResult(string Id, bool Found, TaskRefDto? Task, BatchTaskDetailDto? TaskFull, string? Error); + +// Batch-only counterpart to TaskDto (deliberately not TaskDto itself — get_task's own shape stays +// untouched). Every field defaults to null; BatchGetTasks only populates the ones the caller asked +// for via `fields` (or all of them when `fields` is omitted). Description/Result are cut to +// descriptionMaxChars with the *Truncated/*FullLength pair telling the caller it happened and how +// much text there really is. RoadblockText is the tail of Result after TaskRunner's roadblock +// marker (see ComposeReviewResult in Runner/TaskRunner.cs) — the few bullet lines a roadblocked +// run reported, without pulling in the rest of Result/Description to get them. +public sealed record BatchTaskDetailDto( + string Id, + string? ListId = null, + string? Title = null, + string? Description = null, + string? Status = null, + string? Result = null, + string? CreatedBy = null, + DateTime? CreatedAt = null, + DateTime? StartedAt = null, + DateTime? FinishedAt = null, + bool? IsMyDay = null, + int? SortOrder = null, + int? RoadblockCount = null, + string? RoadblockText = null, + bool DescriptionTruncated = false, + int? DescriptionFullLength = null, + bool ResultTruncated = false, + int? ResultFullLength = null); public sealed record BatchAddTaskResult( int Index, string Title, bool Ok, TaskRefDto? Task, IReadOnlyList? PossibleDuplicates, string? Error); @@ -35,6 +64,18 @@ public sealed class BatchMcpTools { private const int MaxBatchSize = 100; + // Roughly 6k tokens of headroom under a typical 25k-token tool-result limit — the incident + // this guards against was a single 51k-char line from 8 tasks' full Description/Result. + private const int MaxResponseChars = 25_000; + + private const string RoadblockMarker = "Roadblocks reported during the run:"; + + private static readonly string[] KnownDetailFields = + { + "listId", "title", "description", "status", "result", "createdBy", "createdAt", + "startedAt", "finishedAt", "isMyDay", "sortOrder", "roadblockCount", "roadblockText", + }; + private readonly ExternalMcpService _svc; public BatchMcpTools(ExternalMcpService svc) => _svc = svc; @@ -42,14 +83,27 @@ public sealed class BatchMcpTools [McpServerTool, Description( "Fetch a snapshot of many tasks in one call — use for an overview or polling a fan-out instead of " + "calling get_task per id. A missing id comes back as found=false, not an error; error is only set " + - "for an unexpected failure." + McpToolDocs.MaxBatch)] + "for an unexpected failure. When includeDescription=true, Description/Result are cut to " + + "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)] public async Task> BatchGetTasks( string[] taskIds, [Description("If true, return the full task (incl. Description/Result) in `taskFull`; if false " + "(default), return a lean reference in `task`.")] bool includeDescription = false, + [Description("Max chars kept for Description and for Result (each independently) when " + + "includeDescription=true; longer text is cut and flagged via *Truncated/*FullLength. Ignored " + + "when includeDescription=false.")] int descriptionMaxChars = 1500, + [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) { EnsureWithinCap(taskIds, nameof(taskIds)); + if (descriptionMaxChars < 0) + throw new InvalidOperationException($"{nameof(descriptionMaxChars)} must be >= 0."); + ValidateFields(fields); var results = new List(taskIds.Length); foreach (var id in taskIds) @@ -59,7 +113,7 @@ public sealed class BatchMcpTools if (includeDescription) { var task = await _svc.GetTask(id, cancellationToken); - results.Add(new BatchGetTaskResult(id, true, null, task, null)); + results.Add(new BatchGetTaskResult(id, true, null, BuildDetail(task, fields, descriptionMaxChars), null)); } else { @@ -77,9 +131,69 @@ public sealed class BatchMcpTools results.Add(new BatchGetTaskResult(id, false, null, null, ex.Message)); } } + + var responseLength = JsonSerializer.Serialize(results).Length; + if (responseLength > MaxResponseChars) + throw new InvalidOperationException( + $"Response too large: {responseLength} chars (max {MaxResponseChars}). Lower " + + $"{nameof(descriptionMaxChars)}, narrow `fields`, or split {nameof(taskIds)} into a smaller batch."); + return results; } + private static void ValidateFields(string[]? fields) + { + if (fields is null) return; + var unknown = fields.Where(f => !KnownDetailFields.Contains(f, StringComparer.OrdinalIgnoreCase)).ToArray(); + if (unknown.Length > 0) + throw new InvalidOperationException( + $"Unknown field(s) in `fields`: {string.Join(", ", unknown)}. Known fields: " + + string.Join(", ", KnownDetailFields)); + } + + private static BatchTaskDetailDto BuildDetail(TaskDto t, string[]? fields, int descriptionMaxChars) + { + bool Want(string name) => fields is null || fields.Contains(name, StringComparer.OrdinalIgnoreCase); + + var (description, descTruncated, descFullLength) = + Want("description") ? Truncate(t.Description, descriptionMaxChars) : (null, false, null); + var (result, resultTruncated, resultFullLength) = + Want("result") ? Truncate(t.Result, descriptionMaxChars) : (null, false, null); + + return new BatchTaskDetailDto( + Id: t.Id, + ListId: Want("listId") ? t.ListId : null, + Title: Want("title") ? t.Title : null, + Description: description, + Status: Want("status") ? t.Status : null, + Result: result, + CreatedBy: Want("createdBy") ? t.CreatedBy : null, + CreatedAt: Want("createdAt") ? t.CreatedAt : null, + StartedAt: Want("startedAt") ? t.StartedAt : null, + FinishedAt: Want("finishedAt") ? t.FinishedAt : null, + IsMyDay: Want("isMyDay") ? t.IsMyDay : null, + SortOrder: Want("sortOrder") ? t.SortOrder : null, + RoadblockCount: Want("roadblockCount") ? t.RoadblockCount : null, + RoadblockText: Want("roadblockText") ? ExtractRoadblockText(t.Result) : null, + DescriptionTruncated: descTruncated, + DescriptionFullLength: descFullLength, + ResultTruncated: resultTruncated, + ResultFullLength: resultFullLength); + } + + private static (string? Text, bool Truncated, int? FullLength) Truncate(string? text, int maxChars) + { + if (text is null || text.Length <= maxChars) return (text, false, null); + return (text[..maxChars], true, text.Length); + } + + private static string? ExtractRoadblockText(string? result) + { + if (string.IsNullOrEmpty(result)) return null; + var idx = result.IndexOf(RoadblockMarker, StringComparison.Ordinal); + return idx < 0 ? null : result[(idx + RoadblockMarker.Length)..].Trim(); + } + [McpServerTool, Description( "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 " + diff --git a/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs index 361220fd..b6228b39 100644 --- a/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/BatchMcpToolsTests.cs @@ -216,6 +216,156 @@ public sealed class BatchMcpToolsTests : IDisposable Assert.Equal("the full description", found.TaskFull!.Description); } + [Fact] + public async Task BatchGetTasks_DescriptionLongerThanCap_IsTruncated_WithFlagAndFullLength() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + var full = new string('x', 50); + task.Description = full; + await _tasks.UpdateAsync(task); + var sut = BuildSut(); + + var results = await sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, descriptionMaxChars: 10, cancellationToken: CancellationToken.None); + + var found = results.Single(r => r.Id == task.Id).TaskFull!; + Assert.Equal(new string('x', 10), found.Description); + Assert.True(found.DescriptionTruncated); + Assert.Equal(50, found.DescriptionFullLength); + } + + [Fact] + public async Task BatchGetTasks_DescriptionUnderCap_IsNotTruncated() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + task.Description = "short"; + await _tasks.UpdateAsync(task); + var sut = BuildSut(); + + var results = await sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, cancellationToken: CancellationToken.None); + + var found = results.Single(r => r.Id == task.Id).TaskFull!; + Assert.Equal("short", found.Description); + Assert.False(found.DescriptionTruncated); + Assert.Null(found.DescriptionFullLength); + } + + [Fact] + public async Task BatchGetTasks_FieldsRestrictsPayload_ToRequestedFieldsOnly() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + task.Description = "a description that should not come back when fields excludes it"; + await _tasks.UpdateAsync(task); + var sut = BuildSut(); + + var results = await sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, fields: new[] { "title", "status" }, + cancellationToken: CancellationToken.None); + + var found = results.Single(r => r.Id == task.Id).TaskFull!; + Assert.Equal(task.Title, found.Title); + Assert.NotNull(found.Status); + Assert.Null(found.Description); + Assert.Null(found.ListId); + Assert.Null(found.RoadblockCount); + } + + [Fact] + public async Task BatchGetTasks_UnknownField_Throws() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + var sut = BuildSut(); + + var ex = await Assert.ThrowsAsync( + () => sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, fields: new[] { "notAField" }, + cancellationToken: CancellationToken.None)); + Assert.Contains("notAField", ex.Message); + } + + [Fact] + public async Task BatchGetTasks_RoadblockTextField_ExtractsTailOfResult_WithoutFullResult() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + task.Result = "the task's own result\n\n⚠ Roadblocks reported during the run:\n- missing credentials"; + await _tasks.UpdateAsync(task); + var sut = BuildSut(); + + var results = await sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, fields: new[] { "roadblockText" }, + cancellationToken: CancellationToken.None); + + var found = results.Single(r => r.Id == task.Id).TaskFull!; + Assert.Equal("- missing credentials", found.RoadblockText); + Assert.Null(found.Result); + } + + [Fact] + public async Task BatchGetTasks_NoRoadblockMarker_RoadblockTextIsNull() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + task.Result = "plain result, no roadblocks"; + await _tasks.UpdateAsync(task); + var sut = BuildSut(); + + var results = await sut.BatchGetTasks( + new[] { task.Id }, includeDescription: true, fields: new[] { "roadblockText" }, + cancellationToken: CancellationToken.None); + + Assert.Null(results.Single(r => r.Id == task.Id).TaskFull!.RoadblockText); + } + + [Fact] + public async Task BatchGetTasks_OversizedResponse_ThrowsNamingTheFixingParameters() + { + var listId = await SeedListAsync(); + var ids = new List(); + for (var i = 0; i < 10; i++) + { + var task = await SeedTaskAsync(listId, $"t{i}"); + task.Description = new string('d', 6000); + await _tasks.UpdateAsync(task); + ids.Add(task.Id); + } + var sut = BuildSut(); + + var ex = await Assert.ThrowsAsync( + () => sut.BatchGetTasks( + ids.ToArray(), includeDescription: true, descriptionMaxChars: 6000, cancellationToken: CancellationToken.None)); + Assert.Contains("descriptionMaxChars", ex.Message); + Assert.Contains("fields", ex.Message); + Assert.Contains("taskIds", ex.Message); + } + + [Fact] + public async Task BatchGetTasks_TenTasksWithDescriptions_StaysWithinTokenBudget() + { + var listId = await SeedListAsync(); + var ids = new List(); + for (var i = 0; i < 10; i++) + { + var task = await SeedTaskAsync(listId, $"t{i}"); + task.Description = new string('d', 5000); + await _tasks.UpdateAsync(task); + ids.Add(task.Id); + } + var sut = BuildSut(); + + var results = await sut.BatchGetTasks(ids.ToArray(), includeDescription: true, cancellationToken: CancellationToken.None); + + Assert.Equal(10, results.Count); + Assert.All(results, r => Assert.True(r.TaskFull!.DescriptionTruncated)); + var json = System.Text.Json.JsonSerializer.Serialize(results); + Assert.True(json.Length < 25_000, $"response was {json.Length} chars, expected < 25000"); + } + [Fact] public async Task BatchDeleteTasks_RunningTask_ReportedNotOk_OthersDeleted() {