-
Notifications
You must be signed in to change notification settings - Fork 174
feat: implement HTTP MCP endpoint and adapt coding_env to MCP (RFC 003) #334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR implements the HTTP MCP endpoint at POST /mcp as specified in RFC 003,
and adapts the coding_env environment to use MCPEnvironment with FastMCP tools.
## Changes
### Core MCP Implementation
- Add POST /mcp endpoint for direct MCP access (tools/list, tools/call)
- Add MCPRequest/MCPResponse types for HTTP JSON-RPC
- Add MCPAction discriminated union for polymorphic action deserialization
- Add MCPHttpClient for lightweight HTTP-only MCP access
- Update deserialize_action to handle MCP union types
### coding_env MCP Adaptation
- Convert PythonCodeActEnv from Environment to MCPEnvironment
- Add FastMCP tools: execute_code, check_syntax
- Maintain backward compatibility with legacy CodeAction
- Update app.py to use MCP types
### Tests
- Add 21 new tests for HTTP /mcp endpoint
- Add tests for MCPRequest/MCPResponse types
- Add tests for non-MCP environments with /mcp endpoint
- All 112 MCP and coding_env tests pass
## API Usage
```python
# Direct HTTP MCP access (production/inference)
POST /mcp
{"jsonrpc": "2.0", "method": "tools/list", "id": 1}
POST /mcp
{"jsonrpc": "2.0", "method": "tools/call",
"params": {"name": "execute_code", "arguments": {"code": "print(1)"}}, "id": 2}
# Or via MCPHttpClient
from openenv.core.mcp_client import MCPHttpClient
client = MCPHttpClient(base_url="http://localhost:8000")
tools = client.list_tools()
result = client.call_tool("execute_code", code="print('Hello')")
```
Implements RFC 003: MCP Support
|
Hi @2Maze! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Greptile OverviewGreptile SummaryThis PR implements HTTP MCP endpoint (POST Key Changes:
Critical Issue - State Management: RFC 003 describes this endpoint as "stateless", but the implementation applies this limitation to ALL environments, including those designed for stateful interactions. This needs clarification or session management. Alignment with RFC 003: Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant HTTP as FastAPI /mcp Endpoint
participant Env as PythonCodeActEnv (temp)
participant MCP as FastMCP Server
participant Executor as PyExecutor
Note over Client,Executor: HTTP MCP Access (RFC 003)
Client->>HTTP: POST /mcp<br/>{"method": "tools/list", "id": 1}
HTTP->>Env: _env_factory() - Create temp env
Env->>MCP: Initialize FastMCP with tools
HTTP->>MCP: async with mcp_client:<br/>list_tools()
MCP-->>HTTP: [execute_code, check_syntax]
HTTP->>Env: close()
HTTP-->>Client: {"result": {"tools": [...]}, "id": 1}
Note over Client,Executor: Tool Call via HTTP MCP
Client->>HTTP: POST /mcp<br/>{"method": "tools/call",<br/>"params": {"name": "execute_code",<br/>"arguments": {"code": "x=1"}}}
HTTP->>Env: _env_factory() - Create NEW temp env
Env->>MCP: Initialize FastMCP with tools
HTTP->>MCP: async with mcp_client:<br/>call_tool("execute_code", ...)
MCP->>Executor: run(code="x=1")
Executor-->>MCP: {stdout, stderr, exit_code}
Note over Env: State updated but lost<br/>(env destroyed after response)
MCP-->>HTTP: result
HTTP->>Env: close()
HTTP-->>Client: {"result": {...}, "id": 2}
Note over Client,Executor: State is NOT preserved between calls
Client->>HTTP: POST /mcp<br/>{"method": "tools/call",<br/>"params": {"name": "execute_code",<br/>"arguments": {"code": "print(x)"}}}
HTTP->>Env: _env_factory() - Create ANOTHER temp env
Note over Env,Executor: 'x' is undefined<br/>(fresh environment)
Env->>MCP: Initialize FastMCP with tools
HTTP->>MCP: async with mcp_client:<br/>call_tool("execute_code", ...)
MCP->>Executor: run(code="print(x)")
Executor-->>MCP: NameError: 'x' not defined
MCP-->>HTTP: error
HTTP->>Env: close()
HTTP-->>Client: {"error": {...}, "id": 3}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 3 comments
| # Create a temporary environment for this request | ||
| _env = self._env_factory() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creates temporary environment per request with no state persistence. Each /mcp call gets fresh environment, losing execution context between calls. For coding_env, this means variables defined in one execute_code call won't be available in the next.
RFC 003 describes this as "stateless tool discovery and invocation", but this conflicts with environments that rely on persistent state (like Python REPL contexts). Consider either:
- Documenting this limitation clearly in endpoint description
- Implementing session management for
/mcpendpoint - Clarifying that this endpoint is only for stateless tools
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/openenv/core/env_server/http_server.py
Line: 756:757
Comment:
Creates temporary environment per request with no state persistence. Each `/mcp` call gets fresh environment, losing execution context between calls. For `coding_env`, this means variables defined in one `execute_code` call won't be available in the next.
RFC 003 describes this as "stateless tool discovery and invocation", but this conflicts with environments that rely on persistent state (like Python REPL contexts). Consider either:
- Documenting this limitation clearly in endpoint description
- Implementing session management for `/mcp` endpoint
- Clarifying that this endpoint is only for stateless tools
How can I resolve this? If you propose a fix, please make it concise.| result = env_self._executor.run(code) | ||
|
|
||
| # Update state | ||
| env_self._state.step_count += 1 | ||
| env_self._state.last_exit_code = result.exit_code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MCP tool directly modifies environment instance state (env_self._state.step_count and env_self._state.last_exit_code). When called via HTTP /mcp endpoint, these state updates are lost since a temporary environment is created per request (see http_server.py:757). This breaks state tracking for MCP-based interactions.
The dual-path design (MCP tools vs step()) creates state synchronization issues.
Prompt To Fix With AI
This is a comment left during a code review.
Path: envs/coding_env/server/python_codeact_env.py
Line: 99:103
Comment:
MCP tool directly modifies environment instance state (`env_self._state.step_count` and `env_self._state.last_exit_code`). When called via HTTP `/mcp` endpoint, these state updates are lost since a temporary environment is created per request (see `http_server.py:757`). This breaks state tracking for MCP-based interactions.
The dual-path design (MCP tools vs step()) creates state synchronization issues.
How can I resolve this? If you propose a fix, please make it concise.| id=request_id, | ||
| ) | ||
| finally: | ||
| _env.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment cleanup in finally block is good, but if environment has async cleanup or background processes, calling close() immediately after returning response may cause issues. Consider whether environments need graceful shutdown time.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/openenv/core/env_server/http_server.py
Line: 839:839
Comment:
Environment cleanup in `finally` block is good, but if environment has async cleanup or background processes, calling `close()` immediately after returning response may cause issues. Consider whether environments need graceful shutdown time.
How can I resolve this? If you propose a fix, please make it concise.|
@Darktex Can you please see my PR? |
Summary
Implement HTTP MCP endpoint at POST /mcp as specified in RFC 003, enabling direct MCP access for production/inference use cases. Also adapts coding_env to use MCPEnvironment with FastMCP tools (execute_code, check_syntax).
Type of Change
Alignment Checklist
Before submitting, verify:
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violated/pre-submit-pr(orbash .claude/hooks/lint.shand tests) and addressed all issuesRFC Status
Test Plan
Claude Code Review