From 8d537fcfb9bb04fa869f18c9be55cdb16dc22f1f Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Wed, 15 Apr 2026 16:01:14 -0500 Subject: [PATCH 1/5] fix: route orchestrator messages through dispatch even with images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, messages with image attachments sent to orchestrator sessions bypassed SendToMultiAgentGroupAsync entirely, going directly through SendPromptAsync. The orchestrator would produce @worker blocks in its response, but nobody parsed them — workers were never dispatched. Changes: - Dashboard.razor: Always route orchestrator messages through the dispatch pipeline. Images are stripped (pipeline doesn't support forwarding them) with a user-visible warning. This ensures @worker blocks are always parsed and workers are dispatched. - Dashboard.razor: Dispatch errors now logged via LogDispatchError (persisted to event-diagnostics.log) instead of Console.WriteLine (invisible). - CopilotService.Organization.cs: Added LogDispatchError method for persisting dispatch failures to diagnostics log. - CopilotService.Organization.cs: ResumeOrchestrationIfPendingAsync now resets stale ReflectionState (IsActive=false, CompletedAt set) after completing resume synthesis. Without this, StartGroupReflection returns early on the next user prompt because the old IsActive=true persists across app restarts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Pages/Dashboard.razor | 37 ++++++++++++------- .../Services/CopilotService.Organization.cs | 24 ++++++++++++ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/PolyPilot/Components/Pages/Dashboard.razor b/PolyPilot/Components/Pages/Dashboard.razor index 5373f2cc56..790e57308d 100644 --- a/PolyPilot/Components/Pages/Dashboard.razor +++ b/PolyPilot/Components/Pages/Dashboard.razor @@ -1919,8 +1919,23 @@ // Diagnostic: log every send attempt so we can trace routing failures CopilotService.LogDispatchRoute(sessionName, sessionMeta != null, group?.Name, group?.IsMultiAgent, group?.OrchestratorMode, orchSession, isOrchestrator); - if (isOrchestrator && (imagePaths == null || imagePaths.Count == 0)) + if (isOrchestrator) { + // Always route orchestrator messages through the dispatch pipeline, + // even when images are attached. Images are stripped (the pipeline + // doesn't forward them to workers) but the prompt still gets parsed + // for @worker blocks. Without this, image-bearing messages bypass + // dispatch entirely and @worker blocks in the response go nowhere. + if (imagePaths is { Count: > 0 }) + { + var imgSession = sessions.FirstOrDefault(s => s.Name == sessionName); + if (imgSession != null) + { + imgSession.History.Add(ChatMessage.SystemMessage( + "⚠️ Images stripped for orchestration — images are not forwarded to workers. The text prompt is dispatched normally.")); + imgSession.MessageCount = imgSession.History.Count; + } + } AutoStartReflectionIfNeeded(group!.Id, finalPrompt); _ = CopilotService.SendToMultiAgentGroupAsync(group!.Id, finalPrompt).ContinueWith(t => { @@ -1929,24 +1944,20 @@ var msg = t.Exception?.InnerException?.Message ?? t.Exception?.Message ?? "unknown"; InvokeAsync(() => { - Console.WriteLine($"[DISPATCH] Error sending to multi-agent group: {msg}"); + CopilotService.LogDispatchError($"[DISPATCH] Error sending to multi-agent group '{group!.Name}': {msg}"); + var s = sessions.FirstOrDefault(s => s.Name == sessionName); + if (s != null) + { + s.History.Add(ChatMessage.ErrorMessage($"Dispatch failed: {msg}")); + s.MessageCount = s.History.Count; + } + _ = SafeRefreshAsync(); }); } }); } else { - // If this is an orchestrator but images are present, warn the user - if (isOrchestrator && imagePaths is { Count: > 0 }) - { - var imgSession = sessions.FirstOrDefault(s => s.Name == sessionName); - if (imgSession != null) - { - imgSession.History.Add(ChatMessage.SystemMessage( - "⚠️ Images sent directly — orchestration routing does not support images yet.")); - imgSession.MessageCount = imgSession.History.Count; - } - } _ = CopilotService.SendPromptAsync(sessionName, finalPrompt, imagePaths, agentMode: agentMode).ContinueWith(t => { if (t.IsFaulted) diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index b5cb588bb2..abfefde313 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -1646,6 +1646,16 @@ public void LogDispatchRoute(string sessionName, bool hasMeta, string? groupName Debug($"[DISPATCH-ROUTE] session='{sessionName}' hasMeta={hasMeta} group='{groupName}' isMulti={isMulti} mode={mode} orchSession='{orchSession}' isOrch={isOrch}"); } + /// + /// Log dispatch errors to the diagnostics file (not just Console.WriteLine). + /// Called from Dashboard.razor's ContinueWith error handler so dispatch failures + /// are visible in event-diagnostics.log for post-mortem analysis. + /// + public void LogDispatchError(string message) + { + Debug(message); + } + /// /// Returns the group ID if the given session is an orchestrator in an active multi-agent group. /// Used by the message queue drain to route dequeued messages through the dispatch pipeline. @@ -3321,6 +3331,20 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance } ClearPendingOrchestration(); + + // Reset the stale ReflectionState so StartGroupReflection can create + // fresh state on the next user prompt. Without this, the old IsActive=true + // from the killed reflect loop persists across restarts and + // StartGroupReflection returns early without resetting iteration counters. + var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pending.GroupId); + if (resumeGroup?.ReflectionState is { IsActive: true }) + { + resumeGroup.ReflectionState.IsActive = false; + resumeGroup.ReflectionState.CompletedAt = DateTime.Now; + Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {resumeGroup.ReflectionState.CurrentIteration})"); + SaveOrganization(); + } + InvokeOnUI(() => OnOrchestratorPhaseChanged?.Invoke(pending.GroupId, OrchestratorPhase.Complete, null)); } From ddd27f6f7b2977eb2c64a71b79964fe08bf462f5 Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Thu, 16 Apr 2026 10:26:32 -0500 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20address=20PR=20#590=20review=20?= =?UTF-8?q?=E2=80=94=20thread=20safety,=20warning=20text,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap ReflectionState reset in InvokeOnUI to match UI-thread-only convention (fixes threading bug and TOCTOU race) - Update image-strip warning to accurately communicate total image loss - Capture group name as local before async closure (null safety) - Add 2 behavioral tests for ReflectionState reset after resume Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/MultiAgentRegressionTests.cs | 71 +++++++++++++++++++ PolyPilot/Components/Pages/Dashboard.razor | 10 +-- .../Services/CopilotService.Organization.cs | 28 ++++---- 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/PolyPilot.Tests/MultiAgentRegressionTests.cs b/PolyPilot.Tests/MultiAgentRegressionTests.cs index b0514d22b6..532a869e35 100644 --- a/PolyPilot.Tests/MultiAgentRegressionTests.cs +++ b/PolyPilot.Tests/MultiAgentRegressionTests.cs @@ -2933,4 +2933,75 @@ public void CreateGroupFromPresetPayload_CoversAllPresetFields() } #endregion + + #region ReflectionState Reset After Resume (PR #590) + + [Fact] + public void ReflectionState_StaleActiveState_CanBeResetAfterResume() + { + // When PolyPilot restarts mid-reflect-loop, the persisted ReflectionState + // has IsActive=true. After resume completes (MonitorAndSynthesizeAsync), + // the code must reset IsActive=false so StartGroupReflection can create + // fresh state on the next user prompt. This test verifies the model-level + // behavior: an active ReflectionState can be detected and reset. + var group = new SessionGroup + { + Id = Guid.NewGuid().ToString(), + Name = "TestOrchestrator", + IsMultiAgent = true, + OrchestratorMode = MultiAgentMode.OrchestratorReflect, + ReflectionState = ReflectionCycle.Create("Fix all bugs", maxIterations: 10) + }; + + // Simulate mid-loop state (iteration 3, still active) + group.ReflectionState.CurrentIteration = 3; + Assert.True(group.ReflectionState.IsActive); + Assert.Null(group.ReflectionState.CompletedAt); + + // Simulate what ResumeOrchestrationIfPendingAsync does after resume completes: + // detect stale IsActive=true and reset it + if (group.ReflectionState is { IsActive: true }) + { + group.ReflectionState.IsActive = false; + group.ReflectionState.CompletedAt = DateTime.Now; + } + + Assert.False(group.ReflectionState.IsActive); + Assert.NotNull(group.ReflectionState.CompletedAt); + // Iteration count preserved (not reset — that's RetryOrchestration's job) + Assert.Equal(3, group.ReflectionState.CurrentIteration); + } + + [Fact] + public void ReflectionState_AlreadyInactive_NotModifiedOnResume() + { + // If ReflectionState is already inactive (completed normally before restart), + // the resume path should not touch it. + var group = new SessionGroup + { + Id = Guid.NewGuid().ToString(), + Name = "TestOrchestrator", + IsMultiAgent = true, + OrchestratorMode = MultiAgentMode.OrchestratorReflect, + ReflectionState = ReflectionCycle.Create("Fix all bugs", maxIterations: 5) + }; + + group.ReflectionState.IsActive = false; + group.ReflectionState.GoalMet = true; + var originalCompleted = DateTime.Now.AddMinutes(-10); + group.ReflectionState.CompletedAt = originalCompleted; + + // Simulate resume check — should NOT match because IsActive is false + if (group.ReflectionState is { IsActive: true }) + { + group.ReflectionState.IsActive = false; + group.ReflectionState.CompletedAt = DateTime.Now; + } + + // CompletedAt should remain the original value + Assert.Equal(originalCompleted, group.ReflectionState.CompletedAt); + Assert.True(group.ReflectionState.GoalMet); + } + + #endregion } diff --git a/PolyPilot/Components/Pages/Dashboard.razor b/PolyPilot/Components/Pages/Dashboard.razor index 790e57308d..b8cc962ec8 100644 --- a/PolyPilot/Components/Pages/Dashboard.razor +++ b/PolyPilot/Components/Pages/Dashboard.razor @@ -1932,19 +1932,21 @@ if (imgSession != null) { imgSession.History.Add(ChatMessage.SystemMessage( - "⚠️ Images stripped for orchestration — images are not forwarded to workers. The text prompt is dispatched normally.")); + "⚠️ Images dropped — orchestration dispatch does not support image attachments. Only the text prompt is dispatched to the orchestrator and workers.")); imgSession.MessageCount = imgSession.History.Count; } } - AutoStartReflectionIfNeeded(group!.Id, finalPrompt); - _ = CopilotService.SendToMultiAgentGroupAsync(group!.Id, finalPrompt).ContinueWith(t => + var groupId = group!.Id; + var groupName = group.Name; + AutoStartReflectionIfNeeded(groupId, finalPrompt); + _ = CopilotService.SendToMultiAgentGroupAsync(groupId, finalPrompt).ContinueWith(t => { if (t.IsFaulted) { var msg = t.Exception?.InnerException?.Message ?? t.Exception?.Message ?? "unknown"; InvokeAsync(() => { - CopilotService.LogDispatchError($"[DISPATCH] Error sending to multi-agent group '{group!.Name}': {msg}"); + CopilotService.LogDispatchError($"[DISPATCH] Error sending to multi-agent group '{groupName}': {msg}"); var s = sessions.FirstOrDefault(s => s.Name == sessionName); if (s != null) { diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index abfefde313..fcd3182ef1 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -3332,20 +3332,24 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance ClearPendingOrchestration(); - // Reset the stale ReflectionState so StartGroupReflection can create - // fresh state on the next user prompt. Without this, the old IsActive=true - // from the killed reflect loop persists across restarts and + // Reset the stale ReflectionState on the UI thread so StartGroupReflection + // can create fresh state on the next user prompt. Without this, the old + // IsActive=true from the killed reflect loop persists across restarts and // StartGroupReflection returns early without resetting iteration counters. - var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pending.GroupId); - if (resumeGroup?.ReflectionState is { IsActive: true }) + // Must run on UI thread: Organization.Groups is a plain List (not thread-safe). + var pendingGroupId = pending.GroupId; + InvokeOnUI(() => { - resumeGroup.ReflectionState.IsActive = false; - resumeGroup.ReflectionState.CompletedAt = DateTime.Now; - Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {resumeGroup.ReflectionState.CurrentIteration})"); - SaveOrganization(); - } - - InvokeOnUI(() => OnOrchestratorPhaseChanged?.Invoke(pending.GroupId, OrchestratorPhase.Complete, null)); + var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pendingGroupId); + if (resumeGroup?.ReflectionState is { IsActive: true }) + { + resumeGroup.ReflectionState.IsActive = false; + resumeGroup.ReflectionState.CompletedAt = DateTime.Now; + Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {resumeGroup.ReflectionState.CurrentIteration})"); + SaveOrganization(); + } + OnOrchestratorPhaseChanged?.Invoke(pendingGroupId, OrchestratorPhase.Complete, null); + }); } #endregion From de7b7b51adc2bcc7b4c4637ec621d46c199312b7 Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Thu, 16 Apr 2026 11:35:25 -0500 Subject: [PATCH 3/5] fix: close sequential TOCTOU race with StartedAt guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The InvokeOnUI callback could reset a fresh ReflectionState created by a user prompt between ClearPendingOrchestration() and the callback firing. Now captures PendingOrchestration.StartedAt and compares it to the cycle's StartedAt — a fresh cycle (newer timestamp) is left untouched. Added test: ReflectionState_FreshCycleNotResetByStaleResume Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/MultiAgentRegressionTests.cs | 61 ++++++++++++++++--- .../Services/CopilotService.Organization.cs | 13 ++-- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/PolyPilot.Tests/MultiAgentRegressionTests.cs b/PolyPilot.Tests/MultiAgentRegressionTests.cs index 532a869e35..c58d155d79 100644 --- a/PolyPilot.Tests/MultiAgentRegressionTests.cs +++ b/PolyPilot.Tests/MultiAgentRegressionTests.cs @@ -2942,8 +2942,8 @@ public void ReflectionState_StaleActiveState_CanBeResetAfterResume() // When PolyPilot restarts mid-reflect-loop, the persisted ReflectionState // has IsActive=true. After resume completes (MonitorAndSynthesizeAsync), // the code must reset IsActive=false so StartGroupReflection can create - // fresh state on the next user prompt. This test verifies the model-level - // behavior: an active ReflectionState can be detected and reset. + // fresh state on the next user prompt. The StartedAt guard ensures we only + // reset the stale cycle, not a fresh one created by the user. var group = new SessionGroup { Id = Guid.NewGuid().ToString(), @@ -2958,12 +2958,16 @@ public void ReflectionState_StaleActiveState_CanBeResetAfterResume() Assert.True(group.ReflectionState.IsActive); Assert.Null(group.ReflectionState.CompletedAt); + // Capture StartedAt as the production code does (from PendingOrchestration) + var staleStartedAt = group.ReflectionState.StartedAt; + // Simulate what ResumeOrchestrationIfPendingAsync does after resume completes: - // detect stale IsActive=true and reset it - if (group.ReflectionState is { IsActive: true }) + // detect stale IsActive=true and reset it (with StartedAt guard) + if (group.ReflectionState is { IsActive: true } rs + && (staleStartedAt == default || rs.StartedAt <= staleStartedAt)) { - group.ReflectionState.IsActive = false; - group.ReflectionState.CompletedAt = DateTime.Now; + rs.IsActive = false; + rs.CompletedAt = DateTime.Now; } Assert.False(group.ReflectionState.IsActive); @@ -2990,12 +2994,14 @@ public void ReflectionState_AlreadyInactive_NotModifiedOnResume() group.ReflectionState.GoalMet = true; var originalCompleted = DateTime.Now.AddMinutes(-10); group.ReflectionState.CompletedAt = originalCompleted; + var staleStartedAt = group.ReflectionState.StartedAt; // Simulate resume check — should NOT match because IsActive is false - if (group.ReflectionState is { IsActive: true }) + if (group.ReflectionState is { IsActive: true } rs + && (staleStartedAt == default || rs.StartedAt <= staleStartedAt)) { - group.ReflectionState.IsActive = false; - group.ReflectionState.CompletedAt = DateTime.Now; + rs.IsActive = false; + rs.CompletedAt = DateTime.Now; } // CompletedAt should remain the original value @@ -3003,5 +3009,42 @@ public void ReflectionState_AlreadyInactive_NotModifiedOnResume() Assert.True(group.ReflectionState.GoalMet); } + [Fact] + public void ReflectionState_FreshCycleNotResetByStaleResume() + { + // Sequential TOCTOU guard: if the user sends a new prompt (creating a fresh + // ReflectionState) before the queued InvokeOnUI callback fires, the callback + // must NOT reset the fresh cycle. The StartedAt comparison prevents this. + var group = new SessionGroup + { + Id = Guid.NewGuid().ToString(), + Name = "TestOrchestrator", + IsMultiAgent = true, + OrchestratorMode = MultiAgentMode.OrchestratorReflect, + ReflectionState = ReflectionCycle.Create("Original goal", maxIterations: 5) + }; + + // Capture stale StartedAt (simulates PendingOrchestration.StartedAt) + var staleStartedAt = DateTime.Now.AddMinutes(-5); + + // Simulate user sending a new prompt → StartGroupReflection creates fresh state + group.ReflectionState = ReflectionCycle.Create("New goal", maxIterations: 3); + Assert.True(group.ReflectionState.IsActive); + Assert.True(group.ReflectionState.StartedAt > staleStartedAt); + + // Simulate the queued InvokeOnUI callback firing AFTER the fresh cycle was created + if (group.ReflectionState is { IsActive: true } rs + && (staleStartedAt == default || rs.StartedAt <= staleStartedAt)) + { + rs.IsActive = false; + rs.CompletedAt = DateTime.Now; + } + + // Fresh cycle should NOT have been reset + Assert.True(group.ReflectionState.IsActive); + Assert.Null(group.ReflectionState.CompletedAt); + Assert.Equal("New goal", group.ReflectionState.Goal); + } + #endregion } diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index fcd3182ef1..1eee9ff00f 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -3337,15 +3337,20 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance // IsActive=true from the killed reflect loop persists across restarts and // StartGroupReflection returns early without resetting iteration counters. // Must run on UI thread: Organization.Groups is a plain List (not thread-safe). + // Capture StartedAt to guard against sequential TOCTOU: if the user sends a new + // prompt between ClearPendingOrchestration() and the InvokeOnUI callback, + // StartGroupReflection creates a fresh cycle with a different StartedAt. var pendingGroupId = pending.GroupId; + var staleStartedAt = pending.StartedAt; InvokeOnUI(() => { var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pendingGroupId); - if (resumeGroup?.ReflectionState is { IsActive: true }) + if (resumeGroup?.ReflectionState is { IsActive: true } rs + && (staleStartedAt == default || rs.StartedAt == null || rs.StartedAt <= staleStartedAt)) { - resumeGroup.ReflectionState.IsActive = false; - resumeGroup.ReflectionState.CompletedAt = DateTime.Now; - Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {resumeGroup.ReflectionState.CurrentIteration})"); + rs.IsActive = false; + rs.CompletedAt = DateTime.Now; + Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {rs.CurrentIteration})"); SaveOrganization(); } OnOrchestratorPhaseChanged?.Invoke(pendingGroupId, OrchestratorPhase.Complete, null); From 4a00094ff988dabaa7747b448bc1143150da0961 Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Thu, 16 Apr 2026 11:51:47 -0500 Subject: [PATCH 4/5] fix: normalize UTC StartedAt to local time in TOCTOU guard PendingOrchestration.StartedAt uses DateTime.UtcNow while ReflectionCycle.StartedAt uses DateTime.Now. In UTC-negative timezones (all Americas), the raw comparison incorrectly kills fresh cycles. Apply the existing ToLocalTime() pattern from line 3091-3093. Updated test to use DateTime.UtcNow for staleStartedAt to match the production clock source. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot.Tests/MultiAgentRegressionTests.cs | 8 ++++++-- PolyPilot/Services/CopilotService.Organization.cs | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/PolyPilot.Tests/MultiAgentRegressionTests.cs b/PolyPilot.Tests/MultiAgentRegressionTests.cs index c58d155d79..7bc190597d 100644 --- a/PolyPilot.Tests/MultiAgentRegressionTests.cs +++ b/PolyPilot.Tests/MultiAgentRegressionTests.cs @@ -3015,6 +3015,8 @@ public void ReflectionState_FreshCycleNotResetByStaleResume() // Sequential TOCTOU guard: if the user sends a new prompt (creating a fresh // ReflectionState) before the queued InvokeOnUI callback fires, the callback // must NOT reset the fresh cycle. The StartedAt comparison prevents this. + // Uses DateTime.UtcNow to match PendingOrchestration.StartedAt (UTC clock), + // then converts to local to match ReflectionCycle.StartedAt (local clock). var group = new SessionGroup { Id = Guid.NewGuid().ToString(), @@ -3024,8 +3026,10 @@ public void ReflectionState_FreshCycleNotResetByStaleResume() ReflectionState = ReflectionCycle.Create("Original goal", maxIterations: 5) }; - // Capture stale StartedAt (simulates PendingOrchestration.StartedAt) - var staleStartedAt = DateTime.Now.AddMinutes(-5); + // Capture stale StartedAt as UTC (simulates PendingOrchestration.StartedAt) + // then normalize to local time (as the production code does) + var pendingStartedAtUtc = DateTime.UtcNow.AddMinutes(-5); + var staleStartedAt = pendingStartedAtUtc.ToLocalTime(); // Simulate user sending a new prompt → StartGroupReflection creates fresh state group.ReflectionState = ReflectionCycle.Create("New goal", maxIterations: 3); diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 1eee9ff00f..67759ba3ec 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -3341,7 +3341,9 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance // prompt between ClearPendingOrchestration() and the InvokeOnUI callback, // StartGroupReflection creates a fresh cycle with a different StartedAt. var pendingGroupId = pending.GroupId; - var staleStartedAt = pending.StartedAt; + var staleStartedAt = pending.StartedAt.Kind == DateTimeKind.Utc + ? pending.StartedAt.ToLocalTime() + : pending.StartedAt; InvokeOnUI(() => { var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pendingGroupId); From 87035dc101eb40d75f10138d4af2b8e43631ffbb Mon Sep 17 00:00:00 2001 From: "Shane Neuville (HE/HIM)" Date: Thu, 16 Apr 2026 12:12:51 -0500 Subject: [PATCH 5/5] fix: reset ReflectionState on all resume early-exit paths + cleanup temp files Extract ClearPendingOrchestrationAndResetState() helper that combines ClearPendingOrchestration + ReflectionState reset + phase notification. Applied to all 6 ClearPendingOrchestration sites in the resume flow: - Group not found - Orchestrator session not found - Exception handler - Cancellation requested - No worker responses - Happy path (was inline, now uses helper) Also: delete temp image files when images are dropped for orchestrator dispatch, instead of leaking them until OS temp cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- PolyPilot/Components/Pages/Dashboard.razor | 4 ++ .../Services/CopilotService.Organization.cs | 68 +++++++++---------- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/PolyPilot/Components/Pages/Dashboard.razor b/PolyPilot/Components/Pages/Dashboard.razor index b8cc962ec8..6226dbb19e 100644 --- a/PolyPilot/Components/Pages/Dashboard.razor +++ b/PolyPilot/Components/Pages/Dashboard.razor @@ -1935,6 +1935,10 @@ "⚠️ Images dropped — orchestration dispatch does not support image attachments. Only the text prompt is dispatched to the orchestrator and workers.")); imgSession.MessageCount = imgSession.History.Count; } + foreach (var p in imagePaths) + { + try { File.Delete(p); } catch { } + } } var groupId = group!.Id; var groupName = group.Name; diff --git a/PolyPilot/Services/CopilotService.Organization.cs b/PolyPilot/Services/CopilotService.Organization.cs index 67759ba3ec..3cecd31a30 100644 --- a/PolyPilot/Services/CopilotService.Organization.cs +++ b/PolyPilot/Services/CopilotService.Organization.cs @@ -3006,6 +3006,33 @@ private static void ClearPendingOrchestration() internal static PendingOrchestration? LoadPendingOrchestrationForTest() => LoadPendingOrchestration(); internal static void ClearPendingOrchestrationForTest() => ClearPendingOrchestration(); + /// + /// Clear pending orchestration file AND reset any stale ReflectionState on the UI thread. + /// All early-exit and completion paths in the resume flow must call this instead of bare + /// ClearPendingOrchestration() to avoid leaving ReflectionState.IsActive=true persisted. + /// + private void ClearPendingOrchestrationAndResetState(PendingOrchestration pending) + { + ClearPendingOrchestration(); + var pendingGroupId = pending.GroupId; + var staleStartedAt = pending.StartedAt.Kind == DateTimeKind.Utc + ? pending.StartedAt.ToLocalTime() + : pending.StartedAt; + InvokeOnUI(() => + { + var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pendingGroupId); + if (resumeGroup?.ReflectionState is { IsActive: true } rs + && (staleStartedAt == default || rs.StartedAt == null || rs.StartedAt <= staleStartedAt)) + { + rs.IsActive = false; + rs.CompletedAt = DateTime.Now; + Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {rs.CurrentIteration})"); + SaveOrganization(); + } + OnOrchestratorPhaseChanged?.Invoke(pendingGroupId, OrchestratorPhase.Complete, null); + }); + } + /// /// After session restore, check for a pending orchestration dispatch that was interrupted /// by an app relaunch. If found, monitor workers and auto-synthesize when all complete. @@ -3019,7 +3046,7 @@ internal async Task ResumeOrchestrationIfPendingAsync(CancellationToken ct = def if (group == null) { Debug($"[DISPATCH] Pending orchestration group '{pending.GroupId}' no longer exists — clearing"); - ClearPendingOrchestration(); + ClearPendingOrchestrationAndResetState(pending); return; } @@ -3027,7 +3054,7 @@ internal async Task ResumeOrchestrationIfPendingAsync(CancellationToken ct = def if (!_sessions.ContainsKey(pending.OrchestratorName)) { Debug($"[DISPATCH] Pending orchestration orchestrator '{pending.OrchestratorName}' not found — clearing"); - ClearPendingOrchestration(); + ClearPendingOrchestrationAndResetState(pending); return; } @@ -3051,8 +3078,7 @@ internal async Task ResumeOrchestrationIfPendingAsync(CancellationToken ct = def Debug($"[DISPATCH] Resume orchestration failed: {ex.Message}"); AddOrchestratorSystemMessage(pending.OrchestratorName, $"⚠️ Failed to resume orchestration: {ex.Message}"); - ClearPendingOrchestration(); - InvokeOnUI(() => OnOrchestratorPhaseChanged?.Invoke(pending.GroupId, OrchestratorPhase.Complete, null)); + ClearPendingOrchestrationAndResetState(pending); } }); } @@ -3086,8 +3112,7 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance if (ct.IsCancellationRequested) { - ClearPendingOrchestration(); - InvokeOnUI(() => OnOrchestratorPhaseChanged?.Invoke(pending.GroupId, OrchestratorPhase.Complete, null)); + ClearPendingOrchestrationAndResetState(pending); return; } @@ -3241,8 +3266,7 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance { AddOrchestratorSystemMessage(pending.OrchestratorName, "⚠️ No worker responses available after restart — orchestration aborted."); - ClearPendingOrchestration(); - InvokeOnUI(() => OnOrchestratorPhaseChanged?.Invoke(pending.GroupId, OrchestratorPhase.Complete, null)); + ClearPendingOrchestrationAndResetState(pending); return; } @@ -3330,33 +3354,7 @@ private async Task MonitorAndSynthesizeAsync(PendingOrchestration pending, Cance $"⚠️ Failed to send synthesis: {ex.Message}"); } - ClearPendingOrchestration(); - - // Reset the stale ReflectionState on the UI thread so StartGroupReflection - // can create fresh state on the next user prompt. Without this, the old - // IsActive=true from the killed reflect loop persists across restarts and - // StartGroupReflection returns early without resetting iteration counters. - // Must run on UI thread: Organization.Groups is a plain List (not thread-safe). - // Capture StartedAt to guard against sequential TOCTOU: if the user sends a new - // prompt between ClearPendingOrchestration() and the InvokeOnUI callback, - // StartGroupReflection creates a fresh cycle with a different StartedAt. - var pendingGroupId = pending.GroupId; - var staleStartedAt = pending.StartedAt.Kind == DateTimeKind.Utc - ? pending.StartedAt.ToLocalTime() - : pending.StartedAt; - InvokeOnUI(() => - { - var resumeGroup = Organization.Groups.FirstOrDefault(g => g.Id == pendingGroupId); - if (resumeGroup?.ReflectionState is { IsActive: true } rs - && (staleStartedAt == default || rs.StartedAt == null || rs.StartedAt <= staleStartedAt)) - { - rs.IsActive = false; - rs.CompletedAt = DateTime.Now; - Debug($"[DISPATCH] Resume cleared stale ReflectionState for group '{resumeGroup.Name}' (was iteration {rs.CurrentIteration})"); - SaveOrganization(); - } - OnOrchestratorPhaseChanged?.Invoke(pendingGroupId, OrchestratorPhase.Complete, null); - }); + ClearPendingOrchestrationAndResetState(pending); } #endregion