-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Bump conformance harness to 0.2.0-alpha.11 #3282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+19
−2
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The bare
next()on line 268 has no default, so ifjson_schema_2020_12_toolis ever absent from thetools/listresult (a future harness pin bump renaming the fixture, or the SDK's x-mcp-header filtering dropping it), the escapingStopIterationsurfaces as an opaqueRuntimeError: coroutine raised StopIterationthat names neither the missing tool nor the listing. Considernext((t for t in listed.tools if t.name == "json_schema_2020_12_tool"), None)followed by an explicitRuntimeErrorlisting the surfaced tool names, matching this file's existing precondition-guard convention.Extended reasoning...
What the issue is.
run_json_schema_2020_12_preservationlocates the focal tool with a barenext(tool for tool in listed.tools if tool.name == "json_schema_2020_12_tool")— no default argument. When the generator is exhausted without a match,next()raisesStopIteration. Because this happens inside a coroutine, PEP 479 semantics kick in: aStopIterationescaping a coroutine frame is converted by the interpreter intoRuntimeError: coroutine raised StopIteration. The resulting traceback names neither the missing fixture tool nor thetools/listresult — it reads as interpreter internals, not as "the harness fixture wasn't found".The code path that triggers it. The scenario handler lists tools via
client.list_tools()and then searches for the harness-owned fixture tool by name. Two realistic triggers can make that search come up empty: (1) a future harness pin bump renamesjson_schema_2020_12_tool— this file exists precisely to track the pinned harness, and the workflow comment (conformance.yml) instructs bumping deliberately and reconciling baselines, so pin bumps are this file's expected lifecycle; (2) the SDK's own tool filtering drops the tool from the listing —run_http_invalid_tool_headersin this same file documents that the SDK filters tools with malformedx-mcp-headerannotations out oflist_toolsresults, so a filtering change in_absorb_tool_listing(src/mcp/client/session.py) is a second real path to an empty match.Why existing code doesn't prevent it. Nothing guards the lookup. The rest of the file follows an explicit-diagnostics convention for missing preconditions — there are multiple
raise RuntimeError("MCP_CONFORMANCE_CONTEXT missing 'client_id'")-style guards — but this handler skips that pattern for the one precondition it has.Impact. CI-only. On the happy path (harness alpha.11 as pinned, where the PR reports 9/9 passing) nothing goes wrong. In the failure scenario the leg goes red either way; only the diagnostic quality differs — whoever does the next pin bump has to reverse-engineer
coroutine raised StopIterationfrom a solo-rerun log instead of reading a message that points at the fixture and shows which tools were surfaced (which would also distinguish 'harness renamed the fixture' from 'SDK filtered the tool').Fix. Identical passing-path behavior, strictly better failure diagnostics:
Step-by-step proof. Suppose harness alpha.12 renames the fixture tool to
json_schema_2020_12_focal. (1) CI bumpsCONFORMANCE_PKG; the scenario runs andlisted.toolscontainsjson_schema_2020_12_focalandjson_schema_echobut nojson_schema_2020_12_tool. (2) The generator insidenext()is exhausted with no match, sonext()raisesStopIteration. (3) The exception propagates out of the coroutine framerun_json_schema_2020_12_preservation; per PEP 479, the interpreter replaces it withRuntimeError: coroutine raised StopIteration. (4)asyncio.run(handler(server_url))inmain()propagates that, the client process exits non-zero, and the harness records a failure whose only Python-side evidence is the interpreter-internals message — no mention of the tool name or the listing. With the fix, step (3) instead producesRuntimeError: json_schema_2020_12_tool not in tools/list result: ['json_schema_2020_12_focal', 'json_schema_echo'], immediately identifying the rename.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't seem like an issue for a conformance test?