diff --git a/PolyPilot.Tests/SessionPersistenceTests.cs b/PolyPilot.Tests/SessionPersistenceTests.cs index df30ec812..991942ed8 100644 --- a/PolyPilot.Tests/SessionPersistenceTests.cs +++ b/PolyPilot.Tests/SessionPersistenceTests.cs @@ -206,19 +206,22 @@ public void Merge_SomeDirsExist_OnlyThoseKept() // --- MergeSessionEntries: display name deduplication --- [Fact] - public void Merge_DuplicateDisplayName_ActiveWins_PersistedDropped() + public void Merge_DuplicateDisplayName_ActiveWins_PersistedRecoveredWithNewName() { // 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. + // Both entries should survive — the persisted one gets renamed to prevent data loss. 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(2, result.Count); Assert.Equal("new-id", result[0].SessionId); + Assert.Equal("MyChat", result[0].DisplayName); + Assert.Equal("old-id", result[1].SessionId); + Assert.Equal("MyChat (previous)", result[1].DisplayName); } [Fact] @@ -1114,17 +1117,19 @@ public void Merge_RecreatedSessionWithNewId_OverridesOldEntry() var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); - // Only the active (new ID) entry should survive — persisted entry has same display name - Assert.Single(result); + // Active entry keeps original name; persisted entry is recovered with "(previous)" suffix + Assert.Equal(2, result.Count); Assert.Equal("new-id-after-recreate", result[0].SessionId); Assert.Equal("AndroidShellHandler", result[0].DisplayName); + Assert.Equal("old-stale-id", result[1].SessionId); + Assert.Equal("AndroidShellHandler (previous)", result[1].DisplayName); } [Fact] public void Merge_RecreatedSessionNewId_OldIdNotResurrected() { // Even if the old session directory still exists, the persisted entry with a - // matching display name must not be re-added (it has a stale SessionId). + // matching display name is kept under a renamed display name to prevent data loss. var active = new List { Entry("recreated-id", "MySession") }; var persisted = new List { Entry("original-id", "MySession") }; var closed = new HashSet(); @@ -1132,8 +1137,11 @@ public void Merge_RecreatedSessionNewId_OldIdNotResurrected() var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), sessionId => true); // All dirs exist - Assert.Single(result); + Assert.Equal(2, result.Count); Assert.Equal("recreated-id", result[0].SessionId); + Assert.Equal("MySession", result[0].DisplayName); + Assert.Equal("original-id", result[1].SessionId); + Assert.Equal("MySession (previous)", result[1].DisplayName); } // --- Regression: events.jsonl copy when new file already exists --- @@ -1615,4 +1623,52 @@ public void CopyEventsToNewSession_CleansTempFileOnFailure() Assert.Contains("tmpFile = null; // Move succeeded", persistenceFile); Assert.Contains("File.Delete(tmpFile)", persistenceFile); } + + // --- MergeSessionEntries: name collision recovery --- + + [Fact] + public void Merge_NameCollision_MultiplePersisted_AllRecoveredWithUniqueNames() + { + var active = new List { Entry("new-id", "MyChat") }; + var persisted = new List + { + Entry("old-id-1", "MyChat"), + Entry("old-id-2", "MyChat") + }; + var closed = new HashSet(); + + var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet(), _ => true); + + Assert.Equal(3, result.Count); + Assert.Equal("MyChat", result[0].DisplayName); + Assert.Contains(result, e => e.SessionId == "old-id-1"); + Assert.Contains(result, e => e.SessionId == "old-id-2"); + var names = result.Select(e => e.DisplayName).ToList(); + Assert.Equal(names.Count, names.Distinct(StringComparer.OrdinalIgnoreCase).Count()); + } + + [Fact] + public void Merge_NameCollision_ClosedPersistedStillExcluded() + { + var active = new List { Entry("new-id", "MyChat") }; + var persisted = new List { Entry("closed-old", "MyChat") }; + var closedIds = new HashSet(StringComparer.OrdinalIgnoreCase) { "closed-old" }; + + var result = CopilotService.MergeSessionEntries(active, persisted, closedIds, new HashSet(), _ => true); + + Assert.Single(result); + Assert.Equal("new-id", result[0].SessionId); + } + + [Fact] + public void Merge_NameCollision_MissingDirStillExcluded() + { + var active = new List { Entry("new-id", "MyChat") }; + var persisted = new List { Entry("old-id", "MyChat") }; + + var result = CopilotService.MergeSessionEntries(active, persisted, new HashSet(), new HashSet(), _ => false); + + Assert.Single(result); + Assert.Equal("new-id", result[0].SessionId); + } } diff --git a/PolyPilot.Tests/StuckSessionRecoveryTests.cs b/PolyPilot.Tests/StuckSessionRecoveryTests.cs index c3508d1bc..52fb2d1e0 100644 --- a/PolyPilot.Tests/StuckSessionRecoveryTests.cs +++ b/PolyPilot.Tests/StuckSessionRecoveryTests.cs @@ -117,6 +117,34 @@ public void IsSessionStillProcessing_RecentFile_IdleEvent_ReturnsFalse() } } + [Fact] + public void IsSessionStillProcessing_RecentFile_AbortEvent_ReturnsFalse() + { + var svc = CreateService(); + var tmpDir = Path.Combine(Path.GetTempPath(), "polypilot-test-" + Guid.NewGuid().ToString("N")); + var sessionId = Guid.NewGuid().ToString(); + var sessionDir = Path.Combine(tmpDir, sessionId); + Directory.CreateDirectory(sessionDir); + var eventsFile = Path.Combine(sessionDir, "events.jsonl"); + + try + { + File.WriteAllText(eventsFile, + """{"type":"session.resume","data":{}}""" + "\n" + + """{"type":"user.message","data":{"content":"test"}}""" + "\n" + + """{"type":"assistant.turn_start","data":{}}""" + "\n" + + """{"type":"abort","data":{"reason":"user initiated"}}"""); + + var result = svc.IsSessionStillProcessing(sessionId, tmpDir); + + Assert.False(result, "events.jsonl ending with abort should not report still processing — user explicitly cancelled"); + } + finally + { + Directory.Delete(tmpDir, true); + } + } + [Fact] public void IsSessionStillProcessing_MissingFile_ReturnsFalse() { diff --git a/PolyPilot/Services/CopilotService.Persistence.cs b/PolyPilot/Services/CopilotService.Persistence.cs index 18bde439a..99cd8e078 100644 --- a/PolyPilot/Services/CopilotService.Persistence.cs +++ b/PolyPilot/Services/CopilotService.Persistence.cs @@ -155,6 +155,9 @@ private void WriteActiveSessionsFile(List entries) /// Merge active (in-memory) session entries with persisted (on-disk) entries. /// Persisted entries are kept if they aren't already active, weren't explicitly /// closed (by ID or display name), and their session directory still exists. + /// When a persisted entry has a name collision with an active entry that has a + /// DIFFERENT session ID, the persisted entry is kept under a renamed display + /// name to prevent data loss from session replacement. /// internal static List MergeSessionEntries( List active, @@ -169,17 +172,52 @@ internal static List MergeSessionEntries( // 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); + // Track all merged display names to avoid collisions when renaming recovered entries + var allMergedNames = new HashSet(activeNames, 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; - merged.Add(existing); - activeIds.Add(existing.SessionId); + // Name collision: an active session has the same display name but a different + // session ID. This happens when a session is replaced (reconnect, lazy-resume + // fallback, etc.). Instead of silently dropping the persisted entry (losing its + // history), keep it under a unique name so the user can find it. + // NOTE: We clone the entry to avoid mutating the caller's persisted list. + var entryToAdd = existing; + if (activeNames.Contains(existing.DisplayName)) + { + entryToAdd = new ActiveSessionEntry + { + SessionId = existing.SessionId, + DisplayName = existing.DisplayName, + Model = existing.Model, + ReasoningEffort = existing.ReasoningEffort, + WorkingDirectory = existing.WorkingDirectory, + LastPrompt = existing.LastPrompt, + GroupId = existing.GroupId, + TotalInputTokens = existing.TotalInputTokens, + TotalOutputTokens = existing.TotalOutputTokens, + ContextCurrentTokens = existing.ContextCurrentTokens, + ContextTokenLimit = existing.ContextTokenLimit, + PremiumRequestsUsed = existing.PremiumRequestsUsed, + TotalApiTimeSeconds = existing.TotalApiTimeSeconds, + CreatedAt = existing.CreatedAt, + LastUpdatedAt = existing.LastUpdatedAt, + }; + var recoveredName = $"{existing.DisplayName} (previous)"; + var suffix = 2; + while (allMergedNames.Contains(recoveredName)) + recoveredName = $"{existing.DisplayName} (previous {suffix++})"; + entryToAdd.DisplayName = recoveredName; + } + + merged.Add(entryToAdd); + activeIds.Add(entryToAdd.SessionId); + allMergedNames.Add(entryToAdd.DisplayName); } return merged; diff --git a/PolyPilot/Services/CopilotService.Utilities.cs b/PolyPilot/Services/CopilotService.Utilities.cs index 181f8e666..749857377 100644 --- a/PolyPilot/Services/CopilotService.Utilities.cs +++ b/PolyPilot/Services/CopilotService.Utilities.cs @@ -138,7 +138,8 @@ internal bool IsSessionStillProcessing(string sessionId, string basePath) // session.idle is ephemeral (never on disk). session.start can also be the only // on-disk event for a never-used session, so treat it as terminal on restore. - var terminalEvents = new[] { "session.idle", "session.error", "session.shutdown", "session.start" }; + // "abort" means the user explicitly cancelled — the session is definitively not processing. + var terminalEvents = new[] { "session.idle", "session.error", "session.shutdown", "session.start", "abort" }; if (terminalEvents.Contains(type)) return false; // Smart completion for assistant.message / assistant.turn_end: session.idle is not