fix: rescue stuck chat sessions + 41 behavioral safety tests

[PureWeen]

Summary

Rescues chat sessions that get stuck for 600s waiting for a SessionIdleEvent that never arrives (SDK bug #299 / network hiccup). Adds 41 behavioral safety tests exercising the real CopilotService pipeline via reflection.

Root-cause bugs fixed

1. TurnEnd fallback permanently disabled for tool sessions: When HasUsedToolsThisTurn was true, the TurnEnd→Idle 4s fallback returned early with a skip log — meaning tool sessions had zero fallback if SessionIdleEvent was lost. The session would sit stuck until the 600s watchdog killed it with an error message, discarding the (complete) response.

2. Watchdog couldn't distinguish active tools from lost SDK events: The watchdog treated all elapsed >= effectiveTimeout the same — kill with error. But there are three distinct situations:

   • Tool actively running (ActiveToolCallCount > 0, server alive): should reset timer, not kill
   • Tool finished, SessionIdleEvent lost (ActiveToolCallCount = 0, HasUsedToolsThisTurn = true): should complete cleanly (response IS done)
   • Genuinely stuck / server dead: should kill with error (existing behavior)

Four fixes

1. TurnEnd 30s fallback for tool sessions

Instead of skipping the fallback entirely when tools were used, wait an additional 30s (TurnEndIdleToolFallbackAdditionalMs). If no TurnStart arrives in that window, SessionIdleEvent was lost → fire CompleteResponse. Total wait for tool sessions = 4s + 30s = 34s.

2. Watchdog 3-way branch

When elapsed >= effectiveTimeout and max time not exceeded:

• Case A (active tool + server alive): reset LastEventAtTicks, continue loop
• Case B (no active tool + tools were used): CompleteResponse cleanly (no error message)
• Case C (max time exceeded / server dead / demo): kill with error (unchanged)

3. Periodic watchdog flush

Every watchdog check interval, if CurrentResponse has accumulated content, flush it to History. This ensures partial responses are visible in the chat even while IsProcessing=true.

4. ExternalToolRequestedEvent in EventMatrix

Classified as TimelineOnly to prevent "Unhandled" log spam while still allowing LastEventAtTicks updates.

The stuck session scenario this resolves

Before this fix: User sends a prompt that triggers tool use → tools execute → LLM responds → AssistantTurnEndEvent fires → fallback timer starts but immediately returns ("tools were used, TurnStart likely coming") → SessionIdleEvent is lost (SDK bug #299) → session sits stuck showing "Thinking..." for 600s → watchdog kills it with error message, discarding the complete response.

After this fix: Same scenario → fallback timer waits 34s total → no TurnStart arrives → CompleteResponse fires → user sees the complete response with no error. If a tool is genuinely still running, the watchdog probes server TCP liveness and resets its timer.

Tests

• 41 behavioral safety tests (ChatExperienceSafetyTests.cs): Exercise real CompleteResponse, FlushCurrentResponse, AbortSessionAsync via reflection. Cover INV-1 cleanup, generation guards, content preservation, TCS completion, dedup guards, race conditions.
• Watchdog decision tests (ProcessingWatchdogTests.cs): Verify the 3-way branch logic, ExternalToolRequestedEvent classification, periodic flush presence/ordering.
• TurnEnd fallback tests (TurnEndFallbackTests.cs): Verify TurnEndIdleToolFallbackAdditionalMs bounds, total delay < watchdog timeout, cancellation-by-TurnStart, eventual-fire.

All 2344 tests pass (3 pre-existing failures: 2 symlink tests on Windows, 1 flaky unobserved-task test).

[PureWeen]

🔍 PR Review Squad — Round 1 Review (5-model consensus)

Models: claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex

Production Code Analysis

1. 🟡 MODERATE — Case A effectively disables the 10-min tool timeout (3/5 consensus)

CopilotService.Events.cs — When hasActiveTool && serverAlive, the watchdog resets LastEventAtTicks and continues. After the reset, elapsed drops to ~15s, far below effectiveTimeout (600s), so the 600s tool timeout block is never re-entered until another 600s of silence. The 10-minute tool execution timeout is effectively bypassed for any session where the server process stays alive.

The session IS still bounded by WatchdogMaxProcessingTimeSeconds (3600s / 60 min) — totalProcessingSeconds is recomputed from ProcessingStartedAt each iteration, so after 60 min total, exceededMaxTime triggers the kill. This is NOT an infinite loop. But the effective timeout jumps from 10 min to 60 min for stuck-tool-with-alive-server scenarios.

Suggestion: Check totalProcessingSeconds >= WatchdogToolExecutionTimeoutSeconds * N (e.g., 3x) inside Case A to cap at ~30 min.

2. 🟡 MODERATE — Periodic flush fires too eagerly during normal tool execution (3/5 consensus)

CopilotService.Events.cs:1512-1522 — The condition elapsed >= WatchdogCheckIntervalSeconds (15s) is true on virtually every watchdog cycle where no events arrived in the last 15s. During normal tool execution (e.g., a 60s build), there are 15s+ gaps between events. The flush will split a single response into multiple History entries, producing fragmented chat messages.

Suggestion: Gate on a higher threshold (e.g., elapsed >= 60) to only flush when content has been sitting for a significant time.

3. 🟡 MODERATE — 34s TurnEnd fallback may be too short for complex tool gaps (3/5 consensus)

CopilotService.Events.cs:489-503 — The TurnEndIdleToolFallbackAdditionalMs (30s) + base 4s = 34s total. In multi-agent or complex reasoning scenarios, the LLM may legitimately take >30s between TurnEnd and next TurnStart.

Mitigating factor: The fallback token is cancelled by TurnStart events, so this only fires if NO new TurnStart arrives in 34s.

Suggestion: Consider 60s for the additional delay.

4. 🟡 MODERATE — Source-code string scanning tests are brittle (4/5 consensus)

ProcessingWatchdogTests.cs:2157-2303 — 7 tests use File.ReadAllText + IndexOf to verify code structure. These break on refactoring even when behavior is preserved.

Suggestion: Convert to behavioral tests using the existing reflection infrastructure.

5. 🟡 MODERATE — Duplicate ComputeEffectiveTimeout helpers (2/5 consensus)

ChatExperienceSafetyTests.cs:419 and ProcessingWatchdogTests.cs:895 copy the production timeout formula with different parameter orders. If production formula changes, both silently drift.

Suggestion: Extract to a single internal static method on CopilotService.

Safe / No Issues Found

• Double-completion (TurnEnd fallback + watchdog Case B) — safe, generation guards + IsProcessing early-exit (5/5 agree)
• IsServerRunning TCP probe on watchdog thread — safe, new TcpClient per call (3/5 agree)
• ExternalToolRequestedEvent classification as TimelineOnly — correct

CI Status: ⚠️ No checks configured

Existing Reviews: None

Test Coverage

• ✅ 34 behavioral tests via reflection (CompleteResponse, AbortSession, FlushCurrentResponse, INV-1)
• ✅ TurnEnd fallback boundary tests (constant bounds, cancellation, fire)
• ⚠️ Gap: SessionErrorEvent INV-1 cleanup path has no behavioral test
• ⚠️ Gap: No behavioral test for watchdog Case B clean completion

Verdict: ⚠️ Request Changes

Actionable asks:

1. Raise periodic flush threshold from 15s to 60s+ (prevents message fragmentation)
2. Consider raising TurnEnd tool fallback from 30s to 60s
3. Extract ComputeEffectiveTimeout to single source of truth