feat(worker): accept #123 task numbers as MCP tool input
TaskIdResolver resolves a #123/bare-123 taskId parameter to its GUID before any lookup, across every External/ MCP tool that takes a task id, including the batch tools' id arrays (via delegation to the already-resolving single-entity methods) and update_task's dependsOnTaskId (empty string still passes through unchanged as the clear-link sentinel). An unknown number throws a clear error instead of a silent null. McpToolDocs.TaskNumberHint tells the agent to refer to tasks as #<number> when reporting to the user, added to the description of get_task, list_tasks, add_task, update_task_status and review_task.
This commit is contained in:
@@ -205,6 +205,22 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
Assert.Null(found.TaskFull);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchGetTasks_MixedNumberAndGuidIds_ResolvesBoth()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var a = await SeedTaskAsync(listId);
|
||||
var b = await SeedTaskAsync(listId);
|
||||
var sut = BuildSut();
|
||||
|
||||
var results = await sut.BatchGetTasks(new[] { $"#{a.Number}", b.Id }, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.True(results[0].Found);
|
||||
Assert.Equal(a.Id, results[0].Task!.Id);
|
||||
Assert.True(results[1].Found);
|
||||
Assert.Equal(b.Id, results[1].Task!.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchGetTasks_IncludeDescriptionTrue_ReturnsTaskFull()
|
||||
{
|
||||
|
||||
@@ -229,6 +229,41 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Equal(task.Number, dto.Number);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_ByHashNumber_ResolvesToTask()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.GetTask($"#{task.Number}", CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, dto.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_ByBareNumber_ResolvesToTask()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var task = await SeedTaskAsync(listId);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.GetTask(task.Number.ToString(), CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, dto.Id);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_UnknownNumber_ThrowsWithNumberInMessage()
|
||||
{
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
sut.GetTask("#999999", CancellationToken.None));
|
||||
|
||||
Assert.Contains("999999", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_ReturnsFullDtoIncludingDescription()
|
||||
{
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
using System.ComponentModel;
|
||||
using System.Reflection;
|
||||
using ClaudeDo.Worker.External;
|
||||
using Microsoft.Extensions.AI;
|
||||
@@ -71,4 +72,16 @@ public sealed class ExternalMcpToolSchemaTests
|
||||
// (e.g. namespace/attribute mismatch) and the schema test above would pass vacuously.
|
||||
Assert.True(ExternalToolMethods().Count() > 20);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AtLeastOneTaskIdTool_DescriptionCarriesTaskNumberHint()
|
||||
{
|
||||
// The whole point of task numbers: without this clause the agent never learns to speak
|
||||
// #<number> to the user, even though every DTO already carries it.
|
||||
var hasHint = ExternalToolMethods()
|
||||
.Select(m => m.GetCustomAttribute<DescriptionAttribute>()?.Description ?? "")
|
||||
.Any(d => d.Contains(McpToolDocs.TaskNumberHint.Trim(), StringComparison.Ordinal));
|
||||
|
||||
Assert.True(hasHint, "No external tool description carries McpToolDocs.TaskNumberHint.");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,13 +11,15 @@ public sealed class RunHistoryMcpToolsTests : IDisposable
|
||||
private readonly DbFixture _db = new();
|
||||
private readonly ClaudeDoDbContext _ctx;
|
||||
private readonly TaskRunRepository _runs;
|
||||
private readonly TaskRepository _tasks;
|
||||
private readonly RunHistoryMcpTools _sut;
|
||||
|
||||
public RunHistoryMcpToolsTests()
|
||||
{
|
||||
_ctx = _db.CreateContext();
|
||||
_runs = new TaskRunRepository(_ctx);
|
||||
_sut = new RunHistoryMcpTools(_runs);
|
||||
_tasks = new TaskRepository(_ctx);
|
||||
_sut = new RunHistoryMcpTools(_runs, _tasks);
|
||||
}
|
||||
|
||||
public void Dispose() { _ctx.Dispose(); _db.Dispose(); }
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests.External;
|
||||
|
||||
public sealed class TaskIdResolverTests : IDisposable
|
||||
{
|
||||
private readonly DbFixture _db = new();
|
||||
private readonly ClaudeDoDbContext _ctx;
|
||||
private readonly TaskRepository _tasks;
|
||||
private readonly ListRepository _lists;
|
||||
|
||||
public TaskIdResolverTests()
|
||||
{
|
||||
_ctx = _db.CreateContext();
|
||||
_tasks = new TaskRepository(_ctx);
|
||||
_lists = new ListRepository(_ctx);
|
||||
}
|
||||
|
||||
public void Dispose() { _ctx.Dispose(); _db.Dispose(); }
|
||||
|
||||
private async Task<TaskEntity> SeedTaskAsync()
|
||||
{
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", CreatedAt = DateTime.UtcNow });
|
||||
var task = new TaskEntity
|
||||
{
|
||||
Id = Guid.NewGuid().ToString(), ListId = listId, Title = "t",
|
||||
Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow, CommitType = "chore",
|
||||
};
|
||||
await _tasks.AddAsync(task);
|
||||
return task;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveAsync_HashNumber_ResolvesToGuid()
|
||||
{
|
||||
var task = await SeedTaskAsync();
|
||||
|
||||
var resolved = await TaskIdResolver.ResolveAsync(_tasks, $"#{task.Number}", CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, resolved);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveAsync_BareNumber_ResolvesToGuid()
|
||||
{
|
||||
var task = await SeedTaskAsync();
|
||||
|
||||
var resolved = await TaskIdResolver.ResolveAsync(_tasks, task.Number.ToString(), CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, resolved);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveAsync_Guid_PassesThroughUnchanged()
|
||||
{
|
||||
var task = await SeedTaskAsync();
|
||||
|
||||
var resolved = await TaskIdResolver.ResolveAsync(_tasks, task.Id, CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, resolved);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveAsync_UnknownNumber_ThrowsWithNumberInMessage()
|
||||
{
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
TaskIdResolver.ResolveAsync(_tasks, "#999999", CancellationToken.None));
|
||||
|
||||
Assert.Contains("999999", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveOptionalAsync_EmptyString_PassesThroughUnchanged()
|
||||
{
|
||||
var resolved = await TaskIdResolver.ResolveOptionalAsync(_tasks, "", CancellationToken.None);
|
||||
|
||||
Assert.Equal("", resolved);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveOptionalAsync_Null_ReturnsNull()
|
||||
{
|
||||
var resolved = await TaskIdResolver.ResolveOptionalAsync(_tasks, null, CancellationToken.None);
|
||||
|
||||
Assert.Null(resolved);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResolveManyAsync_MixedArray_ResolvesBoth()
|
||||
{
|
||||
var a = await SeedTaskAsync();
|
||||
var b = await SeedTaskAsync();
|
||||
|
||||
var resolved = await TaskIdResolver.ResolveManyAsync(_tasks, [$"#{a.Number}", b.Id], CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { a.Id, b.Id }, resolved);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user