fix(runner): substitute acceptEdits for haiku's silent auto-mode downgrade, fail permission-denial-only runs
This commit is contained in:
@@ -41,12 +41,8 @@ public sealed class ClaudeArgsBuilder
|
||||
"--verbose",
|
||||
};
|
||||
|
||||
var permissionMode = string.IsNullOrWhiteSpace(config.PermissionMode)
|
||||
|| config.PermissionMode.Equals("bypassPermissions", StringComparison.OrdinalIgnoreCase)
|
||||
? "auto"
|
||||
: config.PermissionMode;
|
||||
args.Add("--permission-mode");
|
||||
args.Add(permissionMode);
|
||||
args.Add(PermissionModeResolver.Resolve(config.Model, config.PermissionMode));
|
||||
|
||||
if (config.Model is not null)
|
||||
{
|
||||
|
||||
@@ -127,6 +127,7 @@ public sealed class ClaudeProcess : IClaudeProcess
|
||||
ResultSubtype = streamResult.ResultSubtype,
|
||||
TerminalReason = streamResult.TerminalReason,
|
||||
Errors = streamResult.Errors,
|
||||
PermissionDenials = streamResult.PermissionDenials,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -150,6 +151,7 @@ public sealed class ClaudeProcess : IClaudeProcess
|
||||
ResultSubtype = streamResult.ResultSubtype,
|
||||
TerminalReason = streamResult.TerminalReason,
|
||||
Errors = streamResult.Errors,
|
||||
PermissionDenials = streamResult.PermissionDenials,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,7 +47,7 @@ public static class EffectiveRunConfigResolver
|
||||
maxTurns, maxTurnsSource, requestedMaxTurns, maxTurns < requestedMaxTurns,
|
||||
preset.Effort,
|
||||
agentPath, agentPathSource,
|
||||
global.DefaultPermissionMode,
|
||||
PermissionModeResolver.Resolve(model, global.DefaultPermissionMode),
|
||||
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;
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,7 @@ public sealed record RunResult
|
||||
public string? ResultSubtype { get; init; }
|
||||
public string? TerminalReason { get; init; }
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ public sealed class StreamResult
|
||||
public string? ResultSubtype { get; set; }
|
||||
public string? TerminalReason { get; set; }
|
||||
public IReadOnlyList<string> Errors { get; set; } = Array.Empty<string>();
|
||||
public IReadOnlyList<string> PermissionDenials { get; set; } = Array.Empty<string>();
|
||||
}
|
||||
|
||||
public sealed class StreamAnalyzer
|
||||
@@ -31,6 +32,7 @@ public sealed class StreamAnalyzer
|
||||
private string? _resultSubtype;
|
||||
private string? _terminalReason;
|
||||
private readonly List<string> _errors = new();
|
||||
private readonly List<string> _permissionDenials = new();
|
||||
private const string BlockedPrefix = "CLAUDEDO_BLOCKED:";
|
||||
|
||||
public void ProcessLine(string ndjsonLine)
|
||||
@@ -65,6 +67,17 @@ public sealed class StreamAnalyzer
|
||||
&& !string.IsNullOrEmpty(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.
|
||||
if (root.TryGetProperty("usage", out var resultUsage))
|
||||
{
|
||||
@@ -107,6 +120,7 @@ public sealed class StreamAnalyzer
|
||||
ResultSubtype = _resultSubtype,
|
||||
TerminalReason = _terminalReason,
|
||||
Errors = _errors,
|
||||
PermissionDenials = _permissionDenials,
|
||||
};
|
||||
|
||||
private string? FallbackResult()
|
||||
|
||||
@@ -457,9 +457,10 @@ public sealed class TaskRunner
|
||||
|
||||
private async Task HandleSuccess(TaskEntity task, ListEntity list, string slot, WorktreeContext? wtCtx, RunResult result, CancellationToken ct)
|
||||
{
|
||||
var committed = false;
|
||||
if (wtCtx is not null)
|
||||
{
|
||||
var committed = await _wtManager.CommitIfChangedAsync(wtCtx, task, list, ct);
|
||||
committed = await _wtManager.CommitIfChangedAsync(wtCtx, task, list, ct);
|
||||
if (committed)
|
||||
{
|
||||
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
|
||||
// is never left as 'running' because of a cancel that arrived
|
||||
// after the Claude run already succeeded.
|
||||
|
||||
@@ -170,6 +170,29 @@ public sealed class ClaudeArgsBuilderTests
|
||||
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]
|
||||
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);
|
||||
}
|
||||
|
||||
[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]
|
||||
public void Duplicate_Marker_In_Assistant_And_Result_Is_Collected_Once()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user