feat(queue): warn when the base branch has uncommitted changes
Queuing (update_task_status, batch_update_task_status) and run_task_now now surface a non-blocking baseDirty warning (separate modified/untracked counts) when the list's working dir has uncommitted changes at enqueue time, since a new worktree forks from the commit tip and silently misses them. BaseDirtyChecker caches per working dir for a few seconds so a batch queue over many tasks in one list only shells out to git once. The UI surfaces the same warning via the footer error strip on queue actions.
This commit is contained in:
@@ -92,7 +92,7 @@ public abstract class StubWorkerClient : IWorkerClient
|
||||
public virtual Task<List<string>> InstallSessionSkillAsync(string url) => Task.FromResult(new List<string>());
|
||||
public virtual Task UpdateSessionSkillAsync(string sourceUrl) => Task.CompletedTask;
|
||||
public virtual Task RemoveSessionSkillAsync(string sourceUrl) => Task.CompletedTask;
|
||||
public virtual Task SetTaskStatusAsync(string taskId, TaskStatus status) => Task.CompletedTask;
|
||||
public virtual Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status) => Task.FromResult<BaseDirtyWarningDto?>(null);
|
||||
public virtual Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch) => Task.FromResult<MergeResultDto?>(null);
|
||||
public virtual Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch) => Task.FromResult<MergePreviewDto?>(null);
|
||||
public virtual Task<MergeResultDto> MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
|
||||
|
||||
@@ -228,14 +228,15 @@ public class MissionControlViewModelTests : IDisposable
|
||||
|
||||
public QueueingWorkerClient(Func<ClaudeDoDbContext> newContext) => _newContext = newContext;
|
||||
|
||||
public override async Task SetTaskStatusAsync(string taskId, TaskStatus status)
|
||||
public override async Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status)
|
||||
{
|
||||
QueuedTaskIds.Add(taskId);
|
||||
await using var db = _newContext();
|
||||
var entity = await db.Tasks.FirstOrDefaultAsync(t => t.Id == taskId);
|
||||
if (entity is null) return;
|
||||
if (entity is null) return null;
|
||||
entity.Status = status;
|
||||
await db.SaveChangesAsync();
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Lifecycle;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
@@ -86,7 +87,8 @@ public sealed class AddSubtaskToolTests : IDisposable
|
||||
return new ExternalMcpService(
|
||||
_tasks, _lists, queue, broadcaster,
|
||||
state,
|
||||
git, dbFactory, maintenance, merge, planningMerge);
|
||||
git, dbFactory, maintenance, merge, planningMerge,
|
||||
new BaseDirtyChecker(git, NullLogger<BaseDirtyChecker>.Instance));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
+35
-3
@@ -4,6 +4,7 @@ using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Config;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Lifecycle;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
@@ -24,6 +25,9 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
private readonly TaskRepository _tasks;
|
||||
private readonly ListRepository _lists;
|
||||
private readonly HubBroadcaster _broadcaster;
|
||||
private readonly List<GitRepoFixture> _repos = new();
|
||||
|
||||
private static bool GitAvailable => GitRepoFixture.IsGitAvailable();
|
||||
|
||||
public BatchMcpToolsTests()
|
||||
{
|
||||
@@ -35,14 +39,15 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
foreach (var r in _repos) r.Dispose();
|
||||
_ctx.Dispose();
|
||||
_db.Dispose();
|
||||
}
|
||||
|
||||
private async Task<string> SeedListAsync()
|
||||
private async Task<string> SeedListAsync(string? workingDir = null)
|
||||
{
|
||||
var id = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = id, Name = "L", CreatedAt = DateTime.UtcNow });
|
||||
await _lists.AddAsync(new ListEntity { Id = id, Name = "L", CreatedAt = DateTime.UtcNow, WorkingDir = workingDir });
|
||||
return id;
|
||||
}
|
||||
|
||||
@@ -74,7 +79,8 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
var svc = new ExternalMcpService(
|
||||
_tasks, _lists, CreateQueue(), _broadcaster,
|
||||
state,
|
||||
git, factory, maintenance, merge, planningMerge);
|
||||
git, factory, maintenance, merge, planningMerge,
|
||||
new BaseDirtyChecker(git, NullLogger<BaseDirtyChecker>.Instance));
|
||||
return new BatchMcpTools(svc);
|
||||
}
|
||||
|
||||
@@ -247,6 +253,32 @@ public sealed class BatchMcpToolsTests : IDisposable
|
||||
Assert.Equal(TaskStatus.Queued, (await _tasks.GetByIdAsync(t2.Id))!.Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchUpdateTaskStatus_DirtyBaseRepo_ReportsWarningPerItem_OneListOneRepo()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "scratch.txt"), "new");
|
||||
|
||||
var listId = await SeedListAsync(repo.RepoDir);
|
||||
var t1 = await SeedTaskAsync(listId, "a", TaskStatus.Idle);
|
||||
var t2 = await SeedTaskAsync(listId, "b", TaskStatus.Idle);
|
||||
var sut = BuildSut();
|
||||
|
||||
// Two tasks queued from the same list -- BaseDirtyChecker's TTL cache means only one
|
||||
// `git status` actually runs underneath, but both items still see the warning.
|
||||
var results = await sut.BatchUpdateTaskStatus(new[] { t1.Id, t2.Id }, "Queued", CancellationToken.None);
|
||||
|
||||
Assert.All(results, r => Assert.True(r.Ok));
|
||||
Assert.All(results, r =>
|
||||
{
|
||||
Assert.NotNull(r.BaseDirty);
|
||||
Assert.Equal(1, r.BaseDirty!.UntrackedCount);
|
||||
});
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BatchUpdateTaskStatus_Done_MixedWorktreeState_ReportsPerItemAndDoesNotAbort()
|
||||
{
|
||||
|
||||
@@ -4,6 +4,7 @@ using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Config;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Lifecycle;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
@@ -136,7 +137,8 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
return new ExternalMcpService(
|
||||
_tasks, _lists, queue, _broadcaster,
|
||||
state,
|
||||
git, factory, maintenance, merge, planningMerge);
|
||||
git, factory, maintenance, merge, planningMerge,
|
||||
new BaseDirtyChecker(git, NullLogger<BaseDirtyChecker>.Instance));
|
||||
}
|
||||
|
||||
private QueueService CreateQueue()
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests.Git;
|
||||
|
||||
public sealed class BaseDirtyCheckerTests : IDisposable
|
||||
{
|
||||
private readonly List<GitRepoFixture> _repos = new();
|
||||
|
||||
private static bool GitAvailable => GitRepoFixture.IsGitAvailable();
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
foreach (var r in _repos) r.Dispose();
|
||||
}
|
||||
|
||||
private GitRepoFixture CreateRepo()
|
||||
{
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
return repo;
|
||||
}
|
||||
|
||||
private static BaseDirtyChecker CreateSut() =>
|
||||
new(new GitService(), NullLogger<BaseDirtyChecker>.Instance);
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_NullWorkingDir_ReturnsNull()
|
||||
{
|
||||
var sut = CreateSut();
|
||||
Assert.Null(await sut.CheckAsync(null, default));
|
||||
Assert.Null(await sut.CheckAsync("", default));
|
||||
Assert.Null(await sut.CheckAsync(" ", default));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_NotAGitRepo_ReturnsNull()
|
||||
{
|
||||
var dir = Path.Combine(Path.GetTempPath(), $"claudedo_notgit_{Guid.NewGuid():N}");
|
||||
Directory.CreateDirectory(dir);
|
||||
try
|
||||
{
|
||||
var sut = CreateSut();
|
||||
Assert.Null(await sut.CheckAsync(dir, default));
|
||||
}
|
||||
finally
|
||||
{
|
||||
Directory.Delete(dir, true);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_CleanRepo_ReturnsNull()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = CreateRepo();
|
||||
var sut = CreateSut();
|
||||
|
||||
Assert.Null(await sut.CheckAsync(repo.RepoDir, default));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_UntrackedFile_ReturnsUntrackedCountOnly()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = CreateRepo();
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "scratch.txt"), "new");
|
||||
var sut = CreateSut();
|
||||
|
||||
var warning = await sut.CheckAsync(repo.RepoDir, default);
|
||||
|
||||
Assert.NotNull(warning);
|
||||
Assert.Equal(0, warning!.ModifiedCount);
|
||||
Assert.Equal(1, warning.UntrackedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_ModifiedTrackedFile_ReturnsModifiedCountOnly()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = CreateRepo();
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "edited");
|
||||
var sut = CreateSut();
|
||||
|
||||
var warning = await sut.CheckAsync(repo.RepoDir, default);
|
||||
|
||||
Assert.NotNull(warning);
|
||||
Assert.Equal(1, warning!.ModifiedCount);
|
||||
Assert.Equal(0, warning.UntrackedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CheckAsync_ModifiedAndUntracked_ReportsBothSeparately()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = CreateRepo();
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "edited");
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "scratch.txt"), "new");
|
||||
var sut = CreateSut();
|
||||
|
||||
var warning = await sut.CheckAsync(repo.RepoDir, default);
|
||||
|
||||
Assert.NotNull(warning);
|
||||
Assert.Equal(1, warning!.ModifiedCount);
|
||||
Assert.Equal(1, warning.UntrackedCount);
|
||||
}
|
||||
|
||||
// Proves the TTL cache collapses repeated checks against the same working dir into a
|
||||
// single underlying `git status` call -- the acceptance requirement that a batch queue
|
||||
// op over many tasks in one list must not shell out once per task. A second check
|
||||
// immediately after a repo mutation still returns the first (stale) answer instead of
|
||||
// re-running git.
|
||||
[Fact]
|
||||
public async Task CheckAsync_RepeatedCallsWithinTtl_ReuseCachedResult()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = CreateRepo();
|
||||
var sut = CreateSut();
|
||||
|
||||
var first = await sut.CheckAsync(repo.RepoDir, default);
|
||||
Assert.Null(first);
|
||||
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "scratch.txt"), "new");
|
||||
var second = await sut.CheckAsync(repo.RepoDir, default);
|
||||
|
||||
Assert.Null(second);
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
using ClaudeDo.Worker.Queue;
|
||||
@@ -21,7 +22,9 @@ public static class TaskStateServiceBuilder
|
||||
RunCancellationRegistry RunCancels);
|
||||
|
||||
public static Built Build(
|
||||
IDbContextFactory<ClaudeDoDbContext> dbFactory, Func<IActiveMergeState>? mergeState = null)
|
||||
IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||
Func<IActiveMergeState>? mergeState = null,
|
||||
IBaseDirtyChecker? baseDirtyChecker = null)
|
||||
{
|
||||
var hub = new CapturingHubContext();
|
||||
var broadcaster = new HubBroadcaster(hub);
|
||||
@@ -37,6 +40,7 @@ public static class TaskStateServiceBuilder
|
||||
chain,
|
||||
runCancels,
|
||||
mergeState ?? (() => NoActiveMergeState.Instance),
|
||||
baseDirtyChecker ?? new BaseDirtyChecker(new ClaudeDo.Data.Git.GitService(), NullLogger<BaseDirtyChecker>.Instance),
|
||||
NullLogger<TaskStateService>.Instance);
|
||||
|
||||
return new Built(state, chain, hub, () => waker.Count, waker, runCancels);
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Git;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
using ClaudeDo.Worker.State;
|
||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests.State;
|
||||
@@ -22,6 +24,9 @@ public sealed class TaskStateServiceTests : IDisposable
|
||||
private readonly TaskStateServiceBuilder.Built _built;
|
||||
private readonly ITaskStateService _sut;
|
||||
private readonly string _listId;
|
||||
private readonly List<GitRepoFixture> _repos = new();
|
||||
|
||||
private static bool GitAvailable => GitRepoFixture.IsGitAvailable();
|
||||
|
||||
public TaskStateServiceTests()
|
||||
{
|
||||
@@ -41,7 +46,19 @@ public sealed class TaskStateServiceTests : IDisposable
|
||||
ctx.SaveChanges();
|
||||
}
|
||||
|
||||
public void Dispose() => _db.Dispose();
|
||||
public void Dispose()
|
||||
{
|
||||
foreach (var r in _repos) r.Dispose();
|
||||
_db.Dispose();
|
||||
}
|
||||
|
||||
private async Task SetListWorkingDirAsync(string workingDir)
|
||||
{
|
||||
await using var ctx = _factory.CreateDbContext();
|
||||
var list = await ctx.Lists.FirstAsync(l => l.Id == _listId);
|
||||
list.WorkingDir = workingDir;
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
private async Task<string> SeedTaskAsync(
|
||||
TaskStatus status,
|
||||
@@ -98,6 +115,74 @@ public sealed class TaskStateServiceTests : IDisposable
|
||||
Assert.Contains(_built.Hub.Proxy.Calls, c => c.Method == "TaskUpdated");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EnqueueAsync_NoWorkingDir_QueuesNormally_NoBaseDirtyWarning()
|
||||
{
|
||||
// The list created in the constructor has no WorkingDir -- covers the
|
||||
// "no valid repo" acceptance case: queuing must not throw or block.
|
||||
var id = await SeedTaskAsync(TaskStatus.Idle);
|
||||
|
||||
var result = await _sut.EnqueueAsync(id, default);
|
||||
|
||||
Assert.True(result.Ok);
|
||||
Assert.Null(result.BaseDirty);
|
||||
Assert.Equal(TaskStatus.Queued, await GetStatusAsync(id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EnqueueAsync_CleanBaseRepo_NoBaseDirtyWarning()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
await SetListWorkingDirAsync(repo.RepoDir);
|
||||
var id = await SeedTaskAsync(TaskStatus.Idle);
|
||||
|
||||
var result = await _sut.EnqueueAsync(id, default);
|
||||
|
||||
Assert.True(result.Ok);
|
||||
Assert.Null(result.BaseDirty);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EnqueueAsync_UntrackedFileInBaseRepo_ReturnsBaseDirtyWarning()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "new-file.txt"), "not committed");
|
||||
await SetListWorkingDirAsync(repo.RepoDir);
|
||||
var id = await SeedTaskAsync(TaskStatus.Idle);
|
||||
|
||||
var result = await _sut.EnqueueAsync(id, default);
|
||||
|
||||
Assert.True(result.Ok);
|
||||
Assert.NotNull(result.BaseDirty);
|
||||
Assert.Equal(0, result.BaseDirty!.ModifiedCount);
|
||||
Assert.Equal(1, result.BaseDirty!.UntrackedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EnqueueAsync_ModifiedFileInBaseRepo_ReturnsBaseDirtyWarning()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "changed on disk");
|
||||
await SetListWorkingDirAsync(repo.RepoDir);
|
||||
var id = await SeedTaskAsync(TaskStatus.Idle);
|
||||
|
||||
var result = await _sut.EnqueueAsync(id, default);
|
||||
|
||||
Assert.True(result.Ok);
|
||||
Assert.NotNull(result.BaseDirty);
|
||||
Assert.Equal(1, result.BaseDirty!.ModifiedCount);
|
||||
Assert.Equal(0, result.BaseDirty!.UntrackedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task EnqueueAsync_ManualTask_Rejected_AndStaysIdle()
|
||||
{
|
||||
|
||||
@@ -60,10 +60,10 @@ sealed class FakeWorkerClient : IWorkerClient
|
||||
public Task<List<string>> InstallSessionSkillAsync(string url) => Task.FromResult(new List<string>());
|
||||
public Task UpdateSessionSkillAsync(string sourceUrl) => Task.CompletedTask;
|
||||
public Task RemoveSessionSkillAsync(string sourceUrl) => Task.CompletedTask;
|
||||
public Task SetTaskStatusAsync(string taskId, TaskStatus status)
|
||||
public Task<BaseDirtyWarningDto?> SetTaskStatusAsync(string taskId, TaskStatus status)
|
||||
{
|
||||
SetTaskStatusCalls.Add((taskId, status));
|
||||
return Task.CompletedTask;
|
||||
return Task.FromResult<BaseDirtyWarningDto?>(null);
|
||||
}
|
||||
public Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch) => Task.FromResult<MergeResultDto?>(null);
|
||||
public Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch) => Task.FromResult<MergePreviewDto?>(null);
|
||||
|
||||
Reference in New Issue
Block a user