fix(worker,ui): clean up three review leftovers from the list-handler run
Remove the redundant TaskUpdated broadcast in TaskRunner.ContinueAsync's queue-claim path, consolidate InteractiveLaunchSpecService's seven MCP_TOOL_TIMEOUT literals into one constant (fixing the merge-helper handoff spec's stale 200000ms value), and surface OpenQuickClaudeSession's two failure cases via ErrorReported/footer instead of a silent no-op, with a less ambiguous icon.
This commit is contained in:
@@ -0,0 +1,95 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Ui.ViewModels.Islands;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
|
||||
namespace ClaudeDo.Ui.Tests.ViewModels;
|
||||
|
||||
public class TasksIslandOpenQuickClaudeSessionTests : IDisposable
|
||||
{
|
||||
private readonly string _dbPath;
|
||||
|
||||
public TasksIslandOpenQuickClaudeSessionTests()
|
||||
{
|
||||
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_quickclaude_{Guid.NewGuid():N}.db");
|
||||
using var ctx = NewContext();
|
||||
ctx.Database.EnsureCreated();
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
try { File.Delete(_dbPath); } catch { }
|
||||
try { File.Delete(_dbPath + "-wal"); } catch { }
|
||||
try { File.Delete(_dbPath + "-shm"); } catch { }
|
||||
}
|
||||
|
||||
private ClaudeDoDbContext NewContext()
|
||||
{
|
||||
var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
|
||||
.UseSqlite($"Data Source={_dbPath}")
|
||||
.Options;
|
||||
return new ClaudeDoDbContext(opts);
|
||||
}
|
||||
|
||||
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
|
||||
{
|
||||
private readonly Func<ClaudeDoDbContext> _create;
|
||||
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
|
||||
public ClaudeDoDbContext CreateDbContext() => _create();
|
||||
}
|
||||
|
||||
private TasksIslandViewModel BuildViewModel() => new(new TestDbFactory(NewContext), worker: null);
|
||||
|
||||
private static ListNavItemViewModel UserList(string? workingDir) =>
|
||||
new() { Id = "user:l1", Kind = ListKind.User, Name = "L1", WorkingDir = workingDir };
|
||||
|
||||
[Fact]
|
||||
public void NoWorkingDirConfigured_ReportsError_DoesNotRaiseRequested()
|
||||
{
|
||||
var vm = BuildViewModel();
|
||||
vm.LoadForList(UserList(workingDir: null));
|
||||
string? error = null;
|
||||
var requested = false;
|
||||
vm.ErrorReported += msg => error = msg;
|
||||
vm.OpenQuickClaudeSessionRequested += _ => requested = true;
|
||||
|
||||
vm.OpenQuickClaudeSessionCommand.Execute(null);
|
||||
|
||||
Assert.NotNull(error);
|
||||
Assert.False(requested);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WorkingDirMissingOnDisk_ReportsError_DoesNotRaiseRequested()
|
||||
{
|
||||
var missingDir = Path.Combine(Path.GetTempPath(), $"claudedo_missing_{Guid.NewGuid():N}");
|
||||
var vm = BuildViewModel();
|
||||
vm.LoadForList(UserList(workingDir: missingDir));
|
||||
string? error = null;
|
||||
var requested = false;
|
||||
vm.ErrorReported += msg => error = msg;
|
||||
vm.OpenQuickClaudeSessionRequested += _ => requested = true;
|
||||
|
||||
vm.OpenQuickClaudeSessionCommand.Execute(null);
|
||||
|
||||
Assert.NotNull(error);
|
||||
Assert.Contains(missingDir, error);
|
||||
Assert.False(requested);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WorkingDirExists_RaisesRequested_WithDir_NoError()
|
||||
{
|
||||
var dir = Path.GetTempPath();
|
||||
var vm = BuildViewModel();
|
||||
vm.LoadForList(UserList(workingDir: dir));
|
||||
string? error = null;
|
||||
string? requestedDir = null;
|
||||
vm.ErrorReported += msg => error = msg;
|
||||
vm.OpenQuickClaudeSessionRequested += d => requestedDir = d;
|
||||
|
||||
vm.OpenQuickClaudeSessionCommand.Execute(null);
|
||||
|
||||
Assert.Null(error);
|
||||
Assert.Equal(dir, requestedDir);
|
||||
}
|
||||
}
|
||||
@@ -242,7 +242,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Equal(_worktreeDir, spec.Cwd);
|
||||
Assert.Equal(_claudeStubPath, spec.Exe);
|
||||
Assert.Equal(new[] { "--resume", "sess-123" }, ArgsAfterEffort(spec));
|
||||
Assert.Equal("930000", spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -401,7 +401,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Equal(_tempDir, spec.Cwd);
|
||||
Assert.Equal(_claudeStubPath, spec.Exe);
|
||||
Assert.Empty(ArgsAfterEffort(spec));
|
||||
Assert.Equal("930000", spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Empty(_seeder.Calls);
|
||||
}
|
||||
|
||||
@@ -535,7 +535,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Contains(briefPath, kickoff);
|
||||
Assert.DoesNotContain('\n', kickoff);
|
||||
|
||||
Assert.Equal("930000", spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -791,7 +791,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Contains(survivor, handoffBrief);
|
||||
Assert.Contains("phase 3", handoffBrief, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
Assert.Equal("200000", spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
}
|
||||
|
||||
private async Task<int> CountTasksAsync()
|
||||
@@ -841,4 +841,57 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Equal("tok-2", spec.Env["CLAUDEDO_PLANNING_TOKEN"]);
|
||||
Assert.Equal(_worktreeDir, spec.Cwd);
|
||||
}
|
||||
|
||||
// Regression guard for the bug where BuildForMergeHelperHandoffAsync set MCP_TOOL_TIMEOUT to
|
||||
// an older, shorter value (200000) than every other ConPTY spec (930000) after a parallel
|
||||
// merge landed the two changes independently. Every spec this service builds must carry the
|
||||
// SAME value, sourced from the one constant, so the value can never drift again.
|
||||
[Fact]
|
||||
public async Task AllBuiltSpecs_CarryTheSameMcpToolTimeout()
|
||||
{
|
||||
var repo = Path.Combine(_tempDir, "repoTimeoutCheck");
|
||||
Directory.CreateDirectory(repo);
|
||||
|
||||
var listId = await SeedListAsync(workingDir: repo, name: "Alpha");
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
await SeedTaskAsync(taskId, listId, TaskStatus.Idle);
|
||||
await SeedWorktreeAsync(taskId, WorktreeState.Active);
|
||||
await SeedRunAsync(taskId, "sess-timeout-check");
|
||||
|
||||
var handlerTaskId = Guid.NewGuid().ToString();
|
||||
await SeedTaskAsync(handlerTaskId, listId, TaskStatus.Idle, title: "Handler");
|
||||
var survivor = Guid.NewGuid().ToString();
|
||||
await SeedTaskAsync(survivor, listId, TaskStatus.WaitingForReview, title: "Survivor");
|
||||
|
||||
var sessionDir = Path.Combine(_tempDir, "sess-timeout-check");
|
||||
Directory.CreateDirectory(sessionDir);
|
||||
var planningStartCtx = new PlanningSessionStartContext(
|
||||
ParentTaskId: "p1", WorkingDir: _worktreeDir, Token: "tok-1",
|
||||
WorktreePath: _worktreeDir, BranchName: "claudedo/planning/p1",
|
||||
Files: new PlanningSessionFiles(sessionDir,
|
||||
Path.Combine(sessionDir, "system-prompt.md"),
|
||||
Path.Combine(sessionDir, "initial-prompt.txt")));
|
||||
var planningResumeCtx = new PlanningSessionResumeContext(
|
||||
ParentTaskId: "p1", WorkingDir: _worktreeDir,
|
||||
ClaudeSessionId: "sess-42", Token: "tok-2", WorktreePath: _worktreeDir);
|
||||
|
||||
var svc = BuildService();
|
||||
var mergeHelperSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, CancellationToken.None);
|
||||
TrackSessionDir(mergeHelperSpec);
|
||||
var handoffSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None);
|
||||
TrackSessionDir(handoffSpec);
|
||||
|
||||
var specs = new List<LaunchSpec>
|
||||
{
|
||||
await svc.BuildForTaskAsync(taskId, CancellationToken.None),
|
||||
await svc.BuildForDirectoryAsync(_tempDir, CancellationToken.None),
|
||||
mergeHelperSpec,
|
||||
handoffSpec,
|
||||
svc.BuildPlanningStart(planningStartCtx),
|
||||
svc.BuildPlanningResume(planningResumeCtx),
|
||||
};
|
||||
|
||||
Assert.All(specs, spec =>
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -79,4 +79,46 @@ public sealed class QueueClaimTaskUpdatedBroadcastTests : IDisposable
|
||||
releaseProcess.TrySetResult();
|
||||
await runTask;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ContinueAsync_with_alreadyClaimed_broadcasts_TaskUpdated_exactly_once_before_the_run_finishes()
|
||||
{
|
||||
string listId = Guid.NewGuid().ToString(), taskId = Guid.NewGuid().ToString();
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = null, CreatedAt = DateTime.UtcNow });
|
||||
ctx.Tasks.Add(new TaskEntity
|
||||
{
|
||||
Id = taskId, ListId = listId, Title = "T", Status = TaskStatus.Running,
|
||||
StartedAt = DateTime.UtcNow, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
ctx.TaskRuns.Add(new TaskRunEntity
|
||||
{
|
||||
Id = Guid.NewGuid().ToString(), TaskId = taskId, RunNumber = 1, IsRetry = false,
|
||||
Prompt = "p", SessionId = "sess-1",
|
||||
StartedAt = DateTime.UtcNow.AddMinutes(-5), FinishedAt = DateTime.UtcNow.AddMinutes(-1),
|
||||
ExitCode = 0, ResultMarkdown = "ok",
|
||||
});
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var processStarted = new TaskCompletionSource();
|
||||
var releaseProcess = new TaskCompletionSource();
|
||||
var fake = new FakeClaudeProcess(async (_, _, _, _, _) =>
|
||||
{
|
||||
processStarted.TrySetResult();
|
||||
await releaseProcess.Task;
|
||||
return new RunResult { ExitCode = 0, ResultMarkdown = "ok" };
|
||||
});
|
||||
var runner = BuildRunner(fake);
|
||||
|
||||
var runTask = runner.ContinueAsync(taskId, "follow up", "queue", CancellationToken.None, alreadyClaimed: true);
|
||||
|
||||
await processStarted.Task;
|
||||
|
||||
Assert.Single(_hubContext.Proxy.Calls, c => c.Method == "TaskUpdated" && (string)c.Args[0]! == taskId);
|
||||
|
||||
releaseProcess.TrySetResult();
|
||||
await runTask;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user