feat: use SystemMessageConfig with SectionOverride for worker prompts (fixes #496)

PolyPilot.IntegrationTests/AppBootstrapTests.cs

@@ -0,0 +1,52 @@
+using PolyPilot.IntegrationTests.Fixtures;
+
+namespace PolyPilot.IntegrationTests;
+
+/// <summary>
+/// Smoke tests verifying app bootstrap succeeds after the SystemMessageConfig
+/// refactoring (issue #496). These don't test section overrides directly —
+/// they confirm the refactored CreateSessionAsync doesn't break initialization.
+/// </summary>
+[Collection("PolyPilot")]
+[Trait("Category", "AppBootstrap")]
+public class AppBootstrapTests : IntegrationTestBase
+{
+    public AppBootstrapTests(AppFixture app, ITestOutputHelper output)
+        : base(app, output) { }
+
+    [Fact]
+    public async Task AppBootstrap_DashboardLoads()
+    {
+        await WaitForCdpReadyAsync();
+
+        // Verify the dashboard is accessible — the multi-agent group creation
+        // UI is on the dashboard. If the dashboard loads, the underlying
+        // CreateMultiAgentGroupAsync (which now passes worker system message
+        // sections) is available.
+        var dashboardExists = await WaitForAsync("#dashboard-page", TimeSpan.FromSeconds(10));
+        if (!dashboardExists)
+        {
+            // Try navigating to dashboard
+            await NavigateToAsync("Dashboard", "#dashboard-page");
+            dashboardExists = await ExistsAsync("#dashboard-page");
+        }
+
+        // Even if dashboard element ID isn't present, the app should be responsive
+        var bodyText = await GetTextAsync("body");
+        Assert.False(string.IsNullOrWhiteSpace(bodyText), "App body should have content");
+        Output.WriteLine($"Dashboard content preview: {bodyText[..Math.Min(bodyText.Length, 200)]}");
+
+        await ScreenshotAsync("dashboard-worker-system-message");
+    }
+
+    [Fact]
+    public async Task AppBootstrap_RespondsToApiStatus()
+    {
+        // Verify the app is running and responsive — this confirms that the
+        // refactored CreateSessionAsync (with systemMessageSections parameter)
+        // didn't break app initialization.
+        var status = await GetJsonAsync("/api/status");
+        Assert.True(status.TryGetProperty("agentReady", out var ready));
+        Output.WriteLine($"Agent ready: {ready}");
+    }
+}

PolyPilot.Tests/ConnectionRecoveryTests.cs

@@ -338,6 +338,8 @@ public void SendPromptAsync_FreshSessionConfig_IncludesSystemMessage()
         var helperBlock = source.Substring(helperIdx, Math.Min(3000, source.Length - helperIdx));
         Assert.Contains("SystemMessage = ", helperBlock);
         Assert.Contains("SystemMessageMode.Append", helperBlock);
+        Assert.Contains("SystemMessageMode.Customize", helperBlock);
+        Assert.Contains("BuildWorkerSectionsForSession", helperBlock);
     }
 
     [Fact]

PolyPilot.Tests/WorkerToolHonestyTests.cs

@@ -1,3 +1,4 @@
+using GitHub.Copilot.SDK;
 using Microsoft.Extensions.DependencyInjection;
 using PolyPilot.Models;
 using PolyPilot.Services;
@@ -19,26 +20,195 @@ private CopilotService CreateService()
             new RepoManager(), services.BuildServiceProvider(), new StubDemoService());
     }
 
-    #region Worker Prompt Tool-Honesty Instructions
+    #region Worker Prompt — Task-Only Content
 
     [Fact]
-    public void WorkerPrompt_ContainsToolHonestyInstructions()
+    public void WorkerPrompt_ContainsOnlyTaskContent()
     {
-        var svc = CreateService();
-        var method = typeof(CopilotService).GetMethod("BuildWorkerPrompt",
-            System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static);
-        Assert.NotNull(method);
+        var workerPrompt = CopilotService.BuildWorkerPrompt("Fix the tests", "Run the unit tests");
+
+        Assert.Contains("Original User Request", workerPrompt);
+        Assert.Contains("Fix the tests", workerPrompt);
+        Assert.Contains("Your Assigned Task", workerPrompt);
+        Assert.Contains("Run the unit tests", workerPrompt);
+        // System-level content is now in sections, not the user prompt
+        Assert.DoesNotContain("CRITICAL: Tool Usage & Honesty Policy", workerPrompt);
+        Assert.DoesNotContain("NEVER fabricate", workerPrompt);
+        // No dynamic context when called without optional params
+        Assert.DoesNotContain("Your Role", workerPrompt);
+        Assert.DoesNotContain("Team Context (latest)", workerPrompt);
+        Assert.DoesNotContain("Current Worktree", workerPrompt);
+    }
+
+    [Fact]
+    public void WorkerPrompt_IncludesFreshWorktreeNoteWhenProvided()
+    {
+        var workerPrompt = CopilotService.BuildWorkerPrompt(
+            "Fix the tests", "Run the unit tests",
+            freshWorktreeNote: "\n\n## Your Worktree\nAt `/tmp/wt` (branch: main).\n");
+
+        Assert.Contains("Current Worktree (latest)", workerPrompt);
+        Assert.Contains("/tmp/wt", workerPrompt);
+    }
+
+    [Fact]
+    public void WorkerPrompt_OmitsWorktreeWhenEmpty()
+    {
+        var workerPrompt = CopilotService.BuildWorkerPrompt(
+            "Fix the tests", "Run the unit tests",
+            freshWorktreeNote: "");
+
+        Assert.DoesNotContain("Current Worktree", workerPrompt);
+    }
+
+    [Fact]
+    public void WorkerPrompt_DoesNotDuplicateIdentityOrSharedContext()
+    {
+        // Identity and shared context are delivered via system message sections only.
+        // The user prompt must NOT include them to avoid token waste and conflicting instructions.
+        var workerPrompt = CopilotService.BuildWorkerPrompt("Fix the tests", "Run the unit tests");
+
+        Assert.DoesNotContain("Your Role", workerPrompt);
+        Assert.DoesNotContain("Team Context", workerPrompt);
+    }
 
-        var workerPrompt = (string)method!.Invoke(null, new object[] {
+    #endregion
+
+    #region Worker System Message Sections — Tool Honesty
+
+    [Fact]
+    public void WorkerSystemMessageSections_ContainsToolHonestyInstructions()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
             "You are a worker agent. Complete the following task thoroughly.",
-            "", "", "Fix the tests", "Run the unit tests"
-        })!;
+            worktreeNote: "",
+            sharedContext: "");
+
+        Assert.True(sections.ContainsKey(SystemPromptSections.ToolEfficiency));
+        var toolSection = sections[SystemPromptSections.ToolEfficiency];
+        Assert.Equal(SectionOverrideAction.Append, toolSection.Action);
+        Assert.Contains("CRITICAL: Tool Usage & Honesty Policy", toolSection.Content);
+        Assert.Contains("NEVER fabricate", toolSection.Content);
+        Assert.Contains("TOOL_FAILURE:", toolSection.Content);
+        Assert.Contains("REPORT THE FAILURE", toolSection.Content);
+        Assert.Contains("NEVER evaluate or assess", toolSection.Content);
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_ContainsIdentity()
+    {
+        var charter = "You are a code review specialist.";
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            charter, worktreeNote: "", sharedContext: "");
+
+        Assert.True(sections.ContainsKey(SystemPromptSections.Identity));
+        var identitySection = sections[SystemPromptSections.Identity];
+        Assert.Equal(SectionOverrideAction.Append, identitySection.Action);
+        Assert.Contains(charter, identitySection.Content);
+        Assert.Contains("synthesized with other workers", identitySection.Content);
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_IncludesWorktreeNote()
+    {
+        var worktreeNote = "\n\n## Your Worktree\nYou have an isolated git worktree at `/tmp/wt` (branch: main).\n";
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: worktreeNote, sharedContext: "");
+
+        Assert.True(sections.ContainsKey(SystemPromptSections.EnvironmentContext));
+        var envSection = sections[SystemPromptSections.EnvironmentContext];
+        Assert.Equal(SectionOverrideAction.Append, envSection.Action);
+        Assert.Contains("/tmp/wt", envSection.Content);
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_OmitsWorktreeWhenEmpty()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "", sharedContext: "");
+
+        Assert.False(sections.ContainsKey(SystemPromptSections.EnvironmentContext));
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_IncludesSharedContext()
+    {
+        var sharedContext = "Always use TDD. Run tests before committing.";
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "", sharedContext: sharedContext);
+
+        Assert.True(sections.ContainsKey(SystemPromptSections.CustomInstructions));
+        var customSection = sections[SystemPromptSections.CustomInstructions];
+        Assert.Equal(SectionOverrideAction.Append, customSection.Action);
+        Assert.Contains("Team Context", customSection.Content);
+        Assert.Contains(sharedContext, customSection.Content);
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_OmitsSharedContextWhenEmpty()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "", sharedContext: "");
+
+        Assert.False(sections.ContainsKey(SystemPromptSections.CustomInstructions));
+    }
+
+    [Fact]
+    public void WorkerSystemMessageSections_AllSectionsUseAppendAction()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "You are a specialist.",
+            "\n\n## Your Worktree\nAt /tmp/wt (branch: dev).\n",
+            "Shared team decisions.");
+
+        foreach (var (key, section) in sections)
+        {
+            Assert.Equal(SectionOverrideAction.Append, section.Action);
+        }
+    }
+
+    #endregion
+
+    #region MergeDynamicContentIntoSections
+
+    [Fact]
+    public void MergeDynamicContent_AddsToEnvironmentContext_WhenNoExistingSection()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "", sharedContext: "");
+
+        Assert.False(sections.ContainsKey(SystemPromptSections.EnvironmentContext));
+
+        var merged = CopilotService.MergeDynamicContentIntoSections(sections, "MCP guidance here");
+
+        Assert.True(merged.ContainsKey(SystemPromptSections.EnvironmentContext));
+        Assert.Contains("MCP guidance here", merged[SystemPromptSections.EnvironmentContext].Content);
+    }
+
+    [Fact]
+    public void MergeDynamicContent_MergesWithExistingEnvironmentContext()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "\n\n## Your Worktree\nAt /tmp/wt\n", sharedContext: "");
+
+        Assert.True(sections.ContainsKey(SystemPromptSections.EnvironmentContext));
+
+        var merged = CopilotService.MergeDynamicContentIntoSections(sections, "Relaunch instructions");
+
+        var envContent = merged[SystemPromptSections.EnvironmentContext].Content;
+        Assert.Contains("/tmp/wt", envContent);
+        Assert.Contains("Relaunch instructions", envContent);
+    }
+
+    [Fact]
+    public void MergeDynamicContent_NoOpWhenContentEmpty()
+    {
+        var sections = CopilotService.BuildWorkerSystemMessageSections(
+            "worker", worktreeNote: "", sharedContext: "");
+
+        var merged = CopilotService.MergeDynamicContentIntoSections(sections, "  ");
 
-        Assert.Contains("CRITICAL: Tool Usage & Honesty Policy", workerPrompt);
-        Assert.Contains("NEVER fabricate", workerPrompt);
-        Assert.Contains("TOOL_FAILURE:", workerPrompt);
-        Assert.Contains("REPORT THE FAILURE", workerPrompt);
-        Assert.Contains("NEVER evaluate or assess", workerPrompt);
+        Assert.False(merged.ContainsKey(SystemPromptSections.EnvironmentContext));
     }
 
     #endregion