Fix MCP HTTP session persistence across tool calls

[neubig]

Summary

Simplified fix for MCP session persistence. Keeps MCP connections open after listing tools instead of reconnecting for each tool call.

Changes

• utils.py: Keep connection open after _list_tools_and_keep_connected()
• tool.py: Remove async with from executor since client is already connected
• client.py: Reverted to original simple implementation

Key Insight

The cleanest fix is proper object lifecycle:

1. Create ONE client per server
2. Keep it open after listing tools
3. Close it when done

No reference counting, no session managers, no global state. Just ~100 lines changed instead of 900+.

Test Results

All 58 MCP tests pass.

Live Testing with Playwright MCP

Verified session persistence with the Playwright MCP server (a stateful browser automation server):

1. Step 1: Agent navigated to https://www.openhands.dev/blog using browser_navigate
2. Step 2: Agent used browser_snapshot to verify it was still on the same page

Result: ✅ SUCCESS - The browser state was maintained across multiple tool calls. The agent confirmed it was still on https://www.openhands.dev/blog with the correct page content.

This proves the MCP session persistence fix works correctly - without the fix, the browser context would have been lost when the MCP connection was closed and reopened between tool calls.

Fixes: #1739

---

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python	amd64, arm64	nikolaik/python-nodejs:python3.12-nodejs22	Link
golang	amd64, arm64	golang:1.21-bookworm	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:974b18e-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-974b18e-python \
  ghcr.io/openhands/agent-server:974b18e-python

All tags pushed for this build

ghcr.io/openhands/agent-server:974b18e-golang-amd64
ghcr.io/openhands/agent-server:974b18e-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:974b18e-golang-arm64
ghcr.io/openhands/agent-server:974b18e-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:974b18e-java-amd64
ghcr.io/openhands/agent-server:974b18e-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:974b18e-java-arm64
ghcr.io/openhands/agent-server:974b18e-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:974b18e-python-amd64
ghcr.io/openhands/agent-server:974b18e-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-amd64
ghcr.io/openhands/agent-server:974b18e-python-arm64
ghcr.io/openhands/agent-server:974b18e-nikolaik_s_python-nodejs_tag_python3.12-nodejs22-arm64
ghcr.io/openhands/agent-server:974b18e-golang
ghcr.io/openhands/agent-server:974b18e-java
ghcr.io/openhands/agent-server:974b18e-python

About Multi-Architecture Support

• Each variant tag (e.g., 974b18e-python) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., 974b18e-python-amd64) are also available if needed

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-sdk/openhands/sdk/mcp
client.py	37	3	91%	43, 69–70
tool.py	95	17	82%	56, 70–73, 98, 101, 160, 167, 169–171, 237–238, 243, 251, 279
utils.py	46	7	84%	25–26, 29, 32, 48, 99–100
TOTAL	15695	4557	70%

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Pre-commit checks

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #1740 at branch `openhands/fix-mcp-session-persistence`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[neubig]

@OpenHands fix pre-commit

[openhands-ai]

I'm on it! neubig can track my progress at all-hands.dev

[openhands-ai]

I've fixed the pre-commit issues in PR #1740. The changes have been pushed to the openhands/fix-mcp-session-persistence branch.

Summary of Changes

Fixed pre-commit failures in tests/sdk/mcp/test_stateful_mcp.py:

1. Line length issues (E501) - Fixed 3 lines that exceeded 88 characters:

   • Line 72: Split long error message string across multiple lines
   • Line 85: Extracted counter value to a variable before using in f-string
   • Line 218: Shortened comment text

2. Pyright type errors - Fixed 5 type errors where executor could be None:

   • Added assertions (assert ... is not None) for all 4 tool executors before using them in test_multiple_operations_same_session

3. Unused import - Removed unused MCPToolAction import (auto-fixed by ruff)

All pre-commit checks now pass:

• ✅ Format YAML files
• ✅ Ruff format
• ✅ Ruff lint
• ✅ PEP8 style check (pycodestyle)
• ✅ Type check with pyright
• ✅ Check import dependency rules
• ✅ Check Tool subclass registration

View full conversation

[all-hands-bot]

The session persistence fix works (tests pass), but there are concerns about the approach that should be addressed. Key issues: context manager protocol violation and relying on del for cleanup. Details in inline comments.

[neubig]

@OpenHands try to reflect the comments above without making the code overly complex.

[openhands-ai]

I'm on it! neubig can track my progress at all-hands.dev

[openhands-ai]

I've addressed the review comments on PR #1740 without making the code overly complex. The changes have been pushed to the openhands/fix-mcp-session-persistence branch.

Summary of Changes

Addressed all 4 review comments from all-hands-bot:

1. 🟠 Important - Protocol violation fix: Added explicit connect() and disconnect() methods to MCPClient as a cleaner alternative to calling __aenter__() directly. The _list_tools_and_keep_connected() function now uses await client.connect() instead.

2. 🟡 Context manager support: Added __enter__ and __exit__ methods to MCPToolExecutor for deterministic cleanup. Users can now write:

   with tool.executor as executor:
       # Use tools
       pass  # Automatic cleanup

3. 🟡 Exception handling: Improved the exception handler in utils.py to wrap client.sync_close() in a try-except block, ensuring the original exception is preserved if cleanup fails.

4. 🟢 Better error message: Enhanced the RuntimeError message when the client is disconnected to include more debugging context.

Files Changed

• client.py: +16 lines (added connect() and disconnect() methods)
• tool.py: +10 lines (added context manager support)
• utils.py: +7/-7 lines (use connect() method, improved exception handling)

All 61 MCP tests pass and all pre-commit checks pass.

View full conversation

[jpshackelford]

Issue: Setting self._closed = True before attempting cleanup means that if cleanup fails, future calls to sync_close() will return early without retrying.

Suggested change: Move self._closed = True to after the cleanup operations:

def sync_close(self) -> None:
    if self._closed:
        return

    # Best-effort: try async close if parent provides it
    if hasattr(self, "close") and inspect.iscoroutinefunction(self.close):
        try:
            self._executor.run_async(self.close, timeout=10.0)
        except Exception:
            pass

    self._executor.close()
    self._closed = True  # Mark closed only after cleanup succeeds

Why this is safe: Both underlying close methods are idempotent:

• AsyncExecutor.close() uses an atomic swap pattern with a None check
• fastmcp.Client.close() → _disconnect() checks session_task is None before acting

Multiple calls to either are harmless, so if cleanup partially fails, a retry can safely re-attempt without double-closing anything.