Skip to content

Refactor Resource/ResourceTemplate to subclass Annotated #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Redth
Copy link
Contributor

@Redth Redth commented Mar 21, 2025

The Resource and ResourceTemplate types should derive from a base Annotated type having the Annotations property as per the spec

This small change makes the C# types match the schema more closely.

Also renamed Resources.cs to Resource.cs to reflect the type name.

Motivation and Context

As per #20 refactoring these two types

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

The `Resource` and `ResourceTemplate` types should derive from a base `Annotated` type having the `Annotations` property as per the [spec](https://github.com/modelcontextprotocol/specification/blob/main/schema/2024-11-05/schema.ts#L821)

This small change makes the C# types match the schema more closely.

Also renamed Resources.cs to Resource.cs to reflect the type name.
Copy link
Collaborator

@PederHP PederHP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct to me. Also checked the Annotations type, just in case there was a discrepancy on that one too. It matches the spec, although the Min and Mix properties don't really have a equivalent in .NET, unless I there's a facility I don't know about. But it's documented in the comment. And it's tangential to this.

@stephentoub
Copy link
Contributor

the Min and Mix properties don't really have a equivalent in .NET, unless I there's a facility I don't know about

Can you tell me more? What are these such that they don't have an equivalent?

@PederHP
Copy link
Collaborator

PederHP commented Mar 21, 2025

the Min and Mix properties don't really have a equivalent in .NET, unless I there's a facility I don't know about

Can you tell me more? What are these such that they don't have an equivalent?

"annotations": {
                    "properties": {
                        "audience": {
                            "description": "Describes who the intended customer of this object or data is.\n\nIt can include multiple entries to indicate content useful for multiple audiences (e.g., `[\"user\", \"assistant\"]`).",
                            "items": {
                                "$ref": "#/definitions/Role"
                            },
                            "type": "array"
                        },
                        "priority": {
                            "description": "Describes how important this data is for operating the server.\n\nA value of 1 means \"most important,\" and indicates that the data is\neffectively required, while 0 means \"least important,\" and indicates that\nthe data is entirely optional.",
                            "maximum": 1,
                            "minimum": 0,
                            "type": "number"
                        }
                    },
                    "type": "object"
                }

I don't know of an attribute (Range perhaps but it's not really used with JSON serialization afaik) that matches this pattern.

(I also think it's an oddly narrow property, as a Client author (as a service/app developer) I wouldn't know how to interpret this on a Resource from an arbitrary server).

@stephentoub
Copy link
Contributor

I don't know of an attribute (Range perhaps but it's not really used with JSON serialization afaik) that matches this pattern.

Ah, I see. @eiriktsarpalis, is there something built-in you'd expect to be used here?

@stephentoub stephentoub merged commit 0cbcad7 into modelcontextprotocol:main Mar 21, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants