chore(claude-do): Worktrees sollten nach dem Task benannt werden

Durch die GUID namen kann ein User die Worktrees keinem Task zuordnen, und in Rider Müllen mir die Worktrees die BRanches voll. Also sollten wir einfach den Task titel oder Nummer stattdessen nehmen

ClaudeDo-Task: 63f08d944d3341c081267158ef4bbb3e
This commit is contained in:
Mika Kuns
2026-09-30 10:49:07 +02:00
parent 0d029717f8
commit 44ec89b0e6
4 changed files with 68 additions and 18 deletions
+2 -2
View File
@@ -25,8 +25,8 @@ is wired through `TaskIdResolver`, which resolves:
This is **not** ambiguous: a GUID is never all-digits, so a pure-integer parameter is always a This is **not** ambiguous: a GUID is never all-digits, so a pure-integer parameter is always a
number, not a partial GUID. All task-returning tools (`GetTask`, `ListTasks`, `AddTask`, number, not a partial GUID. All task-returning tools (`GetTask`, `ListTasks`, `AddTask`,
`BatchGetTasks`, etc.) stamp the resolved task's `Number` in the DTO — both `TaskDto` and the `BatchGetTasks`, etc.) stamp the resolved task's `Number` in the DTO — both `TaskDto` and the
lean `TaskRefDto` carry an `int Number` field. **Branch names and worktree paths** (`claudedo/{id}`) lean `TaskRefDto` carry an `int Number` field. **Task worktree branches and folders** are `claudedo/{number}-{title-slug}`;
continue to use the GUID; the number is a display alias, never the identity. otherwise the number is a display alias, never the identity.
Every tool description carries a shared boilerplate clause (defined in `McpToolDocs.TaskNumberHint`) Every tool description carries a shared boilerplate clause (defined in `McpToolDocs.TaskNumberHint`)
instructing the agent to **refer to tasks as `#<number>` when reporting results to the user** — instructing the agent to **refer to tasks as `#<number>` when reporting results to the user** —
+3 -3
View File
@@ -55,7 +55,7 @@ subfolder within their area; the namespace stays the area namespace.
- **RunCancellationRegistry** — taskId → running-run CTS. Lets `TaskStateService.CancelAsync` kill a cancelled task's process without a DI cycle. - **RunCancellationRegistry** — taskId → running-run CTS. Lets `TaskStateService.CancelAsync` kill a cancelled task's process without a DI cycle.
- **OverrideSlotService** — owns `RunNow` / `ContinueTask`; goes through `TaskStateService.StartRunningAsync` (caller-driven, serialized by slot lock). - **OverrideSlotService** — owns `RunNow` / `ContinueTask`; goes through `TaskStateService.StartRunningAsync` (caller-driven, serialized by slot lock).
- **StaleTaskRecovery** — startup-only; calls `TaskStateService.RecoverStaleRunningAsync` to flip orphaned `Running` rows to `Failed`. - **StaleTaskRecovery** — startup-only; calls `TaskStateService.RecoverStaleRunningAsync` to flip orphaned `Running` rows to `Failed`.
- **External/*** — always-on MCP tools for general Claude sessions, scoped to *starting* and *observing* sessions (no multi-turn, planning internals, or app-settings writes). Auth via optional `X-ClaudeDo-Key`. **Task-ID resolution:** every tool parameter that accepts a task id (`taskId`, `parentId`, `taskIds`, etc.) is wired through `TaskIdResolver`, which accepts both `#123` (short number-based handle) and bare GUIDs. A bare all-digits string is never a GUID and is unambiguous. Branch names and worktree paths continue to use the GUID (`claudedo/{id}`) — the `Number` is a display alias, never identity. Every task-returning tool stamps the task's `Number` in its DTO payload and the MCP tool descriptions carry a shared clause instructing the agent to **refer to tasks as `#<number>` when reporting to the user** (otherwise the number sits unused in the payload). **Hard convention (test-enforced):** every optional/filter parameter needs a C# default value — the MCP schema only marks a parameter optional when one exists; `ExternalMcpToolSchemaTests` guards this. **Other conventions (not test-enforced):** no tool returns bare `Task`/a nullable payload directly (write tools return a confirmation record like `RunTaskNowResult`; read tools use an explicit `Found`/`Available` flag); tool descriptions follow rules documented in `External/McpToolDocs.cs`. Full tool inventory + per-tool behaviour → [external-mcp](../../docs/explore-notes/external-mcp.md). - **External/*** — always-on MCP tools for general Claude sessions, scoped to *starting* and *observing* sessions (no multi-turn, planning internals, or app-settings writes). Auth via optional `X-ClaudeDo-Key`. **Task-ID resolution:** every tool parameter that accepts a task id (`taskId`, `parentId`, `taskIds`, etc.) is wired through `TaskIdResolver`, which accepts both `#123` (short number-based handle) and bare GUIDs. A bare all-digits string is never a GUID and is unambiguous. Task worktree branches and folders are named `{number}-{title-slug}` (`claudedo/{number}-{title-slug}`; stored in the `worktrees` row, never re-derived) — the `Number` is otherwise a display alias, never identity. Every task-returning tool stamps the task's `Number` in its DTO payload and the MCP tool descriptions carry a shared clause instructing the agent to **refer to tasks as `#<number>` when reporting to the user** (otherwise the number sits unused in the payload). **Hard convention (test-enforced):** every optional/filter parameter needs a C# default value — the MCP schema only marks a parameter optional when one exists; `ExternalMcpToolSchemaTests` guards this. **Other conventions (not test-enforced):** no tool returns bare `Task`/a nullable payload directly (write tools return a confirmation record like `RunTaskNowResult`; read tools use an explicit `Found`/`Available` flag); tool descriptions follow rules documented in `External/McpToolDocs.cs`. Full tool inventory + per-tool behaviour → [external-mcp](../../docs/explore-notes/external-mcp.md).
## Status Model ## Status Model
@@ -130,7 +130,7 @@ Full flow, invariants, and model/effort/max-turns resolution (including the low-
- **ClaudeProcess** — spawns `claude -p --output-format stream-json --verbose --permission-mode auto` (or whatever app settings specify). Prompt via stdin, NDJSON from stdout. CancellationToken kills the process tree. Resolves `_cfg.ClaudeBin` via `ExecutableResolver` (`ClaudeDo.Data`, shared with `ClaudeCliPreflight` and the Installer's checks) before spawning — a `.cmd`/`.bat` shim (e.g. an npm-installed `claude`) is launched through `cmd.exe /c` since `UseShellExecute = false` can't exec a shim directly; a resolved `.exe` starts exactly as before. Throws if nothing resolves. *(Not yet on `main` — see `Environment Checks` in `ClaudeDo.Installer/CLAUDE.md`.)* - **ClaudeProcess** — spawns `claude -p --output-format stream-json --verbose --permission-mode auto` (or whatever app settings specify). Prompt via stdin, NDJSON from stdout. CancellationToken kills the process tree. Resolves `_cfg.ClaudeBin` via `ExecutableResolver` (`ClaudeDo.Data`, shared with `ClaudeCliPreflight` and the Installer's checks) before spawning — a `.cmd`/`.bat` shim (e.g. an npm-installed `claude`) is launched through `cmd.exe /c` since `UseShellExecute = false` can't exec a shim directly; a resolved `.exe` starts exactly as before. Throws if nothing resolves. *(Not yet on `main` — see `Environment Checks` in `ClaudeDo.Installer/CLAUDE.md`.)*
- **ClaudeArgsBuilder** — `--model`, `--effort`, `--max-turns`, `--append-system-prompt`, `--agents`, `--json-schema`, `--resume` - **ClaudeArgsBuilder** — `--model`, `--effort`, `--max-turns`, `--append-system-prompt`, `--agents`, `--json-schema`, `--resume`
- **StreamAnalyzer** — parses NDJSON; extracts session_id, token counts, turn counts, result text, structured output. Replaced MessageParser. - **StreamAnalyzer** — parses NDJSON; extracts session_id, token counts, turn counts, result text, structured output. Replaced MessageParser.
- **WorktreeManager** — worktrees on `claudedo/{id}` branches (full task GUID, dashes stripped — a prefix isn't enough to rule out collisions); commits with semantic messages, updates DB with head commit + diff stats - **WorktreeManager** — worktrees on `claudedo/{number}-{title-slug}` branches, same leaf as the folder (`NameFor`; the unique task number rules out collisions, slug capped at 40 chars); commits with semantic messages, updates DB with head commit + diff stats
- **CommitMessageBuilder** — `{commitType}(slug): title\n\ndescription\n\nClaudeDo-Task: taskId`; `BuildMerge` is the merge-commit variant (`{commitType}(slug): merge title` + trailer). **Every merge caller passes a blank commit message on purpose** — `TaskMergeService` fills in `BuildMerge` from the task's commit type and its list's name, which is the only place that knows both. Don't reintroduce a caller-side literal. - **CommitMessageBuilder** — `{commitType}(slug): title\n\ndescription\n\nClaudeDo-Task: taskId`; `BuildMerge` is the merge-commit variant (`{commitType}(slug): merge title` + trailer). **Every merge caller passes a blank commit message on purpose** — `TaskMergeService` fills in `BuildMerge` from the task's commit type and its list's name, which is the only place that knows both. Don't reintroduce a caller-side literal.
- **TaskResetService** — discards a failed task's worktree, resets the row to Idle, preserves run history - **TaskResetService** — discards a failed task's worktree, resets the row to Idle, preserves run history
- **AgentFileService** — manages `~/.claudeDo/agents/*.md`; list/refresh via SignalR - **AgentFileService** — manages `~/.claudeDo/agents/*.md`; list/refresh via SignalR
@@ -309,6 +309,6 @@ distinguish the two singletons). The token never touches `worker.config.json` or
- The worker runs standalone — start it separately from the UI. Loopback only (127.0.0.1). - The worker runs standalone — start it separately from the UI. Loopback only (127.0.0.1).
- `--permission-mode auto` by default; legacy `bypassPermissions` settings map to `auto` at dispatch time. `acceptEdits`, `plan`, `default` pass through unchanged. - `--permission-mode auto` by default; legacy `bypassPermissions` settings map to `auto` at dispatch time. `acceptEdits`, `plan`, `default` pass through unchanged.
- Worktree branches follow `claudedo/{id}`. - Task worktree branches follow `claudedo/{number}-{title-slug}`; planning worktrees stay `claudedo/planning/{guid}` (their path is re-derived from the id on resume).
- **Both MCP servers run `Stateless = true`** (`Program.cs`). No `Mcp-Session-Id`, so a worker restart can't 404 a session that outlives it, and a 2026-07-28 client isn't refused back to the initialize handshake. The trade: **no** sampling, elicitation, resource subscriptions, unsolicited notifications, or legacy SSE endpoint — don't reach for those. In-tool progress notifications ride the POST's own response stream and work in both modes. All three consumers (`TaskRunner`, `PlanningSessionManager`, the installer's `RegisterMcpStep`) already use `type: "http"`. - **Both MCP servers run `Stateless = true`** (`Program.cs`). No `Mcp-Session-Id`, so a worker restart can't 404 a session that outlives it, and a 2026-07-28 client isn't refused back to the initialize handshake. The trade: **no** sampling, elicitation, resource subscriptions, unsolicited notifications, or legacy SSE endpoint — don't reach for those. In-tool progress notifications ride the POST's own response stream and work in both modes. All three consumers (`TaskRunner`, `PlanningSessionManager`, the installer's `RegisterMcpStep`) already use `type: "http"`.
- **An MCP tool that can run longer than ~5s reports progress.** Staying silent lets the MCP client abort after 300s idle while the worker keeps working — the caller sees an abort even though the operation is still running. `Lifecycle/ProgressReporter` is the one implementation (elapsed-time reporting via `RunAsync`, per-item `i/n` via `ReportItem`) — thread an `IProgress<ProgressNotificationValue>? progress = null` parameter through instead of writing another polling loop. - **An MCP tool that can run longer than ~5s reports progress.** Staying silent lets the MCP client abort after 300s idle while the worker keeps working — the caller sees an abort even though the operation is still running. `Lifecycle/ProgressReporter` is the one implementation (elapsed-time reporting via `RunAsync`, per-item `i/n` via `ReportItem`) — thread an `IProgress<ProgressNotificationValue>? progress = null` parameter through instead of writing another polling loop.
+23 -8
View File
@@ -38,6 +38,14 @@ public sealed class WorktreeManager
private Task BroadcastCreating(string taskId) => private Task BroadcastCreating(string taskId) =>
_broadcaster?.OperationProgress(taskId, PhaseCreatingWorktree, 0, 0) ?? Task.CompletedTask; _broadcaster?.OperationProgress(taskId, PhaseCreatingWorktree, 0, 0) ?? Task.CompletedTask;
/// <summary>Branch leaf and folder name for a task worktree: <c>{number}-{title-slug}</c>, unique via the task number.</summary>
public static string NameFor(TaskEntity task)
{
var titleSlug = CommitMessageBuilder.ToSlug(task.Title);
if (titleSlug.Length > 40) titleSlug = titleSlug[..40].TrimEnd('-');
return titleSlug.Length == 0 ? task.Number.ToString() : $"{task.Number}-{titleSlug}";
}
public async Task<WorktreeContext> CreateAsync(TaskEntity task, ListEntity list, CancellationToken ct) public async Task<WorktreeContext> CreateAsync(TaskEntity task, ListEntity list, CancellationToken ct)
{ {
var workingDir = list.WorkingDir var workingDir = list.WorkingDir
@@ -51,10 +59,7 @@ public sealed class WorktreeManager
_logger.LogInformation("Repository at {Dir} had no commits; created an empty bootstrap commit", workingDir); _logger.LogInformation("Repository at {Dir} had no commits; created an empty bootstrap commit", workingDir);
var baseCommit = await ResolveBaseCommitAsync(task, workingDir, ct); var baseCommit = await ResolveBaseCommitAsync(task, workingDir, ct);
// Use the full task id (dashes stripped) in the branch name so var name = NameFor(task);
// two GUIDs sharing an 8-char prefix cannot collide on the same branch.
var idForBranch = task.Id.Replace("-", "");
var branchName = $"claudedo/{idForBranch}";
var slug = CommitMessageBuilder.ToSlug(list.Name); var slug = CommitMessageBuilder.ToSlug(list.Name);
string strategy; string strategy;
@@ -68,11 +73,21 @@ public sealed class WorktreeManager
: _cfg.CentralWorktreeRoot; : _cfg.CentralWorktreeRoot;
} }
var worktreePath = strategy.Equals("central", StringComparison.OrdinalIgnoreCase) var parentDir = strategy.Equals("central", StringComparison.OrdinalIgnoreCase)
? Path.Combine(centralRoot ?? _cfg.CentralWorktreeRoot, slug, task.Id) ? Path.Combine(centralRoot ?? _cfg.CentralWorktreeRoot, slug)
: Path.Combine(WorktreeRootResolver.ResolveSiblingRoot(workingDir), ".claudedo-worktrees", slug, task.Id); : Path.Combine(WorktreeRootResolver.ResolveSiblingRoot(workingDir), ".claudedo-worktrees", slug);
worktreePath = Path.GetFullPath(worktreePath); // Numbers restart after a DB reset: a taken name may be another task's unmerged branch,
// which the self-heal below would force-delete — the id suffix keeps it untouched.
if (Directory.Exists(Path.Combine(parentDir, name))
|| (await _git.ListLocalBranchesAsync(workingDir, ct)).Contains($"claudedo/{name}"))
{
var id = task.Id.Replace("-", "");
name = $"{name}-{id[..Math.Min(8, id.Length)]}";
}
var branchName = $"claudedo/{name}";
var worktreePath = Path.GetFullPath(Path.Combine(parentDir, name));
WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir); WorktreeRootResolver.EnsureOutsideWorkingDir(worktreePath, workingDir);
// Ensure parent directory exists. // Ensure parent directory exists.
@@ -65,7 +65,9 @@ public class WorktreeManagerTests : IDisposable
Assert.NotNull(ctx); Assert.NotNull(ctx);
Assert.True(Directory.Exists(ctx.WorktreePath)); Assert.True(Directory.Exists(ctx.WorktreePath));
Assert.Equal($"claudedo/{task.Id.Replace("-", "")}", ctx.BranchName); // StartsWith: a folder leaked into the shared %TEMP% root by another test forces the id suffix.
Assert.StartsWith($"claudedo/{task.Number}-test-task", ctx.BranchName);
Assert.StartsWith($"{task.Number}-test-task", Path.GetFileName(ctx.WorktreePath));
Assert.Equal(repo.BaseCommit, ctx.BaseCommit); Assert.Equal(repo.BaseCommit, ctx.BaseCommit);
using var readCtx = db.CreateContext(); using var readCtx = db.CreateContext();
@@ -77,6 +79,15 @@ public class WorktreeManagerTests : IDisposable
Assert.Null(row.HeadCommit); Assert.Null(row.HeadCommit);
} }
[Theory]
[InlineData("Fix: Löschen des Tasks!", "7-fix-lschen-des-tasks")]
[InlineData("äöü", "7")]
[InlineData("a very long title that goes well beyond the forty char cap", "7-a-very-long-title-that-goes-well-beyond")]
public void NameFor_NumberPlusCappedTitleSlug(string title, string expected)
{
Assert.Equal(expected, WorktreeManager.NameFor(new TaskEntity { Id = "x", ListId = "y", Title = title, Number = 7, CreatedAt = DateTime.UtcNow }));
}
[Fact] [Fact]
public async Task CreateAsync_BroadcastsCreatingWorktreePhase() public async Task CreateAsync_BroadcastsCreatingWorktreePhase()
{ {
@@ -107,6 +118,28 @@ public class WorktreeManagerTests : IDisposable
Assert.Equal(WorktreeManager.PhaseCreatingWorktree, (string?)progressCalls[0].Args[1]); Assert.Equal(WorktreeManager.PhaseCreatingWorktree, (string?)progressCalls[0].Args[1]);
} }
[Fact]
public async Task CreateAsync_NameTakenByForeignBranch_FallsBackToIdSuffixAndLeavesBranchIntact()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var repo = CreateRepo();
var (task, list) = MakeEntities(repo.RepoDir);
var (mgr, _) = await CreateManagerAsync(task, list);
// A leftover from an earlier DB whose task happened to get the same number and title.
var foreign = $"claudedo/{WorktreeManager.NameFor(task)}";
GitRepoFixture.RunGit(repo.RepoDir, "branch", foreign, repo.BaseCommit);
var ctx = await mgr.CreateAsync(task, list, CancellationToken.None);
_worktreeCleanups.Add((repo.RepoDir, ctx.WorktreePath));
var suffixed = $"{WorktreeManager.NameFor(task)}-{task.Id.Replace("-", "")[..8]}";
Assert.Equal($"claudedo/{suffixed}", ctx.BranchName);
Assert.Equal(suffixed, Path.GetFileName(ctx.WorktreePath));
Assert.False(string.IsNullOrWhiteSpace(GitRepoFixture.RunGit(repo.RepoDir, "branch", "--list", foreign)));
}
[Fact] [Fact]
public async Task CreateAsync_SelfHealPath_BroadcastsCreatingWorktreePhaseForEachSection() public async Task CreateAsync_SelfHealPath_BroadcastsCreatingWorktreePhaseForEachSection()
{ {
@@ -122,9 +155,11 @@ public class WorktreeManagerTests : IDisposable
await new TaskRepository(seedCtx).AddAsync(task); await new TaskRepository(seedCtx).AddAsync(task);
} }
// Pre-create the branch so `git worktree add -b` hits the "already exists" self-heal path. // Occupy the plain name (-> id-suffixed fallback) and the fallback too, so
var branchName = $"claudedo/{task.Id.Replace("-", "")}"; // `git worktree add -b` hits the "already exists" self-heal path.
var branchName = $"claudedo/{WorktreeManager.NameFor(task)}";
GitRepoFixture.RunGit(repo.RepoDir, "branch", branchName, repo.BaseCommit); GitRepoFixture.RunGit(repo.RepoDir, "branch", branchName, repo.BaseCommit);
GitRepoFixture.RunGit(repo.RepoDir, "branch", $"{branchName}-{task.Id.Replace("-", "")[..8]}", repo.BaseCommit);
var fakeHub = new CapturingHubContext(); var fakeHub = new CapturingHubContext();
var broadcaster = new HubBroadcaster(fakeHub); var broadcaster = new HubBroadcaster(fakeHub);
@@ -276,13 +311,13 @@ public class WorktreeManagerTests : IDisposable
await Assert.ThrowsAnyAsync<Exception>( await Assert.ThrowsAnyAsync<Exception>(
() => mgr.CreateAsync(task, list, CancellationToken.None)); () => mgr.CreateAsync(task, list, CancellationToken.None));
var branchName = $"claudedo/{task.Id.Replace("-", "")}"; var branchName = $"claudedo/{WorktreeManager.NameFor(task)}";
var branchList = GitRepoFixture.RunGit(repo.RepoDir, "branch", "--list", branchName); var branchList = GitRepoFixture.RunGit(repo.RepoDir, "branch", "--list", branchName);
Assert.True(string.IsNullOrWhiteSpace(branchList), Assert.True(string.IsNullOrWhiteSpace(branchList),
$"orphaned branch {branchName} should be cleaned up, got: {branchList}"); $"orphaned branch {branchName} should be cleaned up, got: {branchList}");
var worktreeList = GitRepoFixture.RunGit(repo.RepoDir, "worktree", "list"); var worktreeList = GitRepoFixture.RunGit(repo.RepoDir, "worktree", "list");
Assert.DoesNotContain(task.Id, worktreeList); Assert.DoesNotContain(WorktreeManager.NameFor(task), worktreeList);
} }
[Fact] [Fact]