Skip to content

Commit b14455c

Browse files
authored
Merge branch 'main' into copilot/add-mcp-apps-support
2 parents 7f54b8c + a98fc36 commit b14455c

32 files changed

Lines changed: 743 additions & 206 deletions

.github/copilot-instructions.md

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,22 +85,39 @@ The SDK consists of three main packages:
8585
- Test servers in `tests/ModelContextProtocol.Test*Server/` for integration scenarios
8686
- Filter manual tests with `[Trait("Execution", "Manual")]` - these require external dependencies
8787

88-
### Test Infrastructure and Helpers
89-
- **LoggedTest**: Base class for tests that need logging output captured to xUnit test output
90-
- Provides `ILoggerFactory` and `ITestOutputHelper` for test logging
91-
- Use when debugging or when tests need to verify log output
92-
- **TestServerTransport**: In-memory transport for testing client-server interactions without network I/O
93-
- **MockLoggerProvider**: For capturing and asserting on log messages
94-
- **XunitLoggerProvider**: Routes `ILogger` output to xUnit's `ITestOutputHelper`
95-
- **KestrelInMemoryTransport** (AspNetCore.Tests): In-memory Kestrel connection for HTTP transport testing without network stack
96-
97-
### Test Best Practices
98-
- Inherit from `LoggedTest` for tests needing logging infrastructure
99-
- Use `TestServerTransport` for in-memory client-server testing
100-
- Mock external dependencies (filesystem, HTTP clients) rather than calling real services
101-
- Use `CancellationTokenSource` with timeouts to prevent hanging tests
102-
- Dispose resources properly (servers, clients, transports) using `IDisposable` or `await using`
103-
- Run tests with: `dotnet test --filter '(Execution!=Manual)'`
88+
### Test Base Classes
89+
- **`LoggedTest`**: Base class that wires up `ILoggerFactory` with `XunitLoggerProvider` (test output) and `MockLoggerProvider` (log assertions). Inherit from this for any test needing logging.
90+
- **`ClientServerTestBase`**: Sets up in-memory client/server pair via `Pipe`. Override `ConfigureServices` to register tools/prompts/resources, then call `CreateMcpClientForServer()`. Handles async disposal automatically.
91+
- **`KestrelInMemoryTest`** (AspNetCore tests): Hosts ASP.NET Core with in-memory transport — no ports needed.
92+
- **`TestServerTransport`**: In-memory mock transport for testing client logic without a real server.
93+
94+
### Transport Selection in Tests
95+
- **Never use `WithStdioServerTransport()` in unit tests.** It reads from the test host's stdin, which cannot be closed, permanently leaking a thread pool thread per test.
96+
- For DI-only tests: `WithStreamServerTransport(Stream.Null, Stream.Null)`
97+
- For client/server interaction: inherit `ClientServerTestBase`
98+
- For client-only logic: use `TestServerTransport`
99+
- For HTTP/SSE: inherit `KestrelInMemoryTest`
100+
- For process lifecycle tests: `StdioClientTransport` (only when testing actual process behavior)
101+
102+
### Resource Management
103+
- **Always `await using` the `ServiceProvider`** when MCP server services are registered — `McpServerImpl` only implements `IAsyncDisposable`. Synchronous `using` throws at runtime.
104+
- **Always dispose clients and servers** — use `await using var client = ...`
105+
- **Use `TestContext.Current.CancellationToken`** for async MCP calls so xUnit can cancel on timeout.
106+
107+
### Timeouts
108+
- **Always use `TestConstants.DefaultTimeout`** (60s) instead of hardcoded values. CI machines are slower than dev workstations.
109+
- For HTTP polling operations use `TestConstants.HttpClientPollingTimeout` (2s).
110+
111+
### Synchronization
112+
- **Never use `Task.Delay` for synchronization.** Use `TaskCompletionSource`, `SemaphoreSlim`, or `Channel` so tests don't depend on timing.
113+
114+
### Background Logging
115+
- `ITestOutputHelper.WriteLine` throws after the test method returns. Background threads (process event handlers, async continuations) can outlive the test, causing unhandled exceptions that crash the test host.
116+
- Route logging through `LoggedTest.LoggerFactory``XunitLoggerProvider` already catches post-test exceptions.
117+
- If calling `ITestOutputHelper` directly from an event handler, wrap in try/catch for `InvalidOperationException`.
118+
119+
### Parallelism
120+
- Tests run in parallel by default. Apply `[Collection(nameof(DisableParallelization))]` to test classes that touch global state (e.g., `ActivitySource` listeners).
104121

105122
## Build and Development
106123

.github/workflows/ci-build-test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ jobs:
5050
9.0.x
5151
5252
- name: 🔧 Set up Node.js
53-
uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0
53+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
5454
with:
5555
node-version: '20'
5656

@@ -76,7 +76,7 @@ jobs:
7676

7777
- name: 📤 Upload test results artifact
7878
if: always()
79-
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
79+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
8080
with:
8181
name: testresults-${{ matrix.os }}-${{ matrix.configuration }}
8282
path: artifacts/testresults/**

.github/workflows/ci-code-coverage.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
pattern: testresults-*
2525

2626
- name: Combine coverage reports
27-
uses: danielpalme/ReportGenerator-GitHub-Action@5.5.4
27+
uses: danielpalme/ReportGenerator-GitHub-Action@5.5.5
2828
with:
2929
reports: "**/*.cobertura.xml"
3030
targetdir: "${{ github.workspace }}/report"
@@ -36,7 +36,7 @@ jobs:
3636
toolpath: "reportgeneratortool"
3737

3838
- name: Upload combined coverage XML
39-
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
39+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
4040
with:
4141
name: coverage
4242
path: ${{ github.workspace }}/report
@@ -56,7 +56,7 @@ jobs:
5656
thresholds: "60 80"
5757

5858
- name: Upload combined coverage markdown
59-
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
59+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
6060
with:
6161
name: coverage-markdown
6262
path: ${{ github.workspace }}/code-coverage-results.md

.github/workflows/codeql.yml

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
name: "CodeQL"
2+
3+
on:
4+
push:
5+
branches: [ "main", "validation/**" ]
6+
pull_request:
7+
branches: [ "main", "validation/**" ]
8+
schedule:
9+
- cron: '23 9 * * 6'
10+
11+
jobs:
12+
analyze:
13+
if: github.event.repository.fork == false
14+
name: Analyze (${{ matrix.language }})
15+
# Runner size impacts CodeQL analysis time. To learn more, please see:
16+
# - https://gh.io/recommended-hardware-resources-for-running-codeql
17+
# - https://gh.io/supported-runners-and-hardware-resources
18+
# - https://gh.io/using-larger-runners (GitHub.com only)
19+
# Consider using larger runners or machines with greater resources for possible analysis time improvements.
20+
runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
21+
permissions:
22+
# required for all workflows
23+
security-events: write
24+
25+
# required to fetch internal or private CodeQL packs
26+
packages: read
27+
28+
# only required for workflows in private repositories
29+
actions: read
30+
contents: read
31+
32+
strategy:
33+
fail-fast: false
34+
matrix:
35+
include:
36+
- language: actions
37+
build-mode: none
38+
- language: csharp
39+
build-mode: none
40+
# CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'rust', 'swift'
41+
# Use `c-cpp` to analyze code written in C, C++ or both
42+
# Use 'java-kotlin' to analyze code written in Java, Kotlin or both
43+
# Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both
44+
# To learn more about changing the languages that are analyzed or customizing the build mode for your analysis,
45+
# see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning.
46+
# If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how
47+
# your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages
48+
steps:
49+
- name: Checkout repository
50+
uses: actions/checkout@v6
51+
52+
# Add any setup steps before running the `github/codeql-action/init` action.
53+
# This includes steps like installing compilers or runtimes (`actions/setup-node`
54+
# or others). This is typically only required for manual builds.
55+
# - name: Setup runtime (example)
56+
# uses: actions/setup-example@v1
57+
58+
# Initializes the CodeQL tools for scanning.
59+
- name: Initialize CodeQL
60+
uses: github/codeql-action/init@v4
61+
with:
62+
languages: ${{ matrix.language }}
63+
build-mode: ${{ matrix.build-mode }}
64+
# If you wish to specify custom queries, you can do so here or in a config file.
65+
# By default, queries listed here will override any specified in a config file.
66+
# Prefix the list here with "+" to use these queries and those in the config file.
67+
68+
# For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
69+
# queries: security-extended,security-and-quality
70+
71+
# If the analyze step fails for one of the languages you are analyzing with
72+
# "We were unable to automatically build your code", modify the matrix above
73+
# to set the build mode to "manual" for that language. Then modify this step
74+
# to build your code.
75+
# ℹ️ Command-line programs to run using the OS shell.
76+
# 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
77+
- name: Run manual build steps
78+
if: matrix.build-mode == 'manual'
79+
shell: bash
80+
run: |
81+
echo 'If you are using a "manual" build mode for one or more of the' \
82+
'languages you are analyzing, replace this with the commands to build' \
83+
'your code, for example:'
84+
echo ' make bootstrap'
85+
echo ' make release'
86+
exit 1
87+
88+
- name: Perform CodeQL Analysis
89+
uses: github/codeql-action/analyze@v4
90+
with:
91+
category: "/language:${{matrix.language}}"

.github/workflows/docs.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ jobs:
4040
run: make generate-docs
4141

4242
- name: Upload Pages artifact
43-
uses: actions/upload-pages-artifact@7b1f4a764d45c48632c6b24a0339c27f5614fb0b # v4.0.0
43+
uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0
4444
with:
4545
path: 'artifacts/_site'
4646

.github/workflows/release.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ on:
3232

3333
jobs:
3434
build-all-configs:
35+
# Don't run scheduled/release triggers on forks; allow manual workflow_dispatch
36+
if: ${{ github.repository == 'modelcontextprotocol/csharp-sdk' || github.event_name == 'workflow_dispatch' }}
3537
strategy:
3638
matrix:
3739
os: [ubuntu-latest, windows-latest, macos-latest]
@@ -89,7 +91,7 @@ jobs:
8991
--output "${{ github.workspace }}/artifacts/packages"
9092

9193
- name: Upload artifact
92-
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
94+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
9395
if: ${{ !cancelled() }}
9496
with:
9597
name: build-artifacts

CONTRIBUTING.md

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,70 @@ dotnet test tests/ModelContextProtocol.Tests/
6565

6666
Tools like Visual Studio, JetBrains Rider, and VS Code also provide integrated test runners that can be used to run and debug individual tests.
6767

68+
### Writing Tests
69+
70+
The test projects include shared infrastructure in `tests/Common/Utils/` that most tests build on. Familiarize yourself with these helpers before writing new tests.
71+
72+
#### Test base classes
73+
74+
- **`LoggedTest`** — Base class that wires up `ILoggerFactory` with both `XunitLoggerProvider` (routes to test output) and `MockLoggerProvider` (captures logs for assertions). Inherit from this for any test that needs logging.
75+
- **`ClientServerTestBase`** — Sets up an in-memory client/server pair connected via `Pipe` with proper async disposal. Override `ConfigureServices` to register tools, prompts, and resources, then call `CreateMcpClientForServer()` to get a connected client.
76+
- **`KestrelInMemoryTest`** (ASP.NET Core tests) — Hosts an ASP.NET Core server with in-memory transport so HTTP/SSE tests run without allocating ports.
77+
78+
#### Choosing a transport
79+
80+
| Scenario | Transport | Why |
81+
|---|---|---|
82+
| Unit tests that only need DI | `WithStreamServerTransport(Stream.Null, Stream.Null)` | No threads blocked, no process spawned |
83+
| Client/server interaction tests | `ClientServerTestBase` (uses `Pipe`) | Full bidirectional MCP, in-process |
84+
| Client-only logic | `TestServerTransport` | In-memory mock that auto-responds to standard MCP requests |
85+
| HTTP/SSE integration | `KestrelInMemoryTest` | Real HTTP stack, no network |
86+
| External process tests | `StdioClientTransport` | Only when testing actual process lifecycle |
87+
88+
> **Do not** use `WithStdioServerTransport()` in unit tests. The stdio server transport reads from the test host process's standard input, which the test does not own and cannot close. This means the transport's background read loop can never terminate, permanently leaking a thread pool thread per test. Use `WithStreamServerTransport(Stream.Null, Stream.Null)` for tests that only need the DI container.
89+
90+
#### Resource management
91+
92+
- **Always `await using` the `ServiceProvider`** when MCP server services are registered — `McpServerImpl` only implements `IAsyncDisposable`, not `IDisposable`. A synchronous `using` will throw at runtime, and skipping disposal leaks transports and background threads.
93+
- **Use `TestContext.Current.CancellationToken`** when calling async MCP methods so that xUnit can cancel the test on timeout rather than hanging.
94+
- **Dispose clients and servers** explicitly. Prefer `await using var client = ...` over relying on finalizers. `ClientServerTestBase` handles this if you inherit from it.
95+
96+
#### Timeouts
97+
98+
Use `TestConstants.DefaultTimeout` (60 seconds) rather than hardcoded values. CI machines are often slower than developer workstations, and short timeouts cause flaky failures.
99+
100+
```csharp
101+
// Good
102+
using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken);
103+
cts.CancelAfter(TestConstants.DefaultTimeout);
104+
await client.CallToolAsync("my-tool", args, cts.Token);
105+
106+
// Bad — too short for CI
107+
cts.CancelAfter(TimeSpan.FromSeconds(5));
108+
```
109+
110+
#### Synchronization
111+
112+
Avoid `Task.Delay` for synchronization. Use explicit signaling primitives (`TaskCompletionSource`, `SemaphoreSlim`, `Channel`) so tests don't depend on timing. If a producer/consumer test writes events for a streaming reader, use a `TaskCompletionSource` to confirm the reader is active before writing.
113+
114+
#### Background logging
115+
116+
`ITestOutputHelper.WriteLine` throws after the test method returns. Background threads (process event handlers, async continuations) can outlive the test. This manifests as unhandled exceptions that crash the test host. Two mitigations:
117+
118+
1. **`XunitLoggerProvider`** already catches these exceptions. Route logging through `LoggedTest.LoggerFactory` rather than calling `ITestOutputHelper` directly from callbacks.
119+
2. **If you must call `ITestOutputHelper` from an event handler**, wrap it in a try/catch:
120+
```csharp
121+
process.ErrorDataReceived += (s, e) =>
122+
{
123+
try { testOutputHelper.WriteLine(e.Data); }
124+
catch (InvalidOperationException) { }
125+
};
126+
```
127+
128+
#### Parallelism
129+
130+
Tests run in parallel by default. If a test class touches global state (e.g., `ActivitySource` listeners in diagnostics tests), apply `[Collection(nameof(DisableParallelization))]` to run it sequentially.
131+
68132
### Building the Documentation
69133

70134
This project uses [DocFX](https://dotnet.github.io/docfx/) to generate its conceptual and reference documentation.

Directory.Packages.props

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
44
<System8Version>8.0.22</System8Version>
55
<System9Version>9.0.11</System9Version>
6-
<System10Version>10.0.5</System10Version>
7-
<MicrosoftExtensionsVersion>10.4.1</MicrosoftExtensionsVersion>
6+
<System10Version>10.0.7</System10Version>
7+
<MicrosoftExtensionsVersion>10.5.0</MicrosoftExtensionsVersion>
88
</PropertyGroup>
99

1010
<!-- Product dependencies shared -->
@@ -62,8 +62,8 @@
6262

6363
<!-- Testing dependencies -->
6464
<ItemGroup>
65-
<PackageVersion Include="Anthropic" Version="12.9.0" />
66-
<PackageVersion Include="coverlet.collector" Version="8.0.1">
65+
<PackageVersion Include="Anthropic" Version="12.11.0" />
66+
<PackageVersion Include="coverlet.collector" Version="10.0.0">
6767
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
6868
<PrivateAssets>all</PrivateAssets>
6969
</PackageVersion>
@@ -74,14 +74,14 @@
7474
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="$(System10Version)" />
7575
<PackageVersion Include="Microsoft.Extensions.Options" Version="$(System10Version)" />
7676
<PackageVersion Include="Microsoft.Extensions.TimeProvider.Testing" Version="10.1.0" />
77-
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.3.0" />
77+
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.4.0" />
7878
<PackageVersion Include="Moq" Version="4.20.72" />
79-
<PackageVersion Include="OpenTelemetry" Version="1.15.1" />
80-
<PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.15.1" />
81-
<PackageVersion Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.15.1" />
82-
<PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="1.15.0" />
83-
<PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.15.1" />
84-
<PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.15.1" />
79+
<PackageVersion Include="OpenTelemetry" Version="1.15.3" />
80+
<PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.15.3" />
81+
<PackageVersion Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.15.3" />
82+
<PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="1.15.1" />
83+
<PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.15.3" />
84+
<PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.15.2" />
8585
<PackageVersion Include="Serilog.Extensions.Hosting" Version="10.0.0" />
8686
<PackageVersion Include="Serilog.Extensions.Logging" Version="10.0.0" />
8787
<PackageVersion Include="Serilog.Sinks.Console" Version="6.1.1" />
@@ -92,6 +92,6 @@
9292
<PackageVersion Include="xunit.v3" Version="3.2.2" />
9393
<PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
9494
<PackageVersion Include="System.Net.Http" Version="4.3.4" />
95-
<PackageVersion Include="JsonSchema.Net" Version="9.1.3" />
95+
<PackageVersion Include="JsonSchema.Net" Version="9.2.0" />
9696
</ItemGroup>
9797
</Project>

0 commit comments

Comments
 (0)