fix(worker): preview_merge braucht einen runVerify-Parameter wie preview_merge_set

preview_merge baute bisher immer, sobald die Liste ein Verify-Kommando
konfiguriert hatte -- anders als das Geschwister-Tool preview_merge_set,
dessen runVerify-Parameter per Default false ist. Ein einzelnes preview_merge
im Merge-Helper-Recheck stiess dadurch einen ungewollten Scratch-Build an.

runVerify (Default false) jetzt symmetrisch bei preview_merge ergaenzt,
Description-Text und die Merge-Helper-Prompt-Doku angepasst.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Mika Kuns
2026-09-16 15:43:37 +02:00
committed by mika kuns
co-authored by Claude Sonnet 5
parent c874bf7e52
commit 7452f6cfe4
4 changed files with 59 additions and 27 deletions
+3 -3
View File
@@ -184,9 +184,9 @@ detached-HEAD scratch worktree under the OS temp dir (`GitService.WorktreeAddDet
is always removed afterward (`finally`, non-cancellable cleanup). No verify command configured, or is always removed afterward (`finally`, non-cancellable cleanup). No verify command configured, or
`runVerify=false`, reproduces the pre-existing preview exactly — no delay, no scratch worktree. `runVerify=false`, reproduces the pre-existing preview exactly — no delay, no scratch worktree.
Wired into `ExternalMcpService`: `preview_merge` (single task) always requests a verify run when the Wired into `ExternalMcpService`: both `preview_merge` (single task) and `preview_merge_set` only
list has a command configured; `preview_merge_set` only does when its `runVerify` parameter is request a verify run when their `runVerify` parameter is explicitly set (default `false`) — a
explicitly set (default `false`) — a set preview never starts N builds unasked. The Hub's preview never starts a build unasked. The Hub's
`PreviewMerge` (the UI's live mergeability indicator) always passes `runVerify: false`, since `PreviewMerge` (the UI's live mergeability indicator) always passes `runVerify: false`, since
running a build on every poll would be a bad regression; `MergePreviewDto` carries the running a build on every poll would be a bad regression; `MergePreviewDto` carries the
verify fields anyway so a future UI entry point can opt in. Result fields: `VerifyExitCode` (null = verify fields anyway so a future UI entry point can opt in. Result fields: `VerifyExitCode` (null =
+1 -1
View File
@@ -523,7 +523,7 @@ public static class PromptFiles
- Clean merge → mergeStatus "merged_verify_pending" (expected: the task deliberately stays WaitingForReview until verify_merges passes) or "merged" (list has no verify command; the task is Done). Either way, move on. - Clean merge → mergeStatus "merged_verify_pending" (expected: the task deliberately stays WaitingForReview until verify_merges passes) or "merged" (list has no verify command; the task is Done). Either way, move on.
- Conflict (markers left in the working tree, repoPath returned) → resolve it below. - Conflict (markers left in the working tree, repoPath returned) → resolve it below.
After each merge, main has moved — a clean preview_merge_set result from before this merge is now stale for the remaining tasks. If you are unsure whether an earlier overlap warning still matters, call preview_merge for the next task again before merging it. After each merge, main has moved — a clean preview_merge_set result from before this merge is now stale for the remaining tasks. If you are unsure whether an earlier overlap warning still matters, call preview_merge for the next task again before merging it (default runVerify=false — this is just a conflict recheck, not a build; the verify command still runs once via verify_merges after the last merge).
Every branch in this run forked from the same base, so conflicts between them are the NORMAL case, not a failure. Resolve them and keep going; do not abandon the run because a merge conflicted. Every branch in this run forked from the same base, so conflicts between them are the NORMAL case, not a failure. Resolve them and keep going; do not abandon the run because a merge conflicted.
+16 -12
View File
@@ -446,26 +446,30 @@ public sealed class MergeMcpTools
"untracked, in the target working directory — merge-tree can't see the working tree at all, so this is the " + "untracked, in the target working directory — merge-tree can't see the working tree at all, so this is the " +
"only way to catch it before a real merge either refuses or, if that path became tracked in the meantime, " + "only way to catch it before a real merge either refuses or, if that path became tracked in the meantime, " +
"silently overwrites it); behind counts commits on targetBranch not yet on this branch, which flags a stale " + "silently overwrites it); behind counts commits on targetBranch not yet on this branch, which flags a stale " +
"branch even when the preview is clean. If the list has a verify command configured, a clean preview is " + "branch even when the preview is clean. runVerify=false (default) never builds. runVerify=true additionally " +
"additionally built/tested in a scratch worktree (still without touching the real working tree) — " + "builds/tests a clean preview in a scratch worktree (still without touching the real working tree), if the " +
"verifyExitCode 0 means it built clean, non-zero or -1 (timeout/failed to start) means it doesn't, with the " + "list has a verify command configured — verifyExitCode 0 means it built clean, non-zero or -1 " +
"tail of its output in verifyOutputTail; verifyExitCode stays null when no verify command is configured. " + "(timeout/failed to start) means it doesn't, with the tail of its output in verifyOutputTail; " +
"isEmpty=true means the task's review range contributed nothing; check that flag rather than reading a " + "verifyExitCode/verifyDurationMs/verifyOutputTail stay null without runVerify=true or without a configured " +
"small changedFileCount as empty. staleFiles lists files this branch touches that the target branch ALSO " + "verify command. isEmpty=true means the task's review range contributed nothing; check that flag rather " +
"changed since this branch's fork point — a more honest staleness signal than `behind` alone, since a " + "than reading a small changedFileCount as empty. staleFiles lists files this branch touches that the " +
"branch can be far behind yet touch nothing the target changed, or barely behind yet collide on the one " + "target branch ALSO changed since this branch's fork point — a more honest staleness signal than `behind` " +
"file that matters (always empty for a worktree-less handler task, which has no fork point). Throws if the " + "alone, since a branch can be far behind yet touch nothing the target changed, or barely behind yet " +
"task has neither an active worktree nor a handler commit range, or the list's working directory is missing " + "collide on the one file that matters (always empty for a worktree-less handler task, which has no fork " +
"from disk." + McpToolDocs.ProgressHint)] "point). Throws if the task has neither an active worktree nor a handler commit range, or the list's " +
"working directory is missing 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,
[Description("true: also run the verify command (if configured), building in a scratch worktree. " +
"false (default): no build.")]
bool runVerify = false,
CancellationToken cancellationToken = default, CancellationToken cancellationToken = default,
IProgress<ProgressNotificationValue>? progress = null) 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, progress); var (preview, behind, _, isEmpty, staleFiles, _, _) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, 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);
} }
+39 -11
View File
@@ -793,7 +793,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var headBefore = await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None); var headBefore = await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None);
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, target, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status); Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Empty(result.ConflictFiles); Assert.Empty(result.ConflictFiles);
@@ -815,7 +815,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!, CancellationToken.None); var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!, CancellationToken.None);
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, target, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status); Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Equal(0, result.ChangedFileCount); Assert.Equal(0, result.ChangedFileCount);
@@ -843,7 +843,7 @@ public sealed class MergeMcpToolsTests : IDisposable
} }
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, null, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status); Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Equal(0, result.ChangedFileCount); Assert.Equal(0, result.ChangedFileCount);
@@ -875,7 +875,7 @@ public sealed class MergeMcpToolsTests : IDisposable
} }
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, null, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status); Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Equal(1, result.ChangedFileCount); Assert.Equal(1, result.ChangedFileCount);
@@ -893,7 +893,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var headBefore = await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None); var headBefore = await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None);
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, target, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewConflict, result.Status); Assert.Equal(TaskMergeService.PreviewConflict, result.Status);
Assert.Contains("README.md", result.ConflictFiles); Assert.Contains("README.md", result.ConflictFiles);
@@ -910,7 +910,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
await Assert.ThrowsAsync<InvalidOperationException>( await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.PreviewMerge(task.Id, null, CancellationToken.None)); () => sut.PreviewMerge(task.Id, null, cancellationToken: CancellationToken.None));
} }
[Fact] [Fact]
@@ -928,7 +928,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var ex = await Assert.ThrowsAsync<InvalidOperationException>( var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => sut.PreviewMerge(task.Id, null, CancellationToken.None)); () => sut.PreviewMerge(task.Id, null, cancellationToken: CancellationToken.None));
Assert.Contains("working directory", ex.Message, StringComparison.OrdinalIgnoreCase); Assert.Contains("working directory", ex.Message, StringComparison.OrdinalIgnoreCase);
} }
@@ -952,7 +952,7 @@ public sealed class MergeMcpToolsTests : IDisposable
var fakeVerify = new FakeVerifyCommandRunner { Result = new VerifyCommandResult(0, false, "ok") }; var fakeVerify = new FakeVerifyCommandRunner { Result = new VerifyCommandResult(0, false, "ok") };
var sut = BuildSut(CreateQueue(), fakeVerify); var sut = BuildSut(CreateQueue(), fakeVerify);
var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, target, runVerify: true, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status); Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Equal(0, result.VerifyExitCode); Assert.Equal(0, result.VerifyExitCode);
@@ -962,6 +962,34 @@ public sealed class MergeMcpToolsTests : IDisposable
Assert.False(await git.HasChangesAsync(list.WorkingDir!, CancellationToken.None)); Assert.False(await git.HasChangesAsync(list.WorkingDir!, CancellationToken.None));
} }
[Fact]
public async Task PreviewMerge_DefaultRunVerifyFalse_NeverInvokesRunnerEvenWithCommandConfigured()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var (task, list, wt) = await SeedWorktreeAsync(TaskStatus.WaitingForReview);
File.WriteAllText(Path.Combine(wt.WorktreePath, "added.txt"), "content");
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 git = new GitService();
var target = await git.GetCurrentBranchAsync(list.WorkingDir!, CancellationToken.None);
var fakeVerify = new FakeVerifyCommandRunner { Result = new VerifyCommandResult(0, false, "ok") };
var sut = BuildSut(CreateQueue(), fakeVerify);
var result = await sut.PreviewMerge(task.Id, target, cancellationToken: CancellationToken.None);
Assert.Equal(TaskMergeService.PreviewClean, result.Status);
Assert.Null(result.VerifyExitCode);
Assert.Null(result.VerifyDurationMs);
Assert.Null(result.VerifyOutputTail);
Assert.Null(fakeVerify.CapturedCommand);
}
[Fact] [Fact]
public async Task PreviewMerge_TargetChangedSameFileSinceFork_ReportsStaleFiles() public async Task PreviewMerge_TargetChangedSameFileSinceFork_ReportsStaleFiles()
{ {
@@ -982,7 +1010,7 @@ public sealed class MergeMcpToolsTests : IDisposable
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main also touched shared.txt"); GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main also touched shared.txt");
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, "main", CancellationToken.None); var result = await sut.PreviewMerge(task.Id, "main", cancellationToken: CancellationToken.None);
Assert.Equal(1, result.Behind); Assert.Equal(1, result.Behind);
Assert.NotNull(result.StaleFiles); Assert.NotNull(result.StaleFiles);
@@ -1010,7 +1038,7 @@ public sealed class MergeMcpToolsTests : IDisposable
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main touched something else"); GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main touched something else");
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, "main", CancellationToken.None); var result = await sut.PreviewMerge(task.Id, "main", cancellationToken: CancellationToken.None);
Assert.Equal(1, result.Behind); Assert.Equal(1, result.Behind);
Assert.NotNull(result.StaleFiles); Assert.NotNull(result.StaleFiles);
@@ -1042,7 +1070,7 @@ public sealed class MergeMcpToolsTests : IDisposable
} }
var sut = BuildSut(CreateQueue()); var sut = BuildSut(CreateQueue());
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None); var result = await sut.PreviewMerge(task.Id, null, cancellationToken: CancellationToken.None);
Assert.NotNull(result.StaleFiles); Assert.NotNull(result.StaleFiles);
Assert.Empty(result.StaleFiles!); Assert.Empty(result.StaleFiles!);