Merge claudedo/d8199f1f3df3447f8de2e7aca9ec5064
This commit is contained in:
+28
-14
@@ -15,6 +15,7 @@ using ClaudeDo.Worker.Queue;
|
|||||||
using ClaudeDo.Worker.State;
|
using ClaudeDo.Worker.State;
|
||||||
using ClaudeDo.Worker.Worktrees;
|
using ClaudeDo.Worker.Worktrees;
|
||||||
using Microsoft.EntityFrameworkCore;
|
using Microsoft.EntityFrameworkCore;
|
||||||
|
using ModelContextProtocol;
|
||||||
using ModelContextProtocol.Server;
|
using ModelContextProtocol.Server;
|
||||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||||
|
|
||||||
@@ -644,7 +645,7 @@ public sealed class ExternalMcpService
|
|||||||
"means the merge stopped on conflicts, with the files listed. emptyChildren (parent approve only) lists " +
|
"means the merge stopped on conflicts, with the files listed. emptyChildren (parent approve only) lists " +
|
||||||
"the Done children about to be unit-merged whose own review range contributed nothing (e.g. a child that " +
|
"the Done children about to be unit-merged whose own review range contributed nothing (e.g. a child that " +
|
||||||
"reported CLAUDEDO_BLOCKED and committed no code) — check it before trusting that every child actually " +
|
"reported CLAUDEDO_BLOCKED and committed no code) — check it before trusting that every child actually " +
|
||||||
"delivered something." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint)]
|
"delivered something." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint + McpToolDocs.ProgressHint)]
|
||||||
public async Task<ReviewTaskResult> ReviewTask(
|
public async Task<ReviewTaskResult> ReviewTask(
|
||||||
string taskId,
|
string taskId,
|
||||||
[Description("'approve', 'reject_rerun', 'reject_park' or 'cancel'.")]
|
[Description("'approve', 'reject_rerun', 'reject_park' or 'cancel'.")]
|
||||||
@@ -660,8 +661,15 @@ public sealed class ExternalMcpService
|
|||||||
"in the working tree (repoPath in the result) so you can resolve them and call continue_merge, " +
|
"in the working tree (repoPath in the result) so you can resolve them and call continue_merge, " +
|
||||||
"or abort_merge to cancel.")]
|
"or abort_merge to cancel.")]
|
||||||
bool leaveConflictsInTree = false,
|
bool leaveConflictsInTree = false,
|
||||||
CancellationToken cancellationToken = default)
|
CancellationToken cancellationToken = default,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
|
// First report fires before any git/verify work starts -- an approve that has to wait on
|
||||||
|
// the per-repo merge gate (another merge/verify already running) must not sit silent long
|
||||||
|
// enough to trip Claude Code's ~300s MCP idle-silence abort before RunVerifyGateAsync's own
|
||||||
|
// periodic reports even begin.
|
||||||
|
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = "review_task started" });
|
||||||
|
|
||||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||||
@@ -702,7 +710,7 @@ public sealed class ExternalMcpService
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", leaveConflictsInTree, cancellationToken);
|
var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", leaveConflictsInTree, cancellationToken, progress);
|
||||||
if (r.Status == TaskMergeService.StatusBlocked)
|
if (r.Status == TaskMergeService.StatusBlocked)
|
||||||
throw new InvalidOperationException(r.ErrorMessage ?? "approve failed");
|
throw new InvalidOperationException(r.ErrorMessage ?? "approve failed");
|
||||||
mergeStatus = r.Status;
|
mergeStatus = r.Status;
|
||||||
@@ -947,7 +955,7 @@ public sealed class ExternalMcpService
|
|||||||
[McpServerTool, Description(
|
[McpServerTool, Description(
|
||||||
"Merge a Done task's worktree branch into targetBranch. For a task still in WaitingForReview prefer " +
|
"Merge a Done task's worktree branch into targetBranch. For a task still in WaitingForReview prefer " +
|
||||||
"review_task, which merges as part of approving. merged=true carries the new mergeCommit SHA; on conflict " +
|
"review_task, which merges as part of approving. merged=true carries the new mergeCommit SHA; on conflict " +
|
||||||
"merged=false and conflicts lists the affected files.")]
|
"merged=false and conflicts lists the affected files." + McpToolDocs.ProgressHint)]
|
||||||
public async Task<MergeTaskResultDto> MergeTask(
|
public async Task<MergeTaskResultDto> MergeTask(
|
||||||
string taskId,
|
string taskId,
|
||||||
string targetBranch = "main",
|
string targetBranch = "main",
|
||||||
@@ -961,8 +969,11 @@ public sealed class ExternalMcpService
|
|||||||
"leave the conflict markers in the working tree at repoPath (conflictsInTree=true) so you can " +
|
"leave the conflict markers in the working tree at repoPath (conflictsInTree=true) so you can " +
|
||||||
"resolve them there and call continue_merge, or abort_merge to cancel.")]
|
"resolve them there and call continue_merge, or abort_merge to cancel.")]
|
||||||
bool leaveConflictsInTree = false,
|
bool leaveConflictsInTree = false,
|
||||||
CancellationToken cancellationToken = default)
|
CancellationToken cancellationToken = default,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
|
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = "merge_task started" });
|
||||||
|
|
||||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||||
@@ -988,7 +999,7 @@ public sealed class ExternalMcpService
|
|||||||
|
|
||||||
var commitMessage = $"Merge task branch for: {task.Title}";
|
var commitMessage = $"Merge task branch for: {task.Title}";
|
||||||
var result = await _merge.MergeAsync(
|
var result = await _merge.MergeAsync(
|
||||||
taskId, targetBranch, removeWorktree: false, commitMessage, leaveConflictsInTree, cancellationToken);
|
taskId, targetBranch, removeWorktree: false, commitMessage, leaveConflictsInTree, cancellationToken, progress);
|
||||||
|
|
||||||
if (result.Status == TaskMergeService.StatusMerged)
|
if (result.Status == TaskMergeService.StatusMerged)
|
||||||
{
|
{
|
||||||
@@ -1226,15 +1237,16 @@ public sealed class ExternalMcpService
|
|||||||
"branch can be far behind yet touch nothing the target changed, or barely behind yet collide on the one " +
|
"branch can be far behind yet touch nothing the target changed, or barely behind yet collide on the one " +
|
||||||
"file that matters (always empty for a worktree-less handler task, which has no fork point). Throws if the " +
|
"file that matters (always empty for a worktree-less handler task, which has no fork point). Throws if the " +
|
||||||
"task has neither an active worktree nor a handler commit range, or the list's working directory is missing " +
|
"task has neither an active worktree nor a handler commit range, or the list's working directory is missing " +
|
||||||
"from disk.")]
|
"from disk." + McpToolDocs.ProgressHint)]
|
||||||
public async Task<MergePreviewToolDto> PreviewMerge(
|
public async Task<MergePreviewToolDto> PreviewMerge(
|
||||||
string taskId,
|
string taskId,
|
||||||
[Description("Branch to preview against; defaults to the repo's current branch.")]
|
[Description("Branch to preview against; defaults to the repo's current branch.")]
|
||||||
string? targetBranch = null,
|
string? targetBranch = null,
|
||||||
CancellationToken cancellationToken = default)
|
CancellationToken cancellationToken = default,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||||
var (preview, behind, _, isEmpty, staleFiles, _) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify: true, cancellationToken);
|
var (preview, behind, _, isEmpty, staleFiles, _) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify: true, cancellationToken, progress);
|
||||||
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, isEmpty,
|
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, isEmpty,
|
||||||
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles);
|
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles);
|
||||||
}
|
}
|
||||||
@@ -1251,7 +1263,7 @@ public sealed class ExternalMcpService
|
|||||||
"collide unflagged, and as with preview_merge a clean result does not mean the merge builds. " +
|
"collide unflagged, and as with preview_merge a clean result does not mean the merge builds. " +
|
||||||
"runVerify=false (default) never builds — set it true to also run each task's list's verify command in a " +
|
"runVerify=false (default) never builds — set it true to also run each task's list's verify command in a " +
|
||||||
"scratch worktree per entry (same fields as preview_merge); this can take a long time across many tasks, " +
|
"scratch worktree per entry (same fields as preview_merge); this can take a long time across many tasks, " +
|
||||||
"since builds run one at a time.")]
|
"since builds run one at a time." + McpToolDocs.ProgressHint)]
|
||||||
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
||||||
IReadOnlyList<string> taskIds,
|
IReadOnlyList<string> taskIds,
|
||||||
[Description("Branch to preview every task against; defaults to the repo's current branch.")]
|
[Description("Branch to preview every task against; defaults to the repo's current branch.")]
|
||||||
@@ -1259,7 +1271,8 @@ public sealed class ExternalMcpService
|
|||||||
[Description("true: also run the verify command (if configured) for each task, one build at a time. " +
|
[Description("true: also run the verify command (if configured) for each task, one build at a time. " +
|
||||||
"false (default): no builds, however many tasks are given.")]
|
"false (default): no builds, however many tasks are given.")]
|
||||||
bool runVerify = false,
|
bool runVerify = false,
|
||||||
CancellationToken cancellationToken = default)
|
CancellationToken cancellationToken = default,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
if (taskIds is null || taskIds.Count == 0)
|
if (taskIds is null || taskIds.Count == 0)
|
||||||
throw new InvalidOperationException("taskIds must contain at least one task id.");
|
throw new InvalidOperationException("taskIds must contain at least one task id.");
|
||||||
@@ -1274,7 +1287,7 @@ public sealed class ExternalMcpService
|
|||||||
{
|
{
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var (preview, behind, changedFiles, isEmpty, staleFiles, number) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, cancellationToken);
|
var (preview, behind, changedFiles, isEmpty, staleFiles, number) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, cancellationToken, progress);
|
||||||
entries.Add(new MergePreviewSetEntryDto(
|
entries.Add(new MergePreviewSetEntryDto(
|
||||||
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null, isEmpty,
|
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null, isEmpty,
|
||||||
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles, number));
|
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles, number));
|
||||||
@@ -1343,7 +1356,8 @@ public sealed class ExternalMcpService
|
|||||||
// HandlerBaseCommit..HandlerHeadCommit range, reporting a synthetic "clean" preview of that
|
// HandlerBaseCommit..HandlerHeadCommit range, reporting a synthetic "clean" preview of that
|
||||||
// range's own diff-stat instead of throwing "has no worktree".
|
// range's own diff-stat instead of throwing "has no worktree".
|
||||||
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles, bool IsEmpty, IReadOnlyList<string> StaleFiles, int Number)> PreviewMergeCoreAsync(
|
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles, bool IsEmpty, IReadOnlyList<string> StaleFiles, int Number)> PreviewMergeCoreAsync(
|
||||||
string taskId, string? targetBranch, bool runVerify, CancellationToken ct)
|
string taskId, string? targetBranch, bool runVerify, CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
using var ctx = _dbFactory.CreateDbContext();
|
using var ctx = _dbFactory.CreateDbContext();
|
||||||
var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct)
|
var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct)
|
||||||
@@ -1360,7 +1374,7 @@ public sealed class ExternalMcpService
|
|||||||
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
||||||
throw new InvalidOperationException("The list's working directory no longer exists.");
|
throw new InvalidOperationException("The list's working directory no longer exists.");
|
||||||
|
|
||||||
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", runVerify, ct);
|
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", runVerify, ct, progress);
|
||||||
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
||||||
throw new InvalidOperationException(
|
throw new InvalidOperationException(
|
||||||
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
||||||
|
|||||||
+11
@@ -45,4 +45,15 @@ internal static class McpToolDocs
|
|||||||
"'theirs' side (the branch being merged in), plus an optional 'base' — the common-ancestor text for that " +
|
"'theirs' side (the branch being merged in), plus an optional 'base' — the common-ancestor text for that " +
|
||||||
"hunk, shown between a third '|||||||' marker and the '=======' separator. base is null when git recorded " +
|
"hunk, shown between a third '|||||||' marker and the '=======' separator. base is null when git recorded " +
|
||||||
"none for that hunk.";
|
"none for that hunk.";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A merge/verify gate here can run for minutes -- warns the caller that MCP progress pings
|
||||||
|
/// keep the call alive across the calling client's own idle-silence abort. Same rationale as
|
||||||
|
/// WaitForTaskChange's own clause (External/TaskWaitMcpTools.cs), worded for a merge instead
|
||||||
|
/// of a wait.
|
||||||
|
/// </summary>
|
||||||
|
public const string ProgressHint =
|
||||||
|
" Sends MCP progress pings periodically while a merge or verify gate runs, so a long call survives the " +
|
||||||
|
"calling client's own idle-silence abort (Claude Code defaults to killing an MCP call after ~300s of " +
|
||||||
|
"silence) -- this is not guaranteed by every possible MCP client.";
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ using ClaudeDo.Data.Repositories;
|
|||||||
using ClaudeDo.Worker.Hub;
|
using ClaudeDo.Worker.Hub;
|
||||||
using ClaudeDo.Worker.State;
|
using ClaudeDo.Worker.State;
|
||||||
using Microsoft.EntityFrameworkCore;
|
using Microsoft.EntityFrameworkCore;
|
||||||
|
using ModelContextProtocol;
|
||||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||||
|
|
||||||
namespace ClaudeDo.Worker.Lifecycle;
|
namespace ClaudeDo.Worker.Lifecycle;
|
||||||
@@ -68,6 +69,14 @@ public sealed class TaskMergeService
|
|||||||
// per-list configurable value on top of what the spec calls for.
|
// per-list configurable value on top of what the spec calls for.
|
||||||
private static readonly TimeSpan VerifyTimeout = TimeSpan.FromMinutes(10);
|
private static readonly TimeSpan VerifyTimeout = TimeSpan.FromMinutes(10);
|
||||||
|
|
||||||
|
// Mirrors TaskWaitMcpTools.ProgressReportInterval (External/TaskWaitMcpTools.cs): a verify
|
||||||
|
// run can take up to VerifyTimeout, well past Claude Code's ~300s MCP idle-silence abort, so
|
||||||
|
// RunReportingProgressAsync below reports on this cadence to keep the calling review_task/
|
||||||
|
// merge_task/preview_merge* call alive. Not readonly -- tests shrink it to observe a report
|
||||||
|
// without waiting 30s. A separate field from TaskWaitMcpTools' own (rather than sharing it)
|
||||||
|
// so shrinking one for a test can't race the other's tests.
|
||||||
|
internal static TimeSpan ProgressReportInterval = TimeSpan.FromSeconds(30);
|
||||||
|
|
||||||
// Serializes merge (+ verify) against the same repo working dir: a verify command running
|
// 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
|
// 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.
|
// TaskMergeService is a process-wide singleton and merges across different lists are independent.
|
||||||
@@ -122,14 +131,16 @@ public sealed class TaskMergeService
|
|||||||
/// CLAUDE.md — only the Done transition is withheld).
|
/// CLAUDE.md — only the Done transition is withheld).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private async Task<MergeResult?> RunVerifyGateAsync(
|
private async Task<MergeResult?> RunVerifyGateAsync(
|
||||||
string? verifyCommand, string workingDir, CancellationToken ct)
|
string? verifyCommand, string workingDir, CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(verifyCommand)) return null;
|
if (string.IsNullOrWhiteSpace(verifyCommand)) return null;
|
||||||
|
|
||||||
VerifyCommandResult result;
|
VerifyCommandResult result;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
result = await _verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct);
|
result = await RunReportingProgressAsync(
|
||||||
|
_verify.RunAsync(workingDir, verifyCommand, VerifyTimeout, ct), progress, "verify gate running");
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
@@ -146,6 +157,26 @@ public sealed class TaskMergeService
|
|||||||
return new MergeResult(StatusVerifyFailed, Array.Empty<string>(), $"{reason}\n{TailOutput(result.Output)}");
|
return new MergeResult(StatusVerifyFailed, Array.Empty<string>(), $"{reason}\n{TailOutput(result.Output)}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <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).
|
||||||
|
/// </summary>
|
||||||
|
private static async Task<T> RunReportingProgressAsync<T>(
|
||||||
|
Task<T> work, IProgress<ProgressNotificationValue>? progress, string message)
|
||||||
|
{
|
||||||
|
if (progress 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})" });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static string TailOutput(string output, int maxChars = 4000)
|
private static string TailOutput(string output, int maxChars = 4000)
|
||||||
{
|
{
|
||||||
var trimmed = output.Trim();
|
var trimmed = output.Trim();
|
||||||
@@ -335,7 +366,8 @@ public sealed class TaskMergeService
|
|||||||
bool removeWorktree,
|
bool removeWorktree,
|
||||||
string commitMessage,
|
string commitMessage,
|
||||||
bool leaveConflictsInTree,
|
bool leaveConflictsInTree,
|
||||||
CancellationToken ct)
|
CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
@@ -425,7 +457,7 @@ public sealed class TaskMergeService
|
|||||||
await MarkWorktreeMergedAsync(taskId, mergeSha, ct);
|
await MarkWorktreeMergedAsync(taskId, mergeSha, ct);
|
||||||
await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct);
|
await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct);
|
||||||
|
|
||||||
var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct);
|
var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct, progress);
|
||||||
if (verifyFailure is not null)
|
if (verifyFailure is not null)
|
||||||
{
|
{
|
||||||
_logger.LogWarning("Verify command failed after merging task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
|
_logger.LogWarning("Verify command failed after merging task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
|
||||||
@@ -722,7 +754,8 @@ public sealed class TaskMergeService
|
|||||||
/// delay, no scratch worktree.
|
/// delay, no scratch worktree.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public async Task<MergePreviewResult> PreviewAsync(
|
public async Task<MergePreviewResult> PreviewAsync(
|
||||||
string taskId, string targetBranch, bool runVerify, CancellationToken ct)
|
string taskId, string targetBranch, bool runVerify, CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
var (_, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
var (_, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
@@ -753,7 +786,7 @@ public sealed class TaskMergeService
|
|||||||
return new MergePreviewResult(PreviewClean, Array.Empty<string>(), count);
|
return new MergePreviewResult(PreviewClean, Array.Empty<string>(), count);
|
||||||
|
|
||||||
var (exitCode, durationMs, outputTail) = await RunPreviewVerifyAsync(
|
var (exitCode, durationMs, outputTail) = await RunPreviewVerifyAsync(
|
||||||
list.WorkingDir, target, preview.TreeOid, verifyCommand, ct);
|
list.WorkingDir, target, preview.TreeOid, verifyCommand, ct, progress);
|
||||||
return new MergePreviewResult(PreviewClean, Array.Empty<string>(), count, exitCode, durationMs, outputTail);
|
return new MergePreviewResult(PreviewClean, Array.Empty<string>(), count, exitCode, durationMs, outputTail);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -764,7 +797,8 @@ public sealed class TaskMergeService
|
|||||||
/// cancellation.
|
/// cancellation.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private async Task<(int ExitCode, long DurationMs, string OutputTail)> RunPreviewVerifyAsync(
|
private async Task<(int ExitCode, long DurationMs, string OutputTail)> RunPreviewVerifyAsync(
|
||||||
string repoDir, string targetBranch, string treeOid, string verifyCommand, CancellationToken ct)
|
string repoDir, string targetBranch, string treeOid, string verifyCommand, CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
var sw = System.Diagnostics.Stopwatch.StartNew();
|
var sw = System.Diagnostics.Stopwatch.StartNew();
|
||||||
var scratchPath = Path.Combine(Path.GetTempPath(), "claudedo-preview-verify", Guid.NewGuid().ToString("N"));
|
var scratchPath = Path.Combine(Path.GetTempPath(), "claudedo-preview-verify", Guid.NewGuid().ToString("N"));
|
||||||
@@ -778,7 +812,8 @@ public sealed class TaskMergeService
|
|||||||
VerifyCommandResult result;
|
VerifyCommandResult result;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
result = await _verify.RunAsync(scratchPath, verifyCommand, VerifyTimeout, ct);
|
result = await RunReportingProgressAsync(
|
||||||
|
_verify.RunAsync(scratchPath, verifyCommand, VerifyTimeout, ct), progress, "merge preview verify running");
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
@@ -808,7 +843,8 @@ public sealed class TaskMergeService
|
|||||||
=> ApproveAndMergeAsync(taskId, targetBranch, leaveConflictsInTree: false, ct);
|
=> ApproveAndMergeAsync(taskId, targetBranch, leaveConflictsInTree: false, ct);
|
||||||
|
|
||||||
public async Task<MergeResult> ApproveAndMergeAsync(
|
public async Task<MergeResult> ApproveAndMergeAsync(
|
||||||
string taskId, string targetBranch, bool leaveConflictsInTree, CancellationToken ct)
|
string taskId, string targetBranch, bool leaveConflictsInTree, CancellationToken ct,
|
||||||
|
IProgress<ProgressNotificationValue>? progress = null)
|
||||||
{
|
{
|
||||||
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
@@ -828,7 +864,7 @@ public sealed class TaskMergeService
|
|||||||
await verifyGate.WaitAsync(ct);
|
await verifyGate.WaitAsync(ct);
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var failed = await RunVerifyGateAsync(verifyCommand, list.WorkingDir!, ct);
|
var failed = await RunVerifyGateAsync(verifyCommand, list.WorkingDir!, ct, progress);
|
||||||
if (failed is not null) return failed;
|
if (failed is not null) return failed;
|
||||||
}
|
}
|
||||||
finally { verifyGate.Release(); }
|
finally { verifyGate.Release(); }
|
||||||
@@ -850,7 +886,7 @@ public sealed class TaskMergeService
|
|||||||
// MergeAsync transitions the task WaitingForReview -> Done on a successful merge.
|
// MergeAsync transitions the task WaitingForReview -> Done on a successful merge.
|
||||||
// Remove the worktree on approve (matching the unit-merge path) so merged
|
// 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.
|
// 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);
|
return await MergeAsync(taskId, target, removeWorktree: true, $"Merge {wt.BranchName}", leaveConflictsInTree, ct, progress);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static MergeResult Blocked(string reason) =>
|
private static MergeResult Blocked(string reason) =>
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ using ClaudeDo.Worker.Usage;
|
|||||||
using ClaudeDo.Worker.Worktrees;
|
using ClaudeDo.Worker.Worktrees;
|
||||||
using Microsoft.AspNetCore.SignalR;
|
using Microsoft.AspNetCore.SignalR;
|
||||||
using Microsoft.Extensions.Logging.Abstractions;
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
|
using ModelContextProtocol;
|
||||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||||
|
|
||||||
namespace ClaudeDo.Worker.Tests.External;
|
namespace ClaudeDo.Worker.Tests.External;
|
||||||
@@ -566,6 +567,79 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
|||||||
Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == task.Id).State);
|
Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == task.Id).State);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Regression coverage for the incident this fixes: an approve whose post-merge verify gate
|
||||||
|
// runs long enough gets no MCP traffic at all until RunVerifyGateAsync returns, so Claude
|
||||||
|
// Code's ~300s idle-silence abort kills the call -- the merge had already landed and
|
||||||
|
// committed, but the task never reached Done and every dependsOn successor stayed blocked
|
||||||
|
// until someone noticed and force-set the status by hand. Shrinks TaskMergeService's own
|
||||||
|
// progress interval (not TaskWaitMcpTools') to observe a report without waiting 30s real time.
|
||||||
|
[Fact]
|
||||||
|
public async Task ReviewTask_ApproveWithSlowVerifyCommand_ReportsProgressWellBeforeIdleTimeout()
|
||||||
|
{
|
||||||
|
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||||
|
|
||||||
|
var original = TaskMergeService.ProgressReportInterval;
|
||||||
|
TaskMergeService.ProgressReportInterval = TimeSpan.FromMilliseconds(50);
|
||||||
|
try
|
||||||
|
{
|
||||||
|
var (task, list, wt) = await SeedWorktreeAsync(TaskStatus.WaitingForReview);
|
||||||
|
File.WriteAllText(Path.Combine(wt.WorktreePath, "feature.txt"), "x\n");
|
||||||
|
var cfg = new WorkerConfig { WorktreeRootStrategy = "sibling" };
|
||||||
|
var mgr = new WorktreeManager(new GitService(), _db.CreateFactory(), cfg, NullLogger<WorktreeManager>.Instance);
|
||||||
|
await mgr.CommitIfChangedAsync(wt, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
using (var ctx = _db.CreateContext())
|
||||||
|
await new ListRepository(ctx).SetConfigAsync(new ListConfigEntity { ListId = list.Id, VerifyCommand = "dotnet build" });
|
||||||
|
|
||||||
|
var fakeVerify = new FakeVerifyCommandRunner
|
||||||
|
{
|
||||||
|
Result = new VerifyCommandResult(0, false, "ok"),
|
||||||
|
Delay = TimeSpan.FromMilliseconds(300),
|
||||||
|
};
|
||||||
|
var sut = BuildSut(CreateQueue(), fakeVerify);
|
||||||
|
|
||||||
|
var reports = new List<ProgressNotificationValue>();
|
||||||
|
var progress = new Progress<ProgressNotificationValue>(reports.Add);
|
||||||
|
|
||||||
|
var result = await sut.ReviewTask(
|
||||||
|
task.Id, "approve", null, null, cancellationToken: CancellationToken.None, progress: progress);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus);
|
||||||
|
Assert.Equal("Done", result.Task.Status);
|
||||||
|
// Progress<T> marshals via the SynchronizationContext captured at construction; give
|
||||||
|
// any queued callbacks a beat to run before asserting on `reports`.
|
||||||
|
await Task.Delay(200);
|
||||||
|
Assert.Contains(reports, r => r.Message != null && r.Message.Contains("verify gate running"));
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
TaskMergeService.ProgressReportInterval = original;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ReviewTask_ApproveWithVerifyCommand_NoProgressToken_DoesNotThrow()
|
||||||
|
{
|
||||||
|
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||||
|
|
||||||
|
var (task, list, wt) = await SeedWorktreeAsync(TaskStatus.WaitingForReview);
|
||||||
|
File.WriteAllText(Path.Combine(wt.WorktreePath, "feature.txt"), "x\n");
|
||||||
|
var cfg = new WorkerConfig { WorktreeRootStrategy = "sibling" };
|
||||||
|
var mgr = new WorktreeManager(new GitService(), _db.CreateFactory(), cfg, NullLogger<WorktreeManager>.Instance);
|
||||||
|
await mgr.CommitIfChangedAsync(wt, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
using (var ctx = _db.CreateContext())
|
||||||
|
await new ListRepository(ctx).SetConfigAsync(new ListConfigEntity { ListId = list.Id, VerifyCommand = "dotnet build" });
|
||||||
|
|
||||||
|
var fakeVerify = new FakeVerifyCommandRunner { Result = new VerifyCommandResult(0, false, "ok") };
|
||||||
|
var sut = BuildSut(CreateQueue(), fakeVerify);
|
||||||
|
|
||||||
|
var result = await sut.ReviewTask(task.Id, "approve", null, null, cancellationToken: CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus);
|
||||||
|
Assert.Equal("Done", result.Task.Status);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task ReviewTask_Approve_ParentWithChildren_RunsUnitMerge()
|
public async Task ReviewTask_Approve_ParentWithChildren_RunsUnitMerge()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -1675,13 +1675,19 @@ internal sealed class FakeVerifyCommandRunner : IVerifyCommandRunner
|
|||||||
public string? FileToCheck { get; set; }
|
public string? FileToCheck { get; set; }
|
||||||
public bool? WorkingDirHadFile { get; private set; }
|
public bool? WorkingDirHadFile { get; private set; }
|
||||||
|
|
||||||
public Task<VerifyCommandResult> RunAsync(string workingDir, string command, TimeSpan timeout, CancellationToken ct)
|
// Simulates a slow build/test run so a progress-reporting test can observe at least one
|
||||||
|
// report without the real verify command ever taking that long.
|
||||||
|
public TimeSpan? Delay { get; set; }
|
||||||
|
|
||||||
|
public async Task<VerifyCommandResult> RunAsync(string workingDir, string command, TimeSpan timeout, CancellationToken ct)
|
||||||
{
|
{
|
||||||
CapturedWorkingDir = workingDir;
|
CapturedWorkingDir = workingDir;
|
||||||
CapturedCommand = command;
|
CapturedCommand = command;
|
||||||
if (FileToCheck is not null)
|
if (FileToCheck is not null)
|
||||||
WorkingDirHadFile = File.Exists(Path.Combine(workingDir, FileToCheck));
|
WorkingDirHadFile = File.Exists(Path.Combine(workingDir, FileToCheck));
|
||||||
return Task.FromResult(Result);
|
if (Delay is { } delay)
|
||||||
|
await Task.Delay(delay, ct);
|
||||||
|
return Result;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user