feat(mcp): paths filter for get_task_diff, subset hint for preview_merge_set
get_task_diff gained an optional paths param (git pathspec, both stat and full-diff modes) so a large multi-file task can be narrowed to the files actually in question instead of shipping the whole diff. preview_merge_set now also reports Subsets: pairs where one task's changed files are a proper subset of another's in the same set, the strongest available post-hoc signal that a task may be redundant with another.
This commit is contained in:
@@ -143,10 +143,12 @@ public sealed class GitService
|
||||
/// (committed-on-branch changes + uncommitted work). Used for viewing a Claude
|
||||
/// task's total impact relative to where the branch started.
|
||||
/// </summary>
|
||||
public async Task<string> GetBranchDiffAsync(string worktreePath, string baseRef, CancellationToken ct = default)
|
||||
public async Task<string> GetBranchDiffAsync(
|
||||
string worktreePath, string baseRef, IReadOnlyList<string>? paths = null, CancellationToken ct = default)
|
||||
{
|
||||
var (exitCode, stdout, _) = await RunGitAsync(worktreePath,
|
||||
["diff", baseRef], ct);
|
||||
var args = new List<string> { "diff", baseRef };
|
||||
AppendPathFilter(args, paths);
|
||||
var (exitCode, stdout, _) = await RunGitAsync(worktreePath, args, ct);
|
||||
if (exitCode == 0 && !string.IsNullOrWhiteSpace(stdout))
|
||||
return stdout;
|
||||
// Fallback: whatever the worktree has vs HEAD (uncommitted only).
|
||||
@@ -158,24 +160,38 @@ public sealed class GitService
|
||||
/// task's changes after its worktree has been merged away (the commits survive on
|
||||
/// the target branch even though the worktree directory and branch ref are gone).
|
||||
/// </summary>
|
||||
public async Task<string> GetCommitRangeDiffAsync(string repoDir, string baseCommit, string headCommit, CancellationToken ct = default)
|
||||
public async Task<string> GetCommitRangeDiffAsync(
|
||||
string repoDir, string baseCommit, string headCommit, IReadOnlyList<string>? paths = null, CancellationToken ct = default)
|
||||
{
|
||||
var (exitCode, stdout, stderr) = await RunGitAsync(repoDir,
|
||||
["diff", $"{baseCommit}..{headCommit}"], ct);
|
||||
var args = new List<string> { "diff", $"{baseCommit}..{headCommit}" };
|
||||
AppendPathFilter(args, paths);
|
||||
var (exitCode, stdout, stderr) = await RunGitAsync(repoDir, args, ct);
|
||||
if (exitCode != 0)
|
||||
throw new InvalidOperationException($"git diff {baseCommit}..{headCommit} failed (exit {exitCode}): {stderr}");
|
||||
return stdout;
|
||||
}
|
||||
|
||||
public async Task<string> DiffStatAsync(string worktreePath, string baseCommit, string headCommit, CancellationToken ct = default)
|
||||
public async Task<string> DiffStatAsync(
|
||||
string worktreePath, string baseCommit, string headCommit, IReadOnlyList<string>? paths = null, CancellationToken ct = default)
|
||||
{
|
||||
var (exitCode, stdout, stderr) = await RunGitAsync(worktreePath,
|
||||
["diff", "--stat", $"{baseCommit}..{headCommit}"], ct);
|
||||
var args = new List<string> { "diff", "--stat", $"{baseCommit}..{headCommit}" };
|
||||
AppendPathFilter(args, paths);
|
||||
var (exitCode, stdout, stderr) = await RunGitAsync(worktreePath, args, ct);
|
||||
if (exitCode != 0)
|
||||
throw new InvalidOperationException($"git diff --stat failed (exit {exitCode}): {stderr}");
|
||||
return stdout.Trim();
|
||||
}
|
||||
|
||||
// Appends a `-- <paths>` pathspec filter so git itself narrows the diff instead of the
|
||||
// caller filtering the result after the fact (works identically for --stat and full diffs).
|
||||
// No-op when paths is null/empty so existing callers see no behavior change.
|
||||
private static void AppendPathFilter(List<string> args, IReadOnlyList<string>? paths)
|
||||
{
|
||||
if (paths is not { Count: > 0 }) return;
|
||||
args.Add("--");
|
||||
args.AddRange(paths);
|
||||
}
|
||||
|
||||
public async Task<string> GetFileDiffAsync(string worktreePath, string? baseCommit, string relativePath, CancellationToken ct = default)
|
||||
{
|
||||
string[] args = string.IsNullOrEmpty(baseCommit)
|
||||
|
||||
@@ -162,9 +162,9 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
try
|
||||
{
|
||||
raw = FromCommitRange && BaseRef is not null && HeadCommit is not null
|
||||
? await _git.GetCommitRangeDiffAsync(WorktreePath, BaseRef, HeadCommit, ct)
|
||||
? await _git.GetCommitRangeDiffAsync(WorktreePath, BaseRef, HeadCommit, ct: ct)
|
||||
: BaseRef is not null
|
||||
? await _git.GetBranchDiffAsync(WorktreePath, BaseRef, ct)
|
||||
? await _git.GetBranchDiffAsync(WorktreePath, BaseRef, ct: ct)
|
||||
: await _git.GetDiffAsync(WorktreePath, ct);
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
+55
-13
@@ -100,8 +100,15 @@ public sealed record MergePreviewSetEntryDto(
|
||||
|
||||
public sealed record FileOverlapDto(string File, IReadOnlyList<string> TaskIds);
|
||||
|
||||
// TaskId's changed-file list is a non-empty PROPER subset of SupersetTaskId's -- e.g. one task
|
||||
// deletes two files and another deletes just one of those same two. Stronger than an Overlaps
|
||||
// entry (which only says some files are shared): a real subset is the best available machine
|
||||
// signal that TaskId's work may already be entirely covered by SupersetTaskId.
|
||||
public sealed record SubsetRelationDto(string TaskId, string SupersetTaskId);
|
||||
|
||||
public sealed record MergePreviewSetResultDto(
|
||||
IReadOnlyList<MergePreviewSetEntryDto> Tasks, IReadOnlyList<FileOverlapDto> Overlaps);
|
||||
IReadOnlyList<MergePreviewSetEntryDto> Tasks, IReadOnlyList<FileOverlapDto> Overlaps,
|
||||
IReadOnlyList<SubsetRelationDto> Subsets);
|
||||
|
||||
public sealed record WorktreeListItemDto(
|
||||
string? TaskId, string Path, string Branch,
|
||||
@@ -620,7 +627,7 @@ public sealed class ExternalMcpService
|
||||
{
|
||||
if (!Directory.Exists(child.Worktree.Path)) return false;
|
||||
var files = ParseDiffStatFileNames(
|
||||
await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct));
|
||||
await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct: ct));
|
||||
return files.Count == 0;
|
||||
}
|
||||
if (child.HandlerBaseCommit is { Length: > 0 } handlerBase && child.HandlerHeadCommit is { Length: > 0 } handlerHead)
|
||||
@@ -714,6 +721,10 @@ public sealed class ExternalMcpService
|
||||
[Description("false (default): the full unified diff, capped at 200 KB. true: a --stat summary with " +
|
||||
"per-file insertion/deletion counts — start here when the diff may be large.")]
|
||||
bool stat = false,
|
||||
[Description("Restrict the diff to these paths (relative to the repo root), e.g. after a --stat pass or a " +
|
||||
"conflict report already narrowed down which files matter. Omit/empty for every changed file " +
|
||||
"— the default and the only prior behavior. Works in both stat and full-diff mode.")]
|
||||
IReadOnlyList<string>? paths = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
var (repoPath, baseCommit, headCommit) = await LoadDiffRangeAsync(taskId, cancellationToken);
|
||||
@@ -722,13 +733,13 @@ public sealed class ExternalMcpService
|
||||
|
||||
if (stat)
|
||||
{
|
||||
var diffStat = await _git.DiffStatAsync(repoPath, baseCommit, headCommit ?? "HEAD", cancellationToken);
|
||||
var diffStat = await _git.DiffStatAsync(repoPath, baseCommit, headCommit ?? "HEAD", paths, cancellationToken);
|
||||
return new TaskDiffDto(diffStat, ParseDiffStatFileNames(diffStat), false, diffStat.Length);
|
||||
}
|
||||
|
||||
var diff = headCommit is null
|
||||
? await _git.GetBranchDiffAsync(repoPath, baseCommit, cancellationToken)
|
||||
: await _git.GetCommitRangeDiffAsync(repoPath, baseCommit, headCommit, cancellationToken);
|
||||
? await _git.GetBranchDiffAsync(repoPath, baseCommit, paths, cancellationToken)
|
||||
: await _git.GetCommitRangeDiffAsync(repoPath, baseCommit, headCommit, paths, cancellationToken);
|
||||
var files = ParseDiffFileNames(diff);
|
||||
|
||||
if (diff.Length <= maxBytes)
|
||||
@@ -949,11 +960,13 @@ public sealed class ExternalMcpService
|
||||
[McpServerTool, Description(
|
||||
"Plan a batch merge: preview_merge for several tasks against the same targetBranch, plus a file-overlap " +
|
||||
"check between them. Per entry you get preview_merge's fields, or error instead when that task could not " +
|
||||
"be previewed (it is then left out of the overlap computation). overlaps names, for each file touched by " +
|
||||
"MORE THAN ONE of the given tasks, which tasks touch it — a single taskId always yields no overlaps. " +
|
||||
"IMPORTANT: overlap is a HINT and its absence is not safety — two tasks touching entirely different files " +
|
||||
"(one deleting a symbol, another still referencing it) can still collide unflagged, and as with " +
|
||||
"preview_merge a clean result does not mean the merge builds.")]
|
||||
"be previewed (it is then left out of the overlap/subset computation). overlaps names, for each file " +
|
||||
"touched by MORE THAN ONE of the given tasks, which tasks touch it — a single taskId always yields no " +
|
||||
"overlaps. subsets flags a stronger relation: TaskId's changed files are a PROPER subset of " +
|
||||
"SupersetTaskId's — the strongest hint you get post-hoc that TaskId may be redundant with SupersetTaskId, " +
|
||||
"worth checking before merging both. IMPORTANT: neither overlap nor subset is a safety guarantee — two " +
|
||||
"tasks touching entirely different files (one deleting a symbol, another still referencing it) can still " +
|
||||
"collide unflagged, and as with preview_merge a clean result does not mean the merge builds.")]
|
||||
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
||||
IReadOnlyList<string> taskIds,
|
||||
[Description("Branch to preview every task against; defaults to the repo's current branch.")]
|
||||
@@ -990,7 +1003,36 @@ public sealed class ExternalMcpService
|
||||
.OrderBy(o => o.File, StringComparer.OrdinalIgnoreCase)
|
||||
.ToList();
|
||||
|
||||
return new MergePreviewSetResultDto(entries, overlaps);
|
||||
var subsets = FindSubsetRelations(filesByTask);
|
||||
|
||||
return new MergePreviewSetResultDto(entries, overlaps, subsets);
|
||||
}
|
||||
|
||||
// A task's changed-file set is flagged only against another task's STRICTLY LARGER set that
|
||||
// contains every one of its files -- an empty set (isEmpty task) never qualifies, since that
|
||||
// case is already surfaced via IsEmpty and would otherwise match every other task trivially.
|
||||
private static List<SubsetRelationDto> FindSubsetRelations(Dictionary<string, IReadOnlyList<string>> filesByTask)
|
||||
{
|
||||
var fileSets = filesByTask.ToDictionary(
|
||||
kv => kv.Key,
|
||||
kv => new HashSet<string>(kv.Value, StringComparer.OrdinalIgnoreCase));
|
||||
|
||||
var subsets = new List<SubsetRelationDto>();
|
||||
foreach (var (taskId, fileSet) in fileSets)
|
||||
{
|
||||
if (fileSet.Count == 0) continue;
|
||||
foreach (var (otherTaskId, otherFileSet) in fileSets)
|
||||
{
|
||||
if (otherTaskId == taskId || otherFileSet.Count <= fileSet.Count) continue;
|
||||
if (fileSet.IsSubsetOf(otherFileSet))
|
||||
subsets.Add(new SubsetRelationDto(taskId, otherTaskId));
|
||||
}
|
||||
}
|
||||
|
||||
return subsets
|
||||
.OrderBy(s => s.TaskId, StringComparer.OrdinalIgnoreCase)
|
||||
.ThenBy(s => s.SupersetTaskId, StringComparer.OrdinalIgnoreCase)
|
||||
.ToList();
|
||||
}
|
||||
|
||||
// Shared core for PreviewMerge/PreviewMergeSet: throws a clear InvalidOperationException instead of
|
||||
@@ -1029,7 +1071,7 @@ public sealed class ExternalMcpService
|
||||
var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct);
|
||||
|
||||
var changedFiles = Directory.Exists(wt.Path)
|
||||
? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct))
|
||||
? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct: ct))
|
||||
: Array.Empty<string>();
|
||||
|
||||
return (preview, behind, changedFiles, changedFiles.Count == 0);
|
||||
@@ -1043,7 +1085,7 @@ public sealed class ExternalMcpService
|
||||
var isEmpty = string.Equals(handlerBase, handlerHead, StringComparison.Ordinal);
|
||||
var changedFiles = isEmpty
|
||||
? Array.Empty<string>()
|
||||
: ParseDiffStatFileNames(await _git.DiffStatAsync(list.WorkingDir, handlerBase, handlerHead, ct));
|
||||
: ParseDiffStatFileNames(await _git.DiffStatAsync(list.WorkingDir, handlerBase, handlerHead, ct: ct));
|
||||
|
||||
var preview = new MergePreviewResult(TaskMergeService.PreviewClean, Array.Empty<string>(), changedFiles.Count);
|
||||
return (preview, 0, changedFiles, isEmpty);
|
||||
|
||||
@@ -65,7 +65,7 @@ public sealed class PlanningAggregator
|
||||
string unified;
|
||||
try
|
||||
{
|
||||
unified = await _git.GetBranchDiffAsync(wt.Path, wt.BaseCommit, ct);
|
||||
unified = await _git.GetBranchDiffAsync(wt.Path, wt.BaseCommit, ct: ct);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
|
||||
@@ -147,7 +147,7 @@ public sealed class WorktreeManager
|
||||
await _git.CommitAsync(ctx.WorktreePath, message, ct);
|
||||
|
||||
var head = await _git.RevParseHeadAsync(ctx.WorktreePath, ct);
|
||||
var diffStat = await _git.DiffStatAsync(ctx.WorktreePath, ctx.BaseCommit, head, ct);
|
||||
var diffStat = await _git.DiffStatAsync(ctx.WorktreePath, ctx.BaseCommit, head, ct: ct);
|
||||
|
||||
using var context = _dbFactory.CreateDbContext();
|
||||
var wtRepo = new WorktreeRepository(context);
|
||||
|
||||
@@ -769,7 +769,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => sut.GetTaskDiff(task.Id, false, CancellationToken.None));
|
||||
() => sut.GetTaskDiff(task.Id, false, null, CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -785,12 +785,52 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
await mgr.CommitIfChangedAsync(wt, task, list, CancellationToken.None);
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, CancellationToken.None);
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, null, CancellationToken.None);
|
||||
|
||||
Assert.Contains("added.txt", diff.Files);
|
||||
Assert.False(diff.Truncated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTaskDiff_WithPathsFilter_ReturnsOnlyRequestedFile()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var (task, list, wt) = await SeedWorktreeAsync();
|
||||
File.WriteAllText(Path.Combine(wt.WorktreePath, "added.txt"), "content");
|
||||
File.WriteAllText(Path.Combine(wt.WorktreePath, "other.txt"), "other 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);
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, ["added.txt"], CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { "added.txt" }, diff.Files);
|
||||
Assert.DoesNotContain("other.txt", diff.Content);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTaskDiff_StatMode_WithPathsFilter_ReturnsOnlyRequestedFile()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var (task, list, wt) = await SeedWorktreeAsync();
|
||||
File.WriteAllText(Path.Combine(wt.WorktreePath, "added.txt"), "content");
|
||||
File.WriteAllText(Path.Combine(wt.WorktreePath, "other.txt"), "other 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);
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var diff = await sut.GetTaskDiff(task.Id, true, ["other.txt"], CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { "other.txt" }, diff.Files);
|
||||
Assert.DoesNotContain("added.txt", diff.Content);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetTaskDiff_WorktreeLessHandlerTask_UsesHandlerCommitRangeOverListWorkingDir()
|
||||
{
|
||||
@@ -819,7 +859,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, CancellationToken.None);
|
||||
var diff = await sut.GetTaskDiff(task.Id, false, null, CancellationToken.None);
|
||||
|
||||
Assert.Contains("handled.txt", diff.Files);
|
||||
Assert.False(diff.Truncated);
|
||||
@@ -850,7 +890,7 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var diff = await sut.GetTaskDiff(task.Id, true, CancellationToken.None);
|
||||
var diff = await sut.GetTaskDiff(task.Id, true, null, CancellationToken.None);
|
||||
|
||||
Assert.Contains("handled.txt", diff.Content);
|
||||
}
|
||||
@@ -1637,6 +1677,52 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Equal(
|
||||
new[] { taskA.Id, taskB.Id }.OrderBy(x => x),
|
||||
overlap.TaskIds.OrderBy(x => x));
|
||||
// Equal file sets overlap but neither is a PROPER subset of the other.
|
||||
Assert.Empty(result.Subsets);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMergeSet_TaskFilesAreProperSubsetOfAnother_ReportsSubset()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var subsetTask = await SeedTaskAsync(listId, "subset", TaskStatus.WaitingForReview);
|
||||
var supersetTask = await SeedTaskAsync(listId, "superset", TaskStatus.WaitingForReview);
|
||||
|
||||
await CreateActiveWorktreeAsync(repo, subsetTask.Id, "shared.txt", "shared change\n");
|
||||
|
||||
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||
_worktreeCleanups.Add((repo.RepoDir, wtPath));
|
||||
var branch = $"claudedo/{supersetTask.Id[..8]}";
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", branch, wtPath, repo.BaseCommit);
|
||||
File.WriteAllText(Path.Combine(wtPath, "shared.txt"), "shared change too\n");
|
||||
File.WriteAllText(Path.Combine(wtPath, "extra.txt"), "extra\n");
|
||||
GitRepoFixture.RunGit(wtPath, "add", "-A");
|
||||
GitRepoFixture.RunGit(wtPath, "commit", "-m", "edit shared+extra");
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
ctx.Worktrees.Add(new WorktreeEntity
|
||||
{
|
||||
TaskId = supersetTask.Id, Path = wtPath, BranchName = branch,
|
||||
BaseCommit = repo.BaseCommit,
|
||||
HeadCommit = GitRepoFixture.RunGit(wtPath, "rev-parse", "HEAD").Trim(),
|
||||
State = WorktreeState.Active, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMergeSet([subsetTask.Id, supersetTask.Id], "main", CancellationToken.None);
|
||||
|
||||
var relation = Assert.Single(result.Subsets);
|
||||
Assert.Equal(subsetTask.Id, relation.TaskId);
|
||||
Assert.Equal(supersetTask.Id, relation.SupersetTaskId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user