Skip to content

fix: forward proxy server notifications#4191

Open
he-yufeng wants to merge 1 commit into
PrefectHQ:mainfrom
he-yufeng:fix/proxy-forward-server-notifications
Open

fix: forward proxy server notifications#4191
he-yufeng wants to merge 1 commit into
PrefectHQ:mainfrom
he-yufeng:fix/proxy-forward-server-notifications

Conversation

@he-yufeng
Copy link
Copy Markdown

Summary

Fixes #4161.

This makes FastMCP proxies forward upstream server notifications to the downstream client while a proxied request is active. It covers ordinary MCP notifications such as tool/resource/prompt list changes and resource updates.

For create_proxy(Client(...)), the wrapper also forwards logging/progress notifications because a plain client does not use ProxyClient's proxy log/progress callbacks. For ProxyClient, the existing log/progress callbacks stay in charge, so this only fills the missing ordinary-notification path.

To verify

  • PYTHONUTF8=1 TMP=.tmp/pytest TEMP=.tmp/pytest python -m pytest tests/server/providers/proxy/test_proxy_server.py -q
  • python -m ruff check fastmcp_slim/fastmcp/server/providers/proxy.py tests/server/providers/proxy/test_proxy_server.py
  • uv run --frozen ty check fastmcp_slim/fastmcp/server/providers/proxy.py tests/server/providers/proxy/test_proxy_server.py
  • git diff --check
  • python -m py_compile fastmcp_slim/fastmcp/server/providers/proxy.py tests/server/providers/proxy/test_proxy_server.py

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels May 20, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b5fa38941

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +802 to 804
if not isinstance(client, ProxyClient):
_install_plain_client_proxy_message_handler(client)
return client
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Apply proxy message handler before reusing connected client

When create_proxy() receives a connected plain Client, this branch calls _install_plain_client_proxy_message_handler(client) and then reuses the same connection, but _install_* only mutates client._session_kwargs. For an already-open session, connect_session(**_session_kwargs) has already run, so the active receive loop keeps the old handler and upstream notifications are still not forwarded in this mode. This means connected-client proxying does not get the behavior this change intends unless the client disconnects/reconnects first.

Useful? React with 👍 / 👎.

@he-yufeng he-yufeng force-pushed the fix/proxy-forward-server-notifications branch from 1b5fa38 to 8b5a32e Compare May 20, 2026 14:33
Copy link
Copy Markdown
Author

Updated to address the connected-client review finding.

For a plain Client that is already connected when passed to create_proxy(), the proxy handler is now installed on both future session kwargs and the active ClientSession. I also wired the existing request-context restoration path into plain proxy clients, so notifications emitted from the reused receive loop can still be forwarded to the current downstream proxy request.

Added a regression test for create_proxy() receiving an already-connected plain Client.

Validation:

  • uv run --frozen pytest tests/server/providers/proxy/test_proxy_server.py -q (69 passed)
  • uv run --frozen ruff check fastmcp_slim/fastmcp/server/providers/proxy.py tests/server/providers/proxy/test_proxy_server.py
  • uv run --frozen ty check fastmcp_slim/fastmcp/server/providers/proxy.py
  • uv run --frozen python -m py_compile fastmcp_slim/fastmcp/server/providers/proxy.py tests/server/providers/proxy/test_proxy_server.py
  • git diff --check

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

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should proxies forward upstream list/resource notifications downstream?

1 participant