Fix: Restore fallback preserves conversation history

[PureWeen]

Problem

When ResumeSessionAsync fails during app restart ("Session not found" / "corrupt" / "session file" errors), the fallback path called CreateSessionAsync which created a blank session with zero messages. The old session's events.jsonl was never loaded, so all conversation history was lost.

User impact: Sessions appeared empty after app relaunch — the user would see their session restored but with 0 messages, forcing them to create duplicate sessions.

Root Cause

Introduced in PR #225 (commit 19219f1) which added the "Session not found" fallback. Worsened in PR #308 which expanded the catch conditions to include "corrupt" and "session file" errors. Neither PR added history recovery to the fallback path.

ResumeSessionAsync (the success path) correctly loads history via LoadHistoryFromDisk(sessionId). The fallback path skipped this entirely.

Fix

In the fallback path of RestorePreviousSessionsAsync:

1. Load history from the old session's events.jsonl via LoadHistoryFromDisk(entry.SessionId) before creating the new session
2. Inject history into the recreated session's Info.History
3. Set message counts (MessageCount, LastReadMessageCount) so the UI shows the correct state
4. Restore usage stats (CreatedAt, token counts) via RestoreUsageStats(entry)
5. Sync to DB via BulkInsertAsync under the new session ID
6. Add indicator system message: "🔄 Session recreated — conversation history recovered from previous session."

Duplicate Session Issue

The user also observed a duplicate session (session-20260311-001729) created with the same repo as the original. This was a downstream consequence: after seeing the original session restored with zero messages, the user manually created a new session. With this fix, the full history is preserved, eliminating the need for duplicates.

Tests

Added 5 structural regression tests in SessionPersistenceTests.cs:

• RestoreFallback_LoadsHistoryFromOldSession — verifies LoadHistoryFromDisk appears before CreateSessionAsync
• RestoreFallback_InjectsHistoryIntoRecreatedSession — verifies history injection + message count
• RestoreFallback_RestoresUsageStats — verifies RestoreUsageStats call
• RestoreFallback_SyncsHistoryToDatabase — verifies BulkInsertAsync call
• RestoreFallback_AddsReconnectionIndicator — verifies system message

All 2464 tests pass.

[PureWeen]

PR #364 Review — Multi-Agent Consensus Report

PR: Fix: Restore fallback preserves conversation history
CI: ⚠️ No checks configured
Prior reviews: None
Models responding: 6 across 2 rounds (3× Sonnet/Gemini/GPT each round; Opus timed out both rounds)

---

Consensus Findings (2+ models agree)

🔴 CRITICAL — Corrupt events.jsonl propagated via File.Copy creates persistent retry loop

File: CopilotService.Persistence.cs:431-434
Models: 5/6 flagged this

When ResumeSessionAsync fails due to "corrupt" events.jsonl, File.Copy(oldEvents, newEvents) copies the entire file — including corruption — to the new session directory. On next restart:

1. active-sessions.json has the NEW session ID
2. Resume tries to load the copied corrupt file → fails again
3. Fallback fires → creates yet another session → copies corrupt file again
4. Each restart produces a new session entry with the same corrupt data

LoadHistoryFromDisk already handles corrupt lines gracefully (line-by-line parsing with try/catch). The copy should use the same approach: read each line, validate it parses as JSON, write only valid lines to the new file.

Fix: Replace File.Copy(oldEvents, newEvents) with a sanitizing copy that writes only valid JSONL lines.

🟡 MODERATE — Null DisplayName in activeNames HashSet → ArgumentNullException

File: CopilotService.Persistence.cs:165
Models: 3/6 flagged this

var activeNames = new HashSet<string>(active.Select(e => e.DisplayName), StringComparer.OrdinalIgnoreCase);

StringComparer.OrdinalIgnoreCase.GetHashCode(null) throws ArgumentNullException. While DisplayName defaults to "", JSON deserialization can set it to null if the JSON contains "DisplayName": null. This crashes MergeSessionEntries during startup — all persisted sessions would be lost.

Fix: active.Select(e => e.DisplayName).Where(n => n != null) or guard the Contains call at line 170.

🟡 MODERATE — MessageCount off-by-one after system message injection

File: CopilotService.Persistence.cs:446-460
Models: 3/6 flagged this

MessageCount and LastReadMessageCount are set at line 446-447 to History.Count, then a system message is added at line 460. The system message is NOT counted, so History.Count > MessageCount. The UI delta (MessageCount - LastReadMessageCount) reads as 0, so the "🔄 Session recreated" message may appear without an unread indicator — minor UX issue but technically a bug.

Fix: Set MessageCount/LastReadMessageCount after the system message, or increment both by 1 after line 460.

🟢 MINOR — SaveActiveSessionsToDisk() debounced, not immediate as comment claims

File: CopilotService.cs:2722-2725
Models: 2/6 flagged this

Comment says "Persist the new session ID immediately" but SaveActiveSessionsToDisk() uses a 2s debounce timer. If the app crashes within that window, the stale session ID is still on disk, causing the fallback path to trigger again on next restart. The debounce is probably fine in practice (the fallback handles it gracefully), but the comment is misleading.

Fix: Either call FlushSaveActiveSessionsToDisk() for a synchronous write, or update the comment to say "schedule persistence."

---

Non-Consensus Findings (noted but excluded)

• History copy skipped when SDK already created events.jsonl (1/6 — Gemini)
• _sessions.TryGetValue race after async CreateSessionAsync (1/6 — Sonnet)
• Old session directory never cleaned up (1/6 — Gemini)

---

Test Coverage Assessment

Tested (structural regression guards):

• ✅ LoadHistoryFromDisk called before CreateSessionAsync
• ✅ History injection + MessageCount set
• ✅ RestoreUsageStats called
• ✅ BulkInsertAsync DB sync
• ✅ File.Copy for events.jsonl
• ✅ Incomplete tool/reasoning normalization
• ✅ Display name dedup in MergeSessionEntries
• ✅ sessionDirExists filesystem behavior

Missing tests:

• ❌ Corrupt events.jsonl propagation (no test verifies corrupt lines aren't copied)
• ❌ Null DisplayName in activeNames HashSet
• ❌ MessageCount after system message injection
• ❌ End-to-end fallback integration (tests are source-text-matching, not behavioral)

---

Verdict: ⚠️ Request Changes

1. [MUST] Sanitize events.jsonl during copy — write only valid JSON lines to new file (prevents restart loop)
2. [MUST] Guard against null DisplayName in activeNames HashSet construction
3. [NICE] Fix MessageCount after system message
4. [NICE] Clarify debounce comment or use synchronous flush

[dinisdopion-sys]

---------- Forwarded message --------- От: Shane Neuville ***@***.***> Date: чт, 12 мар. 2026 г., 23:35 Subject: [PureWeen/PolyPilot] Fix: Restore fallback preserves conversation history (PR #364) To: PureWeen/PolyPilot ***@***.***> Cc: Subscribed ***@***.***> Problem When ResumeSessionAsync fails during app restart ("Session not found" / "corrupt" / "session file" errors), the fallback path called CreateSessionAsync which created a *blank* session with zero messages. The old session's events.jsonl was never loaded, so all conversation history was lost. *User impact:* Sessions appeared empty after app relaunch — the user would see their session restored but with 0 messages, forcing them to create duplicate sessions. Root Cause Introduced in PR #225 <#225> (commit 19219f1) which added the "Session not found" fallback. Worsened in PR #308 <#308> which expanded the catch conditions to include "corrupt" and "session file" errors. Neither PR added history recovery to the fallback path. ResumeSessionAsync (the success path) correctly loads history via LoadHistoryFromDisk(sessionId). The fallback path skipped this entirely. Fix In the fallback path of RestorePreviousSessionsAsync: 1. *Load history* from the old session's events.jsonl via LoadHistoryFromDisk(entry.SessionId) before creating the new session 2. *Inject history* into the recreated session's Info.History 3. *Set message counts* (MessageCount, LastReadMessageCount) so the UI shows the correct state 4. *Restore usage stats* (CreatedAt, token counts) via RestoreUsageStats(entry) 5. *Sync to DB* via BulkInsertAsync under the new session ID 6. *Add indicator* system message: "🔄 Session recreated — conversation history recovered from previous session." Duplicate Session Issue The user also observed a duplicate session (session-20260311-001729) created with the same repo as the original. This was a downstream consequence: after seeing the original session restored with zero messages, the user manually created a new session. With this fix, the full history is preserved, eliminating the need for duplicates. Tests Added 5 structural regression tests in SessionPersistenceTests.cs: - RestoreFallback_LoadsHistoryFromOldSession — verifies LoadHistoryFromDisk appears before CreateSessionAsync - RestoreFallback_InjectsHistoryIntoRecreatedSession — verifies history injection + message count - RestoreFallback_RestoresUsageStats — verifies RestoreUsageStats call - RestoreFallback_SyncsHistoryToDatabase — verifies BulkInsertAsync call - RestoreFallback_AddsReconnectionIndicator — verifies system message All 2464 tests pass. ------------------------------ You can view, comment on, or merge this pull request online at: #364 Commit Summary - 7317b63 <7317b63> Fix: Restore fallback preserves conversation history File Changes (2 files <https://github.com/PureWeen/PolyPilot/pull/364/files>) - *M* PolyPilot.Tests/SessionPersistenceTests.cs <https://github.com/PureWeen/PolyPilot/pull/364/files#diff-82798322386bd26094c2c24f37e9f4f9ec63e27fe627b1d11024ee68c21209fd> (95) - *M* PolyPilot/Services/CopilotService.Persistence.cs <https://github.com/PureWeen/PolyPilot/pull/364/files#diff-c221c32d009ca0183dbee5ee195bb36b4b6c7a53df116c572a8f241dbb3c7db1> (27) Patch Links: - https://github.com/PureWeen/PolyPilot/pull/364.patch - https://github.com/PureWeen/PolyPilot/pull/364.diff — Reply to this email directly, view it on GitHub <#364>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/B4FTARGFXN7NLXGWZ6E3KJT4QMNRXAVCNFSM6AAAAACWQFDIWKVHI2DSMVQWIX3LMV43ASLTON2WKOZUGA3DOMJQG43TKNI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>