fix(worker): surface empty review ranges and blocked children over MCP
preview_merge/preview_merge_set now report isEmpty (ahead==0, or HandlerBaseCommit==HandlerHeadCommit for a worktree-less list-handler task) instead of leaving an empty branch indistinguishable from a small one. preview_merge also stops throwing for worktree-less handler tasks, falling back to their fixed commit range. review_task's parent approve returns emptyChildren, naming the Done children whose review range contributed nothing before the unit merge lands. TaskRefDto/TaskDto now expose roadblockCount so a CLAUDEDO_BLOCKED child is identifiable over MCP, since it still reaches Done per the unified parent model.
This commit is contained in:
mika kuns
@@ -1,8 +1,8 @@
|
||||
# External MCP tool surface
|
||||
|
||||
> **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative.
|
||||
> Last verified against commit `86f962e` (2026-08-06).
|
||||
> Drift check: `git log --oneline 86f962e..HEAD -- src/ClaudeDo.Worker/External`
|
||||
> Last verified against commit `8247a74` (2026-08-06).
|
||||
> Drift check: `git log --oneline 8247a74..HEAD -- src/ClaudeDo.Worker/External`
|
||||
> Stable structure only (no line numbers). See docs/explore-notes/README.md.
|
||||
|
||||
Covers `src/ClaudeDo.Worker/External/` — the always-on MCP tools ClaudeDo exposes to general
|
||||
@@ -85,17 +85,35 @@ happens before the lean/full projection either way.
|
||||
`WaitingForReview` task. Approve is review+merge exactly like the hub's `ApproveReview`: unit
|
||||
merge for parents, worktree merge into optional `targetBranch` for childless tasks. Conflicts
|
||||
are reported in `ReviewTaskResult`.
|
||||
- A parent's approve also returns `emptyChildren`: the `Done` children about to be unit-merged
|
||||
whose own review range contributed nothing (computed the same way as `PreviewMerge`'s
|
||||
`isEmpty`, before the merge starts so it reflects what's about to be approved). Surfaces a
|
||||
child that reported `CLAUDEDO_BLOCKED` and committed no code — previously that child reached
|
||||
`Done` and merged silently with `changedFileCount: 0`, indistinguishable from a small-but-real
|
||||
change. `TaskRefDto.roadblockCount` (on every task-returning tool, stamped by `TaskRunner` from
|
||||
`result.Blocks.Count`) is the MCP-visible signal for *why* a child is empty.
|
||||
|
||||
**`PreviewMerge`** — non-destructive `git merge-tree --write-tree` mergeability check for one
|
||||
task's worktree branch against `targetBranch` (default: the repo's current branch). Returns
|
||||
status / conflictFiles / changedFileCount / `behind`. Unlike `TaskMergeService.PreviewAsync`'s
|
||||
silent *"unavailable"*, this **throws a clear error** when the task has no worktree, the
|
||||
worktree isn't `Active`, or the list's working dir is missing.
|
||||
status / conflictFiles / changedFileCount / `behind` / `isEmpty`. Unlike
|
||||
`TaskMergeService.PreviewAsync`'s silent *"unavailable"*, this **throws a clear error** when the
|
||||
task has neither an active worktree nor a handler commit range, or the list's working dir is
|
||||
missing.
|
||||
- `isEmpty` = the review range contributed nothing — zero files changed against the worktree's
|
||||
base commit, or (for a worktree-less list-handler host task) `HandlerBaseCommit ==
|
||||
HandlerHeadCommit`. Distinguishes a genuinely empty branch from one that merely made a small
|
||||
change (`changedFileCount: 0` alone reads as "tiny", not "nothing to review") — the gap that
|
||||
let two blocked planning children reach `Done` with unmerged empty branches unnoticed.
|
||||
- A worktree-less handler task has no separate branch to `merge-tree`-preview (its commits
|
||||
already sit in `list.WorkingDir`) — `PreviewMergeCoreAsync` falls back to a synthetic `clean`
|
||||
preview over its own `HandlerBaseCommit..HandlerHeadCommit` diff-stat instead of throwing
|
||||
"has no worktree".
|
||||
|
||||
**`PreviewMergeSet`** — same preview for a batch, plus a file→tasks overlap report built from
|
||||
each task's own diff-stat. ⚠️ That overlap report is a **same-file-name hint only** — it is
|
||||
blind to cross-file collisions (e.g. the CS0103 case that motivated it). A task that fails to
|
||||
preview gets `error` set and is excluded from the overlap instead of aborting the batch.
|
||||
**`PreviewMergeSet`** — same preview for a batch (each entry also carries `isEmpty`), plus a
|
||||
file→tasks overlap report built from each task's own diff-stat. ⚠️ That overlap report is a
|
||||
**same-file-name hint only** — it is blind to cross-file collisions (e.g. the CS0103 case that
|
||||
motivated it). A task that fails to preview gets `error` set and is excluded from the overlap
|
||||
instead of aborting the batch.
|
||||
|
||||
**`RevertMerge`** — undoes a previously merged task's merge commit on `targetBranch` via
|
||||
`git revert -m 1`. Always a **new commit**, never a reset/rewrite, because the target working
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
# Review, merge & conflict resolution
|
||||
|
||||
> **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative.
|
||||
> Last verified against commit `f6cb825` (2026-08-05).
|
||||
> Drift check: `git log --oneline f6cb825..HEAD -- src/ClaudeDo.Worker/Lifecycle src/ClaudeDo.Worker/State src/ClaudeDo.Worker/Planning src/ClaudeDo.Ui/ViewModels/Conflicts`
|
||||
> Last verified against commit `8247a74` (2026-08-06).
|
||||
> Drift check: `git log --oneline 8247a74..HEAD -- src/ClaudeDo.Worker/Lifecycle src/ClaudeDo.Worker/State src/ClaudeDo.Worker/Planning src/ClaudeDo.Ui/ViewModels/Conflicts src/ClaudeDo.Worker/External`
|
||||
> Stable structure only (no line numbers). See docs/explore-notes/README.md.
|
||||
|
||||
Covers the review→merge path: `TaskStateService` review transitions, `TaskMergeService`,
|
||||
@@ -47,6 +47,15 @@ advance the parent — the parent stays in `WaitingForChildren` until every chil
|
||||
The UI surfaces blocked children on the parent's Session tab (`ChildOutcomes` + a "children
|
||||
need attention" band) so the roadblock is visible without forcing a transition.
|
||||
|
||||
A blocked planning/improvement child still goes straight to `Done` per the unified parent model
|
||||
above — it committed nothing, but nothing prevents its (empty) branch from being unit-merged
|
||||
like any other `Done` child once the parent is approved. The MCP surface has no UI equivalent of
|
||||
`ChildOutcomes`, so `review_task`'s approve on a parent additionally returns `emptyChildren` (the
|
||||
`Done` children whose review range is empty) and every task-returning tool exposes
|
||||
`TaskRefDto.roadblockCount` → [external-mcp.md](external-mcp.md) → `ReviewTask`/`PreviewMerge`.
|
||||
An empty branch is still mergeable by design (some tasks — e.g. an audit — legitimately produce
|
||||
no diff); this is a visibility fix, not a merge gate.
|
||||
|
||||
## Post-merge verify gate
|
||||
|
||||
A list can set `ListConfigEntity.VerifyCommand` (List Settings modal → Verification).
|
||||
|
||||
+136
-36
@@ -19,7 +19,10 @@ namespace ClaudeDo.Worker.External;
|
||||
public sealed record TaskListDto(string Id, string Name, string? WorkingDir);
|
||||
public sealed record DeleteTaskResult(bool Deleted, string Id);
|
||||
public sealed record CancelTaskResult(bool Cancelled, string Id);
|
||||
public sealed record ReviewTaskResult(TaskRefDto Task, string? MergeStatus, IReadOnlyList<string> MergeConflicts, string? MergeMessage, string? RepoPath = null);
|
||||
// EmptyChildren is non-null only for a parent's approve (unit merge): the Done children whose
|
||||
// review range (worktree ahead, or HandlerBaseCommit..HandlerHeadCommit for a worktree-less
|
||||
// child) contributed nothing, so a reviewer sees them before approving instead of after.
|
||||
public sealed record ReviewTaskResult(TaskRefDto Task, string? MergeStatus, IReadOnlyList<string> MergeConflicts, string? MergeMessage, string? RepoPath = null, IReadOnlyList<TaskRefDto>? EmptyChildren = null);
|
||||
public sealed record StatusValueDto(string Status, string Meaning);
|
||||
public sealed record RunTaskNowResult(bool Started, string TaskId);
|
||||
|
||||
@@ -35,7 +38,12 @@ public sealed record TaskDto(
|
||||
DateTime? StartedAt,
|
||||
DateTime? FinishedAt,
|
||||
bool IsMyDay,
|
||||
int SortOrder);
|
||||
int SortOrder,
|
||||
// Count of CLAUDEDO_BLOCKED roadblocks the run reported, stamped by TaskRunner on finish.
|
||||
// A planning/improvement child reporting > 0 still goes straight to Done (see
|
||||
// ClaudeDo.Worker/CLAUDE.md → Unified parent model) -- this is the only MCP-visible signal
|
||||
// that it may have delivered nothing despite that Done status.
|
||||
int RoadblockCount = 0);
|
||||
|
||||
// Lean counterpart to TaskDto for writing/status-changing tools: echoes back what changed
|
||||
// without re-sending Description/Result, which the caller just sent or already has.
|
||||
@@ -45,7 +53,8 @@ public sealed record TaskRefDto(
|
||||
string Title,
|
||||
string Status,
|
||||
int SortOrder,
|
||||
bool IsMyDay);
|
||||
bool IsMyDay,
|
||||
int RoadblockCount = 0);
|
||||
|
||||
// tasks is populated when includeDescription=false (the default): lean references, no
|
||||
// Description/Result. tasksFull is populated when includeDescription=true: full tasks incl.
|
||||
@@ -71,11 +80,15 @@ public sealed record MergeContinuationResultDto(
|
||||
bool Merged, string TaskStatus, IReadOnlyList<string> Conflicts,
|
||||
string? RepoPath, string? Message);
|
||||
|
||||
// IsEmpty = the review range contributed nothing (worktree ahead-of-base is empty, or a
|
||||
// worktree-less handler task's HandlerBaseCommit == HandlerHeadCommit) -- distinguishable from
|
||||
// a merge that is merely small, so an empty branch can't be misread as "changedFileCount: 0
|
||||
// means tiny" when it actually means "nothing to review".
|
||||
public sealed record MergePreviewToolDto(
|
||||
string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind);
|
||||
string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind, bool IsEmpty = false);
|
||||
|
||||
public sealed record MergePreviewSetEntryDto(
|
||||
string TaskId, string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind, string? Error);
|
||||
string TaskId, string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind, string? Error, bool IsEmpty = false);
|
||||
|
||||
public sealed record FileOverlapDto(string File, IReadOnlyList<string> TaskIds);
|
||||
|
||||
@@ -396,7 +409,10 @@ public sealed class ExternalMcpService
|
||||
"decision='reject_park' → Idle for manual editing (feedback ignored). " +
|
||||
"decision='cancel' → Cancelled. " +
|
||||
"Fails if the task is not currently WaitingForReview (except cancel, which also works while Running/Queued). " +
|
||||
"The result's task field is a lean reference (id, listId, title, status, sortOrder, isMyDay), not the task's description.")]
|
||||
"The result's task field is a lean reference (id, listId, title, status, sortOrder, isMyDay), not the task's description. " +
|
||||
"emptyChildren (parent approve only) lists 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 delivered something.")]
|
||||
public async Task<ReviewTaskResult> ReviewTask(
|
||||
string taskId,
|
||||
string decision,
|
||||
@@ -412,6 +428,7 @@ public sealed class ExternalMcpService
|
||||
IReadOnlyList<string> mergeConflicts = Array.Empty<string>();
|
||||
string? mergeMessage = null;
|
||||
string? repoPath = null;
|
||||
IReadOnlyList<TaskRefDto>? emptyChildren = null;
|
||||
|
||||
if (decision.Trim().ToLowerInvariant() == "approve")
|
||||
{
|
||||
@@ -423,6 +440,10 @@ public sealed class ExternalMcpService
|
||||
|
||||
if (hasChildren)
|
||||
{
|
||||
// Compute before the merge starts -- children are still Done with their own
|
||||
// pre-merge worktree/commit range at this point, so "did it contribute anything"
|
||||
// reflects the review range the reviewer is about to approve.
|
||||
emptyChildren = await GetEmptyDoneChildrenAsync(taskId, cancellationToken);
|
||||
await _planningMerge.StartAsync(taskId, targetBranch ?? "", cancellationToken);
|
||||
var parentDone = (await _tasks.GetByIdAsync(taskId, cancellationToken))!.Status == TaskStatus.Done;
|
||||
mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict;
|
||||
@@ -478,7 +499,51 @@ public sealed class ExternalMcpService
|
||||
|
||||
return new ReviewTaskResult(
|
||||
ToRefDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!),
|
||||
mergeStatus, mergeConflicts, mergeMessage, repoPath);
|
||||
mergeStatus, mergeConflicts, mergeMessage, repoPath, emptyChildren);
|
||||
}
|
||||
|
||||
// Done children about to be unit-merged whose own review range contributed nothing: an
|
||||
// active worktree with zero changed files against its base commit, or a worktree-less
|
||||
// handler child whose HandlerBaseCommit == HandlerHeadCommit. A child with neither (never
|
||||
// committed anything at all) also counts as empty. Best-effort per child -- a diff failure
|
||||
// on one child must not block the caller from seeing the others or from approving.
|
||||
private async Task<IReadOnlyList<TaskRefDto>> GetEmptyDoneChildrenAsync(string parentTaskId, CancellationToken ct)
|
||||
{
|
||||
await using var ctx = await _dbFactory.CreateDbContextAsync(ct);
|
||||
var children = await ctx.Tasks
|
||||
.AsNoTracking()
|
||||
.Include(t => t.Worktree)
|
||||
.Where(t => t.ParentTaskId == parentTaskId && t.Status == TaskStatus.Done)
|
||||
.ToListAsync(ct);
|
||||
|
||||
var empty = new List<TaskRefDto>();
|
||||
foreach (var child in children)
|
||||
{
|
||||
if (await IsChildEmptyAsync(child, ct))
|
||||
empty.Add(ToRefDto(child));
|
||||
}
|
||||
return empty;
|
||||
}
|
||||
|
||||
private async Task<bool> IsChildEmptyAsync(TaskEntity child, CancellationToken ct)
|
||||
{
|
||||
try
|
||||
{
|
||||
if (child.Worktree is not null)
|
||||
{
|
||||
if (!Directory.Exists(child.Worktree.Path)) return false;
|
||||
var files = ParseDiffStatFileNames(
|
||||
await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct));
|
||||
return files.Count == 0;
|
||||
}
|
||||
if (child.HandlerBaseCommit is { Length: > 0 } handlerBase && child.HandlerHeadCommit is { Length: > 0 } handlerHead)
|
||||
return string.Equals(handlerBase, handlerHead, StringComparison.Ordinal);
|
||||
return true;
|
||||
}
|
||||
catch
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
[McpServerTool, Description("Immediately run a task in the override execution slot (bypasses the agent queue). Returns { started: true, taskId } on success.")]
|
||||
@@ -800,15 +865,18 @@ public sealed class ExternalMcpService
|
||||
"IMPORTANT: a clean preview says nothing about whether the merged result compiles or passes tests — git " +
|
||||
"can merge two changes cleanly (e.g. one file deletes a symbol another file still references) and still " +
|
||||
"break the build. " +
|
||||
"Throws a clear error if the task has no worktree, the worktree is not Active, or the list's working " +
|
||||
"directory is missing from disk.")]
|
||||
"isEmpty=true means the task's review range contributed nothing (no commits ahead of base, or — for a " +
|
||||
"worktree-less list-handler host task — HandlerBaseCommit == HandlerHeadCommit); do not mistake a small " +
|
||||
"changedFileCount for an empty one, check isEmpty instead. " +
|
||||
"Throws a clear error if the task has neither an active worktree nor a handler commit range, or the " +
|
||||
"list's working directory is missing from disk.")]
|
||||
public async Task<MergePreviewToolDto> PreviewMerge(
|
||||
string taskId,
|
||||
string? targetBranch = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
var (preview, behind, _) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind);
|
||||
var (preview, behind, _, isEmpty) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, isEmpty);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
@@ -820,7 +888,8 @@ public sealed class ExternalMcpService
|
||||
"which tasks touch it — passing a single taskId always yields an empty overlaps list. " +
|
||||
"IMPORTANT: file-name overlap is a HINT, not a guarantee of a real collision, and its absence is not a " +
|
||||
"guarantee of safety — two tasks touching different files entirely (e.g. one deletes a symbol, another " +
|
||||
"still references it elsewhere) can still collide, and this tool will not flag that case.")]
|
||||
"still references it elsewhere) can still collide, and this tool will not flag that case. " +
|
||||
"isEmpty=true (per entry) means that task's review range contributed nothing — see preview_merge.")]
|
||||
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
||||
IReadOnlyList<string> taskIds,
|
||||
string? targetBranch = null,
|
||||
@@ -836,9 +905,9 @@ public sealed class ExternalMcpService
|
||||
{
|
||||
try
|
||||
{
|
||||
var (preview, behind, changedFiles) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
var (preview, behind, changedFiles, isEmpty) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
entries.Add(new MergePreviewSetEntryDto(
|
||||
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null));
|
||||
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null, isEmpty));
|
||||
filesByTask[taskId] = changedFiles;
|
||||
}
|
||||
catch (InvalidOperationException ex)
|
||||
@@ -861,32 +930,61 @@ public sealed class ExternalMcpService
|
||||
|
||||
// Shared core for PreviewMerge/PreviewMergeSet: throws a clear InvalidOperationException instead of
|
||||
// TaskMergeService.PreviewAsync's silent "unavailable" status, and adds `behind` + the task's own
|
||||
// changed-file list (via diff-stat, not the merge-tree preview) for overlap detection.
|
||||
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles)> PreviewMergeCoreAsync(
|
||||
// changed-file list (via diff-stat, not the merge-tree preview) for overlap detection, plus
|
||||
// `isEmpty`. A worktree-less list-handler host task has no branch to merge-tree-preview at all
|
||||
// (its commits already sit on the list's working dir) — falls back to the fixed
|
||||
// HandlerBaseCommit..HandlerHeadCommit range, reporting a synthetic "clean" preview of that
|
||||
// range's own diff-stat instead of throwing "has no worktree".
|
||||
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles, bool IsEmpty)> PreviewMergeCoreAsync(
|
||||
string taskId, string? targetBranch, CancellationToken ct)
|
||||
{
|
||||
var (_, list, wt) = await LoadWorktreeContextAsync(taskId, ct);
|
||||
if (wt.State != WorktreeState.Active)
|
||||
throw new InvalidOperationException(
|
||||
$"Worktree state must be Active to preview a merge (current: {wt.State}).");
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
||||
throw new InvalidOperationException("The list's working directory no longer exists.");
|
||||
using var ctx = _dbFactory.CreateDbContext();
|
||||
var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct)
|
||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||
var list = await new ListRepository(ctx).GetByIdAsync(task.ListId, ct)
|
||||
?? throw new InvalidOperationException("List not found.");
|
||||
var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(taskId, ct);
|
||||
|
||||
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", ct);
|
||||
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
||||
throw new InvalidOperationException(
|
||||
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
||||
if (wt is not null)
|
||||
{
|
||||
if (wt.State != WorktreeState.Active)
|
||||
throw new InvalidOperationException(
|
||||
$"Worktree state must be Active to preview a merge (current: {wt.State}).");
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
||||
throw new InvalidOperationException("The list's working directory no longer exists.");
|
||||
|
||||
var target = string.IsNullOrWhiteSpace(targetBranch)
|
||||
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||
: targetBranch;
|
||||
var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct);
|
||||
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", ct);
|
||||
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
||||
throw new InvalidOperationException(
|
||||
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
||||
|
||||
var changedFiles = Directory.Exists(wt.Path)
|
||||
? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct))
|
||||
: Array.Empty<string>();
|
||||
var target = string.IsNullOrWhiteSpace(targetBranch)
|
||||
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||
: targetBranch;
|
||||
var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct);
|
||||
|
||||
return (preview, behind, changedFiles);
|
||||
var changedFiles = Directory.Exists(wt.Path)
|
||||
? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct))
|
||||
: Array.Empty<string>();
|
||||
|
||||
return (preview, behind, changedFiles, changedFiles.Count == 0);
|
||||
}
|
||||
|
||||
if (task.HandlerBaseCommit is { Length: > 0 } handlerBase && task.HandlerHeadCommit is { Length: > 0 } handlerHead)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
||||
throw new InvalidOperationException("The list's working directory no longer exists.");
|
||||
|
||||
var isEmpty = string.Equals(handlerBase, handlerHead, StringComparison.Ordinal);
|
||||
var changedFiles = isEmpty
|
||||
? Array.Empty<string>()
|
||||
: ParseDiffStatFileNames(await _git.DiffStatAsync(list.WorkingDir, handlerBase, handlerHead, ct));
|
||||
|
||||
var preview = new MergePreviewResult(TaskMergeService.PreviewClean, Array.Empty<string>(), changedFiles.Count);
|
||||
return (preview, 0, changedFiles, isEmpty);
|
||||
}
|
||||
|
||||
throw new InvalidOperationException($"Task {taskId} has no worktree.");
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
@@ -1165,7 +1263,8 @@ public sealed class ExternalMcpService
|
||||
t.StartedAt,
|
||||
t.FinishedAt,
|
||||
t.IsMyDay,
|
||||
t.SortOrder);
|
||||
t.SortOrder,
|
||||
t.RoadblockCount);
|
||||
|
||||
private static TaskRefDto ToRefDto(TaskEntity t) => new(
|
||||
t.Id,
|
||||
@@ -1173,7 +1272,8 @@ public sealed class ExternalMcpService
|
||||
t.Title,
|
||||
t.Status.ToString(),
|
||||
t.SortOrder,
|
||||
t.IsMyDay);
|
||||
t.IsMyDay,
|
||||
t.RoadblockCount);
|
||||
}
|
||||
|
||||
internal static class DailyPrepFilter
|
||||
|
||||
@@ -367,6 +367,71 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == childId).State);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ReviewTask_Approve_ParentWithChildren_ReportsEmptyChildByName()
|
||||
{
|
||||
// A blocked child (CLAUDEDO_BLOCKED, committed nothing) still gets an active worktree at
|
||||
// its base commit -- TaskRunner creates one before the run starts -- so "empty" shows up
|
||||
// as zero commits ahead of base, not as a missing worktree.
|
||||
if (!GitAvailable) return;
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
|
||||
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
var parentId = Guid.NewGuid().ToString();
|
||||
var normalChildId = Guid.NewGuid().ToString();
|
||||
var blockedChildId = Guid.NewGuid().ToString();
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
ctx.Tasks.Add(new TaskEntity { Id = parentId, ListId = listId, Title = "plan", CreatedAt = DateTime.UtcNow,
|
||||
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.Finalized });
|
||||
ctx.Tasks.Add(new TaskEntity { Id = normalChildId, ListId = listId, Title = "normal child", CreatedAt = DateTime.UtcNow,
|
||||
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1 });
|
||||
ctx.Tasks.Add(new TaskEntity { Id = blockedChildId, ListId = listId, Title = "blocked child", CreatedAt = DateTime.UtcNow,
|
||||
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2, RoadblockCount = 1 });
|
||||
|
||||
var normalWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||
_worktreeCleanups.Add((repo.RepoDir, normalWtPath));
|
||||
var normalBranch = $"claudedo/{normalChildId[..8]}";
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", normalBranch, normalWtPath, repo.BaseCommit);
|
||||
File.WriteAllText(Path.Combine(normalWtPath, "child.txt"), "c\n");
|
||||
GitRepoFixture.RunGit(normalWtPath, "add", "child.txt");
|
||||
GitRepoFixture.RunGit(normalWtPath, "commit", "-m", "add child.txt");
|
||||
ctx.Worktrees.Add(new WorktreeEntity
|
||||
{
|
||||
TaskId = normalChildId, Path = normalWtPath, BranchName = normalBranch,
|
||||
BaseCommit = repo.BaseCommit,
|
||||
HeadCommit = GitRepoFixture.RunGit(normalWtPath, "rev-parse", "HEAD").Trim(),
|
||||
State = WorktreeState.Active, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
|
||||
var blockedWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||
_worktreeCleanups.Add((repo.RepoDir, blockedWtPath));
|
||||
var blockedBranch = $"claudedo/{blockedChildId[..8]}";
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", blockedBranch, blockedWtPath, repo.BaseCommit);
|
||||
ctx.Worktrees.Add(new WorktreeEntity
|
||||
{
|
||||
TaskId = blockedChildId, Path = blockedWtPath, BranchName = blockedBranch,
|
||||
BaseCommit = repo.BaseCommit,
|
||||
HeadCommit = repo.BaseCommit,
|
||||
State = WorktreeState.Active, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.ReviewTask(parentId, "approve", null, "main", cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus);
|
||||
Assert.Equal("Done", result.Task.Status);
|
||||
Assert.NotNull(result.EmptyChildren);
|
||||
var empty = Assert.Single(result.EmptyChildren!);
|
||||
Assert.Equal(blockedChildId, empty.Id);
|
||||
Assert.DoesNotContain(result.EmptyChildren!, c => c.Id == normalChildId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DeleteTask_RemovesTask()
|
||||
{
|
||||
@@ -1424,10 +1489,89 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Empty(result.ConflictFiles);
|
||||
Assert.Equal(1, result.ChangedFileCount);
|
||||
Assert.Equal(0, result.Behind);
|
||||
Assert.False(result.IsEmpty);
|
||||
Assert.Equal(headBefore, await git.RevParseHeadAsync(list.WorkingDir!, CancellationToken.None));
|
||||
Assert.False(await git.HasChangesAsync(list.WorkingDir!, CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_EmptyWorktree_ReturnsIsEmptyTrue()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
// Worktree created but nothing committed beyond base -- e.g. a child that reported
|
||||
// CLAUDEDO_BLOCKED before writing any code.
|
||||
var (task, list, _) = await SeedWorktreeAsync(TaskStatus.WaitingForReview);
|
||||
var target = await new GitService().GetCurrentBranchAsync(list.WorkingDir!, CancellationToken.None);
|
||||
var sut = BuildSut(CreateQueue());
|
||||
|
||||
var result = await sut.PreviewMerge(task.Id, target, CancellationToken.None);
|
||||
|
||||
Assert.Equal(TaskMergeService.PreviewClean, result.Status);
|
||||
Assert.Equal(0, result.ChangedFileCount);
|
||||
Assert.True(result.IsEmpty);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_WorktreeLessHandlerTask_EmptyRange_ReturnsIsEmptyTrue()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview);
|
||||
// Worktree-less list-handler host task whose handler range never actually committed
|
||||
// anything: HandlerBaseCommit == HandlerHeadCommit, not "no worktree -> unknown".
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
var t = await ctx.Tasks.FindAsync(task.Id);
|
||||
t!.HandlerBaseCommit = repo.BaseCommit;
|
||||
t.HandlerHeadCommit = repo.BaseCommit;
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None);
|
||||
|
||||
Assert.Equal(TaskMergeService.PreviewClean, result.Status);
|
||||
Assert.Equal(0, result.ChangedFileCount);
|
||||
Assert.True(result.IsEmpty);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_WorktreeLessHandlerTask_NonEmptyRange_ReturnsIsEmptyFalse()
|
||||
{
|
||||
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
|
||||
|
||||
var repo = new GitRepoFixture();
|
||||
_repos.Add(repo);
|
||||
var listId = Guid.NewGuid().ToString();
|
||||
await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow });
|
||||
var task = await SeedTaskAsync(listId, status: TaskStatus.WaitingForReview);
|
||||
|
||||
File.WriteAllText(Path.Combine(repo.RepoDir, "handled.txt"), "content");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "handled task merge");
|
||||
var headCommit = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
var t = await ctx.Tasks.FindAsync(task.Id);
|
||||
t!.HandlerBaseCommit = repo.BaseCommit;
|
||||
t.HandlerHeadCommit = headCommit;
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMerge(task.Id, null, CancellationToken.None);
|
||||
|
||||
Assert.Equal(TaskMergeService.PreviewClean, result.Status);
|
||||
Assert.Equal(1, result.ChangedFileCount);
|
||||
Assert.False(result.IsEmpty);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMerge_ConflictingBranches_ReturnsConflictStatusAndDoesNotChangeRepo()
|
||||
{
|
||||
@@ -1546,6 +1690,49 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
Assert.Empty(result.Overlaps);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PreviewMergeSet_MixedEmptyAndNonEmpty_ReportsIsEmptyPerEntry()
|
||||
{
|
||||
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 emptyTask = await SeedTaskAsync(listId, "empty", TaskStatus.WaitingForReview);
|
||||
var fullTask = await SeedTaskAsync(listId, "full", TaskStatus.WaitingForReview);
|
||||
|
||||
var emptyBranch = $"claudedo/{emptyTask.Id[..8]}";
|
||||
var emptyWtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
|
||||
_worktreeCleanups.Add((repo.RepoDir, emptyWtPath));
|
||||
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", emptyBranch, emptyWtPath, repo.BaseCommit);
|
||||
using (var ctx = _db.CreateContext())
|
||||
{
|
||||
ctx.Worktrees.Add(new WorktreeEntity
|
||||
{
|
||||
TaskId = emptyTask.Id, Path = emptyWtPath, BranchName = emptyBranch,
|
||||
BaseCommit = repo.BaseCommit,
|
||||
HeadCommit = repo.BaseCommit,
|
||||
State = WorktreeState.Active, CreatedAt = DateTime.UtcNow,
|
||||
});
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
|
||||
await CreateActiveWorktreeAsync(repo, fullTask.Id, "full.txt", "content\n");
|
||||
|
||||
var sut = BuildSut(CreateQueue());
|
||||
var result = await sut.PreviewMergeSet([emptyTask.Id, fullTask.Id], "main", CancellationToken.None);
|
||||
|
||||
var emptyEntry = result.Tasks.Single(t => t.TaskId == emptyTask.Id);
|
||||
var fullEntry = result.Tasks.Single(t => t.TaskId == fullTask.Id);
|
||||
Assert.Null(emptyEntry.Error);
|
||||
Assert.True(emptyEntry.IsEmpty);
|
||||
Assert.Null(fullEntry.Error);
|
||||
Assert.False(fullEntry.IsEmpty);
|
||||
}
|
||||
|
||||
// ── AddTask model override ────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user