-
Notifications
You must be signed in to change notification settings - Fork 3k
Add conformance testing CI pipeline #1915
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
Conversation
Add a conformance testing pipeline that validates the Python SDK against the @modelcontextprotocol/conformance npm package. Changes: - Add conformance-client example that handles all test scenarios (auth flows, metadata, scope handling) - Add run-server-conformance.sh script to test the everything-server - Add GitHub Actions workflow with server-conformance and client-conformance jobs - Both jobs use continue-on-error: true to match TS SDK approach The conformance client supports: - OAuth authorization code flow (default) - Client credentials with client_secret_basic - Client credentials with private_key_jwt - All auth/metadata and sep-835 scope scenarios Test results: - Server: 24/24 passed - Client: 192/192 passed (metadata, auth, sep-835 suites)
The --all-packages flag triggers a pre-existing issue where mcp-simple-chatbot is missing its README.md. Use --package to install only the specific packages needed for each job.
The conformance package requires either --suite or --scenario. The newer version (0.1.10) supports --suite all.
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.
Can this not be a package, and not be in examples? You can create its own GitHub action for it under .github/actions, and use it in conformance.yml.
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.
yep fair, this draft was claude mostly one shotting it and then I was gonna clean it up. Any thoughts on where it should go?
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.
changed it, let me know what you think
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.
You didn't do what I proposed. 🤔
You can create a folder in .github/actions directory that is called conformance, and paste what you have in scripts there.
- Extract shared default_elicitation_callback that applies schema defaults from ElicitRequestFormParams.requested_schema.properties.*.default - Add elicitation callback to _run_auth_session so all OAuth flows advertise elicitation capability - Call first available tool dynamically instead of hardcoded test-tool - Add results/ directory to .gitignore
The conformance client now passes a client_metadata_url to OAuthClientProvider, matching the TypeScript SDK's conformance client. This allows the SDK's existing CIMD support to activate when the server advertises client_id_metadata_document_supported=true, resolving the auth/basic-cimd warning.
Address review feedback: the conformance client is CI test infrastructure, not a user-facing example. Move it to scripts/conformance/client.py as a standalone script and colocate it with the server script (renamed from scripts/run-server-conformance.sh to scripts/conformance/run-server.sh). This removes the mcp-conformance-client workspace member and its lockfile entry. The client script is now run directly via uv run python scripts/conformance/client.py.
Same treatment as the unified conformance client: this is CI test infrastructure, not a user-facing example. Moved to scripts/conformance/auth-client.py and removed as a workspace member.
pcarleton
left a comment
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.
approach lgtm
Match the pinning pattern used in shared.yml for supply chain security.
pcarleton
left a comment
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.
LGTM
scripts/conformance/client.py
Outdated
| try: | ||
| asyncio.run(run_auth_code_client(server_url)) | ||
| except Exception: | ||
| logger.exception("Client failed") | ||
| sys.exit(1) |
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.
| try: | |
| asyncio.run(run_auth_code_client(server_url)) | |
| except Exception: | |
| logger.exception("Client failed") | |
| sys.exit(1) | |
| asyncio.run(run_auth_code_client(server_url)) |
| def get_conformance_context() -> dict[str, Any]: | ||
| """Load conformance test context from MCP_CONFORMANCE_CONTEXT environment variable.""" | ||
| context_json = os.environ.get("MCP_CONFORMANCE_CONTEXT") | ||
| if not context_json: | ||
| raise RuntimeError( | ||
| "MCP_CONFORMANCE_CONTEXT environment variable not set. " | ||
| "Expected JSON with client_id, client_secret, and/or private_key_pem." | ||
| ) | ||
| try: | ||
| return json.loads(context_json) | ||
| except json.JSONDecodeError as e: | ||
| raise RuntimeError(f"Failed to parse MCP_CONFORMANCE_CONTEXT as JSON: {e}") from e |
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.
You do have the same in the other auth-client.py file.
scripts/conformance/client.py
Outdated
| try: | ||
| if handler: | ||
| asyncio.run(handler(server_url)) | ||
| elif scenario.startswith("auth/"): | ||
| asyncio.run(run_auth_code_client(server_url)) | ||
| else: | ||
| print(f"Unknown scenario: {scenario}", file=sys.stderr) | ||
| sys.exit(1) | ||
| except Exception: | ||
| logger.exception("Client failed") | ||
| sys.exit(1) |
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.
Drop the except here as well, please. It already exits with status 1.
- Move client.py and run-server.sh to .github/actions/conformance/ - Remove redundant auth-client.py (superseded by client.py) - Drop try/except wrappers in main() - unhandled exceptions already exit non-zero with tracebacks - Update workflow paths accordingly
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.
This should probably be an action by itself that could be released from the conformance repo, but it's okay.
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.
ah yea good idea, will leave that for outside pr unless you think it's a blocker
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.
done here: modelcontextprotocol/conformance#113
with python sdk test pr here: #1921
Summary
Add a conformance testing pipeline that validates the Python SDK against the
@modelcontextprotocol/conformancenpm package. This mirrors the approach used in the TypeScript SDK.Changes
New Files
examples/clients/conformance-client/- Unified conformance test clientclient_secret_basicandprivate_key_jwtscripts/run-server-conformance.sh- Server conformance test runnernpx @modelcontextprotocol/conformance server.github/workflows/conformance.yml- CI workflowserver-conformancejob: Tests the Python SDK server implementationclient-conformancejob: Tests the Python SDK client implementationcontinue-on-error: trueto match the TS SDK approachTest Results (Local)
Running Locally