Repository navigation
Verify MRTR confirmation and legacy elicitation compatibility - #529
Merged
Merged
Conversation
Cover native MRTR wire exchanges, legacy elicitation and resolver behavior, confirmation side effects, and stdio compatibility. Document the single-round flow and SDK retry limits without changing production behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the expanded destructive-operation confirmation tests and documentation.
No open issues 🎉
What changed in this PR
This PR strengthens verification of destructive-operation confirmation in the .NET MCP server, addressing #486 without changing production behavior or dependencies.
Changes:
- Add native wire-level confirmation tests, solution-removal safety checks, and invalid-response coverage.
- Verify legacy single-call behavior and the SDK’s backward-compatibility resolver; extend real-stdio coverage to both Clean and Remove.
- Document native, legacy, and Tasks confirmation paths and their relationship to SDK retry limits.
| File | Description |
|---|---|
DotNetMcp.Tests/Tools/ElicitationTests.cs |
Clarify the fallback test’s name and documentation. |
DotNetMcp.Tests/Server/Sdk20CompatibilityTests.cs |
Add wire-level, side-effect safety, invalid-input, and resolver tests. |
DotNetMcp.Tests/Server/McpConformanceTests.cs |
Cover Clean and Remove confirmation over stdio on both protocols. |
doc/testing.md |
Describe the expanded confirmation coverage. |
doc/sdk-v1.1-compatibility.md |
Explain confirmation paths and retry limits; update the MRTR documentation link. |
Review effort: Balanced
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.
Destructive-operation confirmation already supports native MRTR and legacy elicitation, but its coverage did not fully establish wire behavior, solution-removal safety, or the effect of the SDK's retry limits. This closes those verification gaps without changing production confirmation behavior or dependencies.
Changes
inputRequestsand submitinputResponsesusing a transport-only client, independently of the SDK's automatic retry loop.Legacy production calls intentionally retain
ElicitAsync; the explicit-MRTR resolver is exercised by a test-only tool. Existing Tasks and no-elicitation fallback behavior remain unchanged.Validation
Fixes: #486