Skip to content

Add MCP proxy helper#2388

Draft
Kludex wants to merge 4 commits intomainfrom
fix/mcp-proxy-review-comments
Draft

Add MCP proxy helper#2388
Kludex wants to merge 4 commits intomainfrom
fix/mcp-proxy-review-comments

Conversation

@Kludex
Copy link
Copy Markdown
Member

@Kludex Kludex commented Apr 3, 2026

Summary

  • add a top-level mcp_proxy() helper in mcp/proxy.py and export it from mcp
  • implement the proxy with the repo's generic read/write stream protocols and the reviewed on_error API
  • replace the original timing- and coverage-driven test approach with deterministic tests for forwarding, error handling, and stream closure behavior

Why

PR #1763 was closed after accumulating review feedback. This reimplements the proxy from current main while applying the requested changes:

  • move the module to mcp/proxy.py
  • avoid new pragma: no cover additions
  • remove timing-based sleeps from tests
  • remove type: ignore assertions in tests
  • simplify logging and broad exception handling

Testing

  • uv run pytest tests/test_proxy.py -q
  • UV_CACHE_DIR=/tmp/uv-cache uv run ruff check src/mcp/proxy.py tests/test_proxy.py
  • UV_CACHE_DIR=/tmp/uv-cache uv run pyright src/mcp/proxy.py tests/test_proxy.py

async def mcp_proxy(
transport_to_client: MessageStream,
transport_to_server: MessageStream,
on_error: ErrorHandler | None = None,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
on_error: ErrorHandler | None = None,
*,
on_error: ErrorHandler | None = None,

Comment on lines +62 to +67
try:
result = on_error(error)
if inspect.isawaitable(result):
await result
except Exception:
return
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have an internal function somewhere to see if it's coroutine or awaitable. Please use that, and we should use to_thread.run_sync to avoid blocking the event loop.

@Kludex Kludex changed the title [codex] Add reviewed MCP proxy helper Add MCP proxy helper Apr 3, 2026
print(weather.content[0].text)
print(alerts.content[0].text)

upstream_server.should_exit = True
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want a new file! Isn't this implemented somewhere in this repo already?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant