Merge task branch for: MCP: add missing external tools + fix status enum, branchDeleted, merge-from-review

This commit is contained in:
mika kuns
2026-06-09 10:00:11 +02:00
4 changed files with 201 additions and 28 deletions

View File

@@ -61,4 +61,14 @@ public sealed class ConfigMcpTools
await _tasks.UpdateAgentSettingsAsync(taskId, model.NullIfBlank(), systemPrompt.NullIfBlank(), agentPath.NullIfBlank(), maxTurns, cancellationToken); await _tasks.UpdateAgentSettingsAsync(taskId, model.NullIfBlank(), systemPrompt.NullIfBlank(), agentPath.NullIfBlank(), maxTurns, cancellationToken);
await _broadcaster.TaskUpdated(taskId); await _broadcaster.TaskUpdated(taskId);
} }
[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)
{
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);
}
} }

View File

@@ -104,7 +104,7 @@ public sealed class ExternalMcpService
[McpServerTool, Description( [McpServerTool, Description(
"List tasks in a given list. Optionally filter by creator (createdBy) and/or status. " + "List tasks in a given list. Optionally filter by creator (createdBy) and/or status. " +
"Valid status values: Idle, Queued, Running, WaitingForReview, Done, Failed, Cancelled.")] "Valid status values: Idle, Queued, Running, WaitingForReview, WaitingForChildren, Done, Failed, Cancelled.")]
public async Task<IReadOnlyList<TaskDto>> ListTasks( public async Task<IReadOnlyList<TaskDto>> ListTasks(
string listId, string listId,
string? createdBy, string? createdBy,
@@ -116,7 +116,7 @@ public sealed class ExternalMcpService
{ {
if (!Enum.TryParse<TaskStatus>(status, ignoreCase: true, out var parsed)) if (!Enum.TryParse<TaskStatus>(status, ignoreCase: true, out var parsed))
throw new InvalidOperationException( throw new InvalidOperationException(
$"Unknown status '{status}'. Valid values: Idle, Queued, Running, WaitingForReview, Done, Failed, Cancelled."); $"Unknown status '{status}'. Valid values: Idle, Queued, Running, WaitingForReview, WaitingForChildren, Done, Failed, Cancelled.");
statusFilter = parsed; statusFilter = parsed;
} }
@@ -360,13 +360,14 @@ public sealed class ExternalMcpService
[McpServerTool, Description("Returns all valid task status values and their meanings. Use before filtering by status or interpreting task state.")] [McpServerTool, Description("Returns all valid task status values and their meanings. Use before filtering by status or interpreting task state.")]
public Task<IReadOnlyList<StatusValueDto>> GetTaskStatusValues() => public Task<IReadOnlyList<StatusValueDto>> GetTaskStatusValues() =>
Task.FromResult<IReadOnlyList<StatusValueDto>>([ Task.FromResult<IReadOnlyList<StatusValueDto>>([
new("Idle", "Not yet queued; task is editable and will not run until enqueued."), new("Idle", "Not yet queued; task is editable and will not run until enqueued."),
new("Queued", "Waiting for an agent execution slot. Tasks with a blocker (BlockedByTaskId) are skipped by the queue picker until their predecessor finishes."), new("Queued", "Waiting for an agent execution slot. Tasks with a blocker (BlockedByTaskId) are skipped by the queue picker until their predecessor finishes."),
new("Running", "Agent is actively executing the task; cannot be edited or deleted until cancelled."), new("Running", "Agent is actively executing the task; cannot be edited or deleted until cancelled."),
new("WaitingForReview", "Run finished successfully and awaits review. Use review_task: approve (→ Done), reject_rerun (→ Queued, resumes the session with feedback), reject_park (→ Idle), or cancel (→ Cancelled)."), new("WaitingForReview", "Run finished successfully and awaits review. Use review_task: approve (→ Done), reject_rerun (→ Queued, resumes the session with feedback), reject_park (→ Idle), or cancel (→ Cancelled)."),
new("Done", "Completed successfully and approved; result text is available in the result field. Can be reset to Idle for re-execution."), new("WaitingForChildren", "Planning parent whose child tasks are still running. The parent resumes once all children reach a terminal state."),
new("Failed", "Execution ended with an error; task can be reset to Idle or re-queued directly."), new("Done", "Completed successfully and approved; result text is available in the result field. Can be reset to Idle for re-execution."),
new("Cancelled", "Cancelled by the user; task can be reset to Idle or re-queued directly."), new("Failed", "Execution ended with an error; task can be reset to Idle or re-queued directly."),
new("Cancelled", "Cancelled by the user; task can be reset to Idle or re-queued directly."),
]); ]);
// ── Worktree / git tools ────────────────────────────────────────────────── // ── Worktree / git tools ──────────────────────────────────────────────────
@@ -428,7 +429,7 @@ public sealed class ExternalMcpService
"Merge a task's worktree branch into targetBranch (default: main). " + "Merge a task's worktree branch into targetBranch (default: main). " +
"noFf=true (default): always creates a merge commit (--no-ff). " + "noFf=true (default): always creates a merge commit (--no-ff). " +
"dryRun=true: validates preconditions only, does not perform the merge; merged=false in the result means 'not actually merged'. " + "dryRun=true: validates preconditions only, does not perform the merge; merged=false in the result means 'not actually merged'. " +
"Refuses if task status is not Done (status values: Idle, Queued, Running, Done, Failed, Cancelled). " + "allowWaitingForReview=true: also allows merging a task in WaitingForReview (default false, which only allows Done). " +
"On success: merged=true, mergeCommit contains the new merge commit SHA. " + "On success: merged=true, mergeCommit contains the new merge commit SHA. " +
"On conflict: the merge is cleanly aborted (no half-merged state left); merged=false and conflicts lists the affected files.")] "On conflict: the merge is cleanly aborted (no half-merged state left); merged=false and conflicts lists the affected files.")]
public async Task<MergeTaskResultDto> MergeTask( public async Task<MergeTaskResultDto> MergeTask(
@@ -436,14 +437,17 @@ public sealed class ExternalMcpService
string targetBranch = "main", string targetBranch = "main",
bool noFf = true, bool noFf = true,
bool dryRun = false, bool dryRun = false,
bool allowWaitingForReview = false,
CancellationToken cancellationToken = default) CancellationToken cancellationToken = default)
{ {
var task = await _tasks.GetByIdAsync(taskId, cancellationToken) var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
?? throw new InvalidOperationException($"Task {taskId} not found."); ?? throw new InvalidOperationException($"Task {taskId} not found.");
if (task.Status != TaskStatus.Done) var canMerge = task.Status == TaskStatus.Done ||
(allowWaitingForReview && task.Status == TaskStatus.WaitingForReview);
if (!canMerge)
throw new InvalidOperationException( throw new InvalidOperationException(
$"Task must be Done to merge (current status: {task.Status}). " + $"Task must be Done to merge (current status: {task.Status}). " +
"Valid statuses for merge: Done."); "Pass allowWaitingForReview=true to also merge a WaitingForReview task.");
var list = await _lists.GetByIdAsync(task.ListId, cancellationToken); var list = await _lists.GetByIdAsync(task.ListId, cancellationToken);
@@ -528,7 +532,37 @@ public sealed class ExternalMcpService
var path = wt.Path; var path = wt.Path;
var result = await _maintenance.ForceRemoveAsync(taskId, cancellationToken); var result = await _maintenance.ForceRemoveAsync(taskId, cancellationToken);
return new CleanupWorktreeResult(result.Removed, path, result.Removed); return new CleanupWorktreeResult(result.Removed, path, result.BranchDeleted);
}
[McpServerTool, Description(
"Send a follow-up prompt to an existing Claude session (multi-turn continuation). " +
"The agent resumes using --resume with the session ID from the task's last run. " +
"Runs in the override execution slot; throws if the slot is busy — try again later. " +
"Returns a status string from the execution slot.")]
public async Task<string> ContinueTask(
string taskId,
string followUpPrompt,
CancellationToken cancellationToken)
{
if (string.IsNullOrWhiteSpace(followUpPrompt))
throw new InvalidOperationException("followUpPrompt is required.");
string result;
try
{
result = await _queue.ContinueTask(taskId, followUpPrompt);
}
catch (InvalidOperationException)
{
throw new InvalidOperationException("Override slot busy. Try again later.");
}
catch (KeyNotFoundException)
{
throw new InvalidOperationException($"Task {taskId} not found.");
}
await _broadcaster.TaskUpdated(taskId);
return result;
} }
// ── Daily prep ─────────────────────────────────────────────────────────── // ── Daily prep ───────────────────────────────────────────────────────────

View File

@@ -9,7 +9,7 @@ public sealed class WorktreeMaintenanceService
{ {
public sealed record CleanupResult(int Removed, IReadOnlyList<string> RemovedTaskIds); public sealed record CleanupResult(int Removed, IReadOnlyList<string> RemovedTaskIds);
public sealed record ResetResult(int Removed, int TasksAffected, bool Blocked, int RunningTasks, IReadOnlyList<string> RemovedTaskIds); public sealed record ResetResult(int Removed, int TasksAffected, bool Blocked, int RunningTasks, IReadOnlyList<string> RemovedTaskIds);
public sealed record ForceRemoveResult(bool Removed, string? Reason); public sealed record ForceRemoveResult(bool Removed, string? Reason, bool BranchDeleted);
private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory; private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory;
private readonly GitService _git; private readonly GitService _git;
@@ -43,7 +43,8 @@ public sealed class WorktreeMaintenanceService
var removedTaskIds = new List<string>(); var removedTaskIds = new List<string>();
foreach (var row in rows) foreach (var row in rows)
{ {
if (await TryRemoveAsync(row, force: false, ct)) var (rowRemoved, _) = await TryRemoveAsync(row, force: false, ct);
if (rowRemoved)
{ {
removed++; removed++;
removedTaskIds.Add(row.TaskId); removedTaskIds.Add(row.TaskId);
@@ -71,7 +72,8 @@ public sealed class WorktreeMaintenanceService
var removedTaskIds = new List<string>(); var removedTaskIds = new List<string>();
foreach (var row in rows) foreach (var row in rows)
{ {
if (await TryRemoveAsync(row, force: true, ct)) var (rowRemoved, _) = await TryRemoveAsync(row, force: true, ct);
if (rowRemoved)
{ {
removed++; removed++;
removedTaskIds.Add(row.TaskId); removedTaskIds.Add(row.TaskId);
@@ -118,16 +120,16 @@ public sealed class WorktreeMaintenanceService
.FirstOrDefaultAsync(ct); .FirstOrDefaultAsync(ct);
if (row is null) if (row is null)
return new ForceRemoveResult(false, "worktree not found"); return new ForceRemoveResult(false, "worktree not found", false);
if (row.Status == ClaudeDo.Data.Models.TaskStatus.Running) if (row.Status == ClaudeDo.Data.Models.TaskStatus.Running)
return new ForceRemoveResult(false, "task is currently running"); return new ForceRemoveResult(false, "task is currently running", false);
var ok = await TryRemoveAsync(row.Row, force: true, ct); var (ok, branchDeleted) = await TryRemoveAsync(row.Row, force: true, ct);
return new ForceRemoveResult(ok, ok ? null : "remove failed"); return new ForceRemoveResult(ok, ok ? null : "remove failed", branchDeleted);
} }
private async Task<bool> TryRemoveAsync(WorktreeRow row, bool force, CancellationToken ct) private async Task<(bool Removed, bool BranchDeleted)> TryRemoveAsync(WorktreeRow row, bool force, CancellationToken ct)
{ {
var repoDirExists = !string.IsNullOrWhiteSpace(row.WorkingDir) && Directory.Exists(row.WorkingDir); var repoDirExists = !string.IsNullOrWhiteSpace(row.WorkingDir) && Directory.Exists(row.WorkingDir);
bool dirRemoved; bool dirRemoved;
@@ -163,8 +165,13 @@ public sealed class WorktreeMaintenanceService
} }
} }
// Drop the DB row only when the on-disk worktree is gone; otherwise we'd silently
// strand a directory while reporting success.
if (!dirRemoved) return (false, false);
// Branch cleanup: otherwise rerunning the task hits "branch already exists". // Branch cleanup: otherwise rerunning the task hits "branch already exists".
// Prune first so git no longer thinks the branch is checked out by a phantom worktree. // Prune first so git no longer thinks the branch is checked out by a phantom worktree.
bool branchDeleted = false;
if (repoDirExists) if (repoDirExists)
{ {
try { await _git.WorktreePruneAsync(row.WorkingDir!, ct); } try { await _git.WorktreePruneAsync(row.WorkingDir!, ct); }
@@ -175,6 +182,7 @@ public sealed class WorktreeMaintenanceService
try try
{ {
await _git.BranchDeleteAsync(row.WorkingDir!, row.BranchName, force: true, ct); await _git.BranchDeleteAsync(row.WorkingDir!, row.BranchName, force: true, ct);
branchDeleted = true;
} }
catch (Exception ex) catch (Exception ex)
{ {
@@ -184,13 +192,9 @@ public sealed class WorktreeMaintenanceService
} }
} }
// Drop the DB row only when the on-disk worktree is gone; otherwise we'd silently
// strand a directory while reporting success.
if (!dirRemoved) return false;
using var context = _dbFactory.CreateDbContext(); using var context = _dbFactory.CreateDbContext();
await context.Worktrees.Where(w => w.TaskId == row.TaskId).ExecuteDeleteAsync(ct); await context.Worktrees.Where(w => w.TaskId == row.TaskId).ExecuteDeleteAsync(ct);
return true; return (true, branchDeleted);
} }
private sealed record WorktreeRow(string TaskId, string Path, string BranchName, string? WorkingDir); private sealed record WorktreeRow(string TaskId, string Path, string BranchName, string? WorkingDir);

View File

@@ -515,7 +515,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var ex = await Assert.ThrowsAsync<InvalidOperationException>( var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.MergeTask(task.Id, "main", true, false, CancellationToken.None)); () => sut.MergeTask(task.Id, "main", true, false, false, CancellationToken.None));
Assert.Contains("Done", ex.Message); Assert.Contains("Done", ex.Message);
} }
@@ -527,7 +527,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
var (task, _, _) = await SeedWorktreeAsync(TaskStatus.Done); var (task, _, _) = await SeedWorktreeAsync(TaskStatus.Done);
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.MergeTask(task.Id, "main", true, dryRun: true, CancellationToken.None); var result = await sut.MergeTask(task.Id, "main", true, dryRun: true, cancellationToken: CancellationToken.None);
Assert.False(result.Merged); Assert.False(result.Merged);
Assert.Null(result.MergeCommit); Assert.Null(result.MergeCommit);
@@ -595,4 +595,129 @@ public sealed class ExternalMcpServiceTests : IDisposable
Assert.True(result.Removed); Assert.True(result.Removed);
Assert.False(Directory.Exists(wt.WorktreePath)); Assert.False(Directory.Exists(wt.WorktreePath));
} }
// ── GetTaskConfig ─────────────────────────────────────────────────────────
private ConfigMcpTools BuildConfigSut() => new(_lists, _tasks, _broadcaster);
[Fact]
public async Task GetTaskConfig_NotFound_Throws()
{
var sut = BuildConfigSut();
await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.GetTaskConfig("does-not-exist", CancellationToken.None));
}
[Fact]
public async Task GetTaskConfig_NoOverrides_ReturnsNull()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId);
var sut = BuildConfigSut();
var result = await sut.GetTaskConfig(task.Id, CancellationToken.None);
Assert.Null(result);
}
[Fact]
public async Task GetTaskConfig_WithOverrides_ReturnsValues()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId);
await _tasks.UpdateAgentSettingsAsync(task.Id, "claude-sonnet-4-6", "be concise", null, 10, CancellationToken.None);
var sut = BuildConfigSut();
var result = await sut.GetTaskConfig(task.Id, CancellationToken.None);
Assert.NotNull(result);
Assert.Equal("claude-sonnet-4-6", result.Model);
Assert.Equal("be concise", result.SystemPrompt);
Assert.Null(result.AgentPath);
Assert.Equal(10, result.MaxTurns);
}
// ── GetTaskStatusValues ───────────────────────────────────────────────────
[Fact]
public async Task GetTaskStatusValues_ContainsAllStatuses()
{
var sut = NewService();
var values = await sut.GetTaskStatusValues();
var names = values.Select(v => v.Status).ToHashSet();
foreach (var status in Enum.GetValues<TaskStatus>())
Assert.Contains(status.ToString(), names);
}
// ── ListTasks status filter ───────────────────────────────────────────────
[Fact]
public async Task ListTasks_FilterByWaitingForReview_ReturnsMatchingTasks()
{
var listId = await SeedListAsync();
await SeedTaskAsync(listId, "wfr", TaskStatus.WaitingForReview);
await SeedTaskAsync(listId, "idle", TaskStatus.Idle);
var sut = NewService();
var result = await sut.ListTasks(listId, null, "WaitingForReview", CancellationToken.None);
Assert.Single(result);
Assert.Equal("WaitingForReview", result[0].Status);
}
[Fact]
public async Task ListTasks_FilterByWaitingForChildren_ReturnsMatchingTasks()
{
var listId = await SeedListAsync();
await SeedTaskAsync(listId, "wfc", TaskStatus.WaitingForChildren);
await SeedTaskAsync(listId, "done", TaskStatus.Done);
var sut = NewService();
var result = await sut.ListTasks(listId, null, "WaitingForChildren", CancellationToken.None);
Assert.Single(result);
Assert.Equal("WaitingForChildren", result[0].Status);
}
// ── MergeTask allowWaitingForReview ───────────────────────────────────────
[Fact]
public async Task MergeTask_WaitingForReview_WithoutFlag_Throws()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview);
var sut = BuildSut(CreateQueue());
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.MergeTask(task.Id, "main", true, false, false, CancellationToken.None));
Assert.Contains("Done", ex.Message);
}
[Fact]
public async Task MergeTask_WaitingForReview_WithFlag_DryRun_Succeeds()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var (task, _, _) = await SeedWorktreeAsync(TaskStatus.WaitingForReview);
var sut = BuildSut(CreateQueue());
var result = await sut.MergeTask(task.Id, "main", true, dryRun: true, allowWaitingForReview: true, CancellationToken.None);
Assert.False(result.Merged);
Assert.Null(result.MergeCommit);
}
// ── ContinueTask validation ───────────────────────────────────────────────
[Fact]
public async Task ContinueTask_EmptyPrompt_Throws()
{
var listId = await SeedListAsync();
var task = await SeedTaskAsync(listId);
var sut = NewService();
await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.ContinueTask(task.Id, " ", CancellationToken.None));
}
} }