refactor(prompts): split the list-handler prompt per session phase
Both merge-helper ConPTY sessions loaded PromptKind.MergeHelper, so the post-handoff session received the phase 0-2 dedupe/enhance instructions and was told to ignore them by its brief alone. Split into MergeHelperTriage (phases 0-2 + handoff) and MergeHelperExecute (phases 3-5), so each session carries only its own phases. Consolidated the generic ask-the-user rule to one place per prompt, scoped Phase 5's summary to what the execute session actually knows, and moved the dedupe/enhance bilanz to the triage handoff. Regression guards assert neither prompt carries the other's phase headings and that the shared-checkout git rule stays in execute.
This commit is contained in:
@@ -47,41 +47,81 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void PathFor_merge_helper_kinds_map_to_their_files()
|
||||
{
|
||||
Assert.EndsWith("merge-helper-system.md", PromptFiles.PathFor(PromptKind.MergeHelper));
|
||||
Assert.EndsWith("merge-helper-triage.md", PromptFiles.PathFor(PromptKind.MergeHelperTriage));
|
||||
Assert.EndsWith("merge-helper-execute.md", PromptFiles.PathFor(PromptKind.MergeHelperExecute));
|
||||
Assert.EndsWith("merge-helper-initial.md", PromptFiles.PathFor(PromptKind.MergeHelperInitial));
|
||||
Assert.EndsWith("merge-helper-handoff.md", PromptFiles.PathFor(PromptKind.MergeHelperHandoff));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_covers_all_five_phases()
|
||||
public void DefaultFor_merge_helper_triage_covers_phases_0_to_2_only()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.False(string.IsNullOrWhiteSpace(d));
|
||||
Assert.Contains("Phase 0", d);
|
||||
Assert.Contains("Phase 1", d);
|
||||
Assert.Contains("Phase 2", d);
|
||||
Assert.Contains("Phase 3", d);
|
||||
Assert.Contains("Phase 4", d);
|
||||
Assert.Contains("Phase 5", d);
|
||||
Assert.Contains("## Phase 0", d);
|
||||
Assert.Contains("## Phase 1", d);
|
||||
Assert.Contains("## Phase 2", d);
|
||||
// The run/review/merge phases belong to the handoff session's prompt, not this one —
|
||||
// carrying them here is what made the handoff session redo dedupe work.
|
||||
Assert.DoesNotContain("## Phase 3", d);
|
||||
Assert.DoesNotContain("## Phase 4", d);
|
||||
Assert.DoesNotContain("## Phase 5", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_names_the_tools_each_phase_needs()
|
||||
public void DefaultFor_merge_helper_execute_covers_phases_3_to_5_only()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
Assert.False(string.IsNullOrWhiteSpace(d));
|
||||
Assert.Contains("## Phase 3", 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);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_triage_names_the_tools_its_phases_need()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("batch_get_tasks", d); // phase 0
|
||||
Assert.Contains("update_task", d); // phase 1 + 2
|
||||
Assert.Contains("handoff_list_handler", d); // handoff
|
||||
// Triage never runs or merges anything.
|
||||
Assert.DoesNotContain("review_task", d);
|
||||
Assert.DoesNotContain("continue_merge", d);
|
||||
Assert.DoesNotContain("preview_merge_set", d);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_names_the_tools_its_phases_need()
|
||||
{
|
||||
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
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_execute_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);
|
||||
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_only_asks_dedupe_questions_when_a_candidate_exists()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("move straight to Phase 2", d);
|
||||
Assert.Contains("do not ask the user to confirm the absence of duplicates", d);
|
||||
Assert.Contains("Cancel nothing without an explicit answer", d);
|
||||
@@ -91,7 +131,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_phase0_treats_brief_as_primary_source()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("primary source", d, StringComparison.OrdinalIgnoreCase);
|
||||
Assert.Contains("batch_get_tasks", d);
|
||||
Assert.Contains("Do not act on any single task before you have read", d);
|
||||
@@ -100,7 +140,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_allows_splitting_bundled_tasks_in_phase_2()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("Propose splitting it to the user", d);
|
||||
Assert.Contains("add_task/add_subtask", d);
|
||||
Assert.Contains("do not invent requirements", d);
|
||||
@@ -109,7 +149,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_checks_effective_max_turns_before_queuing()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
Assert.Contains("effective max-turns", d);
|
||||
Assert.Contains("get_list_config", d);
|
||||
Assert.Contains("set_task_config", d);
|
||||
@@ -119,7 +159,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_checks_file_collisions_before_merge_order()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperExecute);
|
||||
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);
|
||||
@@ -192,7 +232,7 @@ public class PromptFilesTests
|
||||
[Fact]
|
||||
public void DefaultFor_merge_helper_tells_the_session_to_hand_off_after_phase_2()
|
||||
{
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelper);
|
||||
var d = PromptFiles.DefaultFor(PromptKind.MergeHelperTriage);
|
||||
Assert.Contains("handoff_list_handler", d);
|
||||
Assert.Contains("do not continue into phase 3 yourself", d, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
@@ -905,7 +905,9 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
var appendIdx = args.IndexOf("--append-system-prompt-file");
|
||||
var systemPromptPath = args[appendIdx + 1];
|
||||
Assert.Equal(Path.Combine(sessionDir, "system-prompt.md"), systemPromptPath);
|
||||
Assert.Equal(PromptFiles.ReadOrDefault(PromptKind.MergeHelper), File.ReadAllText(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));
|
||||
|
||||
var kickoff = args[^1];
|
||||
var handoffPath = Path.Combine(sessionDir, "handoff.md");
|
||||
@@ -970,6 +972,40 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Equal(_worktreeDir, spec.Cwd);
|
||||
}
|
||||
|
||||
// Regression guard for the bug where both merge-helper builders wrote the SAME system prompt,
|
||||
// so the handoff session received the phase 0-2 (dedupe/enhance) instructions and was told to
|
||||
// ignore them by its brief alone. Each session must carry only its own phases.
|
||||
[Fact]
|
||||
public async Task MergeHelperBuilders_WriteDifferentSystemPrompts_EachScopedToItsOwnPhases()
|
||||
{
|
||||
var repo = Path.Combine(_tempDir, "repoPromptSplit");
|
||||
Directory.CreateDirectory(repo);
|
||||
|
||||
var listId = await SeedListAsync(workingDir: repo, name: "Alpha");
|
||||
var handlerTaskId = Guid.NewGuid().ToString();
|
||||
await SeedTaskAsync(handlerTaskId, listId, TaskStatus.Idle, title: "Handler");
|
||||
var survivor = Guid.NewGuid().ToString();
|
||||
await SeedTaskAsync(survivor, listId, TaskStatus.WaitingForReview, title: "Survivor");
|
||||
|
||||
var svc = BuildService();
|
||||
|
||||
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 executeDir = TrackSessionDir(executeSpec);
|
||||
|
||||
var triagePrompt = File.ReadAllText(Path.Combine(triageDir, "system-prompt.md"));
|
||||
var executePrompt = File.ReadAllText(Path.Combine(executeDir, "system-prompt.md"));
|
||||
|
||||
Assert.NotEqual(triagePrompt, executePrompt);
|
||||
|
||||
Assert.Contains("## Phase 1", triagePrompt);
|
||||
Assert.DoesNotContain("## Phase 4", triagePrompt);
|
||||
|
||||
Assert.Contains("## Phase 4", executePrompt);
|
||||
Assert.DoesNotContain("## Phase 1", executePrompt);
|
||||
}
|
||||
|
||||
// Regression guard for the bug where BuildForMergeHelperHandoffAsync set MCP_TOOL_TIMEOUT to
|
||||
// an older, shorter value (200000) than every other ConPTY spec (930000) after a parallel
|
||||
// merge landed the two changes independently. Every spec this service builds must carry the
|
||||
|
||||
Reference in New Issue
Block a user