Skip to content
This repository was archived by the owner on May 24, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions PolyPilot.Tests/ChatExperienceSafetyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,99 @@ public void LazyResumeFallback_AttachesEventHandler()
// F. Race Condition & Edge Case Tests
// =========================================================================

/// <summary>
/// Simulates the sibling reconnect state transfer from CopilotService.cs:
/// old state with IsProcessing=true and flushed content → new state must
/// carry forward FlushedResponse, preserve IsProcessing, and have a fresh TCS.
/// Exercises the same field assignments the reconnect code performs.
/// Verifies PR #600 / issue #599.
/// </summary>
[Fact]
public async Task SiblingReconnect_PreservesProcessingState_OnNewState()
{
var svc = CreateService();
await svc.ReconnectAsync(new ConnectionSettings { Mode = ConnectionMode.Demo });
var session = await svc.CreateSessionAsync("reconnect-preserve");

// --- Set up OLD state as if it was actively processing with flushed content ---
session.IsProcessing = true;
session.IsResumed = true;
session.ProcessingPhase = 3;
session.ProcessingStartedAt = DateTime.UtcNow.AddSeconds(-30);

var oldState = GetSessionState(svc, "reconnect-preserve");
SetField(oldState, "HasUsedToolsThisTurn", true);
SetResponseCompletion(oldState, new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously));
var oldFlushed = GetFlushedResponse(oldState);
oldFlushed.Append("partial tool output from before reconnect");

// --- Capture state (mirrors CopilotService.cs line 3699-3700) ---
var siblingWasProcessing = session.IsProcessing;
var siblingProcessingPhase = session.ProcessingPhase;
Assert.True(siblingWasProcessing, "siblingWasProcessing must be true for this test");

// --- Simulate old TCS cancellation (mirrors line 3762) ---
var oldTcs = GetResponseCompletion(oldState)!;
oldTcs.TrySetCanceled();
Assert.True(oldTcs.Task.IsCanceled, "Old TCS must be canceled on reconnect");

// --- Simulate FlushedResponse carry-forward (mirrors line 3775-3776) ---
// In real code: siblingState.FlushedResponse.Append(capturedOtherState.FlushedResponse)
var carriedContent = oldFlushed.ToString();
Assert.Equal("partial tool output from before reconnect", carriedContent);

// --- Simulate state restoration on new state (mirrors lines 3793-3800) ---
// In real code these are set on siblingState which shares Info with old state
if (siblingWasProcessing)
{
session.IsProcessing = true;
session.IsResumed = true;
session.ProcessingPhase = siblingProcessingPhase > 0 ? siblingProcessingPhase : 3;
}

// --- Verify the full state contract ---
Assert.True(session.IsProcessing, "IsProcessing must be preserved through reconnect");
Assert.True(session.IsResumed, "IsResumed must be set for watchdog tier");
Assert.Equal(3, session.ProcessingPhase);
Assert.NotNull(session.ProcessingStartedAt);
Assert.True(GetField<bool>(oldState, "HasUsedToolsThisTurn"),
"HasUsedToolsThisTurn must stay true for 600s watchdog tier");
Assert.True(carriedContent.Length > 0,
"FlushedResponse must be non-empty for carry-forward");
}

/// <summary>
/// The resurrection guard must prevent re-setting IsProcessing=true after
/// CompleteResponse has already cleared it. Without this guard, a session
/// that completed during reconnect would show stuck "Thinking..." for 600s.
/// Verifies the fix from PR #600 round 3 (commit de8c4d37).
/// </summary>
[Fact]
public async Task SiblingReconnect_ResurrectionGuard_PreventsStuckSession()
{
var svc = CreateService();
await svc.ReconnectAsync(new ConnectionSettings { Mode = ConnectionMode.Demo });
var session = await svc.CreateSessionAsync("resurrection-guard");

var state = GetSessionState(svc, "resurrection-guard");

// Simulate: session was processing, but CompleteResponse ran during
// the reconnect window and cleared IsProcessing to false.
session.IsProcessing = false;
session.IsResumed = false;
session.ProcessingPhase = 0;
SetField(state, "HasUsedToolsThisTurn", false);

// The InvokeOnUI lambda checks: if (!siblingState.Info.IsProcessing) return;
// This prevents resurrection. Verify the guard condition holds:
var siblingWasProcessing = true; // was true when captured BEFORE CompleteResponse ran
var shouldRestore = siblingWasProcessing && session.IsProcessing;
Assert.False(shouldRestore,
"Resurrection guard must prevent restoring IsProcessing when CompleteResponse already ran");
Assert.False(session.IsProcessing,
"IsProcessing must stay false — CompleteResponse completed the turn");
}

/// <summary>
/// Sequential sends don't leave ghost processing state.
/// Each send must complete fully before the next can proceed.
Expand Down
4 changes: 0 additions & 4 deletions PolyPilot.Tests/ConnectionRecoveryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,6 @@ public void SendPromptAsync_SiblingReconnect_SkipsProviderAndVirtualSessions()
Assert.True(skipIndex > loopIndex,
"Sibling reconnect loop must skip provider/virtual sessions before attempting ResumeSessionAsync");

var forceCompleteIndex = source.IndexOf("ForceCompleteProcessingAsync(kvp.Key", loopIndex, StringComparison.Ordinal);
Assert.True(forceCompleteIndex > skipIndex,
"Provider-session skip must appear before ForceCompleteProcessingAsync in the sibling reconnect path");

var resumeIndex = source.IndexOf("ResumeSessionAsync(", loopIndex, StringComparison.Ordinal);
Assert.True(resumeIndex > skipIndex,
"Provider-session skip must appear before ResumeSessionAsync in the sibling reconnect path");
Expand Down
6 changes: 3 additions & 3 deletions PolyPilot.Tests/DiffParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,7 @@ public void PathTraversal_SiblingDirectory_IsBlocked()
// Verify that a sibling directory like "projectEvil" doesn't pass
// StartsWith("C:\project") without trailing separator
var workDir = Path.Combine(Path.GetTempPath(), "testproject");
var siblingPath = "..\\testprojectEvil\\secret.txt";
var siblingPath = Path.Combine("..", "testprojectEvil", "secret.txt");

var filePath = Path.GetFullPath(Path.Combine(workDir, siblingPath));
var normalizedWorkDir = Path.GetFullPath(workDir).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
Expand All @@ -1668,7 +1668,7 @@ public void PathTraversal_SiblingDirectory_IsBlocked()
public void PathTraversal_ValidSubpath_IsAllowed()
{
var workDir = Path.Combine(Path.GetTempPath(), "testproject");
var validPath = "src\\Models\\User.cs";
var validPath = Path.Combine("src", "Models", "User.cs");

var filePath = Path.GetFullPath(Path.Combine(workDir, validPath));
var normalizedWorkDir = Path.GetFullPath(workDir).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
Expand All @@ -1681,7 +1681,7 @@ public void PathTraversal_ValidSubpath_IsAllowed()
public void PathTraversal_DotDotEscape_IsBlocked()
{
var workDir = Path.Combine(Path.GetTempPath(), "testproject");
var escapePath = "..\\..\\etc\\passwd";
var escapePath = Path.Combine("..", "..", "etc", "passwd");

var filePath = Path.GetFullPath(Path.Combine(workDir, escapePath));
var normalizedWorkDir = Path.GetFullPath(workDir).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
Expand Down
30 changes: 21 additions & 9 deletions PolyPilot.Tests/MultiAgentRegressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2057,13 +2057,14 @@ public async Task CancellationToken_PropagatedToWorkerTasks()
}

/// <summary>
/// INV-O14: The re-resume loop must NOT skip IsProcessing siblings. Their
/// CopilotSession is tied to the old client (which was disposed), so the event
/// stream is permanently dead. The loop must force-complete them so the orchestrator
/// retries immediately rather than waiting 2–5 min for the watchdog.
/// The sibling re-resume loop must preserve IsProcessing through reconnect instead of
/// force-completing. Force-completing discards in-flight responses — the CLI continues
/// working but PolyPilot EVT-REARM-SKIPs all subsequent events. Instead, the loop
/// captures siblingWasProcessing, re-resumes on the new client, then restores
/// IsProcessing on the UI thread with guards against resurrection of completed turns.
/// </summary>
[Fact]
public void ReconnectLoop_IsProcessingSiblings_ForceCompletedNotSkipped()
public void ReconnectLoop_IsProcessingSiblings_PreservedNotForceCompleted()
{
var source = File.ReadAllText(Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.cs"));

Expand All @@ -2076,12 +2077,23 @@ public void ReconnectLoop_IsProcessingSiblings_ForceCompletedNotSkipped()
Assert.True(blockEnd > taskRunIdx, "Catch block must follow the re-resume loop");
var loopBlock = source.Substring(taskRunIdx, blockEnd - taskRunIdx);

// INV-O14: must NOT use bare 'continue' on IsProcessing — this was the bug
// Must NOT use bare 'continue' on IsProcessing — that was the original bug
Assert.DoesNotContain("if (otherState.Info.IsProcessing) continue;", loopBlock);

// INV-O14: must call ForceCompleteProcessingAsync for IsProcessing siblings
Assert.Contains("ForceCompleteProcessingAsync", loopBlock);
Assert.Contains("client-recreated-dead-event-stream", loopBlock);
// Must NOT force-complete processing siblings — that kills in-flight responses (PR #599)
Assert.DoesNotContain("ForceCompleteProcessingAsync", loopBlock);

// Must capture siblingWasProcessing to preserve state through re-resume
Assert.Contains("siblingWasProcessing", loopBlock);

// Must start watchdog after re-resume for processing siblings
Assert.Contains("StartProcessingWatchdog(siblingState", loopBlock);

// The InvokeOnUI callback must have all 3 guards to prevent resurrection of
// completed turns (took 3 review rounds to get right):
Assert.Contains("if (siblingState.IsOrphaned) return;", loopBlock);
Assert.Contains("ProcessingGeneration) != reconnectGen) return;", loopBlock);
Assert.Contains("if (!siblingState.Info.IsProcessing) return;", loopBlock);
}

#endregion
Expand Down
4 changes: 2 additions & 2 deletions PolyPilot.Tests/SessionStabilityTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ public void SiblingReResume_RegistersHandler_BeforePublishing()

// Handler must appear BEFORE TryUpdate (register before publishing)
var handlerIdx = sendMethod.IndexOf("HandleSessionEvent(siblingState", StringComparison.Ordinal);
var tryUpdateIdx = sendMethod.IndexOf("TryUpdate", StringComparison.Ordinal);
var tryUpdateIdx = sendMethod.IndexOf("_sessions.TryUpdate(capturedKey, siblingState", StringComparison.Ordinal);
Assert.True(handlerIdx >= 0, "HandleSessionEvent(siblingState must be present in reconnect path");
Assert.True(tryUpdateIdx >= 0, "TryUpdate must be present in reconnect path");
Assert.True(tryUpdateIdx >= 0, "_sessions.TryUpdate must be present in reconnect path");
Assert.True(handlerIdx < tryUpdateIdx,
"Handler registration must happen BEFORE TryUpdate (no window where events arrive with no handler)");
}
Expand Down
Loading