feat(worker): add post-merge verification gate for list merges

Per-list optional VerifyCommand (list_config.verify_command) runs via VerifyCommandRunner in the list's working dir right after a successful merge/continue-merge, before the task is allowed to reach Done. A non-zero exit or timeout leaves the merge in place but keeps the task out of Done and reports StatusVerifyFailed with an output excerpt through MergeResultDto/review_task; no command configured behaves exactly as before. Merges against the same repo are now serialized per working dir so a running verify can't be interrupted by a second merge landing mid-build. Adds the field to the List Settings modal (en/de localized) and covers success/failure/timeout in TaskMergeServiceTests + VerifyCommandRunnerTests.
2026-08-05 11:19:22 +02:00
parent 334cf1e1d2
commit c9ba1e2645
30 changed files with 1469 additions and 126 deletions
@@ -0,0 +1,8 @@
+namespace ClaudeDo.Worker.Lifecycle;
+
+public sealed record VerifyCommandResult(int ExitCode, bool TimedOut, string Output);
+
+public interface IVerifyCommandRunner
+{
+    Task<VerifyCommandResult> RunAsync(string workingDir, string command, TimeSpan timeout, CancellationToken ct);
+}
@@ -1,3 +1,4 @@
+using System.Collections.Concurrent;
 using ClaudeDo.Data;
 using ClaudeDo.Data.Git;
 using ClaudeDo.Data.Models;
@@ -34,19 +35,35 @@ public sealed record ConflictDocumentContent(

 public sealed class TaskMergeService
 {
-    public const string StatusMerged   = "merged";
-    public const string StatusConflict = "conflict";
-    public const string StatusBlocked  = "blocked";
-    public const string StatusAborted  = "aborted";
+    public const string StatusMerged      = "merged";
+    public const string StatusConflict    = "conflict";
+    public const string StatusBlocked     = "blocked";
+    public const string StatusAborted     = "aborted";
+    public const string StatusVerifyFailed = "verify_failed";

    public const string PreviewClean       = "clean";
    public const string PreviewConflict    = "conflict";
    public const string PreviewUnavailable = "unavailable";

+    // The verify command is a trusted, list-owner-configured build/test invocation (not
+    // per-request user input), so a generous fixed timeout is enough — no need for a
+    // per-list configurable value on top of what the spec calls for.
+    private static readonly TimeSpan VerifyTimeout = TimeSpan.FromMinutes(10);
+
+    // Serializes merge (+ verify) against the same repo working dir: a verify command running
+    // in list.WorkingDir must not see a second merge land mid-build. Keyed by working dir since
+    // TaskMergeService is a process-wide singleton and merges across different lists are independent.
+    private static readonly ConcurrentDictionary<string, SemaphoreSlim> MergeGates =
+        new(StringComparer.OrdinalIgnoreCase);
+
+    private static SemaphoreSlim GetMergeGate(string workingDir) =>
+        MergeGates.GetOrAdd(workingDir, static _ => new SemaphoreSlim(1, 1));
+
    private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory;
    private readonly GitService _git;
    private readonly HubBroadcaster _broadcaster;
    private readonly ITaskStateService _state;
+    private readonly IVerifyCommandRunner _verify;
    private readonly ILogger<TaskMergeService> _logger;

    public TaskMergeService(
@@ -54,25 +71,67 @@ public sealed class TaskMergeService
        GitService git,
        HubBroadcaster broadcaster,
        ITaskStateService state,
+        IVerifyCommandRunner verify,
        ILogger<TaskMergeService> logger)
    {
        _dbFactory = dbFactory;
        _git = git;
        _broadcaster = broadcaster;
        _state = state;
+        _verify = verify;
        _logger = logger;
    }

-    private async Task<(TaskEntity Task, ListEntity List, WorktreeEntity? Worktree)> LoadMergeContextAsync(
+    private async Task<(TaskEntity Task, ListEntity List, WorktreeEntity? Worktree, string? VerifyCommand)> LoadMergeContextAsync(
        string taskId, CancellationToken ct)
    {
        using var ctx = _dbFactory.CreateDbContext();
        var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct)
            ?? throw new KeyNotFoundException($"Task '{taskId}' not found.");
-        var list = await new ListRepository(ctx).GetByIdAsync(task.ListId, ct)
+        var listRepo = new ListRepository(ctx);
+        var list = await listRepo.GetByIdAsync(task.ListId, ct)
            ?? throw new InvalidOperationException("List not found.");
        var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(taskId, ct);
-        return (task, list, wt);
+        var config = await listRepo.GetConfigAsync(task.ListId, ct);
+        return (task, list, wt, config?.VerifyCommand);
+    }
+
+    /// <summary>
+    /// Runs the list's configured verify command (if any) in <paramref name="workingDir"/> after
+    /// a successful merge. Returns null when there is nothing to gate on (identical to today's
+    /// behavior); otherwise returns the terminal <see cref="MergeResult"/> to report instead of
+    /// merged (the merge itself is left in place either way — see the design notes in Worker's
+    /// CLAUDE.md — only the Done transition is withheld).
+    /// </summary>
+    private async Task<MergeResult?> RunVerifyGateAsync(
+        string? verifyCommand, string workingDir, CancellationToken ct)
+    {
+        if (string.IsNullOrWhiteSpace(verifyCommand)) return null;
+
+        VerifyCommandResult result;
+        try
+        {
+            result = await _verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogWarning(ex, "verify command failed to start: {Command}", verifyCommand);
+            return new MergeResult(StatusVerifyFailed, Array.Empty<string>(),
+                $"verify command failed to start: {ex.Message}");
+        }
+
+        if (!result.TimedOut && result.ExitCode == 0) return null;
+
+        var reason = result.TimedOut
+            ? $"verify command timed out after {VerifyTimeout.TotalMinutes:0} min: {verifyCommand}"
+            : $"verify command failed (exit {result.ExitCode}): {verifyCommand}";
+        return new MergeResult(StatusVerifyFailed, Array.Empty<string>(), $"{reason}\n{TailOutput(result.Output)}");
+    }
+
+    private static string TailOutput(string output, int maxChars = 4000)
+    {
+        var trimmed = output.Trim();
+        return trimmed.Length <= maxChars ? trimmed : trimmed[^maxChars..];
    }

    private async Task MarkWorktreeMergedAsync(string taskId, CancellationToken ct)
@@ -101,7 +160,7 @@ public sealed class TaskMergeService
        bool leaveConflictsInTree,
        CancellationToken ct)
    {
-        var (task, list, wt) = await LoadMergeContextAsync(taskId, ct);
+        var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);

        if (task.Status == TaskStatus.Running)
            return Blocked("task is running");
@@ -111,79 +170,95 @@ public sealed class TaskMergeService
            return Blocked($"worktree state is {wt.State}");
        if (string.IsNullOrWhiteSpace(list.WorkingDir))
            return Blocked("list has no working directory");
-        if (!await _git.IsGitRepoAsync(list.WorkingDir, ct))
-            return Blocked("working directory is not a git repository");
-        if (await _git.IsMidMergeAsync(list.WorkingDir, ct))
-            return Blocked("target working directory is mid-merge");
-        if (await _git.HasChangesAsync(list.WorkingDir, includeUntracked: false, ct))
-            return Blocked("target working tree has uncommitted changes");

-        var currentBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct);
-        if (!string.Equals(currentBranch, targetBranch, StringComparison.Ordinal))
+        var gate = GetMergeGate(list.WorkingDir);
+        await gate.WaitAsync(ct);
+        try
        {
-            try { await _git.CheckoutBranchAsync(list.WorkingDir, targetBranch, ct); }
-            catch (Exception ex) { return Blocked($"failed to switch target branch: {ex.Message}"); }
-        }
+            if (!await _git.IsGitRepoAsync(list.WorkingDir, ct))
+                return Blocked("working directory is not a git repository");
+            if (await _git.IsMidMergeAsync(list.WorkingDir, ct))
+                return Blocked("target working directory is mid-merge");
+            if (await _git.HasChangesAsync(list.WorkingDir, includeUntracked: false, ct))
+                return Blocked("target working tree has uncommitted changes");

-        var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct);
-        if (exitCode != 0)
-        {
-            List<string> files;
-            try { files = await _git.ListConflictedFilesAsync(list.WorkingDir, ct); }
-            catch { files = new(); }
-
-            if (leaveConflictsInTree && files.Count > 0)
+            var currentBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct);
+            if (!string.Equals(currentBranch, targetBranch, StringComparison.Ordinal))
            {
+                try { await _git.CheckoutBranchAsync(list.WorkingDir, targetBranch, ct); }
+                catch (Exception ex) { return Blocked($"failed to switch target branch: {ex.Message}"); }
+            }
+
+            var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct);
+            if (exitCode != 0)
+            {
+                List<string> files;
+                try { files = await _git.ListConflictedFilesAsync(list.WorkingDir, ct); }
+                catch { files = new(); }
+
+                if (leaveConflictsInTree && files.Count > 0)
+                {
+                    return new MergeResult(StatusConflict, files, null);
+                }
+
+                // If abort fails the repo is left mid-merge; the caller must resolve manually.
+                // Return Blocked (not conflict) so the UI does not offer a stale conflict list.
+                try { await _git.MergeAbortAsync(list.WorkingDir, ct); }
+                catch (Exception ex)
+                {
+                    _logger.LogError(ex, "git merge --abort failed after conflict — repo is mid-merge");
+                    return Blocked($"merge conflict and abort failed: {ex.Message} — repo is mid-merge, resolve manually");
+                }
+
+                if (files.Count == 0)
+                {
+                    // Non-conflict failure (e.g. unrelated histories).
+                    return new MergeResult(StatusBlocked, Array.Empty<string>(), $"merge failed: {stderr}");
+                }
+
                return new MergeResult(StatusConflict, files, null);
            }

-            // If abort fails the repo is left mid-merge; the caller must resolve manually.
-            // Return Blocked (not conflict) so the UI does not offer a stale conflict list.
-            try { await _git.MergeAbortAsync(list.WorkingDir, ct); }
-            catch (Exception ex)
+            string? cleanupWarning = null;
+            if (removeWorktree)
            {
-                _logger.LogError(ex, "git merge --abort failed after conflict — repo is mid-merge");
-                return Blocked($"merge conflict and abort failed: {ex.Message} — repo is mid-merge, resolve manually");
-            }
-
-            if (files.Count == 0)
-            {
-                // Non-conflict failure (e.g. unrelated histories).
-                return new MergeResult(StatusBlocked, Array.Empty<string>(), $"merge failed: {stderr}");
-            }
-
-            return new MergeResult(StatusConflict, files, null);
-        }
-
-        string? cleanupWarning = null;
-        if (removeWorktree)
-        {
-            try
-            {
-                await _git.WorktreeRemoveAsync(list.WorkingDir, wt.Path, force: false, ct);
-                try { await _git.BranchDeleteAsync(list.WorkingDir, wt.BranchName, force: false, ct); }
+                try
+                {
+                    await _git.WorktreeRemoveAsync(list.WorkingDir, wt.Path, force: false, ct);
+                    try { await _git.BranchDeleteAsync(list.WorkingDir, wt.BranchName, force: false, ct); }
+                    catch (Exception ex)
+                    {
+                        _logger.LogWarning(ex, "branch delete failed for {Branch}", wt.BranchName);
+                        cleanupWarning = $"worktree removed, branch delete failed: {ex.Message}";
+                    }
+                }
                catch (Exception ex)
                {
-                    _logger.LogWarning(ex, "branch delete failed for {Branch}", wt.BranchName);
-                    cleanupWarning = $"worktree removed, branch delete failed: {ex.Message}";
+                    _logger.LogWarning(ex, "worktree remove failed for {Path}", wt.Path);
+                    cleanupWarning = $"worktree remove failed: {ex.Message}";
                }
            }
-            catch (Exception ex)
+
+            await MarkWorktreeMergedAsync(taskId, ct);
+
+            var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct);
+            if (verifyFailure is not null)
            {
-                _logger.LogWarning(ex, "worktree remove failed for {Path}", wt.Path);
-                cleanupWarning = $"worktree remove failed: {ex.Message}";
+                _logger.LogWarning("Verify command failed after merging task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
+                await _broadcaster.WorkerLog($"Verify failed for \"{task.Title}\" after merge into {targetBranch}", WorkerLogLevel.Warn, DateTime.UtcNow);
+                return verifyFailure;
            }
+
+            await ApproveIfWaitingForReviewAsync(task, ct);
+
+            _logger.LogInformation(
+                "Merged task {TaskId} branch {Branch} into {Target} (remove worktree: {Remove})",
+                taskId, wt.BranchName, targetBranch, removeWorktree);
+            await _broadcaster.WorkerLog($"Merged \"{task.Title}\" into {targetBranch}", WorkerLogLevel.Success, DateTime.UtcNow);
+
+            return new MergeResult(StatusMerged, Array.Empty<string>(), cleanupWarning);
        }
-
-        await MarkWorktreeMergedAsync(taskId, ct);
-        await ApproveIfWaitingForReviewAsync(task, ct);
-
-        _logger.LogInformation(
-            "Merged task {TaskId} branch {Branch} into {Target} (remove worktree: {Remove})",
-            taskId, wt.BranchName, targetBranch, removeWorktree);
-        await _broadcaster.WorkerLog($"Merged \"{task.Title}\" into {targetBranch}", WorkerLogLevel.Success, DateTime.UtcNow);
-
-        return new MergeResult(StatusMerged, Array.Empty<string>(), cleanupWarning);
+        finally { gate.Release(); }
    }

    public Task<MergeResult> MergeAsync(
@@ -196,53 +271,69 @@ public sealed class TaskMergeService

    public async Task<MergeResult> ContinueMergeAsync(string taskId, CancellationToken ct)
    {
-        var (task, list, wt) = await LoadMergeContextAsync(taskId, ct);
+        var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);

        if (wt is null) return Blocked("task has no worktree");
        if (wt.State != WorktreeState.Active) return Blocked($"worktree state is {wt.State}");
        if (string.IsNullOrWhiteSpace(list.WorkingDir)) return Blocked("list has no working directory");
-        if (!await _git.IsMidMergeAsync(list.WorkingDir, ct))
-            return Blocked("repo is not mid-merge");

-        // Validate BEFORE staging: `git add` marks a conflicted path resolved regardless of
-        // its content, so an unresolved file with markers still in it would otherwise get
-        // staged (and committed) as-is. Check text content for markers first; binary files
-        // can't carry markers, so they're left to the post-stage index check below.
-        var unresolved = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
-        var stillConflicted = new List<string>();
-        foreach (var path in unresolved)
+        var gate = GetMergeGate(list.WorkingDir);
+        await gate.WaitAsync(ct);
+        try
        {
-            var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
-            string text;
-            try { text = await File.ReadAllTextAsync(full, ct); }
-            catch { continue; }
+            if (!await _git.IsMidMergeAsync(list.WorkingDir, ct))
+                return Blocked("repo is not mid-merge");

-            if (!LooksBinary(text) && ConflictMarkerParser.HasConflicts(text))
-                stillConflicted.Add(path);
+            // Validate BEFORE staging: `git add` marks a conflicted path resolved regardless of
+            // its content, so an unresolved file with markers still in it would otherwise get
+            // staged (and committed) as-is. Check text content for markers first; binary files
+            // can't carry markers, so they're left to the post-stage index check below.
+            var unresolved = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
+            var stillConflicted = new List<string>();
+            foreach (var path in unresolved)
+            {
+                var full = Path.Combine(list.WorkingDir, path.Replace('/', Path.DirectorySeparatorChar));
+                string text;
+                try { text = await File.ReadAllTextAsync(full, ct); }
+                catch { continue; }
+
+                if (!LooksBinary(text) && ConflictMarkerParser.HasConflicts(text))
+                    stillConflicted.Add(path);
+            }
+
+            if (stillConflicted.Count > 0)
+                return new MergeResult(StatusConflict, stillConflicted, "conflicts not fully resolved");
+
+            await _git.AddAllAsync(list.WorkingDir, ct);
+
+            var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
+            if (remaining.Count > 0)
+                return new MergeResult(StatusConflict, remaining, "conflicts not fully resolved");
+
+            try { await _git.CommitAsync(list.WorkingDir, $"Merge branch '{wt.BranchName}'", ct); }
+            catch (Exception ex) { return Blocked($"commit failed: {ex.Message}"); }
+
+            await MarkWorktreeMergedAsync(taskId, ct);
+
+            var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct);
+            if (verifyFailure is not null)
+            {
+                _logger.LogWarning("Verify command failed after continuing merge of task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
+                await _broadcaster.WorkerLog($"Verify failed for \"{task.Title}\" after merge", WorkerLogLevel.Warn, DateTime.UtcNow);
+                return verifyFailure;
+            }
+
+            await ApproveIfWaitingForReviewAsync(task, ct);
+            _logger.LogInformation("Continued merge of task {TaskId} branch {Branch}", taskId, wt.BranchName);
+
+            return new MergeResult(StatusMerged, Array.Empty<string>(), null);
        }
-
-        if (stillConflicted.Count > 0)
-            return new MergeResult(StatusConflict, stillConflicted, "conflicts not fully resolved");
-
-        await _git.AddAllAsync(list.WorkingDir, ct);
-
-        var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct);
-        if (remaining.Count > 0)
-            return new MergeResult(StatusConflict, remaining, "conflicts not fully resolved");
-
-        try { await _git.CommitAsync(list.WorkingDir, $"Merge branch '{wt.BranchName}'", ct); }
-        catch (Exception ex) { return Blocked($"commit failed: {ex.Message}"); }
-
-        await MarkWorktreeMergedAsync(taskId, ct);
-        await ApproveIfWaitingForReviewAsync(task, ct);
-        _logger.LogInformation("Continued merge of task {TaskId} branch {Branch}", taskId, wt.BranchName);
-
-        return new MergeResult(StatusMerged, Array.Empty<string>(), null);
+        finally { gate.Release(); }
    }

    public async Task<MergeResult> AbortMergeAsync(string taskId, CancellationToken ct)
    {
-        var (_, list, wt) = await LoadMergeContextAsync(taskId, ct);
+        var (_, list, wt, _) = await LoadMergeContextAsync(taskId, ct);

        if (wt is null) return Blocked("task has no worktree");
        if (wt.State != WorktreeState.Active) return Blocked($"worktree state is {wt.State}");
@@ -263,7 +354,7 @@ public sealed class TaskMergeService
    /// </summary>
    public async Task<ConflictDocuments> GetConflictDocumentsAsync(string taskId, CancellationToken ct)
    {
-        var (_, list, _) = await LoadMergeContextAsync(taskId, ct);
+        var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct);
        if (string.IsNullOrWhiteSpace(list.WorkingDir))
            throw new InvalidOperationException("list has no working directory");

@@ -298,7 +389,7 @@ public sealed class TaskMergeService

    public async Task WriteResolutionAsync(string taskId, string path, string content, CancellationToken ct)
    {
-        var (_, list, _) = await LoadMergeContextAsync(taskId, ct);
+        var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct);
        if (string.IsNullOrWhiteSpace(list.WorkingDir))
            throw new InvalidOperationException("list has no working directory");

@@ -309,7 +400,7 @@ public sealed class TaskMergeService

    public async Task<MergeTargets> GetTargetsAsync(string taskId, CancellationToken ct)
    {
-        var (_, list, _) = await LoadMergeContextAsync(taskId, ct);
+        var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct);

        if (string.IsNullOrWhiteSpace(list.WorkingDir))
            return new MergeTargets("", Array.Empty<string>());
@@ -321,7 +412,7 @@ public sealed class TaskMergeService

    public async Task<MergePreviewResult> PreviewAsync(string taskId, string targetBranch, CancellationToken ct)
    {
-        var (_, list, wt) = await LoadMergeContextAsync(taskId, ct);
+        var (_, list, wt, _) = await LoadMergeContextAsync(taskId, ct);

        if (wt is null || wt.State != WorktreeState.Active)
            return new MergePreviewResult(PreviewUnavailable, Array.Empty<string>(), 0);
@@ -348,7 +439,7 @@ public sealed class TaskMergeService
    public async Task<MergeResult> ApproveAndMergeAsync(
        string taskId, string targetBranch, bool leaveConflictsInTree, CancellationToken ct)
    {
-        var (task, list, wt) = await LoadMergeContextAsync(taskId, ct);
+        var (task, list, wt, _) = await LoadMergeContextAsync(taskId, ct);

        if (task.Status != TaskStatus.WaitingForReview)
            return Blocked("task is not waiting for review");
@@ -0,0 +1,75 @@
+using System.Diagnostics;
+using System.Text;
+
+namespace ClaudeDo.Worker.Lifecycle;
+
+/// <summary>
+/// Runs a list's configured post-merge verification command (e.g. a build/test invocation)
+/// via cmd.exe, mirroring GitService's ProcessStartInfo discipline (no shell string
+/// concatenation beyond the single /c argument cmd.exe itself requires to parse a command line).
+/// </summary>
+public sealed class VerifyCommandRunner : IVerifyCommandRunner
+{
+    // Safety cap so a runaway/chatty command can't exhaust memory; only the tail matters anyway.
+    private const int MaxOutputChars = 512_000;
+
+    public async Task<VerifyCommandResult> RunAsync(
+        string workingDir, string command, TimeSpan timeout, CancellationToken ct)
+    {
+        var psi = new ProcessStartInfo
+        {
+            FileName = "cmd.exe",
+            WorkingDirectory = workingDir,
+            RedirectStandardOutput = true,
+            RedirectStandardError = true,
+            UseShellExecute = false,
+            CreateNoWindow = true,
+            StandardOutputEncoding = Encoding.UTF8,
+            StandardErrorEncoding = Encoding.UTF8,
+        };
+        psi.ArgumentList.Add("/c");
+        psi.ArgumentList.Add(command);
+
+        using var process = new Process { StartInfo = psi };
+        var output = new StringBuilder();
+        var sync = new object();
+
+        void Append(string? line)
+        {
+            if (line is null) return;
+            lock (sync)
+            {
+                if (output.Length >= MaxOutputChars) return;
+                output.AppendLine(line);
+            }
+        }
+
+        process.OutputDataReceived += (_, e) => Append(e.Data);
+        process.ErrorDataReceived += (_, e) => Append(e.Data);
+
+        process.Start();
+        process.BeginOutputReadLine();
+        process.BeginErrorReadLine();
+
+        using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
+        cts.CancelAfter(timeout);
+
+        var timedOut = false;
+
+        // On cancellation (timeout or caller): kill the tree. Killing unblocks WaitForExitAsync
+        // below and lets the async output readers drain/complete naturally.
+        await using var ctr = cts.Token.Register(() =>
+        {
+            timedOut = !ct.IsCancellationRequested;
+            try { process.Kill(entireProcessTree: true); }
+            catch { /* already exited */ }
+        });
+
+        await process.WaitForExitAsync(CancellationToken.None);
+
+        string finalOutput;
+        lock (sync) finalOutput = output.ToString();
+
+        return new VerifyCommandResult(process.ExitCode, timedOut, finalOutput);
+    }
+}