Fix #25094: Upgrade MCP server SDK support

[harshach]

Describe your changes:

Fixes #25094

I worked on ... because ...

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• Updated MCP SDK:
  • Upgraded mcp-sdk.version from 0.14.0 to 0.17.0 in openmetadata-mcp/pom.xml
• API migration:
  • Replaced McpSchema.CallToolResult builder pattern with direct constructor calls in DefaultToolContext.java
  • Simplified McpTransportContext creation to use immutable Map.of() in AuthEnrichedMcpContextExtractor.java
• New test suite:
  • Added McpSdkUpgradeTest.java (185 lines) validating all SDK 0.17.0 API changes

_{This will update automatically on new commits.}

---

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	65.25% (52213/80020)	43.14% (25832/59878)	46.53% (8094/17394)

[gitar-bot]

🔍 CI failure analysis for 6bf1c08: Python tests blocked by type checking error (missing 'extension' parameter). Playwright tests mostly passing (880+) with a few UI test failures unrelated to MCP SDK changes.

Issue

Two types of CI failures:

1. Python tests: Type checking error blocking all Python test jobs
2. Playwright tests: Some test failures (not build errors)

Root Causes

Python Tests - Type Checking Error

Missing required parameter in SDK example:

ingestion/src/metadata/sdk/examples/builder_end_to_end.py:254:15
- error: Argument missing for parameter "extension"

The basedpyright type checker blocks all Python tests before they can run.

Playwright Tests - Test Failures

Multiple Playwright jobs with similar patterns:

• Job 59894006008 (3/6): 455 passed, 3 failed, 9 flaky
• Job 59894006020 (6/6): 425 passed, 1 failed, 4 flaky

Common failures: Element visibility assertions in Glossary, Tags, Users, SubDomains

Impact

Python test jobs: All blocked by type checking error

• ✗ Python tests (3.10) - Multiple runs
• ✗ Python tests (3.11)

Playwright test jobs: Mixed results

• ✅ 880+ tests passed across jobs
• ❌ 4+ tests failed (UI/visibility)
• ⚠️ 13+ flaky tests

The Playwright failures are in UI components unrelated to the MCP SDK backend changes. They appear to be pre-existing flaky tests or timing issues in the test environment, not caused by this PR.

Code Review 👍 Approved with suggestions

SDK upgrade looks solid with good test coverage. One behavioral change for null token handling needs clarification to ensure auth works as expected.

⚠️ Edge Case: Null token replaced with empty string changes behavior

📄 openmetadata-mcp/src/main/java/org/openmetadata/mcp/AuthEnrichedMcpContextExtractor.java:14

The previous implementation stored a null token when JwtFilter.extractToken() returned null, but the new implementation converts it to an empty string (token != null ? token : ""). This changes the behavior for downstream consumers that may rely on null checks to determine if authentication is missing.\n\nUsing Map.of() requires non-null values which forces this change, but storing an empty string instead of null could cause authentication to silently fail or behave unexpectedly if downstream code checks for null to detect missing authentication.\n\nConsider whether empty string should be handled differently (e.g., throw an exception early for missing auth) or ensure all consumers treat empty string as equivalent to missing authentication.

More details 💡 1 suggestion ✅ 1 resolved

💡 Code Quality: Test assertion contradicts behavior: expects exception but code handles null

📄 openmetadata-mcp/src/test/java/org/openmetadata/mcp/McpSdkUpgradeTest.java:72

The test testAuthEnrichedMcpContextExtractorHandlesMissingToken comments that it expects an exception for missing Authorization header, but looking at line 47 of the production code, the implementation now handles null tokens by substituting an empty string (token != null ? token : ""). This means the test's comment and expected behavior may be incorrect - the code will silently succeed with an empty Authorization value rather than throwing an exception.\n\nEither:\n1. The production code should throw an exception for missing auth (matching the test expectation)\n2. The test expectation should be updated to reflect the new silent-failure behavior\n\nThis inconsistency between test and implementation should be resolved.

✅ Bug: NullPointerException when token is null with Map.of()

📄 openmetadata-mcp/src/main/java/org/openmetadata/mcp/AuthEnrichedMcpContextExtractor.java:15
The code uses Map.of(AUTHORIZATION_HEADER, token != null ? token : "") with a ternary to handle null tokens. However, looking at the test testAuthEnrichedMcpContextExtractorHandlesMissingToken, it expects an exception to be thrown when the Authorization header is null.

The issue is that JwtFilter.extractToken() may throw an exception for null input, but if it returns null instead (for malformed tokens or edge cases), the current code will put an empty string in the context, potentially allowing unauthenticated access.

The test at line 250-264 explicitly states "JwtFilter.extractToken throws AuthenticationException for null token" and expects an exception. If the behavior changes and extractToken returns null instead of throwing, the empty string fallback could mask authentication failures.

Suggestion: Consider explicitly throwing an AuthenticationException when the token is null or empty, rather than silently substituting an empty string:

String token = JwtFilter.extractToken(request.getHeader(AUTHORIZATION_HEADER));
if (token == null || token.isEmpty()) {
    throw new AuthenticationException("Missing or invalid Authorization header");
}
return McpTransportContext.create(Map.of(AUTHORIZATION_HEADER, token));

This makes the authentication requirement explicit rather than relying on downstream validation.

What Works Well

Good addition of comprehensive test suite (McpSdkUpgradeTest.java) that validates all the SDK 0.17.0 API changes. Clean migration of JSON schema validator types from JsonSchema to Schema and ValidationMessage to Error.

Recommendations

Verify that downstream consumers of McpTransportContext properly handle the empty string case for missing Authorization tokens, or consider throwing an explicit exception early when auth is required but missing.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off Gitar will not commit updates to this branch.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Failed to cherry-pick changes to the 1.11.5 branch.
Please cherry-pick the changes manually.
You can find more details here.

[pmbrull]

manually picked