Skip to content

Conversation

@bhosmer-ant
Copy link
Contributor

@bhosmer-ant bhosmer-ant commented Mar 17, 2025

Removes Annotated interface, and instead adds the annotations property directly to the interfaces that used to inherit from it.

Previously:

export interface Annotated {
  annotations?: {
    // annotation properties
  }
}

export interface Foo extends Annotated {
  ...
}

Now:

export interface Annotations {
  // annotation properties
}

export interface Foo {
  annotations?: Annotations;
  ...
}

Motivation and Context

Simplifies the schema, reduces friction for hosting different combinations of annotation properties on different protocol objects.

How Has This Been Tested?

Breaking Changes

  • Wire format is unchanged.
  • Typed interface hierarchy is changed, SDKs should update/provide backwards compatibility as needed.

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

Additional context

Copy link
Member

@jerome3o-anthropic jerome3o-anthropic left a comment

Choose a reason for hiding this comment

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

Thanks - I think this is good

[q] Was the old implementation part of an official release?

@bhosmer-ant
Copy link
Contributor Author

bhosmer-ant commented Mar 17, 2025

Thanks - I think this is good

[q] Was the old implementation part of an official release?

Looks like it's part of the schema in 2024-11-05

LMK if this means I should hold off on merging btw, will wait to hear 😁

@jspahrsummers
Copy link
Member

As long as it's backwards compatible on the wire, that's the key thing. SDKs can always provide their own backward compatibility for typed interfaces.

Small nitpick: I wouldn't generally refer to these as "classes," as they're just descriptions of JSON that gets sent on the wire.

@bhosmer-ant
Copy link
Contributor Author

bhosmer-ant commented Mar 18, 2025

As long as it's backwards compatible on the wire, that's the key thing. SDKs can always provide their own backward compatibility for typed interfaces.

Couple questions re syncing schema changes to the SDKs we maintain:

  • the Python SDK should continue to work as-is since it doesn't change the wire format, but we'll want to update the classes soonish. Are there other dependencies/ripple effects beyond the immediate SDK codebase we need to coordinate?
  • if the TS SDK uses these definitions here, does that mean code in the SDK will break until we update it?

Small nitpick: I wouldn't generally refer to these as "classes," as they're just descriptions of JSON that gets sent on the wire.

Ah yep missed the edit (only in the description right?)

@jspahrsummers
Copy link
Member

This doesn't change anything on the wire, so it shouldn't break any SDK immediately. When we incorporate the typing changes, we might want to make sure the (e.g.) Python and JS changes are backwards compatible, but I don't expect that will be difficult. Easiest way would be to just bring this change in and see what breaks.

@bhosmer-ant
Copy link
Contributor Author

This doesn't change anything on the wire, so it shouldn't break any SDK immediately. When we incorporate the typing changes, we might want to make sure the (e.g.) Python and JS changes are backwards compatible, but I don't expect that will be difficult. Easiest way would be to just bring this change in and see what breaks.

Makes sense. FWIW I looked around to see what the coupling was between the spec schema and the SDKs, recording observations here for posterity/reality check:

  • it looks like the Python SDK already declares the annotations property inline (example, so the change in this PR actually moves the schema closer to the way types are laid out there
  • if I'm reading it right, the TS sdk doesn't declare annotations properties at all? example

Anyway, merging.

@bhosmer-ant bhosmer-ant merged commit eb4abdf into main Mar 18, 2025
6 checks passed
@bhosmer-ant bhosmer-ant deleted the basil/annotations_prop branch March 18, 2025 15:28
bhosmer-ant added a commit that referenced this pull request Mar 18, 2025
jspahrsummers added a commit that referenced this pull request Apr 7, 2025
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.

4 participants