Skip to content

Make ResourceTemplate list callback optional #566

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.

Sign up for GitHub

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
wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −3
Open

Make ResourceTemplate list callback optional #566

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/server/mcp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,17 +934,17 @@ export class ResourceTemplate {
uriTemplate: string | UriTemplate,
private _callbacks: {
/**
* A callback to list all resources matching this template. This is required to specified, even if `undefined`, to avoid accidentally forgetting resource listing.
* A callback to list all resources matching this template.
*/
list: ListResourcesCallback | undefined;
list?: ListResourcesCallback;
Copy link

Choose a reason for hiding this comment

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

this is interesting, i need to read the code more to grok what's going, but i see that foo: bar | undefined is frequently preferred over the usual foo?: bar here, and I suspect it's intentional, and my current guess is it has to do with zodToJsonSchema, where this is a distinction with a difference, apparently, because in the generated JSONSchema, list will be included in "required" if you use the bar | undefined syntax, whereas it will not be required with the foo?: syntax... I don't know how all these pieces fit together, and my interpretation does not comport well with the the comment about forgetting things, but my suspicion is there is (or was thought to be) a good reason for this. So far I haven't found how JSONSchema is used in this repo, maybe there is validation against schema.ts to check compliance(?). Obviously someone more directly involved in writing this should probably weigh in but without further context about the issues discussed above, I don't think it's so simple.

Copy link
Author

@kentcdodds kentcdodds May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Basically, the way it is currently, you have to include the callback object with a list property even if you can't list your resources and you must set it to undefined.

@jspahrsummers was the original author of this code, and no explanation was given other than the comment in the code. I do not think other SDKs have this same requirement. But in any case, I don't think supplying an undefined callback should be required.

Copy link

@hesreallyhim hesreallyhim May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Basically, the way it is currently, you have to include the callback object with a list property even if you can't list your resources and you must set it to undefined.

Right, so from what I gleaned, this makes a difference if you use something like zodToJsonSchema, because it means the schema will put "list" in the "required" properties array (even if the value is undefined, which I'm not sure if that's recommended anyway). But, either way, you can see this style repeated in the file, so it appears deliberate to me, so I'll just defer to someone who's more intimate with the codebase. But in any case, I hope you see my point, I just don't know how zodToJsonSchema is being leveraged in this repo.

EDIT: If I had to speculate (which I don't), I'd guess it might enable you to generate a JSONSchema and then validate it against the protocol's JSONSchema...


/**
* An optional callback to autocomplete variables within the URI template. Useful for clients and users to discover possible values.
*/
complete?: {
[variable: string]: CompleteResourceTemplateCallback;
};
},
} = {},
) {
this._uriTemplate =
typeof uriTemplate === "string"
Expand Down