From 18569439250d13d39c36403e7ee41ff1699bbb62 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Sat, 30 May 2026 15:51:14 +0200 Subject: [PATCH] refactor: merge TaskRunner failure handlers and reuse NullIfBlank Unify the near-identical HandleFailure/MarkFailed into a single MarkFailed that always persists the failed state and never throws, and replace the inline null-if-blank checks in ListMcpTools with the existing extension. Co-Authored-By: Claude Opus 4.7 --- src/ClaudeDo.Worker/External/ListMcpTools.cs | 8 +++--- src/ClaudeDo.Worker/Runner/TaskRunner.cs | 29 +++++++------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/ClaudeDo.Worker/External/ListMcpTools.cs b/src/ClaudeDo.Worker/External/ListMcpTools.cs index 15479a8..c4e8f46 100644 --- a/src/ClaudeDo.Worker/External/ListMcpTools.cs +++ b/src/ClaudeDo.Worker/External/ListMcpTools.cs @@ -31,8 +31,8 @@ public sealed class ListMcpTools { Id = Guid.NewGuid().ToString(), Name = name, - WorkingDir = string.IsNullOrWhiteSpace(workingDir) ? null : workingDir, - DefaultCommitType = string.IsNullOrWhiteSpace(commitType) ? CommitTypeRegistry.DefaultType : commitType, + WorkingDir = workingDir.NullIfBlank(), + DefaultCommitType = commitType.NullIfBlank() ?? CommitTypeRegistry.DefaultType, CreatedAt = DateTime.UtcNow, }; await _lists.AddAsync(entity, cancellationToken); @@ -51,9 +51,9 @@ public sealed class ListMcpTools throw new InvalidOperationException("name cannot be blank."); if (name is not null) entity.Name = name; if (workingDir is not null) - entity.WorkingDir = string.IsNullOrWhiteSpace(workingDir) ? null : workingDir; + entity.WorkingDir = workingDir.NullIfBlank(); if (commitType is not null) - entity.DefaultCommitType = string.IsNullOrWhiteSpace(commitType) ? CommitTypeRegistry.DefaultType : commitType; + entity.DefaultCommitType = commitType.NullIfBlank() ?? CommitTypeRegistry.DefaultType; await _lists.UpdateAsync(entity, cancellationToken); await _broadcaster.ListUpdated(listId); diff --git a/src/ClaudeDo.Worker/Runner/TaskRunner.cs b/src/ClaudeDo.Worker/Runner/TaskRunner.cs index a40fe08..8f9867c 100644 --- a/src/ClaudeDo.Worker/Runner/TaskRunner.cs +++ b/src/ClaudeDo.Worker/Runner/TaskRunner.cs @@ -129,12 +129,12 @@ public sealed class TaskRunner } else { - await HandleFailure(task.Id, task.Title, slot, retryResult); + await MarkFailed(task.Id, task.Title, slot, retryResult.ErrorMarkdown, retryResult.TurnCount); } } else { - await HandleFailure(task.Id, task.Title, slot, result); + await MarkFailed(task.Id, task.Title, slot, result.ErrorMarkdown, result.TurnCount); } } @@ -210,7 +210,7 @@ public sealed class TaskRunner } else { - await HandleFailure(taskId, task.Title, slot, result); + await MarkFailed(taskId, task.Title, slot, result.ErrorMarkdown, result.TurnCount); } await _broadcaster.TaskUpdated(taskId); @@ -331,26 +331,17 @@ public sealed class TaskRunner task.Id, result.TurnCount, result.TokensIn, result.TokensOut); } - private async Task HandleFailure(string taskId, string taskTitle, string slot, RunResult result) - { - // Intentionally does not accept a CancellationToken: this is the - // terminal write for a failed task and must always be persisted. - var finishedAt = DateTime.UtcNow; - await _state.FailAsync(taskId, finishedAt, result.ErrorMarkdown, CancellationToken.None); - await _broadcaster.WorkerLog($"Finished \"{taskTitle}\" (failed)", WorkerLogLevel.Error, DateTime.UtcNow); - await _broadcaster.TaskFinished(slot, taskId, "failed", finishedAt); - _logger.LogWarning("Task {TaskId} failed (turns={Turns}): {Error}", taskId, result.TurnCount, result.ErrorMarkdown); - } - - private async Task MarkFailed(string taskId, string taskTitle, string slot, string error) + private async Task MarkFailed(string taskId, string taskTitle, string slot, string? error, int turnCount = 0) { + // Terminal write for a failed task: never cancel (the status must always + // be persisted) and never throw (a logging failure must not mask the error). try { - var now = DateTime.UtcNow; - // Terminal write — never cancel. - await _state.FailAsync(taskId, now, error, CancellationToken.None); + var finishedAt = DateTime.UtcNow; + await _state.FailAsync(taskId, finishedAt, error, CancellationToken.None); await _broadcaster.WorkerLog($"Finished \"{taskTitle}\" (failed)", WorkerLogLevel.Error, DateTime.UtcNow); - await _broadcaster.TaskFinished(slot, taskId, "failed", now); + await _broadcaster.TaskFinished(slot, taskId, "failed", finishedAt); + _logger.LogWarning("Task {TaskId} failed (turns={Turns}): {Error}", taskId, turnCount, error); } catch (Exception ex) {