fix: route MCP production mode through JSON-RPC

[mugenkyou]

Summary

Fixes MCPToolClient production mode so it uses the HTTP /mcp JSON-RPC transport instead of falling back to the WebSocket step() path.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• New environment
• Refactoring

Alignment Checklist

Before submitting, verify:

• I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
• I have checked .claude/docs/INVARIANTS.md and no invariants are violated
• I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

• Not required (bug fix, docs, minor refactoring)
• RFC exists: #___
• RFC needed (will create before merge)

Test Plan

Updated the production-mode tests to verify that:

• mode="production" sets use_production_mode to True
• list_tools() uses tools/list through the HTTP JSON-RPC path
• call_tool() uses tools/call through the HTTP JSON-RPC path
• production mode does not fall back to step()

Tests run:

• PYTHONPATH="src;envs" python -m pytest tests/core/test_mode_selection.py -q — 36 passed
• PYTHONPATH="src;envs" python -m pytest -p no:flask tests/core/test_mcp/ tests/core/test_production_mode_routes.py -q — 143 passed, 1 skipped
• PYTHONPATH="src;envs" python -m pytest -p no:flask tests/core/ -q — 601 passed, 2 skipped

Claude Code Review

N/A

Closes #1168

---

Note

Medium Risk
Changes session ownership, idle reaping, and close ordering on the env server; production clients depend on the new attach/teardown contract, though behavior is covered by expanded integration tests.

Overview
Production MCP clients now share one server session across HTTP JSON-RPC and WebSocket, so list_tools / call_tool and reset / step / state hit the same environment instance.

On the server, /ws can attach to an HTTP-created session via a session_id query parameter (one WebSocket per session). Attached sessions are exempt from idle reaping; openenv/session/close while a socket is attached returns closing: true and tears down only after the WebSocket disconnects. WebSocket-owned sessions still destroy on disconnect.

MCPToolClient connect creates the HTTP session first, opens the WebSocket with that session_id (preserving other query params), and on close disconnects the WebSocket before closing the MCP session. HTTP /mcp URL construction is normalized from the stable base_url. Connect/cancel paths use locks and cleanup so orphaned sessions are not left behind.

^{Reviewed by Cursor Bugbot for commit 00173d4. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Stale Bugbot comment from a previous run.

[mugenkyou]

@burtenshaw, tagging you regarding the MCP production-mode fix. The latest head, d165323, includes the cleanup fix identified by Bugbot, and the latest Cursor Bugbot review is passing.

The required CI workflows (lint, test (3.11), and test (3.12)) are currently awaiting maintainer approval. When you have a chance, could you please approve the workflows so the required checks can run?

Thank you!

[burtenshaw]

@mugenkyou thanks for the ping. Running. We should be able to get this down today.

[bot-ci-comment]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Stale comment

Blocking for merge at exact head 9dc05e30: Bugbot’s high-severity finding is correct.

Production connect() now creates a persistent HTTP MCP session, but session/httpx cleanup lives only in MCPClientBase.close(). Sync teardown (SyncEnvClient.close(), sync __exit__, and EnvClient.close() via _dispatch) calls _close_async() directly, so those paths never close the MCP session. Idle timeout defaults to off, so leaked sessions can accumulate toward MAX_CONCURRENT_ENVS.

Please move production session + HTTP client cleanup into an overridden _close_async() (or otherwise ensure every sync/async close path hits it), keep the existing connect-failure cleanup, and add a regression that sync close() / context-manager exit actually invokes session close. Do not merge until that lands and exact-head CI is green.

If you’d rather keep authorship off the fork path for today, say the word and I’ll open an on-repo replacement with the fix.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Stale comment

Conflict resolution

This fork is CONFLICTING with current main after #1175, and the release bot cannot push your branch (403).

Opened on-repo resolution with your single-session design + Co-authored-by credit:
#1180

That PR attaches /ws?session_id= to the HTTP MCP session (and teaches the server to attach without destroying HTTP-owned sessions), which fixes the dual-session capacity hole left by #1175. Please close this PR as superseded once #1180 lands.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Holding merge per Bugbot high on 17006d52: production _close_async calls openenv/session/close while /ws is still attached; the attachment guard rejects it, the client still clears _production_session_id, then WS detach leaves the HTTP session alive → capacity leak under max_concurrent_envs=1.

Please either:

1. Land the fix on this fork (disconnect WS before session/close; optionally make HTTP session/close authoritative), or
2. Squash-merge on-repo #1182 which already has that fix + regressions.

I will not merge this tip as-is.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Clearing CHANGES_REQUESTED: tip 41cb4e58 detaches WebSocket before openenv/session/close, which addresses the Bugbot high close/attach leak for the normal client path.

Still watching exact-head CI. Note: server still rejects session/close while a socket is attached — fine if clients always disconnect first; on-repo #1182 additionally makes HTTP close authoritative for races.

Will squash-merge this tip when required checks are green on the exact head (per Ben), unless a new blocking finding appears.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Follow-up: tip 41cb4e58 addresses the close/attach leak via disconnect-before-session-close. Remaining gate is fork workflow Approve-and-run on that exact head (Tests/Package/docs action_required). Will squash-merge when green.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Holding again: Bugbot high on 41cb4e58 is real — _disconnect_async returning does not guarantee the server cleared _session_websocket_attachments before openenv/session/close, so the ownership guard can still reject close; the client ignores the JSON-RPC error and clears the id → leak.

Needed: make HTTP openenv/session/close authoritative (discard attachment + destroy) and/or retry close until success after detach. On-repo #1182 already has the authoritative server close + regressions.

Please squash-merge #1182 when green, or port that server change onto this fork. I will not merge this tip as-is.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Clearing my earlier CHANGES_REQUESTED (that hold targeted the 41cb4e58 close/attach race).

Tip 715d5af6 addresses it correctly: while /ws is attached, HTTP openenv/session/close returns closing: true and queues _session_pending_closes; destroy runs on WebSocket detach (cancel-safe discard-before-await). Client still disconnects before session/close. Local focused suite 103 passed / 1 skipped.

Per Ben: will squash-merge #1169 when exact-head required CI is green. Gate right now is fork Approve-and-run on this head (Tests / Package CI / Docs are action_required; I get 403 approving those).

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Approving tip 715d5af6: deferred openenv/session/close until WebSocket detach (closing + _session_pending_closes) clears the prior attach-race hold. Will squash-merge when exact-head Tests/Package/docs are green (fork Approve-and-run still required).

View PR

  

_{Sent by Cursor Automation: Release}

The exact-head approval clears a still-reproducible connect-cancellation leak and unresolved RFC 003 capability boundary. CancelledError during super()._connect_async() bypasses except Exception after the HTTP session is created, leaving the session/client/provider allocated. Do not merge on CI alone.

[cursor]

Stale comment

Blocking exact head 715d5af6: the close/attachment race is fixed, but production connect remains cancellation-unsafe. After _ensure_production_session() allocates the HTTP session, cancellation during super()._connect_async() raises CancelledError (a BaseException) past the except Exception cleanup, leaving the HTTP session/client/provider allocated. Add a cancelled-WebSocket-connect regression proving all resources and capacity are released. Also retain requested Darktex/sergiopaniego review: the new bearer session_id lets an /mcp session attach to orchestration /ws reset/step/state and requires an explicit RFC 003 security decision. Do not merge on generic CI alone.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 715d5af. Configure here.}

[cursor]

Pending close can destroy reattached session

Medium Severity

Deferred session close drops the WebSocket attachment and then awaits _destroy_session without claiming exclusive ownership. A new WebSocket can attach to the same session_id in that gap because attach never consults _session_pending_closes, so the delayed destroy can close the environment out from under the new connection.

Additional Locations (2)

• src/openenv/core/env_server/http_server.py#L816-L843
• src/openenv/core/env_server/http_server.py#L1534-L1552

 

^{Reviewed by Cursor Bugbot for commit 715d5af. Configure here.}

[cursor]

Confirmed hold on tip 715d5af6:

1. Cancel leak (real): production _connect_async uses except Exception, so CancelledError after _ensure_production_session() skips cleanup → capacity leak.
2. Bugbot medium (real): attach does not consult _session_pending_closes, so a reattach can race delayed destroy.

Both fixed on on-repo #1182 (d7c9a7e0) with regressions. Prefer merging #1182 (Co-authored-by mugenkyou) when its CI is green, or wait for the author to port onto this fork. Won’t squash-merge this tip as-is.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Tip 00173d47 clears the cancel-after-session-create leak (except BaseException + close). Resuming Ben’s merge-when-green for this exact head.

Residual Bugbot medium (reattach while _session_pending_closes) is still open on this tip; fixed on on-repo #1182. Treating that as follow-up unless Ben says hold — normal client close does not reattach the same session id in the destroy gap.

Will squash-merge when exact-head Tests/Package/docs are green.

View PR

  

_{Sent by Cursor Automation: Release}

[cursor]

Blocking exact head 00173d47: connect cancellation is improved, but Bugbot’s pending-close reattachment race remains. WebSocket finally drops the attachment marker before _destroy_session() acquires the lock; attach does not reject _session_pending_closes, so a new socket can attach in that gap and have its environment destroyed underneath it. Port the pending-close attach guard and regression from #1182, then retain current Darktex/sergiopaniego review for the RFC 003 bearer-capability boundary. Do not merge on generic CI alone.

View PR

  

_{Sent by Cursor Automation: Release}

[burtenshaw]

Thanks @mugenkyou . Cursor really had a hard time with this one! 🤣

If you're interested, we're in need of someone taking on MCP > v4. Think you could handle that?

[mugenkyou]

Yeah, definitely @burtenshaw. I’d love to take this on. Let me know what you have in mind for MCP > v4.