Simplify built-in presets: keep PR Review Squad + add Implement & Challenge (#225)

## Summary

Simplify built-in multi-agent presets from 6 down to 2:

### Kept
- **📋 PR Review Squad** — Orchestrator mode, 5 specialized reviewers
- **⚔️ Implement & Challenge** (new) — OrchestratorReflect adversarial
loop

### Removed
- Code Review Team (overlapped with PR Review Squad)
- Multi-Perspective Analysis (broadcast mode, niche use case)
- Quick Reflection Cycle (overlapped with Implement & Challenge)
- Deep Research (niche use case)

### Implement & Challenge Preset
Uses OrchestratorReflect mode with two workers:
- **Implementer** (claude-sonnet-4.6): Codes the solution
- **Challenger** (claude-opus-4.6): Reviews, finds issues, suggests
improvements

The loop iterates until the challenger is satisfied
(`[[GROUP_REFLECT_COMPLETE]]`), with stall detection and max 5
iterations (overridable with `--max N`).

Users who need the removed presets can recreate them as custom presets
or via `.squad/` team definitions.

### Test Changes
- Updated all test references from removed presets to remaining ones
- All 1400 tests pass

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: PureWeen

.github/copilot-instructions.md

@@ -35,6 +35,12 @@ Fast Deployment requires `dotnet build -t:Install` — it pushes assemblies to `
 **Package name**: `com.microsoft.PolyPilot` (not `com.companyname.PolyPilot`)  
 **Launch activity**: `crc64ef8e1bf56c865459.MainActivity`
 
+### Windows
+```bash
+dotnet build -f net10.0-windows10.0.19041.0       # Build only
+dotnet run -f net10.0-windows10.0.19041.0          # Build + launch
+```
+
 ### iOS (physical device)
 ```bash
 dotnet build -f net10.0-ios -r ios-arm64         # Build only
@@ -68,7 +74,7 @@ PolyPilot discovers [bradygaster/squad](https://github.com/bradygaster/squad) te
 
 **Squad write-back:** When saving a multi-agent group as a preset, PolyPilot writes the team definition back to `.squad/` format in the worktree root via `SquadWriter`. This creates `team.md`, `agents/{name}/charter.md`, and optional `decisions.md`/`routing.md`. The preset is also saved to `presets.json` as a personal backup. This enables round-tripping: discover → modify → save back → share via repo.
 
-**Preset priority (three-tier merge):** Built-in presets < User presets (`~/.polypilot/presets.json`) < Repo teams (`.squad/`). Repo teams shadow presets with the same name. The preset picker shows three sections: "📂 From Repo", "⚙️ Built-in", and "👤 My Presets".
+**Preset priority (three-tier merge):** Built-in presets are always shown and cannot be overridden. User presets (`~/.polypilot/presets.json`) and repo teams (`.squad/`) with the same name as a built-in are skipped. The preset picker shows three sections: "📂 From Repo" (with delete button), "⚙️ Built-in", and "👤 My Presets".
 
 **Group deletion:** Deleting a multi-agent team closes and removes all its sessions (they're meaningless without the team). Deleting a regular group moves sessions to the default group.
 
@@ -113,6 +119,9 @@ When switching between Embedded and Persistent modes (via Settings → Save & Re
 
 ## Critical Conventions
 
+### Build & Launch
+- **NEVER use `--no-build`** when running the app (`dotnet run`). Always do a full build to catch and fix compile errors before launching. Using `--no-build` can cause silent crashes from stale binaries.
+
 ### Git Workflow
 - **NEVER use `git push --force`** — always use `git push --force-with-lease` instead when a force push is needed (e.g., after a rebase). This prevents overwriting remote changes made by others.
 - **NEVER commit screenshots, images, or binary files** — use `git diff --stat` or `git status` before committing to verify no `.png`, `.jpg`, `.bmp`, or other image files are staged. Screenshots from PolyPilot (e.g., `screenshot_*.png`) are generated locally and must NEVER be committed. The `.gitignore` blocks common patterns, but always double-check.

PolyPilot.Tests/MultiAgentRegressionTests.cs

@@ -1243,7 +1243,7 @@ public void OrchestratorPlanningPrompt_IncludesWorkerPersonas()
         Assert.NotNull(method);
 
         var workers = new List<string> { "sec-worker", "perf-worker" };
-        var result = (string)method!.Invoke(svc, new object?[] { "Review this code", workers, null, null })!;
+        var result = (string)method!.Invoke(svc, new object?[] { "Review this code", workers, null, null, false })!;
 
         Assert.Contains("security auditor", result);
         Assert.Contains("performance optimizer", result);
@@ -1265,12 +1265,12 @@ public void BuiltInPresets_WorkerSystemPrompts_MatchWorkerCount()
     }
 
     /// <summary>
-    /// Code Review Team preset has distinct personas for each worker.
+    /// Implement & Challenge preset has distinct personas for each worker.
     /// </summary>
     [Fact]
-    public void CodeReviewTeam_Preset_HasDistinctPersonas()
+    public void ImplementAndChallenge_Preset_HasDistinctPersonas()
     {
-        var preset = GroupPreset.BuiltIn.First(p => p.Name == "Code Review Team");
+        var preset = GroupPreset.BuiltIn.First(p => p.Name == "Implement & Challenge");
         Assert.NotNull(preset.WorkerSystemPrompts);
         Assert.Equal(2, preset.WorkerSystemPrompts!.Length);
         Assert.All(preset.WorkerSystemPrompts, p => Assert.False(string.IsNullOrWhiteSpace(p)));
@@ -1569,4 +1569,45 @@ public void GetOrchestratorGroupId_ReturnsNull_ForBroadcastMode()
     }
 
     #endregion
+
+    #region Bug #7: FlushedResponse — TurnEnd flush clears CurrentResponse before CompleteResponse reads it
+
+    [Fact]
+    public void ParseTaskAssignments_WithFlushedResponse_ReturnsAssignments()
+    {
+        // Regression: when FlushCurrentResponse runs on TurnEnd before CompleteResponse
+        // on SessionIdle, the orchestrator plan text was lost and ParseTaskAssignments
+        // returned 0 assignments — breaking worker delegation.
+        var workers = new List<string> { "squad-worker-1", "squad-worker-2" };
+        var plan = """
+            I'll assign review tasks to each worker.
+
+            @worker:squad-worker-1
+            Review the authentication module for security issues.
+            @end
+
+            @worker:squad-worker-2
+            Review the database queries for SQL injection.
+            @end
+            """;
+
+        var assignments = CopilotService.ParseTaskAssignments(plan, workers);
+        Assert.Equal(2, assignments.Count);
+        Assert.Equal("squad-worker-1", assignments[0].WorkerName);
+        Assert.Contains("authentication", assignments[0].Task);
+        Assert.Equal("squad-worker-2", assignments[1].WorkerName);
+        Assert.Contains("SQL injection", assignments[1].Task);
+    }
+
+    [Fact]
+    public void ParseTaskAssignments_EmptyResponse_ReturnsNoAssignments()
+    {
+        // Documents the root cause: when plan response is empty string,
+        // no worker assignments can be parsed.
+        var workers = new List<string> { "squad-worker-1" };
+        var assignments = CopilotService.ParseTaskAssignments("", workers);
+        Assert.Empty(assignments);
+    }
+
+    #endregion
 }

PolyPilot.Tests/ReasoningDedupTests.cs

@@ -0,0 +1,102 @@
+using System.Collections.Concurrent;
+using PolyPilot.Models;
+using PolyPilot.Services;
+
+namespace PolyPilot.Tests;
+
+/// <summary>
+/// Tests for the PendingReasoningMessages dedup mechanism that prevents
+/// duplicate reasoning ChatMessages when rapid deltas arrive before
+/// InvokeOnUI fires.
+/// </summary>
+public class ReasoningDedupTests
+{
+    [Fact]
+    public void PendingReasoningMessages_PreventsDuplicateCreation()
+    {
+        // Simulates the race: two rapid deltas for the same reasoningId
+        // arrive before InvokeOnUI adds the first to History.
+        var pending = new ConcurrentDictionary<string, ChatMessage>();
+        var history = new List<ChatMessage>();
+        var reasoningId = "reason-1";
+
+        // First delta: not in pending, not in history → create new
+        var msg1 = pending.GetValueOrDefault(reasoningId)
+            ?? history.LastOrDefault(m => m.MessageType == ChatMessageType.Reasoning && m.ReasoningId == reasoningId);
+
+        Assert.Null(msg1); // nothing found → would create new
+
+        var newMsg = ChatMessage.ReasoningMessage(reasoningId);
+        pending[reasoningId] = newMsg;
+
+        // Second delta: found in pending map → reuse existing
+        var msg2 = pending.GetValueOrDefault(reasoningId)
+            ?? history.LastOrDefault(m => m.MessageType == ChatMessageType.Reasoning && m.ReasoningId == reasoningId);
+
+        Assert.NotNull(msg2);
+        Assert.Same(newMsg, msg2); // Same object, no duplicate
+
+        // Simulate InvokeOnUI completing: move to history, remove from pending
+        history.Add(newMsg);
+        pending.TryRemove(reasoningId, out _);
+
+        // Third delta: now found in history → still no duplicate
+        var msg3 = pending.GetValueOrDefault(reasoningId)
+            ?? history.LastOrDefault(m => m.MessageType == ChatMessageType.Reasoning && m.ReasoningId == reasoningId);
+
+        Assert.NotNull(msg3);
+        Assert.Same(newMsg, msg3);
+        Assert.Single(history); // Only one message ever created
+    }
+
+    [Fact]
+    public void PendingReasoningMessages_DifferentIds_CreateSeparateMessages()
+    {
+        var pending = new ConcurrentDictionary<string, ChatMessage>();
+
+        var msg1 = ChatMessage.ReasoningMessage("reason-1");
+        var msg2 = ChatMessage.ReasoningMessage("reason-2");
+        pending["reason-1"] = msg1;
+        pending["reason-2"] = msg2;
+
+        Assert.Equal(2, pending.Count);
+        Assert.NotSame(pending["reason-1"], pending["reason-2"]);
+    }
+
+    [Fact]
+    public void PendingReasoningMessages_ClearedOnNewTurn()
+    {
+        // Verify that pending map is properly clearable (as done in SendPromptAsync/CompleteResponse)
+        var pending = new ConcurrentDictionary<string, ChatMessage>();
+        pending["reason-1"] = ChatMessage.ReasoningMessage("reason-1");
+        pending["reason-2"] = ChatMessage.ReasoningMessage("reason-2");
+
+        Assert.Equal(2, pending.Count);
+
+        pending.Clear();
+
+        Assert.Empty(pending);
+    }
+
+    [Fact]
+    public void PendingReasoningMessages_ConcurrentAccess_IsThreadSafe()
+    {
+        // ConcurrentDictionary should handle simultaneous reads/writes without corruption
+        var pending = new ConcurrentDictionary<string, ChatMessage>();
+        var reasoningId = "reason-concurrent";
+
+        // Simulate 10 concurrent deltas for the same reasoningId
+        var results = new ConcurrentBag<ChatMessage>();
+        Parallel.For(0, 10, _ =>
+        {
+            var existing = pending.GetOrAdd(reasoningId, _ => ChatMessage.ReasoningMessage(reasoningId));
+            results.Add(existing);
+        });
+
+        // Only one entry in the dictionary
+        Assert.Single(pending);
+        // All 10 callers got the same object reference
+        var first = results.First();
+        Assert.All(results, r => Assert.Same(first, r));
+    }
+}