fix(merge): conventional merge-commit default and live verify progress
The merge commit message was hand-rolled per caller ("Merge task: <title>",
"Merge <branch>", "Merge subtask") and ignored the task's commit type. Every
caller now passes a blank message and TaskMergeService fills in
CommitMessageBuilder.BuildMerge -> {commitType}(list-slug): merge <title> plus the
ClaudeDo-Task trailer; the merge modal prefills it from GetMergeTargets.
A merge whose list has a verify command holds the MergeTask call for minutes (5m46s
on this repo), during which the modal only disabled its button - no spinner, no
message, so a landed merge looked like a dead app. TaskMergeService now broadcasts
MergeProgress(taskId, phase, elapsedSeconds) for the merging and verifying phases
(re-reported every 30s) plus a WorkerLog line when verify starts; the modal shows a
spinner and the localized phase.
This commit is contained in:
@@ -4,6 +4,7 @@ using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Hub;
|
||||
using ClaudeDo.Worker.Runner;
|
||||
using ClaudeDo.Worker.State;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using ModelContextProtocol;
|
||||
@@ -18,7 +19,8 @@ public sealed record MergeResult(
|
||||
|
||||
public sealed record MergeTargets(
|
||||
string DefaultBranch,
|
||||
IReadOnlyList<string> LocalBranches);
|
||||
IReadOnlyList<string> LocalBranches,
|
||||
string DefaultCommitMessage);
|
||||
|
||||
// VerifyExitCode/VerifyDurationMs/VerifyOutputTail are only populated when the caller asked for
|
||||
// a verify run (runVerify=true) AND the list has a verify command configured AND the preview came
|
||||
@@ -59,6 +61,10 @@ public sealed class TaskMergeService
|
||||
public const string StatusReverted = "reverted";
|
||||
public const string StatusConflictAborted = "conflict_aborted";
|
||||
|
||||
// Phase tokens for the MergeProgress broadcast — stable identifiers, localized by the UI.
|
||||
public const string PhaseMerging = "merging";
|
||||
public const string PhaseVerifying = "verifying";
|
||||
|
||||
public const string PreviewClean = "clean";
|
||||
public const string PreviewConflict = "conflict";
|
||||
public const string PreviewUnavailable = "unavailable";
|
||||
@@ -132,7 +138,8 @@ public sealed class TaskMergeService
|
||||
/// </summary>
|
||||
private async Task<MergeResult?> RunVerifyGateAsync(
|
||||
string? verifyCommand, string workingDir, CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
IProgress<ProgressNotificationValue>? progress = null,
|
||||
Action<TimeSpan>? onTick = null)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(verifyCommand)) return null;
|
||||
|
||||
@@ -140,7 +147,7 @@ public sealed class TaskMergeService
|
||||
try
|
||||
{
|
||||
result = await RunReportingProgressAsync(
|
||||
_verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct), progress, "verify gate running");
|
||||
_verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct), progress, "verify gate running", onTick);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
@@ -160,20 +167,23 @@ public sealed class TaskMergeService
|
||||
/// <summary>
|
||||
/// Awaits <paramref name="work"/> while reporting MCP progress every
|
||||
/// <see cref="ProgressReportInterval"/> so a caller waiting on a long verify run doesn't hit
|
||||
/// the MCP client's own idle-silence abort. No-op passthrough when <paramref name="progress"/>
|
||||
/// is null (every non-MCP caller, e.g. the Hub).
|
||||
/// the MCP client's own idle-silence abort. <paramref name="onTick"/> rides the same cadence
|
||||
/// for non-MCP callers (the Hub, which turns it into a MergeProgress broadcast). No-op
|
||||
/// passthrough when both are null.
|
||||
/// </summary>
|
||||
private static async Task<T> RunReportingProgressAsync<T>(
|
||||
Task<T> work, IProgress<ProgressNotificationValue>? progress, string message)
|
||||
Task<T> work, IProgress<ProgressNotificationValue>? progress, string message,
|
||||
Action<TimeSpan>? onTick = null)
|
||||
{
|
||||
if (progress is null) return await work;
|
||||
if (progress is null && onTick is null) return await work;
|
||||
|
||||
var sw = System.Diagnostics.Stopwatch.StartNew();
|
||||
while (true)
|
||||
{
|
||||
var finished = await Task.WhenAny(work, Task.Delay(ProgressReportInterval));
|
||||
if (finished == work) return await work;
|
||||
progress.Report(new ProgressNotificationValue { Progress = 0, Message = $"{message} ({sw.Elapsed:mm\\:ss})" });
|
||||
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = $"{message} ({sw.Elapsed:mm\\:ss})" });
|
||||
onTick?.Invoke(sw.Elapsed);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -380,6 +390,10 @@ public sealed class TaskMergeService
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||
return Blocked("list has no working directory");
|
||||
|
||||
// Announced before the gate wait: another merge holding the repo is itself a reason the
|
||||
// caller sees nothing happen, and a UI waiting on this call needs a phase to show at once.
|
||||
await _broadcaster.MergeProgress(taskId, PhaseMerging, 0);
|
||||
|
||||
var gate = GetMergeGate(list.WorkingDir);
|
||||
await gate.WaitAsync(ct);
|
||||
try
|
||||
@@ -402,7 +416,10 @@ public sealed class TaskMergeService
|
||||
if (collision is not null) return collision;
|
||||
|
||||
var oldTargetTip = await _git.RevParseHeadAsync(list.WorkingDir, ct);
|
||||
var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct);
|
||||
var message = string.IsNullOrWhiteSpace(commitMessage)
|
||||
? DefaultMergeMessage(task, list)
|
||||
: commitMessage;
|
||||
var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, message, ct);
|
||||
if (exitCode != 0)
|
||||
{
|
||||
List<string> files;
|
||||
@@ -457,7 +474,20 @@ public sealed class TaskMergeService
|
||||
await MarkWorktreeMergedAsync(taskId, mergeSha, ct);
|
||||
await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct);
|
||||
|
||||
var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct, progress);
|
||||
// The merge itself is instant; the verify gate is what makes this call take minutes.
|
||||
// Tell every client (the waiting modal and the footer log strip) that it started —
|
||||
// silence here is what makes a working merge look like a dead button.
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand))
|
||||
{
|
||||
await _broadcaster.MergeProgress(taskId, PhaseVerifying, 0);
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Verify command running after merging #{task.Number} \"{task.Title}\" into {targetBranch}",
|
||||
WorkerLogLevel.Info, DateTime.UtcNow);
|
||||
}
|
||||
|
||||
var verifyFailure = await RunVerifyGateAsync(
|
||||
verifyCommand, list.WorkingDir, ct, progress,
|
||||
elapsed => _ = _broadcaster.MergeProgress(taskId, PhaseVerifying, (int)elapsed.TotalSeconds));
|
||||
if (verifyFailure is not null)
|
||||
{
|
||||
_logger.LogWarning("Verify command failed after merging task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
|
||||
@@ -732,16 +762,22 @@ public sealed class TaskMergeService
|
||||
|
||||
public async Task<MergeTargets> GetTargetsAsync(string taskId, CancellationToken ct)
|
||||
{
|
||||
var (_, list, _, _) = await LoadMergeContextAsync(taskId, ct);
|
||||
var (task, list, _, _) = await LoadMergeContextAsync(taskId, ct);
|
||||
var defaultMessage = DefaultMergeMessage(task, list);
|
||||
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||
return new MergeTargets("", Array.Empty<string>());
|
||||
return new MergeTargets("", Array.Empty<string>(), defaultMessage);
|
||||
|
||||
var current = await _git.GetCurrentBranchAsync(list.WorkingDir, ct);
|
||||
var branches = await _git.ListLocalBranchesAsync(list.WorkingDir, ct);
|
||||
return new MergeTargets(current, branches);
|
||||
return new MergeTargets(current, branches, defaultMessage);
|
||||
}
|
||||
|
||||
/// The commit message a merge uses when the caller passes none. Built here rather than in the
|
||||
/// UI/MCP callers because only this layer knows the task's commit type and its list's name.
|
||||
private static string DefaultMergeMessage(TaskEntity task, ListEntity list) =>
|
||||
CommitMessageBuilder.BuildMerge(task.CommitType, list.Name, task.Title, task.Id);
|
||||
|
||||
public Task<MergePreviewResult> PreviewAsync(string taskId, string targetBranch, CancellationToken ct)
|
||||
=> PreviewAsync(taskId, targetBranch, runVerify: false, ct);
|
||||
|
||||
@@ -861,10 +897,14 @@ public sealed class TaskMergeService
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand) && !string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||
{
|
||||
var verifyGate = GetMergeGate(list.WorkingDir!);
|
||||
await _broadcaster.MergeProgress(taskId, PhaseVerifying, 0);
|
||||
await verifyGate.WaitAsync(ct);
|
||||
try
|
||||
{
|
||||
var failed = await RunVerifyGateAsync(verifyCommand, list.WorkingDir!, ct, progress);
|
||||
// Same reason as the post-merge gate: this holds the approve call for minutes.
|
||||
var failed = await RunVerifyGateAsync(
|
||||
verifyCommand, list.WorkingDir!, ct, progress,
|
||||
elapsed => _ = _broadcaster.MergeProgress(taskId, PhaseVerifying, (int)elapsed.TotalSeconds));
|
||||
if (failed is not null) return failed;
|
||||
}
|
||||
finally { verifyGate.Release(); }
|
||||
@@ -886,7 +926,7 @@ public sealed class TaskMergeService
|
||||
// MergeAsync transitions the task WaitingForReview -> Done on a successful merge.
|
||||
// Remove the worktree on approve (matching the unit-merge path) so merged
|
||||
// worktrees don't pile up; the merge commit on the target branch is the record.
|
||||
return await MergeAsync(taskId, target, removeWorktree: true, $"Merge {wt.BranchName}", leaveConflictsInTree, ct, progress);
|
||||
return await MergeAsync(taskId, target, removeWorktree: true, commitMessage: "", leaveConflictsInTree, ct, progress);
|
||||
}
|
||||
|
||||
private static MergeResult Blocked(string reason) =>
|
||||
|
||||
Reference in New Issue
Block a user