diff --git a/src/ClaudeDo.Data/PromptFiles.cs b/src/ClaudeDo.Data/PromptFiles.cs index 4e604252..543d6254 100644 --- a/src/ClaudeDo.Data/PromptFiles.cs +++ b/src/ClaudeDo.Data/PromptFiles.cs @@ -547,6 +547,9 @@ public static class PromptFiles Scope: {scope} Repo: {repo} + Your own handler task id (for handoff_list_handler / submit_task_for_review — do NOT confuse + this with the ids of the tasks you are handling below): {handlerTaskId} + Handle the following tasks. Work Phases 0–5 as your instructions describe, asking me whenever you are unsure. {tasks} @@ -560,6 +563,9 @@ public static class PromptFiles Scope: {scope} Repo: {repo} + Your own handler task id (for handoff_list_handler / submit_task_for_review — do NOT confuse + this with the ids of the surviving tasks below): {handlerTaskId} + A prior session already read, deduped and enhanced this list's tasks. Pick up at phase 3 for the tasks below — their descriptions are already sharpened. diff --git a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs index 1598aac2..45a0db68 100644 --- a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs @@ -130,8 +130,10 @@ public interface IWorkerClient : INotifyPropertyChanged /// no task, no worktree. Task GetAdHocLaunchSpecAsync(string directory, CancellationToken ct = default); /// Launch spec for an embedded ConPTY "merge helper" session that drives the given - /// tasks to a merged/Done state. listId scopes the session (and cwd) to that list. - Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, CancellationToken ct = default); + /// tasks to a merged/Done state. listId scopes the session (and cwd) to that list. + /// handlerTaskId (from CreateMergeHelperTaskAsync) is rendered into the brief so the session + /// can call handoff_list_handler on its own handler task. + Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct = default); /// Creates the ClaudeDo task that owns a list-handler run (one per run, Idle/IsManual, /// never queued) so the ConPTY tile can be task-based instead of ad-hoc. Returns the new task id. Task CreateMergeHelperTaskAsync( diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index b1a20da9..2c5482bf 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -614,8 +614,8 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC public async Task GetAdHocLaunchSpecAsync(string directory, CancellationToken ct = default) => await InvokeTimedAsync("GetAdHocLaunchSpec", () => _hub.InvokeAsync("GetAdHocLaunchSpec", directory, ct)); - public async Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, CancellationToken ct = default) - => await InvokeTimedAsync("GetMergeHelperLaunchSpec", () => _hub.InvokeAsync("GetMergeHelperLaunchSpec", taskIds, listId, ct)); + public async Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct = default) + => await InvokeTimedAsync("GetMergeHelperLaunchSpec", () => _hub.InvokeAsync("GetMergeHelperLaunchSpec", taskIds, listId, handlerTaskId, ct)); public async Task CreateMergeHelperTaskAsync( IReadOnlyList taskIds, string listId, string title, string descriptionHeader, CancellationToken ct = default) diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs index 9621c2d5..a0baa856 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs @@ -279,7 +279,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable } AddConPtyPane(new ConPtyPaneViewModel(taskId, title, - () => DescribeAsync(() => _worker.GetMergeHelperLaunchSpecAsync(taskIds, listId)))); + () => DescribeAsync(() => _worker.GetMergeHelperLaunchSpecAsync(taskIds, listId, taskId)))); } finally { diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 504d792b..96be513c 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -864,12 +864,13 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub // Builds the launch spec for an embedded ConPTY "merge helper" session that drives the given // tasks to a merged/Done state via the mcp__claudedo__* tools. listId scopes the brief label - // and cwd to that list. - public Task GetMergeHelperLaunchSpec(string[] taskIds, string listId) => HubGuard(() => + // and cwd to that list. handlerTaskId is the id returned by CreateMergeHelperTask -- it is + // rendered into the brief so the session can call handoff_list_handler on itself. + public Task GetMergeHelperLaunchSpec(string[] taskIds, string listId, string handlerTaskId) => HubGuard(() => { if (_interactiveLaunchSpec is null) throw new InvalidOperationException("Interactive launch spec service is not configured."); - return _interactiveLaunchSpec.BuildForMergeHelperAsync(taskIds, listId, Context.ConnectionAborted); + return _interactiveLaunchSpec.BuildForMergeHelperAsync(taskIds, listId, handlerTaskId, Context.ConnectionAborted); }); // Creates the ClaudeDo task that owns a list-handler run, before the ConPTY tile opens -- diff --git a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs index c93d09b2..2929966b 100644 --- a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs @@ -241,7 +241,7 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService private const string MergeHelperAllowedTools = "mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task"; - public async Task BuildForMergeHelperAsync(IReadOnlyList taskIds, string listId, CancellationToken ct) + public async Task BuildForMergeHelperAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct) { if (taskIds.Count == 0) throw new InvalidOperationException("No tasks selected for the list handler."); @@ -278,6 +278,7 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService { ["scope"] = $"List: {list.Name}", ["repo"] = repoDir, + ["handlerTaskId"] = handlerTaskId, ["tasks"] = string.Join("\n", briefLines), }), ct); @@ -371,6 +372,7 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService { ["scope"] = $"List: {list.Name}", ["repo"] = repoDir, + ["handlerTaskId"] = taskId, ["tasks"] = string.Join("\n", briefLines), ["finalNote"] = finalNote, }), ct); diff --git a/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs index 4c21cce3..b1aa56b9 100644 --- a/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs @@ -34,9 +34,11 @@ public interface IInteractiveLaunchSpecService /// given tasks to a merged/Done state via the mcp__claudedo__* tools. Writes a per-session /// system prompt + task brief under ~/.todo-app/merge-helper-sessions/<guid> and exposes /// that dir plus the list's repo dir via --add-dir. cwd is the list's working directory. + /// handlerTaskId (the id returned by CreateMergeHelperTaskAsync) is rendered into the brief so + /// the session can call handoff_list_handler/submit_task_for_review on its own handler task. /// Throws KeyNotFoundException if the list doesn't exist; InvalidOperationException if /// taskIds is empty or the list has no existing working directory. - Task BuildForMergeHelperAsync(IReadOnlyList taskIds, string listId, CancellationToken ct); + Task BuildForMergeHelperAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct); /// Creates the ClaudeDo task that hosts a list-handler run (Mission Control's /// "Let Claude handle it") and stamps the list repo's current HEAD as the review range's diff --git a/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs b/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs index 83553226..c295892f 100644 --- a/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs +++ b/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs @@ -360,6 +360,13 @@ public class PromptFilesTests Assert.Contains("{tasks}", d); } + [Fact] + public void DefaultFor_merge_helper_initial_has_handler_task_id_token() + { + var d = PromptFiles.DefaultFor(PromptKind.MergeHelperInitial); + Assert.Contains("{handlerTaskId}", d); + } + [Fact] public void RenderTemplate_merge_helper_initial_substitutes_scope_and_tasks() { @@ -382,6 +389,13 @@ public class PromptFilesTests Assert.Contains("{tasks}", d); } + [Fact] + public void DefaultFor_merge_helper_handoff_has_handler_task_id_token() + { + var d = PromptFiles.DefaultFor(PromptKind.MergeHelperHandoff); + Assert.Contains("{handlerTaskId}", d); + } + [Fact] public void DefaultFor_merge_helper_handoff_points_at_phase_3() { diff --git a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs index 148ddd29..19ef5b77 100644 --- a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs +++ b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs @@ -118,7 +118,7 @@ public abstract class StubWorkerClient : IWorkerClient => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); public virtual Task GetAdHocLaunchSpecAsync(string directory, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(directory, "claude", Array.Empty(), new Dictionary())); - public virtual Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, CancellationToken ct = default) + public virtual Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); public virtual Task CreateMergeHelperTaskAsync( IReadOnlyList taskIds, string listId, string title, string descriptionHeader, CancellationToken ct = default) diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs index 16c54eb2..17f16365 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs @@ -502,7 +502,7 @@ public class MissionControlViewModelTests : IDisposable private sealed class ThrowingMergeHelperLaunchSpecWorker : StubWorkerClient { - public override Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, CancellationToken ct = default) + public override Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct = default) => throw new InvalidOperationException("spec failed"); } diff --git a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs index eae10ea2..cf433340 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs @@ -628,7 +628,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var listId = await SeedListAsync(workingDir: _tempDir); var svc = BuildService(); await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperAsync(Array.Empty(), listId, CancellationToken.None)); + () => svc.BuildForMergeHelperAsync(Array.Empty(), listId, Guid.NewGuid().ToString(), CancellationToken.None)); } [Fact] @@ -640,7 +640,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); var ex = await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperAsync(new[] { taskId }, listId, CancellationToken.None)); + () => svc.BuildForMergeHelperAsync(new[] { taskId }, listId, Guid.NewGuid().ToString(), CancellationToken.None)); Assert.Contains("working directory", ex.Message); } @@ -653,7 +653,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperAsync(new[] { taskId }, "no-such-list", CancellationToken.None)); + () => svc.BuildForMergeHelperAsync(new[] { taskId }, "no-such-list", Guid.NewGuid().ToString(), CancellationToken.None)); } [Fact] @@ -669,7 +669,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(t2, listId, TaskStatus.Idle, title: "Second task"); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); Assert.Equal(repo, spec.Cwd); @@ -726,7 +726,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(t1, listId, TaskStatus.WaitingForReview, title: "First task"); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); var args = spec.Args.ToList(); @@ -749,7 +749,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(t2, listId, TaskStatus.Idle, title: "Second task"); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); var brief = File.ReadAllText(Path.Combine(sessionDir, "brief.md")); @@ -761,6 +761,28 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable Assert.Contains(t2, brief); } + // Regression guard: no session ever learned its own handler task id, so it could never call + // handoff_list_handler(taskId, ...) on itself -- the handoff chain broke exactly where it was + // needed. The brief must carry it. + [Fact] + public async Task BuildForMergeHelperAsync_BriefIncludesHandlerTaskId() + { + var repo = Path.Combine(_tempDir, "repoHandlerId"); + Directory.CreateDirectory(repo); + + var listId = await SeedListAsync(workingDir: repo, name: "Alpha"); + var t1 = Guid.NewGuid().ToString(); + await SeedTaskAsync(t1, listId, TaskStatus.Idle, title: "First task"); + var handlerTaskId = Guid.NewGuid().ToString(); + + var svc = BuildService(); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, handlerTaskId, CancellationToken.None); + var sessionDir = TrackSessionDir(spec); + + var brief = File.ReadAllText(Path.Combine(sessionDir, "brief.md")); + Assert.Contains(handlerTaskId, brief); + } + [Fact] public async Task BuildForMergeHelperAsync_BriefIncludesTaskDescription() { @@ -773,7 +795,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable description: "Do the thing carefully and report back."); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); var brief = File.ReadAllText(Path.Combine(sessionDir, "brief.md")); @@ -791,7 +813,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(t1, listId, TaskStatus.Idle, title: "No description task", description: null); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); var brief = File.ReadAllText(Path.Combine(sessionDir, "brief.md")); @@ -822,7 +844,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(t2, listId, TaskStatus.WaitingForReview, title: "Second task", description: "plain description"); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, CancellationToken.None); + var spec = await svc.BuildForMergeHelperAsync(new[] { t1, t2 }, listId, Guid.NewGuid().ToString(), CancellationToken.None); var sessionDir = TrackSessionDir(spec); var brief = File.ReadAllText(Path.Combine(sessionDir, "brief.md")); @@ -967,6 +989,26 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable Assert.DoesNotContain(args, a => a.EndsWith('\\') || a.EndsWith('/')); } + // Regression guard: the handoff session must see the SAME handler task id as the run that + // called handoff_list_handler -- it's the id already passed in as taskId, just never rendered + // into the handoff kickoff. + [Fact] + public async Task BuildForMergeHelperHandoffAsync_HandoffIncludesSameHandlerTaskId() + { + var listId = await SeedListAsync(workingDir: _tempDir); + var handlerTaskId = Guid.NewGuid().ToString(); + await SeedTaskAsync(handlerTaskId, listId, TaskStatus.Idle, title: "Handler"); + var survivor = Guid.NewGuid().ToString(); + await SeedTaskAsync(survivor, listId, TaskStatus.WaitingForReview); + + var svc = BuildService(); + var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "wait", CancellationToken.None); + var sessionDir = TrackSessionDir(spec); + + var handoff = File.ReadAllText(Path.Combine(sessionDir, "handoff.md")); + Assert.Contains(handlerTaskId, handoff); + } + [Fact] public async Task BuildForMergeHelperHandoffAsync_UnknownPhase_Throws() { @@ -1126,7 +1168,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); - var triageSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, CancellationToken.None); + var triageSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, handlerTaskId, CancellationToken.None); var triageDir = TrackSessionDir(triageSpec); var executeSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "merge", CancellationToken.None); var executeDir = TrackSessionDir(executeSpec); @@ -1177,7 +1219,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable ClaudeSessionId: "sess-42", Token: "tok-2", WorktreePath: _worktreeDir); var svc = BuildService(); - var mergeHelperSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, CancellationToken.None); + var mergeHelperSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, handlerTaskId, CancellationToken.None); TrackSessionDir(mergeHelperSpec); var handoffSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "wait", CancellationToken.None); TrackSessionDir(handoffSpec); diff --git a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs index 5acef16d..2b0d2878 100644 --- a/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs +++ b/tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs @@ -85,7 +85,7 @@ sealed class FakeWorkerClient : IWorkerClient public Task SubmitTaskForReviewAsync(string taskId, CancellationToken ct = default) => Task.CompletedTask; public Task GetInteractiveLaunchSpecAsync(string taskId, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); - public Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, CancellationToken ct = default) + public Task GetMergeHelperLaunchSpecAsync(IReadOnlyList taskIds, string listId, string handlerTaskId, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); public Task CreateMergeHelperTaskAsync( IReadOnlyList taskIds, string listId, string title, string descriptionHeader, CancellationToken ct = default)