diff --git a/PolyPilot.Tests/ChatExperienceSafetyTests.cs b/PolyPilot.Tests/ChatExperienceSafetyTests.cs index 3a6d70c4a4..0c9d7960aa 100644 --- a/PolyPilot.Tests/ChatExperienceSafetyTests.cs +++ b/PolyPilot.Tests/ChatExperienceSafetyTests.cs @@ -295,6 +295,73 @@ public async Task CompleteResponse_GenerationMatch_Executes() "Normal completion should allow one late TurnStart to recover from premature idle"); } + /// + /// ClearProcessingState must increment ProcessingGeneration so that any InvokeOnUI + /// callback captured before completion sees a generation mismatch and bails out. + /// Without this, the generation guard passes and only the !IsProcessing check + /// prevents resurrection of completed turns — a fragile single-point-of-failure. + /// + [Fact] + public async Task ClearProcessingState_IncrementsGeneration() + { + var svc = CreateService(); + await svc.ReconnectAsync(new ConnectionSettings { Mode = ConnectionMode.Demo }); + var session = await svc.CreateSessionAsync("gen-increment-test"); + + var state = GetSessionState(svc, "gen-increment-test"); + session.IsProcessing = true; + SetField(state, "ProcessingGeneration", 5L); + SetField(state, "SendingFlag", 1); + SetResponseCompletion(state, new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + + var genBefore = GetField(state, "ProcessingGeneration"); + + // Act: CompleteResponse calls ClearProcessingState internally + InvokeCompleteResponse(svc, state, 5L); + + var genAfter = GetField(state, "ProcessingGeneration"); + + Assert.False(session.IsProcessing); + Assert.True(genAfter > genBefore, + $"ClearProcessingState must increment ProcessingGeneration (was {genBefore}, now {genAfter}). " + + "This prevents resurrection of completed turns by stale InvokeOnUI callbacks."); + } + + /// + /// Demonstrates the resurrection scenario: a stale callback that captured the + /// generation before CompleteResponse ran must see a mismatch and bail out, + /// rather than re-setting IsProcessing=true on a completed session. + /// + [Fact] + public async Task GenerationGuard_PreventsResurrection_AfterCompletion() + { + var svc = CreateService(); + await svc.ReconnectAsync(new ConnectionSettings { Mode = ConnectionMode.Demo }); + var session = await svc.CreateSessionAsync("resurrection-test"); + + var state = GetSessionState(svc, "resurrection-test"); + session.IsProcessing = true; + SetField(state, "ProcessingGeneration", 5L); + SetField(state, "SendingFlag", 1); + SetResponseCompletion(state, new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously)); + + // Simulate: stale callback captures generation BEFORE completion + var capturedGen = GetField(state, "ProcessingGeneration"); + Assert.Equal(5L, capturedGen); + + // Turn completes — ClearProcessingState increments generation + InvokeCompleteResponse(svc, state, 5L); + Assert.False(session.IsProcessing); + + // Stale callback fires and checks its captured generation + var currentGen = GetField(state, "ProcessingGeneration"); + var guardPasses = currentGen == capturedGen; + + Assert.False(guardPasses, + $"Stale callback with captured gen={capturedGen} must see mismatch (current={currentGen}). " + + "Without the increment, this would pass and allow resurrection."); + } + /// /// Late TurnStart events should only revive sessions after speculative auto-completion. /// Explicit aborts, watchdog kills, and force-complete recovery paths must not be re-armed diff --git a/PolyPilot/Services/CopilotService.Events.cs b/PolyPilot/Services/CopilotService.Events.cs index 68ac9ddea7..2f224bddaf 100644 --- a/PolyPilot/Services/CopilotService.Events.cs +++ b/PolyPilot/Services/CopilotService.Events.cs @@ -3114,6 +3114,11 @@ private void ClearProcessingStateForRecoveryFailure(SessionState state, string s state.PendingReasoningMessages.Clear(); if (state.Info.ProcessingStartedAt is { } started) state.Info.TotalApiTimeSeconds += (DateTime.UtcNow - started).TotalSeconds; + // Increment generation to invalidate stale InvokeOnUI callbacks — + // mirrors ClearProcessingState (see PR #612). + // Skip if orphaned (long.MaxValue) to avoid overflow. + if (Interlocked.Read(ref state.ProcessingGeneration) != long.MaxValue) + Interlocked.Increment(ref state.ProcessingGeneration); state.Info.IsProcessing = false; state.Info.IsResumed = false; state.HasUsedToolsThisTurn = false; diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index ec5a8beb2c..3ceb11a193 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -786,6 +786,14 @@ private void ClearProcessingState(SessionState state, bool accumulateApiTime = t ClearFlushedReplayDedup(state); state.PendingReasoningMessages.Clear(); + // Increment generation so any InvokeOnUI callback that captured the old + // generation before this ClearProcessingState call will see a mismatch and + // bail out — preventing resurrection of a completed turn. Without this, + // the generation guard passes and only the !IsProcessing check saves us. + // Skip if orphaned (long.MaxValue) — incrementing would overflow to long.MinValue. + if (Interlocked.Read(ref state.ProcessingGeneration) != long.MaxValue) + Interlocked.Increment(ref state.ProcessingGeneration); + state.Info.IsProcessing = false; state.Info.IsResumed = false; state.Info.ProcessingStartedAt = null; @@ -4475,6 +4483,8 @@ await InvokeOnUIAsync(() => state.IsReconnectedSend = false; // INV-1 item 8: prevent stale 35s timeout on next watchdog start Interlocked.Exchange(ref state.SendingFlag, 0); // Clear IsProcessing BEFORE completing TCS (INV-O3) + // No MaxValue guard needed: STEER-ERROR only reaches non-orphaned sessions + Interlocked.Increment(ref state.ProcessingGeneration); // Invalidate stale callbacks state.Info.IsProcessing = false; if (state.Info.ProcessingStartedAt is { } steerStarted) state.Info.TotalApiTimeSeconds += (DateTime.UtcNow - steerStarted).TotalSeconds; @@ -4754,6 +4764,8 @@ public async Task ReloadMcpServersAsync(string sessionName) await InvokeOnUIAsync(() => { FlushCurrentResponse(state); + // No MaxValue guard needed: MCP-reload only reaches non-orphaned sessions + Interlocked.Increment(ref state.ProcessingGeneration); // Invalidate stale callbacks state.Info.IsProcessing = false; state.Info.IsResumed = false; state.HasUsedToolsThisTurn = false;