From afe1b68f46a87d9d8155ba0fb633aab4fd6a8c1c Mon Sep 17 00:00:00 2001 From: mika kuns Date: Mon, 10 Aug 2026 12:15:41 +0200 Subject: [PATCH] fix(merge): refuse a merge that would overwrite an untracked file in the target Before merging (or staging a conflict resolution's git add -A), compare the branch's newly-added paths against what's currently untracked in the target working directory. A collision aborts with a new untracked_collision status naming the path and size, surfaced through merge_task/review_task, preview_merge/preview_merge_set (which merge-tree alone can't see), and the UI merge paths via FlashFooterError/ShowErrorAsync instead of a silent catch{}. git's own preflight already refuses this while the path stays untracked at merge time; this closes the gap once a path becomes trackable in between (e.g. an unrelated conflict resolution's git add -A) or across the continue_merge TOCTOU window. --- src/ClaudeDo.Data/Git/GitService.cs | 15 +- src/ClaudeDo.Localization/locales/de.json | 4 +- src/ClaudeDo.Localization/locales/en.json | 4 +- .../Conflicts/ConflictResolverViewModel.cs | 4 +- .../Islands/DetailsIslandViewModel.cs | 4 + .../ViewModels/Modals/MergeModalViewModel.cs | 6 + .../External/ExternalMcpService.cs | 17 ++- .../Lifecycle/TaskMergeService.cs | 94 ++++++++++++ .../Services/TaskMergeServiceTests.cs | 134 ++++++++++++++++++ 9 files changed, 269 insertions(+), 13 deletions(-) diff --git a/src/ClaudeDo.Data/Git/GitService.cs b/src/ClaudeDo.Data/Git/GitService.cs index 651796b4..e0a9c22e 100644 --- a/src/ClaudeDo.Data/Git/GitService.cs +++ b/src/ClaudeDo.Data/Git/GitService.cs @@ -42,6 +42,16 @@ public sealed class GitService }; } + /// + /// The merge base of two refs, or null when git can't find one (e.g. an unresolvable ref) — + /// callers treat that as "can't evaluate", not "no common history". + /// + public async Task MergeBaseAsync(string repoDir, string refA, string refB, CancellationToken ct = default) + { + var (exitCode, stdout, _) = await RunGitAsync(repoDir, ["merge-base", refA, refB], ct); + return exitCode == 0 ? stdout.Trim() : null; + } + public async Task WorktreeAddAsync(string repoDir, string branchName, string worktreePath, string baseCommit, CancellationToken ct = default) { await WorktreeAddGate.WaitAsync(ct); @@ -70,9 +80,12 @@ public sealed class GitService } } + // --untracked-files=all: without it, a brand-new untracked directory collapses into a single + // "?? dir/" entry instead of listing the files inside it — callers matching against specific + // paths (e.g. the untracked-collision guard) need the individual files. public async Task GetStatusPorcelainAsync(string workingDirectory, CancellationToken ct = default) { - var (exitCode, stdout, stderr) = await RunGitAsync(workingDirectory, ["status", "--porcelain"], ct); + var (exitCode, stdout, stderr) = await RunGitAsync(workingDirectory, ["status", "--porcelain", "--untracked-files=all"], ct); if (exitCode != 0) throw new InvalidOperationException($"git status --porcelain failed (exit {exitCode}): {stderr}"); return stdout; diff --git a/src/ClaudeDo.Localization/locales/de.json b/src/ClaudeDo.Localization/locales/de.json index dfdd95ae..6392d51d 100644 --- a/src/ClaudeDo.Localization/locales/de.json +++ b/src/ClaudeDo.Localization/locales/de.json @@ -662,7 +662,7 @@ "tasksIsland": { "completedHeader": "ABGESCHLOSSEN", "completedHeaderCount": "ABGESCHLOSSEN · {0}", "planningOpenFailed": "Planungssitzung konnte nicht geöffnet werden: {0}", "planningResumeFailed": "Planungssitzung konnte nicht fortgesetzt werden: {0}", "approveFailed": "Genehmigen & Mergen fehlgeschlagen: {0}", "cancelReviewFailed": "Prüfung abbrechen fehlgeschlagen: {0}", "sendToQueueFailed": "In die Warteschlange stellen fehlgeschlagen: {0}", "queuePlanBlockedInteractive": "Plan kann nicht in die Warteschlange gestellt werden — {0} hat eine offene interaktive Sitzung und muss zuerst geschlossen werden.", "moveRunningRejected": "Ein laufender Task kann nicht in eine andere Liste verschoben werden.", "moveWorktreeRejected": "Verschieben nicht möglich — dieser Task hat einen aktiven Worktree, der auf sein aktuelles Repo zeigt.", "moveRepoConfirm": "Unterschiedliche Repos — {0} → {1}. Task trotzdem verschieben?", "moveConfirmUnavailable": "Verschieben nicht möglich — der Bestätigungsdialog ist nicht verfügbar.", "quickClaudeNoWorkingDir": "Für diese Liste ist kein Arbeitsverzeichnis konfiguriert.", "quickClaudeDirMissing": "Arbeitsverzeichnis existiert nicht mehr: {0}" }, "diff": { "loadFailed": "Diff konnte nicht geladen werden: {0}", "noChanges": "Keine Änderungen anzuzeigen.", "unavailable": "Diff nicht mehr verfügbar — Commit-Bereich unvollständig." }, "planningDiff": { "hubError": "Kombinierte Vorschau konnte nicht erstellt werden (Hub-Fehler).", "conflict": "Kombinierte Vorschau nicht möglich: Teilaufgabe {0} steht im Konflikt mit einer früheren Teilaufgabe ({1} Dateien).", "buildFailed": "Kombinierte Vorschau konnte nicht erstellt werden: {0}" }, - "merge": { "commitMessage": "Merge-Aufgabe: {0}", "workerOfflineBranches": "Worker offline — Branches können nicht aufgelistet werden.", "loadBranchesFailed": "Branches konnten nicht geladen werden: {0}", "merged": "Zusammengeführt.", "conflict": "Merge-Konflikt — Ziel-Branch wiederhergestellt. Manuell oder über Fortsetzen lösen, dann erneut versuchen.", "blocked": "Blockiert: {0}", "verifyFailed": "Merge ist gelandet, aber das Verify-Kommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf Erledigt gesetzt.", "unknownStatus": "Unbekannter Status: {0}", "mergeFailed": "Merge fehlgeschlagen: {0}" }, + "merge": { "commitMessage": "Merge-Aufgabe: {0}", "workerOfflineBranches": "Worker offline — Branches können nicht aufgelistet werden.", "loadBranchesFailed": "Branches konnten nicht geladen werden: {0}", "merged": "Zusammengeführt.", "conflict": "Merge-Konflikt — Ziel-Branch wiederhergestellt. Manuell oder über Fortsetzen lösen, dann erneut versuchen.", "blocked": "Blockiert: {0}", "verifyFailed": "Merge ist gelandet, aber das Verify-Kommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf Erledigt gesetzt.", "untrackedCollision": "Merge abgelehnt — er würde eine unversionierte Datei im Ziel-Arbeitsverzeichnis überschreiben.", "unknownStatus": "Unbekannter Status: {0}", "mergeFailed": "Merge fehlgeschlagen: {0}" }, "conflictResolution": { "vsCodeError": "VS Code konnte nicht gestartet werden: {0}. Die Pfade sind oben aufgeführt — kopiere sie manuell.", "subtaskPrefix": "Konflikte in Teilaufgabe: {0}", "targetPrefix": "Zusammenführen in: {0}" }, "settingsModal": { "workerOffline": "Worker offline — Einstellungen schreibgeschützt.", "saveFailed": "Speichern fehlgeschlagen: {0}" }, "onlineInbox": { "workerOffline": "Worker offline — Konfiguration kann nicht geladen werden.", "saved": "Konfiguration gespeichert.", "saveFailed": "Speichern fehlgeschlagen: {0}", "signedIn": "Erfolgreich angemeldet.", "signedInNoRole": "Angemeldet, aber diesem Konto fehlt die Rolle 'user' in Zitadel — die Online-Synchronisierung wird abgelehnt, bis die Rolle im ClaudeDo-Projekt zugewiesen wird.", "signInFailed": "Anmeldung fehlgeschlagen: {0}", "signedOut": "Abgemeldet.", "signOutFailed": "Abmeldung fehlgeschlagen: {0}" }, @@ -677,7 +677,7 @@ "worktreesTab": { "workerOffline": "Worker offline.", "removed": "{0} Worktree(s) entfernt.", "blocked": "Zwangsentfernung nicht möglich: {0} Aufgabe(n) laufen noch. Brich sie zuerst ab.", "removedFrom": "{0} Worktree(s) von {1} Aufgabe(n) entfernt.", "cleanupFailed": "Aufräumen fehlgeschlagen: {0}", "resetFailed": "Zurücksetzen fehlgeschlagen: {0}" }, "worktreesOverview": { "titleAll": "Worktrees", "titleList": "Worktrees — {0}", "listFallback": "Liste", "cleanupFailed": "Aufräumen fehlgeschlagen.", "cleanupFailedDetailed": "Aufräumen fehlgeschlagen: {0}", "removed": "{0} Worktree(s) entfernt.", "discardFailed": "Worktree konnte nicht verworfen werden.", "keepFailed": "Worktree konnte nicht behalten werden.", "cannotForceRunning": "Eine laufende Aufgabe kann nicht zwangsweise entfernt werden.", "forceRemoveFailed": "Zwangsentfernung fehlgeschlagen.", "forceRemoveFailedDetailed": "Zwangsentfernung fehlgeschlagen: {0}", "batchProgress": "Merge {0}/{1}…", "batchDone": "{0} gemergt, {1} zu lösen." }, "listSettings": { "untitled": "Unbenannt" }, - "detailsIsland": { "verifyFailed": "Merge ist erfolgt, aber das Verifikationskommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf 'Erledigt' gesetzt." }, + "detailsIsland": { "verifyFailed": "Merge ist erfolgt, aber das Verifikationskommando der Liste ist fehlgeschlagen — die Aufgabe wurde nicht auf 'Erledigt' gesetzt.", "untrackedCollision": "Merge abgelehnt — er würde eine unversionierte Datei im Ziel-Arbeitsverzeichnis überschreiben." }, "lists": { "localSuffix": "{0} / lokal", "smartMyDay": "Mein Tag", "smartImportant": "Wichtig", "smartPlanned": "Geplant", "virtualQueue": "Warteschlange", "virtualRunning": "Läuft", "virtualReview": "Prüfung", "newList": "Neue Liste" }, "repoImport": { "loadFailed": "Gespeicherte Ordner konnten nicht geladen werden: {0}", "saveFailed": "Ordner konnten nicht gespeichert werden: {0}" } }, diff --git a/src/ClaudeDo.Localization/locales/en.json b/src/ClaudeDo.Localization/locales/en.json index a4220211..a33fef17 100644 --- a/src/ClaudeDo.Localization/locales/en.json +++ b/src/ClaudeDo.Localization/locales/en.json @@ -662,7 +662,7 @@ "tasksIsland": { "completedHeader": "COMPLETED", "completedHeaderCount": "COMPLETED · {0}", "planningOpenFailed": "Couldn't open planning session: {0}", "planningResumeFailed": "Couldn't resume planning session: {0}", "approveFailed": "Approve & merge failed: {0}", "cancelReviewFailed": "Cancel review failed: {0}", "sendToQueueFailed": "Send to queue failed: {0}", "queuePlanBlockedInteractive": "Can't queue the plan — {0} has an open interactive session and must be closed first.", "moveRunningRejected": "Can't move a running task to another list.", "moveWorktreeRejected": "Can't move — this task has an active worktree pointing at its current repo.", "moveRepoConfirm": "Different repos — {0} → {1}. Move the task anyway?", "moveConfirmUnavailable": "Can't move — the confirmation dialog isn't available.", "quickClaudeNoWorkingDir": "This list has no working directory configured.", "quickClaudeDirMissing": "Working directory no longer exists: {0}" }, "diff": { "loadFailed": "Failed to load diff: {0}", "noChanges": "No changes to show.", "unavailable": "Diff no longer available — commit range incomplete." }, "planningDiff": { "hubError": "Could not build combined preview (hub error).", "conflict": "Cannot build combined preview: subtask {0} conflicts with an earlier subtask ({1} files).", "buildFailed": "Could not build combined preview: {0}" }, - "merge": { "commitMessage": "Merge task: {0}", "workerOfflineBranches": "Worker offline — cannot list branches.", "loadBranchesFailed": "Failed to load branches: {0}", "merged": "Merged.", "conflict": "Merge conflict — target branch restored. Resolve manually or via Continue, then retry.", "blocked": "Blocked: {0}", "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "unknownStatus": "Unknown status: {0}", "mergeFailed": "Merge failed: {0}" }, + "merge": { "commitMessage": "Merge task: {0}", "workerOfflineBranches": "Worker offline — cannot list branches.", "loadBranchesFailed": "Failed to load branches: {0}", "merged": "Merged.", "conflict": "Merge conflict — target branch restored. Resolve manually or via Continue, then retry.", "blocked": "Blocked: {0}", "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "untrackedCollision": "Merge refused — it would overwrite an untracked file in the target working directory.", "unknownStatus": "Unknown status: {0}", "mergeFailed": "Merge failed: {0}" }, "conflictResolution": { "vsCodeError": "Could not launch VS Code: {0}. Paths are listed above — copy them manually.", "subtaskPrefix": "Conflicts in subtask: {0}", "targetPrefix": "Merging into: {0}" }, "settingsModal": { "workerOffline": "Worker offline — settings read-only.", "saveFailed": "Save failed: {0}" }, "onlineInbox": { "workerOffline": "Worker offline — cannot load config.", "saved": "Config saved.", "saveFailed": "Save failed: {0}", "signedIn": "Signed in successfully.", "signedInNoRole": "Signed in, but this account is missing the 'user' role in Zitadel — online sync will be rejected until the role is granted in the ClaudeDo project.", "signInFailed": "Sign-in failed: {0}", "signedOut": "Signed out.", "signOutFailed": "Sign-out failed: {0}" }, @@ -677,7 +677,7 @@ "worktreesTab": { "workerOffline": "Worker offline.", "removed": "Removed {0} worktree(s).", "blocked": "Cannot force-remove: {0} task(s) still running. Cancel them first.", "removedFrom": "Removed {0} worktree(s) from {1} task(s).", "cleanupFailed": "Cleanup failed: {0}", "resetFailed": "Reset failed: {0}" }, "worktreesOverview": { "titleAll": "Worktrees", "titleList": "Worktrees — {0}", "listFallback": "list", "cleanupFailed": "Cleanup failed.", "cleanupFailedDetailed": "Cleanup failed: {0}", "removed": "Removed {0} worktree(s).", "discardFailed": "Failed to discard worktree.", "keepFailed": "Failed to keep worktree.", "cannotForceRunning": "Cannot force-remove a running task.", "forceRemoveFailed": "Force remove failed.", "forceRemoveFailedDetailed": "Force remove failed: {0}", "batchProgress": "Merging {0}/{1}…", "batchDone": "Merged {0}, {1} need resolution." }, "listSettings": { "untitled": "Untitled" }, - "detailsIsland": { "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done." }, + "detailsIsland": { "verifyFailed": "Merge landed, but the list's verify command failed — the task was kept out of Done.", "untrackedCollision": "Merge refused — it would overwrite an untracked file in the target working directory." }, "lists": { "localSuffix": "{0} / local", "smartMyDay": "My Day", "smartImportant": "Important", "smartPlanned": "Planned", "virtualQueue": "Queue", "virtualRunning": "Running", "virtualReview": "Review", "newList": "New list" }, "repoImport": { "loadFailed": "Couldn't load remembered folders: {0}", "saveFailed": "Couldn't save folders: {0}" } }, diff --git a/src/ClaudeDo.Ui/ViewModels/Conflicts/ConflictResolverViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Conflicts/ConflictResolverViewModel.cs index 186d8484..39098c9c 100644 --- a/src/ClaudeDo.Ui/ViewModels/Conflicts/ConflictResolverViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Conflicts/ConflictResolverViewModel.cs @@ -143,7 +143,9 @@ public sealed partial class ConflictResolverViewModel : ObservableObject var start = await _worker.StartConflictMergeAsync(_taskId, targetBranch); if (!string.Equals(start.Status, "conflict", StringComparison.Ordinal)) { - if (string.Equals(start.Status, "blocked", StringComparison.Ordinal)) + // Any non-conflict outcome here means nothing was left mid-merge for this dialog + // to resolve -- surface why instead of quietly closing with no explanation. + if (start.Status is "blocked" or "untracked_collision") Error = start.ErrorMessage; return false; } diff --git a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs index 71b86dbe..fcd4ccf5 100644 --- a/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs @@ -1207,6 +1207,10 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable // out of Done. Surface that instead of silently looking like nothing happened. else if (!hasChildren && result?.Status == "verify_failed" && ShowErrorAsync != null) await ShowErrorAsync(result.ErrorMessage ?? Loc.T("vm.detailsIsland.verifyFailed")); + // Nothing landed -- the merge was refused before touching the repo because the + // branch would have overwritten an untracked file in the target working directory. + else if (!hasChildren && result?.Status == "untracked_collision" && ShowErrorAsync != null) + await ShowErrorAsync(result.ErrorMessage ?? Loc.T("vm.detailsIsland.untrackedCollision")); } catch (Exception ex) { diff --git a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs index 7197a7fa..a81b5789 100644 --- a/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/Modals/MergeModalViewModel.cs @@ -116,6 +116,12 @@ public sealed partial class MergeModalViewModel : ViewModelBase // point of the gate. ErrorMessage = result.ErrorMessage ?? Loc.T("vm.merge.verifyFailed"); break; + case "untracked_collision": + // Nothing landed -- the merge was refused before touching the repo. Show the + // real message (it names the colliding path and size); the locale string is + // only a fallback. + ErrorMessage = result.ErrorMessage ?? Loc.T("vm.merge.untrackedCollision"); + break; default: ErrorMessage = Loc.T("vm.merge.unknownStatus", result.Status); break; diff --git a/src/ClaudeDo.Worker/External/ExternalMcpService.cs b/src/ClaudeDo.Worker/External/ExternalMcpService.cs index 23fc6319..ee123f24 100644 --- a/src/ClaudeDo.Worker/External/ExternalMcpService.cs +++ b/src/ClaudeDo.Worker/External/ExternalMcpService.cs @@ -929,13 +929,16 @@ 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. IMPORTANT: a clean preview says nothing about whether the result compiles or " + - "passes tests — git can merge two changes cleanly (one file deleting a symbol another still references) and " + - "still break the build. 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. IMPORTANT: a clean preview says nothing about whether the result " + + "compiles or passes tests — git can merge two changes cleanly (one file deleting a symbol another still " + + "references) and still break the build. 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 PreviewMerge( string taskId, [Description("Branch to preview against; defaults to the repo's current branch.")] diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index e98c1c05..e24c372c 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -46,6 +46,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"; @@ -53,6 +54,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 @@ -143,6 +145,82 @@ public sealed class TaskMergeService return trimmed.Length <= maxChars ? trimmed : trimmed[^maxChars..]; } + /// + /// 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 + /// newly added since its merge base with + /// against what's currently untracked in — reusing + /// (run against the branch's own worktree, where + /// HEAD is the branch tip) and rather than adding + /// new git plumbing. Returns null when there's nothing to flag. + /// + private async Task 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 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(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 ParseAddedPaths(string nameStatus) + { + var result = new List(); + 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 ParseUntrackedPaths(string porcelain) + { + var result = new List(); + 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()) @@ -198,6 +276,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) { @@ -315,6 +396,13 @@ 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, and `add -A` + // below stages the whole working directory, not just the conflicted paths. + var collision = await CheckUntrackedCollisionsAsync(list.WorkingDir, "HEAD", wt.Path, wt.BranchName, ct); + if (collision is not null) return collision; + await _git.AddAllAsync(list.WorkingDir, ct); var remaining = await _git.ListConflictedFilesAsync(list.WorkingDir, ct); @@ -513,6 +601,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(), 0); diff --git a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs index 2abefead..70086bbf 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs @@ -349,6 +349,53 @@ public class TaskMergeServiceTests : IDisposable Assert.Equal("merged", result.Status); } + [Fact] + public async Task MergeAsync_BranchAddsPathUntrackedInTarget_RefusesAndLeavesLocalContentIntact() + { + if (!GitRepoFixture.IsGitAvailable()) return; + var repo = NewRepo(); + var db = NewDb(); + var (list, task) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done); + + var wtMgr = BuildWorktreeManager(db); + var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None); + _wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath)); + + // The task branch creates a brand new file at a path it has never seen tracked. + Directory.CreateDirectory(Path.Combine(wtCtx.WorktreePath, "docs")); + File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "docs", "PLUGIN.md"), "agent-written replacement\n"); + await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None); + + // The same path already exists, untracked, in the target working directory — never + // committed, not ignored — carrying content unrelated to the task branch. + Directory.CreateDirectory(Path.Combine(repo.RepoDir, "docs")); + var localPath = Path.Combine(repo.RepoDir, "docs", "PLUGIN.md"); + const string localContent = "local plugin docs — must survive\n"; + File.WriteAllText(localPath, localContent); + + var (svc, proxy) = BuildService(db); + var currentBranch = await new GitService().GetCurrentBranchAsync(repo.RepoDir); + var headBefore = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim(); + + var result = await svc.MergeAsync(task.Id, currentBranch, removeWorktree: false, + commitMessage: "Merge task", ct: CancellationToken.None); + + Assert.Equal(TaskMergeService.StatusUntrackedCollision, result.Status); + Assert.Contains("docs/PLUGIN.md", result.ConflictFiles); + Assert.Contains("docs/PLUGIN.md", result.ErrorMessage ?? ""); + Assert.Contains($"{System.Text.Encoding.UTF8.GetByteCount(localContent)} bytes", result.ErrorMessage ?? ""); + + // Nothing landed: no merge was attempted, the local file is untouched, worktree unaffected. + Assert.Equal(localContent, File.ReadAllText(localPath)); + Assert.Equal(headBefore, GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim()); + Assert.False(await new GitService().IsMidMergeAsync(repo.RepoDir)); + Assert.DoesNotContain(proxy.Calls, c => c.Method == "WorktreeUpdated"); + + using var ctx = db.CreateContext(); + var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.Id); + Assert.Equal(WorktreeState.Active, wt!.State); + } + [Fact] public async Task MergeAsync_TargetBranchDifferentFromHead_ChecksOutBeforeMerging() { @@ -493,6 +540,66 @@ public class TaskMergeServiceTests : IDisposable Assert.True(string.IsNullOrWhiteSpace(GitRepoFixture.RunGit(repo.RepoDir, "status", "--porcelain"))); } + [Fact] + public async Task ContinueMergeAsync_PathBecomesUntrackedDuringResolution_RefusesBeforeAddAll() + { + // git's own preflight already refuses a merge whose incoming branch collides with a path + // that is untracked in the target *at merge-start* (verified against real git while + // investigating this guard). The only path left uncovered is one that becomes untracked + // again *during* conflict resolution, after the initial merge already staged it cleanly — + // this reproduces exactly that narrow window, right before `git add -A` would sweep it + // back in unnoticed. + if (!GitRepoFixture.IsGitAvailable()) return; + + var db = NewDb(); + var repo = NewRepo(); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# main change\n"); + GitRepoFixture.RunGit(repo.RepoDir, "commit", "-am", "main change"); + + var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPath)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/t-untracked", wtPath, repo.BaseCommit); + File.WriteAllText(Path.Combine(wtPath, "README.md"), "# branch change\n"); + File.WriteAllText(Path.Combine(wtPath, "extra.md"), "agent content\n"); + GitRepoFixture.RunGit(wtPath, "add", "-A"); + GitRepoFixture.RunGit(wtPath, "commit", "-m", "branch change + add extra.md"); + + var (_, task) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done); + await SeedWorktree(db, task.Id, wtPath, "claudedo/t-untracked", repo.BaseCommit); + + var (svc, _) = BuildService(db); + + var first = await svc.MergeAsync(task.Id, "main", false, "msg", + leaveConflictsInTree: true, CancellationToken.None); + Assert.Equal(TaskMergeService.StatusConflict, first.Status); + + // extra.md landed clean already (git stages any non-conflicting file the moment the + // merge starts, even while README.md is left conflicted) -- confirm that, then unstage + // it and drop in different, untracked local content, reproducing "became untracked + // again mid-resolution". + Assert.Equal("A", GitRepoFixture.RunGit(repo.RepoDir, "diff", "--name-status", "--cached", "--", "extra.md").Trim()[..1]); + GitRepoFixture.RunGit(repo.RepoDir, "rm", "--cached", "extra.md"); + const string localContent = "important local content that must survive\n"; + File.WriteAllText(Path.Combine(repo.RepoDir, "extra.md"), localContent); + + // Resolve the real (unrelated) conflict. + File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# resolved\n"); + + var result = await svc.ContinueMergeAsync(task.Id, CancellationToken.None); + + Assert.Equal(TaskMergeService.StatusUntrackedCollision, result.Status); + Assert.Contains("extra.md", result.ConflictFiles); + Assert.Equal(localContent, File.ReadAllText(Path.Combine(repo.RepoDir, "extra.md"))); + + // Nothing committed; still mid-merge so abort/continue can still resolve it. + Assert.True(await new GitService().IsMidMergeAsync(repo.RepoDir)); + using var ctx = db.CreateContext(); + var wt = ctx.Worktrees.Single(w => w.TaskId == task.Id); + Assert.Equal(WorktreeState.Active, wt.State); + } + [Fact] public async Task AbortMergeAsync_AfterConflict_RestoresCleanStateAndLeavesWorktreeActive() { @@ -577,6 +684,33 @@ public class TaskMergeServiceTests : IDisposable Assert.Contains("README.md", preview.ConflictFiles); } + [Fact] + public async Task PreviewAsync_BranchAddsPathUntrackedInTarget_ReturnsUntrackedCollision() + { + // merge-tree (the real preview mechanism) never touches the working directory, so + // without this check the preview would say "clean" for a merge that can't actually land. + if (!GitRepoFixture.IsGitAvailable()) return; + var repo = NewRepo(); + var db = NewDb(); + var (list, task) = await SeedListAndTask(db, repo.RepoDir, TaskStatus.WaitingForReview); + + var wtMgr = BuildWorktreeManager(db); + var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None); + _wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath)); + File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "PLUGIN.md"), "agent content\n"); + await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None); + + File.WriteAllText(Path.Combine(repo.RepoDir, "PLUGIN.md"), "local content\n"); + + var (svc, _) = BuildService(db); + var target = await new GitService().GetCurrentBranchAsync(repo.RepoDir); + + var preview = await svc.PreviewAsync(task.Id, target, CancellationToken.None); + + Assert.Equal(TaskMergeService.PreviewUntrackedCollision, preview.Status); + Assert.Contains("PLUGIN.md", preview.ConflictFiles); + } + [Fact] public async Task PreviewAsync_NoActiveWorktree_ReturnsUnavailable() {