Merge branch 'claudedo/9f963a215bb34ca4a28050d4f39178d3'
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -414,6 +420,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;
|
||||
@@ -139,7 +140,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