Merge claudedo/0020cd0c4696431996a158f8e1b91cba

src/ClaudeDo.Worker/CLAUDE.md

@@ -33,7 +33,7 @@ Interfaces (e.g. `IQueueWaker`, `IPrimeClock`, `ITaskStateService`) live in an `
 - **IQueueWaker / IQueuePicker / QueueService** — waker is a singleton `SemaphoreSlim`; picker performs the atomic `Queued → Running` claim filtered by `BlockedByTaskId IS NULL`, `is_manual = 0` and schedule; QueueService is a thin `BackgroundService` that loops on the waker and dispatches via `TaskRunner`.
 - **OverrideSlotService** — owns `RunNow` / `ContinueTask`; goes through `TaskStateService.StartRunningAsync` (caller-driven, serialized by slot lock).
 - **StaleTaskRecovery** — startup-only service; calls `TaskStateService.RecoverStaleRunningAsync` to flip orphaned `Running` rows to `Failed`.
-- **External/*** — always-on MCP tools for general Claude sessions, scoped to *starting* and *observing* sessions (no worktree/merge, multi-turn, planning, or app-settings writes). Auth via optional `X-ClaudeDo-Key` header. Registered explicitly in `Program.cs`'s external app via `.WithTools<T>()`. Every optional/filter parameter across these tools must carry a C# default value (e.g. `string? status = null`) — the MCP schema only marks a parameter optional when it has one; nullability alone doesn't do it (`ExternalMcpToolSchemaTests` guards this by reflection). `ExternalMcpExceptionFilter.Wrap` is registered as a call-tool filter so `InvalidOperationException`/`ArgumentException` messages survive as `McpException` — otherwise the SDK's own catch-all replaces any non-`McpException` with a generic "An error occurred invoking 'X'." Organized by concern:
+- **External/*** — always-on MCP tools for general Claude sessions, scoped to *starting* and *observing* sessions (no worktree/merge, multi-turn, planning, or app-settings writes). Auth via optional `X-ClaudeDo-Key` header. Registered explicitly in `Program.cs`'s external app via `.WithTools<T>()`. Every optional/filter parameter across these tools must carry a C# default value (e.g. `string? status = null`) — the MCP schema only marks a parameter optional when it has one; nullability alone doesn't do it (`ExternalMcpToolSchemaTests` guards this by reflection). `ExternalMcpExceptionFilter.Wrap` is registered as a call-tool filter so `InvalidOperationException`/`ArgumentException` messages survive as `McpException` — otherwise the SDK's own catch-all replaces any non-`McpException` with a generic "An error occurred invoking 'X'." **No external tool returns bare `Task` or a nullable payload directly** — an MCP client can't tell an empty/omitted response apart from a dropped one. Write tools return a small confirmation record (`{ ok/deleted/removed/reset/started: true, <id>, ... }`, e.g. `DeleteListResult`, `RunTaskNowResult`, `ResetFailedTaskResult`, `RemoveAttachmentResult`; `SetListConfigResult`/`SetTaskConfigResult` additionally echo 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`, pre-existing `BatchGetTaskResult`, `TaskLogResult`) instead of returning null outright. Organized by concern:
   - `ExternalMcpService` — task CRUD + execution: `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask`, `AddSubtask`, `UpdateTask`, `UpdateTaskStatus` (`Idle` / `Queued` / `Cancelled` / `Done` — `Cancelled` goes through `TaskStateService.CancelAsync(..., allowFromIdle: true)`, the only caller that opts into cancelling from `Idle`; `PlanningChainCoordinator` relies on `Idle` staying a no-op there by default, since a child parked back to `Idle` mid-chain is a manual opt-out signal; `Done` goes through `TaskStateService.ForceSetStatusAsync` — same unconditional write the UI's "set status freely" affordance uses — but is refused here with an active-worktree task, since that would skip `review_task`'s merge), `GetTaskStatusValues`, `ReviewTask` (`approve` / `reject_rerun` / `reject_park` / `cancel` for a WaitingForReview task; approve is review+merge exactly like the hub's `ApproveReview` — unit merge for parents, worktree merge into optional `targetBranch` for childless tasks, conflicts reported in the `ReviewTaskResult`), `RunTaskNow`, `ContinueTask`, `CancelTask`, `DeleteTask`; worktree/git: `GetTaskWorktree`, `GetTaskDiff`, `MergeTask`, `ListWorktrees`, `CleanupTaskWorktree`
   - `BatchMcpTools` — best-effort batch variants that loop the `ExternalMcpService` single-entity methods (sequential — the scoped DbContext is not thread-safe; merge/review stay single-task): `BatchGetTasks`, `BatchAddTasks`, `BatchUpdateTaskStatus`, `BatchCancelTasks`, `BatchDeleteTasks`, `BatchSetMyDay`, `BatchCleanupTaskWorktrees`. 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.
   - `ListMcpTools` — `CreateList`, `UpdateList`, `DeleteList`

src/ClaudeDo.Worker/External/AttachmentMcpTools.cs

@@ -10,6 +10,7 @@ using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
 namespace ClaudeDo.Worker.External;
 
 public sealed record AttachmentDto(string FileName, long ByteSize, DateTime CreatedAt);
+public sealed record RemoveAttachmentResult(bool Removed, string TaskId, string FileName);
 
 [McpServerToolType]
 public sealed class AttachmentMcpTools
@@ -103,8 +104,8 @@ public sealed class AttachmentMcpTools
 
     [McpServerTool, Description(
         "Remove a single attachment from a task. Deletes both the file on disk and the database record. " +
-        "Refuses if the task is currently Running — cancel it first.")]
-    public async Task RemoveTaskAttachment(
+        "Refuses if the task is currently Running — cancel it first. Returns { removed: true, taskId, fileName } on success.")]
+    public async Task<RemoveAttachmentResult> RemoveTaskAttachment(
         string taskId, string fileName, CancellationToken ct = default)
     {
         var task = await _tasks.GetByIdAsync(taskId, ct)
@@ -115,5 +116,6 @@ public sealed class AttachmentMcpTools
         _store.DeleteFile(taskId, fileName);
         await _attachments.DeleteAsync(taskId, fileName, ct);
         await _broadcaster.TaskUpdated(taskId);
+        return new RemoveAttachmentResult(true, taskId, fileName);
     }
 }

src/ClaudeDo.Worker/External/ConfigMcpTools.cs

@@ -7,6 +7,9 @@ using ModelContextProtocol.Server;
 namespace ClaudeDo.Worker.External;
 
 public sealed record TaskConfigDto(string? Model, string? SystemPrompt, string? AgentPath, int? MaxTurns);
+public sealed record TaskConfigResult(bool Found, TaskConfigDto? Config);
+public sealed record SetListConfigResult(bool Ok, string ListId, TaskConfigDto? Config);
+public sealed record SetTaskConfigResult(bool Ok, string TaskId, TaskConfigDto? Config);
 
 [McpServerToolType]
 public sealed class ConfigMcpTools
@@ -22,15 +25,20 @@ public sealed class ConfigMcpTools
         _broadcaster = broadcaster;
     }
 
-    [McpServerTool, Description("Get a list's default config (model, system prompt, agent path). Returns null if no config is set.")]
-    public async Task<TaskConfigDto?> GetListConfig(string listId, CancellationToken cancellationToken)
+    [McpServerTool, Description("Get a list's default config (model, system prompt, agent path). Returns { found: false, config: null } if no config is set.")]
+    public async Task<TaskConfigResult> GetListConfig(string listId, CancellationToken cancellationToken)
     {
         var cfg = await _lists.GetConfigAsync(listId, cancellationToken);
-        return cfg is null ? null : new TaskConfigDto(cfg.Model, cfg.SystemPrompt, cfg.AgentPath, cfg.MaxTurns);
+        return cfg is null
+            ? new TaskConfigResult(false, null)
+            : new TaskConfigResult(true, new TaskConfigDto(cfg.Model, cfg.SystemPrompt, cfg.AgentPath, cfg.MaxTurns));
     }
 
-    [McpServerTool, Description("Set a list's default model/system prompt/agent path/max turns. Passing all four as null clears the list config.")]
-    public async Task SetListConfig(
+    [McpServerTool, Description(
+        "Set a list's default model/system prompt/agent path/max turns. Passing all four as null clears the list " +
+        "config. Returns { ok, listId, config } — config is null when the config was cleared, otherwise it echoes " +
+        "the fields that were set (a field is null there if it was individually left unset/cleared).")]
+    public async Task<SetListConfigResult> SetListConfig(
         string listId, string? model = null, string? systemPrompt = null, string? agentPath = null,
         int? maxTurns = null, CancellationToken cancellationToken = default)
     {
@@ -41,36 +49,52 @@ public sealed class ConfigMcpTools
         var sp = systemPrompt.NullIfBlank();
         var ap = agentPath.NullIfBlank();
 
+        TaskConfigDto? config;
         if (m is null && sp is null && ap is null && maxTurns is null)
+        {
             await _lists.DeleteConfigAsync(listId, cancellationToken);
+            config = null;
+        }
         else
+        {
             await _lists.SetConfigAsync(new ListConfigEntity
             {
                 ListId = listId, Model = m, SystemPrompt = sp, AgentPath = ap, MaxTurns = maxTurns,
             }, cancellationToken);
+            config = new TaskConfigDto(m, sp, ap, maxTurns);
+        }
 
         await _broadcaster.ListUpdated(listId);
+        return new SetListConfigResult(true, listId, config);
     }
 
-    [McpServerTool, Description("Set per-task config overrides (model/system prompt/agent path/max turns). Pass null for any field to clear that override.")]
-    public async Task SetTaskConfig(
+    [McpServerTool, Description(
+        "Set per-task config overrides (model/system prompt/agent path/max turns). Pass null for any field to " +
+        "clear that override. Returns { ok, taskId, config } — config echoes the resulting overrides (a field is " +
+        "null there if it was cleared or never set).")]
+    public async Task<SetTaskConfigResult> SetTaskConfig(
         string taskId, string? model = null, string? systemPrompt = null, string? agentPath = null,
         int? maxTurns = null, CancellationToken cancellationToken = default)
     {
         _ = await _tasks.GetByIdAsync(taskId, cancellationToken)
             ?? throw new InvalidOperationException($"Task {taskId} not found.");
 
-        await _tasks.UpdateAgentSettingsAsync(taskId, model.NullIfBlank(), systemPrompt.NullIfBlank(), agentPath.NullIfBlank(), maxTurns, ct: cancellationToken);
+        var m = model.NullIfBlank();
+        var sp = systemPrompt.NullIfBlank();
+        var ap = agentPath.NullIfBlank();
+
+        await _tasks.UpdateAgentSettingsAsync(taskId, m, sp, ap, maxTurns, ct: cancellationToken);
         await _broadcaster.TaskUpdated(taskId);
+        return new SetTaskConfigResult(true, taskId, new TaskConfigDto(m, sp, ap, maxTurns));
     }
 
-    [McpServerTool, Description("Get per-task config overrides (model/system prompt/agent path/max turns). Returns null if no override is set on this task.")]
-    public async Task<TaskConfigDto?> GetTaskConfig(string taskId, CancellationToken cancellationToken)
+    [McpServerTool, Description("Get per-task config overrides (model/system prompt/agent path/max turns). Returns { found: false, config: null } if no override is set on this task.")]
+    public async Task<TaskConfigResult> GetTaskConfig(string taskId, CancellationToken cancellationToken)
     {
         var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
             ?? throw new InvalidOperationException($"Task {taskId} not found.");
         if (task.Model is null && task.SystemPrompt is null && task.AgentPath is null && task.MaxTurns is null)
-            return null;
-        return new TaskConfigDto(task.Model, task.SystemPrompt, task.AgentPath, task.MaxTurns);
+            return new TaskConfigResult(false, null);
+        return new TaskConfigResult(true, new TaskConfigDto(task.Model, task.SystemPrompt, task.AgentPath, task.MaxTurns));
     }
 }

src/ClaudeDo.Worker/External/ExternalMcpService.cs

@@ -21,6 +21,7 @@ public sealed record DeleteTaskResult(bool Deleted, string Id);
 public sealed record CancelTaskResult(bool Cancelled, string Id);
 public sealed record ReviewTaskResult(TaskDto Task, string? MergeStatus, IReadOnlyList<string> MergeConflicts, string? MergeMessage, string? RepoPath = null);
 public sealed record StatusValueDto(string Status, string Meaning);
+public sealed record RunTaskNowResult(bool Started, string TaskId);
 
 public sealed record TaskDto(
     string Id,
@@ -422,8 +423,8 @@ public sealed class ExternalMcpService
             mergeStatus, mergeConflicts, mergeMessage, repoPath);
     }
 
-    [McpServerTool, Description("Immediately run a task in the override execution slot (bypasses the agent queue).")]
-    public async Task RunTaskNow(string taskId, CancellationToken cancellationToken)
+    [McpServerTool, Description("Immediately run a task in the override execution slot (bypasses the agent queue). Returns { started: true, taskId } on success.")]
+    public async Task<RunTaskNowResult> RunTaskNow(string taskId, CancellationToken cancellationToken)
     {
         try
         {
@@ -438,6 +439,7 @@ public sealed class ExternalMcpService
             throw new InvalidOperationException($"Task {taskId} not found.");
         }
         await _broadcaster.TaskUpdated(taskId);
+        return new RunTaskNowResult(true, taskId);
     }
 
     [McpServerTool, Description("Cancel a running task. Returns { cancelled: true, id } if the task was running and cancellation was requested; cancelled is false if the task was not running.")]

src/ClaudeDo.Worker/External/LifecycleMcpTools.cs

@@ -6,6 +6,8 @@ using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
 
 namespace ClaudeDo.Worker.External;
 
+public sealed record ResetFailedTaskResult(bool Reset, string TaskId);
+
 [McpServerToolType]
 public sealed class LifecycleMcpTools
 {
@@ -18,8 +20,8 @@ public sealed class LifecycleMcpTools
         _reset = reset;
     }
 
-    [McpServerTool, Description("Reset a failed task: discards its worktree and returns it to Idle so it can be run again. Only Failed tasks are accepted.")]
-    public async Task ResetFailedTask(string taskId, CancellationToken cancellationToken)
+    [McpServerTool, Description("Reset a failed task: discards its worktree and returns it to Idle so it can be run again. Only Failed tasks are accepted. Returns { reset: true, taskId } on success.")]
+    public async Task<ResetFailedTaskResult> ResetFailedTask(string taskId, CancellationToken cancellationToken)
     {
         var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
             ?? throw new InvalidOperationException($"Task {taskId} not found.");
@@ -27,5 +29,6 @@ public sealed class LifecycleMcpTools
             throw new InvalidOperationException($"Task {taskId} is {task.Status}, not Failed. Only failed tasks can be reset via this tool.");
 
         await _reset.ResetAsync(taskId, cancellationToken);
+        return new ResetFailedTaskResult(true, taskId);
     }
 }

src/ClaudeDo.Worker/External/ListMcpTools.cs

@@ -7,6 +7,7 @@ using ModelContextProtocol.Server;
 namespace ClaudeDo.Worker.External;
 
 public sealed record ListSummaryDto(string Id, string Name, string? WorkingDir, string DefaultCommitType);
+public sealed record DeleteListResult(bool Deleted, string Id);
 
 [McpServerToolType]
 public sealed class ListMcpTools
@@ -61,13 +62,14 @@ public sealed class ListMcpTools
         return ToDto(entity);
     }
 
-    [McpServerTool, Description("Delete a list and its tasks. Irreversible.")]
-    public async Task DeleteList(string listId, CancellationToken cancellationToken)
+    [McpServerTool, Description("Delete a list and its tasks. Irreversible. Returns { deleted: true, id } on success.")]
+    public async Task<DeleteListResult> DeleteList(string listId, CancellationToken cancellationToken)
     {
         _ = await _lists.GetByIdAsync(listId, cancellationToken)
             ?? throw new InvalidOperationException($"List {listId} not found.");
         await _lists.DeleteAsync(listId, cancellationToken);
         await _broadcaster.ListUpdated(listId);
+        return new DeleteListResult(true, listId);
     }
 
     private static ListSummaryDto ToDto(ListEntity l) =>