Project scoped tools #596
Merged
Project scoped tools #596
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on behavior Add `--project-scoped-tools` CLI flag and `UNITY_MCP_PROJECT_SCOPED_TOOLS` environment variable to control whether custom tools are registered globally or scoped to specific Unity projects. Closes CoplayDev#416
Add UI toggle in Connection section to control project-scoped tools flag when using HTTP Local transport. The toggle: - Defaults to enabled (true) - Persists state in EditorPrefs - Only displays when HTTP Local transport is selected - Automatically appends `--project-scoped-tools` flag to uvx server command - Updates manual config display when toggled
Contributor
Reviewer's GuideAdds a configurable "project scoped tools" mode that controls whether custom Unity MCP tools remain per-project via resources/function tools or are registered as global MCP tools, wiring this through server startup, CLI/env flags, HTTP tooling registration, and the Unity editor UI/launch command. Sequence diagram for global custom tool registration and executionsequenceDiagram
participant UnityEditor
participant UnityPlugin as UnityPluginHubClient
participant PluginHub
participant CustomToolService
participant FastMCP
UnityEditor->>UnityPlugin: Register custom tools for project
UnityPlugin->>PluginHub: send RegisterToolsMessage(session_id, tools)
PluginHub->>CustomToolService: register_global_tools(tools)
CustomToolService->>CustomToolService: _register_global_tool for each tool
CustomToolService->>FastMCP: mcp.tool(name, description)(handler)
FastMCP-->>CustomToolService: global tool registered
actor MCPClient
MCPClient->>FastMCP: invoke global custom tool with params
FastMCP->>CustomToolService: global handler(ctx, kwargs)
CustomToolService->>CustomToolService: resolve_project_id_for_unity_instance
CustomToolService->>CustomToolService: execute_tool(project_id, tool_name, unity_instance, params)
CustomToolService-->>FastMCP: MCPResponse
FastMCP-->>MCPClient: tool result
Class diagram for updated CustomToolService project-scoped vs global toolsclassDiagram
class FastMCP
class Context
class ToolParameterModel {
+string name
+string type
+bool required
+string default_value
}
class ToolDefinitionModel {
+string name
+string description
+List~ToolParameterModel~ parameters
}
class MCPResponse {
+bool success
+string message
}
class CustomToolService {
-FastMCP _mcp
-bool _project_scoped_tools
-dict~string, dict~string, ToolDefinitionModel~~ _project_tools
-dict~string, string~ _hash_to_project
-dict~string, ToolDefinitionModel~ _global_tools
-static CustomToolService _instance
+CustomToolService(mcp FastMCP, project_scoped_tools bool)
+get_instance() CustomToolService
+register_tools(request Request) JSONResponse
+execute_tool(project_id string, tool_name string, unity_instance string, params dict~string, object~) MCPResponse
+register_global_tools(tools List~ToolDefinitionModel~) void
-_register_project_tools(project_id string, tools List~ToolDefinitionModel~, project_hash string) tuple~List~string~, List~string~~
-_register_global_tool(definition ToolDefinitionModel) void
-_build_global_tool_handler(definition ToolDefinitionModel) function
-_build_signature(definition ToolDefinitionModel) Signature
-_build_annotations(definition ToolDefinitionModel) dict~string, object~
-_map_param_type(param ToolParameterModel) object
-_coerce_default(value string, param_type string) object
-_register_tool(project_id string, tool ToolDefinitionModel) void
-_is_registered(project_id string, tool_name string) bool
-_safe_response(response object) dict~string, string~
}
CustomToolService --> FastMCP : uses
CustomToolService --> ToolDefinitionModel : registers
ToolDefinitionModel --> ToolParameterModel : has
CustomToolService --> MCPResponse : returns
CustomToolService --> Context : global handler ctx param
Architecture flow diagram for project_scoped_tools true vs falseflowchart TD
UE[Unity Editor]
PL[Unity Plugin]
SV[MCP Server]
CTS[CustomToolService]
RES[custom_tools resource]
EXECTOOL[execute_custom_tool tool]
GLOBALTOOLS[Global custom tools]
UE --> PL
PL -->|Launch server with flag| SV
SV --> CTS
CTS -->|project_scoped_tools true| RES
CTS -->|project_scoped_tools true| EXECTOOL
CTS -->|project_scoped_tools false| GLOBALTOOLS
subgraph Project_scoped_mode
direction LR
CTS -. registers .-> RES
CTS -. uses .-> EXECTOOL
end
subgraph Global_mode
direction LR
PL -->|RegisterToolsMessage| CTS
CTS -. register_global_tools .-> GLOBALTOOLS
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Project-Scoped Tools EditorPref and UI toggle, propagates it into local server launch args, and updates the server to support project-scoped vs. global tool/resource registration and exposure based on that flag. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor UI
participant Prefs as EditorPrefs
participant ServerMgr as ServerManagementService
participant LocalServer as MCP Process
participant CustomTool as CustomToolService
Editor->>Prefs: Set ProjectScopedToolsLocalHttp
Editor->>ServerMgr: Request refreshed launch command
ServerMgr->>Prefs: Read ProjectScopedToolsLocalHttp
ServerMgr->>LocalServer: Launch with/without --project-scoped-tools
LocalServer->>CustomTool: Initialize(project_scoped_tools)
alt project_scoped_tools == true
CustomTool->>LocalServer: Register tools per-project
LocalServer->>LocalServer: Register custom_tools resource
else project_scoped_tools == false
CustomTool->>LocalServer: Register tools globally
LocalServer->>LocalServer: Skip custom_tools resource
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
Works well, this closes #416 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Moving
custom_tool_service = CustomToolService(mcp, ...)from module import time intocreate_mcp_serverchanges when the global is defined; if other modules referencemain.custom_tool_serviceduring import or beforemain()runs, this will now beNameError/None, so consider either keeping a module-level initialization or replacing the global with an accessor (e.g., viaCustomToolService.get_instance()) to avoid lifecycle issues. - The truthy parsing for
--project-scoped-toolsandUNITY_MCP_PROJECT_SCOPED_TOOLS(("1", "true", "yes", "on")) is embedded directly inmain; consider extracting this into a small helper so any future flags/env vars that need the same semantics can share a single, tested implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Moving `custom_tool_service = CustomToolService(mcp, ...)` from module import time into `create_mcp_server` changes when the global is defined; if other modules reference `main.custom_tool_service` during import or before `main()` runs, this will now be `NameError`/`None`, so consider either keeping a module-level initialization or replacing the global with an accessor (e.g., via `CustomToolService.get_instance()`) to avoid lifecycle issues.
- The truthy parsing for `--project-scoped-tools` and `UNITY_MCP_PROJECT_SCOPED_TOOLS` (`("1", "true", "yes", "on")`) is embedded directly in `main`; consider extracting this into a small helper so any future flags/env vars that need the same semantics can share a single, tested implementation.
## Individual Comments
### Comment 1
<location> `Server/src/services/custom_tool_service.py:398` </location>
<code_context>
+ return dict
+ return str
+
+ def _coerce_default(self, value: str | None, param_type: str):
+ if value is None:
+ return None
</code_context>
<issue_to_address>
**suggestion:** Allow `_coerce_default`’s `param_type` annotation to reflect the actual usage.
Since `_build_signature` passes `param.type`, which may be `None`, the current annotation `param_type: str` doesn’t match actual usage. Please update it to `param_type: str | None` (or `Optional[str]`) so the signature reflects the accepted inputs and stays consistent with the internal `param_type or "string"` guard.
```suggestion
def _coerce_default(self, value: str | None, param_type: str | None):
```
</issue_to_address>
### Comment 2
<location> `Server/src/transport/plugin_hub.py:323-324` </location>
<code_context>
+
+ service = CustomToolService.get_instance()
+ service.register_global_tools(payload.tools)
+ except Exception as exc:
+ logger.debug(
+ "Skipping global custom tool registration: %s", exc)
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Broadened exception handling for global tool registration may hide real configuration errors.
Catching `Exception` here will also hide programming bugs in `get_instance`, `register_global_tools`, or access to `payload.tools`, and only log them at debug level. Consider narrowing this to the specific expected failures (e.g., initialization/lookup errors) and logging any truly unexpected exceptions at warning level so operational issues are surfaced.
Suggested implementation:
```python
logger.info(
f"Registered {len(payload.tools)} tools for session {session_id}")
try:
from services.custom_tool_service import CustomToolService
service = CustomToolService.get_instance()
service.register_global_tools(payload.tools)
except (ImportError, AttributeError) as exc:
# Expected issues such as missing service or tools attribute;
# skip global registration quietly in these cases.
logger.debug(
"Skipping global custom tool registration due to expected error: %s",
exc,
)
except Exception as exc:
# Unexpected errors may indicate real configuration or programming issues
logger.warning(
"Unexpected error during global custom tool registration; "
"custom tools may not be available globally",
exc_info=exc,
)
async def _handle_command_result(self, payload: CommandResultMessage) -> None:
```
If `CustomToolService.get_instance()` or `register_global_tools` raise specific, known exception types (e.g., `CustomToolInitializationError`, `ToolRegistryError`), you should also add those to the expected-exception tuple in the first `except` clause so they are treated as anticipated failures rather than escalated as warnings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Server/src/services/tools/__init__.py (1)
49-62: Tool count in log message may be inaccurate when skipping tools.When
execute_custom_toolis skipped, the final log at line 62 still reportslen(tools)which includes the skipped tool. For accuracy, consider tracking the actual registered count likeresources/__init__.pydoes withregistered_count.🔧 Suggested fix
+ registered_count = 0 for tool_info in tools: func = tool_info['func'] tool_name = tool_info['name'] description = tool_info['description'] kwargs = tool_info['kwargs'] if not project_scoped_tools and tool_name == "execute_custom_tool": logger.info( "Skipping execute_custom_tool registration (project-scoped tools disabled)") continue # Apply the `@mcp.tool` decorator, telemetry, and logging wrapped = log_execution(tool_name, "Tool")(func) wrapped = telemetry_tool(tool_name)(wrapped) wrapped = mcp.tool( name=tool_name, description=description, **kwargs)(wrapped) tool_info['func'] = wrapped logger.debug(f"Registered tool: {tool_name} - {description}") + registered_count += 1 - logger.info(f"Registered {len(tools)} MCP tools") + logger.info(f"Registered {registered_count} MCP tools")
🤖 Fix all issues with AI agents
In `@Server/src/main.py`:
- Around line 428-432: The CLI currently only defines
parser.add_argument("--project-scoped-tools", ...) which can turn project-scoped
tools on but cannot override an environment variable to turn it off; add a
complementary flag (e.g., parser.add_argument("--global-tools",
action="store_true", ...)) and place both flags into a mutually exclusive
argparse group (or implement explicit three-state parsing) and update the logic
that reads UNITY_MCP_PROJECT_SCOPED_TOOLS to allow the CLI flag to override the
env var in both directions so a user can explicitly enable or disable
project-scoped tools for a single run.
🧹 Nitpick comments (2)
Server/src/transport/plugin_hub.py (1)
318-325: Consider catching more specific exceptions.The broad
Exceptioncatch (flagged by static analysis) could mask unexpected errors. SinceCustomToolService.get_instance()raisesRuntimeErrorwhen uninitialized, and other failures are likelyAttributeErroror import-related, consider narrowing the scope.♻️ Suggested improvement
try: from services.custom_tool_service import CustomToolService service = CustomToolService.get_instance() service.register_global_tools(payload.tools) - except Exception as exc: + except (RuntimeError, ImportError) as exc: logger.debug( "Skipping global custom tool registration: %s", exc)That said, the current implementation is functionally correct for optional/fallback behavior—this is a minor refinement.
MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs (1)
131-137: Consider removing defensive null checks if the UXML update is guaranteed.If the new elements are now required, it may be cleaner to rely on initialization/tests and let failures surface rather than guarding each access. If older UXML compatibility is still a requirement, consider an explicit upgrade note or test coverage for the fallback path. Based on learnings, prefer relying on the established testing process over defensive null-checks in Editor windows.
♻️ Optional simplification (if older UXML is no longer supported)
- if (projectScopedToolsToggle != null) - { - projectScopedToolsToggle.value = EditorPrefs.GetBool( - EditorPrefKeys.ProjectScopedToolsLocalHttp, - true - ); - } + projectScopedToolsToggle.value = EditorPrefs.GetBool( + EditorPrefKeys.ProjectScopedToolsLocalHttp, + true + );- if (projectScopedToolsToggle != null) - { - projectScopedToolsToggle.RegisterValueChangedCallback(evt => - { - EditorPrefs.SetBool(EditorPrefKeys.ProjectScopedToolsLocalHttp, evt.newValue); - UpdateHttpServerCommandDisplay(); - OnManualConfigUpdateRequested?.Invoke(); - }); - } + projectScopedToolsToggle.RegisterValueChangedCallback(evt => + { + EditorPrefs.SetBool(EditorPrefKeys.ProjectScopedToolsLocalHttp, evt.newValue); + UpdateHttpServerCommandDisplay(); + OnManualConfigUpdateRequested?.Invoke(); + });- if (projectScopedToolsRow == null) - { - return; - } - bool useHttp = transportDropdown != null && (TransportProtocol)transportDropdown.value != TransportProtocol.Stdio; bool httpLocalSelected = IsHttpLocalSelected(); projectScopedToolsRow.style.display = useHttp && httpLocalSelected ? DisplayStyle.Flex : DisplayStyle.None;Also applies to: 246-254, 485-490
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
…conversion Remove unnecessary environment variable conversion for project_scoped_tools flag.
…ation Split exception handling to distinguish between expected RuntimeError (service not initialized) and unexpected errors.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Server/src/main.py`:
- Around line 326-334: The code currently manipulates FastMCP internals
(mcp._get_additional_http_routes and mcp._additional_http_routes) to register a
WebSocketRoute for PluginHub; instead, stop touching private attributes and
register the endpoint via FastMCP's public integration point: either mount a
WebSocket route that delegates to mcp.websocket_endpoint at "/hub/plugin" or
expose PluginHub through the FastMCP app by mounting the FastMCP ASGI app in the
outer ASGI layer. Concretely, remove uses of _get_additional_http_routes and
_additional_http_routes, and replace with logic that, when HTTP/WebSocket
transport is active, wires "/hub/plugin" to mcp.websocket_endpoint (or mounts
the whole mcp app) so PluginHub behavior is reached through the documented
public API (refer to symbols: mcp, PluginHub, WebSocketRoute,
mcp.websocket_endpoint).
Comment on lines
+326
to
334
| # Mount plugin websocket hub at /hub/plugin when HTTP transport is active | ||
| existing_routes = [ | ||
| route for route in mcp._get_additional_http_routes() | ||
| if isinstance(route, WebSocketRoute) and route.path == "/hub/plugin" | ||
| ] | ||
| if not existing_routes: | ||
| mcp._additional_http_routes.append( | ||
| WebSocketRoute("/hub/plugin", PluginHub)) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd Server && head -n 350 src/main.py | tail -n +290Repository: CoplayDev/unity-mcp
Length of output: 2370
🏁 Script executed:
cd Server && rg "_additional_http_routes|_get_additional_http_routes" --type py -B 2 -A 2Repository: CoplayDev/unity-mcp
Length of output: 543
🌐 Web query:
FastMCP WebSocket route public API documentation
💡 Result:
Here are the FastMCP WebSocket / public-route docs and related resources:
- FastMCP WebSocket + route mapping and OpenAPI integration (official docs). Covers WebSocket endpoint usage, route mapping and public routes. [1]
- FastMCP GitHub repository (API reference, examples, websocket endpoint implementation and usage). [2]
- MCPSock (Python WebSocket client/server for FastMCP) — installation and examples for WebSocket routes and handlers. [3]
If you want, I can open any of those pages and extract the exact WebSocket route API examples or copy the relevant code snippets. Which page should I pull details from?
🏁 Script executed:
cd Server && grep -r "class FastMCP" --include="*.py" | head -5Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
cd Server && python3 << 'EOF'
import inspect
try:
from fastmcp import FastMCP
# List all public methods of FastMCP
public_methods = [m for m in dir(FastMCP) if not m.startswith('_')]
print("FastMCP public methods:")
for method in sorted(public_methods):
print(f" {method}")
# Check if there's a method related to routes or websocket
route_methods = [m for m in public_methods if 'route' in m.lower() or 'websocket' in m.lower()]
print("\nRoute/WebSocket related methods:")
for method in route_methods:
print(f" {method}")
except Exception as e:
print(f"Error: {e}")
EOFRepository: CoplayDev/unity-mcp
Length of output: 149
🌐 Web query:
FastMCP public API add_websocket_route custom_route documentation
💡 Result:
Short answer:
-
FastMCP exposes a decorator FastMCP.custom_route(...) to register normal HTTP routes on the server (health checks, OAuth callbacks, small admin APIs). Use
@mcp.custom_route(path, methods=[...]) with an async Starlette Request handler. [1][2] -
There is no public helper named add_websocket_route in the FastMCP docs. To add a WebSocket endpoint, mount FastMCP’s websocket endpoint into an ASGI app (e.g., Starlette/FastAPI) using a WebSocketRoute that points at mcp.websocket_endpoint, or mount the FastMCP app and add WebSocket routes in the outer ASGI app. Example patterns are shown in the docs. [3][1]
References
- FastMCP server API / custom_route docs. [1]
- Running/HTTP deployment and custom routes. [2]
- WebSocket integration example (mount WebSocketRoute -> mcp.websocket_endpoint). [3]
Coupling to FastMCP private API and undocumented WebSocket pattern.
The code accesses mcp._get_additional_http_routes() and mcp._additional_http_routes directly—these are implementation details that FastMCP may change without notice. Additionally, FastMCP's documented approach for WebSocket endpoints is to mount a WebSocketRoute pointing to mcp.websocket_endpoint, not append custom handlers to internal lists.
Replace this pattern with the documented public approach: mount the WebSocket route through FastMCP's proper integration point (either using mcp.websocket_endpoint or mounting the FastMCP app in the outer ASGI layer). This will ensure compatibility with FastMCP updates and follow the intended architecture.
🤖 Prompt for AI Agents
In `@Server/src/main.py` around lines 326 - 334, The code currently manipulates
FastMCP internals (mcp._get_additional_http_routes and
mcp._additional_http_routes) to register a WebSocketRoute for PluginHub;
instead, stop touching private attributes and register the endpoint via
FastMCP's public integration point: either mount a WebSocket route that
delegates to mcp.websocket_endpoint at "/hub/plugin" or expose PluginHub through
the FastMCP app by mounting the FastMCP ASGI app in the outer ASGI layer.
Concretely, remove uses of _get_additional_http_routes and
_additional_http_routes, and replace with logic that, when HTTP/WebSocket
transport is active, wires "/hub/plugin" to mcp.websocket_endpoint (or mounts
the whole mcp app) so PluginHub behavior is reached through the documented
public API (refer to symbols: mcp, PluginHub, WebSocketRoute,
mcp.websocket_endpoint).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a setting on the MCP server that changes the behavior of custom tools. Since v8, custom tools have been executable via another function tool, and available via a resource. This is needed when one MCP server is used for multiple Unity projects, as custom tools are defined on a project level.
However, for many it's fine for the custom tool to be available globally like the other tools (i.e. how they were in v7).
We add a CLI arg to toggle between these 2 different ways to handle custom tools
Summary by Sourcery
Add configurable scoping for custom tools so they can be either project-specific or globally registered, and wire this through the server startup, plugin hub, and Unity editor HTTP launch flow.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.