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();