Merge branch 'claudedo/f359858ac98a439593e459df9c5d0a5d'
This commit is contained in:
mika kuns
@@ -0,0 +1,141 @@
|
||||
# External MCP tool surface
|
||||
|
||||
> **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative.
|
||||
> Last verified against commit `f6cb825` (2026-08-05).
|
||||
> Drift check: `git log --oneline f6cb825..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
|
||||
Claude sessions. Registered explicitly in `Program.cs`'s external app via `.WithTools<T>()`.
|
||||
Server name `claudedo`, registered globally by the installer's `RegisterMcpStep`, so callers
|
||||
need no `--mcp-config`.
|
||||
|
||||
**Scope boundary:** these tools cover *starting* and *observing* sessions plus task/list CRUD
|
||||
and git/merge operations. They deliberately do **not** expose multi-turn control, planning
|
||||
session internals, or app-settings writes. Auth via an optional `X-ClaudeDo-Key` header.
|
||||
|
||||
## Hard conventions (enforced by tests)
|
||||
|
||||
1. **Every optional/filter parameter needs a C# default value** (e.g. `string? status = null`).
|
||||
The MCP schema only marks a parameter optional when it has one — nullability alone does
|
||||
not do it. `ExternalMcpToolSchemaTests` guards this by reflection.
|
||||
2. **No tool returns bare `Task` or a nullable payload directly.** An MCP client cannot tell
|
||||
an empty/omitted response apart from a dropped one.
|
||||
- *Write* tools return a small confirmation record — `{ ok/deleted/removed/reset/started:
|
||||
true, <id>, ... }` (`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`, `BatchGetTaskResult`, `TaskLogResult`).
|
||||
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
|
||||
*"An error occurred invoking 'X'."*
|
||||
|
||||
## Tool classes
|
||||
|
||||
### `ExternalMcpService` — task CRUD, execution, git
|
||||
|
||||
Task: `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask`, `AddSubtask`, `UpdateTask`,
|
||||
`UpdateTaskStatus`, `GetTaskStatusValues`, `ReviewTask`, `RunTaskNow`, `ContinueTask`,
|
||||
`CancelTask`, `DeleteTask`.
|
||||
|
||||
Worktree/git: `GetTaskWorktree`, `GetTaskDiff`, `MergeTask`, `ContinueMerge`, `AbortMerge`,
|
||||
`PreviewMerge`, `PreviewMergeSet`, `RevertMerge`, `ListWorktrees`, `CleanupTaskWorktree`.
|
||||
|
||||
Daily prep: `GetDailyPrepCandidates`, `SetMyDay`.
|
||||
|
||||
### Other classes
|
||||
|
||||
| Class | Tools |
|
||||
|---|---|
|
||||
| `BatchMcpTools` | `BatchGetTasks`, `BatchAddTasks`, `BatchUpdateTaskStatus`, `BatchCancelTasks`, `BatchDeleteTasks`, `BatchSetMyDay`, `BatchCleanupTaskWorktrees` |
|
||||
| `ListMcpTools` | `CreateList`, `UpdateList`, `DeleteList` |
|
||||
| `ConfigMcpTools` | `GetListConfig`, `SetListConfig`, `GetTaskConfig`, `SetTaskConfig` |
|
||||
| `RunHistoryMcpTools` | `ListRuns`, `GetRun`, `GetTaskLog` |
|
||||
| `AgentMcpTools` | `ListAgents` |
|
||||
| `LifecycleMcpTools` | `ResetFailedTask` |
|
||||
| `AppSettingsMcpTools` | `GetAppSettings` (read-only) |
|
||||
| `TaskWaitMcpTools` | `WaitForTaskChange` |
|
||||
| `AttachmentMcpTools` | `AddTaskAttachment`, `ListTaskAttachments`, `RemoveTaskAttachment` |
|
||||
|
||||
## Per-tool behaviour worth knowing
|
||||
|
||||
**`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
|
||||
`Idle` staying a no-op there by default, because a child parked back to `Idle` mid-chain is
|
||||
a manual opt-out signal.
|
||||
- `Done` goes through `TaskStateService.ForceSetStatusAsync` (the same unconditional write the
|
||||
UI's "set status freely" affordance uses) but is **refused** for a task with an active
|
||||
worktree, since that would skip `review_task`'s merge.
|
||||
|
||||
**`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
|
||||
are reported in `ReviewTaskResult`.
|
||||
|
||||
**`PreviewMerge`** — non-destructive `git merge-tree --write-tree` mergeability check for one
|
||||
task's worktree branch against `targetBranch` (default: the repo's current branch). Returns
|
||||
status / conflictFiles / changedFileCount / `behind`. Unlike `TaskMergeService.PreviewAsync`'s
|
||||
silent *"unavailable"*, this **throws a clear error** when the task has no worktree, the
|
||||
worktree isn't `Active`, or the list's working dir is missing.
|
||||
|
||||
**`PreviewMergeSet`** — same preview for a batch, plus a file→tasks overlap report built from
|
||||
each task's own diff-stat. ⚠️ That overlap report is a **same-file-name hint only** — it is
|
||||
blind to cross-file collisions (e.g. the CS0103 case that motivated it). A task that fails to
|
||||
preview gets `error` set and is excluded from the overlap instead of aborting the batch.
|
||||
|
||||
**`RevertMerge`** — undoes a previously merged task's merge commit on `targetBranch` via
|
||||
`git revert -m 1`. Always a **new commit**, never a reset/rewrite, because the target working
|
||||
directory is shared with other concurrent sessions.
|
||||
- Requires the task to be `Done` with a `Merged` worktree carrying a recorded
|
||||
`WorktreeEntity.MergeCommit`. A task merged before that field existed has none and is
|
||||
**refused rather than guessed** via `git log`.
|
||||
- On success the task returns to `WaitingForReview` and the worktree moves to `Kept` — not
|
||||
`Active` (its directory/branch are typically already gone from the original merge's cleanup)
|
||||
and not `Merged`/`Discarded` (`WorktreeMaintenanceService` sweeps those).
|
||||
- A conflicting revert is aborted immediately; no half-resolved state is left in the tree.
|
||||
|
||||
**`BatchMcpTools`** — best-effort loops over the `ExternalMcpService` single-entity methods.
|
||||
**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**.
|
||||
|
||||
**`GetTaskLog`** — latest run's log, tail-capped at 256 KB.
|
||||
|
||||
**`WaitForTaskChange(taskIds, timeoutSeconds = 60)`** — blocks until any given task leaves
|
||||
`Queued`/`Running`, or times out. Returns immediately for a task already outside those two
|
||||
(unknown ids reported as status `"NotFound"`, also immediate).
|
||||
- Implemented as an **async DB poll** (short-lived `DbContext` per check, 500 ms delay, no
|
||||
held connection, no busy loop) rather than hooking `HubBroadcaster` — deliberately isolated
|
||||
so it can't regress the existing broadcast callers.
|
||||
- `timeoutSeconds` is clamped server-side to `TaskWaitMcpTools.MaxTimeoutSeconds` (170 s),
|
||||
comfortably under the list handler's `MCP_TOOL_TIMEOUT` (200 s, see
|
||||
`InteractiveLaunchSpecService`), so the tool reports `timedOut: true` instead of racing the
|
||||
client's own abort.
|
||||
- Replaced the list handler's old "sleep + poll `get_task` in a loop" Phase 3 instruction.
|
||||
|
||||
**`AttachmentMcpTools`** — re-attaching the same `fileName` overwrites. Add/remove refuse on a
|
||||
`Running` task.
|
||||
|
||||
**`GetDailyPrepCandidates`** — Idle, non-blocked tasks in a git repo **not** excluded by
|
||||
`AppSettings.ReportExcludedPaths` and not already `IsMyDay`, plus the current Idle MyDay tasks
|
||||
and `maxTasks` (= `DailyPrepMaxTasks`). Repo-exclusion logic lives in the `DailyPrepFilter`
|
||||
helper in the same file.
|
||||
|
||||
**`SetMyDay`** — sets `IsMyDay` (+ optional `SortOrder`). A server-side cap-guard rejects
|
||||
turning on MyDay beyond `DailyPrepMaxTasks` open (Idle) MyDay tasks.
|
||||
|
||||
## Model / max-turns on task creation
|
||||
|
||||
Task-generating tools (`AddTask`, planning `CreateChildTask`, `SuggestImprovement`) accept an
|
||||
optional `model`, alias-validated via `ModelRegistry.NormalizeAlias` (`haiku`/`sonnet`/`opus`,
|
||||
blank = inherit), so Claude can assign the cheapest capable model at creation time — the
|
||||
planning/system/improvement prompts instruct it to do so, using
|
||||
`ModelRegistry.ByCostAscending` as the cost order.
|
||||
|
||||
Planning's `CreateChildTask` **additionally** accepts an optional `maxTurns` (positive int;
|
||||
`0`/negative rejected with `ArgumentException`, null = inherit list/global default) so the
|
||||
planner can raise the turn budget for a subtask it knows will run long.
|
||||
`SuggestImprovement` and `AddTask` do **not** expose it.
|
||||
Reference in New Issue
Block a user