Merge subtask

This commit is contained in:
mika kuns
2026-08-10 15:05:11 +02:00
10 changed files with 267 additions and 7 deletions
@@ -41,12 +41,8 @@ public sealed class ClaudeArgsBuilder
"--verbose", "--verbose",
}; };
var permissionMode = string.IsNullOrWhiteSpace(config.PermissionMode)
|| config.PermissionMode.Equals("bypassPermissions", StringComparison.OrdinalIgnoreCase)
? "auto"
: config.PermissionMode;
args.Add("--permission-mode"); args.Add("--permission-mode");
args.Add(permissionMode); args.Add(PermissionModeResolver.Resolve(config.Model, config.PermissionMode));
if (config.Model is not null) if (config.Model is not null)
{ {
@@ -127,6 +127,7 @@ public sealed class ClaudeProcess : IClaudeProcess
ResultSubtype = streamResult.ResultSubtype, ResultSubtype = streamResult.ResultSubtype,
TerminalReason = streamResult.TerminalReason, TerminalReason = streamResult.TerminalReason,
Errors = streamResult.Errors, Errors = streamResult.Errors,
PermissionDenials = streamResult.PermissionDenials,
}; };
} }
@@ -150,6 +151,7 @@ public sealed class ClaudeProcess : IClaudeProcess
ResultSubtype = streamResult.ResultSubtype, ResultSubtype = streamResult.ResultSubtype,
TerminalReason = streamResult.TerminalReason, TerminalReason = streamResult.TerminalReason,
Errors = streamResult.Errors, Errors = streamResult.Errors,
PermissionDenials = streamResult.PermissionDenials,
}; };
} }
} }
@@ -47,7 +47,7 @@ public static class EffectiveRunConfigResolver
maxTurns, maxTurnsSource, requestedMaxTurns, maxTurns < requestedMaxTurns, maxTurns, maxTurnsSource, requestedMaxTurns, maxTurns < requestedMaxTurns,
preset.Effort, preset.Effort,
agentPath, agentPathSource, agentPath, agentPathSource,
global.DefaultPermissionMode, PermissionModeResolver.Resolve(model, global.DefaultPermissionMode),
systemPromptSources.Count > 0, systemPromptSources); systemPromptSources.Count > 0, systemPromptSources);
} }
} }
@@ -0,0 +1,28 @@
using ClaudeDo.Data.Models;
namespace ClaudeDo.Worker.Runner;
/// <summary>Single source of truth for the permission mode actually started, shared by
/// <see cref="ClaudeArgsBuilder"/>'s real dispatch and <see cref="EffectiveRunConfigResolver"/>'s
/// read-only report — so the two can never drift apart.</summary>
public static class PermissionModeResolver
{
public static string Resolve(string? model, string? requestedPermissionMode)
{
var mode = string.IsNullOrWhiteSpace(requestedPermissionMode)
|| requestedPermissionMode.Equals("bypassPermissions", StringComparison.OrdinalIgnoreCase)
? "auto"
: requestedPermissionMode;
// claude-cli 2.1.220 silently downgrades "--permission-mode auto" to the interactive
// "default" mode when --model resolves to a haiku model (confirmed by capturing the
// stream-json `init` event for both models side by side; acceptEdits/bypassPermissions
// pass through unaffected). An unattended run then blocks forever on an edit confirmation
// that never arrives, so substitute the closest unattended-safe mode instead.
if (string.Equals(mode, "auto", StringComparison.OrdinalIgnoreCase)
&& ModelRegistry.TryNormalizeAlias(model) == "haiku")
return "acceptEdits";
return mode;
}
}
+1
View File
@@ -14,6 +14,7 @@ public sealed record RunResult
public string? ResultSubtype { get; init; } public string? ResultSubtype { get; init; }
public string? TerminalReason { get; init; } public string? TerminalReason { get; init; }
public IReadOnlyList<string> Errors { get; init; } = Array.Empty<string>(); public IReadOnlyList<string> Errors { get; init; } = Array.Empty<string>();
public IReadOnlyList<string> PermissionDenials { get; init; } = Array.Empty<string>();
public bool IsSuccess => ExitCode == 0 && ResultMarkdown is not null; public bool IsSuccess => ExitCode == 0 && ResultMarkdown is not null;
} }
@@ -16,6 +16,7 @@ public sealed class StreamResult
public string? ResultSubtype { get; set; } public string? ResultSubtype { get; set; }
public string? TerminalReason { get; set; } public string? TerminalReason { get; set; }
public IReadOnlyList<string> Errors { get; set; } = Array.Empty<string>(); public IReadOnlyList<string> Errors { get; set; } = Array.Empty<string>();
public IReadOnlyList<string> PermissionDenials { get; set; } = Array.Empty<string>();
} }
public sealed class StreamAnalyzer public sealed class StreamAnalyzer
@@ -31,6 +32,7 @@ public sealed class StreamAnalyzer
private string? _resultSubtype; private string? _resultSubtype;
private string? _terminalReason; private string? _terminalReason;
private readonly List<string> _errors = new(); private readonly List<string> _errors = new();
private readonly List<string> _permissionDenials = new();
private const string BlockedPrefix = "CLAUDEDO_BLOCKED:"; private const string BlockedPrefix = "CLAUDEDO_BLOCKED:";
public void ProcessLine(string ndjsonLine) public void ProcessLine(string ndjsonLine)
@@ -65,6 +67,17 @@ public sealed class StreamAnalyzer
&& !string.IsNullOrEmpty(errorText)) && !string.IsNullOrEmpty(errorText))
_errors.Add(errorText); _errors.Add(errorText);
} }
// A CLI-level permission-mode failure (e.g. haiku silently downgrading
// "auto" to the interactive "default") still reports is_error:false — the
// denials only show up here, on the result event.
if (root.TryGetProperty("permission_denials", out var denialsProp)
&& denialsProp.ValueKind == JsonValueKind.Array)
{
foreach (var denial in denialsProp.EnumerateArray())
if (denial.TryGetProperty("tool_name", out var toolNameProp)
&& toolNameProp.GetString() is { } toolName && !string.IsNullOrEmpty(toolName))
_permissionDenials.Add(toolName);
}
// Authoritative token totals live on the result event. // Authoritative token totals live on the result event.
if (root.TryGetProperty("usage", out var resultUsage)) if (root.TryGetProperty("usage", out var resultUsage))
{ {
@@ -107,6 +120,7 @@ public sealed class StreamAnalyzer
ResultSubtype = _resultSubtype, ResultSubtype = _resultSubtype,
TerminalReason = _terminalReason, TerminalReason = _terminalReason,
Errors = _errors, Errors = _errors,
PermissionDenials = _permissionDenials,
}; };
private string? FallbackResult() private string? FallbackResult()
+18 -1
View File
@@ -457,9 +457,10 @@ public sealed class TaskRunner
private async Task HandleSuccess(TaskEntity task, ListEntity list, string slot, WorktreeContext? wtCtx, RunResult result, CancellationToken ct) private async Task HandleSuccess(TaskEntity task, ListEntity list, string slot, WorktreeContext? wtCtx, RunResult result, CancellationToken ct)
{ {
var committed = false;
if (wtCtx is not null) if (wtCtx is not null)
{ {
var committed = await _wtManager.CommitIfChangedAsync(wtCtx, task, list, ct); committed = await _wtManager.CommitIfChangedAsync(wtCtx, task, list, ct);
if (committed) if (committed)
{ {
await _broadcaster.WorkerLog($"Committed changes in \"{task.Title}\"", WorkerLogLevel.Info, DateTime.UtcNow); await _broadcaster.WorkerLog($"Committed changes in \"{task.Title}\"", WorkerLogLevel.Info, DateTime.UtcNow);
@@ -467,6 +468,22 @@ public sealed class TaskRunner
} }
} }
// A run can report success (exit 0, non-null result text) while every write it
// attempted was denied by the permission gate — e.g. the claude-cli haiku/"auto"
// downgrade to interactive "default" (see PermissionModeResolver). Left alone this
// lands as a normal WaitingForReview with an empty diff, and a reviewer sees only
// that emptiness with no clue why. Surface it as a failure instead.
if (!committed && result.PermissionDenials.Count > 0)
{
var tools = string.Join(", ", result.PermissionDenials.Distinct());
await MarkFailed(
task.Id, task.Title, slot,
$"All edits were blocked by permission denials ({tools}) and nothing was changed. " +
"Check the run's permission mode (get_effective_run_config).",
result.TurnCount);
return;
}
// Terminal DB write uses CancellationToken.None so the task status // Terminal DB write uses CancellationToken.None so the task status
// is never left as 'running' because of a cancel that arrived // is never left as 'running' because of a cancel that arrived
// after the Claude run already succeeded. // after the Claude run already succeeded.
@@ -170,6 +170,29 @@ public sealed class ClaudeArgsBuilderTests
Assert.DoesNotContain("--dangerously-skip-permissions", args); Assert.DoesNotContain("--dangerously-skip-permissions", args);
} }
[Theory]
[InlineData("haiku", "auto", "acceptEdits")]
[InlineData("haiku", null, "acceptEdits")]
[InlineData("haiku", "bypassPermissions", "acceptEdits")]
[InlineData("claude-haiku-4-5-20251001", "auto", "acceptEdits")]
[InlineData("haiku", "acceptEdits", "acceptEdits")]
[InlineData("haiku", "plan", "plan")]
[InlineData("sonnet", "auto", "auto")]
[InlineData("claude-sonnet-4-6", "auto", "auto")]
[InlineData(null, "auto", "auto")]
public void PermissionMode_ForHaikuModel_SubstitutesAutoWithAcceptEdits(
string? model, string? requestedMode, string expectedMode)
{
// claude-cli 2.1.220 silently downgrades "--permission-mode auto" to the interactive
// "default" mode when the resolved model is a haiku model (verified against the real
// CLI's stream-json `init` event), which would block an unattended run forever.
var args = _builder.Build(new ClaudeRunConfig(model, null, null, null, PermissionMode: requestedMode));
var list = args.ToList();
var idx = list.IndexOf("--permission-mode");
Assert.True(idx >= 0);
Assert.Equal(expectedMode, list[idx + 1]);
}
[Fact] [Fact]
public void Build_emits_mcpConfig_and_allowedTools_when_set() public void Build_emits_mcpConfig_and_allowedTools_when_set()
{ {
@@ -0,0 +1,144 @@
using ClaudeDo.Data;
using ClaudeDo.Data.Git;
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 Microsoft.Extensions.Logging.Abstractions;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
using Xunit;
namespace ClaudeDo.Worker.Tests.Runner;
/// <summary>
/// A run that reports success (exit 0, non-null result text) but whose only tool calls were
/// permission-denied — e.g. the claude-cli haiku/"auto" downgrade to interactive "default" — must
/// not land as a normal WaitingForReview with an empty diff. See PermissionModeResolver.
/// </summary>
public sealed class PermissionDenialFailureTests : IDisposable
{
private readonly List<GitRepoFixture> _repos = new();
private readonly List<DbFixture> _dbs = new();
private readonly List<(string repoDir, string wtPath)> _cleanups = new();
[Fact]
public async Task Success_withPermissionDenials_andNoChanges_endsFailed()
{
if (!GitRepoFixture.IsGitAvailable()) { Assert.True(true, "git not available -- skipping"); return; }
var repo = new GitRepoFixture(); _repos.Add(repo);
var db = new DbFixture(); _dbs.Add(db);
var dbFactory = db.CreateFactory();
var tempDir = Path.Combine(Path.GetTempPath(), $"cd_permdenial_{Guid.NewGuid():N}");
Directory.CreateDirectory(tempDir);
var cfg = new WorkerConfig { SandboxRoot = tempDir, LogRoot = tempDir, WorktreeRootStrategy = "sibling" };
using (var ctx = db.CreateContext())
{
ctx.Lists.Add(new ListEntity { Id = "l1", Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
ctx.Tasks.Add(new TaskEntity
{
Id = "t1", ListId = "l1", Title = "Edit a file", CommitType = "chore",
Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow,
});
await ctx.SaveChangesAsync();
}
// The Edit tool was denied; no file changed. Mirrors the real "result" event's shape.
var fake = new FakeClaudeProcess((_, _, _, _, _) => Task.FromResult(new RunResult
{
ExitCode = 0,
ResultMarkdown = "I need permission to edit the file.",
PermissionDenials = new[] { "Edit" },
}));
var state = TaskStateServiceBuilder.Build(dbFactory).State;
var wt = new WorktreeManager(new GitService(), dbFactory, cfg, NullLogger<WorktreeManager>.Instance);
var runner = new TaskRunner(fake, dbFactory, new HubBroadcaster(new CapturingHubContext()), wt,
new ClaudeArgsBuilder(), cfg, NullLogger<TaskRunner>.Instance, state, new TaskRunTokenRegistry(),
new AttachmentStore(), new FakeSessionSkillSeeder(), new FakeTranscriptUsageReader());
try
{
using (var ctx = db.CreateContext())
await runner.RunAsync((await new TaskRepository(ctx).GetByIdAsync("t1"))!, "slot-1", CancellationToken.None);
using var verify = db.CreateContext();
var task = (await new TaskRepository(verify).GetByIdAsync("t1"))!;
Assert.Equal(TaskStatus.Failed, task.Status);
Assert.Contains("permission", task.Result, StringComparison.OrdinalIgnoreCase);
}
finally
{
using var ctx = db.CreateContext();
var wtRow = await new WorktreeRepository(ctx).GetByTaskIdAsync("t1");
if (wtRow is not null) _cleanups.Add((repo.RepoDir, wtRow.Path));
try { Directory.Delete(tempDir, true); } catch { }
}
}
[Fact]
public async Task Success_withPermissionDenials_butSomeChangesCommitted_stillWaitsForReview()
{
if (!GitRepoFixture.IsGitAvailable()) { Assert.True(true, "git not available -- skipping"); return; }
var repo = new GitRepoFixture(); _repos.Add(repo);
var db = new DbFixture(); _dbs.Add(db);
var dbFactory = db.CreateFactory();
var tempDir = Path.Combine(Path.GetTempPath(), $"cd_permdenial_{Guid.NewGuid():N}");
Directory.CreateDirectory(tempDir);
var cfg = new WorkerConfig { SandboxRoot = tempDir, LogRoot = tempDir, WorktreeRootStrategy = "sibling" };
using (var ctx = db.CreateContext())
{
ctx.Lists.Add(new ListEntity { Id = "l1", Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
ctx.Tasks.Add(new TaskEntity
{
Id = "t1", ListId = "l1", Title = "Edit some files", CommitType = "chore",
Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow,
});
await ctx.SaveChangesAsync();
}
// One edit succeeded (a file was actually written), another was denied.
var fake = new FakeClaudeProcess((_, workingDirectory, _, _, _) =>
{
File.WriteAllText(Path.Combine(workingDirectory, "changed.txt"), "partial success");
return Task.FromResult(new RunResult
{
ExitCode = 0,
ResultMarkdown = "Made one change; one edit was denied.",
PermissionDenials = new[] { "Write" },
});
});
var state = TaskStateServiceBuilder.Build(dbFactory).State;
var wt = new WorktreeManager(new GitService(), dbFactory, cfg, NullLogger<WorktreeManager>.Instance);
var runner = new TaskRunner(fake, dbFactory, new HubBroadcaster(new CapturingHubContext()), wt,
new ClaudeArgsBuilder(), cfg, NullLogger<TaskRunner>.Instance, state, new TaskRunTokenRegistry(),
new AttachmentStore(), new FakeSessionSkillSeeder(), new FakeTranscriptUsageReader());
try
{
using (var ctx = db.CreateContext())
await runner.RunAsync((await new TaskRepository(ctx).GetByIdAsync("t1"))!, "slot-1", CancellationToken.None);
using var verify = db.CreateContext();
var task = (await new TaskRepository(verify).GetByIdAsync("t1"))!;
Assert.Equal(TaskStatus.WaitingForReview, task.Status);
}
finally
{
using var ctx = db.CreateContext();
var wtRow = await new WorktreeRepository(ctx).GetByTaskIdAsync("t1");
if (wtRow is not null) _cleanups.Add((repo.RepoDir, wtRow.Path));
try { Directory.Delete(tempDir, true); } catch { }
}
}
public void Dispose()
{
foreach (var (repoDir, wtPath) in _cleanups)
try { GitRepoFixture.RunGit(repoDir, "worktree", "remove", "--force", wtPath); } catch { }
foreach (var r in _repos) r.Dispose();
foreach (var d in _dbs) d.Dispose();
}
}
@@ -186,6 +186,41 @@ public sealed class StreamAnalyzerTests
Assert.Empty(result.Errors); Assert.Empty(result.Errors);
} }
[Fact]
public void Extracts_Permission_Denials_From_Result_Event()
{
var analyzer = new StreamAnalyzer();
analyzer.ProcessLine("""
{"type":"result","result":"I need permission to edit the file.","session_id":"s1",
"permission_denials":[{"tool_name":"Edit","tool_use_id":"t1","tool_input":{}}]}
""");
var result = analyzer.GetResult();
Assert.Single(result.PermissionDenials);
Assert.Equal("Edit", result.PermissionDenials[0]);
}
[Fact]
public void No_Permission_Denials_Field_Means_Empty_List()
{
var analyzer = new StreamAnalyzer();
analyzer.ProcessLine("""{"type":"result","result":"done","session_id":"s1"}""");
Assert.Empty(analyzer.GetResult().PermissionDenials);
}
[Fact]
public void Multiple_Permission_Denials_Are_All_Collected()
{
var analyzer = new StreamAnalyzer();
analyzer.ProcessLine("""
{"type":"result","result":"blocked","session_id":"s1","permission_denials":[
{"tool_name":"Edit","tool_use_id":"t1","tool_input":{}},
{"tool_name":"Write","tool_use_id":"t2","tool_input":{}}
]}
""");
var result = analyzer.GetResult();
Assert.Equal(new[] { "Edit", "Write" }, result.PermissionDenials);
}
[Fact] [Fact]
public void Duplicate_Marker_In_Assistant_And_Result_Is_Collected_Once() public void Duplicate_Marker_In_Assistant_And_Result_Is_Collected_Once()
{ {