fix(worker): make list_tasks/batch_get_tasks lean by default
list_tasks on a list of ~100 verbosely-described tasks could return 390k+ characters in one call, blowing past the caller's token limit. Both tools now default to lean TaskRefDto references (no Description/Result) and take an includeDescription flag to opt back into the full TaskDto payload — same flag-alongside-nullable-payload idiom already used by BatchGetTaskResult/TaskConfigResult. get_task is unchanged.
This commit is contained in:
mika kuns
@@ -1,8 +1,8 @@
|
||||
# External MCP tool surface
|
||||
|
||||
> **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative.
|
||||
> Last verified against commit `bdee731` (2026-08-05).
|
||||
> Drift check: `git log --oneline bdee731..HEAD -- src/ClaudeDo.Worker/External`
|
||||
> Last verified against commit `86f962e` (2026-08-06).
|
||||
> Drift check: `git log --oneline 86f962e..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
|
||||
@@ -27,6 +27,11 @@ session internals, or app-settings writes. Auth via an optional `X-ClaudeDo-Key`
|
||||
the resulting config so the caller can see which fields were set vs. cleared to null).
|
||||
- *Read* tools that may have nothing to return use an explicit `Found` / `Available` flag
|
||||
alongside the nullable payload (`TaskConfigResult`, `BatchGetTaskResult`, `TaskLogResult`).
|
||||
- The same flag-alongside-nullable-payload idiom also covers "which of two shapes did you
|
||||
get": `ListTasks`/`BatchGetTasks` take `includeDescription` (default `false`) and return
|
||||
`ListTasksResult`/`BatchGetTaskResult`, where exactly one of the lean (`TaskRefDto`) and
|
||||
full (`TaskDto`, incl. Description/Result) fields is populated per the flag — keeps a
|
||||
list of verbosely-described tasks from blowing past the response size limit by default.
|
||||
3. `ExternalMcpExceptionFilter.Wrap` is registered as a call-tool filter so
|
||||
`InvalidOperationException` / `ArgumentException` messages survive as `McpException` —
|
||||
otherwise the SDK's catch-all replaces any non-`McpException` with a generic
|
||||
@@ -62,6 +67,11 @@ Daily prep: `GetDailyPrepCandidates`, `SetMyDay`.
|
||||
|
||||
## Per-tool behaviour worth knowing
|
||||
|
||||
**`ListTasks`** — `includeDescription=false` (default) returns lean `TaskRefDto` references in
|
||||
`tasks` (`tasksFull` null); `includeDescription=true` returns full `TaskDto`s (incl.
|
||||
Description/Result) in `tasksFull` instead (`tasks` null). Filtering by `createdBy`/`status`
|
||||
happens before the lean/full projection either way.
|
||||
|
||||
**`UpdateTaskStatus`** accepts `Idle` / `Queued` / `Cancelled` / `Done` only.
|
||||
- `Cancelled` goes through `TaskStateService.CancelAsync(..., allowFromIdle: true)` — the
|
||||
**only** caller that opts into cancelling from `Idle`. `PlanningChainCoordinator` relies on
|
||||
@@ -102,6 +112,8 @@ directory is shared with other concurrent sessions.
|
||||
**Sequential**, because the scoped `DbContext` is not thread-safe. Merge/review stay
|
||||
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.
|
||||
|
||||
**`GetTaskLog`** — latest run's log, tail-capped at 256 KB.
|
||||
|
||||
|
||||
+22
-8
@@ -6,7 +6,10 @@ namespace ClaudeDo.Worker.External;
|
||||
public sealed record BatchAddTaskInput(string Title, string? Description = null, string? Model = null);
|
||||
public sealed record BatchSetMyDayInput(string TaskId, bool IsMyDay, int? SortOrder = null);
|
||||
|
||||
public sealed record BatchGetTaskResult(string Id, bool Found, TaskDto? Task, string? Error);
|
||||
// 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);
|
||||
public sealed record BatchAddTaskResult(int Index, string Title, bool Ok, TaskRefDto? Task, string? Error);
|
||||
public sealed record BatchTaskResult(string TaskId, bool Ok, string? Error);
|
||||
public sealed record BatchCancelResult(string TaskId, bool Ok, bool Cancelled, string? Error);
|
||||
@@ -30,10 +33,13 @@ public sealed class BatchMcpTools
|
||||
|
||||
[McpServerTool, Description(
|
||||
"Fetch a snapshot of many tasks in one call (overview / polling a fan-out). " +
|
||||
"Returns one result per id: { id, found, task, error }. A missing id is found=false " +
|
||||
"(not an error); error is only set for an unexpected failure. Max 100 ids.")]
|
||||
"Returns one result per id: { id, found, task, taskFull, error }. " +
|
||||
"includeDescription=false (default): found tasks come back in `task` (lean reference, no " +
|
||||
"Description/Result). includeDescription=true: found tasks come back in `taskFull` (incl. " +
|
||||
"Description/Result) instead. A missing id is found=false (not an error; task and taskFull both null); " +
|
||||
"error is only set for an unexpected failure. Max 100 ids.")]
|
||||
public async Task<IReadOnlyList<BatchGetTaskResult>> BatchGetTasks(
|
||||
string[] taskIds, CancellationToken cancellationToken)
|
||||
string[] taskIds, bool includeDescription = false, CancellationToken cancellationToken = default)
|
||||
{
|
||||
EnsureWithinCap(taskIds, nameof(taskIds));
|
||||
|
||||
@@ -42,17 +48,25 @@ public sealed class BatchMcpTools
|
||||
{
|
||||
try
|
||||
{
|
||||
var task = await _svc.GetTask(id, cancellationToken);
|
||||
results.Add(new BatchGetTaskResult(id, true, task, null));
|
||||
if (includeDescription)
|
||||
{
|
||||
var task = await _svc.GetTask(id, cancellationToken);
|
||||
results.Add(new BatchGetTaskResult(id, true, null, task, null));
|
||||
}
|
||||
else
|
||||
{
|
||||
var taskRef = await _svc.GetTaskRefAsync(id, cancellationToken);
|
||||
results.Add(new BatchGetTaskResult(id, true, taskRef, null, null));
|
||||
}
|
||||
}
|
||||
catch (OperationCanceledException) { throw; }
|
||||
catch (InvalidOperationException)
|
||||
{
|
||||
results.Add(new BatchGetTaskResult(id, false, null, null));
|
||||
results.Add(new BatchGetTaskResult(id, false, null, null, null));
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
results.Add(new BatchGetTaskResult(id, false, null, ex.Message));
|
||||
results.Add(new BatchGetTaskResult(id, false, null, null, ex.Message));
|
||||
}
|
||||
}
|
||||
return results;
|
||||
|
||||
+30
-3
@@ -47,6 +47,15 @@ public sealed record TaskRefDto(
|
||||
int SortOrder,
|
||||
bool IsMyDay);
|
||||
|
||||
// tasks is populated when includeDescription=false (the default): lean references, no
|
||||
// Description/Result. tasksFull is populated when includeDescription=true: full tasks incl.
|
||||
// Description/Result. Exactly one of the two is non-null per the includeDescription flag —
|
||||
// same "flag alongside nullable payload" idiom as BatchGetTaskResult.
|
||||
public sealed record ListTasksResult(
|
||||
bool IncludeDescription,
|
||||
IReadOnlyList<TaskRefDto>? Tasks,
|
||||
IReadOnlyList<TaskDto>? TasksFull);
|
||||
|
||||
public sealed record WorktreeInfoDto(
|
||||
string Path, string Branch, string HeadCommit, string BaseCommit,
|
||||
int Ahead, int Behind, bool IsDirty, string? MergeCommit = null);
|
||||
@@ -139,11 +148,17 @@ public sealed class ExternalMcpService
|
||||
|
||||
[McpServerTool, Description(
|
||||
"List tasks in a given list. Optionally filter by creator (createdBy) and/or status. " +
|
||||
"Valid status values: Idle, Queued, Running, WaitingForReview, WaitingForChildren, Done, Failed, Cancelled.")]
|
||||
public async Task<IReadOnlyList<TaskDto>> ListTasks(
|
||||
"Valid status values: Idle, Queued, Running, WaitingForReview, WaitingForChildren, Done, Failed, Cancelled. " +
|
||||
"includeDescription=false (default): returns lean task references in `tasks` (no Description/Result) — " +
|
||||
"use this unless you actually need the description text, since a list of verbosely-described tasks can " +
|
||||
"otherwise blow past the response size limit. " +
|
||||
"includeDescription=true: returns full tasks (incl. Description/Result) in `tasksFull` instead; `tasks` is " +
|
||||
"null in that case.")]
|
||||
public async Task<ListTasksResult> ListTasks(
|
||||
string listId,
|
||||
string? createdBy = null,
|
||||
string? status = null,
|
||||
bool includeDescription = false,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
TaskStatus? statusFilter = null;
|
||||
@@ -162,7 +177,10 @@ public sealed class ExternalMcpService
|
||||
if (statusFilter is not null)
|
||||
query = query.Where(t => t.Status == statusFilter);
|
||||
|
||||
return query.Select(ToDto).ToList();
|
||||
var filtered = query.ToList();
|
||||
return includeDescription
|
||||
? new ListTasksResult(true, null, filtered.Select(ToDto).ToList())
|
||||
: new ListTasksResult(false, filtered.Select(ToRefDto).ToList(), null);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
@@ -177,6 +195,15 @@ public sealed class ExternalMcpService
|
||||
return ToDto(task);
|
||||
}
|
||||
|
||||
// Lean counterpart to GetTask, used internally by BatchGetTasks' default (includeDescription=false)
|
||||
// path. Not an MCP tool itself — GetTask's own behavior stays untouched.
|
||||
internal async Task<TaskRefDto> GetTaskRefAsync(string taskId, CancellationToken cancellationToken)
|
||||
{
|
||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||
return ToRefDto(task);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
"Create a new task in the given list. Set queueImmediately=true to enqueue it for agent execution. " +
|
||||
"Set model to the cheapest model that can do the task well — 'haiku' for trivial/mechanical work, " +
|
||||
|
||||
+41
-3
@@ -149,17 +149,55 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
var task = await SeedTaskAsync(listId);
|
||||
var sut = BuildSut();
|
||||
|
||||
var results = await sut.BatchGetTasks(new[] { task.Id, "nope" }, CancellationToken.None);
|
||||
var results = await sut.BatchGetTasks(new[] { task.Id, "nope" }, cancellationToken: CancellationToken.None);
|
||||
|
||||
var found = results.Single(r => r.Id == task.Id);
|
||||
var missing = results.Single(r => r.Id == "nope");
|
||||
Assert.True(found.Found);
|
||||
Assert.NotNull(found.Task);
|
||||
Assert.Null(found.TaskFull);
|
||||
Assert.False(missing.Found);
|
||||
Assert.Null(missing.Task);
|
||||
Assert.Null(missing.TaskFull);
|
||||
Assert.Null(missing.Error);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchGetTasks_Default_ReturnsLeanTask_NoTaskFull()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId);
|
||||
task.Description = "a description that should not come back by default";
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = BuildSut();
|
||||
|
||||
var results = await sut.BatchGetTasks(new[] { task.Id }, cancellationToken: CancellationToken.None);
|
||||
|
||||
var found = results.Single(r => r.Id == task.Id);
|
||||
Assert.True(found.Found);
|
||||
Assert.NotNull(found.Task);
|
||||
Assert.Equal(task.Id, found.Task!.Id);
|
||||
Assert.Null(found.TaskFull);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchGetTasks_IncludeDescriptionTrue_ReturnsTaskFull()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId);
|
||||
task.Description = "the full description";
|
||||
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);
|
||||
Assert.True(found.Found);
|
||||
Assert.Null(found.Task);
|
||||
Assert.NotNull(found.TaskFull);
|
||||
Assert.Equal("the full description", found.TaskFull!.Description);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchDeleteTasks_RunningTask_ReportedNotOk_OthersDeleted()
|
||||
{
|
||||
@@ -212,7 +250,7 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
{
|
||||
var sut = BuildSut();
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => sut.BatchGetTasks(Array.Empty<string>(), CancellationToken.None));
|
||||
() => sut.BatchGetTasks(Array.Empty<string>(), cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -222,7 +260,7 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
var ids = Enumerable.Range(0, 101).Select(i => i.ToString()).ToArray();
|
||||
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => sut.BatchGetTasks(ids, CancellationToken.None));
|
||||
() => sut.BatchGetTasks(ids, cancellationToken: CancellationToken.None));
|
||||
Assert.Contains("max", ex.Message, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -947,10 +947,11 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
await SeedTaskAsync(listId, "idle", TaskStatus.Idle);
|
||||
var sut = NewService();
|
||||
|
||||
var result = await sut.ListTasks(listId, null, "WaitingForReview", CancellationToken.None);
|
||||
var result = await sut.ListTasks(listId, null, "WaitingForReview", cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Single(result);
|
||||
Assert.Equal("WaitingForReview", result[0].Status);
|
||||
Assert.NotNull(result.Tasks);
|
||||
Assert.Single(result.Tasks!);
|
||||
Assert.Equal("WaitingForReview", result.Tasks![0].Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -961,10 +962,47 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
await SeedTaskAsync(listId, "done", TaskStatus.Done);
|
||||
var sut = NewService();
|
||||
|
||||
var result = await sut.ListTasks(listId, null, "WaitingForChildren", CancellationToken.None);
|
||||
var result = await sut.ListTasks(listId, null, "WaitingForChildren", cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Single(result);
|
||||
Assert.Equal("WaitingForChildren", result[0].Status);
|
||||
Assert.NotNull(result.Tasks);
|
||||
Assert.Single(result.Tasks!);
|
||||
Assert.Equal("WaitingForChildren", result.Tasks![0].Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListTasks_Default_ReturnsLeanReferences_NoDescriptionOrTasksFull()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId, "with desc");
|
||||
task.Description = "a long description that should not come back by default";
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = NewService();
|
||||
|
||||
var result = await sut.ListTasks(listId, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.False(result.IncludeDescription);
|
||||
Assert.NotNull(result.Tasks);
|
||||
Assert.Null(result.TasksFull);
|
||||
Assert.Single(result.Tasks!);
|
||||
Assert.Equal(task.Id, result.Tasks![0].Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListTasks_IncludeDescriptionTrue_ReturnsFullTasks()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId, "with desc");
|
||||
task.Description = "the full description";
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = NewService();
|
||||
|
||||
var result = await sut.ListTasks(listId, includeDescription: true, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.True(result.IncludeDescription);
|
||||
Assert.Null(result.Tasks);
|
||||
Assert.NotNull(result.TasksFull);
|
||||
Assert.Single(result.TasksFull!);
|
||||
Assert.Equal("the full description", result.TasksFull![0].Description);
|
||||
}
|
||||
|
||||
// ── MergeTask allowWaitingForReview ───────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user