feat: add client conformance testing #1360
Conversation
- Copy everything-client.ts and helpers from conformance repo - Add conformance:client npm script for running initialize scenario - Add GitHub Actions workflow (non-blocking with continue-on-error) - Use @modelcontextprotocol/client workspace package for imports
- Change tools-call to tools_call (underscore)
- Change elicitation-defaults to elicitation-sep1034-client-defaults
- Add sse-retry scenario handler
- Update auth scenario names to match conformance suite:
- auth/basic-dcr -> auth/basic-cimd
- auth/basic-metadata-* -> auth/metadata-*
- Add missing: auth/scope-retry-limit, auth/token-endpoint-auth-*,
auth/client-credentials-*
- Make tools_call handler actually call the add_numbers tool
Test results: 188 passed, 9 failed, 1 warning
(Known failures: elicitation defaults and client_credentials not yet supported)
Claude-Generated-By: Claude Code (cli/claude-opus-4-5=100%)
Claude-Steers: 0
Claude-Permission-Prompts: 0
Claude-Escapes: 0
Add dedicated handlers for auth/client-credentials-jwt and auth/client-credentials-basic scenarios using the SDK's ClientCredentialsProvider and PrivateKeyJwtProvider. These scenarios require special handling because they use the client_credentials OAuth grant type (machine-to-machine auth) instead of the authorization code flow used by other auth scenarios. The conformance runner passes context via MCP_CONFORMANCE_CONTEXT env var containing client_id, private_key_pem/client_secret as appropriate. Claude-Generated-By: Claude Code (cli/claude-opus-4-5=100%) Claude-Steers: 0 Claude-Permission-Prompts: 0 Claude-Escapes: 0
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
commit: |
There was a problem hiding this comment.
Review from Claude
Overall this looks good - clean architecture with the scenario handler registry pattern, good type safety with the Zod discriminated union for context parsing, and comprehensive coverage (193 tests across 21 scenarios). A few items to consider:
Issues
1. CI Workflow: Wrong cache type
.github/workflows/conformance.yml:21 uses cache: npm but the project uses pnpm:
- uses: actions/setup-node@v4
with:
node-version: 24
cache: npm # Should be: cache: pnpmThis won't properly cache pnpm dependencies.
2. Lock file changes look suspicious
The pnpm-lock.yaml diff shows removal of entire catalog sections (runtimeClientOnly, partial runtimeServerOnly, vite-tsconfig-paths, etc.) that seem unrelated to adding two devDependencies. Worth verifying the lock file wasn't inadvertently regenerated in a way that removes needed entries.
3. Inconsistent error output
everything-client.ts mixes console.error (lines 398-406, 408-413) with logger.error/logger.debug. For consistency, consider using the logger throughout.
4. TODO comment should be tracked
withOAuthRetry.ts:30-31 has a TODO about incorporating retry logic into the SDK. Created #1370 to track this.
Minor Suggestions
- The scripts use
npx @modelcontextprotocol/conformancewhich downloads the package each run. For local development, consider adding it as a devDependency or documenting that first runs will be slower.
Nice work on getting all 21 scenarios passing!
bhosmer-ant
left a comment
bhosmer-ant
left a comment
There was a problem hiding this comment.
LGTM! idk if the claude comments are useful but fyi
- Change CI workflow cache from npm to pnpm - Use logger.error consistently instead of console.error - Regenerate pnpm-lock.yaml to fix spurious catalog deletions
This avoids downloading on every run and allows manual version bumps when new conformance tests are released.
The setup-node action with cache: pnpm requires pnpm to be installed first to locate the lockfile and cache directory.
|
1 more 1 more, sorry about that @bhosmer-ant |
Adds client conformance testing support for the TypeScript SDK.
Once this is in, the plan will be to remove the typescript conformance example from the conformance repo, and have this be the source of truth for typescript-sdk conformance.
Changes
elicitation.form.applyDefaults)pnpm run conformance:client- run client directlypnpm run test:conformance:client- run conformance tests (pass your own flags)pnpm run test:conformance:client:all- run all suitesTest Results
193 passed, 0 failed, 0 warnings across all 21 client scenarios.
This should be a test-only change so should not need a changeset / release.
Dependencies
Requires modelcontextprotocol/conformance#94 for
--suite allsupport.