[RSPEED-2331] Figure out how to do different things for clients w/wo mcp-app support#288
Conversation
Jazzcort
left a comment
There was a problem hiding this comment.
@owtaylor Here is the feature to detect mcp-app support from the client side. The main thing I'm not quite sure is that if we should keep the env configuration working and overwrite the detection. From my point of view, I think it's good to keep it for local testing purpose. Also, since we can detect the mcp-app support from client now, maybe we can edit docs/usage.md to omit the LINUX_MCP_USE_MCP_APPS setup. I remember you mentioned you're going to make a separate PR to update usage.md, so I'll leave it to you! 😁
src/linux_mcp_server/server.py
Outdated
| return filtered_tools | ||
|
|
||
| async def on_initialize(self, context: MiddlewareContext, call_next): | ||
| assert isinstance(context.message, InitializeRequest), ( |
There was a problem hiding this comment.
Not sure if using assert here is the best solution. Open to change it if there is a better way of doing this.
There was a problem hiding this comment.
narrowing with assert seems fine to me.
src/linux_mcp_server/server.py
Outdated
|
|
||
| # The configuration can overwrite the MCP app support detection, so we have the flexibility to | ||
| # manually turn the Mcp app feature on/off for developing/testing purposes. | ||
| if CONFIG.use_mcp_apps or MCP_APP_MIME_TYPE in mime_types: |
There was a problem hiding this comment.
I think it's helpful to keep the env configuration working so we can easily switch the feature on/off for local testing purpose.
There was a problem hiding this comment.
Yes, but I'd make use_mcp_apps bool | None and make this:
if CONFIG.use_mcp_apps or (CONFIG.use_mcp_apps is None and MCP_APP_MIME_TYPE in mime_types):so you can switch the feature off and not just on.
There was a problem hiding this comment.
Thanks for the catch! 😁 Will change!
owtaylor
left a comment
There was a problem hiding this comment.
Nice that this is in Goose now!
src/linux_mcp_server/server.py
Outdated
| # This is a temperory implementation. When "extensions" property is officially introduced in FastMcp, | ||
| # it will be easier for us to verify mime type. | ||
| capabilities = context.message.params.capabilities.model_dump() | ||
| extensions = capabilities.get("extensions") or {} |
There was a problem hiding this comment.
You should be able to just do getattr(capabilities, "extensions", {})
src/linux_mcp_server/server.py
Outdated
|
|
||
| # This is a temperory implementation. When "extensions" property is officially introduced in FastMcp, | ||
| # it will be easier for us to verify mime type. | ||
| capabilities = context.message.params.capabilities.model_dump() |
There was a problem hiding this comment.
As discussed in a private slack conversation, the extra("allow") was removed from python-sdk's types in modelcontextprotocol/python-sdk@a1d330d - I think we assume that extensions will be be there properly once v2 of python-sdk is released - hopefully in a compatible way.
Note that this is not FastMCP - it's the base mcp module:
# For python-sdk -1.x, count on extensibility of protocol types - while this is being
# removed for v2, hopefully extensions will be there properly.
src/linux_mcp_server/server.py
Outdated
| assert fastmcp_context is not None, ( | ||
| "FastMCP framework error: context.fastmcp_context should not be None inside on_initialize" | ||
| ) | ||
| fastmcp_instance = fastmcp_context.fastmcp |
There was a problem hiding this comment.
Isn't this just the same as our global mcp variable? Retrieving it this way makes it look like something special is going on.
There was a problem hiding this comment.
Yes, it's the same as our global mcp. I was thinking "what if we don't have the global mcp access" when I implemented this. 😅 Will change it to use the global variable so it looks neat.
src/linux_mcp_server/server.py
Outdated
| fastmcp_instance.add_tool(execute_script) | ||
| fastmcp_instance.add_tool(reject_script) | ||
| else: | ||
| fastmcp_instance.add_tool(run_script_modify) |
There was a problem hiding this comment.
This isn't going to work with http transport. Can we A) add everything B) set a flag on the context C) Filter list_tools? If that ends up being too hard, we might need to upgrade to FastMCP 3.0 for it's builtin tool visibility support.
There was a problem hiding this comment.
It would be great if we can upgrade it to 3.0.
B) set a flag on the context
@owtaylor Are you referring to set_status to set a flag? Last time I tried it, the status was not persistent among requests for 2.x - I set status for Client A in the initialize request but can't get the status I set in the following tool call request. I have to manually create a store to save the session_id. However, set_status works fine for 3.0.
Let me test set_status again for both 2.x and 3.0! Will report to you later!
src/linux_mcp_server/server.py
Outdated
| ) | ||
| fastmcp_instance = fastmcp_context.fastmcp | ||
|
|
||
| # This is a temperory implementation. When "extensions" property is officially introduced in FastMcp, |
There was a problem hiding this comment.
| # This is a temperory implementation. When "extensions" property is officially introduced in FastMcp, | |
| # python-sdk v1.x uses pydantic's extra("allow") capability to extend the protocol types | |
| # with unknown fields. This is being removed for v2.0 | |
| # https://github.com/modelcontextprotocol/python-sdk/pull/1937 | |
| # but hopefully that will support `extensions` properly when released. |
src/linux_mcp_server/server.py
Outdated
| # This is a temperory implementation. When "extensions" property is officially introduced in FastMcp, | ||
| # it will be easier for us to verify mime type. | ||
| capabilities = context.message.params.capabilities.model_dump() | ||
| extensions = capabilities.get("extensions") or {} |
There was a problem hiding this comment.
I think you can simply do:
extensions = capabilities.getattr("extensions", {})
src/linux_mcp_server/server.py
Outdated
|
|
||
| # The configuration can overwrite the MCP app support detection, so we have the flexibility to | ||
| # manually turn the Mcp app feature on/off for developing/testing purposes. | ||
| if CONFIG.use_mcp_apps or MCP_APP_MIME_TYPE in mime_types: |
There was a problem hiding this comment.
Yes, but I'd make use_mcp_apps bool | None and make this:
if CONFIG.use_mcp_apps or (CONFIG.use_mcp_apps is None and MCP_APP_MIME_TYPE in mime_types):so you can switch the feature off and not just on.
ea0c7c5 to
2742930
Compare
…mcp-app support Since both block/goose#6927 and block/goose#6931 are merged into Goose, we can now detect mcp app compatibility from Goose client during initialize phase. feat: dynamically inject tools based on client UI capabilities - Change `use_mcp_apps` config default from `False` to `None` to enable auto-detection, while preserving manual overrides. - Add `MCP_UI_EXTENSION` constant (`io.modelcontextprotocol/ui`) to check for UI extension support. - Register both interactive and non-interactive script modification tools by default. - Update `DynamicDiscoveryMiddleware` to inspect `client_params.capabilities.extensions` during `on_list_tools`. - Dynamically filter out either `run_script_modify` or `run_script_modify_interactive` based on whether the client supports the `MCP_APP_MIME_TYPE`.
2742930 to
51792c7
Compare
| mcp.add_tool(execute_script) | ||
| mcp.add_tool(reject_script) | ||
| # With mcp-app feature | ||
| mcp.add_tool(run_script_modify_interactive) |
There was a problem hiding this comment.
@owtaylor Since we're now adding all tools, there is actually no need to transform functions into Tool and inject tools like this. Do you think we should change these back to decorator format @mcp.tool?
Since both block/goose#6927 and block/goose#6931 are merged into Goose, we can now detect mcp app compatibility from Goose client during initialize phase.
feat: dynamically inject tools based on client UI capabilities
use_mcp_appsconfig default fromFalsetoNoneto enable auto-detection, while preserving manual overrides.MCP_UI_EXTENSIONconstant (io.modelcontextprotocol/ui) to check for UI extension support.DynamicDiscoveryMiddlewareto inspectclient_params.capabilities.extensionsduringon_list_tools.run_script_modifyorrun_script_modify_interactivebased on whether the client supports theMCP_APP_MIME_TYPE