deps: bump MCP 1.2 -> 1.3 + e2e bootstrap - #347
Merged
Conversation
First-run output of the qyl-e2e-tests routine — adds the infrastructure needed for the next cycle to land the first end-to-end scenario. - tests/qyl.e2e.tests: new xUnit v3 / MTP project with WireMock.Net + Testcontainers. Programmatic QylTopologyFixture spins up a bridge network, qyl-collector + qyl-mcp containers from local image tags, and a process-local WireMock the containers reach via host.docker.internal. WithImagePullPolicy(_ => false) enforces "test against fresh local builds (DockerImageBuild), never a registry tag". - WireMockLlmSeamTests (Category=E2EBootstrap, no Docker): proves the WireMock stub roundtrips a scripted /v1/chat/completions and surfaces in LogEntries. Ran 3x locally, 0 flakes, ~1s each. - eng/build/BuildTest.cs: new E2ETests Nuke target depending on IDocker.DockerImageBuild + filtering Category=E2E. Default Test target now also excludes Category=E2E (E2EBootstrap stays in). - Version.props + Directory.Packages.props: pin WireMock.Net 2.6.0 and Testcontainers 4.11.0. AspNetCore instrumentation split-pinned to 1.15.2 (forced by WireMock.Net 2.6.0 transitive); Http + Runtime stay at the umbrella 1.15.1 — no 1.15.2 release exists for them. - qyl.slnx: register the project. - .agents/routine-last-run.md: routine ledger update for 2026-05-17. No scenario tests here, per the skill's bootstrap section — scenario work waits until reviewers can verify the seams independently. Co-authored-by: Claude <noreply@anthropic.com>
…ootstrap # Conflicts: # .agents/routine-last-run.md # eng/build/BuildTest.cs
MCP 1.3 changes how transport failures surface from McpClient.CreateAsync: SSE/HTTP connect failures now throw IOException (via the new public ClientTransportClosedException, which derives from IOException) rather than InvalidOperationException. Caller-triggered OperationCanceledException is also no longer wrapped. services/qyl.mcp/Formatting/ErrorFormatter.cs: add an IOException arm before InvalidOperationException so transport-closed failures (including the new ClientTransportClosedException) format as "Connection Error" instead of falling through to FormatUnknown. Arm order matters — C# pattern switches dispatch top-down. Verified: dotnet build qyl.slnx --no-restore -> 0 errors. Ref: https://github.com/modelcontextprotocol/csharp-sdk/releases/tag/v1.3.0 Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Risk: Moderate — dependency bump plus new E2E harness plumbing; one confirmed topology bug will prevent the new E2E stack from working as intended.
Blockers
- tests/qyl.e2e.tests/Topology/QylTopologyFixture.cs: MCP container port is wired to 5200, but the
qyl-mcpimage listens on 8080 by default (services/qyl.mcp/Dockerfile). This breaks port binding and makesMcpBaseUrlincorrect. (Comment ID: 001)
Important issues
- tests/qyl.e2e.tests/Topology/QylTopologyFixture.cs: WireMock is started and
host.docker.internalis added, but the MCP container isn’t configured to use the WireMock endpoint (noQYL_AGENT_ENDPOINT/related config), so “deterministic LLM stubbing” won’t actually happen once topology tests are added. (Comment ID: 002) - eng/build/BuildTest.cs:
E2ETestsignores MTP’s “no tests discovered” exit code (8) but has no post-run TRX guard, so the target can succeed while running zero E2E tests, masking misconfiguration/broken discovery. (Comment ID: 003) - Directory.Packages.props:
OpenTelemetry.Instrumentation.AspNetCoreis hard-pinned inline to1.15.2instead of being owned inVersion.propslike most other versions, which makes ongoing version management easier to drift. (Comment ID: 004) - services/qyl.mcp/Formatting/ErrorFormatter.cs: The PR description claims Rider MCP connect failures will be formatted via
ErrorFormatter, butErrorFormatterisn’t used on that path today; the newIOExceptionarm may not produce the intended UX improvement unless the exception flow is wired accordingly. (Comment ID: 005)
Minor issues
- None noted.
Changes:
- Bump ModelContextProtocol packages from 1.2.0 → 1.3.0 and adjust error formatting.
- Add an initial E2E test project + Docker/WireMock topology fixture scaffolding.
- Add a Nuke
E2ETeststarget and central-package pins for WireMock/Testcontainers.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Version.props | Bumps MCP version; adds WireMock/Testcontainers version properties. |
| Directory.Packages.props | Pins WireMock/Testcontainers; adjusts OTel AspNetCore instrumentation pin. |
| services/qyl.mcp/Formatting/ErrorFormatter.cs | Adds IOException arm intended to improve transport-failure messaging. |
| eng/build/BuildTest.cs | Adds E2ETests Nuke target and excludes Category=E2E from default runs. |
| qyl.slnx | Registers the new tests/qyl.e2e.tests project. |
| tests/qyl.e2e.tests/qyl.e2e.tests.csproj | New xUnit v3/MTP E2E test project with WireMock + Testcontainers deps. |
| tests/qyl.e2e.tests/E2ECollection.cs | Defines a non-parallelized E2E collection using the topology fixture. |
| tests/qyl.e2e.tests/Bootstrap/WireMockLlmSeamTests.cs | Adds a WireMock-only bootstrap smoke test (no Docker). |
| tests/qyl.e2e.tests/Topology/QylTopologyOptions.cs | Adds basic topology options (image tags + timeout). |
| tests/qyl.e2e.tests/Topology/QylTopologyFixture.cs | Defines the Docker + WireMock topology fixture (currently has a port wiring bug). |
| .agents/routine-last-run.md | Records the new routine run / bootstrap PR handoff notes. |
| public sealed class QylTopologyFixture : IAsyncLifetime | ||
| { | ||
| private const int CollectorInternalPort = 5100; | ||
| private const int McpInternalPort = 5200; |
Comment on lines
+74
to
+78
| .WithNetworkAliases("qyl-mcp") | ||
| .WithPortBinding(McpInternalPort, true) | ||
| .WithEnvironment("QYL_COLLECTOR_URL", $"http://qyl-collector:{CollectorInternalPort}") | ||
| .WithExtraHost("host.docker.internal", "host-gateway") | ||
| .WithWaitStrategy(Wait.ForUnixContainer() |
Comment on lines
+267
to
+271
| var mtp = MtpExtensions.Mtp() | ||
| .ReportTrx($"{project.Name}.E2E.trx") | ||
| .IgnoreExitCode(8) | ||
| .FilterTrait("Category", "E2E"); | ||
|
|
Comment on lines
+23
to
28
| SDK version (1.15.3). AspNetCore is bumped to 1.15.2 because | ||
| WireMock.Net 2.6.0 (consumed by tests/qyl.e2e.tests) transitively | ||
| requires it; Http and Runtime have no 1.15.2 release, hence the split. --> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.15.2"/> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="$(OpenTelemetryAspNetCoreInstrumentationVersion)"/> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.Runtime" Version="$(OpenTelemetryAspNetCoreInstrumentationVersion)"/> |
Comment on lines
+14
to
19
| // ModelContextProtocol 1.3 throws IOException (incl. ClientTransportClosedException) | ||
| // for transport connect/closure failures that 1.2 wrapped as InvalidOperationException. | ||
| // This arm must precede InvalidOperationException — order matters in C# pattern switches. | ||
| IOException ioEx => FormatTransportError(ioEx, transport), | ||
| InvalidOperationException configEx => FormatConfigError(configEx, transport), | ||
| _ => FormatUnknown(error, transport) |
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.
Summary
Combines two pieces of work into one PR per request:
deps— BumpsModelContextProtocol/ModelContextProtocol.Core/ModelContextProtocol.AspNetCorefrom 1.2.0 to 1.3.0.tests/e2e— Folds in the auto-e2e-2026-05-17 bootstrap PR (newtests/qyl.e2e.testsproject +E2ETestsNuke target + WireMock/Testcontainers package pins).MCP 1.2 -> 1.3 behavior change
MCP 1.3 changes how transport failures surface from
McpClient.CreateAsync:IOException(specifically the new publicClientTransportClosedException, which derives fromIOException) rather thanInvalidOperationException.OperationCanceledExceptionis no longer wrapped.In qyl, only one caller is affected:
services/qyl.mcp/Tools/Debug/RiderMcpProxy.cs:59(HttpClientTransport+StreamableHttp). The call site itself needs no change — error handling is downstream inErrorFormatter.ErrorFormatter update
services/qyl.mcp/Formatting/ErrorFormatter.cspattern-matchedInvalidOperationExceptionas "Configuration Error". With 1.2, that arm caught transport connect failures (which were wrapped asInvalidOperationException) and mislabeled them. With 1.3 they bypass that arm and fall through toFormatUnknown— less useful for the LLM.Added an
IOExceptionarm beforeInvalidOperationException(order matters in C# pattern switches) that formats transport-closed failures as "Connection Error" with a transport-mode-aware hint. This both (a) restores LLM-friendly messaging and (b) actually improves on the 1.2 behavior, where transport failures were being mislabeled as config errors.Reference: https://github.com/modelcontextprotocol/csharp-sdk/releases/tag/v1.3.0
E2E bootstrap (from
tests/auto-e2e-2026-05-17)Infrastructure-only — no scenario tests yet. Adds:
tests/qyl.e2e.tests/— new xUnit v3 / MTP project using WireMock.Net + Testcontainerstests/qyl.e2e.tests/Topology/QylTopologyFixture.cs— programmatic Testcontainers topology (bridge network ->qyl-collector:latest->qyl-mcp:latest); WireMock process-local, reached viahost.docker.internal.WithImagePullPolicy(_ => false)enforces local-image-only.tests/qyl.e2e.tests/Bootstrap/WireMockLlmSeamTests.cs—Category=E2EBootstrapsmoke test (no Docker) proving the WireMock seam roundtrips.eng/build/BuildTest.cs— newE2ETestsNuke target depending onIDocker.DockerImageBuild, filtersCategory=E2E. DefaultTestexcludesCategory=E2Ebut keepsE2EBootstrap.Version.props+Directory.Packages.props— pinnedWireMock.Net 2.6.0andTestcontainers 4.11.0. Split-pinnedOpenTelemetry.Instrumentation.AspNetCoreto1.15.2(forced by WireMock.Net 2.6.0 transitive);HttpandRuntimestay at the umbrella1.15.1.Merge conflict resolutions
Two conflicts when merging
tests/auto-e2e-2026-05-17into a branch off the latestorigin/main:.agents/routine-last-run.md— two routines (qyl-functional-testsandqyl-e2e-tests) wrote entries on the same day. Resolved by keeping both entries under the canonical## <routine-name> YYYY-MM-DD HH:MMheading format from main.eng/build/BuildTest.cs— main added aFunctionalTeststarget; auto-e2e added anE2ETeststarget. Both kept (they target different test scopes and are independent).Test plan
dotnet restore qyl.slnxcleandotnet build qyl.slnx --no-restore-> 0 errors, 19 pre-existing analyzer warnings (unrelated to changes)nuke FunctionalTests(covered by main's pre-existing infrastructure)nuke E2ETestsnot run in this PR — requires rebuilding all four qyl Docker images; out of scope for the bootstrap step. Nextqyl-e2e-testsroutine cycle will land the first scenario.qyl.mcpruntime call toMcpClient.CreateAsyncwith a non-running endpoint produces the new "Connection Error" formatting rather than "Configuration Error".Out of scope
.NET SDK 10.0.300 + MSBuild SDKs 3.4.33/2.2.16bump on the paralleldeps/ancplua-sdk-bumpsbranch is not included here. It's a separate concern that can land on its own.claude/happy-kapitsa-3510e8worktree was removed during prep (0 unique commits vs main, 0 file diffs, clean working tree).🤖 Generated with Claude Code