Skip to content

test: verify the SSE crawl transport end to end - #104

Merged
memtomem merged 1 commit into
mainfrom
test/verify-sse-transport
Sep 10, 2026
Merged

test: verify the SSE crawl transport end to end#104
memtomem merged 1 commit into
mainfrom
test/verify-sse-transport

Conversation

@memtomem

Copy link
Copy Markdown
Owner

What and why

ServerSpec has always accepted transport: sse, and crawler/transports.py
has always had a branch for it. 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
(#99) rewrote that module with SSE untested.

That is the failure shape this repo has been bitten by before: a broken script
sat green through 14 PR checks because only a scheduled workflow ran it. A
public surface looks supported because the type admits it.

The good news: SSE works. It was verified green at both mcp 2.1.1, the
declared floor, and 2.2.0, the current lockfile version. Nothing was broken.
This PR makes that a fact CI enforces rather than an assumption.

Changes

file change
tests/fixtures/sample_server.py new sse argv mode; without it SSE was untestable
tests/test_crawler.py _serve(mode) helper shared by both HTTP fixtures; new test_crawl_sse
scripts/verify_mcp_floor.py streamable_http() generalized to http_transport(mode, transport, path); sse leg added
tests/test_release_hardening.py pins credential redaction for SSE URLs
toolgraph/crawler/transports.py docstring only, no runtime change
README.md, CHANGELOG.md docs

The floor script is the deliberate home for the SSE leg. minimum-mcp is a
required PR check that installs the declared floor, so this cannot go stale the
way a schedule-only script can. It adds one server subprocess per invocation.

Review findings applied

Codex reviewed this and returned SHIP with no blockers and no majors. Two
minors were real and are fixed here:

  • The fixture teardown called proc.kill() without a following wait(), so
    teardown could return with the child still exiting and the next test racing
    it for a port. Pre-existing, but now shared by two fixtures.
  • The floor script built all its legs as coroutine objects up front, so a
    failing leg left the later ones unawaited and buried the real error under
    RuntimeWarnings. They are now built from factories.

Verification

uv run pytest -q                                      583 passed
uv run ruff check .                                   All checks passed
scripts/verify_mcp_floor.py @ mcp 2.2.0               [ok] sse lists tools
scripts/verify_mcp_floor.py @ mcp 2.1.1 (floor)       [ok] sse lists tools

Not covered

ServerSpec.headers is forwarded to upstream MCP servers by both HTTP
transports and is still never tested with a non-empty value. The two transports
use different SDK mechanisms there, so one can break without the other, and it
is the auth path. Filed as #103; this PR does not close it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ue4wAZWpL7PFe7ARenR7fz

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ue4wAZWpL7PFe7ARenR7fz
@memtomem
memtomem merged commit 2f7beb1 into main Sep 10, 2026
14 checks passed
@memtomem
memtomem deleted the test/verify-sse-transport branch September 10, 2026 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant