Skip to content

Conversation

@pcarleton
Copy link
Member

Summary

  • Add FAILURE checks to scope scenarios when authorization flow doesn't complete
  • Report client timeout and exit errors in test results
  • Add OVERALL: PASSED/FAILED summary at end of test output
  • Exit with code 1 on any failure (check failures, client timeout, or exit error)

Problem

Previously, if the OAuth authorization flow didn't complete (e.g., client timed out or exited with an error before the authorization request reached the server), the scope checks were never triggered and the test would incorrectly report success like "Passed: 6/6, 0 failed".

Solution

The following scope scenarios now emit a FAILURE if their expected scope check wasn't recorded:

  • ScopeFromWwwAuthenticateScenario
  • ScopeFromScopesSupportedScenario
  • ScopeOmittedWhenUndefinedScenario

Additionally, printClientResults now:

  • Accepts optional clientOutput to check for timeout/exit errors
  • Reports warnings for client timeout or non-zero exit codes
  • Shows an overall PASSED/FAILED summary
  • Returns overallFailure boolean for proper exit code

Example output (failure case)

Test Results:
Passed: 6/7, 1 failed, 0 warnings

⚠️  CLIENT EXITED WITH ERROR (code 1) - Test may be incomplete

Failed Checks:
  - Client scope selection from scopes_supported: Client did not complete authorization flow - scope check could not be performed

❌ OVERALL: FAILED

🤖 Generated with Claude Code

- Add FAILURE checks to scope scenarios when authorization flow doesn't
  complete (ScopeFromWwwAuthenticateScenario, ScopeFromScopesSupportedScenario,
  ScopeOmittedWhenUndefinedScenario)
- Report client timeout and exit errors in test results
- Add OVERALL: PASSED/FAILED summary at end of test output
- Exit with code 1 on any failure (check failures, client timeout, or exit error)

Previously, if the OAuth authorization flow didn't complete, the scope
checks were never triggered and the test would incorrectly report success.
Now these scenarios emit a FAILURE if the expected scope check wasn't recorded.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 15, 2025

Open in StackBlitz

npx https://pkg.pr.new/modelcontextprotocol/conformance/@modelcontextprotocol/conformance@80

commit: 1c42539

pcarleton and others added 2 commits December 15, 2025 17:25
Warnings now contribute to the overall failure status, causing the test
to exit with code 1 when any warnings are present. Also displays warning
checks in the output summary, similar to how failures are displayed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Update the suite summary to show ✗ for scenarios with warnings,
and exit with code 1 if any warnings are present across the suite.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Collaborator

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me parece bien

(c) => c.id === 'scope-from-www-authenticate'
);
if (!hasScopeCheck) {
this.checks.push({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: mutations in a getSomething might be slightly weird, but I deduplication above makes it idempotent at least.

const { overallFailure } = printClientResults(
result.checks,
verbose,
result.clientOutput
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more of a suggestion: IIUC we're only showing these output summaries for single scenarios, could consider doing that for test suites as well.

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.

2 participants