fix(worker): harden CLI injection, stuck-Running, chain wedge, and Fail guard

1. ArgumentList (fix injection): ClaudeArgsBuilder.Build() now returns
   IReadOnlyList<string>; ClaudeProcess populates ProcessStartInfo.ArgumentList
   instead of Arguments, so values like system prompts are never shell-split.
   DailyPrepPrompt, RefinePrompt, and WeekReportService migrated similarly.
   All IClaudeProcess fakes updated.

2. ContinueAsync exception guard: wrap RunOnceAsync in try/catch matching
   the RunAsync pattern so an unexpected exception never leaves the task
   stuck in Running status.

3. Planning chain cascade: OnChildFinishedAsync now calls CancelAsync on
   the immediate blocked successor when a child fails or is cancelled,
   triggering a recursive cascade that clears the entire remaining chain
   instead of leaving it wedged.

4. FailAsync guard: restrict valid source states to Running and Queued;
   WaitingForReview -> Failed is now rejected, preventing an invalid
   transition that could corrupt the review workflow.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
mika kuns
2026-06-09 10:05:40 +02:00
parent 763732a9b3
commit 33bdff8a6e
20 changed files with 331 additions and 99 deletions

View File

@@ -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));
}
}

View File

@@ -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;
/// <summary>
/// Verifies that ContinueAsync wraps RunOnceAsync exceptions so the task
/// is never left stuck in Running status on an unexpected error.
/// </summary>
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<WorktreeManager>.Instance);
return new TaskRunner(claude, dbFactory, broadcaster, wt, new ClaudeArgsBuilder(), _cfg,
NullLogger<TaskRunner>.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<RunResult> RunAsync(
IReadOnlyList<string> arguments, string prompt, string workingDirectory,
Func<string, Task> onStdoutLine, CancellationToken ct)
=> throw _ex;
}
}