From 5d362b697337a0abf23130d7361da66a4a6648cc Mon Sep 17 00:00:00 2001 From: mika kuns Date: Mon, 10 Aug 2026 11:58:34 +0200 Subject: [PATCH] fix(worker): stop sibling worktrees from landing inside the repo tree Path.GetDirectoryName returned workingDir itself instead of its parent whenever list.WorkingDir ended in a directory separator, placing .claudedo-worktrees inside the target repo's own working tree. Add WorktreeRootResolver to normalize the trailing separator before deriving the sibling root and to guard that the resulting worktree path never lands inside workingDir, used by both WorktreeManager and PlanningSessionManager. Add a startup sweep (LegacyWorktreeFolderRecovery) that warns when a list's working dir already contains a leftover .claudedo-worktrees folder from before the fix. --- .../Git/WorktreeRootResolver.cs | 36 ++++++++ .../Lifecycle/LegacyWorktreeFolderRecovery.cs | 54 ++++++++++++ .../Planning/PlanningSessionManager.cs | 7 +- src/ClaudeDo.Worker/Program.cs | 1 + src/ClaudeDo.Worker/Runner/WorktreeManager.cs | 4 +- .../Git/WorktreeRootResolverTests.cs | 83 +++++++++++++++++++ .../LegacyWorktreeFolderRecoveryTests.cs | 71 ++++++++++++++++ .../Planning/PlanningSessionManagerTests.cs | 24 ++++++ .../Runner/WorktreeManagerTests.cs | 20 +++++ 9 files changed, 297 insertions(+), 3 deletions(-) create mode 100644 src/ClaudeDo.Worker/Git/WorktreeRootResolver.cs create mode 100644 src/ClaudeDo.Worker/Lifecycle/LegacyWorktreeFolderRecovery.cs create mode 100644 tests/ClaudeDo.Worker.Tests/Git/WorktreeRootResolverTests.cs create mode 100644 tests/ClaudeDo.Worker.Tests/Lifecycle/LegacyWorktreeFolderRecoveryTests.cs diff --git a/src/ClaudeDo.Worker/Git/WorktreeRootResolver.cs b/src/ClaudeDo.Worker/Git/WorktreeRootResolver.cs new file mode 100644 index 00000000..9ff3f6c2 --- /dev/null +++ b/src/ClaudeDo.Worker/Git/WorktreeRootResolver.cs @@ -0,0 +1,36 @@ +namespace ClaudeDo.Worker.Git; + +/// +/// Resolves the "sibling" worktree root and guards against a worktree landing inside the +/// repository it was created from. A list.WorkingDir ending in a directory separator +/// makes return the same directory instead of its +/// parent, which used to place .claudedo-worktrees inside the target repo's own tree. +/// +public static class WorktreeRootResolver +{ + /// The parent directory of , trailing-separator safe. + public static string ResolveSiblingRoot(string workingDir) + { + var full = Path.TrimEndingDirectorySeparator(Path.GetFullPath(workingDir)); + var parent = Path.GetDirectoryName(full); + if (string.IsNullOrEmpty(parent)) + throw new InvalidOperationException( + $"Cannot place a sibling worktree next to '{workingDir}': it has no parent directory."); + return parent; + } + + /// Throws if is or lies beneath it. + public static void EnsureOutsideWorkingDir(string worktreePath, string workingDir) + { + var comparison = OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + var worktreeFull = Path.GetFullPath(worktreePath); + var workingDirFull = Path.TrimEndingDirectorySeparator(Path.GetFullPath(workingDir)); + + var isInside = worktreeFull.Equals(workingDirFull, comparison) + || worktreeFull.StartsWith(workingDirFull + Path.DirectorySeparatorChar, comparison); + if (isInside) + throw new InvalidOperationException( + $"Refusing to create worktree '{worktreeFull}' inside the target repository's working " + + $"tree '{workingDirFull}'. Check list.WorkingDir for a trailing separator or a misconfigured worktree root."); + } +} diff --git a/src/ClaudeDo.Worker/Lifecycle/LegacyWorktreeFolderRecovery.cs b/src/ClaudeDo.Worker/Lifecycle/LegacyWorktreeFolderRecovery.cs new file mode 100644 index 00000000..6b685f3f --- /dev/null +++ b/src/ClaudeDo.Worker/Lifecycle/LegacyWorktreeFolderRecovery.cs @@ -0,0 +1,54 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using Microsoft.EntityFrameworkCore; + +namespace ClaudeDo.Worker.Lifecycle; + +/// +/// Startup-only sweep: warns when a list's working directory already contains a +/// .claudedo-worktrees folder inside its own tree — the symptom of the sibling-worktree +/// path bug (see ) from before it was +/// fixed. Detection only; moving or ignoring existing folders is left to the user. +/// +public sealed class LegacyWorktreeFolderRecovery : IHostedService +{ + private const string LegacyFolderName = ".claudedo-worktrees"; + + private readonly IDbContextFactory _dbFactory; + private readonly ILogger _logger; + + public LegacyWorktreeFolderRecovery( + IDbContextFactory dbFactory, + ILogger logger) + { + _dbFactory = dbFactory; + _logger = logger; + } + + public async Task StartAsync(CancellationToken cancellationToken) + { + await using var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken); + var lists = await new ListRepository(ctx).GetAllAsync(cancellationToken); + + var affected = FindAffectedListNames(lists); + + if (affected.Count > 0) + _logger.LogWarning( + "Legacy worktree folder recovery: found '{Folder}' inside the working tree of {Count} list(s): {Lists}. " + + "This was previously caused by a trailing separator on the list's working directory; existing worktrees were not moved.", + LegacyFolderName, affected.Count, string.Join(", ", affected)); + else + _logger.LogInformation("Legacy worktree folder recovery: no lists affected"); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// Names of lists whose working dir already contains a .claudedo-worktrees folder. + public static List FindAffectedListNames(IEnumerable lists) => + lists + .Where(l => !string.IsNullOrWhiteSpace(l.WorkingDir)) + .Where(l => Directory.Exists(Path.Combine(l.WorkingDir!, LegacyFolderName))) + .Select(l => l.Name) + .ToList(); +} diff --git a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs index 9e8358b6..dd51f714 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningSessionManager.cs @@ -5,6 +5,7 @@ using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; using ClaudeDo.Data.Repositories; using ClaudeDo.Worker.Config; +using ClaudeDo.Worker.Git; using ClaudeDo.Worker.State; using Microsoft.EntityFrameworkCore; using TaskStatus = ClaudeDo.Data.Models.TaskStatus; @@ -366,9 +367,11 @@ public sealed class PlanningSessionManager var raw = strategy.Equals("central", StringComparison.OrdinalIgnoreCase) ? Path.Combine(centralRoot, "planning", taskId) - : Path.Combine(Path.GetDirectoryName(listWorkingDir)!, ".claudedo-worktrees", "planning", taskId); + : Path.Combine(WorktreeRootResolver.ResolveSiblingRoot(listWorkingDir), ".claudedo-worktrees", "planning", taskId); - return Path.GetFullPath(raw); + var worktreePath = Path.GetFullPath(raw); + WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, listWorkingDir); + return worktreePath; } private static string TokenFilePathFor(string sessionDir) => diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index 2b8d2166..5dec1a2b 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -69,6 +69,7 @@ builder.Services.AddHostedService(); builder.Services.AddHostedService(); builder.Services.AddHostedService(); builder.Services.AddHostedService(); +builder.Services.AddHostedService(); builder.Services.AddSignalR().AddJsonProtocol(options => { options.PayloadSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter()); diff --git a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs index 1836887b..8fe5481d 100644 --- a/src/ClaudeDo.Worker/Runner/WorktreeManager.cs +++ b/src/ClaudeDo.Worker/Runner/WorktreeManager.cs @@ -3,6 +3,7 @@ using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; using ClaudeDo.Data.Repositories; using ClaudeDo.Worker.Config; +using ClaudeDo.Worker.Git; using Microsoft.EntityFrameworkCore; namespace ClaudeDo.Worker.Runner; @@ -52,9 +53,10 @@ public sealed class WorktreeManager var worktreePath = strategy.Equals("central", StringComparison.OrdinalIgnoreCase) ? Path.Combine(centralRoot ?? _cfg.CentralWorktreeRoot, slug, task.Id) - : Path.Combine(Path.GetDirectoryName(workingDir)!, ".claudedo-worktrees", slug, task.Id); + : Path.Combine(WorktreeRootResolver.ResolveSiblingRoot(workingDir), ".claudedo-worktrees", slug, task.Id); worktreePath = Path.GetFullPath(worktreePath); + WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir); // Ensure parent directory exists. Directory.CreateDirectory(Path.GetDirectoryName(worktreePath)!); diff --git a/tests/ClaudeDo.Worker.Tests/Git/WorktreeRootResolverTests.cs b/tests/ClaudeDo.Worker.Tests/Git/WorktreeRootResolverTests.cs new file mode 100644 index 00000000..209ad34c --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Git/WorktreeRootResolverTests.cs @@ -0,0 +1,83 @@ +using ClaudeDo.Worker.Git; + +namespace ClaudeDo.Worker.Tests.Git; + +public class WorktreeRootResolverTests +{ + [Fact] + public void ResolveSiblingRoot_NoTrailingSeparator_ReturnsParent() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo_no_sep"); + + var root = WorktreeRootResolver.ResolveSiblingRoot(workingDir); + + Assert.Equal(Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar), root); + } + + [Fact] + public void ResolveSiblingRoot_WithTrailingSeparator_StillReturnsParent() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo_with_sep") + Path.DirectorySeparatorChar; + + var root = WorktreeRootResolver.ResolveSiblingRoot(workingDir); + + Assert.Equal(Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar), root); + } + + [Fact] + public void ResolveSiblingRoot_WithTrailingAltSeparator_StillReturnsParent() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo_with_alt_sep") + "/"; + + var root = WorktreeRootResolver.ResolveSiblingRoot(workingDir); + + Assert.Equal(Path.GetFullPath(Path.GetTempPath()).TrimEnd(Path.DirectorySeparatorChar), root); + } + + [Fact] + public void ResolveSiblingRoot_DriveRoot_Throws() + { + var driveRoot = Path.GetPathRoot(Path.GetTempPath())!; + + Assert.Throws(() => WorktreeRootResolver.ResolveSiblingRoot(driveRoot)); + } + + [Fact] + public void EnsureOutsideWorkingDir_PathOutside_DoesNotThrow() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo"); + var worktreePath = Path.Combine(Path.GetTempPath(), ".claudedo-worktrees", "slug", "task1"); + + WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir); + } + + [Fact] + public void EnsureOutsideWorkingDir_PathInsideWorkingDir_Throws() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo"); + var worktreePath = Path.Combine(workingDir, ".claudedo-worktrees", "slug", "task1"); + + Assert.Throws( + () => WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir)); + } + + [Fact] + public void EnsureOutsideWorkingDir_PathEqualsWorkingDir_Throws() + { + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo"); + + Assert.Throws( + () => WorktreeRootResolver.EnsureOutsideWorkingDir(workingDir, workingDir)); + } + + [Fact] + public void EnsureOutsideWorkingDir_SiblingWithSharedPrefix_DoesNotThrow() + { + // "cd_repo-other" is NOT under "cd_repo" even though it shares a string prefix; + // a naive StartsWith(workingDir) check would incorrectly reject this. + var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo"); + var worktreePath = Path.Combine(Path.GetTempPath(), "cd_repo-other", "task1"); + + WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Lifecycle/LegacyWorktreeFolderRecoveryTests.cs b/tests/ClaudeDo.Worker.Tests/Lifecycle/LegacyWorktreeFolderRecoveryTests.cs new file mode 100644 index 00000000..1248dd1c --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Lifecycle/LegacyWorktreeFolderRecoveryTests.cs @@ -0,0 +1,71 @@ +using ClaudeDo.Data.Models; +using ClaudeDo.Worker.Lifecycle; +using ClaudeDo.Worker.Tests.Infrastructure; +using Microsoft.Extensions.Logging.Abstractions; + +namespace ClaudeDo.Worker.Tests.Lifecycle; + +public sealed class LegacyWorktreeFolderRecoveryTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly List _tempDirs = new(); + + public void Dispose() + { + _db.Dispose(); + foreach (var d in _tempDirs) + { + try { Directory.Delete(d, recursive: true); } catch { } + } + } + + private string MakeWorkingDir() + { + var dir = Path.Combine(Path.GetTempPath(), $"claudedo_legacy_wt_{Guid.NewGuid():N}"); + Directory.CreateDirectory(dir); + _tempDirs.Add(dir); + return dir; + } + + [Fact] + public void FindAffectedListNames_FolderInsideWorkingDir_ReturnsListName() + { + var wd = MakeWorkingDir(); + Directory.CreateDirectory(Path.Combine(wd, ".claudedo-worktrees")); + var list = new ListEntity { Id = "1", Name = "Bandel.Hub", WorkingDir = wd, CreatedAt = DateTime.UtcNow }; + + var affected = LegacyWorktreeFolderRecovery.FindAffectedListNames(new[] { list }); + + Assert.Equal(new[] { "Bandel.Hub" }, affected); + } + + [Fact] + public void FindAffectedListNames_NoLegacyFolder_ReturnsEmpty() + { + var wd = MakeWorkingDir(); + var list = new ListEntity { Id = "1", Name = "Clean", WorkingDir = wd, CreatedAt = DateTime.UtcNow }; + + var affected = LegacyWorktreeFolderRecovery.FindAffectedListNames(new[] { list }); + + Assert.Empty(affected); + } + + [Fact] + public void FindAffectedListNames_NullWorkingDir_IsSkipped() + { + var list = new ListEntity { Id = "1", Name = "NoRepo", WorkingDir = null, CreatedAt = DateTime.UtcNow }; + + var affected = LegacyWorktreeFolderRecovery.FindAffectedListNames(new[] { list }); + + Assert.Empty(affected); + } + + [Fact] + public async Task StartAsync_NoLists_DoesNotThrow() + { + var sut = new LegacyWorktreeFolderRecovery(_db.CreateFactory(), NullLogger.Instance); + + await sut.StartAsync(CancellationToken.None); + await sut.StopAsync(CancellationToken.None); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs index 1e273113..4b96e8e6 100644 --- a/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Planning/PlanningSessionManagerTests.cs @@ -123,6 +123,30 @@ public sealed class PlanningSessionManagerTests : IDisposable Assert.NotNull(loaded.PlanningSessionToken); } + [Fact] + public async Task StartAsync_TrailingSeparatorOnWorkingDir_PlacesWorktreeOutsideRepo() + { + var listId = Guid.NewGuid().ToString(); + var wd = Path.Combine(Path.GetTempPath(), $"cd_wd_{Guid.NewGuid():N}"); + GitRepoFixture.InitRepoWithInitialCommit(wd); + await _lists.AddAsync(new ListEntity + { + Id = listId, + Name = "Test", + WorkingDir = wd + Path.DirectorySeparatorChar, + CreatedAt = DateTime.UtcNow, + }); + var parent = await SeedManualTaskAsync(listId); + + var ctx = await _sut.StartAsync(parent.Id, CancellationToken.None); + + var wdFull = Path.GetFullPath(wd); + Assert.False( + ctx.WorktreePath.StartsWith(wdFull + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase), + $"worktree path {ctx.WorktreePath} should not be nested inside the repo dir {wdFull}"); + Assert.True(Directory.Exists(ctx.WorktreePath)); + } + [Fact] public async Task StartAsync_TaskNotManual_Throws() { diff --git a/tests/ClaudeDo.Worker.Tests/Runner/WorktreeManagerTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/WorktreeManagerTests.cs index ff72d81c..fac749d0 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/WorktreeManagerTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/WorktreeManagerTests.cs @@ -189,6 +189,26 @@ public class WorktreeManagerTests : IDisposable Assert.DoesNotContain(task.Id, worktreeList); } + [Fact] + public async Task CreateAsync_TrailingSeparatorOnWorkingDir_PlacesWorktreeOutsideRepo() + { + if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; } + + var repo = CreateRepo(); + var workingDirWithSeparator = repo.RepoDir + Path.DirectorySeparatorChar; + var (task, list) = MakeEntities(workingDirWithSeparator); + var (mgr, db) = await CreateManagerAsync(task, list); + + var ctx = await mgr.CreateAsync(task, list, CancellationToken.None); + _worktreeCleanups.Add((repo.RepoDir, ctx.WorktreePath)); + + var repoDirFull = Path.GetFullPath(repo.RepoDir); + Assert.False( + ctx.WorktreePath.StartsWith(repoDirFull + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase), + $"worktree path {ctx.WorktreePath} should not be nested inside the repo dir {repoDirFull}"); + Assert.True(Directory.Exists(ctx.WorktreePath)); + } + private static (TaskEntity task, ListEntity list) MakeEntities(string workingDir) { var listId = Guid.NewGuid().ToString();