Skip to content

Comments

Fix Metadata delegation in DelegatingMcpServerTool/Prompt/Resource#1338

Merged
stephentoub merged 8 commits intomainfrom
copilot/fix-metadata-delegation
Feb 21, 2026
Merged

Fix Metadata delegation in DelegatingMcpServerTool/Prompt/Resource#1338
stephentoub merged 8 commits intomainfrom
copilot/fix-metadata-delegation

Conversation

Copy link
Contributor

Copilot AI commented Feb 20, 2026

  • Add Metadata override to DelegatingMcpServerTool, DelegatingMcpServerPrompt, DelegatingMcpServerResource
  • Add thorough tests in DelegatingMcpServerToolTests.cs, DelegatingMcpServerPromptTests.cs, DelegatingMcpServerResourceTests.cs
  • Use BindingFlags.DeclaredOnly instead of DeclaringType check in reflection-based override validation
  • Fixed net472 build failure
  • All tests pass

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…erPrompt, and DelegatingMcpServerResource

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub stephentoub marked this pull request as ready for review February 21, 2026 00:37
@stephentoub
Copy link
Contributor

@copilot please improve the tests around these delegating types in a way that validates all members are appropriately overridden

…ase class members

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 21, 2026

@copilot please improve the tests around these delegating types in a way that validates all members are appropriately overridden

Added DelegatingMcpServerPrimitiveTests with a reflection-based test that validates all virtual/abstract members declared on the base types (McpServerTool, McpServerPrompt, McpServerResource) are overridden in the corresponding delegating types. This will catch future regressions if new members are added to base classes but not delegated. See 610c3b8.

Copilot AI changed the title [WIP] Fix Metadata delegation in DelegatingMcpServerTool Fix Metadata delegation in DelegatingMcpServerTool/Prompt/Resource Feb 21, 2026
Copilot AI requested a review from stephentoub February 21, 2026 00:49
@stephentoub
Copy link
Contributor

@copilot please split the tests into files based on each type. If there's already a test file for the type, add the relevant test(s) there instead

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 21, 2026

@copilot please split the tests into files based on each type. If there's already a test file for the type, add the relevant test(s) there instead

Split the tests into the existing per-type files: McpServerToolTests, McpServerPromptTests, and McpServerResourceTests. Removed DelegatingMcpServerPrimitiveTests.cs. See 42d685f.

jeffhandley
jeffhandley previously approved these changes Feb 21, 2026
@stephentoub
Copy link
Contributor

stephentoub commented Feb 21, 2026

@copilot tests for Delegating McpServerTool should really be in a DelegatingMcpServerToolTests.cs file; similarly for prompt and resource. Please move all such tests into the right test file/class.

@jeffhandley
Copy link
Collaborator

@copilot See #1338 (comment). There was a typo when you were mentioned, but that's fixed now. Address that feedback please.

Co-authored-by: jeffhandley <1031940+jeffhandley@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 21, 2026

@copilot tests for Delegating McpServerTool should really be in a DelegatingMcpServerToolTests.cs file; similarly for prompt and resource. Please move all such tests into the right test file/class.

Moved the tests into dedicated files: DelegatingMcpServerToolTests.cs, DelegatingMcpServerPromptTests.cs, and DelegatingMcpServerResourceTests.cs. Reverted McpServerToolTests.cs, McpServerPromptTests.cs, and McpServerResourceTests.cs to their original state. See d6d7123.

Copilot AI requested a review from jeffhandley February 21, 2026 02:14
jeffhandley
jeffhandley previously approved these changes Feb 21, 2026
Copy link
Collaborator

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

API changes look good to me assuming the expanded test coverage doesn't uncover anything that needs to be changed further.

…ation validation

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…olidate into single test per type, fix net472 build

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
…ction tests

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub stephentoub merged commit 79b2dbe into main Feb 21, 2026
10 checks passed
@stephentoub stephentoub deleted the copilot/fix-metadata-delegation branch February 21, 2026 05:36
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.

4 participants