Skip to content

Commit 34f2992

Browse files
committed
Fix MCP get_dataset_details crash on display-apps serialization
MCP tools constructed a bare WorkRequestContext, which has no .request attribute. HDASerializer.serialize_old_display_applications reads trans.request.base to build absolute display-app URLs, so any tool running detailed HDA serialization crashed with AttributeError when enable_old_display_applications was enabled. Switch to SessionRequestContext backed by minimal _StaticRequest / _StaticResponse stubs derived from galaxy_infrastructure_url (the same base URL already used for the MCP URL builder).
1 parent 3e7b3ed commit 34f2992

2 files changed

Lines changed: 131 additions & 4 deletions

File tree

lib/galaxy/webapps/galaxy/api/mcp.py

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,26 @@
77

88
import logging
99
from contextlib import contextmanager
10-
from typing import Any
10+
from typing import (
11+
Any,
12+
Optional,
13+
)
14+
from urllib.parse import urlparse
1115

1216
from fastmcp import (
1317
Context as MCPContext,
1418
FastMCP,
1519
settings as fastmcp_settings,
1620
)
21+
from starlette.datastructures import URL
1722

1823
from galaxy.agents.operations import AgentOperationsManager
1924
from galaxy.managers.users import UserManager
20-
from galaxy.work.context import WorkRequestContext
25+
from galaxy.work.context import (
26+
GalaxyAbstractRequest,
27+
GalaxyAbstractResponse,
28+
SessionRequestContext,
29+
)
2130

2231
logger = logging.getLogger(__name__)
2332

@@ -72,6 +81,68 @@ def __call__(self, name: str, **path_params):
7281
return MCPUrlBuilder(fallback_base_url)
7382

7483

84+
class _StaticRequest(GalaxyAbstractRequest):
85+
"""Minimal GalaxyAbstractRequest backed by a configured base URL.
86+
87+
MCP tools run outside of an HTTP request, but downstream serializers
88+
(notably HDASerializer.serialize_old_display_applications) read
89+
``trans.request.base`` to build absolute display-app URLs.
90+
"""
91+
92+
def __init__(self, base_url: str) -> None:
93+
# Match GalaxyASGIRequest.base shape (Starlette base_url always has trailing slash).
94+
self._base = base_url if base_url.endswith("/") else f"{base_url}/"
95+
self._parsed = urlparse(self._base)
96+
97+
@property
98+
def base(self) -> str:
99+
return self._base
100+
101+
@property
102+
def url_path(self) -> str:
103+
return self._base
104+
105+
@property
106+
def host(self) -> str:
107+
return self._parsed.netloc
108+
109+
@property
110+
def is_secure(self) -> bool:
111+
return self._parsed.scheme == "https"
112+
113+
def get_cookie(self, name: str) -> Optional[str]:
114+
return None
115+
116+
@property
117+
def url(self) -> URL:
118+
return URL(self._base)
119+
120+
121+
class _StaticResponse(GalaxyAbstractResponse):
122+
"""No-op GalaxyAbstractResponse for MCP contexts (no HTTP response surface)."""
123+
124+
def __init__(self) -> None:
125+
self._headers: dict = {}
126+
127+
@property
128+
def headers(self) -> dict:
129+
return self._headers
130+
131+
def set_cookie(
132+
self,
133+
key: str,
134+
value: str = "",
135+
max_age: Optional[int] = None,
136+
expires: Optional[int] = None,
137+
path: str = "/",
138+
domain: Optional[str] = None,
139+
secure: bool = False,
140+
httponly: bool = False,
141+
samesite: Optional[str] = "lax",
142+
) -> None:
143+
return None
144+
145+
75146
@contextmanager
76147
def _mcp_error_handler(operation: str):
77148
"""Standard error handling for MCP tool calls."""
@@ -90,7 +161,7 @@ def get_mcp_app(gx_app):
90161

91162
mcp = FastMCP("Galaxy")
92163

93-
base_url = getattr(gx_app.config, "galaxy_infrastructure_url", "http://localhost:8080")
164+
base_url = gx_app.config.galaxy_infrastructure_url
94165
user_manager = UserManager(gx_app)
95166

96167
def get_operations_manager(api_key: str, ctx: MCPContext) -> AgentOperationsManager:
@@ -109,7 +180,13 @@ def get_operations_manager(api_key: str, ctx: MCPContext) -> AgentOperationsMana
109180
)
110181

111182
url_builder = get_mcp_url_builder(base_url)
112-
trans = WorkRequestContext(app=gx_app, user=user, url_builder=url_builder)
183+
trans = SessionRequestContext(
184+
app=gx_app,
185+
user=user,
186+
url_builder=url_builder,
187+
request=_StaticRequest(base_url),
188+
response=_StaticResponse(),
189+
)
113190

114191
return AgentOperationsManager(app=gx_app, trans=trans)
115192

test/unit/webapps/api/test_mcp.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
"""Unit tests for the MCP request/response stubs.
2+
3+
Regression coverage for the bug where MCP tools constructed a bare
4+
``WorkRequestContext`` (no ``.request`` attribute), causing
5+
``HDASerializer.serialize_old_display_applications`` to raise
6+
``AttributeError: 'WorkRequestContext' object has no attribute 'request'``
7+
whenever ``enable_old_display_applications`` was set.
8+
"""
9+
10+
from galaxy.webapps.galaxy.api.mcp import (
11+
_StaticRequest,
12+
_StaticResponse,
13+
)
14+
from galaxy.work.context import (
15+
GalaxyAbstractRequest,
16+
GalaxyAbstractResponse,
17+
)
18+
19+
20+
def test_static_request_implements_abstract_interface():
21+
request = _StaticRequest("http://localhost:8080")
22+
assert isinstance(request, GalaxyAbstractRequest)
23+
24+
25+
def test_static_request_base_always_has_trailing_slash():
26+
# Matches GalaxyASGIRequest.base shape (Starlette base_url has trailing slash).
27+
assert _StaticRequest("http://localhost:8080").base == "http://localhost:8080/"
28+
assert _StaticRequest("http://localhost:8080/").base == "http://localhost:8080/"
29+
assert _StaticRequest("https://example.org/galaxy").base == "https://example.org/galaxy/"
30+
31+
32+
def test_static_request_host_and_security():
33+
insecure = _StaticRequest("http://localhost:8080")
34+
assert insecure.host == "localhost:8080"
35+
assert insecure.is_secure is False
36+
37+
secure = _StaticRequest("https://galaxy.example.org")
38+
assert secure.host == "galaxy.example.org"
39+
assert secure.is_secure is True
40+
41+
42+
def test_static_request_get_cookie_returns_none():
43+
assert _StaticRequest("http://localhost:8080").get_cookie("anything") is None
44+
45+
46+
def test_static_response_implements_abstract_interface():
47+
response = _StaticResponse()
48+
assert isinstance(response, GalaxyAbstractResponse)
49+
assert response.headers == {}
50+
response.set_cookie("k", "v")

0 commit comments

Comments
 (0)