Merge branch 'claudedo/295d5d409c194ba28dbb95ab13630832'

src/ClaudeDo.Worker/External/ExternalMcpService.cs

@@ -1059,15 +1059,18 @@ public sealed class ExternalMcpService
 
     [McpServerTool, Description(
         "Check whether a task would merge cleanly before touching anything — `git merge-tree --write-tree`, so the " +
-        "working tree, index and HEAD are untouched. status is 'clean' or 'conflict' (conflictFiles lists where git " +
-        "would stop); 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 additionally " +
-        "built/tested in a scratch worktree (still without touching the real working tree) — verifyExitCode 0 means " +
-        "it built clean, non-zero or -1 (timeout/failed to start) means it doesn't, with the tail of its output in " +
-        "verifyOutputTail; verifyExitCode stays null when no verify command is configured. isEmpty=true means the " +
-        "task's review range contributed nothing; check that flag rather than reading a small changedFileCount as " +
-        "empty. Throws if the task has neither an active worktree nor a handler commit range, or the list's working " +
-        "directory is missing from disk.")]
+        "working tree, index and HEAD are untouched. status is 'clean', 'conflict' (conflictFiles lists where git " +
+        "would stop), or 'untracked_collision' (conflictFiles lists a path the branch adds that already exists, " +
+        "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, " +
+        "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 " +
+        "additionally built/tested in a scratch worktree (still without touching the real working tree) — " +
+        "verifyExitCode 0 means it built clean, non-zero or -1 (timeout/failed to start) means it doesn't, with the " +
+        "tail of its output in verifyOutputTail; verifyExitCode stays null when no verify command is configured. " +
+        "isEmpty=true means the task's review range contributed nothing; check that flag rather than reading a " +
+        "small changedFileCount as empty. Throws 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,
         [Description("Branch to preview against; defaults to the repo's current branch.")]

src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs

@@ -53,6 +53,7 @@ public sealed class TaskMergeService
     public const string StatusBlocked     = "blocked";
     public const string StatusAborted     = "aborted";
     public const string StatusVerifyFailed = "verify_failed";
+    public const string StatusUntrackedCollision = "untracked_collision";
 
     public const string StatusReverted       = "reverted";
     public const string StatusConflictAborted = "conflict_aborted";
@@ -60,6 +61,7 @@ public sealed class TaskMergeService
     public const string PreviewClean       = "clean";
     public const string PreviewConflict    = "conflict";
     public const string PreviewUnavailable = "unavailable";
+    public const string PreviewUntrackedCollision = "untracked_collision";
 
     // The verify command is a trusted, list-owner-configured build/test invocation (not
     // per-request user input), so a generous fixed timeout is enough — no need for a
@@ -150,6 +152,82 @@ public sealed class TaskMergeService
         return trimmed.Length <= maxChars ? trimmed : trimmed[^maxChars..];
     }
 
+    /// <summary>
+    /// Guards against a merge silently overwriting a file that sits untracked in the target
+    /// working directory: git already refuses that itself while the path is still untracked at
+    /// merge time, but a path that only became trackable in between (e.g. an unrelated conflict
+    /// resolution's `git add -A` sweeping it up) loses that protection. Compares the paths
+    /// <paramref name="branchName"/> newly added since its merge base with <paramref name="targetRef"/>
+    /// against what's currently untracked in <paramref name="workingDir"/> — reusing
+    /// <see cref="GitService.GetCommittedFilesAsync"/> (run against the branch's own worktree, where
+    /// HEAD is the branch tip) and <see cref="GitService.GetStatusPorcelainAsync"/> rather than adding
+    /// new git plumbing. Returns null when there's nothing to flag.
+    /// </summary>
+    private async Task<MergeResult?> CheckUntrackedCollisionsAsync(
+        string workingDir, string targetRef, string worktreePath, string branchName, CancellationToken ct)
+    {
+        var mergeBase = await _git.MergeBaseAsync(workingDir, targetRef, branchName, ct);
+        if (mergeBase is null) return null;
+
+        List<string> addedByBranch;
+        try
+        {
+            addedByBranch = ParseAddedPaths(await _git.GetCommittedFilesAsync(worktreePath, mergeBase, ct));
+        }
+        catch { return null; } // worktree missing — nothing to compare, let the merge report its own outcome
+
+        if (addedByBranch.Count == 0) return null;
+
+        var untracked = ParseUntrackedPaths(await _git.GetStatusPorcelainAsync(workingDir, ct));
+        if (untracked.Count == 0) return null;
+
+        var untrackedSet = new HashSet<string>(untracked, StringComparer.OrdinalIgnoreCase);
+        var collisions = addedByBranch.Where(untrackedSet.Contains).ToList();
+        if (collisions.Count == 0) return null;
+
+        var details = collisions.Select(p => $"{p} ({DescribeSize(workingDir, p)})");
+        return new MergeResult(StatusUntrackedCollision, collisions,
+            "merge would overwrite untracked file(s) that exist in the target working directory: " +
+            string.Join(", ", details));
+    }
+
+    private static string DescribeSize(string workingDir, string relativePath)
+    {
+        try
+        {
+            var full = Path.Combine(workingDir, relativePath.Replace('/', Path.DirectorySeparatorChar));
+            return $"{new FileInfo(full).Length} bytes";
+        }
+        catch { return "size unknown"; }
+    }
+
+    // Only "A" (pure add) entries — a path the branch modifies but that already exists on the
+    // target's own history is a normal merge, not a collision with something untracked.
+    private static List<string> ParseAddedPaths(string nameStatus)
+    {
+        var result = new List<string>();
+        foreach (var raw in nameStatus.Split('\n', StringSplitOptions.RemoveEmptyEntries))
+        {
+            var line = raw.TrimEnd('\r');
+            if (line.Length < 2 || line[0] != 'A') continue;
+            var tab = line.IndexOf('\t');
+            if (tab >= 0) result.Add(line[(tab + 1)..].Trim());
+        }
+        return result;
+    }
+
+    private static List<string> ParseUntrackedPaths(string porcelain)
+    {
+        var result = new List<string>();
+        foreach (var raw in porcelain.Split('\n', StringSplitOptions.RemoveEmptyEntries))
+        {
+            var line = raw.TrimEnd('\r');
+            if (line.StartsWith("?? ", StringComparison.Ordinal))
+                result.Add(line[3..].Trim().Trim('"'));
+        }
+        return result;
+    }
+
     private async Task MarkWorktreeMergedAsync(string taskId, string mergeCommitSha, CancellationToken ct)
     {
         using (var ctx = _dbFactory.CreateDbContext())
@@ -205,6 +283,9 @@ public sealed class TaskMergeService
                 catch (Exception ex) { return Blocked($"failed to switch target branch: {ex.Message}"); }
             }
 
+            var collision = await CheckUntrackedCollisionsAsync(list.WorkingDir, "HEAD", wt.Path, wt.BranchName, ct);
+            if (collision is not null) return collision;
+
             var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct);
             if (exitCode != 0)
             {
@@ -322,6 +403,14 @@ public sealed class TaskMergeService
             if (stillConflicted.Count > 0)
                 return new MergeResult(StatusConflict, stillConflicted, "conflicts not fully resolved");
 
+            // Closes the window between the original `git merge` (which already refused any
+            // untracked collision at that point) and this call: an untracked file matching one
+            // of the branch's own added paths could have appeared in the meantime, leaving the
+            // index and the working tree disagreeing about that path — committing then would
+            // either lose the local content or silently drop the branch's own added file.
+            var collision = await CheckUntrackedCollisionsAsync(list.WorkingDir, "HEAD", wt.Path, wt.BranchName, ct);
+            if (collision is not null) return collision;
+
             // Stage exactly the resolved conflict paths — never `git add -A`, which would sweep
             // untracked/unrelated changes left by other sessions into this merge commit (the
             // target working dir is shared).
@@ -554,6 +643,12 @@ public sealed class TaskMergeService
             ? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
             : targetBranch;
 
+        // merge-tree (below) is a pure object-level check — it has no idea the working directory
+        // has an untracked file that would block (or, worse, silently be lost by) the real merge.
+        var collision = await CheckUntrackedCollisionsAsync(list.WorkingDir, target, wt.Path, wt.BranchName, ct);
+        if (collision is not null)
+            return new MergePreviewResult(PreviewUntrackedCollision, collision.ConflictFiles, 0);
+
         var preview = await _git.PreviewMergeAsync(list.WorkingDir, target, wt.BranchName, ct);
         if (!preview.Supported)
             return new MergePreviewResult(PreviewUnavailable, Array.Empty<string>(), 0);