test: cover SSE header forwarding at the declared SDK floor too - #106
Merged
Conversation
#105 left this gap on purpose and said so: `header_forwarding()` drove streamable-http only, so SSE header forwarding was proven at the lockfile version by pytest but not at the pinned floor. The two transports hand headers to the SDK by different spellings, which is the whole reason both need covering, so leaving one of them floor-unverified was the odd half of a pair. `header_forwarding()` now takes (mode, transport, path) the way `http_transport()` already did, and `main()` registers a leg for each. The floor script takes 5s and spawns four fixture servers. Fail fast when the server is up and refusing. The retry loop exists to wait out server STARTUP; a 401 means the server is already answering, so retrying just burned the 30s deadline and then died with a raw transport traceback instead of naming the cause. A dropped header now fails in 3s with "the server refused an authorized request -- headers are not reaching it". `_rejections_or_none()` exists because the obvious version of that fail-fast does not work: reading the tally during normal startup raises URLError, which broke the clean run. Found by running it. The wrapper returns None while the server is not answering yet, so "connection refused" is never mistaken for a verdict. Also retires two claims that this change makes false: the transports docstring no longer says the floor pins only streamable-http, and the comment explaining why the tally beats the exception now covers both transports rather than describing streamable-http's 401/500 collapse as if it were universal. Verified at mcp 2.1.1 and 2.2.0. Mutating either transport's header path makes the script exit 1 in 3s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ue4wAZWpL7PFe7ARenR7fz
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #105, which left this gap deliberately and said so in its body.
The odd half of a pair
ServerSpec.headersreaches the SDK by two different spellings: SSE passesheaders=straight tosse_client, while streamable-http cannot take thatkwarg and needs a client built by
create_mcp_http_client(headers=...)andhanded over as
http_client=. That difference is the entire reason both needcovering — either can stop carrying credentials without the other noticing.
After #105 the coverage was lopsided:
header_forwarding()now takes(mode, transport, path)the wayhttp_transport()already did, andmain()registers a leg for each. Everycell in that table is now filled.
No runtime change. The only file under
toolgraph/iscrawler/transports.py, and only its docstring, which had started claimingsomething this PR makes false.
Fail fast when the server is up and refusing
The authorized retry loop exists to wait out server startup. A 401 means the
server is already answering, so retrying was pure waste: it burned the full 30s
deadline and then died with a raw transport traceback instead of naming the
cause.
httpx2.HTTPStatusErrortraceback[FAIL] sse headers reach the server — the server refused an authorized request -- headers are not reaching it_rejections_or_none()is there because the obvious version of this does notwork. Reading the tally during normal startup raises
URLError: Connection refused, which broke the clean run. Found by running it, not by reading it.The wrapper returns
Nonewhile the server is not answering, so a refusedconnection is never mistaken for a verdict.
Verification
Mutation tests — each makes the script exit 1 in 3s, naming the transport:
sse_client(spec.url, headers=None)if spec.headers:→if False:minimum-mcpnow spawns four fixture servers, sequentially, for four distincttransport/auth combinations. Codex reviewed and returned SHIP with no findings.
Also retires a comment that this change made false: the explanation of why the
rejection tally beats the exception described streamable-http's 401/500 collapse
as if it were universal. SSE surfaces the 401 itself; the tally is what answers
the same way for both.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ue4wAZWpL7PFe7ARenR7fz