From c61efc4fcdca2304d20010b03615ea90f05fc2c9 Mon Sep 17 00:00:00 2001 From: memtomem <118508877+memtomem@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:37:20 +0900 Subject: [PATCH] test: verify the SSE crawl transport end to end `ServerSpec` has accepted `transport: sse` since the beginning and `crawler/transports.py` has always had a branch for it, but nothing in the repository ever executed that branch. No test, no script, no CI job constructed a spec with `transport="sse"`, and the fixture server could not speak SSE at all, so the branch could not have been covered even by accident. The MCP 2.x migration rewrote that module with SSE untested. This is the same failure shape the `minimum-mcp` job was hardened against: a public surface that looks supported because the type admits it. The fixture server grows an `sse` mode, `test_crawl_sse` crawls it for real through `crawl_server`, and `verify_mcp_floor.py` generalizes its streamable-http leg into `http_transport(mode, transport, path)` so both HTTP transports are driven at the declared SDK floor. Putting it there is deliberate: the `minimum-mcp` job is a required PR check, so this cannot go stale the way a schedule-only script can. Verified green at both mcp 2.1.1 (the declared floor) and 2.2.0. No runtime behaviour changes; `transports.py` is docstring-only. Also from review: - reap the fixture child after `kill()`, or teardown returns while it is still exiting and the next test races it for a port - build the floor script's legs from factories, so a failing leg does not leave later coroutines unawaited and bury the real error in RuntimeWarnings Headers forwarding stays unexercised on both HTTP transports; filed as #103. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Ue4wAZWpL7PFe7ARenR7fz --- CHANGELOG.md | 11 +++++++ README.md | 5 +-- scripts/verify_mcp_floor.py | 35 ++++++++++++++------- tests/fixtures/sample_server.py | 7 +++-- tests/test_crawler.py | 55 +++++++++++++++++++++++++++++---- tests/test_release_hardening.py | 6 ++++ toolgraph/crawler/transports.py | 10 ++++-- 7 files changed, 105 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d1e4f4..5e58035 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to Toolgraph are documented here. +## Unreleased + +### Fixed + +- The `sse` crawl transport is now verified end to end. `ServerSpec` has always + accepted `transport: sse`, but no test, script or CI job ever opened an SSE + session, so the branch survived the MCP 2.x migration unexercised. The test + fixture server can now serve SSE, `tests/test_crawler.py` crawls it, and + `scripts/verify_mcp_floor.py` drives it at the declared SDK floor alongside + streamable-http. Endpoint redaction for SSE URLs is pinned by a test. + ## 0.1.0 - 2026-09-10 Second alpha. Upgrading from `0.0.1` takes three things: the MCP SDK 2.x, one diff --git a/README.md b/README.md index 65eaf54..5e1dbdb 100644 --- a/README.md +++ b/README.md @@ -574,7 +574,8 @@ All thirteen are pure graph reads and advertise `readOnlyHint` / ## Inputs -- `servers.yaml` — MCP servers to crawl (stdio `command`/`args`, or http `url`). +- `servers.yaml` — MCP servers to crawl (stdio `command`/`args`, or a `url` for + `streamable-http`; `sse` is accepted for legacy servers that predate it). - `governance.yaml` — authored agents, policies, `CAN_CALL` grants, `READS/WRITES` data access, `GOVERNED_BY` mappings, and `expected_exceptions` for intentional DENY-zone reach. Per-edge `provenance` is optional everywhere. See `examples/`. @@ -596,7 +597,7 @@ Two reference exhibits: ``` toolgraph/ - crawler/ connect to MCP servers (stdio + streamable-http), enumerate tools/resources + crawler/ connect to MCP servers (stdio, streamable-http, legacy sse), enumerate tools/resources graph/ Neo4j driver, schema/constraints, idempotent loader, governance/audit queries manifest/ parse + ingest authored governance (idempotent) server/ MCP server exposing the queries as MCP tools diff --git a/scripts/verify_mcp_floor.py b/scripts/verify_mcp_floor.py index d8ded32..508c9f7 100644 --- a/scripts/verify_mcp_floor.py +++ b/scripts/verify_mcp_floor.py @@ -4,8 +4,9 @@ The `minimum-mcp` CI job installs the declared floor and needs to prove the floor SPEAKS the protocol, not merely that the package imports. A crawl over stdio does not reach the pieces the 2.x migration changed most: the server's -`call_tool` override and its error envelope, a continuation cursor, and the -streamable-http transport. Those are what this script drives. +`call_tool` override and its error envelope, a continuation cursor, and the two +HTTP transports (streamable-http and legacy SSE). Those are what this script +drives. Run it against any interpreter that has toolgraph installed:: @@ -115,16 +116,20 @@ def _free_port() -> int: return s.getsockname()[1] -async def streamable_http() -> None: - """The transport the crawl over stdio never touches.""" +async def http_transport(mode: str, transport: str, path: str) -> None: + """Drive one HTTP-ish transport; the crawl over stdio never touches these. + + `mode` is the fixture server's argv verb, `transport` the ServerSpec value + and `path` the endpoint the SDK's server mounts for it. + """ from toolgraph.crawler.transports import open_session from toolgraph.models import ServerSpec port = _free_port() - proc = subprocess.Popen([sys.executable, str(FIXTURE), "http", str(port)]) + proc = subprocess.Popen([sys.executable, str(FIXTURE), mode, str(port)]) try: - url = f"http://127.0.0.1:{port}/mcp" - spec = ServerSpec(name="floor", transport="streamable-http", url=url) + url = f"http://127.0.0.1:{port}{path}" + spec = ServerSpec(name="floor", transport=transport, url=url) deadline = time.monotonic() + 30 while True: if proc.poll() is not None: @@ -139,7 +144,7 @@ async def streamable_http() -> None: async with open_session(spec) as session: await session.initialize() listed = await session.list_tools() - check("streamable-http lists tools", + check(f"{transport} lists tools", {t.name for t in listed.tools} == {"read_file", "write_file"}) return @@ -162,11 +167,17 @@ async def main() -> int: print(f"verifying the MCP surface against mcp {version('mcp')}") del mcp - for name, coro in (("server boundary", server_boundary()), - ("pagination", pagination()), - ("streamable-http", streamable_http())): + # Factories, not coroutine objects: a leg that fails raises out of the loop, + # and eagerly-built coroutines for the later legs would then be garbage + # collected unawaited, burying the real failure under RuntimeWarnings. + for name, make_coro in ( + ("server boundary", server_boundary), + ("pagination", pagination), + ("streamable-http", lambda: http_transport("http", "streamable-http", "/mcp")), + ("sse", lambda: http_transport("sse", "sse", "/sse")), + ): print(f"{name}:") - await coro + await make_coro() print("floor verification passed") return 0 diff --git a/tests/fixtures/sample_server.py b/tests/fixtures/sample_server.py index 10dd4be..71bebfc 100644 --- a/tests/fixtures/sample_server.py +++ b/tests/fixtures/sample_server.py @@ -1,7 +1,8 @@ -"""A tiny MCP server used to exercise the crawler. Runs over stdio or http. +"""A tiny MCP server used to exercise the crawler. Runs over stdio, http or sse. python sample_server.py # stdio (default) - python sample_server.py http 8077 # streamable-http on :8077 + python sample_server.py http 8077 # streamable-http on :8077, endpoint /mcp + python sample_server.py sse 8078 # legacy HTTP+SSE on :8078, endpoint /sse """ import sys @@ -38,5 +39,7 @@ def customers() -> str: server = build() if transport == "http": server.run(transport="streamable-http", host="127.0.0.1", port=port) + elif transport == "sse": + server.run(transport="sse", host="127.0.0.1", port=port) else: server.run() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 4f17253..ee0659e 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -1,4 +1,4 @@ -"""Phase 2: crawl a real MCP server over stdio and streamable-http.""" +"""Phase 2: crawl a real MCP server over stdio, streamable-http and SSE.""" from __future__ import annotations @@ -6,6 +6,7 @@ import subprocess import sys import time +from contextlib import contextmanager from pathlib import Path import pytest @@ -47,32 +48,56 @@ async def test_crawl_then_load_creates_edges(graph, stdio_spec): } -@pytest.fixture -def http_url(): +@contextmanager +def _serve(mode: str): + """Run the fixture server in `mode` and yield its port. + + Shared by every HTTP-ish transport so streamable-http and SSE cannot drift + apart in how they are started, waited on, or torn down. + """ # Pick an ephemeral free port instead of a hardcoded one: a busy CI # runner with 8077 taken made this fixture flaky. The tiny window # between closing the probe socket and the server binding is accepted. with socket.socket() as probe: probe.bind(("127.0.0.1", 0)) port = probe.getsockname()[1] - proc = subprocess.Popen([sys.executable, FIXTURE, "http", str(port)]) + proc = subprocess.Popen([sys.executable, FIXTURE, mode, str(port)]) deadline = time.time() + 25 try: while time.time() < deadline: + if proc.poll() is not None: + raise RuntimeError( + f"sample {mode} server exited early with code {proc.returncode}" + ) try: with socket.create_connection(("127.0.0.1", port), timeout=0.5): break except OSError: time.sleep(0.3) else: - raise RuntimeError("sample http server did not start") - yield f"http://127.0.0.1:{port}/mcp" + raise RuntimeError(f"sample {mode} server did not start") + yield port finally: proc.terminate() try: proc.wait(timeout=5) except subprocess.TimeoutExpired: + # Reap after the kill too, or teardown returns with the child still + # exiting and the next test races it for a port. proc.kill() + proc.wait(timeout=10) + + +@pytest.fixture +def http_url(): + with _serve("http") as port: + yield f"http://127.0.0.1:{port}/mcp" + + +@pytest.fixture +def sse_url(): + with _serve("sse") as port: + yield f"http://127.0.0.1:{port}/sse" async def test_crawl_streamable_http(http_url): @@ -81,6 +106,24 @@ async def test_crawl_streamable_http(http_url): assert sorted(t.name for t in result.tools) == ["read_file", "write_file"] +async def test_crawl_sse(sse_url): + """`transport: sse` is accepted by ServerSpec, so it has to actually crawl. + + Legacy HTTP+SSE is superseded by streamable-http, but the config surface + still admits it and the SDK still ships it, so the branch in + `crawler/transports.py` needs a real server behind it. + """ + spec = ServerSpec(name="sample-sse", transport="sse", url=sse_url) + result = await crawl_server(spec) + + assert sorted(t.name for t in result.tools) == ["read_file", "write_file"] + assert [r.uri for r in result.resources] == ["file:///data/customers.csv"] + assert result.transport == "sse" + # The endpoint recorded on the result is the redacted origin, never the path. + assert result.endpoint.startswith("http://127.0.0.1:") + assert "/sse" not in result.endpoint + + # --- pagination: Codex PR #1 review -------------------------------------- diff --git a/tests/test_release_hardening.py b/tests/test_release_hardening.py index 9a49dc1..0b45904 100644 --- a/tests/test_release_hardening.py +++ b/tests/test_release_hardening.py @@ -100,10 +100,16 @@ def test_endpoint_labels_never_include_args_or_url_secrets(): transport="streamable-http", url="https://alice:secret@example.test:8443/mcp?token=super-secret#fragment", ) + sse = ServerSpec( + transport="sse", + url="https://alice:secret@example.test:8443/sse?token=super-secret#fragment", + ) assert endpoint_label(stdio) == "stdio:python" assert spec_label(stdio) == "stdio:python" assert endpoint_label(http) == "https://example.test:8443" + # SSE URLs carry the same credentials and query tokens as streamable-http. + assert endpoint_label(sse) == "https://example.test:8443" assert persisted_endpoint("stdio", "python server.py --token=secret") == "stdio:python" diff --git a/toolgraph/crawler/transports.py b/toolgraph/crawler/transports.py index 3421641..4e59dc2 100644 --- a/toolgraph/crawler/transports.py +++ b/toolgraph/crawler/transports.py @@ -2,8 +2,14 @@ Normalizes the transport differences the SDK exposes. Under mcp 2.x every transport yields the same 2-tuple ``(read, write)``; the session-id getter -streamable-http used to return as a third element is gone. SSE is supported -only as a deprecated fallback. +streamable-http used to return as a third element is gone. + +SSE is the legacy HTTP transport the MCP spec superseded with streamable-http; +prefer streamable-http for anything new. The SDK still ships SSE and does not +warn on it, so the branch below stays supported and is exercised for real by +``tests/test_crawler.py::test_crawl_sse`` and by the ``sse`` leg of +``scripts/verify_mcp_floor.py``, which the ``minimum-mcp`` CI job runs at the +declared SDK floor. Uses the preferred ``streamable_http_client``; since it takes headers via a custom httpx client rather than a ``headers=`` kwarg, we build one with the