fix: session stability hardening

[PureWeen]

Problem

Sessions die instantly after reconnect due to compounding bugs:

1. Corrupted session files kill sessions permanently - When events.jsonl contains tool output with words like "ephemeral", the CLI JSON parser chokes on session.resume. No catch existed, so the session dies.
2. Orphaned event handlers race with new state - Stale SessionIdleEvent callbacks from the old session clear IsProcessing on the shared Info object, killing the new session. Each reconnect stacks another handler.
3. Watchdog crashes leave sessions permanently stuck - If watchdog cleanup throws, IsProcessing was never cleared.
4. posix_spawn failures not detected - Shell failures did not trigger auto-recovery.
5. relaunch.sh kills copilot server - Grep pattern matched the bundled copilot binary too.

Fixes

• Corrupted session file recovery: Catch "corrupted" in resume, create fresh session
• IsOrphaned flag: HandleSessionEvent and CompleteResponse skip orphaned states
• Generation invalidation: Old state ProcessingGeneration set to long.MaxValue
• Watchdog crash safety: Outer catch clears IsProcessing (INV-1 compliant)
• Shell failure detection: posix_spawn treated like permission denial for recovery
• relaunch.sh: End-of-line anchor to only match app binary

Testing

2549 tests pass, 5 pre-existing failures (font scaling + DevTunnel - unrelated)

[PureWeen]

PR Review: fix/session-stability-hardening

Multi-model review (4/5 models completed; consensus = flagged by 2+ models)

CI Status

⚠️ No CI checks reported on this branch. Author reports 2549 tests pass locally with 5 pre-existing failures (font scaling + DevTunnel — unrelated). Tests should be verified to pass on a clean CI run.

---

Findings (Consensus, Ranked by Severity)

🔴 CRITICAL — Sibling Re-Resume TOCTOU Race

File: PolyPilot/Services/CopilotService.cs ~line 2646
Flagged by: 4/4 models

if (otherState.Info.IsProcessing) continue;  // check on background thread
// ... await ResumeSessionAsync(...)         // async gap — sibling can start processing here
otherState.ResponseCompletion?.TrySetCanceled();  // may cancel a live TCS

IsProcessing is a plain bool (not volatile) on AgentSessionInfo, written on the UI thread. The check runs on a Task.Run background thread. If a concurrent SendPromptAsync starts between the check and TrySetCanceled(), the active ResponseCompletion TCS is forcibly canceled — throwing TaskCanceledException to any orchestrator worker awaiting it and silently dropping the mid-turn response. Multi-agent orchestration makes this race more likely (workers dispatch sends concurrently).

Suggested fix: After ResumeSessionAsync returns, re-check otherState.Info.IsProcessing before proceeding; if it's now true, skip orphaning and mark siblingState.IsOrphaned = true immediately:

var resumed = await newClient.ResumeSessionAsync(...);
if (otherState.Info.IsProcessing)  // re-check after await
{
    siblingState.IsOrphaned = true;  // discard the just-resumed session
    continue;
}

---

🟡 MODERATE — Sibling ResumeSessionConfig Missing MCP Servers

File: PolyPilot/Services/CopilotService.cs ~line 2655
Flagged by: 3/4 models

The sibling re-resume only registers ShowImageTool:

var cfg = new ResumeSessionConfig
{
    Tools = new List<AIFunction> { ShowImageTool.CreateFunction() },
    OnPermissionRequest = AutoApprovePermissions,
};

The primary reconnect and corrupted-session path both use BuildFreshSessionConfig which includes MCP servers, skill directories, and system messages. Any sibling session that had MCP tools configured will silently lose them after re-resume. Tool calls will fail with confusing errors until the user manually reconnects. Should use BuildFreshSessionConfig(otherState) (or equivalent) for sibling configs.

---

🟡 MODERATE — Watchdog Crash Recovery Loses Un-Flushed Content

File: PolyPilot/Services/CopilotService.Events.cs ~line 2194
Flagged by: 3/4 models

var crashResponse = state.FlushedResponse.ToString();  // only flushed content
state.FlushedResponse.Clear();
state.CurrentResponse.Clear();  // ← streaming content silently discarded
state.ResponseCompletion?.TrySetResult(crashResponse);  // missing CurrentResponse

CurrentResponse holds streaming deltas received since the last FlushCurrentResponse call. The crash path clears it without concatenating it into crashResponse. This is inconsistent with TriggerToolHealthRecovery which correctly combines both. The fix is:

var crashResponse = state.FlushedResponse.ToString() + state.CurrentResponse.ToString();

---

🟡 MODERATE — Overly Broad Corrupted-Session Exception Filter

File: PolyPilot/Services/CopilotService.cs ~line 2772
Flagged by: 3/4 models

catch (Exception resumeEx) when (
    resumeEx.Message.Contains("corrupted", StringComparison.OrdinalIgnoreCase) ||
    resumeEx.Message.Contains("session file", StringComparison.OrdinalIgnoreCase))

"session file" is a very broad substring. Transient errors containing this phrase (e.g., "Cannot read session file: permission denied", "session file locked") will be misidentified as corruption and trigger destructive fresh-session creation — losing the user's entire session history when a retry would have worked. A more specific pattern (e.g., "session file is corrupted", "invalid json", "line N") would reduce false positives.

---

🟢 MINOR — Indentation Anomaly in TriggerToolHealthRecovery

File: PolyPilot/Services/CopilotService.Events.cs ~line 1679
Flagged by: 3/4 models

The InvokeOnUI(() => call has ~24 extra leading spaces compared to surrounding code. C# semantics are unaffected (indentation-insensitive), but it's real source corruption (not a diff artifact) that breaks code navigation and will produce confusing merge conflicts.

---

Test Coverage Assessment

New code with tests: ✅ IsShellEnvironmentFailure (PermissionDenialDetectionTests.cs), ✅ watchdog crash recovery source-inspection (ProcessingWatchdogTests.cs)

Missing test coverage for new code paths:

• ❌ No test for sibling re-resume race condition — the TOCTOU scenario (sibling starts processing while re-resume is in flight) has no test case
• ❌ No test that sibling retains MCP tool registrations after re-resume — would catch the missing BuildFreshSessionConfig issue
• ❌ No test for corrupted session recovery end-to-end — the when filter behavior is source-inspected but not exercised with a real exception message containing "session file" without "corrupted"
• ❌ No test for CurrentResponse content in watchdog crash response — the existing WatchdogCrashRecovery_ClearsCompanionFields test doesn't verify the response payload includes streaming content

---

Recommended Action

⚠️ Request Changes

The sibling re-resume logic (the key new feature) has a real TOCTOU race that can cancel in-flight orchestrator workers. The missing MCP servers will silently break sessions that rely on plugins after any reconnect. These are both straightforward to fix. The remaining items are lower risk but also straightforward.

Specific asks:

1. Re-check IsProcessing after ResumeSessionAsync returns to close the TOCTOU window
2. Use BuildFreshSessionConfig (or equivalent) for sibling ResumeSessionConfig
3. Append CurrentResponse to crashResponse in watchdog crash handler
4. Tighten the "session file" exception filter to a more specific pattern

[PureWeen]

Re-Review: fix/session-stability-hardening

Multi-model re-review (5/5 models) — updated after 5 additional commits

Previous Findings Status

#	Finding	Status
1	🔴 TOCTOU race in sibling re-resume	✅ FIXED (post-await re-check added)
2	🟡 Missing MCP servers in sibling config	✅ FIXED (LoadMcpServers + LoadSkillDirectories added)
3	🟡 Watchdog crash loses CurrentResponse	✅ FIXED (crashResponse = FlushedResponse + CurrentResponse)
4	🟡 Broad "session file" exception filter	✅ FIXED (tightened to "session file is" + "Invalid literal value")
5	🟢 Indentation corruption in TriggerToolHealthRecovery	❌ STILL PRESENT

---

New Finding (Consensus: 3/5 models)

🟡 MODERATE — Leaked in sibling re-resume abandon paths

File: (sibling loop, ~lines 2672 and 2703)

Two paths in the sibling re-resume loop abandon a live without disposing it:

Path 1 — post-await TOCTOU guard:

var resumed = await newClient.ResumeSessionAsync(otherState.Info.SessionId, cfg, CancellationToken.None);
if (otherState.Info.IsProcessing)
{
    Debug($"[RECONNECT] Sibling started processing during re-resume — skipping");
    continue;  // ← 'resumed' is never disposed
}

Path 2 — failure (rapid back-to-back reconnects):

{
    siblingState.IsOrphaned = true;
    continue;  // ← 'resumed' is never disposed
}

In both cases, has already created a server-side session with an open transport (stdio pipe or TCP connection to the copilot CLI process). Without , these handles accumulate on rapid reconnects or in multi-session workspaces. Under flapping network conditions with multiple sessions, this will exhaust file descriptors or produce stale server-side sessions.

Fix:

// Path 1
if (otherState.Info.IsProcessing)
{
    Debug($"[RECONNECT] Sibling started processing during re-resume — skipping");
    try { await resumed.DisposeAsync(); } catch { }
    continue;
}

// Path 2
{
    siblingState.IsOrphaned = true;
    try { await resumed.DisposeAsync(); } catch { }
    continue;
}

---

Remaining Minor Issue

** indentation** () — the call still has 24 extra leading spaces in the updated diff. Functionally harmless in C# but creates misleading visual structure.

---

Recommended Action

⚠️ Request changes (one remaining item)

All four major findings from the initial review are fixed. One new 🟡 MODERATE issue: the two CopilotSession leak paths need . This is a straightforward two-line fix. Once addressed the PR is ready to merge.

[PureWeen]

Round 7 Review — Parallel 3-Agent Deep Dive

Agents: Agent-8 (sibling re-resume), Agent-9 (Events.cs INV-1), Agent-10 (reconnect paths)

Triage Summary

#	Agent	Finding	Verdict	Reasoning
1	Agent-9	CompleteResponse missing FlushCurrentResponse	False positive	CompleteResponse does the flush work inline (lines 968-1002): reads CurrentResponse→adds to history→builds fullResponse→clears. It IS the flush.
2	Agent-9	TriggerToolHealthRecovery missing FlushCurrentResponse	False positive	Also reads both buffers inline and clears them (lines 1699-1708). Documented as "by design" in processing-state-safety skill.
3	Agent-8	Missing ResponseCompletion init on sibling state	False positive	Siblings are idle (IsProcessing=true siblings are skipped). TCS is created by SendPromptAsync when next prompt arrives. Not needed for idle sessions.
4	Agent-8	Missing ProcessingGeneration increment on sibling state	False positive	Old state has long.MaxValue (blocks stale callbacks). New state at 0 is correct — identical to any fresh session. Primary path increments because it's mid-turn.
5	Agent-8	Missing CurrentResponse/FlushedResponse/PendingReasoningMessages clear	False positive	New SessionState object has fresh StringBuilder instances — already empty by construction.
6	Agent-10	Missing DisposeAsync of old session in primary reconnect	Won't fix (risky)	CopilotSession.DisposeAsync may send a close command to the server, breaking the resumed session (same session ID).
7	Agent-10	Log says "old session disposed" but no disposal happens	✅ Fixed	Removed misleading text from log message (commit f2c337e).

Fix Applied

• f2c337e — Corrected misleading "old session disposed" debug log message. The old CopilotSession is intentionally NOT disposed because DisposeAsync could send a server-side close that breaks the resumed session sharing the same session ID.

Status

• 2554/2554 tests passing
• 12 commits on the PR (all incremental, preserving full review history)
• 7 rounds of review (4 external multi-model consensus + 3 from this session)
• All critical and moderate findings across all rounds addressed

The PR is ready for merge. 🚀