diff --git a/PolyPilot.Tests/ChatDatabaseResilienceTests.cs b/PolyPilot.Tests/ChatDatabaseResilienceTests.cs index 951b54e39a..f8cde82c74 100644 --- a/PolyPilot.Tests/ChatDatabaseResilienceTests.cs +++ b/PolyPilot.Tests/ChatDatabaseResilienceTests.cs @@ -240,44 +240,31 @@ public async Task FireAndForget_AddMessageAsync_DoesNotCauseUnobservedTaskExcept // AddMessageAsync with a broken DB must NOT produce unobserved task exceptions. // Before the fix, AggregateException from SQLite async internals would escape // the narrow catch filter and become unobserved. - var unobservedException = false; - void handler(object? s, UnobservedTaskExceptionEventArgs e) - { - unobservedException = true; - e.SetObserved(); - } - - TaskScheduler.UnobservedTaskException += handler; - try - { - ChatDatabase.SetDbPathForTesting(_impossibleDbPath); - var db = new ChatDatabase(); - db.ResetConnection(); - - var msg = ChatMessage.UserMessage("test"); - - // Fire-and-forget — same pattern as CopilotService.Events.cs - _ = db.AddMessageAsync("session-1", msg); - _ = db.UpdateToolCompleteAsync("session-1", "tool-1", "result", true); - _ = db.UpdateReasoningContentAsync("session-1", "reason-1", "content", true); - - // Give tasks time to complete and finalize — multiple GC cycles - // needed to reliably trigger UnobservedTaskException under load - await Task.Delay(500); - for (int i = 0; i < 3; i++) - { - GC.Collect(2, GCCollectionMode.Forced, blocking: true); - GC.WaitForPendingFinalizers(); - } - await Task.Delay(200); - - Assert.False(unobservedException, - "Fire-and-forget ChatDatabase calls must not produce unobserved task exceptions"); - } - finally - { - TaskScheduler.UnobservedTaskException -= handler; - } + // + // Two-pronged verification: + // 1. Await the tasks — they must complete without throwing (internal catch) + // 2. Verify faulted tasks don't exist (no unobserved exception source) + ChatDatabase.SetDbPathForTesting(_impossibleDbPath); + var db = new ChatDatabase(); + db.ResetConnection(); + + var msg = ChatMessage.UserMessage("test"); + + // Fire-and-forget — same pattern as CopilotService.Events.cs + var t1 = db.AddMessageAsync("session-1", msg); + var t2 = db.UpdateToolCompleteAsync("session-1", "tool-1", "result", true); + var t3 = db.UpdateReasoningContentAsync("session-1", "reason-1", "content", true); + + // All tasks should complete without throwing — internal catch handles errors + await Task.WhenAll(t1, t2, t3); + + // Verify none faulted (faulted tasks are the source of unobserved exceptions) + Assert.False(t1.IsFaulted, "AddMessageAsync should catch internally, not fault"); + Assert.False(t2.IsFaulted, "UpdateToolCompleteAsync should catch internally, not fault"); + Assert.False(t3.IsFaulted, "UpdateReasoningContentAsync should catch internally, not fault"); + + // AddMessageAsync returns -1 on error (not an exception) + Assert.Equal(-1, t1.Result); } // ----------------------------------------------------------------------- diff --git a/PolyPilot.Tests/SessionPersistenceTests.cs b/PolyPilot.Tests/SessionPersistenceTests.cs index a292e92a98..1f70047030 100644 --- a/PolyPilot.Tests/SessionPersistenceTests.cs +++ b/PolyPilot.Tests/SessionPersistenceTests.cs @@ -9,8 +9,8 @@ namespace PolyPilot.Tests; /// public class SessionPersistenceTests { - private static ActiveSessionEntry Entry(string id, string name = "s") => - new() { SessionId = id, DisplayName = name, Model = "m", WorkingDirectory = "/w" }; + private static ActiveSessionEntry Entry(string id, string? name = null) => + new() { SessionId = id, DisplayName = name ?? id, Model = "m", WorkingDirectory = "/w" }; // --- MergeSessionEntries: basic behavior --- @@ -202,6 +202,63 @@ public void Merge_SomeDirsExist_OnlyThoseKept() Assert.DoesNotContain(result, e => e.SessionId == "gone"); } + // --- MergeSessionEntries: display name deduplication --- + + [Fact] + public void Merge_DuplicateDisplayName_ActiveWins_PersistedDropped() + { + // Active session "MyChat" has ID "new-id" (from reconnect). + // Persisted has old entry with stale ID "old-id" but same display name. + // Only the active entry should survive — no ghost duplicates. + var active = new List { Entry("new-id", "MyChat") }; + var persisted = new List { Entry("old-id", "MyChat") }; + var closed = new HashSet(); + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + + Assert.Single(result); + Assert.Equal("new-id", result[0].SessionId); + } + + [Fact] + public void Merge_PersistedEntriesWithSameDisplayName_AllKeptWhenNoActiveNameConflict() + { + // Persisted entries should only be deduped against active session names. + // Legitimate persisted sessions that share a display name are restored and + // collision renaming is handled later in the restore flow. + var active = new List(); + var persisted = new List + { + Entry("ghost-1", "MEssagePierce"), + Entry("ghost-2", "MEssagePierce"), + Entry("ghost-3", "MEssagePierce"), + Entry("real-1", "OtherSession"), + }; + var closed = new HashSet(); + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + + Assert.Equal(4, result.Count); + Assert.Equal(3, result.Count(e => e.DisplayName == "MEssagePierce")); + Assert.Contains(result, e => e.SessionId == "ghost-1"); + Assert.Contains(result, e => e.SessionId == "ghost-2"); + Assert.Contains(result, e => e.SessionId == "ghost-3"); + Assert.Single(result, e => e.DisplayName == "OtherSession"); + } + + [Fact] + public void Merge_ActiveAndPersistedDifferentNames_BothKept() + { + // Entries with different display names should both be kept. + var active = new List { Entry("id-1", "Alpha") }; + var persisted = new List { Entry("id-2", "Beta") }; + var closed = new HashSet(); + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + + Assert.Equal(2, result.Count); + } + // --- MergeSessionEntries: mode switch simulation --- [Fact] @@ -449,4 +506,513 @@ public void Merge_DeletedMultiAgentSessions_InClosedIds_Excluded() Assert.Single(result); Assert.Equal("regular-session", result[0].SessionId); } + + // --- Restore fallback: structural regression guards --- + // These verify the fallback path in RestorePreviousSessionsAsync preserves + // history and usage stats when creating a fresh session (PR #225 regression). + + [Fact] + public void RestoreFallback_LoadsHistoryFromOldSession() + { + // STRUCTURAL REGRESSION GUARD: The "Session not found" fallback in + // RestorePreviousSessionsAsync must call LoadHistoryFromDisk(entry.SessionId) + // before CreateSessionAsync so conversation history is recovered. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0, "Could not find fallback path in RestorePreviousSessionsAsync"); + + // LoadHistoryFromDisk must appear BEFORE CreateSessionAsync in the fallback block + var beforeFallback = source.Substring( + Math.Max(0, fallbackIdx - 500), + Math.Min(500, fallbackIdx)); + Assert.Contains("LoadHistoryFromDisk", beforeFallback); + } + + [Fact] + public void RestoreFallback_InjectsHistoryIntoRecreatedSession() + { + // STRUCTURAL REGRESSION GUARD: After CreateSessionAsync, the fallback must + // inject the recovered history into the new session's Info.History. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); Assert.Contains("History.Add", afterFallback); + Assert.Contains("MessageCount", afterFallback); + Assert.Contains("LastReadMessageCount", afterFallback); + } + + [Fact] + public void RestoreFallback_RestoresUsageStats() + { + // STRUCTURAL REGRESSION GUARD: The fallback must call RestoreUsageStats(entry) + // to preserve token counts, CreatedAt, and other stats from the old session. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + Assert.Contains("RestoreUsageStats", afterFallback); + } + + [Fact] + public void RestoreFallback_SyncsHistoryToDatabase() + { + // STRUCTURAL REGRESSION GUARD: The fallback must sync recovered history + // to the chat database under the new session ID so it persists. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + Assert.Contains("BulkInsertAsync", afterFallback); + } + + [Fact] + public void RestoreFallback_CopiesEventsJsonlToNewSessionDirectory() + { + // STRUCTURAL REGRESSION GUARD: The fallback must copy the old + // events.jsonl into the recreated session directory so a later restart + // can reload history from the new session ID. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + Assert.Contains("events.jsonl", afterFallback); + Assert.Contains("Sanitized copy: only write lines that parse as valid JSON", afterFallback); + Assert.Contains("Copied events.jsonl", afterFallback); + } + + [Fact] + public void RestoreFallback_NormalizesIncompleteToolAndReasoningEntries() + { + // STRUCTURAL REGRESSION GUARD: The fallback must mark stale incomplete + // tool-call and reasoning entries complete, matching ResumeSessionAsync. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + Assert.Contains("ChatMessageType.ToolCall", afterFallback); + Assert.Contains("ChatMessageType.Reasoning", afterFallback); + Assert.Contains("msg.IsComplete = true", afterFallback); + } + + [Fact] + public void RestoreFallback_AddsReconnectionIndicator() + { + // STRUCTURAL REGRESSION GUARD: The fallback must add a system message + // indicating the session was recreated with recovered history, so the + // user knows the session state was reconstructed. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + Assert.Contains("Session recreated", afterFallback); + Assert.Contains("SystemMessage", afterFallback); + } + + [Fact] + public void RestoreFallback_MessageCount_SetAfterSystemMessage() + { + // STRUCTURAL REGRESSION GUARD: MessageCount and LastReadMessageCount must be + // set AFTER the system message is added, not before. Otherwise the system + // message ("🔄 Session recreated") isn't counted, and the unread indicator + // doesn't trigger for it. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var fallbackIdx = source.IndexOf("Falling back to CreateSessionAsync", StringComparison.Ordinal); + Assert.True(fallbackIdx > 0); + + var afterFallback = source.Substring(fallbackIdx, Math.Min(7000, source.Length - fallbackIdx)); + var systemMsgIdx = afterFallback.IndexOf("Session recreated", StringComparison.Ordinal); + var messageCountIdx = afterFallback.IndexOf("MessageCount = recreatedState.Info.History.Count", StringComparison.Ordinal); + + Assert.True(systemMsgIdx > 0, "System message not found in fallback path"); + Assert.True(messageCountIdx > 0, "MessageCount assignment not found in fallback path"); + Assert.True(messageCountIdx > systemMsgIdx, + "MessageCount must be set AFTER the system message is added to History"); + } + + private static string GetRepoRoot() + { + var dir = AppContext.BaseDirectory; + while (dir != null && !File.Exists(Path.Combine(dir, "PolyPilot.slnx"))) + dir = Path.GetDirectoryName(dir); + return dir ?? throw new InvalidOperationException("Could not find repo root"); + } + + // --- RECONNECT handler: structural regression guards --- + // These verify the RECONNECT path in SendPromptAsync persists the new session ID. + // Without this, the debounced save captures a stale pre-reconnect session ID, + // causing the next restore to find an empty directory with no events.jsonl. + + [Fact] + public void Reconnect_CallsSaveActiveSessionsToDisk_AfterUpdatingSessionId() + { + // STRUCTURAL REGRESSION GUARD: After RECONNECT updates state.Info.SessionId + // and _sessions[sessionName] = newState, SaveActiveSessionsToDisk() must be + // called so the new session ID is persisted immediately. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.cs")); + + // Find the specific assignment where the new state replaces the old one + var sessionsAssign = source.IndexOf("_sessions[sessionName] = newState", StringComparison.Ordinal); + Assert.True(sessionsAssign > 0, "Could not find _sessions assignment in RECONNECT handler"); + + // SaveActiveSessionsToDisk must appear within the next 500 chars (before StartProcessingWatchdog) + var afterAssign = source.Substring(sessionsAssign, Math.Min(500, source.Length - sessionsAssign)); + Assert.Contains("SaveActiveSessionsToDisk()", afterAssign); + } + + // --- Restore/save: events.jsonl handling --- + + [Fact] + public void Restore_DoesNotSkipSessionsBeforeFallbackCanHandleMissingEvents() + { + // STRUCTURAL REGRESSION GUARD: RestorePreviousSessionsAsync must not + // short-circuit on missing events.jsonl before the existing fallback path + // can recreate legitimate never-used sessions. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var restoreIdx = source.IndexOf("RestorePreviousSessionsAsync", StringComparison.Ordinal); + Assert.True(restoreIdx > 0); + + Assert.DoesNotContain("Skipping '{entry.DisplayName}' — no events.jsonl", source); + Assert.Contains("Session not found", source); + } + + [Fact] + public void SaveActiveSessionsToDisk_AcceptsEventsOrRecentDirectories() + { + // STRUCTURAL REGRESSION GUARD: The sessionDirExists callback in + // WriteActiveSessionsFile/SaveActiveSessionsToDisk must keep used sessions + // via events.jsonl and also preserve newly created directories briefly. + var source = File.ReadAllText( + Path.Combine(GetRepoRoot(), "PolyPilot", "Services", "CopilotService.Persistence.cs")); + + var mergeCallIdx = source.IndexOf("MergeSessionEntries(entries", StringComparison.Ordinal); + Assert.True(mergeCallIdx > 0); + + var mergeCall = source.Substring(mergeCallIdx, Math.Min(1000, source.Length - mergeCallIdx)); + Assert.Contains("Directory.Exists(dir)", mergeCall); + Assert.Contains("events.jsonl", mergeCall); + Assert.Contains("Directory.GetCreationTimeUtc", mergeCall); + Assert.Contains("TotalMinutes < 5", mergeCall); + } + + // --- Behavioral tests: sessionDirExists with real filesystem --- + // These verify the actual directory/events.jsonl logic that the WriteActiveSessionsFile + // callback implements, using real temp directories instead of source-text assertions. + + [Fact] + public void Merge_WithEventsJsonl_SessionKept() + { + // A persisted session whose directory has events.jsonl should survive merge. + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-{Guid.NewGuid():N}"); + try + { + var sessionDir = Path.Combine(tempBase, "sess-good"); + Directory.CreateDirectory(sessionDir); + File.WriteAllText(Path.Combine(sessionDir, "events.jsonl"), "{}"); + + var active = new List(); + var persisted = new List { Entry("sess-good", "Good") }; + var closed = new HashSet(); + + Func dirExists = id => + { + var dir = Path.Combine(tempBase, id); + if (!Directory.Exists(dir)) return false; + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), dirExists); + Assert.Single(result); + Assert.Equal("sess-good", result[0].SessionId); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } + + [Fact] + public void Merge_WithEmptyDir_RecentlyCreated_SessionKept() + { + // A session directory without events.jsonl but created recently (< 5 min) + // should be kept — it's a brand-new session that hasn't received events yet. + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-{Guid.NewGuid():N}"); + try + { + var sessionDir = Path.Combine(tempBase, "sess-new"); + Directory.CreateDirectory(sessionDir); + // No events.jsonl — simulates a just-created session + + var active = new List(); + var persisted = new List { Entry("sess-new", "New") }; + var closed = new HashSet(); + + Func dirExists = id => + { + var dir = Path.Combine(tempBase, id); + if (!Directory.Exists(dir)) return false; + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), dirExists); + Assert.Single(result); + Assert.Equal("sess-new", result[0].SessionId); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } + + [Fact] + public void Merge_WithEmptyDir_NoEvents_GhostDropped() + { + // A session directory without events.jsonl that is NOT recently created + // should be dropped — it's a ghost from a reconnected session. + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-{Guid.NewGuid():N}"); + try + { + var sessionDir = Path.Combine(tempBase, "sess-ghost"); + Directory.CreateDirectory(sessionDir); + // Backdate the directory creation time to simulate a stale ghost + Directory.SetCreationTimeUtc(sessionDir, DateTime.UtcNow.AddHours(-1)); + + var active = new List(); + var persisted = new List { Entry("sess-ghost", "Ghost") }; + var closed = new HashSet(); + + Func dirExists = id => + { + var dir = Path.Combine(tempBase, id); + if (!Directory.Exists(dir)) return false; + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), dirExists); + Assert.Empty(result); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } + + [Fact] + public void Merge_NoDirectory_SessionDropped() + { + // A persisted session with no directory at all should be dropped. + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempBase); + try + { + var active = new List(); + var persisted = new List { Entry("nonexistent", "Gone") }; + var closed = new HashSet(); + + Func dirExists = id => + { + var dir = Path.Combine(tempBase, id); + if (!Directory.Exists(dir)) return false; + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), dirExists); + Assert.Empty(result); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } + + [Fact] + public void Merge_MixedSessions_OnlyValidOnesKept() + { + // End-to-end scenario: mix of valid, ghost, and missing sessions. + // Only sessions with events.jsonl or recently-created dirs should survive. + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-{Guid.NewGuid():N}"); + try + { + // Session with events.jsonl — kept + var goodDir = Path.Combine(tempBase, "good"); + Directory.CreateDirectory(goodDir); + File.WriteAllText(Path.Combine(goodDir, "events.jsonl"), "{}"); + + // New session (no events, recent dir) — kept + var newDir = Path.Combine(tempBase, "brand-new"); + Directory.CreateDirectory(newDir); + + // Ghost session (empty dir, old) — dropped + var ghostDir = Path.Combine(tempBase, "ghost"); + Directory.CreateDirectory(ghostDir); + Directory.SetCreationTimeUtc(ghostDir, DateTime.UtcNow.AddHours(-2)); + + // "missing" — no directory at all — dropped + + var active = new List(); + var persisted = new List + { + Entry("good", "Good"), + Entry("brand-new", "BrandNew"), + Entry("ghost", "Ghost"), + Entry("missing", "Missing"), + }; + var closed = new HashSet(); + + Func dirExists = id => + { + var dir = Path.Combine(tempBase, id); + if (!Directory.Exists(dir)) return false; + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), dirExists); + Assert.Equal(2, result.Count); + Assert.Contains(result, e => e.SessionId == "good"); + Assert.Contains(result, e => e.SessionId == "brand-new"); + Assert.DoesNotContain(result, e => e.SessionId == "ghost"); + Assert.DoesNotContain(result, e => e.SessionId == "missing"); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } + + // --- Null DisplayName guard --- + + [Fact] + public void Merge_NullDisplayNameInActive_DoesNotThrow() + { + var active = new List + { + new() { SessionId = "a1", DisplayName = null!, Model = "m", WorkingDirectory = "/w" }, + Entry("a2", "Session2"), + }; + var persisted = new List { Entry("p1", "Persisted1") }; + var closed = new HashSet(); + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + Assert.Equal(3, result.Count); + } + + [Fact] + public void Merge_NullDisplayNameInPersisted_DoesNotThrow() + { + var active = new List { Entry("a1", "Session1") }; + var persisted = new List + { + new() { SessionId = "p1", DisplayName = null!, Model = "m", WorkingDirectory = "/w" }, + }; + var closed = new HashSet(); + + // Null display name should not crash; entry kept if dir exists + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + Assert.Equal(2, result.Count); + } + + // --- Sanitized events.jsonl copy --- + + [Fact] + public void WriteActiveSessionsFile_SanitizedCopy_Concept() + { + // Validates that corrupt JSON lines are detectable via JsonDocument.Parse, + // which is the same mechanism used in the sanitized copy during fallback. + var validLine = "{\"type\":\"user.message\",\"data\":{\"content\":\"hello\"}}"; + var corruptLine = "{\"type\":\"user.message\",\"data\":{\"content\":\"broken"; + var emptyLine = ""; + + var lines = new[] { validLine, corruptLine, emptyLine }; + var validLines = new List(); + int skipped = 0; + foreach (var line in lines) + { + if (string.IsNullOrWhiteSpace(line)) continue; + try + { + using var doc = System.Text.Json.JsonDocument.Parse(line); + validLines.Add(line); + } + catch (System.Text.Json.JsonException) { skipped++; } + } + + Assert.Single(validLines); + Assert.Equal(validLine, validLines[0]); + Assert.Equal(1, skipped); + } + + [Fact] + public void SanitizedCopy_WritesOnlyValidJsonLines() + { + var tempBase = Path.Combine(Path.GetTempPath(), $"polypilot-test-sanitize-{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(tempBase); + var sourceDir = Path.Combine(tempBase, "old-session"); + var destDir = Path.Combine(tempBase, "new-session"); + Directory.CreateDirectory(sourceDir); + Directory.CreateDirectory(destDir); + + var sourcePath = Path.Combine(sourceDir, "events.jsonl"); + var destPath = Path.Combine(destDir, "events.jsonl"); + + // Write a mix of valid, corrupt, and empty lines + var lines = new[] + { + "{\"type\":\"session.start\",\"data\":{}}", + "THIS IS CORRUPT", + "{\"type\":\"user.message\",\"data\":{\"content\":\"hello\"}}", + "", + "{\"type\":\"assistant.message\",\"data\":{\"content\":\"world\"}", // missing closing brace + "{\"type\":\"tool.execution_start\",\"data\":{\"toolName\":\"grep\"}}", + }; + File.WriteAllLines(sourcePath, lines); + + // Replicate the sanitized copy logic from CopilotService.Persistence.cs + int validCount = 0, skippedCount = 0; + using (var writer = new StreamWriter(destPath)) + { + foreach (var line in File.ReadLines(sourcePath)) + { + if (string.IsNullOrWhiteSpace(line)) continue; + try + { + using var doc = System.Text.Json.JsonDocument.Parse(line); + writer.WriteLine(line); + validCount++; + } + catch (System.Text.Json.JsonException) { skippedCount++; } + } + } + + Assert.Equal(3, validCount); // session.start, user.message, tool.execution_start + Assert.Equal(2, skippedCount); // "THIS IS CORRUPT" and truncated assistant.message + + var writtenLines = File.ReadAllLines(destPath).Where(l => !string.IsNullOrWhiteSpace(l)).ToArray(); + Assert.Equal(3, writtenLines.Length); + Assert.Contains("session.start", writtenLines[0]); + Assert.Contains("user.message", writtenLines[1]); + Assert.Contains("tool.execution_start", writtenLines[2]); + } + finally { try { Directory.Delete(tempBase, true); } catch { } } + } } diff --git a/PolyPilot/Services/CopilotService.Persistence.cs b/PolyPilot/Services/CopilotService.Persistence.cs index efb9917536..e4838690c6 100644 --- a/PolyPilot/Services/CopilotService.Persistence.cs +++ b/PolyPilot/Services/CopilotService.Persistence.cs @@ -113,7 +113,18 @@ private void WriteActiveSessionsFile(List entries) var closedIds = new HashSet(_closedSessionIds.Keys, StringComparer.OrdinalIgnoreCase); var closedNames = new HashSet(_closedSessionNames.Keys, StringComparer.OrdinalIgnoreCase); entries = MergeSessionEntries(entries, existingEntries, closedIds, closedNames, - sessionId => Directory.Exists(Path.Combine(SessionStatePath, sessionId))); + sessionId => + { + var dir = Path.Combine(SessionStatePath, sessionId); + if (!Directory.Exists(dir)) return false; + // Accept if events.jsonl exists (session has been used) + if (File.Exists(Path.Combine(dir, "events.jsonl"))) return true; + // Also accept recently-created directories (new sessions + // that haven't received their first event yet). Ghost + // directories from old reconnects will be stale. + try { return (DateTime.UtcNow - Directory.GetCreationTimeUtc(dir)).TotalMinutes < 5; } + catch { return false; } + }); } } } @@ -148,10 +159,15 @@ internal static List MergeSessionEntries( { var merged = new List(active); var activeIds = new HashSet(active.Select(e => e.SessionId), StringComparer.OrdinalIgnoreCase); + // Track active display names only. + // This stops persisted entries from shadowing active sessions after reconnect. + // Persisted entries may still share names with each other. + var activeNames = new HashSet(active.Select(e => e.DisplayName).Where(n => n != null), StringComparer.OrdinalIgnoreCase); foreach (var existing in persisted) { if (activeIds.Contains(existing.SessionId)) continue; + if (activeNames.Contains(existing.DisplayName)) continue; if (closedIds.Contains(existing.SessionId)) continue; if (closedNames.Contains(existing.DisplayName)) continue; if (!sessionDirExists(existing.SessionId)) continue; @@ -336,14 +352,6 @@ public async Task RestorePreviousSessionsAsync(CancellationToken cancellationTok continue; } - // Check the session still exists on disk - var sessionDir = Path.Combine(SessionStatePath, entry.SessionId); - if (!Directory.Exists(sessionDir)) - { - Debug($"Skipping '{entry.DisplayName}' — session dir not found: {sessionDir}"); - continue; - } - // Codespace sessions: create placeholder state (client not yet connected). // Health check will resume them after the codespace tunnel is established. // When toggle is off, skip entirely — don't create null-session placeholders. @@ -395,15 +403,91 @@ public async Task RestorePreviousSessionsAsync(CancellationToken cancellationTok // "corrupted" / "session file" errors mean the events.jsonl is locked or // unreadable (e.g., another copilot process owns the session). // Fall back to creating a fresh session so multi-agent workers don't vanish. + // Load history from the old session's events.jsonl so messages aren't lost. if (ex.Message.Contains("Session not found", StringComparison.OrdinalIgnoreCase) || ex.Message.Contains("corrupt", StringComparison.OrdinalIgnoreCase) || ex.Message.Contains("session file", StringComparison.OrdinalIgnoreCase)) { try { - Debug($"Falling back to CreateSessionAsync for '{entry.DisplayName}'"); + // Recover history from the old session before creating a new one + var oldHistory = LoadHistoryFromDisk(entry.SessionId); + Debug($"Falling back to CreateSessionAsync for '{entry.DisplayName}' (recovered {oldHistory.Count} messages from old session)"); + await CreateSessionAsync(entry.DisplayName, entry.Model, entry.WorkingDirectory, cancellationToken, entry.GroupId); - Debug($"Recreated session: {entry.DisplayName}"); + + // Inject recovered history into the newly created session + if (_sessions.TryGetValue(entry.DisplayName, out var recreatedState) && oldHistory.Count > 0) + { + // Copy the old events.jsonl to the new session directory so history + // survives future restarts (LoadHistoryFromDisk reads events.jsonl). + if (recreatedState.Info.SessionId != null && recreatedState.Info.SessionId != entry.SessionId) + { + try + { + var oldEvents = Path.Combine(SessionStatePath, entry.SessionId, "events.jsonl"); + var newEventsDir = Path.Combine(SessionStatePath, recreatedState.Info.SessionId); + var newEvents = Path.Combine(newEventsDir, "events.jsonl"); + if (File.Exists(oldEvents) && !File.Exists(newEvents)) + { + Directory.CreateDirectory(newEventsDir); + // Sanitized copy: only write lines that parse as valid JSON. + // If ResumeSessionAsync failed due to corrupt events.jsonl, + // a raw File.Copy would propagate corruption and cause a + // retry loop on every restart. + int validLines = 0, skippedLines = 0; + using (var writer = new StreamWriter(newEvents)) + { + foreach (var line in File.ReadLines(oldEvents)) + { + if (string.IsNullOrWhiteSpace(line)) continue; + try + { + using var doc = JsonDocument.Parse(line); + writer.WriteLine(line); + validLines++; + } + catch (JsonException) + { + skippedLines++; + } + } + } + if (skippedLines > 0) + Debug($"Sanitized events.jsonl copy from {entry.SessionId} to {recreatedState.Info.SessionId}: {validLines} valid, {skippedLines} corrupt lines skipped"); + else + Debug($"Copied events.jsonl from {entry.SessionId} to {recreatedState.Info.SessionId}: {validLines} lines"); + } + } + catch (Exception copyEx) + { + Debug($"Failed to copy events.jsonl: {copyEx.Message}"); + } + } + + foreach (var msg in oldHistory) + recreatedState.Info.History.Add(msg); + + // Normalize stale incomplete entries (same as ResumeSessionAsync) + foreach (var msg in recreatedState.Info.History.Where(m => + (m.MessageType == ChatMessageType.ToolCall || m.MessageType == ChatMessageType.Reasoning) && !m.IsComplete)) + { + msg.IsComplete = true; + } + + // Sync recovered history to DB under the new session ID + if (recreatedState.Info.SessionId != null) + SafeFireAndForget(_chatDb.BulkInsertAsync(recreatedState.Info.SessionId, oldHistory)); + + recreatedState.Info.History.Add(ChatMessage.SystemMessage("🔄 Session recreated — conversation history recovered from previous session.")); + recreatedState.Info.MessageCount = recreatedState.Info.History.Count; + recreatedState.Info.LastReadMessageCount = recreatedState.Info.History.Count; + } + + // Restore usage stats (token counts, CreatedAt, etc.) + RestoreUsageStats(entry); + + Debug($"Recreated session with {oldHistory.Count} recovered messages: {entry.DisplayName}"); continue; } catch (Exception createEx) diff --git a/PolyPilot/Services/CopilotService.cs b/PolyPilot/Services/CopilotService.cs index d72da3fd34..119e975c77 100644 --- a/PolyPilot/Services/CopilotService.cs +++ b/PolyPilot/Services/CopilotService.cs @@ -2719,6 +2719,13 @@ public async Task SendPromptAsync(string sessionName, string prompt, Lis // 120s watchdog tier instead of the inflated 600s from stale tool state. state.HasUsedToolsThisTurn = false; + // Schedule persistence of the new session ID so it survives app restart. + // Without this, the debounced save captures the pre-reconnect snapshot + // and the stale session ID is written to active-sessions.json. + // Note: This is debounced (2s). If the app crashes within that window, + // the fallback path in RestorePreviousSessionsAsync handles it gracefully. + SaveActiveSessionsToDisk(); + // Start fresh watchdog for the new connection StartProcessingWatchdog(state, sessionName);