Merge claudedo/107e62d083e44436b9074c16d14e2edc
This commit is contained in:
@@ -0,0 +1,36 @@
|
||||
namespace ClaudeDo.Worker.Git;
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the "sibling" worktree root and guards against a worktree landing inside the
|
||||
/// repository it was created from. A <c>list.WorkingDir</c> ending in a directory separator
|
||||
/// makes <see cref="Path.GetDirectoryName(string)"/> return the same directory instead of its
|
||||
/// parent, which used to place <c>.claudedo-worktrees</c> inside the target repo's own tree.
|
||||
/// </summary>
|
||||
public static class WorktreeRootResolver
|
||||
{
|
||||
/// <summary>The parent directory of <paramref name="workingDir"/>, trailing-separator safe.</summary>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>Throws if <paramref name="worktreePath"/> is <paramref name="workingDir"/> or lies beneath it.</summary>
|
||||
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.");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,54 @@
|
||||
using ClaudeDo.Data;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
|
||||
namespace ClaudeDo.Worker.Lifecycle;
|
||||
|
||||
/// <summary>
|
||||
/// Startup-only sweep: warns when a list's working directory already contains a
|
||||
/// <c>.claudedo-worktrees</c> folder inside its own tree — the symptom of the sibling-worktree
|
||||
/// path bug (see <see cref="ClaudeDo.Worker.Git.WorktreeRootResolver"/>) from before it was
|
||||
/// fixed. Detection only; moving or ignoring existing folders is left to the user.
|
||||
/// </summary>
|
||||
public sealed class LegacyWorktreeFolderRecovery : IHostedService
|
||||
{
|
||||
private const string LegacyFolderName = ".claudedo-worktrees";
|
||||
|
||||
private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory;
|
||||
private readonly ILogger<LegacyWorktreeFolderRecovery> _logger;
|
||||
|
||||
public LegacyWorktreeFolderRecovery(
|
||||
IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||
ILogger<LegacyWorktreeFolderRecovery> 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;
|
||||
|
||||
/// <summary>Names of lists whose working dir already contains a <c>.claudedo-worktrees</c> folder.</summary>
|
||||
public static List<string> FindAffectedListNames(IEnumerable<ListEntity> lists) =>
|
||||
lists
|
||||
.Where(l => !string.IsNullOrWhiteSpace(l.WorkingDir))
|
||||
.Where(l => Directory.Exists(Path.Combine(l.WorkingDir!, LegacyFolderName)))
|
||||
.Select(l => l.Name)
|
||||
.ToList();
|
||||
}
|
||||
@@ -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) =>
|
||||
|
||||
@@ -69,6 +69,7 @@ builder.Services.AddHostedService<StaleTaskRecovery>();
|
||||
builder.Services.AddHostedService<OrphanRecovery>();
|
||||
builder.Services.AddHostedService<AttachmentOrphanRecovery>();
|
||||
builder.Services.AddHostedService<PromptFileRecovery>();
|
||||
builder.Services.AddHostedService<LegacyWorktreeFolderRecovery>();
|
||||
builder.Services.AddSignalR().AddJsonProtocol(options =>
|
||||
{
|
||||
options.PayloadSerializerOptions.Converters.Add(new System.Text.Json.Serialization.JsonStringEnumConverter());
|
||||
|
||||
@@ -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)!);
|
||||
|
||||
@@ -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<InvalidOperationException>(() => 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<InvalidOperationException>(
|
||||
() => WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void EnsureOutsideWorkingDir_PathEqualsWorkingDir_Throws()
|
||||
{
|
||||
var workingDir = Path.Combine(Path.GetTempPath(), "cd_repo");
|
||||
|
||||
Assert.Throws<InvalidOperationException>(
|
||||
() => 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);
|
||||
}
|
||||
}
|
||||
@@ -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<string> _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<LegacyWorktreeFolderRecovery>.Instance);
|
||||
|
||||
await sut.StartAsync(CancellationToken.None);
|
||||
await sut.StopAsync(CancellationToken.None);
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
{
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user