Skip to content
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
70 changes: 63 additions & 7 deletions PolyPilot.Tests/SessionPersistenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ActiveSessionEntry> { Entry("new-id", "MyChat") };
var persisted = new List<ActiveSessionEntry> { Entry("old-id", "MyChat") };
var closed = new HashSet<string>();

var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet<string>(), _ => 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]
Expand Down Expand Up @@ -1114,26 +1117,31 @@ public void Merge_RecreatedSessionWithNewId_OverridesOldEntry()

var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet<string>(), _ => 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<ActiveSessionEntry> { Entry("recreated-id", "MySession") };
var persisted = new List<ActiveSessionEntry> { Entry("original-id", "MySession") };
var closed = new HashSet<string>();

var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet<string>(),
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 ---
Expand Down Expand Up @@ -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<ActiveSessionEntry> { Entry("new-id", "MyChat") };
var persisted = new List<ActiveSessionEntry>
{
Entry("old-id-1", "MyChat"),
Entry("old-id-2", "MyChat")
};
var closed = new HashSet<string>();

var result = CopilotService.MergeSessionEntries(active, persisted, closed, new HashSet<string>(), _ => 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<ActiveSessionEntry> { Entry("new-id", "MyChat") };
var persisted = new List<ActiveSessionEntry> { Entry("closed-old", "MyChat") };
var closedIds = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "closed-old" };

var result = CopilotService.MergeSessionEntries(active, persisted, closedIds, new HashSet<string>(), _ => true);

Assert.Single(result);
Assert.Equal("new-id", result[0].SessionId);
}

[Fact]
public void Merge_NameCollision_MissingDirStillExcluded()
{
var active = new List<ActiveSessionEntry> { Entry("new-id", "MyChat") };
var persisted = new List<ActiveSessionEntry> { Entry("old-id", "MyChat") };

var result = CopilotService.MergeSessionEntries(active, persisted, new HashSet<string>(), new HashSet<string>(), _ => false);

Assert.Single(result);
Assert.Equal("new-id", result[0].SessionId);
}
}
28 changes: 28 additions & 0 deletions PolyPilot.Tests/StuckSessionRecoveryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
44 changes: 41 additions & 3 deletions PolyPilot/Services/CopilotService.Persistence.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ private void WriteActiveSessionsFile(List<ActiveSessionEntry> 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.
/// </summary>
internal static List<ActiveSessionEntry> MergeSessionEntries(
List<ActiveSessionEntry> active,
Expand All @@ -169,17 +172,52 @@ internal static List<ActiveSessionEntry> MergeSessionEntries(
// This stops persisted entries from shadowing active sessions after reconnect.
// Persisted entries may still share names with each other.
var activeNames = new HashSet<string>(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<string>(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;
Expand Down
3 changes: 2 additions & 1 deletion PolyPilot/Services/CopilotService.Utilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down