Skip to content

Simplify and standardize SDK #452

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

Open
pete-grunge-ai opened this issue May 5, 2025 · 1 comment
Open

Simplify and standardize SDK #452

pete-grunge-ai opened this issue May 5, 2025 · 1 comment
Labels
enhancement New feature or request

Comments

@pete-grunge-ai
Copy link

Is your feature request related to a problem? Please describe.
This library is hard to subclass and leverage in other projects. There are some relatively small changes to make it much extensible and easier to consume. Overly complex method overloading has caused bugs when extending the base classes. Using typescript is very hard because you have to discover the internal file structure to understand how types are exported. Some private member variables should be defined as. protected or public or protected utility methods should be added. Some simple cleanup in the code is needed for efficiency.

Describe the solution you'd like

  1. Overly complex method overloading has caused bugs when extending classes. (opened separate issue). The current approach is just too brittle and results in a complex method to check parameters and fallbacks instead of simply defining a simple, and more declarative definition:

For example:

tool(name: string, cb: ToolCallback): RegisteredTool;
tool(name: string, description: string, cb: ToolCallback): RegisteredTool;
.....

requires consumers to get the order correct, and much worse -- bundling two unrelated types as one parameter like -->
paramsSchemaOrAnnotations: Args | ToolAnnotation

makes it really confusing.

Please consider adding to a simple too() method with a nicely typed props or config object with specific elements for each type of object -- it will be easier to read, and configure, and less prone to error. You can add a secondary method if you want to keep the prior as backwards compatable.

  1. import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js";

ugh? I have to find like 5 different files based on the type I'm importing, and then they are postfixed with .js??

just export them all with unique names like import {McpServer, Transport} from "@mcp/types" or something simple -- this is an sdk -- should be super simple to import without main types without having to get into the weeds of the actual library

  1. Consider making some elements like _registeredTools protected or add a helper method and fire the event when it changes. I've had subclass the main class to work around a bug -- but now i have to keep a fork active because internal member variables are private -- again as it is an sdk -- it should be open enough to subclass with fair warnings to the consumer of the service

  2. General code cleanup. While debugging some of these other items, noticed some poor logic conditions like

for (const message of messages) {
onMessage?(message)
}

checking a conditional on each iteration instead of wrapping it in an if clause to check if the for loop should even execute -- saw this a couple of times

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@pete-grunge-ai pete-grunge-ai added the enhancement New feature or request label May 5, 2025
@kmitchell
Copy link

Strongly agree with revisiting the tool registration path. The current overloading structure is pretty challenging to work with! Adding a well-typed options object to register tools with seems like a no-brainer, though I haven't taken as deep of a look here as Pete has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants