Skip to content

Add private protected ctors to McpClient and McpServer to prevent external derivation#1361

Closed
Copilot wants to merge 10 commits intomainfrom
copilot/add-internal-ctors-to-mcp-classes
Closed

Add private protected ctors to McpClient and McpServer to prevent external derivation#1361
Copilot wants to merge 10 commits intomainfrom
copilot/add-internal-ctors-to-mcp-classes

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 23, 2026

McpClient and McpServer have implicit protected constructors, allowing external derivation. The design hasn't been validated for that, and no external derived types are known.

  • Added private protected parameterless constructors to McpClient and McpServer, restricting derivation to same-assembly types (McpClientImpl, McpServerImpl)
  • Removed TestServerForIChatClient (a test-only McpServer subclass) and rewrote the two AsSamplingChatClient tests to use McpServer.Create() with TestServerTransport
  • Added MockSamplingResult property to TestServerTransport (consistent with existing MockTask/MockTaskResult/MockTaskList pattern) to support customizable sampling responses
  • Replaced all Mock<McpServer> usages across test files with real McpServer instances via McpServer.Create() + TestServerTransport, removing Moq dependency from 6 test files

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

@stephentoub stephentoub added the breaking-change This issue or PR introduces a breaking change label Feb 23, 2026
@stephentoub
Copy link
Copy Markdown
Contributor

@copilot lots of test failures in CI

Copilot AI and others added 7 commits February 23, 2026 20:25
…lTests

Replace all Moq-based Mock<McpServer> usages with real McpServer instances
created via McpServer.Create() using TestServerTransport. This removes the
dependency on Moq in this test file.

Co-authored-by: Copilot <[email protected]>
…mptTests

Replace all Moq-based Mock<McpServer> usages with real McpServer instances
created via McpServer.Create() using TestServerTransport. This removes the
Moq dependency from this test file and uses concrete implementations instead.

Co-authored-by: Copilot <[email protected]>
…ourceTests

Remove Moq dependency and use McpServer.Create() with TestServerTransport
to create real server instances instead of mocks.

Co-authored-by: Copilot <[email protected]>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 23, 2026

@copilot lots of test failures in CI

Fixed in adb6f59. Replaced all Mock<McpServer> usages with real McpServer instances created via McpServer.Create(new TestServerTransport(), new McpServerOptions()). No InternalsVisibleTo or Moq needed — added a CreateTestServer() helper to each test class. All 1824 tests pass across net8.0/net9.0/net10.0.

@stephentoub
Copy link
Copy Markdown
Contributor

Replaced by #1363

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This issue or PR introduces a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants