Resources-enhancements #257
Conversation
|
The failing test appears to indicate that it will sometimes fail accidentally. I don't appear to be able to requeue the tests for evaluation. @PederHP or @stephentoub, would you be able to look into this? The test is in // wait a bit to validate we don't receive another. this is best effort only;
// false negatives are possible.
await Assert.ThrowsAsync<TimeoutException>(() => receivedNotification.Task.WaitAsync(TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken));I would expect that this test should be broadcasting the notifications it expects to intercept instead of random sampling leading to indeterminate output. |
|
That's #228 |
|
This is for addressing #72. I've implemented a portion of the issue that can be extended to support the entire feature request. |
|
Nice stuff. Will be a lot simpler to work with Resources like that! |
src/ModelContextProtocol/Configuration/McpServerBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ModelContextProtocol/Configuration/McpServerBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ModelContextProtocol/Configuration/McpServerBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
| var isMissingListResourceHandlers = originalListResourcesHandler is not { } && listResourceTemplatesHandler is not { }; | ||
| if (resourceCollection is not { IsEmpty: false } && (isMissingListResourceHandlers || readResourceHandler is not { })) | ||
| { | ||
| throw new McpException("Resources capability was enabled, but ListResources, ListResourceTemplates, and/or ReadResource handlers were not specified."); | ||
| } |
There was a problem hiding this comment.
I don't understand this logic. Why is "is missing" true if both originalListResourcesHandler and listResourceTemplatesHandler are non-null?
| // https://modelcontextprotocol.io/specification/2024-11-05/server/tools#capabilities | ||
| // Look to spec for guidance on ListChanged over collection existance. |
| // https://modelcontextprotocol.io/specification/2024-11-05/server/tools#capabilities | ||
| // Look to spec for guidance on ListChanged over collection existance. | ||
| if (capability?.Collection is { } collection) | ||
| //&& capability.ListChanged is true) |
|
This has been sitting for a while. I've started working on a replacement that encompasses equivalents of McpServerTool, McpClientTool, etc. |
…rce methods and update resource handling in tests
…-sdk into resources-enhancements
|
Thanks for the effort here. Replaced by #391. |
Added DI builder enhancements to resources to emulate registration of other MCP list primitives (like tools and prompts).
Motivation and Context
Enhances the api to provide a more consistent way to add resources.
How Has This Been Tested?
I wrote additional tests to cover the cases.
Breaking Changes
Yes. ReadResourceRequest marked its property as nullable instead of required. This was a bug because the spec specified it as not optional. That is changed in the PR.
Types of changes
Checklist
Additional context
Simplified registration of other MCP primitives for future commits.