From f2609d186aac1c2bd9a04a1be8967cfcc7f81e8f Mon Sep 17 00:00:00 2001 From: mika kuns Date: Tue, 11 Aug 2026 09:27:16 +0200 Subject: [PATCH] feat(worker): wire list-handler phase parameter through handoff chain Threads a nextPhase parameter (wait/merge/wait_final/merge_final, validated by the new MergeHelperPhase) from handoff_list_handler through HubBroadcaster/WorkerHub into InteractiveLaunchSpecService, which now picks the next session's system prompt (MergeHelperWait/MergeHelperMerge) and model (HandlerWaitAlias/HandlerMergeAlias) from it instead of hardcoding the old two-phase Execute prompt -- this also fixes a build break left by the prior prompt-split task, which removed PromptKind.MergeHelperExecute without updating its only caller. Also sets --model/--effort/--permission-mode explicitly for every list-handler session (Triage included) via PermissionModeResolver instead of inheriting the CLI's ambient model and hardcoding "auto", and adds Task to the merge-helper allowlist so the Merge phase can delegate diff reviews to subagents. --- docs/explore-notes/conpty-sessions.md | 23 ++++-- src/ClaudeDo.Data/PromptFiles.cs | 1 + .../External/HandoffMcpTools.cs | 25 ++++-- src/ClaudeDo.Worker/Hub/HubBroadcaster.cs | 4 +- src/ClaudeDo.Worker/Hub/WorkerHub.cs | 4 +- .../Runner/InteractiveLaunchSpecService.cs | 68 +++++++++++---- .../IInteractiveLaunchSpecService.cs | 16 ++-- .../Runner/MergeHelperPhase.cs | 21 +++++ .../External/HandoffMcpToolsTests.cs | 53 +++++++++++- .../Hub/MergeHelperTaskHubTests.cs | 6 +- .../InteractiveLaunchSpecServiceTests.cs | 82 +++++++++++++++---- 11 files changed, 240 insertions(+), 63 deletions(-) create mode 100644 src/ClaudeDo.Worker/Runner/MergeHelperPhase.cs diff --git a/docs/explore-notes/conpty-sessions.md b/docs/explore-notes/conpty-sessions.md index 40619768..02db0804 100644 --- a/docs/explore-notes/conpty-sessions.md +++ b/docs/explore-notes/conpty-sessions.md @@ -18,7 +18,7 @@ Autonomous queue tasks are **not** covered here — they stay on the stream-json | Task session | `GetInteractiveLaunchSpec` | Effort from the task/list model preset | | Ad-hoc | `GetAdHocLaunchSpec` | Effort from the global default | | Planning | (planning start/resume) | Effort from `PlanningAlias`; uses `--permission-mode default`, **not** `plan` | -| List handler | `GetMergeHelperLaunchSpec` | Effort from list config; `--permission-mode auto` (unattended) | +| List handler | `GetMergeHelperLaunchSpec` / `GetMergeHelperHandoffLaunchSpec` | Model + effort fixed per role (`ModelRegistry.HandlerTriageAlias`/`HandlerWaitAlias`/`HandlerMergeAlias`), **not** from list config; `--permission-mode` via `PermissionModeResolver` (unattended) | ## ⚠️ Gotcha: never pass task free-text as a CLI argument @@ -99,9 +99,21 @@ launch even started. ## List handler ("Let Claude handle it") -`BuildForMergeHelperAsync` uses `--permission-mode auto` so it runs unattended. The -`--allowedTools` allowlist is the security boundary: -`mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill`. +`BuildForMergeHelperAsync`/`BuildForMergeHelperHandoffAsync` resolve `--permission-mode` via +`PermissionModeResolver` (currently always `auto`, since none of the handler roles run on haiku) +so the session runs unattended. The `--allowedTools` allowlist is the security boundary: +`mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task` (`Task` lets the Merge +role delegate diff reviews to sonnet subagents). + +The run now spans up to **five** phase-scoped sessions instead of two, chained via +`handoff_list_handler(taskId, survivingTaskIds, nextPhase)`: opus Triage → sonnet Wait → opus +Merge → (only if Merge started reruns) sonnet Wait(final) → opus Merge(final). `nextPhase` (`wait` +| `merge` | `wait_final` | `merge_final`, validated by `MergeHelperPhase.Validate`) picks both the +system prompt (`PromptKind.MergeHelperWait`/`MergeHelperMerge`) and the model +(`ModelRegistry.HandlerWaitAlias`/`HandlerMergeAlias`) for the next session; the `_final` variants +share the same prompt/model as their non-final counterpart and differ only in one extra line +rendered into the handoff kickoff file (`PromptKind.MergeHelperHandoff`) marking the final round +and forbidding further reruns. `MCP_TOOL_TIMEOUT` is 200 s here — `TaskWaitMcpTools` clamps its own timeout to 170 s to stay comfortably under it (see [external-mcp.md](external-mcp.md)). @@ -242,7 +254,8 @@ Autonomous and interactive sessions do **not** share a system prompt. Per start | Planning session start | `InteractiveLaunchSpecService.BuildPlanningStart` → `WindowsTerminalLauncher.BuildPlanningStartArgs` | `--append-system-prompt-file ` (`PromptKind.Planning`) | | Planning session resume | `InteractiveLaunchSpecService.BuildPlanningResume` → `WindowsTerminalLauncher.BuildPlanningResumeArgs` | none — only `--permission-mode default --allowedTools --resume ` | | List handler ("Let Claude handle it"), triage | `InteractiveLaunchSpecService.BuildForMergeHelperAsync` | `--append-system-prompt-file ` (`PromptKind.MergeHelperTriage` — phases 0–2 only), always fresh — this path never resumes | -| List handler, post-handoff | `InteractiveLaunchSpecService.BuildForMergeHelperHandoffAsync` | `--append-system-prompt-file ` (`PromptKind.MergeHelperExecute` — phases 3–5 only), fresh session dir, same handler task id | +| List handler, post-handoff (wait/wait_final) | `InteractiveLaunchSpecService.BuildForMergeHelperHandoffAsync` | `--append-system-prompt-file ` (`PromptKind.MergeHelperWait` — phase 3 only), fresh session dir, same handler task id | +| List handler, post-handoff (merge/merge_final) | `InteractiveLaunchSpecService.BuildForMergeHelperHandoffAsync` | `--append-system-prompt-file ` (`PromptKind.MergeHelperMerge` — phases 4–5 only), fresh session dir, same handler task id | So every interactive resume (task session and planning) drops the system prompt entirely — it's not that they inherit the autonomous one, it's that **no** `claude` process on any resume path diff --git a/src/ClaudeDo.Data/PromptFiles.cs b/src/ClaudeDo.Data/PromptFiles.cs index 45cdb958..67298121 100644 --- a/src/ClaudeDo.Data/PromptFiles.cs +++ b/src/ClaudeDo.Data/PromptFiles.cs @@ -567,6 +567,7 @@ public static class PromptFiles Start with phase 3 (run), continuing through review/merge and the summary as your instructions describe. + {finalNote} """; private const string WeeklyReportDefault = """ diff --git a/src/ClaudeDo.Worker/External/HandoffMcpTools.cs b/src/ClaudeDo.Worker/External/HandoffMcpTools.cs index b44d05b7..b77ebb29 100644 --- a/src/ClaudeDo.Worker/External/HandoffMcpTools.cs +++ b/src/ClaudeDo.Worker/External/HandoffMcpTools.cs @@ -1,11 +1,12 @@ using System.ComponentModel; using ClaudeDo.Data.Repositories; using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Runner; using ModelContextProtocol.Server; namespace ClaudeDo.Worker.External; -public sealed record HandoffListHandlerResult(bool Requested, string TaskId, int SurvivingCount); +public sealed record HandoffListHandlerResult(bool Requested, string TaskId, int SurvivingCount, string NextPhase); [McpServerToolType] public sealed class HandoffMcpTools @@ -20,16 +21,22 @@ public sealed class HandoffMcpTools } [McpServerTool, Description( - "Call at the end of Phase 2 of the list handler (\"Let Claude handle it\") to hand this run off " + - "to a fresh ConPTY session that carries out Phases 3-5, without dragging along this session's " + - "dedupe/rewrite context. Reuses the SAME handler task -- no new task is created, and " + - "HandlerBaseCommit is untouched. The current tile stays open; you must end your own turn " + - "immediately after calling this.")] + "Call at the end of a list-handler (\"Let Claude handle it\") phase to hand this run off to a " + + "fresh ConPTY session that carries out the next phase, without dragging along this session's " + + "own context. Reuses the SAME handler task -- no new task is created, and HandlerBaseCommit is " + + "untouched. nextPhase picks the next session's role: \"wait\" (queue + wait; the normal handoff " + + "after triage) or \"merge\" (review + merge; the normal handoff after wait) -- \"wait_final\"/" + + "\"merge_final\" are only for a merge session that started reruns and must eventually stop the " + + "chain instead of restarting again. An unrecognized value is rejected. The current tile stays " + + "open; you must end your own turn immediately after calling this.")] public async Task HandoffListHandler( [Description("This session's own handler task id.")] string taskId, [Description("The tasks that made it past dedupe, in the order to run them.")] IReadOnlyList survivingTaskIds, - CancellationToken cancellationToken) + [Description("Next session's phase: wait | merge | wait_final | merge_final.")] string nextPhase = "wait", + CancellationToken cancellationToken = default) { + MergeHelperPhase.Validate(nextPhase); + if (survivingTaskIds.Count == 0) throw new InvalidOperationException("survivingTaskIds must contain at least one task id."); @@ -40,7 +47,7 @@ public sealed class HandoffMcpTools _ = await _tasks.GetByIdAsync(id, cancellationToken) ?? throw new InvalidOperationException($"Task {id} not found."); - await _broadcaster.HandoffRequested(taskId, survivingTaskIds); - return new HandoffListHandlerResult(true, taskId, survivingTaskIds.Count); + await _broadcaster.HandoffRequested(taskId, survivingTaskIds, nextPhase); + return new HandoffListHandlerResult(true, taskId, survivingTaskIds.Count, nextPhase); } } diff --git a/src/ClaudeDo.Worker/Hub/HubBroadcaster.cs b/src/ClaudeDo.Worker/Hub/HubBroadcaster.cs index c3828c2d..fe90bbf6 100644 --- a/src/ClaudeDo.Worker/Hub/HubBroadcaster.cs +++ b/src/ClaudeDo.Worker/Hub/HubBroadcaster.cs @@ -34,8 +34,8 @@ public sealed class HubBroadcaster : IPrimeBroadcaster, IRefineBroadcaster // A running list-handler session called handoff_list_handler at the end of Phase 2 -- the UI // opens a second ConPTY tile for the same handler task id to carry out Phases 3-5. - public Task HandoffRequested(string taskId, IReadOnlyList survivingTaskIds) => - _hub.Clients.All.SendAsync("HandoffRequested", taskId, survivingTaskIds); + public Task HandoffRequested(string taskId, IReadOnlyList survivingTaskIds, string nextPhase) => + _hub.Clients.All.SendAsync("HandoffRequested", taskId, survivingTaskIds, nextPhase); public Task ListUpdated(string listId) => _hub.Clients.All.SendAsync("ListUpdated", listId); diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 8f2ed712..318abfd5 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -855,11 +855,11 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub // Builds the launch spec for the fresh ConPTY session a merge-helper run hands off to once // Phase 2 (enhance) is done -- reuses the SAME handler task id (no new task created), only a // new session dir + handoff kickoff naming the surviving tasks. - public Task GetMergeHelperHandoffLaunchSpec(string taskId, string[] survivingTaskIds) => HubGuard(() => + public Task GetMergeHelperHandoffLaunchSpec(string taskId, string[] survivingTaskIds, string nextPhase) => HubGuard(() => { if (_interactiveLaunchSpec is null) throw new InvalidOperationException("Interactive launch spec service is not configured."); - return _interactiveLaunchSpec.BuildForMergeHelperHandoffAsync(taskId, survivingTaskIds, Context.ConnectionAborted); + return _interactiveLaunchSpec.BuildForMergeHelperHandoffAsync(taskId, survivingTaskIds, nextPhase, Context.ConnectionAborted); }); // Starts a planning session (worktree + prompt files + token, task -> Planning) and returns diff --git a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs index 7bf267f2..097a292d 100644 --- a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using ClaudeDo.Data; using ClaudeDo.Data.Git; using ClaudeDo.Data.Models; @@ -204,9 +205,10 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService // Tools the merge helper may use without prompting: the claudedo MCP surface (run, poll, // diff, review/merge, continue/abort merge), read/search, Edit + Bash for hand-resolving - // conflict markers the MCP tools left behind, and web/skill lookups. + // conflict markers the MCP tools left behind, web/skill lookups, and Task so the Merge phase + // can delegate diff reviews to sonnet subagents instead of reading every diff itself. private const string MergeHelperAllowedTools = - "mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill"; + "mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task"; public async Task BuildForMergeHelperAsync(IReadOnlyList taskIds, string listId, CancellationToken ct) { @@ -255,13 +257,16 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService // (--allowedTools, --add-dir) first, then a single-value flag, then the single-line // positional kickoff LAST — a multi-line positional prompt truncates at the first // newline, so the full multi-line brief travels via the file exposed through --add-dir. - var listConfig = await listRepo.GetConfigAsync(listId, ct); + // Model/effort/permission-mode are fixed to the Triage role -- the list config's model no + // longer influences a handler session at all, it would otherwise apply the LIST's effort + // preset to a completely different model. var settings = await new AppSettingsRepository(ctx).GetAsync(ct); var args = new List { - "--effort", EffortFor(settings, listConfig?.Model), - "--permission-mode", "auto", + "--model", ModelRegistry.HandlerTriageAlias, + "--effort", EffortFor(settings, ModelRegistry.HandlerTriageAlias), + "--permission-mode", PermissionModeResolver.Resolve(ModelRegistry.HandlerTriageAlias, "auto"), "--allowedTools", MergeHelperAllowedTools, "--add-dir", sessionDir, repoDir, "--append-system-prompt-file", systemPromptPath, @@ -277,19 +282,22 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService return new LaunchSpec(repoDir, resolvedClaude, args, env); } - // Builds the LaunchSpec for the fresh session a merge-helper run hands off to once Phase 2 - // (enhance) is done -- SAME handler task id as the run that called handoff_list_handler, so + // Builds the LaunchSpec for the fresh session a merge-helper run hands off to for the given + // phase -- SAME handler task id as the run that called handoff_list_handler, so // HandlerBaseCommit/HandlerHeadCommit and the review range stay untouched; this never creates - // a task. Uses the MergeHelperExecute system prompt (phases 3-5 only) rather than the Triage - // one this run started with, so the handoff session carries no dedupe/enhance instructions it - // would have to ignore. Writes a fresh handoff kickoff file in a NEW session dir -- the old - // ConPTY tile keeps running against its own session-dir files untouched. + // a task. nextPhase picks the system prompt (MergeHelperWait for wait/wait_final, + // MergeHelperMerge for merge/merge_final) so the handoff session only ever carries the + // instructions for the role it is actually about to run, never the Triage dedupe/enhance ones. + // Writes a fresh handoff kickoff file in a NEW session dir -- the old ConPTY tile keeps running + // against its own session-dir files untouched. public async Task BuildForMergeHelperHandoffAsync( - string taskId, IReadOnlyList survivingTaskIds, CancellationToken ct) + string taskId, IReadOnlyList survivingTaskIds, string nextPhase, CancellationToken ct) { if (survivingTaskIds.Count == 0) throw new InvalidOperationException("No surviving tasks to hand off."); + var phase = ResolveHandoffPhase(nextPhase); + await using var ctx = await _dbFactory.CreateDbContextAsync(ct); var taskRepo = new TaskRepository(ctx); var listRepo = new ListRepository(ctx); @@ -316,7 +324,15 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService Directory.CreateDirectory(sessionDir); var systemPromptPath = Path.Combine(sessionDir, "system-prompt.md"); - await File.WriteAllTextAsync(systemPromptPath, PromptFiles.ReadOrDefault(PromptKind.MergeHelperExecute), ct); + await File.WriteAllTextAsync(systemPromptPath, PromptFiles.ReadOrDefault(phase.PromptKind), ct); + + // A "_final" round differs from its non-final counterpart in exactly one rendered line -- + // the same system prompt and model drive both -- marking the final round and forbidding a + // further rerun, so the chain is bounded structurally rather than by prompt appeal alone. + var finalNote = phase.IsFinal + ? "\nThis is the FINAL round of this list-handler run -- do not start any further reruns; " + + "merge what you can, report the rest, and print the summary." + : ""; var briefPath = Path.Combine(sessionDir, "handoff.md"); await File.WriteAllTextAsync(briefPath, PromptFiles.Render(PromptKind.MergeHelperHandoff, @@ -325,23 +341,24 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService ["scope"] = $"List: {list.Name}", ["repo"] = repoDir, ["tasks"] = string.Join("\n", briefLines), + ["finalNote"] = finalNote, }), ct); var resolvedClaude = WindowsTerminalLauncher.Resolve(_claudePath) ?? throw new InvalidOperationException($"claude executable not found: {_claudePath}"); - var listConfig = await listRepo.GetConfigAsync(handlerTask.ListId, ct); var settings = await new AppSettingsRepository(ctx).GetAsync(ct); var args = new List { - "--effort", EffortFor(settings, listConfig?.Model), - "--permission-mode", "auto", + "--model", phase.Model, + "--effort", EffortFor(settings, phase.Model), + "--permission-mode", PermissionModeResolver.Resolve(phase.Model, "auto"), "--allowedTools", MergeHelperAllowedTools, "--add-dir", sessionDir, repoDir, "--append-system-prompt-file", systemPromptPath, $"Read the file {briefPath} first. It lists the surviving tasks and their status. " + - "After reading it, begin at phase 3 as your instructions describe.", + $"After reading it, begin at {phase.PhaseLabel} as your instructions describe.", }; var env = new Dictionary @@ -443,6 +460,23 @@ public sealed class InteractiveLaunchSpecService : IInteractiveLaunchSpecService return handlerTask.Id; } + // Maps a handoff nextPhase to the system prompt/model the next session runs under, plus the + // "phase N" label used in its short positional kickoff. wait/wait_final share MergeHelperWait + // + HandlerWaitAlias; merge/merge_final share MergeHelperMerge + HandlerMergeAlias -- only + // IsFinal differs between a phase and its "_final" counterpart. + private static (PromptKind PromptKind, string Model, string PhaseLabel, bool IsFinal) ResolveHandoffPhase(string nextPhase) + { + MergeHelperPhase.Validate(nextPhase); + return nextPhase switch + { + MergeHelperPhase.Wait => (PromptKind.MergeHelperWait, ModelRegistry.HandlerWaitAlias, "phase 3", false), + MergeHelperPhase.WaitFinal => (PromptKind.MergeHelperWait, ModelRegistry.HandlerWaitAlias, "phase 3", true), + MergeHelperPhase.Merge => (PromptKind.MergeHelperMerge, ModelRegistry.HandlerMergeAlias, "phase 4", false), + MergeHelperPhase.MergeFinal => (PromptKind.MergeHelperMerge, ModelRegistry.HandlerMergeAlias, "phase 4", true), + _ => throw new UnreachableException(), + }; + } + // The reasoning effort configured for a model in Settings → General. Falls back to the shipped // preset for that model, so a missing/malformed settings row can never block a launch. private static string EffortFor(AppSettingsEntity settings, string? model) diff --git a/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs index 6932f16e..4c21cce3 100644 --- a/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/Interfaces/IInteractiveLaunchSpecService.cs @@ -46,11 +46,15 @@ public interface IInteractiveLaunchSpecService Task CreateMergeHelperTaskAsync( IReadOnlyList taskIds, string listId, string title, string descriptionHeader, CancellationToken ct); - /// Builds a LaunchSpec for the fresh ConPTY session a merge-helper run hands off to - /// once Phase 2 (enhance) is done -- reuses the SAME handler task id (no new task, HandlerBaseCommit - /// untouched) and the unmodified merge-helper system prompt, writing only a fresh handoff kickoff - /// naming the surviving tasks. Throws KeyNotFoundException if the task/list doesn't exist; - /// InvalidOperationException if survivingTaskIds is empty or the list has no working directory. + /// Builds a LaunchSpec for the fresh ConPTY session a merge-helper run hands off to for + /// the given phase -- reuses the SAME handler task id (no new task, HandlerBaseCommit untouched), + /// writing only a fresh handoff kickoff naming the surviving tasks. nextPhase selects both the + /// system prompt and the model: "wait"/"wait_final" -> MergeHelperWait + HandlerWaitAlias, + /// "merge"/"merge_final" -> MergeHelperMerge + HandlerMergeAlias; the "_final" variants render an + /// extra line in the handoff kickoff marking the final round and forbidding further reruns. Throws + /// ArgumentException for an unrecognized nextPhase; KeyNotFoundException if the task/list doesn't + /// exist; InvalidOperationException if survivingTaskIds is empty or the list has no working + /// directory. Task BuildForMergeHelperHandoffAsync( - string taskId, IReadOnlyList survivingTaskIds, CancellationToken ct); + string taskId, IReadOnlyList survivingTaskIds, string nextPhase, CancellationToken ct); } diff --git a/src/ClaudeDo.Worker/Runner/MergeHelperPhase.cs b/src/ClaudeDo.Worker/Runner/MergeHelperPhase.cs new file mode 100644 index 00000000..1ed8d177 --- /dev/null +++ b/src/ClaudeDo.Worker/Runner/MergeHelperPhase.cs @@ -0,0 +1,21 @@ +namespace ClaudeDo.Worker.Runner; + +/// The list handler's phase parameter, threaded from handoff_list_handler (External) +/// through HubBroadcaster/WorkerHub down to InteractiveLaunchSpecService, which picks the next +/// session's system prompt and model from it. Validated centrally here so an unknown value is +/// rejected outright instead of silently falling back to a default. +public static class MergeHelperPhase +{ + public const string Wait = "wait"; + public const string Merge = "merge"; + public const string WaitFinal = "wait_final"; + public const string MergeFinal = "merge_final"; + + public static readonly IReadOnlyList All = new[] { Wait, Merge, WaitFinal, MergeFinal }; + + public static void Validate(string phase) + { + if (!All.Contains(phase, StringComparer.Ordinal)) + throw new ArgumentException($"Unknown list-handler phase '{phase}'. Allowed: {string.Join(", ", All)}."); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs index a22ae794..6309c6db 100644 --- a/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs @@ -53,15 +53,60 @@ public sealed class HandoffMcpToolsTests : IDisposable var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor"); var sut = BuildSut(); - var result = await sut.HandoffListHandler(handlerTask.Id, new[] { survivor.Id }, CancellationToken.None); + var result = await sut.HandoffListHandler(handlerTask.Id, new[] { survivor.Id }, cancellationToken: CancellationToken.None); Assert.True(result.Requested); Assert.Equal(handlerTask.Id, result.TaskId); Assert.Equal(1, result.SurvivingCount); + Assert.Equal("wait", result.NextPhase); var call = Assert.Single(_hubContext.Proxy.Calls); Assert.Equal("HandoffRequested", call.Method); Assert.Equal(handlerTask.Id, call.Args[0]); + Assert.Equal("wait", call.Args[2]); + } + + [Fact] + public async Task HandoffListHandler_NextPhaseDefaultsToWait() + { + var listId = await SeedListAsync(); + var handlerTask = await SeedTaskAsync(listId); + var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor"); + + var sut = BuildSut(); + var result = await sut.HandoffListHandler(handlerTask.Id, new[] { survivor.Id }, cancellationToken: CancellationToken.None); + + Assert.Equal("wait", result.NextPhase); + } + + [Theory] + [InlineData("wait")] + [InlineData("merge")] + [InlineData("wait_final")] + [InlineData("merge_final")] + public async Task HandoffListHandler_AcceptsEveryKnownPhase(string phase) + { + var listId = await SeedListAsync(); + var handlerTask = await SeedTaskAsync(listId); + var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor"); + + var sut = BuildSut(); + var result = await sut.HandoffListHandler(handlerTask.Id, new[] { survivor.Id }, phase, CancellationToken.None); + + Assert.Equal(phase, result.NextPhase); + } + + [Fact] + public async Task HandoffListHandler_UnknownPhase_Throws() + { + var listId = await SeedListAsync(); + var handlerTask = await SeedTaskAsync(listId); + var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor"); + + var sut = BuildSut(); + var ex = await Assert.ThrowsAsync(() => + sut.HandoffListHandler(handlerTask.Id, new[] { survivor.Id }, "phase-9", CancellationToken.None)); + Assert.Contains("phase-9", ex.Message); } [Fact] @@ -72,7 +117,7 @@ public sealed class HandoffMcpToolsTests : IDisposable var sut = BuildSut(); await Assert.ThrowsAsync(() => - sut.HandoffListHandler(handlerTask.Id, Array.Empty(), CancellationToken.None)); + sut.HandoffListHandler(handlerTask.Id, Array.Empty(), cancellationToken: CancellationToken.None)); } [Fact] @@ -80,7 +125,7 @@ public sealed class HandoffMcpToolsTests : IDisposable { var sut = BuildSut(); await Assert.ThrowsAsync(() => - sut.HandoffListHandler("missing", new[] { "x" }, CancellationToken.None)); + sut.HandoffListHandler("missing", new[] { "x" }, cancellationToken: CancellationToken.None)); } [Fact] @@ -91,6 +136,6 @@ public sealed class HandoffMcpToolsTests : IDisposable var sut = BuildSut(); await Assert.ThrowsAsync(() => - sut.HandoffListHandler(handlerTask.Id, new[] { "missing" }, CancellationToken.None)); + sut.HandoffListHandler(handlerTask.Id, new[] { "missing" }, cancellationToken: CancellationToken.None)); } } diff --git a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs index ca412641..2114f562 100644 --- a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs @@ -149,14 +149,14 @@ public sealed class MergeHelperTaskHubTests : IDisposable var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor task"); var hub = CreateHub(); - var spec = await hub.GetMergeHelperHandoffLaunchSpec(handlerTask.Id, new[] { survivor.Id }); + var spec = await hub.GetMergeHelperHandoffLaunchSpec(handlerTask.Id, new[] { survivor.Id }, "wait"); var args = spec.Args.ToList(); var sessionDir = args[args.IndexOf("--add-dir") + 1]; _mergeHelperSessionDirs.Add(sessionDir); Assert.Contains("--allowedTools", args); - Assert.Contains("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill", args); + Assert.Contains("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task", args); var kickoff = args[^1]; Assert.Contains(Path.Combine(sessionDir, "handoff.md"), kickoff); @@ -167,7 +167,7 @@ public sealed class MergeHelperTaskHubTests : IDisposable { var hub = CreateHub(); await Assert.ThrowsAsync( - () => hub.GetMergeHelperHandoffLaunchSpec("no-such-task", new[] { "x" })); + () => hub.GetMergeHelperHandoffLaunchSpec("no-such-task", new[] { "x" }, "wait")); } // ── SubmitTaskForReview (worktree-less branch) ── diff --git a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs index f033c9e8..d109f7be 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs @@ -592,12 +592,20 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var args = spec.Args.ToList(); + var modelIdx = args.IndexOf("--model"); + Assert.True(modelIdx >= 0); + Assert.Equal(ModelRegistry.HandlerTriageAlias, args[modelIdx + 1]); + + var effortIdx = args.IndexOf("--effort"); + Assert.True(effortIdx >= 0); + Assert.Equal(ModelPresets.For(ModelPresets.Defaults, ModelRegistry.HandlerTriageAlias).Effort, args[effortIdx + 1]); + var pmIdx = args.IndexOf("--permission-mode"); Assert.True(pmIdx >= 0); Assert.Equal("auto", args[pmIdx + 1]); var atIdx = args.IndexOf("--allowedTools"); - Assert.Equal("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill", args[atIdx + 1]); + Assert.Equal("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task", args[atIdx + 1]); // --add-dir: session dir + the list's single repo dir var addIdx = args.IndexOf("--add-dir"); @@ -814,14 +822,14 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, Array.Empty(), CancellationToken.None)); + () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, Array.Empty(), "wait", CancellationToken.None)); } [Fact] public async Task BuildForMergeHelperHandoffAsync_UnknownHandlerTask_ThrowsKeyNotFound() { await Assert.ThrowsAsync( - () => BuildService().BuildForMergeHelperHandoffAsync("no-such-task", new[] { "x" }, CancellationToken.None)); + () => BuildService().BuildForMergeHelperHandoffAsync("no-such-task", new[] { "x" }, "wait", CancellationToken.None)); } [Fact] @@ -833,7 +841,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { "no-such-task" }, CancellationToken.None)); + () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { "no-such-task" }, "wait", CancellationToken.None)); } [Fact] @@ -847,7 +855,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); var ex = await Assert.ThrowsAsync( - () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None)); + () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "wait", CancellationToken.None)); Assert.Contains("working directory", ex.Message); } @@ -864,7 +872,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable await SeedTaskAsync(survivor, listId, TaskStatus.WaitingForReview, title: "Survivor"); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None); + var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "wait", CancellationToken.None); var sessionDir = TrackSessionDir(spec); var args = spec.Args.ToList(); @@ -875,9 +883,29 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable } [Fact] - public async Task BuildForMergeHelperHandoffAsync_ReusesHandlerTaskId_NoNewTaskCreated() + public async Task BuildForMergeHelperHandoffAsync_UnknownPhase_Throws() { - var repo = Path.Combine(_tempDir, "repoHandoff"); + var listId = await SeedListAsync(workingDir: _tempDir); + var handlerTaskId = Guid.NewGuid().ToString(); + await SeedTaskAsync(handlerTaskId, listId, TaskStatus.Idle); + var survivor = Guid.NewGuid().ToString(); + await SeedTaskAsync(survivor, listId, TaskStatus.WaitingForReview); + + var svc = BuildService(); + var ex = await Assert.ThrowsAsync( + () => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "phase-9", CancellationToken.None)); + Assert.Contains("phase-9", ex.Message); + } + + [Theory] + [InlineData("wait", "phase 3", false)] + [InlineData("wait_final", "phase 3", true)] + [InlineData("merge", "phase 4", false)] + [InlineData("merge_final", "phase 4", true)] + public async Task BuildForMergeHelperHandoffAsync_ReusesHandlerTaskId_PerPhaseModelEffortPromptAndKickoff( + string nextPhase, string expectedPhaseLabel, bool expectFinalNote) + { + var repo = Path.Combine(_tempDir, $"repoHandoff-{nextPhase}"); Directory.CreateDirectory(repo); var listId = await SeedListAsync(workingDir: repo, name: "Alpha"); @@ -889,7 +917,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var beforeCount = await CountTasksAsync(); var svc = BuildService(); - var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None); + var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, nextPhase, CancellationToken.None); var sessionDir = TrackSessionDir(spec); var afterCount = await CountTasksAsync(); @@ -898,21 +926,35 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable Assert.Equal(repo, spec.Cwd); Assert.Equal(_claudeStubPath, spec.Exe); + var expectedModel = nextPhase is "wait" or "wait_final" ? ModelRegistry.HandlerWaitAlias : ModelRegistry.HandlerMergeAlias; + var expectedPromptKind = nextPhase is "wait" or "wait_final" ? PromptKind.MergeHelperWait : PromptKind.MergeHelperMerge; + var args = spec.Args.ToList(); + + var modelIdx = args.IndexOf("--model"); + Assert.Equal(expectedModel, args[modelIdx + 1]); + + var effortIdx = args.IndexOf("--effort"); + Assert.Equal(ModelPresets.For(ModelPresets.Defaults, expectedModel).Effort, args[effortIdx + 1]); + + var pmIdx = args.IndexOf("--permission-mode"); + Assert.Equal("auto", args[pmIdx + 1]); + var atIdx = args.IndexOf("--allowedTools"); - Assert.Equal("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill", args[atIdx + 1]); + Assert.Equal("mcp__claudedo__*,Read,Grep,Glob,Edit,Bash,WebFetch,WebSearch,Skill,Task", args[atIdx + 1]); var appendIdx = args.IndexOf("--append-system-prompt-file"); var systemPromptPath = args[appendIdx + 1]; Assert.Equal(Path.Combine(sessionDir, "system-prompt.md"), systemPromptPath); - // The handoff session gets the Execute prompt (phases 3-5), NOT the Triage prompt the - // first session ran with — otherwise it carries dedupe/enhance instructions to ignore. - Assert.Equal(PromptFiles.ReadOrDefault(PromptKind.MergeHelperExecute), File.ReadAllText(systemPromptPath)); + // The handoff session only ever gets the prompt for the role it is about to run -- + // never the Triage prompt the first session ran with, and never the OTHER role's prompt. + Assert.Equal(PromptFiles.ReadOrDefault(expectedPromptKind), File.ReadAllText(systemPromptPath)); var kickoff = args[^1]; var handoffPath = Path.Combine(sessionDir, "handoff.md"); Assert.Contains(handoffPath, kickoff); Assert.DoesNotContain('\n', kickoff); + Assert.Contains(expectedPhaseLabel, kickoff); var handoffBrief = File.ReadAllText(handoffPath); Assert.Contains("Scope: List: Alpha", handoffBrief); @@ -921,6 +963,16 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable Assert.Contains(survivor, handoffBrief); Assert.Contains("phase 3", handoffBrief, StringComparison.OrdinalIgnoreCase); + if (expectFinalNote) + { + Assert.Contains("FINAL round", handoffBrief); + Assert.Contains("rerun", handoffBrief, StringComparison.OrdinalIgnoreCase); + } + else + { + Assert.DoesNotContain("FINAL round", handoffBrief); + } + Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]); } @@ -991,7 +1043,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var triageSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, CancellationToken.None); var triageDir = TrackSessionDir(triageSpec); - var executeSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None); + var executeSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "merge", CancellationToken.None); var executeDir = TrackSessionDir(executeSpec); var triagePrompt = File.ReadAllText(Path.Combine(triageDir, "system-prompt.md")); @@ -1042,7 +1094,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable var svc = BuildService(); var mergeHelperSpec = await svc.BuildForMergeHelperAsync(new[] { survivor }, listId, CancellationToken.None); TrackSessionDir(mergeHelperSpec); - var handoffSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, CancellationToken.None); + var handoffSpec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, "wait", CancellationToken.None); TrackSessionDir(handoffSpec); var specs = new List