diff --git a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs index fe8f102..7e87de8 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningChainCoordinator.cs @@ -87,12 +87,8 @@ public sealed class PlanningChainCoordinator public async Task OnChildFinishedAsync( string childTaskId, TaskStatus finalStatus, CancellationToken ct = default) { - if (finalStatus != TaskStatus.Done) return null; - await using var ctx = await _dbFactory.CreateDbContextAsync(ct); // The successor is whichever sibling explicitly blocks on this child. - // No status check — UnblockAsync flips legacy Waiting to Queued and is a no-op - // for already-Queued rows in the new layout. var nextId = await ctx.Tasks .AsNoTracking() .Where(t => t.BlockedByTaskId == childTaskId) @@ -101,7 +97,16 @@ public sealed class PlanningChainCoordinator .FirstOrDefaultAsync(ct); if (nextId is null) return null; - await _state().UnblockAsync(nextId, ct); - return nextId; + if (finalStatus == TaskStatus.Done) + { + await _state().UnblockAsync(nextId, ct); + return nextId; + } + + // Child failed or was cancelled: cancel the immediate successor so the chain + // is not left wedged. CancelAsync triggers OnChildTerminalAsync → OnChildFinishedAsync + // for that successor, cascading cancellation through the rest of the chain. + await _state().CancelAsync(nextId, DateTime.UtcNow, ct); + return null; } } diff --git a/src/ClaudeDo.Worker/Prime/DailyPrepPrompt.cs b/src/ClaudeDo.Worker/Prime/DailyPrepPrompt.cs index 2c99506..89ebd6a 100644 --- a/src/ClaudeDo.Worker/Prime/DailyPrepPrompt.cs +++ b/src/ClaudeDo.Worker/Prime/DailyPrepPrompt.cs @@ -8,10 +8,13 @@ public static class DailyPrepPrompt public static string LogPath() => System.IO.Path.Combine(ClaudeDo.Data.Paths.AppDataRoot(), "logs", "daily-prep.log"); - public static string BuildArgs(int maxTurns) => - "-p --output-format stream-json --verbose --permission-mode acceptEdits " + - $"--max-turns {maxTurns} " + - $"--allowedTools {CandidatesTool} {SetMyDayTool}"; + public static IReadOnlyList BuildArgs(int maxTurns) => + [ + "-p", "--output-format", "stream-json", "--verbose", + "--permission-mode", "acceptEdits", + "--max-turns", maxTurns.ToString(), + "--allowedTools", CandidatesTool, SetMyDayTool, + ]; public static string BuildPrompt(int maxTasks, DateOnly today) => ClaudeDo.Data.PromptFiles.Render( diff --git a/src/ClaudeDo.Worker/Refine/RefinePrompt.cs b/src/ClaudeDo.Worker/Refine/RefinePrompt.cs index 6dc5401..bf2bd89 100644 --- a/src/ClaudeDo.Worker/Refine/RefinePrompt.cs +++ b/src/ClaudeDo.Worker/Refine/RefinePrompt.cs @@ -12,13 +12,22 @@ public static class RefinePrompt public static string LogPath(string taskId) => System.IO.Path.Combine(Paths.AppDataRoot(), "logs", $"refine-{Short(taskId)}.log"); - public static string BuildArgs(int maxTurns, bool canReadRepo) + public static IReadOnlyList BuildArgs(int maxTurns, bool canReadRepo) { - var tools = canReadRepo - ? $"{GetTaskTool} {UpdateTaskTool} {AddSubtaskTool} Read Grep Glob" - : $"{GetTaskTool} {UpdateTaskTool} {AddSubtaskTool}"; - return "-p --output-format stream-json --verbose --permission-mode acceptEdits " + - $"--max-turns {maxTurns} --allowedTools {tools}"; + var args = new List + { + "-p", "--output-format", "stream-json", "--verbose", + "--permission-mode", "acceptEdits", + "--max-turns", maxTurns.ToString(), + "--allowedTools", GetTaskTool, UpdateTaskTool, AddSubtaskTool, + }; + if (canReadRepo) + { + args.Add("Read"); + args.Add("Grep"); + args.Add("Glob"); + } + return args; } public static string BuildPrompt(TaskEntity task, IEnumerable subtasks) diff --git a/src/ClaudeDo.Worker/Report/WeekReportService.cs b/src/ClaudeDo.Worker/Report/WeekReportService.cs index 4e8f3e2..da05195 100644 --- a/src/ClaudeDo.Worker/Report/WeekReportService.cs +++ b/src/ClaudeDo.Worker/Report/WeekReportService.cs @@ -69,7 +69,12 @@ public sealed class WeekReportService : IWeekReportService // alphanumerics, dashes and dots only. var safeModel = new string(model.Where(c => char.IsLetterOrDigit(c) || c is '-' or '.').ToArray()); if (safeModel.Length == 0) safeModel = "sonnet"; - var args = $"-p --output-format stream-json --verbose --permission-mode auto --model {safeModel}"; + IReadOnlyList args = + [ + "-p", "--output-format", "stream-json", "--verbose", + "--permission-mode", "auto", + "--model", safeModel, + ]; var result = await _claude.RunAsync(args, prompt, Path.GetTempPath(), _ => Task.CompletedTask, ct); if (!result.IsSuccess) throw new InvalidOperationException(result.ErrorMarkdown ?? "Claude could not generate the report."); diff --git a/src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs b/src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs index f9f33fc..162891a 100644 --- a/src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs +++ b/src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs @@ -27,12 +27,12 @@ public sealed class ClaudeArgsBuilder required = new[] { "summary" }, }); - public string Build(ClaudeRunConfig config) + public IReadOnlyList Build(ClaudeRunConfig config) { var args = new List { "-p", - "--output-format stream-json", + "--output-format", "stream-json", "--verbose", }; @@ -40,50 +40,55 @@ public sealed class ClaudeArgsBuilder || config.PermissionMode.Equals("bypassPermissions", StringComparison.OrdinalIgnoreCase) ? "auto" : config.PermissionMode; - args.Add($"--permission-mode {permissionMode}"); + args.Add("--permission-mode"); + args.Add(permissionMode); if (config.Model is not null) - args.Add($"--model {config.Model}"); + { + args.Add("--model"); + args.Add(config.Model); + } if (config.MaxTurns is int turns && turns > 0) - args.Add($"--max-turns {turns}"); + { + args.Add("--max-turns"); + args.Add(turns.ToString()); + } if (config.SystemPrompt is not null) - args.Add($"--append-system-prompt {Escape(config.SystemPrompt)}"); + { + args.Add("--append-system-prompt"); + args.Add(config.SystemPrompt); + } if (config.AgentPath is not null) { var agentJson = JsonSerializer.Serialize(new[] { new { file = config.AgentPath } }); - args.Add($"--agents {Escape(agentJson)}"); + args.Add("--agents"); + args.Add(agentJson); } - args.Add($"--json-schema {Escape(ResultSchema)}"); + args.Add("--json-schema"); + args.Add(ResultSchema); if (config.McpConfigPath is not null) - args.Add($"--mcp-config {Escape(config.McpConfigPath)}"); + { + args.Add("--mcp-config"); + args.Add(config.McpConfigPath); + } if (config.AllowedTools is not null) - args.Add($"--allowedTools {config.AllowedTools}"); + { + args.Add("--allowedTools"); + args.Add(config.AllowedTools); + } if (config.ResumeSessionId is not null) - args.Add($"--resume {config.ResumeSessionId}"); - - return string.Join(" ", args); - } - - private static string Escape(string value) - { - if (value.Contains(' ') || value.Contains('"') || value.Contains('\'') - || value.Contains('\t') || value.Contains('\n') || value.Contains('\r')) { - var escaped = value - .Replace("\\", "\\\\") - .Replace("\"", "\\\"") - .Replace("\n", "\\n") - .Replace("\r", "\\r") - .Replace("\t", "\\t"); - return $"\"{escaped}\""; + args.Add("--resume"); + args.Add(config.ResumeSessionId); } - return value; + + return args; } } diff --git a/src/ClaudeDo.Worker/Runner/ClaudeProcess.cs b/src/ClaudeDo.Worker/Runner/ClaudeProcess.cs index fb94b96..6d383f5 100644 --- a/src/ClaudeDo.Worker/Runner/ClaudeProcess.cs +++ b/src/ClaudeDo.Worker/Runner/ClaudeProcess.cs @@ -18,7 +18,7 @@ public sealed class ClaudeProcess : IClaudeProcess } public async Task RunAsync( - string arguments, + IReadOnlyList arguments, string prompt, string workingDirectory, Func onStdoutLine, @@ -27,7 +27,6 @@ public sealed class ClaudeProcess : IClaudeProcess var psi = new ProcessStartInfo { FileName = _cfg.ClaudeBin, - Arguments = arguments, WorkingDirectory = workingDirectory, RedirectStandardInput = true, RedirectStandardOutput = true, @@ -37,6 +36,8 @@ public sealed class ClaudeProcess : IClaudeProcess StandardOutputEncoding = Encoding.UTF8, StandardErrorEncoding = Encoding.UTF8, }; + foreach (var arg in arguments) + psi.ArgumentList.Add(arg); using var process = new Process { StartInfo = psi }; process.Start(); diff --git a/src/ClaudeDo.Worker/Runner/Interfaces/IClaudeProcess.cs b/src/ClaudeDo.Worker/Runner/Interfaces/IClaudeProcess.cs index ebd18b8..7a800f1 100644 --- a/src/ClaudeDo.Worker/Runner/Interfaces/IClaudeProcess.cs +++ b/src/ClaudeDo.Worker/Runner/Interfaces/IClaudeProcess.cs @@ -3,7 +3,7 @@ namespace ClaudeDo.Worker.Runner; public interface IClaudeProcess { Task RunAsync( - string arguments, + IReadOnlyList arguments, string prompt, string workingDirectory, Func onStdoutLine, diff --git a/src/ClaudeDo.Worker/Runner/TaskRunner.cs b/src/ClaudeDo.Worker/Runner/TaskRunner.cs index 816d8b4..b481d86 100644 --- a/src/ClaudeDo.Worker/Runner/TaskRunner.cs +++ b/src/ClaudeDo.Worker/Runner/TaskRunner.cs @@ -211,19 +211,32 @@ public sealed class TaskRunner await _state.StartRunningAsync(taskId, now, ct); await _broadcaster.TaskStarted(slot, taskId, now); - var nextRunNumber = lastRun.RunNumber + 1; - var result = await RunOnceAsync(taskId, task.Title, slot, runDir, resolvedConfig, nextRunNumber, false, followUpPrompt, ct); - - if (result.IsSuccess) + try { - await HandleSuccess(task, list, slot, wtCtx, result, ct); - } - else - { - await MarkFailed(taskId, task.Title, slot, result.ErrorMarkdown, result.TurnCount); - } + var nextRunNumber = lastRun.RunNumber + 1; + var result = await RunOnceAsync(taskId, task.Title, slot, runDir, resolvedConfig, nextRunNumber, false, followUpPrompt, ct); - await _broadcaster.TaskUpdated(taskId); + if (result.IsSuccess) + { + await HandleSuccess(task, list, slot, wtCtx, result, ct); + } + else + { + await MarkFailed(taskId, task.Title, slot, result.ErrorMarkdown, result.TurnCount); + } + + await _broadcaster.TaskUpdated(taskId); + } + catch (OperationCanceledException) + { + _logger.LogInformation("Task {TaskId} was cancelled during continue", taskId); + await MarkFailed(taskId, task.Title, slot, "Task cancelled."); + } + catch (Exception ex) + { + _logger.LogError(ex, "Unhandled exception continuing task {TaskId}", taskId); + await MarkFailed(taskId, task.Title, slot, $"Unhandled error: {ex.Message}"); + } } private readonly record struct RunDirResult(string? RunDir, WorktreeContext? WtCtx, string? FailureReason); diff --git a/src/ClaudeDo.Worker/State/TaskStateService.cs b/src/ClaudeDo.Worker/State/TaskStateService.cs index 17dbe8d..d756f20 100644 --- a/src/ClaudeDo.Worker/State/TaskStateService.cs +++ b/src/ClaudeDo.Worker/State/TaskStateService.cs @@ -196,14 +196,15 @@ public sealed class TaskStateService : ITaskStateService await using (var ctx = await _dbFactory.CreateDbContextAsync(ct)) { var affected = await ctx.Tasks - .Where(t => t.Id == taskId && t.Status != TaskStatus.Done) + .Where(t => t.Id == taskId && + (t.Status == TaskStatus.Running || t.Status == TaskStatus.Queued)) .ExecuteUpdateAsync(s => s .SetProperty(t => t.Status, TaskStatus.Failed) .SetProperty(t => t.FinishedAt, finishedAt) .SetProperty(t => t.Result, error), ct); if (affected == 0) - return new TransitionResult(false, "Task already done; cannot fail."); + return new TransitionResult(false, "Task not in a failable state (must be Running or Queued)."); } await OnChildTerminalAsync(taskId, TaskStatus.Failed); diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs index 1e7b9d5..d2b7239 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningChainCoordinatorTests.cs @@ -125,7 +125,7 @@ public sealed class PlanningChainCoordinatorTests : IDisposable } [Fact] - public async Task OnChildFailed_DoesNotAdvanceChain() + public async Task OnChildFailed_CancelsPendingSuccessors_ChainIsNotWedged() { await SeedPlanningFamilyAsync("P", 3); await _sut.SetupChainAsync("P", default); @@ -139,12 +139,43 @@ public sealed class PlanningChainCoordinatorTests : IDisposable var advanced = await _sut.OnChildFinishedAsync("P-c0", TaskStatus.Failed, default); + // No "advancement" — the chain does not continue on failure. Assert.Null(advanced); var kids = await GetChildrenAsync("P"); Assert.Equal(TaskStatus.Failed, kids[0].Status); - // Successors remain blocked on the failed predecessor. - Assert.Equal(kids[0].Id, kids[1].BlockedByTaskId); - Assert.Equal(kids[1].Id, kids[2].BlockedByTaskId); + // Successors must be Cancelled, not left stuck as Queued. + Assert.Equal(TaskStatus.Cancelled, kids[1].Status); + Assert.Equal(TaskStatus.Cancelled, kids[2].Status); + } + + [Fact] + public async Task OnChildFailed_MidChain_CancelsAllDownstreamSuccessors() + { + // Chain: c0 → c1 → c2 → c3. c1 fails mid-chain; c2 and c3 must be cancelled. + await SeedPlanningFamilyAsync("P", 4); + await _sut.SetupChainAsync("P", default); + + // Mark c0 Done so c1 was unblocked; c1 ran and failed. + await using (var ctx = _factory.CreateDbContext()) + { + var c0 = await ctx.Tasks.FirstAsync(t => t.Id == "P-c0"); + c0.Status = TaskStatus.Done; + c0.BlockedByTaskId = null; + var c1 = await ctx.Tasks.FirstAsync(t => t.Id == "P-c1"); + c1.Status = TaskStatus.Failed; + c1.BlockedByTaskId = null; + await ctx.SaveChangesAsync(); + } + + // Announce that c1 finished as Failed — the coordinator must cascade to c2/c3. + var advanced = await _sut.OnChildFinishedAsync("P-c1", TaskStatus.Failed, default); + + Assert.Null(advanced); + var kids = await GetChildrenAsync("P"); + Assert.Equal(TaskStatus.Done, kids[0].Status); + Assert.Equal(TaskStatus.Failed, kids[1].Status); + Assert.Equal(TaskStatus.Cancelled, kids[2].Status); + Assert.Equal(TaskStatus.Cancelled, kids[3].Status); } [Fact] diff --git a/tests/ClaudeDo.Worker.Tests/Prime/DailyPrepPromptTests.cs b/tests/ClaudeDo.Worker.Tests/Prime/DailyPrepPromptTests.cs index 25f0ff3..febda78 100644 --- a/tests/ClaudeDo.Worker.Tests/Prime/DailyPrepPromptTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Prime/DailyPrepPromptTests.cs @@ -19,8 +19,10 @@ public class DailyPrepPromptTests public void Build_args_allows_only_the_two_tools() { var args = DailyPrepPrompt.BuildArgs(maxTurns: 30); - Assert.Contains("--output-format stream-json", args); - Assert.Contains("--max-turns 30", args); + Assert.Contains("--output-format", args); + Assert.Contains("stream-json", args); + Assert.Contains("--max-turns", args); + Assert.Contains("30", args); Assert.Contains("--allowedTools", args); Assert.Contains("mcp__claudedo__get_daily_prep_candidates", args); Assert.Contains("mcp__claudedo__set_my_day", args); diff --git a/tests/ClaudeDo.Worker.Tests/Prime/PrimeRunnerTests.cs b/tests/ClaudeDo.Worker.Tests/Prime/PrimeRunnerTests.cs index be486df..e7acdc3 100644 --- a/tests/ClaudeDo.Worker.Tests/Prime/PrimeRunnerTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Prime/PrimeRunnerTests.cs @@ -31,7 +31,7 @@ public class PrimeRunnerTests : IDisposable } public async Task RunAsync( - string arguments, + IReadOnlyList arguments, string prompt, string workingDirectory, Func onStdoutLine, diff --git a/tests/ClaudeDo.Worker.Tests/Refine/RefinePromptTests.cs b/tests/ClaudeDo.Worker.Tests/Refine/RefinePromptTests.cs index 57e7027..0d508cb 100644 --- a/tests/ClaudeDo.Worker.Tests/Refine/RefinePromptTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Refine/RefinePromptTests.cs @@ -11,9 +11,12 @@ public sealed class RefinePromptTests { var args = RefinePrompt.BuildArgs(20, canReadRepo: true); - Assert.Contains("--permission-mode acceptEdits", args); + Assert.Contains("--permission-mode", args); + Assert.Contains("acceptEdits", args); Assert.Contains("mcp__claudedo__add_subtask", args); - Assert.Contains(" Read Grep Glob", args); + Assert.Contains("Read", args); + Assert.Contains("Grep", args); + Assert.Contains("Glob", args); } [Fact] diff --git a/tests/ClaudeDo.Worker.Tests/Refine/RefineRunnerTests.cs b/tests/ClaudeDo.Worker.Tests/Refine/RefineRunnerTests.cs index ed4b534..93b6395 100644 --- a/tests/ClaudeDo.Worker.Tests/Refine/RefineRunnerTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Refine/RefineRunnerTests.cs @@ -105,7 +105,7 @@ internal sealed class RecordingClaudeProcess : IClaudeProcess public RecordingClaudeProcess(bool success) => _success = success; - public Task RunAsync(string arguments, string prompt, string workingDirectory, + public Task RunAsync(IReadOnlyList arguments, string prompt, string workingDirectory, Func onStdoutLine, CancellationToken ct) { Interlocked.Increment(ref _callCount); diff --git a/tests/ClaudeDo.Worker.Tests/Report/WeekReportServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Report/WeekReportServiceTests.cs index ff8196b..df6370a 100644 --- a/tests/ClaudeDo.Worker.Tests/Report/WeekReportServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Report/WeekReportServiceTests.cs @@ -27,7 +27,7 @@ public class WeekReportServiceTests : IDisposable { public int Calls; public RunResult Next = new() { ExitCode = 0, ResultMarkdown = "## Bericht" }; - public Task RunAsync(string args, string prompt, string wd, Func onLine, CancellationToken ct) + public Task RunAsync(IReadOnlyList args, string prompt, string wd, Func onLine, CancellationToken ct) { Calls++; return Task.FromResult(Next); } } diff --git a/tests/ClaudeDo.Worker.Tests/Runner/ClaudeArgsBuilderTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/ClaudeArgsBuilderTests.cs index c3e894d..30e0ad6 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/ClaudeArgsBuilderTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/ClaudeArgsBuilderTests.cs @@ -11,9 +11,11 @@ public sealed class ClaudeArgsBuilderTests { var args = _builder.Build(new ClaudeRunConfig(null, null, null, null)); Assert.Contains("-p", args); - Assert.Contains("--output-format stream-json", args); + Assert.Contains("--output-format", args); + Assert.Contains("stream-json", args); Assert.Contains("--verbose", args); - Assert.Contains("--permission-mode auto", args); + Assert.Contains("--permission-mode", args); + Assert.Contains("auto", args); Assert.DoesNotContain("--dangerously-skip-permissions", args); Assert.Contains("--json-schema", args); Assert.DoesNotContain("--model", args); @@ -26,7 +28,8 @@ public sealed class ClaudeArgsBuilderTests public void Model_Adds_Model_Flag() { var args = _builder.Build(new ClaudeRunConfig("sonnet-4-6", null, null, null)); - Assert.Contains("--model sonnet-4-6", args); + Assert.Contains("--model", args); + Assert.Contains("sonnet-4-6", args); } [Fact] @@ -42,63 +45,74 @@ public sealed class ClaudeArgsBuilderTests { var args = _builder.Build(new ClaudeRunConfig(null, null, "/path/to/agent.md", null)); Assert.Contains("--agents", args); - Assert.Contains("/path/to/agent.md", args); + Assert.Contains(args, a => a.Contains("/path/to/agent.md")); } [Fact] public void ResumeSessionId_Adds_Resume_Flag() { var args = _builder.Build(new ClaudeRunConfig(null, null, null, "sess-abc-123")); - Assert.Contains("--resume sess-abc-123", args); + Assert.Contains("--resume", args); + Assert.Contains("sess-abc-123", args); } [Fact] public void All_Options_Set_Includes_All_Flags() { var args = _builder.Build(new ClaudeRunConfig("opus-4-6", "Be thorough.", "/agents/dev.md", "sess-xyz")); - Assert.Contains("--model opus-4-6", args); + Assert.Contains("--model", args); + Assert.Contains("opus-4-6", args); Assert.Contains("--append-system-prompt", args); Assert.Contains("--agents", args); - Assert.Contains("--resume sess-xyz", args); + Assert.Contains("--resume", args); + Assert.Contains("sess-xyz", args); Assert.Contains("--json-schema", args); } [Fact] - public void SystemPrompt_With_Quotes_Is_Escaped() + public void SystemPrompt_With_Quotes_Is_Passed_Verbatim() { - var args = _builder.Build(new ClaudeRunConfig(null, """Don't say "hello".""", null, null)); + var prompt = """Don't say "hello"."""; + var args = _builder.Build(new ClaudeRunConfig(null, prompt, null, null)); Assert.Contains("--append-system-prompt", args); + Assert.Contains(prompt, args); } [Fact] - public void Build_quotes_system_prompt_with_newline() + public void SystemPrompt_With_Newline_Is_Passed_As_Single_Element() { + var prompt = "line1\nline2"; var args = _builder.Build(new ClaudeRunConfig( Model: null, - SystemPrompt: "line1\nline2", + SystemPrompt: prompt, AgentPath: null, ResumeSessionId: null)); - Assert.Contains("--append-system-prompt \"line1\\nline2\"", args); + var list = args.ToList(); + var idx = list.IndexOf("--append-system-prompt"); + Assert.True(idx >= 0); + Assert.Equal(prompt, list[idx + 1]); } [Fact] - public void Build_quotes_system_prompt_with_tab() + public void SystemPrompt_With_Tab_Is_Passed_As_Single_Element() { + var prompt = "col1\tcol2"; var args = _builder.Build(new ClaudeRunConfig( Model: null, - SystemPrompt: "col1\tcol2", + SystemPrompt: prompt, AgentPath: null, ResumeSessionId: null)); - Assert.Contains("\"col1", args); + Assert.Contains(prompt, args); } [Fact] public void MaxTurns_Adds_Flag_When_Positive() { var args = _builder.Build(new ClaudeRunConfig(null, null, null, null, MaxTurns: 25)); - Assert.Contains("--max-turns 25", args); + Assert.Contains("--max-turns", args); + Assert.Contains("25", args); } [Fact] @@ -114,7 +128,8 @@ public sealed class ClaudeArgsBuilderTests public void PermissionMode_bypass_Maps_To_Auto() { var args = _builder.Build(new ClaudeRunConfig(null, null, null, null, PermissionMode: "bypassPermissions")); - Assert.Contains("--permission-mode auto", args); + Assert.Contains("--permission-mode", args); + Assert.Contains("auto", args); Assert.DoesNotContain("--dangerously-skip-permissions", args); } @@ -122,7 +137,8 @@ public sealed class ClaudeArgsBuilderTests public void PermissionMode_acceptEdits_Emits_PermissionMode_Flag() { var args = _builder.Build(new ClaudeRunConfig(null, null, null, null, PermissionMode: "acceptEdits")); - Assert.Contains("--permission-mode acceptEdits", args); + Assert.Contains("--permission-mode", args); + Assert.Contains("acceptEdits", args); Assert.DoesNotContain("--dangerously-skip-permissions", args); } @@ -130,7 +146,8 @@ public sealed class ClaudeArgsBuilderTests public void PermissionMode_Null_Defaults_To_Auto() { var args = _builder.Build(new ClaudeRunConfig(null, null, null, null)); - Assert.Contains("--permission-mode auto", args); + Assert.Contains("--permission-mode", args); + Assert.Contains("auto", args); Assert.DoesNotContain("--dangerously-skip-permissions", args); } @@ -142,8 +159,26 @@ public sealed class ClaudeArgsBuilderTests McpConfigPath: "C:\\tmp\\t_mcp.json", AllowedTools: "mcp__claudedo_run__SuggestImprovement")); Assert.Contains("--mcp-config", args); - Assert.Contains("t_mcp.json", args); - Assert.Contains("--allowedTools mcp__claudedo_run__SuggestImprovement", args); + Assert.Contains(args, a => a.Contains("t_mcp.json")); + Assert.Contains("--allowedTools", args); + Assert.Contains("mcp__claudedo_run__SuggestImprovement", args); + } + + [Fact] + public void SystemPrompt_ContainingDangerousFlag_IsPassedAsLiteral_NotAsFlag() + { + // If the system prompt contains "--dangerously-skip-permissions", it must arrive + // as the VALUE of --append-system-prompt, not as a standalone CLI flag. + const string dangerousPrompt = "--dangerously-skip-permissions"; + var args = _builder.Build(new ClaudeRunConfig(null, dangerousPrompt, null, null)); + + var list = args.ToList(); + var flagIdx = list.IndexOf("--append-system-prompt"); + Assert.True(flagIdx >= 0, "--append-system-prompt flag must be present"); + // The dangerous string sits immediately after the flag as its value. + Assert.Equal(dangerousPrompt, list[flagIdx + 1]); + // It does NOT appear as a separate element (i.e., not treated as its own flag). + Assert.Equal(1, list.Count(a => a == dangerousPrompt)); } } diff --git a/tests/ClaudeDo.Worker.Tests/Runner/ContinueAsyncExceptionTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/ContinueAsyncExceptionTests.cs new file mode 100644 index 0000000..3e1769e --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Runner/ContinueAsyncExceptionTests.cs @@ -0,0 +1,106 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Config; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Runner; +using ClaudeDo.Worker.Tests.Infrastructure; +using ClaudeDo.Worker.Tests.Services; +using Microsoft.Extensions.Logging.Abstractions; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Tests.Runner; + +/// +/// Verifies that ContinueAsync wraps RunOnceAsync exceptions so the task +/// is never left stuck in Running status on an unexpected error. +/// +public sealed class ContinueAsyncExceptionTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly string _tempDir; + private readonly WorkerConfig _cfg; + + public ContinueAsyncExceptionTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), $"cd_continue_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tempDir); + _cfg = new WorkerConfig { SandboxRoot = _tempDir, LogRoot = _tempDir }; + } + + public void Dispose() { _db.Dispose(); try { Directory.Delete(_tempDir, true); } catch { } } + + private TaskRunner BuildRunner(IClaudeProcess claude, ClaudeDoDbContext ctx) + { + var dbFactory = _db.CreateFactory(); + var broadcaster = new HubBroadcaster(new FakeHubContext()); + var state = TaskStateServiceBuilder.Build(dbFactory).State; + var wt = new WorktreeManager(new ClaudeDo.Data.Git.GitService(), dbFactory, _cfg, NullLogger.Instance); + return new TaskRunner(claude, dbFactory, broadcaster, wt, new ClaudeArgsBuilder(), _cfg, + NullLogger.Instance, state, new TaskRunTokenRegistry()); + } + + [Fact] + public async Task ContinueAsync_UnhandledException_MarksTaskFailed_NotStuckRunning() + { + var dbFactory = _db.CreateFactory(); + string listId, taskId; + + using (var ctx = _db.CreateContext()) + { + listId = Guid.NewGuid().ToString(); + ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = null, CreatedAt = DateTime.UtcNow }); + + taskId = Guid.NewGuid().ToString(); + ctx.Tasks.Add(new TaskEntity + { + Id = taskId, + ListId = listId, + Title = "Continue me", + Status = TaskStatus.WaitingForReview, + CreatedAt = DateTime.UtcNow, + }); + await ctx.SaveChangesAsync(); + + // A prior run with a session ID is required for ContinueAsync to proceed. + await new TaskRunRepository(ctx).AddAsync(new TaskRunEntity + { + Id = Guid.NewGuid().ToString(), + TaskId = taskId, + RunNumber = 1, + IsRetry = false, + Prompt = "original prompt", + SessionId = "sess-continue-test", + StartedAt = DateTime.UtcNow.AddMinutes(-5), + FinishedAt = DateTime.UtcNow.AddMinutes(-1), + ExitCode = 0, + ResultMarkdown = "first result", + }); + } + + // This process throws a non-cancellation exception to simulate an unexpected failure. + var throwingProcess = new ThrowingClaudeProcess(new InvalidOperationException("disk full")); + + using var ctx2 = _db.CreateContext(); + var runner = BuildRunner(throwingProcess, ctx2); + + // ContinueAsync must not propagate the exception and must leave the task in Failed. + await runner.ContinueAsync(taskId, "please continue", "slot-1", CancellationToken.None); + + using var verify = _db.CreateContext(); + var task = await new TaskRepository(verify).GetByIdAsync(taskId); + Assert.NotNull(task); + Assert.Equal(TaskStatus.Failed, task.Status); + } + + private sealed class ThrowingClaudeProcess : IClaudeProcess + { + private readonly Exception _ex; + public ThrowingClaudeProcess(Exception ex) => _ex = ex; + + public Task RunAsync( + IReadOnlyList arguments, string prompt, string workingDirectory, + Func onStdoutLine, CancellationToken ct) + => throw _ex; + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Services/QueueServiceSlotGuardTests.cs b/tests/ClaudeDo.Worker.Tests/Services/QueueServiceSlotGuardTests.cs index 3a0e796..61dc6b2 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/QueueServiceSlotGuardTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/QueueServiceSlotGuardTests.cs @@ -46,7 +46,7 @@ public sealed class QueueServiceSlotGuardTests : IDisposable private QueueWaker _waker = null!; private (QueueService service, FakeClaudeProcess fakeProcess) CreateService( - Func, CancellationToken, Task>? handler = null) + Func, Func, CancellationToken, Task>? handler = null) { var fake = new FakeClaudeProcess(handler); var broadcaster = new HubBroadcaster(new FakeHubContext()); diff --git a/tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs index a0db50b..b67be6d 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/QueueServiceTests.cs @@ -47,7 +47,7 @@ public sealed class QueueServiceTests : IDisposable private QueueWaker _waker = null!; private (QueueService service, FakeClaudeProcess fakeProcess) CreateService( - Func, CancellationToken, Task>? handler = null) + Func, Func, CancellationToken, Task>? handler = null) { var fake = new FakeClaudeProcess(handler); var broadcaster = new HubBroadcaster(new FakeHubContext()); @@ -118,7 +118,7 @@ public sealed class QueueServiceTests : IDisposable { var (listId, _) = await SeedListWithAgentTag(); - string? capturedArgs = null; + IReadOnlyList? capturedArgs = null; string? capturedPrompt = null; var done = new TaskCompletionSource(); @@ -157,7 +157,9 @@ public sealed class QueueServiceTests : IDisposable _waker.Wake(); await done.Task.WaitAsync(TimeSpan.FromSeconds(5)); - Assert.Contains("--resume sess-1", capturedArgs); + Assert.NotNull(capturedArgs); + Assert.Contains("--resume", capturedArgs); + Assert.Contains("sess-1", capturedArgs); Assert.Equal("fix the bug", capturedPrompt); // Feedback is cleared after the run reaches a successful terminal state (post-run), @@ -346,19 +348,19 @@ public sealed class QueueServiceTests : IDisposable internal sealed class FakeClaudeProcess : IClaudeProcess { - private readonly Func, CancellationToken, Task> _handler; + private readonly Func, Func, CancellationToken, Task> _handler; private int _callCount; public int CallCount => _callCount; public FakeClaudeProcess( - Func, CancellationToken, Task>? handler = null) + Func, Func, CancellationToken, Task>? handler = null) { _handler = handler ?? ((_, _, _, _, _) => Task.FromResult(new RunResult { ExitCode = 0, ResultMarkdown = "ok" })); } - public async Task RunAsync(string arguments, string prompt, string workingDirectory, + public async Task RunAsync(IReadOnlyList arguments, string prompt, string workingDirectory, Func onStdoutLine, CancellationToken ct) { Interlocked.Increment(ref _callCount); diff --git a/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs b/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs index 7ec4953..531a6e3 100644 --- a/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/State/TaskStateServiceTests.cs @@ -243,6 +243,17 @@ public sealed class TaskStateServiceTests : IDisposable Assert.Equal(TaskStatus.Done, await GetStatusAsync(id)); } + [Fact] + public async Task FailAsync_FromWaitingForReview_IsNoOp() + { + var id = await SeedTaskAsync(TaskStatus.WaitingForReview); + + var result = await _sut.FailAsync(id, DateTime.UtcNow, "oops", default); + + Assert.False(result.Ok); + Assert.Equal(TaskStatus.WaitingForReview, await GetStatusAsync(id)); + } + // ─── CancelAsync ────────────────────────────────────────────────────── [Fact]