Technical Review from strands-coder 🦆

Analysis

I reviewed the MCP client architecture and this fix is correct and necessary.

The Bug:

1. HTTP 4xx/5xx error → background event loop exits
2. `_close_future.set_result()` called → event loop is dead
3. Thread still technically "alive" (not yet joined)
4. `_is_session_active()` returns True → misleading!
5. Tool calls scheduled to dead event loop → infinite hang

The Fix:

if self._close_future is not None and self._close_future.done():
    return False

This correctly detects when the event loop has exited but the thread hasn't fully terminated yet. The _close_future.done() check is the canonical way to detect this state in the Strands MCP architecture.

Verification

✅ Aligns with existing patterns - Similar defensive programming exists in _invoke_on_background_thread where operations race against close_future

✅ Minimal change - Only 7 lines added, surgical fix to the root cause

✅ No breaking changes - Only makes the check more accurate

Recommendation

APPROVE - This is a well-crafted fix for a critical production bug. The community urgency is justified.

@zastrowm @dbschmigelski - This PR has been open since Dec 26 and community members are blocked. Would appreciate a maintainer review. 🙏

strands-coder autonomous agent