feat(data): split list-handler prompt into triage/wait/merge roles
Adds HandlerTriageAlias/HandlerWaitAlias/HandlerMergeAlias to ModelRegistry and replaces PromptKind.MergeHelperExecute with MergeHelperWait (phase 3 only) and MergeHelperMerge (phases 4-5), so the list handler can run as three cost-scoped sessions instead of two. Triage's Phase 2 now checks/sets a wide verify command via get_list_config/set_list_config once per run. Merge now merges before reruns, delegates diff review to a sonnet subagent, rejects 0-file diffs, tracks the merged-but-not-Done verify-gate outcome, and caps reruns at one per task via handoff_list_handler's new nextPhase parameter. InteractiveLaunchSpecService.cs still references the removed PromptKind.MergeHelperExecute and fails to build -- wiring the Worker/UI side onto the new roles is a follow-up task.
This commit is contained in:
@@ -57,4 +57,14 @@ public class ModelRegistryTests
|
||||
{
|
||||
Assert.Null(ModelRegistry.TryNormalizeAlias(input));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void List_handler_role_aliases_are_valid_and_split_cost_by_role()
|
||||
{
|
||||
// Triage and Merge get the wide-context, expensive role; Wait just queues and blocks,
|
||||
// so it stays on the cheaper model.
|
||||
Assert.Equal("opus", ModelRegistry.NormalizeAlias(ModelRegistry.HandlerTriageAlias));
|
||||
Assert.Equal("sonnet", ModelRegistry.NormalizeAlias(ModelRegistry.HandlerWaitAlias));
|
||||
Assert.Equal("opus", ModelRegistry.NormalizeAlias(ModelRegistry.HandlerMergeAlias));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -84,7 +84,8 @@ public class PromptFilesTests
|
||||
public void PathFor_merge_helper_kinds_map_to_their_files()
|
||||
{
|
||||
Assert.EndsWith("merge-helper-triage.md", PromptFiles.PathFor(PromptKind.MergeHelperTriage));
|
||||
Assert.EndsWith("merge-helper-execute.md", PromptFiles.PathFor(PromptKind.MergeHelperExecute));
|
||||
Assert.EndsWith("merge-helper-wait.md", PromptFiles.PathFor(PromptKind.MergeHelperWait));
|
||||
Assert.EndsWith("merge-helper-merge.md", PromptFiles.PathFor(PromptKind.MergeHelperMerge));
|
||||
Assert.EndsWith("merge-helper-initial.md", PromptFiles.PathFor(PromptKind.MergeHelperInitial));
|
||||
Assert.EndsWith("merge-helper-handoff.md", PromptFiles.PathFor(PromptKind.MergeHelperHandoff));
|
||||
}
|
||||
@@ -105,16 +106,29 @@ public class PromptFilesTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_covers_phases_3_to_5_only()
|
||||
public void DefaultFor_merge_helper_wait_covers_phase_3_only()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperWait);
|
||||
Assert.False(string.IsNullOrWhiteSpace(d));
|
||||
Assert.Contains("## Phase 3", d);
|
||||
Assert.DoesNotContain("## Phase 0", d);
|
||||
Assert.DoesNotContain("## Phase 1", d);
|
||||
Assert.DoesNotContain("## Phase 2", d);
|
||||
Assert.DoesNotContain("## Phase 4", d);
|
||||
Assert.DoesNotContain("## Phase 5", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_covers_phases_4_to_5_only()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.False(string.IsNullOrWhiteSpace(d));
|
||||
Assert.Contains("## Phase 4", d);
|
||||
Assert.Contains("## Phase 5", d);
|
||||
Assert.DoesNotContain("## Phase 0", d);
|
||||
Assert.DoesNotContain("## Phase 1", d);
|
||||
Assert.DoesNotContain("## Phase 2", d);
|
||||
Assert.DoesNotContain("## Phase 3", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -123,6 +137,8 @@ public class PromptFilesTests
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("batch_get_tasks", d); // phase 0
|
||||
Assert.Contains("update_task", d); // phase 1 + 2
|
||||
Assert.Contains("get_list_config", d); // phase 2 verify-command check
|
||||
Assert.Contains("set_list_config", d); // phase 2 verify-command check
|
||||
Assert.Contains("handoff_list_handler", d); // handoff
|
||||
// Triage never runs or merges anything.
|
||||
Assert.DoesNotContain("review_task", d);
|
||||
@@ -131,50 +147,112 @@ public class PromptFilesTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_names_the_tools_its_phases_need()
|
||||
public void DefaultFor_merge_helper_wait_names_the_tools_it_needs()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
Assert.Contains("get_app_settings", d); // phase 3
|
||||
Assert.Contains("update_task_status", d); // phase 3
|
||||
Assert.Contains("wait_for_task_change", d); // phase 3
|
||||
Assert.Contains("preview_merge_set", d); // phase 4
|
||||
Assert.Contains("review_task", d); // phase 4
|
||||
Assert.Contains("continue_merge", d); // phase 4
|
||||
Assert.DoesNotContain("run_task_now(", d); // single override slot — must not batch-start
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperWait);
|
||||
Assert.Contains("update_task_status", d);
|
||||
Assert.Contains("wait_for_task_change", d);
|
||||
Assert.Contains("handoff_list_handler", d);
|
||||
// Wait never touches diffs or merges.
|
||||
Assert.DoesNotContain("get_task_diff", d);
|
||||
Assert.DoesNotContain("review_task", d);
|
||||
Assert.DoesNotContain("continue_merge", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_waits_with_the_real_server_side_cap()
|
||||
public void DefaultFor_merge_helper_merge_names_the_tools_it_needs()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("batch_get_tasks", d);
|
||||
Assert.Contains("preview_merge_set", d);
|
||||
Assert.Contains("review_task", d);
|
||||
Assert.Contains("continue_merge", d);
|
||||
Assert.Contains("terminal_reason", d);
|
||||
Assert.Contains("continue_task", d);
|
||||
Assert.Contains("reset_failed_task", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_wait_waits_with_the_real_server_side_cap()
|
||||
{
|
||||
// TaskWaitMcpTools.MaxTimeoutSeconds is 900 and every ClaudeDo launcher sets
|
||||
// MCP_TOOL_TIMEOUT=930000ms. The prompt used to say 170 -- a leftover from the retired
|
||||
// 200000ms era -- which burned ~5x the turns on re-waiting.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperWait);
|
||||
Assert.Contains("timeoutSeconds 900", d);
|
||||
Assert.DoesNotContain("170", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_waits_through_waiting_for_children()
|
||||
public void DefaultFor_merge_helper_wait_waits_through_waiting_for_children()
|
||||
{
|
||||
// Without the flag, a parent with children reports "changed" as soon as it reaches
|
||||
// WaitingForChildren, so the handler would advance to review/merge while children run.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
// WaitingForChildren, so the handler would advance to merge while children run.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperWait);
|
||||
Assert.Contains("treatWaitingForChildrenAsBusy=true", d);
|
||||
Assert.Contains("WaitingForChildren", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_keeps_the_shared_checkout_git_guard()
|
||||
public void DefaultFor_merge_helper_wait_never_touches_diffs_or_merge_state()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperWait);
|
||||
Assert.Contains("Touch nothing else", d);
|
||||
Assert.Contains("do not read a diff, merge, reset, restart, or cancel any task", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_keeps_the_shared_checkout_git_guard()
|
||||
{
|
||||
// The handler session never gets PromptKind.System, so this is the ONLY place the
|
||||
// never-`git add -A`-in-a-shared-checkout rule reaches a list-handler run.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("git add -- <the resolved paths>", d);
|
||||
Assert.Contains("NEVER `git add -A`", d);
|
||||
Assert.Contains("Never use raw `git merge`", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_runs_all_merges_before_any_rerun()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("BEFORE starting any rerun", d);
|
||||
Assert.Contains("only after every merge above is done", d, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_never_reads_diffs_itself()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("Never read a task's diff yourself", d);
|
||||
Assert.Contains("sonnet subagent", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_rejects_empty_diffs_before_approving()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("changedFileCount", d);
|
||||
Assert.Contains("0 changed files means an empty branch", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_caps_reruns_at_one_and_hands_off_to_wait_final()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("at most ONE rerun per task", d, StringComparison.OrdinalIgnoreCase);
|
||||
Assert.Contains("nextPhase: \"wait_final\"", d);
|
||||
Assert.Contains("merge_final", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_merge_holds_back_done_on_a_failed_verify_gate()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("merged-but-not-Done", d);
|
||||
Assert.Contains("do NOT try to fix the verify failure yourself", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_only_asks_dedupe_questions_when_a_candidate_exists()
|
||||
{
|
||||
@@ -204,24 +282,23 @@ public class PromptFilesTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_checks_effective_max_turns_before_queuing()
|
||||
public void DefaultFor_merge_helper_triage_checks_verify_command_once_per_run()
|
||||
{
|
||||
// Must delegate to get_effective_run_config rather than re-deriving the
|
||||
// task/list/preset chain: TaskRunner.ResolveMaxTurns clamps the result to
|
||||
// AppSettings.MaxTurnsCeiling, so a hand-derived number can exceed what actually runs.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
Assert.Contains("get_effective_run_config", d);
|
||||
Assert.Contains("clamped", d);
|
||||
Assert.Contains("set_task_config", d);
|
||||
Assert.DoesNotContain("get_list_config", d);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("get_list_config", d);
|
||||
Assert.Contains("ONCE for this whole run", d);
|
||||
Assert.Contains("set_list_config(verifyCommand:", d);
|
||||
// Never overwrite an operator-set value.
|
||||
Assert.Contains("never overwrite an existing one", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_quotes_the_override_slot_error_verbatim()
|
||||
public void DefaultFor_merge_helper_triage_demands_a_wide_verify_command_proven_by_running_it()
|
||||
{
|
||||
// ExternalMcpService.RunTaskNow rewraps OverrideSlotService's raw lowercase throw.
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
Assert.Contains("Override slot busy. Try again later.", d);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("full build plus the complete test suite", d);
|
||||
Assert.Contains("Run that command yourself once via Bash", d);
|
||||
Assert.Contains("ask the user to confirm it", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -261,7 +338,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_checks_file_collisions_before_merge_order()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperMerge);
|
||||
Assert.Contains("Default to the order the brief lists them", d);
|
||||
Assert.Contains("tell the user which tasks collide", d);
|
||||
Assert.Contains("NORMAL case, not a failure", d);
|
||||
|
||||
Reference in New Issue
Block a user