Add round-trip integration tests for MCP Apps _meta.ui serialization#1698
Open
yayayouyou wants to merge 2 commits into
Open
Add round-trip integration tests for MCP Apps _meta.ui serialization#1698yayayouyou wants to merge 2 commits into
yayayouyou wants to merge 2 commits into
Conversation
Covers the gap called out in modelcontextprotocol#1608: tools registered with [McpAppUi] and processed by WithMcpApps() are listed through McpClient.ListToolsAsync() via ClientServerTestBase, verifying the structured _meta.ui object end-to-end across the transport rather than only on the option pipeline: - resourceUri round-trips for an attributed tool - visibility round-trips for a restricted tool (and is omitted when not restricted) - tools without the attribute stay free of ui metadata Used AI assistance; reviewed and tested by me (3/3 pass on net10.0 locally; net8.0/net9.0 compile but need runtimes not installed locally). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end (client/server wire) integration coverage ensuring MCP Apps _meta.ui tool metadata survives JSON serialization when enumerated via McpClient.ListToolsAsync(), addressing the gap noted in #1608.
Changes:
- Introduces
McpAppsIntegrationTestsbased onClientServerTestBaseto round-tripListToolsAsync()results. - Validates
_meta.ui.resourceUriis preserved and_meta.ui.visibilityround-trips when configured. - Verifies tools without
[McpAppUi]do not emit UI metadata.
Comment on lines
+45
to
+46
| // Visibility was not restricted, so the attribute must not add the property. | ||
| Assert.Null(uiNode["visibility"]); |
|
|
||
| var plainTool = Assert.Single(tools, t => t.Name == "plain_tool"); | ||
|
|
||
| Assert.Null(plainTool.ProtocolTool.Meta?["ui"]); |
Address Copilot review on modelcontextprotocol#1698: Assert.Null on a JsonObject indexer passes both when a key is absent and when it is serialized as JSON null, which doesn't prove the intended omission. Assert the "visibility" and "ui" keys are not present at all (ContainsKey) so the tests fail if the server ever emits an explicit null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1608.
Summary
Adds the round-trip integration coverage requested in #1608 (from @halter73's review comment on #1484): all existing
_meta.uitests are unit tests on the option pipeline, so nothing verified the metadata actually survives serialization throughMcpClient.ListToolsAsync().New
McpAppsIntegrationTests(viaClientServerTestBase) registers tools with[McpAppUi]behindWithTools<T>().WithMcpApps()and asserts, after the client/server wire round-trip:_meta.ui.resourceUriis present and correctly shaped for an attributed tool, withvisibilityomitted when not restricted_meta.ui.visibilityround-trips for a[McpAppUi(Visibility = [McpUiToolVisibility.Model])]tooluimetadataTesting
dotnet test --filter FullyQualifiedName~McpAppsIntegrationTests: 3/3 pass on net10.0 locally; net8.0/net9.0 targets compile but were not run locally (runtimes not installed on this machine — CI covers them)dotnet format --verify-no-changesclean on the new fileUsed AI assistance; reviewed and tested by me.
🤖 Generated with Claude Code