fix: enable project-scoped custom tools in stdio mode#1111
fix: enable project-scoped custom tools in stdio mode#1111Warlander wants to merge 5 commits intoCoplayDev:betafrom
Conversation
- ToolDiscoveryService: add AppDomain fallback scan for [McpForUnityTool] types - StdioBridgeHost: include project_scoped_tools flag in heartbeat JSON - McpToolsSection: update tooltip and default to reflect stdio support - models.py: add project_scoped_tools field to UnityInstanceInfo - port_discovery.py: read project_scoped_tools from status JSON - main.py: enable project-scoped tools when Unity instance requests it
📝 WalkthroughWalkthroughAdds a project-scoped tools flag across editor/server (heartbeat, port discovery, UnityInstanceInfo), makes server startup honor explicit overrides or enablement by instances, strengthens editor tool discovery with TypeCache + AppDomain fallback and per-type error handling, and removes the global-tool registration early-return. UI tooltip and docs updated. ChangesTool Discovery & Project-Scoped Tools
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Warlander
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
1050-1066:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHeartbeat default contradicts UI toggle default for the same EditorPref.
StdioBridgeHost.WriteHeartbeatreadsEditorPrefKeys.ProjectScopedToolsLocalHttpwith defaulttrue, butMcpToolsSection.RegisterCallbacks(Line 76–79) reads the same key with defaultfalse. On a fresh install (key absent):
- The UI toggle visibly shows OFF.
- The heartbeat reports
project_scoped_tools=true.Server/src/main.py(Line 896–909) sees the flag and auto-enables project-scoped mode without the user opting in.Result: the server runs in project-scoped mode while the editor UI claims it's disabled, which is confusing and breaks the "Explicit CLI/env overrides always win; otherwise honor what Unity reports" contract.
Pick one default and use it in both places (and ideally read it through a shared helper).
🛠️ One-line alignment if "off by default" is the intended behavior
bool projectScopedTools = EditorPrefs.GetBool( EditorPrefKeys.ProjectScopedToolsLocalHttp, - true // default to true so stdio behaves like HTTP Local by default + false // must match McpToolsSection toggle default so UI and heartbeat agree );If "on by default" is preferred instead, flip
false→trueinMcpToolsSection.csLine 78 to match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs` around lines 1050 - 1066, StdioBridgeHost.WriteHeartbeat is reading EditorPrefKeys.ProjectScopedToolsLocalHttp with default true while McpToolsSection.RegisterCallbacks uses default false, causing inconsistent behavior; fix by centralizing the preference read (e.g., add a helper like GetProjectScopedToolsDefault or a static method on EditorPrefKeys) and use that helper from both StdioBridgeHost.WriteHeartbeat and McpToolsSection.RegisterCallbacks, or change the literal default in StdioBridgeHost.WriteHeartbeat from true to false so both use the same default; reference EditorPrefKeys.ProjectScopedToolsLocalHttp, StdioBridgeHost.WriteHeartbeat, and McpToolsSection.RegisterCallbacks when making the change.
🧹 Nitpick comments (2)
MCPForUnity/Editor/Services/ToolDiscoveryService.cs (1)
26-49: 💤 Low valueTwo-phase scan with dedupe looks correct.
TypeCacheresults are unioned with an AppDomain reflection pass and deduped byTypeidentity, so post-domain-reload misses on project assemblies are now covered without double-registering. The per-assemblytry/catcharoundGetTypes()is the right pattern for handlingReflectionTypeLoadExceptionfrom third-party assemblies.Minor:
new Type[0]can beArray.Empty<Type>()to avoid an allocation per failing assembly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MCPForUnity/Editor/Services/ToolDiscoveryService.cs` around lines 26 - 49, Replace the allocation of an empty array inside the AppDomain reflection fallback with a shared empty array to avoid per-assembly allocations: in the lambda used after AppDomain.CurrentDomain.GetAssemblies() where you catch exceptions from a.GetTypes() (in the code that calls TypeCache.GetTypesWithAttribute<McpForUnityToolAttribute>() and then SelectMany(...)), return Array.Empty<Type>() instead of new Type[0] in the catch block.Server/src/main.py (1)
893-912: 💤 Low valueAuto-detection works, but consider two follow-ups.
Duplicate discovery work at startup.
pool.discover_all_instances()here (Line 900) is called again insideserver_lifespan(Line 200). Each call walks status files and TCP-probes every discovered port (_try_probe_unity_mcpwith 0.3s timeout each). For a typical single-instance setup this is fine, but you could cache the result on the pool or pass it through the lifespan via the yielded dict to avoid the second probe pass.One-shot decision.
project_scoped_toolsis fixed for the life of the server based on what Unity reported at startup. If the user toggles the EditorPref afterwards, the server will not pick it up until restart. If that's by design, a brief log line documenting it (or a docstring note) would help future debugging.Static-analysis BLE001 on Line 910 is acceptable here — the broad
exceptis paired withlogger.debug(..., exc_info=True)and a safe fallback to the explicit value, so no action needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Server/src/main.py` around lines 893 - 912, The startup code calls get_unity_connection_pool().discover_all_instances() to set project_scoped_tools, which causes duplicate expensive probes because server_lifespan also calls pool.discover_all_instances(); to fix, either cache the discovery result on the pool object (e.g., set pool._cached_discovered_instances after the first call and use it in subsequent discover_all_instances calls) or pass the discovered instances through the lifespan handshake (include the instances list in the dict yielded by server_lifespan) so the second probe is avoided; also add a concise log or docstring near project_scoped_tools / the startup block explaining this is a one-shot decision (toggled Unity EditorPref after startup won’t change the server’s behavior) so future debuggers aren’t surprised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs`:
- Around line 1050-1066: StdioBridgeHost.WriteHeartbeat is reading
EditorPrefKeys.ProjectScopedToolsLocalHttp with default true while
McpToolsSection.RegisterCallbacks uses default false, causing inconsistent
behavior; fix by centralizing the preference read (e.g., add a helper like
GetProjectScopedToolsDefault or a static method on EditorPrefKeys) and use that
helper from both StdioBridgeHost.WriteHeartbeat and
McpToolsSection.RegisterCallbacks, or change the literal default in
StdioBridgeHost.WriteHeartbeat from true to false so both use the same default;
reference EditorPrefKeys.ProjectScopedToolsLocalHttp,
StdioBridgeHost.WriteHeartbeat, and McpToolsSection.RegisterCallbacks when
making the change.
---
Nitpick comments:
In `@MCPForUnity/Editor/Services/ToolDiscoveryService.cs`:
- Around line 26-49: Replace the allocation of an empty array inside the
AppDomain reflection fallback with a shared empty array to avoid per-assembly
allocations: in the lambda used after AppDomain.CurrentDomain.GetAssemblies()
where you catch exceptions from a.GetTypes() (in the code that calls
TypeCache.GetTypesWithAttribute<McpForUnityToolAttribute>() and then
SelectMany(...)), return Array.Empty<Type>() instead of new Type[0] in the catch
block.
In `@Server/src/main.py`:
- Around line 893-912: The startup code calls
get_unity_connection_pool().discover_all_instances() to set
project_scoped_tools, which causes duplicate expensive probes because
server_lifespan also calls pool.discover_all_instances(); to fix, either cache
the discovery result on the pool object (e.g., set
pool._cached_discovered_instances after the first call and use it in subsequent
discover_all_instances calls) or pass the discovered instances through the
lifespan handshake (include the instances list in the dict yielded by
server_lifespan) so the second probe is avoided; also add a concise log or
docstring near project_scoped_tools / the startup block explaining this is a
one-shot decision (toggled Unity EditorPref after startup won’t change the
server’s behavior) so future debuggers aren’t surprised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29975faa-4cf0-437c-beba-ccfe05006e2d
📒 Files selected for processing (8)
MCPForUnity/Editor/Services/ToolDiscoveryService.csMCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csServer/src/main.pyServer/src/models/models.pyServer/src/services/custom_tool_service.pyServer/src/transport/legacy/port_discovery.pydocs/migrations/v8_NEW_NETWORKING_SETUP.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
1050-1053: 💤 Low valueConsider centralizing the default value for project-scoped tools.
The default value
falseis documented to match the UI toggle default. To prevent drift and ensure consistency, consider extracting this default to a shared constant inEditorPrefKeysor a dedicated defaults class.Example: Centralized default
In
EditorPrefKeys.cs:public const bool ProjectScopedToolsDefault = false;Then use it here:
bool projectScopedTools = EditorPrefs.GetBool( EditorPrefKeys.ProjectScopedToolsLocalHttp, - false // must match McpToolsSection toggle default so UI and heartbeat agree + EditorPrefKeys.ProjectScopedToolsDefault );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs` around lines 1050 - 1053, Extract the literal default into a shared constant (e.g., add public const bool ProjectScopedToolsDefault = false to EditorPrefKeys) and replace the inline false in the EditorPrefs.GetBool call in Transports/StdioBridgeHost.cs so the call becomes EditorPrefs.GetBool(EditorPrefKeys.ProjectScopedToolsLocalHttp, EditorPrefKeys.ProjectScopedToolsDefault); this centralizes the default and keeps the UI/heartbeat default in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs`:
- Around line 1050-1053: Extract the literal default into a shared constant
(e.g., add public const bool ProjectScopedToolsDefault = false to
EditorPrefKeys) and replace the inline false in the EditorPrefs.GetBool call in
Transports/StdioBridgeHost.cs so the call becomes
EditorPrefs.GetBool(EditorPrefKeys.ProjectScopedToolsLocalHttp,
EditorPrefKeys.ProjectScopedToolsDefault); this centralizes the default and
keeps the UI/heartbeat default in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f13fa5c5-a08e-4b21-9a42-a0a39798d9ce
📒 Files selected for processing (1)
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
2 participants
Description
This PR enables project-scoped custom tools ([McpForUnityTool]) to work when using the stdio transport, not just HTTP Local. Previously, stdio mode had no way to discover or register project-specific custom tools.
Type of Change
Bug fix (non-breaking change that fixes an issue)
Changes Made
ToolDiscoveryService.cs - Added AppDomain fallback scan in ToolDiscoveryService when TypeCache misses project assemblies after domain reloads
StdioBridgeHost.cs - Heartbeat JSON now includes project_scoped_tools flag so the server knows when a Unity instance wants project-scoped tooling
models.py / port_discovery.py / main.py — UnityInstanceInfo gains project_scoped_tools; PortDiscovery parses it from status JSON; server auto-enables project-scoped tools on startup when a discovered instance requests it
custom_tool_service.py - CustomToolService now registers global custom tools even when project-scoped tools are enabled
v8_NEW_NETWORKING_SETUP.md - Updated migration notes to reflect that custom tools now work in both HTTP and stdio transports
Testing/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.md(none of above apply)
Related Issues
Additional Notes
If you would like to see any changes to this PR, please let me know and I will get to it! :) My personal motivation behind this PR is, Kimi Code plays poorly with HTTP so I'm forced to use stdio with it, and having support for custom MCP's would be nice to have. Changes made by Kimi-k2.6 and human reviewed (and tested in actual project) by me to make sure they make sense and don't introduce possible vulnerabilities or other issues.
Summary by CodeRabbit
New Features
Improvements
Documentation