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.
This commit is contained in:
@@ -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<ArgumentException>(() =>
|
||||
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<InvalidOperationException>(() =>
|
||||
sut.HandoffListHandler(handlerTask.Id, Array.Empty<string>(), CancellationToken.None));
|
||||
sut.HandoffListHandler(handlerTask.Id, Array.Empty<string>(), cancellationToken: CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -80,7 +125,7 @@ public sealed class HandoffMcpToolsTests : IDisposable
|
||||
{
|
||||
var sut = BuildSut();
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() =>
|
||||
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<InvalidOperationException>(() =>
|
||||
sut.HandoffListHandler(handlerTask.Id, new[] { "missing" }, CancellationToken.None));
|
||||
sut.HandoffListHandler(handlerTask.Id, new[] { "missing" }, cancellationToken: CancellationToken.None));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<HubException>(
|
||||
() => hub.GetMergeHelperHandoffLaunchSpec("no-such-task", new[] { "x" }));
|
||||
() => hub.GetMergeHelperHandoffLaunchSpec("no-such-task", new[] { "x" }, "wait"));
|
||||
}
|
||||
|
||||
// ── SubmitTaskForReview (worktree-less branch) ──
|
||||
|
||||
@@ -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<InvalidOperationException>(
|
||||
() => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, Array.Empty<string>(), CancellationToken.None));
|
||||
() => svc.BuildForMergeHelperHandoffAsync(handlerTaskId, Array.Empty<string>(), "wait", CancellationToken.None));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task BuildForMergeHelperHandoffAsync_UnknownHandlerTask_ThrowsKeyNotFound()
|
||||
{
|
||||
await Assert.ThrowsAsync<KeyNotFoundException>(
|
||||
() => 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<KeyNotFoundException>(
|
||||
() => 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<InvalidOperationException>(
|
||||
() => 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<ArgumentException>(
|
||||
() => 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<LaunchSpec>
|
||||
|
||||
Reference in New Issue
Block a user