feat(merge): verify once per batch via skipVerify + verify_merges
Per-merge verify made an N-task batch pay N x ~7 min, each run testing the same moving main. review_task/merge_task/continue_merge now take skipVerify: the merge lands but the gate AND the Done transition are deferred (status merged_verify_pending, task stays WaitingForReview). The new verify_merges tool runs the list verify command once for the explicitly listed tasks and promotes them to Done on success - explicit ids so the handler own submitted task and parked verify_failed tasks are never swept up; an Active worktree is refused per entry. PlanningMergeOrchestrator threads the flag through the unit merge (no more per-child verify) and skips FinalizeParentDoneAsync when verifies were deferred. The merge-helper Merge prompt approves with skipVerify=true and calls verify_merges once after the last merge. UI approve is unchanged; the no-Done-without-green-verify invariant stays server-enforced.
This commit is contained in:
@@ -49,6 +49,15 @@ public sealed record RevertResult(
|
||||
IReadOnlyList<string> ConflictFiles,
|
||||
string? ErrorMessage);
|
||||
|
||||
// Per-task outcome of a batch verify (RunListVerifyAsync): Promoted=true means the task reached
|
||||
// Done; otherwise Reason says why it was left alone (invalid candidate, or the verify failed).
|
||||
public sealed record ListVerifyTaskOutcome(string TaskId, int Number, bool Promoted, string? Reason);
|
||||
|
||||
public sealed record ListVerifyResult(
|
||||
string Status,
|
||||
IReadOnlyList<ListVerifyTaskOutcome> Tasks,
|
||||
string? ErrorMessage);
|
||||
|
||||
public sealed class TaskMergeService
|
||||
{
|
||||
public const string StatusMerged = "merged";
|
||||
@@ -56,6 +65,10 @@ public sealed class TaskMergeService
|
||||
public const string StatusBlocked = "blocked";
|
||||
public const string StatusAborted = "aborted";
|
||||
public const string StatusVerifyFailed = "verify_failed";
|
||||
// Batch mode (skipVerify): the merge landed but the verify gate was deliberately deferred --
|
||||
// the task stays out of Done until RunListVerifyAsync passes and promotes it.
|
||||
public const string StatusMergedVerifyPending = "merged_verify_pending";
|
||||
public const string StatusVerifyPassed = "verify_passed";
|
||||
public const string StatusUntrackedCollision = "untracked_collision";
|
||||
|
||||
public const string StatusReverted = "reverted";
|
||||
@@ -361,7 +374,8 @@ public sealed class TaskMergeService
|
||||
string commitMessage,
|
||||
bool leaveConflictsInTree,
|
||||
CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
IProgress<ProgressNotificationValue>? progress = null,
|
||||
bool skipVerify = false)
|
||||
{
|
||||
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||
|
||||
@@ -463,6 +477,16 @@ public sealed class TaskMergeService
|
||||
// silence here is what makes a working merge look like a dead button.
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand))
|
||||
{
|
||||
if (skipVerify)
|
||||
{
|
||||
// Batch mode: the caller defers the gate to one RunListVerifyAsync at the end.
|
||||
// The Done transition is withheld with it — same invariant as verify_failed.
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Merged #{task.Number} \"{task.Title}\" into {targetBranch} — verify deferred to batch",
|
||||
WorkerLogLevel.Info, DateTime.UtcNow);
|
||||
return new MergeResult(StatusMergedVerifyPending, Array.Empty<string>(), cleanupWarning);
|
||||
}
|
||||
|
||||
await _broadcaster.OperationProgress(taskId, PhaseVerifying, 0, 0);
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Verify command running after merging #{task.Number} \"{task.Title}\" into {targetBranch}",
|
||||
@@ -500,7 +524,8 @@ public sealed class TaskMergeService
|
||||
=> MergeAsync(taskId, targetBranch, removeWorktree, commitMessage, leaveConflictsInTree: false, ct);
|
||||
|
||||
public async Task<MergeResult> ContinueMergeAsync(
|
||||
string taskId, CancellationToken ct, IProgress<ProgressNotificationValue>? progress = null)
|
||||
string taskId, CancellationToken ct, IProgress<ProgressNotificationValue>? progress = null,
|
||||
bool skipVerify = false)
|
||||
{
|
||||
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||
|
||||
@@ -566,6 +591,14 @@ public sealed class TaskMergeService
|
||||
var targetBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct);
|
||||
await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct);
|
||||
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand) && skipVerify)
|
||||
{
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Merged #{task.Number} \"{task.Title}\" (conflict resolved) — verify deferred to batch",
|
||||
WorkerLogLevel.Info, DateTime.UtcNow);
|
||||
return new MergeResult(StatusMergedVerifyPending, Array.Empty<string>(), null);
|
||||
}
|
||||
|
||||
var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct, progress);
|
||||
if (verifyFailure is not null)
|
||||
{
|
||||
@@ -865,7 +898,8 @@ public sealed class TaskMergeService
|
||||
|
||||
public async Task<MergeResult> ApproveAndMergeAsync(
|
||||
string taskId, string targetBranch, bool leaveConflictsInTree, CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
IProgress<ProgressNotificationValue>? progress = null,
|
||||
bool skipVerify = false)
|
||||
{
|
||||
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||
|
||||
@@ -881,6 +915,9 @@ public sealed class TaskMergeService
|
||||
// per-repo gate as the merge path, so a concurrent merge can't land mid-verify.
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand) && !string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||
{
|
||||
if (skipVerify)
|
||||
return new MergeResult(StatusMergedVerifyPending, Array.Empty<string>(), null);
|
||||
|
||||
var verifyGate = GetMergeGate(list.WorkingDir!);
|
||||
await _broadcaster.OperationProgress(taskId, PhaseVerifying, 0, 0);
|
||||
await verifyGate.WaitAsync(ct);
|
||||
@@ -911,7 +948,102 @@ 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, commitMessage: "", leaveConflictsInTree, ct, progress);
|
||||
return await MergeAsync(taskId, target, removeWorktree: true, commitMessage: "", leaveConflictsInTree, ct, progress, skipVerify);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The batch counterpart to the per-merge verify gate: runs the list's verify command ONCE
|
||||
/// (under the same per-repo gate) and, on success, promotes every listed task to Done via
|
||||
/// ApproveReviewAsync. Tasks were merged with skipVerify=true and sit in WaitingForReview.
|
||||
/// A task is only eligible when its work verifiably landed: worktree state Merged, or no
|
||||
/// worktree at all (sandbox/list-handler run) -- an Active worktree means the branch was
|
||||
/// never merged, and promoting it would produce a Done task with an unmerged branch.
|
||||
/// No verify command configured = no gate (same rule as everywhere else): promote directly.
|
||||
/// </summary>
|
||||
public async Task<ListVerifyResult> RunListVerifyAsync(
|
||||
string listId, IReadOnlyList<string> taskIds, CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
if (taskIds.Count == 0)
|
||||
return new ListVerifyResult(StatusBlocked, Array.Empty<ListVerifyTaskOutcome>(), "no task ids given");
|
||||
|
||||
ListEntity? list;
|
||||
string? verifyCommand;
|
||||
var candidates = new List<(TaskEntity Task, string? IneligibleReason)>();
|
||||
using (var ctx = _dbFactory.CreateDbContext())
|
||||
{
|
||||
var listRepo = new ListRepository(ctx);
|
||||
list = await listRepo.GetByIdAsync(listId, ct);
|
||||
if (list is null)
|
||||
return new ListVerifyResult(StatusBlocked, Array.Empty<ListVerifyTaskOutcome>(), $"list '{listId}' not found");
|
||||
verifyCommand = (await listRepo.GetConfigAsync(listId, ct))?.VerifyCommand;
|
||||
|
||||
var taskRepo = new TaskRepository(ctx);
|
||||
var wtRepo = new WorktreeRepository(ctx);
|
||||
foreach (var id in taskIds.Distinct(StringComparer.OrdinalIgnoreCase))
|
||||
{
|
||||
var task = await taskRepo.GetByIdAsync(id, ct);
|
||||
if (task is null)
|
||||
return new ListVerifyResult(StatusBlocked, Array.Empty<ListVerifyTaskOutcome>(), $"task '{id}' not found");
|
||||
var reason = task.ListId != list.Id ? "task belongs to a different list"
|
||||
: task.Status != TaskStatus.WaitingForReview ? $"task is {task.Status}, not WaitingForReview"
|
||||
: (await wtRepo.GetByTaskIdAsync(task.Id, ct)) is { } wt && wt.State != WorktreeState.Merged
|
||||
? $"worktree state is {wt.State}, not Merged — its branch never landed"
|
||||
: null;
|
||||
candidates.Add((task, reason));
|
||||
}
|
||||
}
|
||||
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir))
|
||||
return new ListVerifyResult(StatusBlocked, Array.Empty<ListVerifyTaskOutcome>(), "list has no working directory");
|
||||
|
||||
if (!string.IsNullOrWhiteSpace(verifyCommand))
|
||||
{
|
||||
var gate = GetMergeGate(list.WorkingDir);
|
||||
await _broadcaster.OperationProgress(listId, PhaseVerifying, 0, 0);
|
||||
await gate.WaitAsync(ct);
|
||||
MergeResult? failure;
|
||||
try
|
||||
{
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Batch verify running for {candidates.Count} merged task(s) in \"{list.Name}\"",
|
||||
WorkerLogLevel.Info, DateTime.UtcNow);
|
||||
failure = await RunVerifyGateAsync(
|
||||
verifyCommand, list.WorkingDir, ct, progress,
|
||||
elapsed => _ = _broadcaster.OperationProgress(listId, PhaseVerifying, (int)elapsed.TotalSeconds, 0));
|
||||
}
|
||||
finally { gate.Release(); }
|
||||
|
||||
if (failure is not null)
|
||||
{
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Batch verify failed in \"{list.Name}\" — {candidates.Count} task(s) stay out of Done",
|
||||
WorkerLogLevel.Warn, DateTime.UtcNow);
|
||||
var failed = candidates
|
||||
.Select(c => new ListVerifyTaskOutcome(c.Task.Id, c.Task.Number, false,
|
||||
c.IneligibleReason ?? "verify failed"))
|
||||
.ToList();
|
||||
return new ListVerifyResult(StatusVerifyFailed, failed, failure.ErrorMessage);
|
||||
}
|
||||
}
|
||||
|
||||
var outcomes = new List<ListVerifyTaskOutcome>(candidates.Count);
|
||||
foreach (var (task, ineligibleReason) in candidates)
|
||||
{
|
||||
if (ineligibleReason is not null)
|
||||
{
|
||||
outcomes.Add(new ListVerifyTaskOutcome(task.Id, task.Number, false, ineligibleReason));
|
||||
continue;
|
||||
}
|
||||
var result = await _state.ApproveReviewAsync(task.Id, ct);
|
||||
outcomes.Add(new ListVerifyTaskOutcome(task.Id, task.Number, result.Ok, result.Ok ? null : result.Reason));
|
||||
}
|
||||
|
||||
var promoted = outcomes.Count(o => o.Promoted);
|
||||
await _broadcaster.WorkerLog(
|
||||
$"Batch verify passed in \"{list.Name}\" — {promoted} task(s) promoted to Done",
|
||||
WorkerLogLevel.Success, DateTime.UtcNow);
|
||||
return new ListVerifyResult(StatusVerifyPassed, outcomes, null);
|
||||
}
|
||||
|
||||
private static MergeResult Blocked(string reason) =>
|
||||
|
||||
Reference in New Issue
Block a user