Skip to content

Commit 0e82f3b

Browse files
dwolfsonclaude
andauthored
Some small fixes (#344)
* docs(PYEGERIA_ISSUES): log ISSUE-91 -- mcp dependency floor too loose pyproject.toml declares mcp >=0.1 but pyegeria/core/mcp_server.py imports mcp.server.mcpserver.MCPServer, which requires mcp>=2.0 (confirmed live: this dev venv has mcp==2.0.0 installed and the import works; the module path doesn't exist in the 0.x/1.x mcp package line). Found by Egeria Advisor rebuilding its demo deployment against pyegeria 6.1.10 -- worked fine because they pinned mcp==2.1.1 explicitly, not because pyegeria's own floor would have caught an older mcp. Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> * test(view): cover ISSUE-86's caller-held bearer token in exec_report_spec format_set_executor.py's `token` parameter (ISSUE-86 — let a caller that already holds an Egeria bearer token run a report without a password) is on main with no dedicated test. These tests existed only on an unpushed local branch, claude/cranky-chebyshev-be2494, alongside a second, parallel implementation of the same feature that main had meanwhile gained by another route. Pushing that branch would have added a duplicate implementation; the tests were the part worth keeping, so they are salvaged here on their own. They were written against that branch's `_authenticate_client()` helper, which main does not have — verified they pass unchanged against main's inline implementation (7 passed), so they test the behaviour rather than that refactor's shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> --------- Signed-off-by: Dan Wolfson <dan.wolfson@pdr-associates.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent a013c79 commit 0e82f3b

2 files changed

Lines changed: 236 additions & 0 deletions

File tree

‎PYEGERIA_ISSUES.md‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,50 @@ security context changes; (2) quickstart content: give `generalnpa` read access
199199
elements its engine actions anchor to, or anchor those actions to elements the engine-host identity can
200200
read. Full draft: trellis session scratch `egeria-issue-engine-host-403-loop.md`.
201201

202+
### ISSUE-91: `pyegeria.core.mcp_server` imports `mcp.server.mcpserver` (mcp 2.x only) but `pyproject.toml` declares `mcp >=0.1` — any consumer that resolves mcp 1.x gets a server that dies at import
203+
204+
**Layer:** pyegeria packaging · **Status:** open · **Found:** 2026-09-05 (Egeria Advisor dev startup on the M3 Max)
205+
206+
Commit 2b39ba06 (2026-07-30, "migrate mcp_server.py to mcp 2.0.0's MCPServer") changed the
207+
server's import to:
208+
209+
```python
210+
from mcp.server.mcpserver import MCPServer
211+
```
212+
213+
That module exists only in `mcp >= 2.0.0` (1.x ships `mcp.server.fastmcp` instead). The
214+
dependency declaration was not updated and still reads `"mcp >=0.1"` — verified in the released
215+
6.1.5 wheel's METADATA (`Requires-Dist: mcp>=0.1`) and in the current `pyproject.toml` at 6.1.10.
216+
217+
**How it shows up.** trellis pinned `mcp>=1.0.0` and its lock resolved mcp 1.29.0, which satisfies
218+
pyegeria's declared range. Launching the server then fails before it can speak MCP:
219+
220+
```
221+
$ python -m pyegeria.core.mcp_server
222+
MCP import failed.
223+
File ".../pyegeria/core/mcp_server.py", line 24, in <module>
224+
from mcp.server.mcpserver import MCPServer
225+
ModuleNotFoundError: No module named 'mcp.server.mcpserver'
226+
```
227+
228+
A client sees only "No response from MCP server" because the traceback goes to stderr and the
229+
process exits without writing a JSON-RPC frame. With mcp 2.1.1 installed the same command answers
230+
`initialize` normally (`serverInfo.name = "pyegeria-mcp"`).
231+
232+
**Why it went unnoticed.** The quickstart containers run mcp 2.1.1 (pyegeria 6.1.9) and use their own
233+
`/app/mcp_server.py` mounted over SSE in `pyegeria_handler`, not this stdio module, so the
234+
egeria-workspaces path never exercised it. The egeria-python checkout's own venv also has mcp 2.0.0.
235+
236+
**Proposed fix (backward compatible for callers).** In `pyproject.toml` declare `"mcp >=2.0"`. Nothing
237+
else needs to change: the import is already 2.x-only, so raising the floor only turns a runtime
238+
import crash into a resolver error at install time. If 1.x support is wanted instead, gate the
239+
import (`try: from mcp.server.mcpserver import MCPServer except ImportError: from mcp.server.fastmcp
240+
import FastMCP as MCPServer`) — but the 2.x API differs beyond the class name, so the floor bump is
241+
the honest option.
242+
243+
**Consumer-side workaround applied in trellis (8441efb):** both packages now declare `mcp>=2.0.0`
244+
and the lock carries mcp 2.1.1, matching the quickstart containers.
245+
202246
### ISSUE-38 (PY-18): `count_relationships_between_elements("Exception")` (276) disagrees with `ClassificationExplorer.get_relationships("Exception")` (55)
203247

204248
**Update 2026-08-30, from the Egeria team (Mandy Chessell).** Leaving this
@@ -751,6 +795,37 @@ on an Egeria Server capability that doesn't exist yet — but the pyegeria/
751795
Dr.Egeria-side work each will need once that capability ships is written
752796
into the entry now, so it isn't rediscovered from scratch later.
753797

798+
### ISSUE-91: `pyproject.toml` declares `mcp >=0.1`, but `pyegeria.core.mcp_server` needs `mcp>=2.0` — the declared floor lets a resolver install a version too old to import the module at all
799+
800+
**Layer:** Pyegeria · **Status:** open · **Found:** 2026-09-06 (Egeria Advisor, containerized demo deployment rebuild against pyegeria 6.1.10).
801+
802+
`pyproject.toml`'s `[project.dependencies]` declares `"mcp >=0.1"`, but
803+
`pyegeria/core/mcp_server.py` imports `from mcp.server.mcpserver import
804+
MCPServer` — confirmed live in this checkout's dev venv:
805+
`importlib.metadata.version("mcp")` is `2.0.0`, and
806+
`mcp.server.mcpserver.MCPServer` only exists at that version; the module
807+
path is new to the 2.x line, not present in the 0.x/1.x `mcp` package
808+
history. A resolver that's free to pick anything satisfying `>=0.1` (no
809+
upper or tighter lower bound forcing 2.x) can legitimately land on a much
810+
older `mcp` release, at which point `import pyegeria.core.mcp_server`
811+
fails outright rather than degrading gracefully.
812+
813+
**Why it matters, found how:** a container rebuild of Egeria Advisor's
814+
demo deployment against pyegeria 6.1.10 pinned `mcp==2.1.1` explicitly
815+
(not resolved from pyegeria's own floor) and confirmed EA's MCP agent
816+
pre-warms cleanly on it — so 6.1.10 itself works fine when the caller
817+
pins a modern `mcp`, but nothing in pyegeria's own declared dependency
818+
would have caught a caller who didn't.
819+
820+
**Ask:** tighten `pyproject.toml`'s `mcp` constraint to actually match
821+
what `mcp_server.py` requires (`mcp>=2.0` at minimum — confirm the exact
822+
version `MCPServer`/`mcp.server.mcpserver` was introduced at, and pin to
823+
that) rather than the inherited placeholder `>=0.1` floor. Not
824+
independently verified against the `mcp` package's own changelog/git
825+
history here — the live import check above shows 2.0.0 works and the
826+
module path is absent from the 0.x/1.x line by inspection, but the exact
827+
first-working version wasn't pinned down.
828+
754829
### ISSUE-87: `ClassificationExplorer.add_ownership_to_element`'s docstring sample body says `"class": "OwnerProperties"` — the method itself only accepts `"OwnershipProperties"`, so the documented body cannot be sent
755830

756831
**Layer:** Pyegeria · **Status:** fixed 2026-09-05 (Pyegeria —
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
"""ISSUE-86: `exec_report_spec(..., token=...)` must authenticate the client it
2+
builds with `set_bearer_token(token)` instead of minting one from
3+
user/user_pass, on BOTH client-building paths (the format-row/find path and
4+
the analytic_function path). With no token, behaviour is unchanged
5+
(`create_egeria_bearer_token()`)."""
6+
import pytest
7+
8+
from pyegeria.view import format_set_executor as fse
9+
from pyegeria.core import mcp_adapter
10+
11+
12+
class _RecordingClient:
13+
"""Stands in for any OMVS client class the executor instantiates."""
14+
instances: list = []
15+
16+
def __init__(self, view_server, view_url, user_id=None, user_pwd=None, **kw):
17+
self.view_server = view_server
18+
self.platform_url = view_url
19+
self.user_id = user_id
20+
self.user_pwd = user_pwd
21+
self.token = None
22+
self.calls: list[str] = []
23+
_RecordingClient.instances.append(self)
24+
25+
def create_egeria_bearer_token(self, *a, **kw):
26+
self.calls.append("create")
27+
self.token = "minted"
28+
return self.token
29+
30+
def set_bearer_token(self, token):
31+
self.calls.append("set")
32+
self.token = token
33+
34+
def find_things(self, **kwargs):
35+
return [{"guid": "1", "displayName": "x"}]
36+
37+
38+
@pytest.fixture(autouse=True)
39+
def _reset_instances():
40+
_RecordingClient.instances = []
41+
yield
42+
_RecordingClient.instances = []
43+
44+
45+
_FIND_FMT = {
46+
"action": {
47+
"function": "Fake.find_things",
48+
"required_params": [],
49+
"optional_params": [],
50+
"spec_params": {},
51+
},
52+
"target_type": "Referenceable",
53+
}
54+
55+
_ANALYTIC_FMT = {
56+
"action": {"analytic_function": "fake.analytic", "analytic_spec_params": {}},
57+
"target_type": "Referenceable",
58+
}
59+
60+
61+
def _patch_find_path(monkeypatch):
62+
monkeypatch.setattr(fse, "select_report_spec", lambda name, out: _FIND_FMT)
63+
monkeypatch.setattr(fse, "get_report_registry", lambda: {})
64+
monkeypatch.setattr(fse, "_resolve_client_and_method",
65+
lambda decl: (_RecordingClient, "find_things"))
66+
67+
68+
def _patch_analytic_path(monkeypatch):
69+
def _analytic(client):
70+
return {"count": 3, "token_seen": client.token}
71+
monkeypatch.setattr(fse, "select_report_spec", lambda name, out: _ANALYTIC_FMT)
72+
monkeypatch.setattr(fse, "get_report_registry", lambda: {})
73+
monkeypatch.setattr(fse, "_resolve_analytic_function", lambda decl: _analytic)
74+
monkeypatch.setattr(fse, "EgeriaTech", _RecordingClient)
75+
76+
77+
# --- format-row (find_method) path -------------------------------------------
78+
79+
def test_find_path_with_token_uses_set_bearer_token(monkeypatch):
80+
_patch_find_path(monkeypatch)
81+
result = fse.exec_report_spec(
82+
"Anything", output_format="DICT", view_server="vs", view_url="https://x",
83+
user="svc", user_pass="svc-pw", token="user-token",
84+
)
85+
assert result["kind"] == "json"
86+
(client,) = _RecordingClient.instances
87+
assert client.calls == ["set"]
88+
assert client.token == "user-token"
89+
90+
91+
def test_find_path_without_token_mints_bearer_token(monkeypatch):
92+
_patch_find_path(monkeypatch)
93+
result = fse.exec_report_spec(
94+
"Anything", output_format="DICT", view_server="vs", view_url="https://x",
95+
user="svc", user_pass="svc-pw",
96+
)
97+
assert result["kind"] == "json"
98+
(client,) = _RecordingClient.instances
99+
assert client.calls == ["create"]
100+
assert client.user_id == "svc" and client.user_pwd == "svc-pw"
101+
102+
103+
# --- analytic_function path ---------------------------------------------------
104+
105+
def test_analytic_path_with_token_uses_set_bearer_token(monkeypatch):
106+
_patch_analytic_path(monkeypatch)
107+
result = fse.exec_report_spec(
108+
"Anything", output_format="DICT", view_server="vs", view_url="https://x",
109+
user="svc", user_pass="svc-pw", token="user-token",
110+
)
111+
assert result == {"kind": "json", "data": {"count": 3, "token_seen": "user-token"}}
112+
(client,) = _RecordingClient.instances
113+
assert client.calls == ["set"]
114+
115+
116+
def test_analytic_path_without_token_mints_bearer_token(monkeypatch):
117+
_patch_analytic_path(monkeypatch)
118+
result = fse.exec_report_spec(
119+
"Anything", output_format="DICT", view_server="vs", view_url="https://x",
120+
user="svc", user_pass="svc-pw",
121+
)
122+
assert result["data"]["token_seen"] == "minted"
123+
(client,) = _RecordingClient.instances
124+
assert client.calls == ["create"]
125+
126+
127+
def test_chart_path_threads_token(monkeypatch):
128+
"""SERIES/BAR/PIE dispatch to _exec_analytic_chart before the Format-row
129+
lookup -- the token must survive that hop too."""
130+
_patch_analytic_path(monkeypatch)
131+
monkeypatch.setattr(fse, "get_report_spec_heading", lambda name: "H", raising=False)
132+
fse.exec_report_spec(
133+
"Anything", output_format="BAR", view_server="vs", view_url="https://x",
134+
user="svc", user_pass="svc-pw", token="user-token",
135+
)
136+
(client,) = _RecordingClient.instances
137+
assert client.calls == ["set"]
138+
assert client.token == "user-token"
139+
140+
141+
# --- MCP adapter passthrough --------------------------------------------------
142+
143+
def test_mcp_run_report_forwards_token(monkeypatch):
144+
seen = {}
145+
146+
def _fake_exec(**kwargs):
147+
seen.update(kwargs)
148+
return {"kind": "empty"}
149+
150+
monkeypatch.setattr(mcp_adapter, "exec_report_spec", _fake_exec)
151+
mcp_adapter.run_report(report="R", user="u", user_pass="p", token="tok")
152+
assert seen["token"] == "tok"
153+
assert seen["user"] == "u" and seen["user_pass"] == "p"
154+
155+
156+
def test_mcp_run_report_default_token_is_none(monkeypatch):
157+
seen = {}
158+
monkeypatch.setattr(mcp_adapter, "exec_report_spec",
159+
lambda **kw: seen.update(kw) or {"kind": "empty"})
160+
mcp_adapter.run_report(report="R", user="u", user_pass="p")
161+
assert seen["token"] is None

0 commit comments

Comments
 (0)