feat(worker): dependsOn via MCP + honest staleness signal in merge preview
Adds a user/MCP-declared task dependency (DependsOnTaskId) distinct from the planning chain's internal BlockedByTaskId: add_task/update_task can set it, the queue picker skips a Queued task until the dependency reaches Done, a Failed/Cancelled dependency leaves the dependent blocked instead of starving silently, and setting a link rejects self-reference/unknown-id/cycles. get_task/list_tasks/batch_get_tasks now report blocked/blockedReason, and wait_for_task_change reports "Blocked" immediately instead of running out its timeout on a task the picker will never claim. preview_merge/preview_merge_set gain staleFiles: files a branch touches that the target branch also changed since the branch's fork point, a more honest staleness signal than `behind` alone.
This commit is contained in:
@@ -216,6 +216,23 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
Assert.Equal("the full description", found.TaskFull!.Description);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchGetTasks_QueuedWithUnmetDependsOn_ReportsBlocked()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, "predecessor", TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(listId, "blocked", TaskStatus.Queued);
|
||||
task.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = BuildSut();
|
||||
|
||||
var results = await sut.BatchGetTasks(new[] { task.Id }, cancellationToken: CancellationToken.None);
|
||||
|
||||
var found = results.Single(r => r.Id == task.Id);
|
||||
Assert.True(found.Task!.Blocked);
|
||||
Assert.Contains(predecessor.Id, found.Task!.BlockedReason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchDeleteTasks_RunningTask_ReportedNotOk_OthersDeleted()
|
||||
{
|
||||
|
||||
+248
-4
@@ -182,7 +182,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
var queue = CreateQueue();
|
||||
var sut = BuildSut(queue);
|
||||
|
||||
var dto = await sut.UpdateTask(task.Id, "new title", null, null, CancellationToken.None);
|
||||
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);
|
||||
@@ -203,7 +203,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
await _tasks.UpdateAsync(task, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.UpdateTask(task.Id, "new title", null, null, CancellationToken.None);
|
||||
var dto = await sut.UpdateTask(task.Id, "new title", null, null, null, CancellationToken.None);
|
||||
|
||||
Assert.Equal(task.Id, dto.Id);
|
||||
Assert.Equal(listId, dto.ListId);
|
||||
@@ -224,6 +224,91 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Equal("the full description text", dto.Description);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_QueuedWithUnmetDependsOn_ReportsBlockedTrueAndReason()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.Queued);
|
||||
task.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(task, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.GetTask(task.Id, CancellationToken.None);
|
||||
|
||||
Assert.True(dto.Blocked);
|
||||
Assert.Contains(predecessor.Id, dto.BlockedReason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_QueuedWithDoneDependsOn_ReportsBlockedFalse()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Done);
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.Queued);
|
||||
task.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(task, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.GetTask(task.Id, CancellationToken.None);
|
||||
|
||||
Assert.False(dto.Blocked);
|
||||
Assert.Null(dto.BlockedReason);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTask_IdleWithDependsOnTaskId_ReportsBlockedFalse_NotYetQueued()
|
||||
{
|
||||
// Blocking is only meaningful once the picker would actually be gating the task --
|
||||
// an Idle task hasn't been queued at all, so DependsOnTaskId doesn't apply yet.
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
task.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(task, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.GetTask(task.Id, CancellationToken.None);
|
||||
|
||||
Assert.False(dto.Blocked);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListTasks_ReportsBlockedPerTask()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, "predecessor", TaskStatus.Idle);
|
||||
var blocked = await SeedTaskAsync(listId, "blocked", TaskStatus.Queued);
|
||||
blocked.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(blocked, CancellationToken.None);
|
||||
var unblocked = await SeedTaskAsync(listId, "unblocked", TaskStatus.Queued);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var result = await sut.ListTasks(listId, cancellationToken: CancellationToken.None);
|
||||
|
||||
var blockedRef = result.Tasks!.Single(t => t.Id == blocked.Id);
|
||||
var unblockedRef = result.Tasks!.Single(t => t.Id == unblocked.Id);
|
||||
Assert.True(blockedRef.Blocked);
|
||||
Assert.Contains(predecessor.Id, blockedRef.BlockedReason);
|
||||
Assert.False(unblockedRef.Blocked);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ListTasks_IncludeDescriptionTrue_ReportsBlockedOnFullDto()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, "predecessor", TaskStatus.Idle);
|
||||
var blocked = await SeedTaskAsync(listId, "blocked", TaskStatus.Queued);
|
||||
blocked.DependsOnTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(blocked, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var result = await sut.ListTasks(listId, includeDescription: true, cancellationToken: CancellationToken.None);
|
||||
|
||||
var dto = result.TasksFull!.Single(t => t.Id == blocked.Id);
|
||||
Assert.True(dto.Blocked);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateTask_OnRunning_Throws()
|
||||
{
|
||||
@@ -233,7 +318,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
var sut = BuildSut(queue);
|
||||
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
sut.UpdateTask(task.Id, "x", null, null, CancellationToken.None));
|
||||
sut.UpdateTask(task.Id, "x", null, null, null, CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -243,7 +328,51 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
var sut = BuildSut(queue);
|
||||
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
sut.UpdateTask("does-not-exist", "x", null, null, CancellationToken.None));
|
||||
sut.UpdateTask("does-not-exist", "x", null, null, null, CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateTask_WithDependsOnTaskId_SetsLinkAndReportsBlocked()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.Queued);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var dto = await sut.UpdateTask(task.Id, dependsOnTaskId: predecessor.Id, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Equal(predecessor.Id, dto.DependsOnTaskId);
|
||||
Assert.True(dto.Blocked);
|
||||
var loaded = await _tasks.GetByIdAsync(task.Id);
|
||||
Assert.Equal(predecessor.Id, loaded!.DependsOnTaskId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateTask_WithEmptyStringDependsOnTaskId_ClearsExistingLink()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.Queued);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
await sut.UpdateTask(task.Id, dependsOnTaskId: predecessor.Id, cancellationToken: CancellationToken.None);
|
||||
|
||||
var dto = await sut.UpdateTask(task.Id, dependsOnTaskId: "", cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Null(dto.DependsOnTaskId);
|
||||
Assert.False(dto.Blocked);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task UpdateTask_WithCyclicDependsOnTaskId_Throws()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var a = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var b = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
await sut.UpdateTask(b.Id, dependsOnTaskId: a.Id, cancellationToken: CancellationToken.None);
|
||||
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
sut.UpdateTask(a.Id, dependsOnTaskId: b.Id, cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -1609,6 +1738,92 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Contains("working directory", ex.Message, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_TargetChangedSameFileSinceFork_ReportsStaleFiles()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var task = await SeedTaskAsync(listId, "a", TaskStatus.WaitingForReview);
|
||||
await CreateActiveWorktreeAsync(repo, task.Id, "shared.txt", "from branch\n");
|
||||
|
||||
// The target branch itself moved on and touched the same file after the fork point.
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "shared.txt"), "from main\n");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "add", "shared.txt");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main also touched shared.txt");
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMerge(task.Id, "main", CancellationToken.None);
|
||||
|
||||
Assert.Equal(1, result.Behind);
|
||||
Assert.NotNull(result.StaleFiles);
|
||||
Assert.Contains("shared.txt", result.StaleFiles!);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_TargetBehindButUnrelatedFile_ReportsNoStaleFiles()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var task = await SeedTaskAsync(listId, "a", TaskStatus.WaitingForReview);
|
||||
await CreateActiveWorktreeAsync(repo, task.Id, "branch-only.txt", "from branch\n");
|
||||
|
||||
// Target moved on (behind > 0) but touched an entirely different file -- a stale
|
||||
// branch that still collides with nothing.
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "unrelated.txt"), "from main\n");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "add", "unrelated.txt");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main touched something else");
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMerge(task.Id, "main", CancellationToken.None);
|
||||
|
||||
Assert.Equal(1, result.Behind);
|
||||
Assert.NotNull(result.StaleFiles);
|
||||
Assert.Empty(result.StaleFiles!);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_WorktreeLessHandlerTask_ReportsNoStaleFiles()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview);
|
||||
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "handled.txt"), "content");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "handled task merge");
|
||||
var headCommit = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
var t = await ctx.Tasks.FindAsync(task.Id);
|
||||
t!.HandlerBaseCommit = repo.BaseCommit;
|
||||
t.HandlerHeadCommit = headCommit;
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None);
|
||||
|
||||
Assert.NotNull(result.StaleFiles);
|
||||
Assert.Empty(result.StaleFiles!);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMergeSet_TwoTasksSharedFile_ReportsOverlap()
|
||||
{
|
||||
@@ -1756,6 +1971,35 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
() => sut.AddTask(listId, "t", model: "gpt4", cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddTask_WithDependsOnTaskId_PersistsLinkAndReportsBlocked()
|
||||
{
|
||||
var listId = await SeedListAsync();
|
||||
var predecessor = await SeedTaskAsync(listId, status: TaskStatus.Idle);
|
||||
var sut = NewService();
|
||||
|
||||
var dto = await sut.AddTask(
|
||||
listId, "t", dependsOnTaskId: predecessor.Id, queueImmediately: true, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Equal(predecessor.Id, dto.Task.DependsOnTaskId);
|
||||
Assert.True(dto.Task.Blocked);
|
||||
Assert.Contains(predecessor.Id, dto.Task.BlockedReason);
|
||||
var loaded = await _tasks.GetByIdAsync(dto.Task.Id);
|
||||
Assert.Equal(predecessor.Id, loaded!.DependsOnTaskId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task AddTask_WithSelfReferencingDependsOnTaskId_Throws()
|
||||
{
|
||||
// Can't reference its own not-yet-known id, so this exercises the not-found path --
|
||||
// an unknown dependsOnTaskId is rejected the same way a self-reference would be.
|
||||
var listId = await SeedListAsync();
|
||||
var sut = NewService();
|
||||
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => sut.AddTask(listId, "t", dependsOnTaskId: "does-not-exist", cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
// ── AddTask possible-duplicate check ────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
|
||||
@@ -196,6 +196,64 @@ public sealed class TaskWaitMcpToolsTests : IDisposable
|
||||
Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForTaskChange_QueuedWithBlockedByTaskId_ReportsBlockedImmediately()
|
||||
{
|
||||
var predecessor = await SeedTaskAsync(TaskStatus.Queued);
|
||||
var task = await SeedTaskAsync(TaskStatus.Queued);
|
||||
task.BlockedByTaskId = predecessor.Id;
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = BuildSut();
|
||||
var sw = Stopwatch.StartNew();
|
||||
|
||||
var result = await sut.WaitForTaskChange([task.Id], timeoutSeconds: 30, cancellationToken: CancellationToken.None);
|
||||
|
||||
sw.Stop();
|
||||
Assert.False(result.TimedOut);
|
||||
var change = Assert.Single(result.Changed);
|
||||
Assert.Equal("Blocked", change.Status);
|
||||
Assert.Contains(predecessor.Id, change.BlockedReason);
|
||||
Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForTaskChange_QueuedWithUnmetDependsOn_ReportsBlockedImmediately_InsteadOfTimingOut()
|
||||
{
|
||||
var dependency = await SeedTaskAsync(TaskStatus.Idle);
|
||||
var task = await SeedTaskAsync(TaskStatus.Queued);
|
||||
task.DependsOnTaskId = dependency.Id;
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = BuildSut();
|
||||
var sw = Stopwatch.StartNew();
|
||||
|
||||
var result = await sut.WaitForTaskChange([task.Id], timeoutSeconds: 30, cancellationToken: CancellationToken.None);
|
||||
|
||||
sw.Stop();
|
||||
Assert.False(result.TimedOut);
|
||||
var change = Assert.Single(result.Changed);
|
||||
Assert.Equal("Blocked", change.Status);
|
||||
Assert.Contains(dependency.Id, change.BlockedReason);
|
||||
Assert.True(sw.Elapsed < TimeSpan.FromSeconds(2), $"took {sw.Elapsed}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForTaskChange_QueuedWithDoneDependsOn_IsNotBlocked_StillWaitsAsBusy()
|
||||
{
|
||||
var dependency = await SeedTaskAsync(TaskStatus.Done);
|
||||
var task = await SeedTaskAsync(TaskStatus.Queued);
|
||||
task.DependsOnTaskId = dependency.Id;
|
||||
await _tasks.UpdateAsync(task);
|
||||
var sut = BuildSut();
|
||||
var sw = Stopwatch.StartNew();
|
||||
|
||||
var result = await sut.WaitForTaskChange([task.Id], timeoutSeconds: 1, cancellationToken: CancellationToken.None);
|
||||
|
||||
sw.Stop();
|
||||
Assert.True(result.TimedOut);
|
||||
Assert.Empty(result.Changed);
|
||||
Assert.True(sw.Elapsed >= TimeSpan.FromMilliseconds(900), $"took {sw.Elapsed}");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MaxTimeoutSeconds_StaysComfortablyUnderMcpToolTimeout()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user