diff --git a/CHANGELOG.md b/CHANGELOG.md index 978bb0f..2b016b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,9 @@ `tests/vectors/mcp-binding-vectors.json` (the Node.js twin runs the same bytes) pins the digest, the allowlist verification and the result mapping; the rest of the tests drive a fixture MCP server of this repository over both transports, and the HTTP transport was driven on 2026-11-14 against a public MCP server (the Mermaid Chart endpoint): `describe` - listed its tool with the digest, and a bound route returned the server's structured content. **With it, `Platform` gains `on_startup(callback)` and `on_cleanup(callback)`** - async callbacks run + listed its tool with the digest, and a bound route returned the server's structured content. Every bound route is an alias of one `@preload` function, + `mcp.tool` (private, bound to no tool), registered at boot with the server's `instances`; the function reads the route it + was called as from its execution context (`get_trace().route`). **With it, `Platform` gains `on_startup(callback)` and `on_cleanup(callback)`** - async callbacks run inside the server's event loop before the first request and at shutdown, the place for an extension's asynchronous boot work - and `Platform.create_app()`, the hook-bearing application `run` serves and a test may serve with aiohttp's runner. **Upgrade note:** none - nothing changes for an application that registers no callback. diff --git a/extensions/mcp-binding/README.md b/extensions/mcp-binding/README.md index 625e1a2..30560f3 100644 --- a/extensions/mcp-binding/README.md +++ b/extensions/mcp-binding/README.md @@ -125,6 +125,14 @@ One session per server is shared by its routes, held by a task of its own for th `mcp.health` (the `/health` actuator, `mandatory.health.dependencies`) reports each server, its transport, its tool count and whether it is connected, and answers 503 naming a server whose session is gone. +Every bound route is served by one function, `mcp.tool`, declared with `@preload` like any composable function and private: +the host answers 403 for it on the wire, and it is bound to no tool. At boot, once a server's allowlist is verified, the +binding registers each allowlisted tool's route as an alias of that function - `default_registry.register(route, mcp_tool, +instances=...)`, the pack's counterpart of the engines' `Platform.register`, with the server's `instances` - so every route +is a service of its own, with its own mailbox and workers. A call learns the route it was addressed to from its execution +context (`get_trace().route`, the pack's counterpart of the engines' read-only `my_route` header), which names the server +and the remote tool; a call to `mcp.tool` by its own name is a 404. + ## Security - **No engine runs a child process.** The MCP client, the subprocess and the credentials live here, in the pack, behind diff --git a/extensions/mcp-binding/mcp_binding.py b/extensions/mcp-binding/mcp_binding.py index f2121fb..a4104e5 100644 --- a/extensions/mcp-binding/mcp_binding.py +++ b/extensions/mcp-binding/mcp_binding.py @@ -58,6 +58,7 @@ app_config, default_registry, get_logger, + get_trace, platform, preload, ) @@ -426,12 +427,14 @@ async def _list_tools(session: ClientSession) -> list[dict[str, Any]]: return offered -# --- the application: boot, the routes and the health check ------------------------------ +# --- the application: boot, the tool function and the health check ------------------------------ class Binding: def __init__(self) -> None: self.servers: dict[str, Server] = {} + # what each bound route serves - the server and the allowlisted tool - filled at boot + self.routes: dict[str, tuple[Server, ToolBinding]] = {} async def boot(self) -> None: """Connect every configured server, verify its allowlist and register the routes - before @@ -442,33 +445,50 @@ async def boot(self) -> None: server = Server(spec) await server.start() self.servers[spec.name] = server - for binding, _tool in server.bound: - default_registry.register(binding.route, _handler(server, binding), instances=spec.instances) - log.info("MCP server %s: tool %s served as %s", spec.name, binding.name, binding.route) - log.info("MCP binding ready - %d server(s), %d route(s)", len(self.servers), - sum(len(s.bound) for s in self.servers.values())) + for tool, _schema in server.bound: + # the route is an alias of mcp_tool - the one function under the configured + # name, the engines' Platform.register shape - with the server's instances + self.routes[tool.route] = (server, tool) + default_registry.register(tool.route, mcp_tool, instances=spec.instances) + log.info("MCP server %s: tool %s served as %s", spec.name, tool.name, tool.route) + log.info("MCP binding ready - %d server(s), %d route(s)", len(self.servers), len(self.routes)) except Exception as exc: # noqa: BLE001 - any boot failure stops the application, by name leaf = _leaf(exc) log.error("MCP binding cannot start - %s", leaf) await self.shutdown() raise SystemExit(1) from leaf + def bound(self, route: str | None) -> tuple[Server, ToolBinding]: + """The server and the allowlisted tool behind a bound route; a 404 for any other route.""" + found = self.routes.get(route or "") + if found is None: + raise AppException(404, f"No MCP tool is bound to the route {route or '(none)'}") + return found + async def shutdown(self) -> None: for server in list(self.servers.values()): await server.close() self.servers.clear() + self.routes.clear() -def _handler(server: Server, binding: ToolBinding): - async def call(_headers: dict[str, str], body: Body) -> Any: - if not isinstance(body, dict): - raise AppException(400, f"The arguments of {binding.name} must be a map") - return await server.call(binding.name, body) - call.__name__ = f"mcp_{binding.name}" - return call +binding = Binding() -binding = Binding() +@preload(route="mcp.tool", instances=DEFAULT_INSTANCES, private=True) +async def mcp_tool(_headers: dict[str, str], body: Body) -> Any: + """The one function behind every bound route. The boot registers each allowlisted tool's route + as an alias of this function (the engines' ``Platform.register``: one function, several + routes), so a call learns the route it was addressed to from its execution context - + ``get_trace().route``, the pack's counterpart of the engines' read-only ``my_route`` header - + and the route names the server and the remote tool. The event body is the tool's arguments, + a map; the reply is the result as ``tool_result`` shapes it. ``mcp.tool`` itself is private + and bound to no tool: it declares the function, and a call to it by that name is a 404.""" + info = get_trace() + server, tool = binding.bound(info.route if info else None) + if not isinstance(body, dict): + raise AppException(400, f"The arguments of {tool.name} must be a map") + return await server.call(tool.name, body) @preload(route="mcp.health", instances=5, private=True) diff --git a/memory/continuity.md b/memory/continuity.md index ac68aa8..6cebbdb 100644 --- a/memory/continuity.md +++ b/memory/continuity.md @@ -161,7 +161,14 @@ reply is the structured content else the blocks' text; `is_error` is a 400 carrying the text (the loop returns it to the model), a timeout 408, a protocol failure 502, a lost session 503. The pure half (digest, verification, result mapping) is pinned by `tests/vectors/mcp-binding-vectors.json`, shared byte-identical with the Node twin; the transports by the fixture - server `tests/fixtures/mcp_test_server.py`. **How to apply:** a new MCP tool for an agent is a server entry and an allowlist + server `tests/fixtures/mcp_test_server.py`. **The shape (2026-11-14, Eric's direction):** every bound route is an alias of + ONE `@preload` function, `mcp.tool` - private, bound to no tool - registered at boot with the server's `instances` through + `default_registry.register`, the pack's counterpart of the engines' `Platform.register`; the function reads the route it + was called as from `get_trace().route`, the pack's counterpart of the engines' read-only `my_route` header (the host strips + `my_*` headers at ingress and adds none). A generic client - one route, the server and the tool encoded in the call - was + suggested and set aside the same day: ADR-0033 rejects it as alternative (c), since the catalog cannot certify a target + hidden in the call; the certifiable form is an optional `headers` map on the catalog row, an engine change in both + engines, not taken. **How to apply:** a new MCP tool for an agent is a server entry and an allowlist row here plus a catalog row on the graph, never engine code; a schema change is a digest the developer re-reviews; a credential is a reference in the server file, resolved from this process's environment. diff --git a/memory/open-threads/thread-mcp-binding.md b/memory/open-threads/thread-mcp-binding.md index cb07e7f..26d6771 100644 --- a/memory/open-threads/thread-mcp-binding.md +++ b/memory/open-threads/thread-mcp-binding.md @@ -1,7 +1,10 @@ - [ ] **The MCP binding (RFC-0011 / ADR-0033 build plan step 4): `extensions/mcp-binding/`, built 2026-11-14; PR #51 MERGED 2026-11-14 (merge `ea3783d`).** The extension, the `Platform` startup and cleanup hooks, the `mcp` extra, the shared vector file (sha256 `4bacc319…`) and - 46 tests; the HTTP transport driven live against a public MCP server (the Mermaid Chart endpoint) the same day. Next: the Node twin (mercury-nodejs `extensions/mcp-binding`, the same vector bytes); the engine-side drive with an - MCP tool in a graph's catalog (tracked in mercury-composable `agent-loop-llm-run`); the pack's catch-up release carries it + 46 tests; the HTTP transport driven live against a public MCP server (the Mermaid Chart endpoint) the same day. The Node + twin landed the same day (mercury-nodejs #118, the same vector bytes), and the engine-side drive ran with an MCP tool in a + graph's catalog (mercury-composable #626 and #627, the `diagram-assistant` example on the Mermaid server). The `@preload` + shape - every bound route an alias of the one function `mcp.tool` - is PR #52 from branch `refactor/mcp-binding-preload` + (opened 2026-11-14; the Node twin of that shape is Eric's call). Next: the pack's catch-up release carries it ([[pack-catch-up-4-12-25]]). Fact: [[mcp-binding-route-per-tool]]. → serves: vision-mercury-python diff --git a/memory/sessions/2026-11-14-104712.md b/memory/sessions/2026-11-14-104712.md new file mode 100644 index 0000000..214e37b --- /dev/null +++ b/memory/sessions/2026-11-14-104712.md @@ -0,0 +1,57 @@ +# Session (2026-11-14T10:47:12.000Z) + +**Agent:** Claude Code +**Title:** The MCP binding's bound routes become aliases of one `@preload` function, `mcp.tool` + +## Context + +Follows 2026-11-14-073523. After the engine-side drive of the diagram assistant (mercury-composable #626 and #627), Eric +asked which function of this pack serves `diagram.render.mcp`. The answer was a closure: `_handler(server, binding)` +returned an inner `call` that `Binding.boot` registered with `default_registry.register`, one closure per allowlisted +tool. Eric asked for the composable shape instead: the function exposed with the `@preload` decorator, as `llm.chat` and +`mcp.health` are. + +## The design question, and Eric's ruling + +A `@preload` route is a literal at import time, while the binding's routes come from the servers file at boot, so one +declared function has to serve every bound route and learn which one it was called as. The pack's host strips `my_*` +headers at ingress and adds only `my_correlation_id`, so the engines' `my_route` header is not available to a function; +but the bus sets a `TraceInfo` whose `route` is the executing service's route for every execution, and `get_trace()` is +public - the pack's counterpart of the engines' `PostOffice.getRoute()`. No SDK change was needed. + +Eric then suggested a generic MCP client: one fixed route, the destination server encoded in the call, reusable for any +server. ADR-0033 (accepted the same day in mercury-composable) lists that shape as rejected alternative (c): a target hidden +in the call is one the catalog cannot certify. The certifiable form would be an optional `headers` map on the catalog row, +sent by the loop with each call - an engine change in both engines, with the shared vectors, both packs, the example and +the ADR amended. Offered the three shapes, Eric chose to keep one route per allowlisted tool and refactor the shape only. + +## What changed (branch `refactor/mcp-binding-preload`, PR #52) + +- `extensions/mcp-binding/mcp_binding.py`: `mcp_tool` is declared with `@preload(route="mcp.tool", instances=10, + private=True)`; it reads `get_trace().route`, resolves the server and the allowlisted tool through `Binding.bound(route)` + (404 for a route bound to none, `mcp.tool` itself included), checks that the body is a map (400) and calls the tool. + `Binding.boot` fills `routes` (route to server and tool) and registers each allowlisted tool's route as an alias of + `mcp_tool` with the server's `instances`; `shutdown` clears both tables. The `_handler` factory is gone. +- `tests/test_mcp_binding.py`: the calls go through `PostOffice` local RPC, the path behind the HTTP host, so the alias + dispatch and the route in the execution context are what the tests exercise; the `po` fixture closes the registry's bus + after each test (its workers bind to the loop of their first use); the `servers` fixture installs the fresh `Binding` + as the module's instance, the one `mcp_tool` and `mcp_health` read; the first test pins the shape: every bound route's + handler is `mcp_tool`, public, with the server's `instances` (3), `mcp.tool` private, a call to it a 404. +- `extensions/mcp-binding/README.md` and the CHANGELOG's Unreleased entry describe the shape. + +## Verification + +- `ruff check .` clean; `basedpyright` 0 errors when run with the venv's interpreter (`--pythonpath ~/sandbox/.venv/bin/python`; + without it the checker resolves no package and reports 62 environmental errors); `tests/test_mcp_binding.py` 46 passed; + the suite 397 passed, 64 skipped, as before the change. +- Live, through the serving path (`mercury-serve` with the Playground example's `mermaid-servers.yml`, the public Mermaid + Chart MCP server), four calls over Event-over-HTTP to `/api/event`: `diagram.render.mcp` answered 200 with the server's + structured content (`valid: true`; the host logged `Handled diagram.render.mcp status=200 exec_time=3597 ms`); `mcp.tool` + answered 403 `mcp.tool is private`; a body that is not a map 400 naming the tool; an unknown route 404 from the host. The + process was stopped afterwards. Lesson for the next drive: the PostOffice client posts to the URL as given, so the + endpoint is the full `/api/event` URL, not the origin. + +## Memory References + +- Relied on: mcp-binding-route-per-tool (the binding's contract, unchanged; edited with the shape and the generic-client ruling) +- Threads: mcp-binding (edited: the Node twin and the engine-side drive recorded as done, this refactor added) diff --git a/tests/test_mcp_binding.py b/tests/test_mcp_binding.py index da1d4f8..769838e 100644 --- a/tests/test_mcp_binding.py +++ b/tests/test_mcp_binding.py @@ -4,7 +4,8 @@ the pure half: the input-schema digest, the allowlist verification and the result mapping. The tests after the vector runs drive the real transports against the fixture server of tests/fixtures (a small MCP server on the official SDK, over stdio and over streamable HTTP): the boot that registers -one route per allowlisted tool and fails fast by name, the calls through the registered handlers, the +one route per allowlisted tool and fails fast by name, the calls through the bound routes (local RPC +through the bus, as the host dispatches them: every route is an alias of the one @preload function), the credential references that reach the subprocess and never a log, the health route, the describe output, and the platform's startup hook that runs the boot before the first request. """ @@ -21,14 +22,23 @@ import subprocess import sys import time +from collections.abc import AsyncIterator from pathlib import Path from typing import Any import pytest +import pytest_asyncio import yaml from aiohttp import web -from mercury_composable import AppException, app_config, default_registry, platform +from mercury_composable import ( + AppException, + EventEnvelope, + PostOffice, + app_config, + default_registry, + platform, +) _TESTS = Path(__file__).resolve().parent _BINDING = _TESTS.parent / "extensions" / "mcp-binding" / "mcp_binding.py" @@ -125,56 +135,76 @@ def _stdio(offered: dict[str, dict[str, Any]], tools: dict[str, str], **extra: A return entry -async def _call(route: str, body: Any) -> Any: - service = default_registry.get(route) - assert service is not None, f"{route} is not registered" - return await service.handler({}, body) +@pytest_asyncio.fixture +async def po() -> AsyncIterator[PostOffice]: + """Local RPC through the registry's bus, the path a request takes behind the HTTP host: the bound + route's alias dispatches mcp_tool with the route in its execution context.""" + office = PostOffice(registry=default_registry) + yield office + # the bus binds a route's workers to the loop of their first use - drop them for the next test's loop + await default_registry.bus.close() + + +async def _call(po: PostOffice, route: str, body: Any) -> EventEnvelope: + assert default_registry.get(route) is not None, f"{route} is not registered" + return await po.request(route, body=body, timeout_ms=10000) @pytest.fixture def servers(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): - """Point the binding at a temporary server file and give the test a fresh Binding.""" + """Point the binding at a temporary server file and give the test a fresh Binding, installed as + the module's one instance - the one mcp_tool and mcp_health read.""" def point(entries: dict[str, Any]) -> Any: path = _servers_file(tmp_path, entries) app_config().set("mcp.servers", str(path)) - return binding.Binding() + app = binding.Binding() + monkeypatch.setattr(binding, "binding", app) + return app yield point app_config().set("mcp.servers", binding.DEFAULT_SERVERS_FILE) @pytest.mark.asyncio async def test_boot_registers_one_route_per_allowlisted_tool_and_calls_them( - offered: dict[str, dict[str, Any]], servers: Any) -> None: - app = servers({"fixture": _stdio(offered, {"echo": "fixture.echo", "add": "fixture.add", "fail": "fixture.fail"})}) + offered: dict[str, dict[str, Any]], servers: Any, po: PostOffice) -> None: + app = servers({"fixture": _stdio(offered, {"echo": "fixture.echo", "add": "fixture.add", "fail": "fixture.fail"}, + instances=3)}) await app.boot() try: assert sorted(app.servers) == ["fixture"] assert [b.route for b, _ in app.servers["fixture"].bound] == ["fixture.echo", "fixture.add", "fixture.fail"] - assert await _call("fixture.echo", {"text": "hello"}) == "hello" - assert await _call("fixture.add", {"a": 1, "b": 2}) == {"sum": 3} - with pytest.raises(AppException) as failure: - await _call("fixture.fail", {"reason": "the vault is closed"}) - assert failure.value.status == 400 - assert "the vault is closed" in failure.value.message - with pytest.raises(AppException) as not_a_map: - await _call("fixture.echo", "hello") - assert not_a_map.value.status == 400 - assert "must be a map" in not_a_map.value.message + # the composable shape: one @preload function, each bound route a public alias of it with the + # server's instances; the declared route itself is private and bound to no tool + for route in ("fixture.echo", "fixture.add", "fixture.fail"): + service = default_registry.get(route) + assert service is not None and service.handler is binding.mcp_tool and not service.private + assert service.instances == 3 + declared = default_registry.get("mcp.tool") + assert declared is not None and declared.handler is binding.mcp_tool and declared.private + echo = await _call(po, "fixture.echo", {"text": "hello"}) + assert echo.get_status() == 200 and echo.body == "hello" + assert (await _call(po, "fixture.add", {"a": 1, "b": 2})).body == {"sum": 3} + failed = await _call(po, "fixture.fail", {"reason": "the vault is closed"}) + assert failed.get_status() == 400 and "the vault is closed" in str(failed.body) + not_a_map = await _call(po, "fixture.echo", "hello") + assert not_a_map.get_status() == 400 and "must be a map" in str(not_a_map.body) + unbound = await _call(po, "mcp.tool", {"text": "no tool here"}) + assert unbound.get_status() == 404 and "No MCP tool is bound to the route mcp.tool" in str(unbound.body) finally: await app.shutdown() - assert not app.servers + assert not app.servers and not app.routes @pytest.mark.asyncio -async def test_a_slow_tool_is_a_408_within_the_servers_timeout(offered: dict[str, dict[str, Any]], servers: Any) -> None: +async def test_a_slow_tool_is_a_408_within_the_servers_timeout(offered: dict[str, dict[str, Any]], servers: Any, + po: PostOffice) -> None: app = servers({"fixture": _stdio(offered, {"slow": "fixture.slow"}, timeout_ms=1000)}) await app.boot() try: started = time.monotonic() - with pytest.raises(AppException) as failure: - await _call("fixture.slow", {"seconds": 5}) - assert failure.value.status == 408, failure.value.message - assert "within 1000 ms" in failure.value.message + reply = await _call(po, "fixture.slow", {"seconds": 5}) + assert reply.get_status() == 408, reply.body + assert "within 1000 ms" in str(reply.body) assert time.monotonic() - started < 4 finally: await app.shutdown() @@ -208,15 +238,15 @@ async def test_a_changed_schema_fails_the_boot_by_name(offered: dict[str, dict[s @pytest.mark.asyncio async def test_credential_references_reach_the_subprocess_and_never_a_log( offered: dict[str, dict[str, Any]], servers: Any, monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture) -> None: + caplog: pytest.LogCaptureFixture, po: PostOffice) -> None: monkeypatch.setenv("FIXTURE_TOKEN", "a-value-that-must-not-appear") app = servers({"fixture": _stdio(offered, {"env_present": "fixture.env.present"}, env={"FIXTURE_FORWARDED": "${FIXTURE_TOKEN}"})}) with caplog.at_level(logging.DEBUG): await app.boot() try: - assert await _call("fixture.env.present", {"name": "FIXTURE_FORWARDED"}) == {"present": True} - assert await _call("fixture.env.present", {"name": "FIXTURE_TOKEN"}) == {"present": False}, \ + assert (await _call(po, "fixture.env.present", {"name": "FIXTURE_FORWARDED"})).body == {"present": True} + assert (await _call(po, "fixture.env.present", {"name": "FIXTURE_TOKEN"})).body == {"present": False}, \ "only the listed variables reach the subprocess, not the whole environment" finally: await app.shutdown() @@ -309,7 +339,8 @@ def http_fixture_port(): @pytest.mark.asyncio async def test_the_http_transport_serves_the_same_routes(offered: dict[str, dict[str, Any]], servers: Any, - monkeypatch: pytest.MonkeyPatch, http_fixture_port: int) -> None: + monkeypatch: pytest.MonkeyPatch, http_fixture_port: int, + po: PostOffice) -> None: monkeypatch.setenv("FIXTURE_HTTP_TOKEN", "bearer-value") app = servers({"remote": { "transport": "http", "url": f"http://127.0.0.1:{http_fixture_port}/mcp", @@ -319,8 +350,8 @@ async def test_the_http_transport_serves_the_same_routes(offered: dict[str, dict }}) await app.boot() try: - assert await _call("remote.echo", {"text": "over http"}) == "over http" - assert await _call("remote.add", {"a": 20, "b": 22}) == {"sum": 42} + assert (await _call(po, "remote.echo", {"text": "over http"})).body == "over http" + assert (await _call(po, "remote.add", {"a": 20, "b": 22})).body == {"sum": 42} finally: await app.shutdown() @@ -330,14 +361,12 @@ async def test_health_reports_each_server_and_its_connection(offered: dict[str, app = servers({"fixture": _stdio(offered, {"echo": "fixture.echo"})}) await app.boot() try: - binding.binding.servers = app.servers info = await binding.mcp_health({"type": "info"}, {}) assert info["service"] == "mcp.binding" assert info["servers"] == [{"name": "fixture", "transport": "stdio", "tools": 1, "connected": True}] assert await binding.mcp_health({"type": "health"}, {}) == "mcp.binding is running fine" finally: await app.shutdown() - binding.binding.servers = app.servers assert (await binding.mcp_health({"type": "info"}, {}))["servers"] == [] lost = binding.Server(binding.ServerSpec(name="gone", transport="stdio", tools=(), command="x")) binding.binding.servers = {"gone": lost}