-
Notifications
You must be signed in to change notification settings - Fork 61
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
Utilize tag on fastAPI paths to only create tools when opted-in #5
base: main
Are you sure you want to change the base?
Utilize tag on fastAPI paths to only create tools when opted-in #5
Conversation
can you help me with this i need to integrate local fastapi with claude desktop how config.json will look( #7 ) |
Thanks @natahlie-shopify for implementing the opt-in tags feature! Sorry for the delay - we've been working through our PR backlog. We'll try to get to it early next week, appreciate your help and contribution |
@wolverinex24 check out @shiraayal-tadata 's instructions in the main readme on how to connect to Claude desktop with an SSE-stdio bridge |
the `create_tools_by_default = false` flag into your `add_mcp_server` call. | ||
|
||
Ex ) | ||
|
||
``` | ||
add_mcp_server( | ||
app, | ||
mount_path="/mcp", | ||
name="My API MCP", | ||
create_tools_by_default = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
False
should be capitalized.
Ex) | ||
|
||
``` | ||
@app.post("/items/", response_model=Item, tags=["items", "include_in_mcp"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am interested in using tags to classify resource types for mcp.
I was thinking about using something like mcp_{resource_type}
, where resource type is resource
, prompt
, etc.
How would you feel about changing the keyword tag to mcp_include
rather than include_in_mcp
? That way we can start to develop a tag structure for future functionality.
I'm not an owner/official maintainer of this repository, just voicing my thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's actually funny you suggest- I'm not an owner or maintainer either, but felt like this was needed so we could use the library.
Someone at my company actually suggested we have a list of tags that we utilize instead for defining mcp tools. the idea is that folks could reuse the tags they already have, or add new ones, to indicate what should have tools built upon them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, 100% agreed.
Also, I only included half a thought in my review. What I meant to say is that I just started working on a piece of code to expose endpoints as specific MCP types. I was thinking about using tags to indicate how each endpoint should be classified.
I'm wondering if maybe that might be the solution. In the code I'm working on, maybe you add tags to endpoints if you want to classify the endpoint as a different MCP type (defaults to tool as normal).
Then you could have an option to exclude_untagged
or something like that to only include endpoints that have an MCP type tag.
Thanks for the PR @natahlie-shopify! I wanted to address both this PR and #16 by @Nicholas-Schaub since they're tackling the same idea with slightly different approaches. Some thoughts on the feature: Include vs. ExcludeHaving options for both including and excluding endpoints makes sense - different API setups would benefit from different defaults. I don't think we should only do the "allow-list" approach. My concern with using tagsI don't think tags are the right place for this. Tags in OpenAPI are meant for grouping operations and domain logic separation. Using them to control MCP exposure feels like overloading their purpose. The bigger issue is that it changes the OpenAPI schema itself, to support our MCP layer, which many developers might not be comfortable with. Maybe the better approach would be to use the existing tags in the schema to decide which endpoints should convert to MCP? See my suggestions below. Better alternativesI think we should take a more programmatic approach. A few options:
What do you think about these alternatives? Option 1 feels most natural, and is the most programmatic. It's safe even when API routes change or tags change. But it introduces challenges like type-safety. The IDE or mypy won't know that Option 2 is easy enough, and I'm leaning towards it as the first solution to add. Option 3 can feel too verbose, specifying every route, and also is sensitive to route changes. Option 4 is also easy to implement and might seem convenient at first, but is also very limiting and requires thorough organization of your OpenAPI schema. |
I will partially disagree on tags not being appropriate for this. As you mentioned, tags are intended to group logic, and one set of groups are mcp types. In addition to making the tags functional selectors here, it also has the benefit to flag the endpoints that are being exposed to MCP in the auto-generated documentation. I think this is helpful for people that want to read the documentation, because as of right now the sse endpoint has no docs and its unclear which endpoints are called. By using the tags, it flags each endpoint so that people can do some reading on it. I won't plant my flag too hard on here, but of all the options you provided, I think tags are probably the best fit that balances easy inclusion to the current tooling as well as documentation. Going through the other options, I think option 1 is probably the worst for long term maintenance. The best is probably option 3, and option 4 is good and could be implemented along with 3. Option 2 doesn't feel right. |
I actually would disagree about 3. Ex) I several a lot of endpoints, some of which can modify sensitive data, some of which can't. I don't want to expose all of them to MCP, but they all live under the '/resources' route. How would I pick and choose which of those routes under The decorator in option 2 is nice, and backwards compatible, and doesn't mess with the underlying FastAPI implementation, but is a bit clunky. For option 4, it is very explicit what is/isn't allowed in. My goal with having an 'include' tags, as well as the boolean was to make ease of incorporating/excluding existing endpoints very very easy. In the case where we might have dozens of routes, making it quick and easy to include/exclude what to include in MCP is very important imo. Tags are a best-practice for FastAPI endpoints, and quick to implement. Yes, they do require more organization, but in terms of explicitness, I like what option 4 provides. |
There are scenarios where we do not want to allow all paths on our FastAPI spec to be available to MCP clients-
This PR will create an 'opt-in' tag for paths, that when set, will prevent the path from being registered as a tool on our MCP server.