Skip to content

fix: recognize "Client not connected" as connection error for auto-reconnect#317

Merged
PureWeen merged 1 commit intomainfrom
fix/session-pr-review-squad-orchestrator-wil-20260309-0233
Mar 9, 2026
Merged

fix: recognize "Client not connected" as connection error for auto-reconnect#317
PureWeen merged 1 commit intomainfrom
fix/session-pr-review-squad-orchestrator-wil-20260309-0233

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

@PureWeen PureWeen commented Mar 9, 2026

Bug

Multi-agent orchestrator dispatch fails with:

⚠️ Session disconnected and reconnect failed: Client not connected. Call StartAsync() first.

Root Cause

When the SDK throws InvalidOperationException("Client not connected. Call StartAsync() first."), SendPromptAsync's reconnection logic calls IsConnectionError(ex) to determine whether to recreate the CopilotClient. This error was not recognized as a connection error because:

  • It's an InvalidOperationException (not IOException/SocketException/ObjectDisposedException)
  • The message "Client not connected" doesn't match any existing patterns ("connection refused", "connection lost", "JSON-RPC connection", etc.)

Without recognition, the reconnect skips client recreation and tries ResumeSessionAsync on the broken client — which also fails, surfacing the error to the user.

Fix

Added "not connected" to the message pattern list in IsConnectionError() (1 line in CopilotService.Utilities.cs).

This enables the existing reconnection flow to:

  1. Recognize the error as a connection error
  2. Recreate the CopilotClient (restart persistent server if needed)
  3. Resume/create the session
  4. Retry the send

Tests

Added 5 regression tests to ConnectionRecoveryTests.cs:

  • Direct InvalidOperationException("Client not connected...") detection
  • Variations: "Client not connected", "Server not connected"
  • Wrapped in AggregateException
  • Wrapped as InnerException

All 2278 passing tests continue to pass (1 pre-existing failure on main unrelated to this change).

…connect

When the SDK throws InvalidOperationException("Client not connected. Call
StartAsync() first."), SendPromptAsync's reconnect logic needs to recognize
this as a connection error so it recreates the CopilotClient before retrying.

Without this fix, IsConnectionError() returns false for this error, causing
the reconnect to skip client recreation and attempt ResumeSessionAsync on a
broken client — which also fails, surfacing as "Session disconnected and
reconnect failed" in multi-agent orchestrator dispatch.

The fix adds "not connected" to the message pattern list in IsConnectionError().

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen merged commit 18ce5b6 into main Mar 9, 2026
@PureWeen PureWeen deleted the fix/session-pr-review-squad-orchestrator-wil-20260309-0233 branch March 9, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant