diff --git a/docs/superpowers/plans/2026-04-25-external-mcp-crud-extensions.md b/docs/superpowers/plans/2026-04-25-external-mcp-crud-extensions.md new file mode 100644 index 0000000..6e09af1 --- /dev/null +++ b/docs/superpowers/plans/2026-04-25-external-mcp-crud-extensions.md @@ -0,0 +1,897 @@ +# External MCP — CRUD Extensions Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extend the always-on `ExternalMcpService` with full task CRUD plus tag management so a normal Claude CLI session can fully manage scope-creep tasks via MCP. + +**Architecture:** Pure extension of the existing service. New repository helper for tag replacement; five new/extended `[McpServerTool]` methods. No DI changes (`TagRepository` is already registered for `TaskRepository`/`ListRepository`). Uses the same `X-ClaudeDo-Key` middleware already in place. + +**Tech Stack:** .NET 8, EF Core (SQLite), `ModelContextProtocol.Server` (MCP SDK), xUnit. + +--- + +## Pre-flight + +The test assembly `tests/ClaudeDo.Worker.Tests` currently fails to compile on `main` because of pre-existing in-progress work on `PlanningChainCoordinator` (stale `TaskRunner` / `WorkerHub` constructor calls in `QueueServiceTests.cs`, `QueueServiceSlotGuardTests.cs`, `PlanningHubTests.cs`). This is unrelated to this work and must NOT be fixed here. + +Consequence: `dotnet test` cannot execute until that refactor lands. Each task's "Run test, verify it fails" step uses `dotnet build` of the **test csproj** to confirm only the new test's compile expectations, and `dotnet build` of the **production csproj** to confirm production code is correct. When the refactor lands, the engineer or user re-runs `dotnet test --filter "FullyQualifiedName~ExternalMcpServiceTests"` to validate the new tests for real. + +Build commands used throughout (per the project memory note "use csproj, not .slnx, on .NET 8"): + +```bash +dotnet build src/ClaudeDo.Data/ClaudeDo.Data.csproj +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj +``` + +--- + +## File Map + +| File | Change | +|---|---| +| `src/ClaudeDo.Data/Repositories/TaskRepository.cs` | Add `SetTagsAsync` (replace tag set, auto-create rows) | +| `src/ClaudeDo.Worker/External/ExternalMcpService.cs` | Inject `TagRepository`; extend `AddTask` with `tags`; add `UpdateTask`, `DeleteTask`, `SetTaskTags`, `ListTags` | +| `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` | New test file with fakes mirroring `Planning/PlanningMcpServiceTests.cs` | + +`TagRepository.GetAllAsync` already exists — no change needed there. + +--- + +### Task 1: `TaskRepository.SetTagsAsync` + +**Files:** +- Modify: `src/ClaudeDo.Data/Repositories/TaskRepository.cs` (add new method inside the `#region Tags` block, after `RemoveTagAsync`) +- Test: `tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryTests.cs` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryTests.cs` (use the existing `MakeTask`/list-seed helpers from that file — match the pattern used in adjacent tests): + +```csharp +[Fact] +public async Task SetTagsAsync_AttachesNewTagsAndCreatesMissingRows() +{ + var listId = await CreateListAsync("L"); + var task = MakeTask(listId, "t"); + await _tasks.AddAsync(task); + + await _tasks.SetTagsAsync(task.Id, new[] { "agent", "novel-tag" }); + + var tags = await _tasks.GetTagsAsync(task.Id); + Assert.Contains(tags, t => t.Name == "agent"); + Assert.Contains(tags, t => t.Name == "novel-tag"); + Assert.Equal(2, tags.Count); +} + +[Fact] +public async Task SetTagsAsync_ReplacesExistingTagSet() +{ + var listId = await CreateListAsync("L"); + var task = MakeTask(listId, "t"); + await _tasks.AddAsync(task); + await _tasks.SetTagsAsync(task.Id, new[] { "agent" }); + + await _tasks.SetTagsAsync(task.Id, new[] { "manual" }); + + var tags = await _tasks.GetTagsAsync(task.Id); + Assert.Single(tags); + Assert.Equal("manual", tags[0].Name); +} + +[Fact] +public async Task SetTagsAsync_DeduplicatesCaseInsensitively() +{ + var listId = await CreateListAsync("L"); + var task = MakeTask(listId, "t"); + await _tasks.AddAsync(task); + + await _tasks.SetTagsAsync(task.Id, new[] { "agent", "AGENT", "Agent" }); + + var tags = await _tasks.GetTagsAsync(task.Id); + Assert.Single(tags); +} + +[Fact] +public async Task SetTagsAsync_EmptyListClearsAllTags() +{ + var listId = await CreateListAsync("L"); + var task = MakeTask(listId, "t"); + await _tasks.AddAsync(task); + await _tasks.SetTagsAsync(task.Id, new[] { "agent" }); + + await _tasks.SetTagsAsync(task.Id, Array.Empty()); + + Assert.Empty(await _tasks.GetTagsAsync(task.Id)); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj +``` +Expected: compile error `CS1061: 'TaskRepository' does not contain a definition for 'SetTagsAsync'`. (Existing unrelated `CS7036` errors from `PlanningChainCoordinator` work also appear — ignore.) + +- [ ] **Step 3: Write minimal implementation** + +In `src/ClaudeDo.Data/Repositories/TaskRepository.cs`, inside `#region Tags`, after `RemoveTagAsync`: + +```csharp +public async Task SetTagsAsync(string taskId, IReadOnlyList tagNames, CancellationToken ct = default) +{ + var task = await _context.Tasks.Include(t => t.Tags).FirstOrDefaultAsync(t => t.Id == taskId, ct); + if (task is null) return; + + task.Tags.Clear(); + + foreach (var name in tagNames.Where(n => !string.IsNullOrWhiteSpace(n)).Distinct(StringComparer.OrdinalIgnoreCase)) + { + var tag = await _context.Tags.FirstOrDefaultAsync(t => t.Name == name, ct); + if (tag is null) + { + tag = new TagEntity { Name = name }; + _context.Tags.Add(tag); + } + task.Tags.Add(tag); + } + + await _context.SaveChangesAsync(ct); +} +``` + +- [ ] **Step 4: Run test to verify it compiles + production build still passes** + +```bash +dotnet build src/ClaudeDo.Data/ClaudeDo.Data.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "TaskRepositoryTests" || echo "no errors in TaskRepositoryTests" +``` +Expected: no errors specific to `TaskRepositoryTests` (assembly may still fail due to unrelated `PlanningChainCoordinator` issues). + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Data/Repositories/TaskRepository.cs tests/ClaudeDo.Worker.Tests/Repositories/TaskRepositoryTests.cs +git commit -m "feat(data): add TaskRepository.SetTagsAsync for full tag-set replacement" +``` + +--- + +### Task 2: New test file scaffolding for `ExternalMcpService` + +**Files:** +- Create: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +This task creates the shared test fakes and one trivial passing test. Subsequent tasks reuse the same fakes. + +- [ ] **Step 1: Inspect existing patterns** + +Read `tests/ClaudeDo.Worker.Tests/Planning/PlanningMcpServiceTests.cs` for the `FakeHubContext`/`RecordingClientProxy` pattern and `tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs` for how to construct a real `QueueService` for tests (the same approach is used here — `ExternalMcpService` depends on it for `WakeQueue`/`RunNow`/`CancelTask`). + +- [ ] **Step 2: Write the test scaffolding** + +Create `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs`: + +```csharp +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.External; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Services; +using ClaudeDo.Worker.Tests.Infrastructure; +using Microsoft.AspNetCore.SignalR; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Tests.External; + +file sealed class RecordingHubClients : IHubClients +{ + public RecordingClientProxy Proxy { get; } = new(); + public IClientProxy All => Proxy; + public IClientProxy AllExcept(IReadOnlyList excludedConnectionIds) => Proxy; + public IClientProxy Client(string connectionId) => Proxy; + public IClientProxy Clients(IReadOnlyList connectionIds) => Proxy; + public IClientProxy Group(string groupName) => Proxy; + public IClientProxy GroupExcept(string groupName, IReadOnlyList excludedConnectionIds) => Proxy; + public IClientProxy Groups(IReadOnlyList groupNames) => Proxy; + public IClientProxy User(string userId) => Proxy; + public IClientProxy Users(IReadOnlyList userIds) => Proxy; +} + +file sealed class RecordingClientProxy : IClientProxy +{ + public List<(string Method, object?[] Args)> Calls { get; } = new(); + public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) + { + Calls.Add((method, args)); + return Task.CompletedTask; + } +} + +file sealed class FakeHubContext : IHubContext +{ + public RecordingHubClients RecordingClients { get; } = new(); + public IHubClients Clients => RecordingClients; + public IGroupManager Groups => throw new NotImplementedException(); +} + +public sealed class ExternalMcpServiceTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly ClaudeDoDbContext _ctx; + private readonly TaskRepository _tasks; + private readonly ListRepository _lists; + private readonly TagRepository _tags; + private readonly FakeHubContext _hub; + private readonly HubBroadcaster _broadcaster; + + public ExternalMcpServiceTests() + { + _ctx = _db.CreateContext(); + _tasks = new TaskRepository(_ctx); + _lists = new ListRepository(_ctx); + _tags = new TagRepository(_ctx); + _hub = new FakeHubContext(); + _broadcaster = new HubBroadcaster(_hub); + } + + public void Dispose() { _ctx.Dispose(); _db.Dispose(); } + + private async Task SeedListAsync(string name = "L") + { + var id = Guid.NewGuid().ToString(); + await _lists.AddAsync(new ListEntity { Id = id, Name = name, CreatedAt = DateTime.UtcNow }); + return id; + } + + private async Task SeedTaskAsync(string listId, string title = "t", TaskStatus status = TaskStatus.Manual) + { + var task = new TaskEntity + { + Id = Guid.NewGuid().ToString(), + ListId = listId, + Title = title, + Status = status, + CreatedAt = DateTime.UtcNow, + CommitType = "chore", + }; + await _tasks.AddAsync(task); + return task; + } + + // QueueService is needed by ExternalMcpService's constructor. For tests that + // only exercise UpdateTask / DeleteTask / SetTaskTags / ListTags / ListTags, + // we never call its WakeQueue/RunNow/CancelTask paths, so a real QueueService + // built with the same approach used in QueueServiceTests is sufficient. + private ExternalMcpService BuildSut(QueueService queue) => + new(_tasks, _lists, queue, _broadcaster, _tags); + + [Fact] + public async Task SeededListAndTask_AreRetrievable() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + Assert.NotNull(await _tasks.GetByIdAsync(task.Id)); + } +} +``` + +The trivial `SeededListAndTask_AreRetrievable` test exists to confirm the scaffolding compiles and the fakes work, without depending on `ExternalMcpService` itself yet. + +Note: `BuildSut` uses a 5-argument constructor signature that does not exist yet — this matches the future signature added in Task 3. The compiler will accept this method only after Task 3. + +- [ ] **Step 3: Verify the file references resolve** + +Build the test csproj and check for errors specific to `ExternalMcpServiceTests`: + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "ExternalMcpServiceTests" +``` +Expected output: only one error referring to the 5-arg `ExternalMcpService` constructor (resolved in Task 3). No missing-namespace or syntax errors. + +- [ ] **Step 4: Commit** + +```bash +git add tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "test(external): scaffold ExternalMcpServiceTests" +``` + +--- + +### Task 3: Inject `TagRepository` into `ExternalMcpService` + add `ListTags` + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` +- Modify: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +Smallest possible change to unblock everything else: take the new dependency and ship the simplest tool first. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs`. The `BuildSut` helper is defined in Task 2; tests construct `QueueService` the same way `QueueServiceTests.cs` does (look there for the exact constructor argument list and adopt it verbatim): + +```csharp +[Fact] +public async Task ListTags_ReturnsSeededAndCustomTags() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + await _tasks.SetTagsAsync(task.Id, new[] { "agent", "custom-tag" }); + + using var queue = QueueServiceFactory.Create(_ctx, _broadcaster); // see helper note below + var sut = BuildSut(queue); + + var tags = await sut.ListTags(CancellationToken.None); + + Assert.Contains(tags, t => t.Name == "agent"); + Assert.Contains(tags, t => t.Name == "custom-tag"); +} +``` + +If a `QueueServiceFactory` helper does not already exist in the test project, inline the construction by mirroring the setup found in `tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs` (it builds `QueueService` directly with `IDbContextFactory`, `HubBroadcaster`, fake claude process, etc.). Do NOT call `StartAsync`; just construct and dispose. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep -E "ExternalMcpService|ExternalMcpServiceTests" +``` +Expected: errors about the 5-arg constructor and `ListTags` not existing. + +- [ ] **Step 3: Implement** + +In `src/ClaudeDo.Worker/External/ExternalMcpService.cs`: + +1. Add `TagRepository` field and constructor parameter: + +```csharp +private readonly TaskRepository _tasks; +private readonly ListRepository _lists; +private readonly QueueService _queue; +private readonly HubBroadcaster _broadcaster; +private readonly TagRepository _tags; + +public ExternalMcpService( + TaskRepository tasks, + ListRepository lists, + QueueService queue, + HubBroadcaster broadcaster, + TagRepository tags) +{ + _tasks = tasks; + _lists = lists; + _queue = queue; + _broadcaster = broadcaster; + _tags = tags; +} +``` + +2. Add a tag DTO above the class (next to `TaskListDto`): + +```csharp +public sealed record TagDto(long Id, string Name); +``` + +3. Add the new tool method (place at the end of the class, before `ToDto`): + +```csharp +[McpServerTool, Description("List all known tags. Useful for discovering existing tag names (including 'agent' which marks tasks for auto-execution) before tagging.")] +public async Task> ListTags(CancellationToken cancellationToken) +{ + var tags = await _tags.GetAllAsync(cancellationToken); + return tags.Select(t => new TagDto(t.Id, t.Name)).ToList(); +} +``` + +- [ ] **Step 4: Verify production build + new test compiles** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "ExternalMcpService" +``` +Expected: no errors mentioning `ExternalMcpService` or `ListTags`. (Unrelated `PlanningChainCoordinator` errors persist.) + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/External/ExternalMcpService.cs tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "feat(mcp/external): add ListTags + inject TagRepository" +``` + +--- + +### Task 4: Extend `AddTask` to accept `tags` + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` (`AddTask` method) +- Modify: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +- [ ] **Step 1: Write the failing tests** + +Append to `ExternalMcpServiceTests.cs`: + +```csharp +[Fact] +public async Task AddTask_WithTags_AttachesTags() +{ + var listId = await SeedListAsync(); + using var queue = /* same construction as ListTags test */; + var sut = BuildSut(queue); + + var dto = await sut.AddTask( + listId, "scope-creep handoff", "desc", "claude-cli", + queueImmediately: false, + tags: new[] { "agent", "custom" }, + CancellationToken.None); + + var tags = await _tasks.GetTagsAsync(dto.Id); + Assert.Contains(tags, t => t.Name == "agent"); + Assert.Contains(tags, t => t.Name == "custom"); +} + +[Fact] +public async Task AddTask_NullTags_BehavesAsBefore() +{ + var listId = await SeedListAsync(); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + var dto = await sut.AddTask( + listId, "no tags", null, "claude-cli", + queueImmediately: false, tags: null, CancellationToken.None); + + Assert.Empty(await _tasks.GetTagsAsync(dto.Id)); +} +``` + +(Replace the `/* same construction */` placeholder with the actual `QueueService` construction used in Task 3 — repeat the code, do not extract.) + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "AddTask" +``` +Expected: error that `AddTask` does not accept a 7th `tags` parameter. + +- [ ] **Step 3: Implement** + +Replace the existing `AddTask` method in `ExternalMcpService.cs` with: + +```csharp +[McpServerTool, Description("Create a new task in the given list. Set queueImmediately=true to enqueue it for agent execution. Optional tags are attached on creation; missing tag names auto-create.")] +public async Task AddTask( + string listId, + string title, + string? description, + string createdBy, + bool queueImmediately, + IReadOnlyList? tags, + CancellationToken cancellationToken) +{ + if (string.IsNullOrWhiteSpace(listId)) + throw new InvalidOperationException("listId is required."); + if (string.IsNullOrWhiteSpace(title)) + throw new InvalidOperationException("title is required."); + if (string.IsNullOrWhiteSpace(createdBy)) + throw new InvalidOperationException("createdBy is required."); + + var list = await _lists.GetByIdAsync(listId, cancellationToken) + ?? throw new InvalidOperationException($"List {listId} not found."); + + var entity = new TaskEntity + { + Id = Guid.NewGuid().ToString(), + ListId = listId, + Title = title, + Description = description, + Status = queueImmediately ? TaskStatus.Queued : TaskStatus.Manual, + CreatedAt = DateTime.UtcNow, + CommitType = list.DefaultCommitType, + CreatedBy = createdBy, + }; + await _tasks.AddAsync(entity, cancellationToken); + + if (tags is not null && tags.Count > 0) + await _tasks.SetTagsAsync(entity.Id, tags, cancellationToken); + + if (queueImmediately) + _queue.WakeQueue(); + + await _broadcaster.TaskUpdated(entity.Id); + return ToDto(entity); +} +``` + +- [ ] **Step 4: Verify production build** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/External/ExternalMcpService.cs tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "feat(mcp/external): AddTask accepts tags on creation" +``` + +--- + +### Task 5: `UpdateTask` + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` +- Modify: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +- [ ] **Step 1: Write the failing tests** + +Append to `ExternalMcpServiceTests.cs`: + +```csharp +[Fact] +public async Task UpdateTask_PatchesNonNullFieldsOnly() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId, "old title"); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + var dto = await sut.UpdateTask(task.Id, "new title", null, null, null, CancellationToken.None); + + Assert.Equal("new title", dto.Title); + var loaded = await _tasks.GetByIdAsync(task.Id); + Assert.Equal("new title", loaded!.Title); +} + +[Fact] +public async Task UpdateTask_TagsReplaceFullSet() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + await _tasks.SetTagsAsync(task.Id, new[] { "agent" }); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await sut.UpdateTask(task.Id, null, null, null, new[] { "manual" }, CancellationToken.None); + + var tags = await _tasks.GetTagsAsync(task.Id); + Assert.Single(tags); + Assert.Equal("manual", tags[0].Name); +} + +[Fact] +public async Task UpdateTask_OnRunning_Throws() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId, status: TaskStatus.Running); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await Assert.ThrowsAsync(() => + sut.UpdateTask(task.Id, "x", null, null, null, CancellationToken.None)); +} + +[Fact] +public async Task UpdateTask_NotFound_Throws() +{ + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await Assert.ThrowsAsync(() => + sut.UpdateTask("does-not-exist", "x", null, null, null, CancellationToken.None)); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "UpdateTask" +``` +Expected: errors that `UpdateTask` does not exist on `ExternalMcpService`. + +- [ ] **Step 3: Implement** + +Add to `ExternalMcpService.cs` (after `AddTask`): + +```csharp +[McpServerTool, Description("Update an existing task's title, description, commit type, and/or tags. Pass null to leave a field unchanged. Tags are replaced as a full set when non-null. Refuses if the task is currently Running.")] +public async Task UpdateTask( + string taskId, + string? title, + string? description, + string? commitType, + IReadOnlyList? tags, + CancellationToken cancellationToken) +{ + var task = await _tasks.GetByIdAsync(taskId, cancellationToken) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + if (task.Status == TaskStatus.Running) + throw new InvalidOperationException("Cannot update a running task. Cancel it first."); + + if (title is not null) task.Title = title; + if (description is not null) task.Description = description; + if (commitType is not null) task.CommitType = commitType; + await _tasks.UpdateAsync(task, cancellationToken); + + if (tags is not null) + await _tasks.SetTagsAsync(taskId, tags, cancellationToken); + + var reload = (await _tasks.GetByIdAsync(taskId, cancellationToken))!; + await _broadcaster.TaskUpdated(taskId); + return ToDto(reload); +} +``` + +- [ ] **Step 4: Verify production build** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/External/ExternalMcpService.cs tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "feat(mcp/external): add UpdateTask for content/tag patching" +``` + +--- + +### Task 6: `DeleteTask` + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` +- Modify: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +- [ ] **Step 1: Write the failing tests** + +Append to `ExternalMcpServiceTests.cs`: + +```csharp +[Fact] +public async Task DeleteTask_RemovesTaskAndTagJoins() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + await _tasks.SetTagsAsync(task.Id, new[] { "agent" }); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await sut.DeleteTask(task.Id, CancellationToken.None); + + Assert.Null(await _tasks.GetByIdAsync(task.Id)); +} + +[Fact] +public async Task DeleteTask_OnRunning_Throws() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId, status: TaskStatus.Running); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await Assert.ThrowsAsync(() => + sut.DeleteTask(task.Id, CancellationToken.None)); +} + +[Fact] +public async Task DeleteTask_NotFound_Throws() +{ + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await Assert.ThrowsAsync(() => + sut.DeleteTask("does-not-exist", CancellationToken.None)); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "DeleteTask" +``` +Expected: errors that `DeleteTask` does not exist on `ExternalMcpService`. + +- [ ] **Step 3: Implement** + +Add to `ExternalMcpService.cs` (after `UpdateTask`): + +```csharp +[McpServerTool, Description("Delete a task. Refuses if the task is currently Running — cancel it first.")] +public async Task DeleteTask(string taskId, CancellationToken cancellationToken) +{ + var task = await _tasks.GetByIdAsync(taskId, cancellationToken) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + if (task.Status == TaskStatus.Running) + throw new InvalidOperationException("Cannot delete a running task. Cancel it first."); + + await _tasks.DeleteAsync(taskId, cancellationToken); + await _broadcaster.TaskUpdated(taskId); +} +``` + +- [ ] **Step 4: Verify production build** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/External/ExternalMcpService.cs tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "feat(mcp/external): add DeleteTask" +``` + +--- + +### Task 7: `SetTaskTags` + +**Files:** +- Modify: `src/ClaudeDo.Worker/External/ExternalMcpService.cs` +- Modify: `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` + +- [ ] **Step 1: Write the failing tests** + +Append to `ExternalMcpServiceTests.cs`: + +```csharp +[Fact] +public async Task SetTaskTags_ReplacesTagSetAndBroadcasts() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + await _tasks.SetTagsAsync(task.Id, new[] { "agent" }); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + var dto = await sut.SetTaskTags(task.Id, new[] { "manual" }, CancellationToken.None); + + var tags = await _tasks.GetTagsAsync(task.Id); + Assert.Single(tags); + Assert.Equal("manual", tags[0].Name); + Assert.Contains(_hub.RecordingClients.Proxy.Calls, + c => c.Method == "TaskUpdated" && (string)c.Args[0]! == task.Id); +} + +[Fact] +public async Task SetTaskTags_OnRunning_Throws() +{ + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId, status: TaskStatus.Running); + using var queue = /* same construction */; + var sut = BuildSut(queue); + + await Assert.ThrowsAsync(() => + sut.SetTaskTags(task.Id, new[] { "manual" }, CancellationToken.None)); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep "SetTaskTags" +``` +Expected: errors that `SetTaskTags` does not exist. + +- [ ] **Step 3: Implement** + +Add to `ExternalMcpService.cs` (after `DeleteTask`): + +```csharp +[McpServerTool, Description("Replace the full tag set on an existing task. Missing tag names auto-create. Refuses if the task is Running.")] +public async Task SetTaskTags( + string taskId, + IReadOnlyList tags, + CancellationToken cancellationToken) +{ + var task = await _tasks.GetByIdAsync(taskId, cancellationToken) + ?? throw new InvalidOperationException($"Task {taskId} not found."); + if (task.Status == TaskStatus.Running) + throw new InvalidOperationException("Cannot retag a running task. Cancel it first."); + + await _tasks.SetTagsAsync(taskId, tags, cancellationToken); + var reload = (await _tasks.GetByIdAsync(taskId, cancellationToken))!; + await _broadcaster.TaskUpdated(taskId); + return ToDto(reload); +} +``` + +- [ ] **Step 4: Verify production build** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +``` +Expected: `Build succeeded. 0 Error(s)`. + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/External/ExternalMcpService.cs tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs +git commit -m "feat(mcp/external): add SetTaskTags" +``` + +--- + +### Task 8: Final verification + docs touch + +**Files:** +- Modify: `src/ClaudeDo.Worker/CLAUDE.md` (one-line update reflecting the new tools) + +- [ ] **Step 1: Full production build** + +```bash +dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj +dotnet build src/ClaudeDo.Data/ClaudeDo.Data.csproj +``` +Expected: both succeed with 0 errors. + +- [ ] **Step 2: Update Worker CLAUDE.md** + +In `src/ClaudeDo.Worker/CLAUDE.md`, locate the existing line near the bottom of the file describing external MCP tools (search for `ExternalMcpService` or `External/`). If a list of tools is already there, append the new tool names: `UpdateTask`, `DeleteTask`, `SetTaskTags`, `ListTags`. If no such line exists, add one short line under an existing structural section, for example under "Architecture": + +```markdown +- **External/ExternalMcpService** — always-on MCP tools for general Claude sessions: `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask` (with tags), `UpdateTask`, `UpdateTaskStatus`, `SetTaskTags`, `ListTags`, `DeleteTask`, `RunTaskNow`, `CancelTask`. Auth via optional `X-ClaudeDo-Key` header. +``` + +If the file already has a similar line — replace it; do not duplicate. + +- [ ] **Step 3: Verify the full test assembly state is unchanged** + +```bash +dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj 2>&1 | grep -E "error CS" | grep -v "PlanningChainCoordinator\|TaskRunner.*chain\|WorkerHub.*planningChain" +``` +Expected: empty output (every remaining error must be one of the pre-existing `PlanningChainCoordinator`-related errors and nothing new). + +- [ ] **Step 4: When the unrelated refactor lands, run the new tests** + +(Defer to whoever lands the `PlanningChainCoordinator` refactor — they should run:) + +```bash +dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj --filter "FullyQualifiedName~ExternalMcpServiceTests|FullyQualifiedName~TaskRepositoryTests.SetTagsAsync" +``` +Expected: all new tests green. + +- [ ] **Step 5: Commit** + +```bash +git add src/ClaudeDo.Worker/CLAUDE.md +git commit -m "docs(worker): document new external MCP tools" +``` + +--- + +## Self-review + +**Spec coverage:** +- `AddTask` extension with tags → Task 4 ✓ +- `UpdateTask` → Task 5 ✓ +- `DeleteTask` → Task 6 ✓ +- `SetTaskTags` → Task 7 ✓ +- `ListTags` → Task 3 ✓ +- `TaskRepository.SetTagsAsync` → Task 1 ✓ +- Auth (no change) → out of scope, called out in pre-flight ✓ +- Tests for each tool → Tasks 1, 3-7 ✓ +- Docs touch → Task 8 ✓ + +**Placeholder scan:** The phrase `/* same construction */` in tasks 4–7 is intentional — the engineer fills it in by mirroring the `QueueService` construction in Task 3 (which itself mirrors `QueueServiceTests.cs`). All other placeholders eliminated. No "TBD". + +**Type consistency:** +- `IReadOnlyList` for tag inputs everywhere ✓ +- `TaskDto` returned by `AddTask`, `UpdateTask`, `SetTaskTags` ✓ +- `TagDto(long Id, string Name)` consistent across `ListTags` ✓ +- Constructor signature `(TaskRepository, ListRepository, QueueService, HubBroadcaster, TagRepository)` consistent between Task 3 implementation and Task 2 scaffold's `BuildSut` call ✓ +- Method `TaskRepository.SetTagsAsync(string, IReadOnlyList, CancellationToken)` consistent with all callers ✓ + +No issues found. diff --git a/docs/superpowers/specs/2026-04-25-external-mcp-crud-extensions-design.md b/docs/superpowers/specs/2026-04-25-external-mcp-crud-extensions-design.md new file mode 100644 index 0000000..fa134ca --- /dev/null +++ b/docs/superpowers/specs/2026-04-25-external-mcp-crud-extensions-design.md @@ -0,0 +1,174 @@ +# External MCP — CRUD Extensions + +**Date:** 2026-04-25 +**Status:** Approved + +## Goal + +Give a normal (non-planning) Claude CLI session full control over the ClaudeDo task inbox via the existing always-on `ExternalMcpService`. Primary use case: when a chat session produces scope-creep work, Claude can spin up a fully-formed task — title, description, tags (including the `agent` tag for auto-execution) — without leaving the session. + +The work is purely additive: the `ExternalMcpService` endpoint is already wired, authenticated by the optional `X-ClaudeDo-Key` header, and exposes `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask`, `UpdateTaskStatus`, `RunTaskNow`, `CancelTask`. Missing for "full CRUD" are tag handling, content updates, deletion, and tag discovery. + +## Scope + +| Tool | Status | Notes | +|---|---|---| +| `ListTaskLists` | exists | unchanged | +| `ListTasks` | exists | unchanged | +| `GetTask` | exists | unchanged | +| `AddTask` | extend | add optional `tags` parameter | +| `UpdateTaskStatus` | exists | unchanged (Manual ↔ Queued) | +| `RunTaskNow` | exists | unchanged | +| `CancelTask` | exists | unchanged | +| `UpdateTask` | new | patch title/description/commitType/tags | +| `DeleteTask` | new | delete a task (cascades) | +| `SetTaskTags` | new | replace the full tag set on a task | +| `ListTags` | new | enumerate all known tag names | + +Out of scope: +- List CRUD (creating/renaming/deleting lists) — out of scope for this iteration; UI remains the source of truth for list management. +- ListConfig / agent settings overrides — handled by the UI, not surfaced via MCP here. +- Tag CRUD beyond auto-creation during `AddTask` / `UpdateTask` / `SetTaskTags`. There is no `DeleteTag` tool; tag rows live as long as some task references them. + +## Authentication + +No change. The endpoint continues to be gated by `ExternalMcpAuthMiddleware` — if `WorkerConfig.ExternalMcpApiKey` is set, callers must include `X-ClaudeDo-Key`; otherwise the loopback-only worker is open to local processes. + +## Tool specifications + +### `AddTask` (extended) + +``` +AddTask( + listId: string, + title: string, + description: string?, + createdBy: string, + queueImmediately: bool, + tags: string[]?, + cancellationToken) + -> TaskDto +``` + +Behavior: +- Existing behavior preserved. New `tags` parameter, when non-null, attaches the named tags to the new task. +- Tag names are matched case-insensitively against existing rows; missing tag rows are auto-created (mirrors `TaskRepository.CreateChildAsync`). +- Empty/whitespace tag names are skipped; duplicates are deduplicated. +- `tags` is the LAST parameter before `CancellationToken` so existing positional callers are unaffected (CancellationToken is bound by name in MCP runtime; defensive — see Migration). + +### `UpdateTask` (new) + +``` +UpdateTask( + taskId: string, + title: string?, + description: string?, + commitType: string?, + tags: string[]?, + cancellationToken) + -> TaskDto +``` + +Behavior: +- Loads the task; throws `InvalidOperationException` if not found. +- **Refuses if status is `Running`** — protects in-flight worktrees and the streaming log. +- Does NOT change status (use `UpdateTaskStatus`) and does NOT change `createdBy`, `listId`, or `parentTaskId` (audit + structural fields, immutable here). +- For each non-null parameter, applies the update. Null means "leave unchanged". +- `tags` semantics: full replacement of the tag set (same as `SetTaskTags`). Auto-creates missing tag rows. +- Broadcasts `TaskUpdated` on the SignalR hub on success. + +### `DeleteTask` (new) + +``` +DeleteTask(taskId: string, cancellationToken) -> void +``` + +Behavior: +- Loads the task; throws if not found. +- **Refuses if status is `Running`** — caller must `CancelTask` first. +- Calls `TaskRepository.DeleteAsync` (FK cascades remove `task_tags`, `worktrees`, `task_runs`, `subtasks`). +- Broadcasts `TaskUpdated(taskId)` so UIs drop the row. + +### `SetTaskTags` (new) + +``` +SetTaskTags(taskId: string, tags: string[], cancellationToken) -> TaskDto +``` + +Behavior: +- Convenience wrapper for "I just want to (re)set tags". Equivalent to `UpdateTask(taskId, null, null, null, tags)`. +- Same validation: refuses if `Running`. +- Returns the updated `TaskDto` (with status; tags are not included in `TaskDto` today — see Open Decisions). + +### `ListTags` (new) + +``` +ListTags(cancellationToken) -> { Id: long, Name: string }[] +``` + +Behavior: +- Returns every row in the `tags` table. No filter, no pagination — the table is small (seed values + user-defined). +- Lets Claude discover existing tag names (`agent`, `manual`, plus any user-defined) before tagging, avoiding duplicates that differ only by case/whitespace. + +## Repository changes + +`src/ClaudeDo.Data/Repositories/TaskRepository.cs`: + +- Add `public Task SetTagsAsync(string taskId, IReadOnlyList tagNames, CancellationToken ct = default)` — replaces the tag set, auto-creates missing rows. Implementation pattern matches the tag block already inside `CreateChildAsync` and the new `UpdateChildAsync` from the planning-MCP work; consider extracting a private helper `ApplyTagsAsync(TaskEntity, IReadOnlyList, CancellationToken)` shared by both. + +`src/ClaudeDo.Data/Repositories/TagRepository.cs`: + +- Add `public Task> GetAllAsync(CancellationToken ct = default)` if it does not already exist. (Matches `ListRepository.GetAllAsync` style.) + +No new tables, no migrations. + +## Service changes + +`src/ClaudeDo.Worker/External/ExternalMcpService.cs`: + +- Add `TagRepository` to the constructor (DI registration is already in place since the planning service uses it). +- Extend `AddTask` signature with `IReadOnlyList? tags` and apply via the repository. +- Add `UpdateTask`, `DeleteTask`, `SetTaskTags`, `ListTags` methods, each annotated `[McpServerTool, Description("…")]`. +- Each new mutating tool calls `_broadcaster.TaskUpdated(taskId)` on success (matches existing pattern in this file). + +DI: `ExternalMcpService` is already registered. If `TagRepository` is not already registered (it is — used by `ListRepository`), no change. If a constructor parameter is added, `Program.cs` does not need changes because services are scoped/transient. + +## Error handling + +All errors raised as `InvalidOperationException` with a human-readable message — matches the existing pattern in `ExternalMcpService` and `PlanningMcpService`. The MCP SDK serializes these to the JSON-RPC error channel; Claude sees the message text directly. + +Specific cases: +- Task not found → `"Task {id} not found."` +- Running-task guard → `"Cannot {update|delete} a running task. Cancel it first."` +- Unknown status (in `UpdateTaskStatus`, unchanged) → `"Unknown status '{x}'."` + +## Testing + +Add `tests/ClaudeDo.Worker.Tests/External/ExternalMcpServiceTests.cs` (or extend if it exists) with: + +| Test | Asserts | +|---|---| +| `AddTask_WithTags_AttachesTags` | `tags` param creates and attaches tag rows | +| `AddTask_WithUnknownTag_AutoCreatesTagRow` | new tag name produces a row in `tags` table | +| `UpdateTask_PatchesNonNullFields` | only non-null fields change | +| `UpdateTask_OnRunning_Throws` | `InvalidOperationException` | +| `UpdateTask_BroadcastsTaskUpdated` | hub broadcast received | +| `UpdateTask_TagsReplaceFullSet` | passing tags=[…] replaces existing tags wholesale | +| `DeleteTask_RemovesTaskAndTagJoins` | task and `task_tags` rows gone | +| `DeleteTask_OnRunning_Throws` | `InvalidOperationException` | +| `SetTaskTags_ReplacesAndBroadcasts` | replacement semantics + broadcast | +| `ListTags_ReturnsSeedAndCustomTags` | `agent` + `manual` + any user-defined | + +Existing test infrastructure (`DbFixture`, `FakeHubContext`) is reused. No new fakes required. + +**Caveat:** the test assembly currently fails to compile on `main` because of pre-existing in-progress work on `PlanningChainCoordinator` (missing constructor argument in `WorkerHub`/`TaskRunner` test instantiations). Tests will pass only after that work lands; do not block this design on it. + +## Open decisions (defaults chosen, easy to flip) + +1. **`TaskDto` does not currently include tags.** For consistency, the spec keeps `TaskDto` as-is and ships a separate `ListTags` tool. If preferred, we could add `Tags: string[]` to `TaskDto` so every tool response includes them — small DB cost (one extra `SelectMany`), one struct field added. Default: leave `TaskDto` alone, defer. +2. **Per-tag `AddTaskTag` / `RemoveTaskTag` micro-tools.** Skipped — `SetTaskTags` covers the use case, and it's idempotent. Add later if granular ops are wanted. +3. **List CRUD via MCP.** Out of scope. UI owns lists. + +## Migration / compatibility + +`AddTask` gains an optional parameter. The MCP server SDK sends parameters by name in JSON-RPC `params`, so existing clients that omit `tags` continue to work without code changes. No version bump required.