-
Notifications
You must be signed in to change notification settings - Fork 774
Fix gateway URL resolution #443
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,47 +3,53 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import httpx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from mcp_agent.mcp.mcp_server_registry import ServerRegistry | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from urllib.parse import quote | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _resolve_gateway_url( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_registry: Optional[ServerRegistry] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_name: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_url: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| context_gateway_url: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Resolve the base URL for the MCP gateway. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Precedence: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 1) Explicit override (gateway_url parameter) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 2) Context-provided URL (context_gateway_url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 3) Environment variable MCP_GATEWAY_URL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 4) Fallback to http://127.0.0.1:8000 (dev default) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Highest precedence: explicit override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if gateway_url: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return gateway_url.rstrip("/") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Next: context-provided URL (e.g., from Temporal workflow memo) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if context_gateway_url: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return context_gateway_url.rstrip("/") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Next: environment variable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env_url = os.environ.get("MCP_GATEWAY_URL") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if env_url: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return env_url.rstrip("/") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Next: a registry entry (if provided) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if server_registry and server_name: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cfg = server_registry.get_server_config(server_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if cfg and getattr(cfg, "url", None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return cfg.url.rstrip("/") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Fallback: default local server | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "http://127.0.0.1:8000" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def log_via_proxy( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_registry: Optional[ServerRegistry], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| execution_id: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| level: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data: Dict[str, Any] | None = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_name: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_url: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_token: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = _resolve_gateway_url(server_registry, server_name, gateway_url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def log_via_proxy(*args, **kwargs) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Backward-compatible wrapper. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Accepts either: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - legacy: (server_registry, execution_id, level, namespace, message, data=None, *, gateway_url=None, gateway_token=None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - new: (execution_id, level, namespace, message, data=None, *, gateway_url=None, gateway_token=None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if args and (args[0] is None or not isinstance(args[0], str)): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args = args[1:] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| execution_id, level, namespace, message, *rest = args | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data = rest[0] if rest else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_url = kwargs.get("gateway_url") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_token = kwargs.get("gateway_token") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url = f"{base}/internal/workflows/log" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: Dict[str, str] = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tok = gateway_token or os.environ.get("MCP_GATEWAY_TOKEN") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -74,17 +80,15 @@ async def log_via_proxy( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bool(resp.get("ok", True)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def ask_via_proxy( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_registry: Optional[ServerRegistry], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| execution_id: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prompt: str, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| metadata: Dict[str, Any] | None = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| server_name: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_url: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_token: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Dict[str, Any]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = _resolve_gateway_url(server_registry, server_name, gateway_url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def ask_via_proxy(*args, **kwargs) -> Dict[str, Any]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # legacy: (server_registry, execution_id, prompt, metadata=None, ...) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if args and (args[0] is None or not isinstance(args[0], str)): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args = args[1:] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| execution_id, prompt, *rest = args | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| metadata = rest[0] if rest else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_url = kwargs.get("gateway_url") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gateway_token = kwargs.get("gateway_token") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url = f"{base}/internal/human/prompts" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def ask_via_proxy(*args, **kwargs) -> Dict[str, Any]: | |
| # legacy: (server_registry, execution_id, prompt, metadata=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, prompt, *rest = args | |
| metadata = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None) | |
| url = f"{base}/internal/human/prompts" | |
| async def ask_via_proxy(*args, **kwargs) -> Dict[str, Any]: | |
| # legacy: (server_registry, execution_id, prompt, metadata=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, prompt, *rest = args | |
| metadata = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| context_gateway_url = kwargs.get("context_gateway_url") | |
| base = _resolve_gateway_url( | |
| gateway_url=gateway_url, | |
| context_gateway_url=context_gateway_url | |
| ) | |
| url = f"{base}/internal/human/prompts" | |
| try: | |
| timeout = float(os.environ.get("MCP_GATEWAY_TIMEOUT", "10")) | |
| except ValueError: | |
| timeout = 10.0 | |
| # …rest of implementation… |
Outdated
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.
🛠️ Refactor suggestion
Apply context URL in notify wrapper
- gateway_url = kwargs.get("gateway_url")
- gateway_token = kwargs.get("gateway_token")
- base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None)
+ gateway_url = kwargs.get("gateway_url")
+ gateway_token = kwargs.get("gateway_token")
+ context_gateway_url = kwargs.get("context_gateway_url")
+ base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=context_gateway_url)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def notify_via_proxy(*args, **kwargs) -> bool: | |
| # legacy: (server_registry, execution_id, method, params=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, method, *rest = args | |
| params = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None) | |
| async def notify_via_proxy(*args, **kwargs) -> bool: | |
| # legacy: (server_registry, execution_id, method, params=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, method, *rest = args | |
| params = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| context_gateway_url = kwargs.get("context_gateway_url") | |
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=context_gateway_url) |
🤖 Prompt for AI Agents
In src/mcp_agent/mcp/client_proxy.py around lines 119 to 127, the
notify_via_proxy wrapper always passes context_gateway_url=None into
_resolve_gateway_url, so any provided context URL in kwargs is ignored; update
the wrapper to read context_gateway_url = kwargs.get("context_gateway_url") (or
similarly named key used by callers) and pass that value into
_resolve_gateway_url instead of None, preserving existing
gateway_url/gateway_token handling so resolution uses the provided context when
present.
Outdated
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.
🛠️ Refactor suggestion
Apply context URL in request wrapper
- gateway_url = kwargs.get("gateway_url")
- gateway_token = kwargs.get("gateway_token")
- base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None)
+ gateway_url = kwargs.get("gateway_url")
+ gateway_token = kwargs.get("gateway_token")
+ context_gateway_url = kwargs.get("context_gateway_url")
+ base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=context_gateway_url)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def request_via_proxy(*args, **kwargs) -> Dict[str, Any]: | |
| # legacy: (server_registry, execution_id, method, params=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, method, *rest = args | |
| params = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| base = _resolve_gateway_url(gateway_url=gateway_url, context_gateway_url=None) | |
| async def request_via_proxy(*args, **kwargs) -> Dict[str, Any]: | |
| # legacy: (server_registry, execution_id, method, params=None, ...) | |
| if args and (args[0] is None or not isinstance(args[0], str)): | |
| args = args[1:] | |
| execution_id, method, *rest = args | |
| params = rest[0] if rest else None | |
| gateway_url = kwargs.get("gateway_url") | |
| gateway_token = kwargs.get("gateway_token") | |
| context_gateway_url = kwargs.get("context_gateway_url") | |
| base = _resolve_gateway_url( | |
| gateway_url=gateway_url, | |
| context_gateway_url=context_gateway_url | |
| ) |
🤖 Prompt for AI Agents
In src/mcp_agent/mcp/client_proxy.py around lines 151 to 159, the request
wrapper currently calls _resolve_gateway_url with context_gateway_url=None so
any supplied context URL is ignored; update the call to pass the context URL
from the wrapper (e.g., read kwargs.get("context_gateway_url") or the
appropriate context variable) into context_gateway_url so the resolved base
reflects the provided context; ensure you fall back to None only if no context
is supplied.
Uh oh!
There was an error while loading. Please reload this page.