Files
ClaudeDo/docs/explore-notes/external-mcp.md
T
mika kuns e8e2fcec3f docs: update for task-numbers features (Slice 5)
Document task-number allocation in Data layer (TaskEntity.Number,
next_task_number counter, invariants, insert paths).

Update Worker docs to clarify TaskIdResolver wiring (#123 → GUID lookup),
Number in MCP payloads, and correct the 'Two hard conventions' statement
(only the first is test-enforced).

Bump external-mcp.md verified-against commit to 38af549 (Slice 4 merge)
and add new sections on task ID resolution and numbering.

Add Slice 4 visual verification items to open.md (row/detail number display,
worker-log messages).

Verified against:
- src/ClaudeDo.Data/TaskNumberAllocator.cs
- src/ClaudeDo.Data/Repositories/TaskRepository.cs (AddAsync line 20, CreateChildAsync line 276)
- src/ClaudeDo.Worker/External/TaskIdResolver.cs
- src/ClaudeDo.Worker/External/ExternalMcpService.cs (TaskDto/TaskRefDto DTOs)
- tests/ClaudeDo.Worker.Tests/External/ExternalMcpToolSchemaTests.cs (only first convention test-enforced)
2026-08-11 14:26:22 +02:00

256 lines
17 KiB
Markdown

# External MCP tool surface
> **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative.
> Last verified against commit `38af549` (2026-08-11).
> Drift check: `git log --oneline 38af549..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.
## Task ID resolution and numbering
Every tool parameter accepting a task id — `taskId`, `parentId`, `taskIds` arrays, and similar —
is wired through `TaskIdResolver`, which resolves:
- `#123` (with or without the `#` prefix) → look up by `TaskEntity.Number`
- Bare GUID string → use as-is
- Unknown number → `InvalidOperationException` (`no task with number 123`)
This is **not** ambiguous: a GUID is never all-digits, so a pure-integer parameter is always a
number, not a partial GUID. All task-returning tools (`GetTask`, `ListTasks`, `AddTask`,
`BatchGetTasks`, etc.) stamp the resolved task's `Number` in the DTO — both `TaskDto` and the
lean `TaskRefDto` carry an `int Number` field. **Branch names and worktree paths** (`claudedo/{id}`)
continue to use the GUID; the number is a display alias, never the identity.
Every tool description carries a shared boilerplate clause (defined in `McpToolDocs.TaskNumberHint`)
instructing the agent to **refer to tasks as `#<number>` when reporting results to the user**
without this the agent sees the number in every payload but never learns to speak it.
## Conventions
### Test-enforced
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.
### Not test-enforced, but strongly observed
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`).
- 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. **Description style is documented in `McpToolDocs`** (same folder) and shared boilerplate
lives there as `const` strings. Rules: the first sentence says what the tool does *and* when
to reach for it (MCP clients rank tools by that text, so the trigger must not sit behind
return-shape prose); parameters are documented with `[Description]` **on the parameter**, not
in the tool description; result fields appear only where the caller must branch on them
before calling (`isEmpty`, `truncated`, `conflicts`, `available`); no design rationale or
"since this feature was introduced" history.
4. `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`, `ReviewTask`, `RunTaskNow`, `ContinueTask`, `CancelTask`, `DeleteTask`.
(`GetTaskStatusValues` was removed — a whole tool entry for static reference text. `GetTask`'s
description is now the canonical place for what each status means.)
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`, `GetEffectiveRunConfig` |
| `RunHistoryMcpTools` | `ListRuns`, `GetRun`, `GetTaskLog` |
| `AgentMcpTools` | `ListAgents` |
| `LifecycleMcpTools` | `ResetFailedTask` |
| `AppSettingsMcpTools` | `GetAppSettings` (read-only) |
| `TaskWaitMcpTools` | `WaitForTaskChange` |
| `QueueStateMcpTools` | `GetQueueState` |
| `AttachmentMcpTools` | `AddTaskAttachment`, `ListTaskAttachments`, `RemoveTaskAttachment` |
## 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
`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.
**`AddTask`** — always creates the task; also returns `possibleDuplicates` (up to 3, id/title/status
only, no descriptions) — open (non-terminal) tasks in the *same list* whose normalized title
overlaps strongly with the new one. Cheap word-overlap heuristic (`ExternalMcpService`'s
`FindPossibleDuplicatesAsync`/`NormalizeTitleWords`), no embeddings/LLM call, no blocking —
the caller just gets a heads-up to relay. `BatchAddTasks` carries the same field per item.
**`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`.
- A parent's approve also returns `emptyChildren`: the `Done` children about to be unit-merged
whose own review range contributed nothing (computed the same way as `PreviewMerge`'s
`isEmpty`, before the merge starts so it reflects what's about to be approved). Surfaces a
child that reported `CLAUDEDO_BLOCKED` and committed no code — previously that child reached
`Done` and merged silently with `changedFileCount: 0`, indistinguishable from a small-but-real
change. `TaskRefDto.roadblockCount` (on every task-returning tool, stamped by `TaskRunner` from
`result.Blocks.Count`) is the MCP-visible signal for *why* a child is empty.
**`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` / `isEmpty`. Unlike
`TaskMergeService.PreviewAsync`'s silent *"unavailable"*, this **throws a clear error** when the
task has neither an active worktree nor a handler commit range, or the list's working dir is
missing.
- `isEmpty` = the review range contributed nothing — zero files changed against the worktree's
base commit, or (for a worktree-less list-handler host task) `HandlerBaseCommit ==
HandlerHeadCommit`. Distinguishes a genuinely empty branch from one that merely made a small
change (`changedFileCount: 0` alone reads as "tiny", not "nothing to review") — the gap that
let two blocked planning children reach `Done` with unmerged empty branches unnoticed.
- A worktree-less handler task has no separate branch to `merge-tree`-preview (its commits
already sit in `list.WorkingDir`) — `PreviewMergeCoreAsync` falls back to a synthetic `clean`
preview over its own `HandlerBaseCommit..HandlerHeadCommit` diff-stat instead of throwing
"has no worktree".
**`PreviewMergeSet`** — same preview for a batch (each entry also carries `isEmpty`), 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**.
`BatchGetTasks` mirrors `ListTasks`'s `includeDescription` flag (default `false`): a found item's
`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.
**`WaitForTaskChange(taskIds, timeoutSeconds = 60, treatWaitingForChildrenAsBusy = false)`** —
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` (900 s),
comfortably under the `MCP_TOOL_TIMEOUT` (930 s) every ClaudeDo-owned claude launcher sets —
`ClaudeProcess` for headless queue runs, `InteractiveLaunchSpecService` for every embedded
ConPTY session (list handler, planning, interactive resume) — so the tool reports
`timedOut: true` instead of racing the client's own abort. A caller running claude with a
different (or default: 60 s) `MCP_TOOL_TIMEOUT` will still see its own client-side timeout
fire first; the server has no way to detect or compensate for that.
- **`treatWaitingForChildrenAsBusy` pitfall (default `false`, backward-compatible):** a planning
parent with children goes `Running` → `WaitingForChildren` while children are still working,
and by default that already counts as "changed" (it's outside `Queued`/`Running`) — so waiting
on a parent returns immediately even though the unit isn't done. Set the flag to keep polling
through `WaitingForChildren`; the call then only reports changed once the parent reaches
`WaitingForReview` or a terminal status. Does not list or watch the parent's children —
callers still need their own ids for that.
- Replaced the list handler's old "sleep + poll `get_task` in a loop" Phase 3 instruction.
**`GetQueueState()`** — read-only snapshot so a caller doesn't have to infer queue state from
`maxParallelExecutions` or repeated `wait_for_task_change` rounds:
`{ configuredSlots, effectiveSlots, activeSlots: [{ slot, taskId, startedAt }], waitingTaskIds }`.
- `configuredSlots`/`effectiveSlots` reuse `QueueService.GetSlotCountsAsync` — the same
configured-vs-throttled computation `QueueService.ExecuteAsync` uses each tick (see
[usage-monitoring](usage-monitoring.md) for the throttle staging) — so this tool can't drift
from the queue's actual refill decision.
- `activeSlots` reuses `QueueService.GetActive()` (already the source for the Hub's `GetActive`):
`slot` is `"queue"` for a normal queue slot or `"override"` for the single
`run_task_now`/`continue_task` slot.
- `waitingTaskIds` is a fresh read-only query mirroring `QueuePicker.ClaimNextAsync`'s
eligibility filter and order (`Queued`, unblocked, non-manual, due, `sort_order` then
`created_at`) — it does not claim or mutate anything.
**`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.
**`GetEffectiveRunConfig`** — read-only report of what a task will *actually* run with (model,
max turns, effort, permission mode, agent path, whether a system prompt is set, skill names),
each with its source (`task`/`list`/`preset`/`global`); max turns additionally reports the raw
requested value and whether it was clamped to `AppSettings.MaxTurnsCeiling`. Unlike
`GetAppSettings`/`GetTaskConfig` (raw, possibly-unused config values), this goes through the same
`EffectiveRunConfigResolver.Resolve` that `TaskRunner` itself runs with — see
[worker-task-pipeline](./worker-task-pipeline.md)'s model/effort/max-turns section — so it can't
drift from the real run. Reads (not writes) `AppSettingsRepository.GetAsync`, which backfills
`model_presets` on first read after a null column; that backfill is pre-existing shared behavior,
not a new side effect introduced by this tool.
## 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.