fix: handle ExceptionGroup and CancelledError in MCP error flow (#87)

* fix: handle ExceptionGroup and CancelledError in MCP error flow

When an MCP connection fails, two issues were masking the real error:

1. Cleanup raises ExceptionGroup containing the HTTP error (Python 3.11+)
2. Session initialization raises CancelledError instead of the real error

This fix addresses both:

**ExceptionGroup handling:**
- Catch BaseException in cleanup to handle ExceptionGroup
- Extract and log individual exceptions from the group
- Gracefully handle Python 3.10 (no ExceptionGroup) and 3.11+
- Log HTTPStatusError at debug level (expected during cleanup)

**CancelledError handling:**
- Catch BaseException in connection loop (not just Exception)
- Properly convert CancelledError to ADCPConnectionError
- Preserve KeyboardInterrupt and SystemExit behavior

Now users see the actual connection error (e.g., "405 Method Not Allowed")
instead of confusing cleanup error messages.

Added test to verify ExceptionGroup handling works correctly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: improve ExceptionGroup and error handling robustness

Addresses code review feedback to make error handling more robust:

1. Fix ExceptionGroup import - use built-in type directly, not from builtins
2. Add KeyboardInterrupt/SystemExit re-raise in cleanup path
3. Replace string-based HTTPStatusError check with isinstance
4. Update all references from BaseExceptionGroup to ExceptionGroup

These changes improve type safety, handle critical interrupts correctly,
and make the code more maintainable.

All tests pass and mypy type checking succeeds.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: use try/except for ExceptionGroup to satisfy ruff

Ruff doesn't recognize sys.version_info guards for undefined names.
Changed to try/except pattern which both ruff and mypy accept.

Also removed unused sys import since we no longer need version checking.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: skip ExceptionGroup test on Python 3.10

The test_cleanup_handles_exception_group test uses ExceptionGroup
which doesn't exist in Python 3.10. Added skipif marker to skip
this test on Python < 3.11.

The functionality still works correctly in 3.10 (it gracefully handles
the absence of ExceptionGroup), we just can't test it directly since
we can't create an ExceptionGroup in the test.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: bokelley

src/adcp/protocols/mcp.py

@@ -9,6 +9,14 @@
 from typing import TYPE_CHECKING, Any
 from urllib.parse import urlparse
 
+# ExceptionGroup is available in Python 3.11+
+# In 3.11+, it's a built-in type. For 3.10, we need to handle its absence.
+try:
+    _ExceptionGroup: type[BaseException] | None = ExceptionGroup  # type: ignore[name-defined]
+except NameError:
+    # Python 3.10 - ExceptionGroup doesn't exist
+    _ExceptionGroup = None
+
 logger = logging.getLogger(__name__)
 
 if TYPE_CHECKING:
@@ -23,6 +31,14 @@
 except ImportError:
     MCP_AVAILABLE = False
 
+try:
+    from httpx import HTTPStatusError
+
+    HTTPX_AVAILABLE = True
+except ImportError:
+    HTTPX_AVAILABLE = False
+    HTTPStatusError = None  # type: ignore[assignment, misc]
+
 from adcp.exceptions import ADCPConnectionError, ADCPTimeoutError
 from adcp.protocols.base import ProtocolAdapter
 from adcp.types.core import DebugInfo, TaskResult, TaskStatus
@@ -56,22 +72,45 @@ async def _cleanup_failed_connection(self, context: str) -> None:
             self._session = None
             try:
                 await old_stack.aclose()
-            except asyncio.CancelledError:
-                logger.debug(f"MCP session cleanup cancelled {context}")
-            except RuntimeError as cleanup_error:
-                # Known anyio task group cleanup issue
-                error_msg = str(cleanup_error).lower()
-                if "cancel scope" in error_msg or "async context" in error_msg:
-                    logger.debug(f"Ignoring anyio cleanup error {context}: {cleanup_error}")
+            except BaseException as cleanup_error:
+                # Handle all cleanup errors including ExceptionGroup
+                # Re-raise KeyboardInterrupt and SystemExit immediately
+                if isinstance(cleanup_error, (KeyboardInterrupt, SystemExit)):
+                    raise
+
+                if isinstance(cleanup_error, asyncio.CancelledError):
+                    logger.debug(f"MCP session cleanup cancelled {context}")
+                    return
+
+                # Handle ExceptionGroup from task group failures (Python 3.11+)
+                if _ExceptionGroup is not None and isinstance(
+                    cleanup_error, _ExceptionGroup
+                ):
+                    for exc in cleanup_error.exceptions:  # type: ignore[attr-defined]
+                        self._log_cleanup_error(exc, context)
                 else:
-                    logger.warning(
-                        f"Unexpected RuntimeError during cleanup {context}: {cleanup_error}"
-                    )
-            except Exception as cleanup_error:
-                # Log unexpected cleanup errors but don't raise to preserve original error
-                logger.warning(
-                    f"Unexpected error during cleanup {context}: {cleanup_error}", exc_info=True
-                )
+                    self._log_cleanup_error(cleanup_error, context)
+
+    def _log_cleanup_error(self, exc: BaseException, context: str) -> None:
+        """Log a cleanup error without raising."""
+        # Check for known cleanup error patterns from httpx/anyio
+        exc_str = str(exc).lower()
+
+        # Common cleanup errors that are expected when connection fails
+        is_known_cleanup_error = (
+            isinstance(exc, RuntimeError)
+            and ("cancel scope" in exc_str or "async context" in exc_str)
+        ) or (
+            # HTTP errors during cleanup (if httpx is available)
+            HTTPX_AVAILABLE and HTTPStatusError is not None and isinstance(exc, HTTPStatusError)
+        )
+
+        if is_known_cleanup_error:
+            # Expected cleanup errors - log at debug level without stack trace
+            logger.debug(f"Ignoring expected cleanup error {context}: {exc}")
+        else:
+            # Truly unexpected cleanup errors - log at warning with full context
+            logger.warning(f"Unexpected error during cleanup {context}: {exc}", exc_info=True)
 
     async def _get_session(self) -> ClientSession:
         """
@@ -146,7 +185,11 @@ async def _get_session(self) -> ClientSession:
                         )
 
                     return self._session  # type: ignore[no-any-return]
-                except Exception as e:
+                except BaseException as e:
+                    # Catch BaseException to handle CancelledError from failed initialization
+                    # Re-raise KeyboardInterrupt and SystemExit immediately
+                    if isinstance(e, (KeyboardInterrupt, SystemExit)):
+                        raise
                     last_error = e
                     # Clean up the exit stack on failure to avoid resource leaks
                     await self._cleanup_failed_connection("during connection attempt")

tests/test_protocols.py

@@ -1,5 +1,6 @@
 """Tests for protocol adapters."""
 
+import sys
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
@@ -844,3 +845,37 @@ def create_mock_exit_stack():
         # Verify adapter state is clean after all failed attempts
         assert adapter._exit_stack is None
         assert adapter._session is None
+
+    @pytest.mark.asyncio
+    @pytest.mark.skipif(
+        sys.version_info < (3, 11),
+        reason="ExceptionGroup is only available in Python 3.11+",
+    )
+    async def test_cleanup_handles_exception_group(self, mcp_config):
+        """Test that cleanup handles ExceptionGroup from task group failures."""
+        from contextlib import AsyncExitStack
+
+        import httpx
+
+        adapter = MCPAdapter(mcp_config)
+
+        # Create an ExceptionGroup like what anyio task groups raise
+        http_error = httpx.HTTPStatusError(
+            "Client error '405 Method Not Allowed' for url 'https://test.example.com'",
+            request=MagicMock(),
+            response=MagicMock(status_code=405),
+        )
+        exception_group = ExceptionGroup("unhandled errors in a TaskGroup", [http_error])
+
+        # Mock exit stack that raises ExceptionGroup on cleanup
+        mock_exit_stack = AsyncMock(spec=AsyncExitStack)
+        mock_exit_stack.aclose = AsyncMock(side_effect=exception_group)
+        adapter._exit_stack = mock_exit_stack
+
+        # cleanup should not raise despite the ExceptionGroup
+        await adapter._cleanup_failed_connection("during test")
+
+        # Verify cleanup was attempted and state is clean
+        mock_exit_stack.aclose.assert_called_once()
+        assert adapter._exit_stack is None
+        assert adapter._session is None