-
Notifications
You must be signed in to change notification settings - Fork 900
Replace session-global Unity instance selection with explicit routing #981
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
base: beta
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import functools | ||
| import inspect | ||
| from typing import Annotated, Any, Callable | ||
|
|
||
| UnityInstanceParameter = Annotated[ | ||
| str | None, | ||
| "Target Unity instance (Name@hash, hash prefix, or port number in stdio mode).", | ||
| ] | ||
|
|
||
|
|
||
| def add_optional_unity_instance_parameter( | ||
| func: Callable[..., Any], | ||
| ) -> Callable[..., Any]: | ||
| """Expose an optional unity_instance kwarg without forwarding it downstream.""" | ||
| signature = inspect.signature(func) | ||
| if "unity_instance" in signature.parameters: | ||
| return func | ||
|
|
||
| parameters = list(signature.parameters.values()) | ||
| unity_parameter = inspect.Parameter( | ||
| "unity_instance", | ||
| inspect.Parameter.KEYWORD_ONLY, | ||
| default=None, | ||
| annotation=UnityInstanceParameter, | ||
| ) | ||
|
|
||
| insert_at = len(parameters) | ||
| for index, parameter in enumerate(parameters): | ||
| if parameter.kind == inspect.Parameter.VAR_KEYWORD: | ||
| insert_at = index | ||
| break | ||
| parameters.insert(insert_at, unity_parameter) | ||
|
|
||
| wrapped_signature = signature.replace(parameters=parameters) | ||
|
|
||
| if inspect.iscoroutinefunction(func): | ||
| @functools.wraps(func) | ||
| async def wrapper(*args, **kwargs): | ||
| kwargs.pop("unity_instance", None) | ||
| return await func(*args, **kwargs) | ||
| else: | ||
| @functools.wraps(func) | ||
| def wrapper(*args, **kwargs): | ||
| kwargs.pop("unity_instance", None) | ||
| return func(*args, **kwargs) | ||
|
|
||
| wrapper.__signature__ = wrapped_signature | ||
| wrapper.__annotations__ = { | ||
| **getattr(func, "__annotations__", {}), | ||
| "unity_instance": UnityInstanceParameter, | ||
| } | ||
| return wrapper | ||
|
|
||
|
|
||
| def append_unity_instance_query_template(uri: str) -> str: | ||
| """Append unity_instance to a resource URI template if it is not present.""" | ||
| if "unity_instance" in uri: | ||
| return uri | ||
|
|
||
| if "{?" not in uri: | ||
| return f"{uri}{{?unity_instance}}" | ||
|
|
||
| prefix, _, suffix = uri.partition("{?") | ||
| query_suffix = suffix[:-1] if suffix.endswith("}") else suffix | ||
| query_names = [name for name in query_suffix.split(",") if name] | ||
| if "unity_instance" not in query_names: | ||
| query_names.append("unity_instance") | ||
| return f"{prefix}{{?{','.join(query_names)}}}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,17 @@ async def set_active_instance( | |
| ctx: Context, | ||
| instance: Annotated[str, "Target instance (Name@hash, hash prefix, or port number in stdio mode)"] | ||
| ) -> dict[str, Any]: | ||
| get_state = getattr(ctx, "get_state", None) | ||
| bound_instance = await get_state("bound_unity_instance") if callable(get_state) else None | ||
|
Comment on lines
+26
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n Server/src/services/tools/set_active_instance.py | head -40Repository: CoplayDev/unity-mcp Length of output: 1833 🏁 Script executed: rg "get_state" Server/src/services/tools/ -B 2 -A 2 -t pyRepository: CoplayDev/unity-mcp Length of output: 1749 🏁 Script executed: rg "Context.get_state\|ctx.get_state" --type py -B 2 -A 2 | head -100Repository: CoplayDev/unity-mcp Length of output: 45 🏁 Script executed: rg "@mcp_for_unity_tool" Server/src/services/tools/ -A 5 --type py | grep -E "(`@mcp_for_unity_tool`|group=)" | head -50Repository: CoplayDev/unity-mcp Length of output: 3309 🏁 Script executed: rg "group\s*=" Server/src/services/tools/ --type py -B 1Repository: CoplayDev/unity-mcp Length of output: 3355 🏁 Script executed: cat -n Server/src/services/tools/__init__.py | sed -n '1,40p'Repository: CoplayDev/unity-mcp Length of output: 1788 Add defensive handling for Line 27 calls Suggested fix get_state = getattr(ctx, "get_state", None)
- bound_instance = await get_state("bound_unity_instance") if callable(get_state) else None
+ try:
+ bound_instance = await get_state("bound_unity_instance") if callable(get_state) else None
+ except Exception:
+ bound_instance = None🤖 Prompt for AI Agents |
||
| if bound_instance: | ||
| return { | ||
| "success": False, | ||
| "error": ( | ||
| "set_active_instance is not available on an instance-bound MCP endpoint. " | ||
| "Use the bound endpoint target or switch back to the unbound /mcp endpoint." | ||
| ), | ||
| } | ||
|
|
||
| transport = (config.transport_mode or "stdio").lower() | ||
|
|
||
| # Port number shorthand (stdio only) — resolve to Name@hash via pool discovery | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -549,16 +549,27 @@ def _resolve_instance_id(self, instance_identifier: str | None, instances: list[ | |
| instance_identifier = self._default_instance_id | ||
| logger.debug(f"Using default instance: {instance_identifier}") | ||
| else: | ||
| # Use the most recently active instance | ||
| # Instances with no heartbeat (None) should be sorted last (use 0 as sentinel) | ||
| sorted_instances = sorted( | ||
| instances, | ||
| key=lambda inst: inst.last_heartbeat.timestamp() if inst.last_heartbeat else 0.0, | ||
| reverse=True, | ||
| if len(instances) == 1: | ||
| logger.info( | ||
| "No instance specified, auto-selecting sole instance: %s", | ||
| instances[0].id, | ||
| ) | ||
| return instances[0] | ||
|
Comment on lines
+552
to
+557
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refresh discovery before auto-selecting the “sole” instance. These 🔁 Proposed fix else:
+ instances = self.discover_all_instances(force_refresh=True)
if len(instances) == 1:
logger.info(
"No instance specified, auto-selecting sole instance: %s",
instances[0].id,
)🤖 Prompt for AI Agents |
||
|
|
||
| suggestions = [ | ||
| { | ||
| "id": inst.id, | ||
| "path": inst.path, | ||
| "port": inst.port, | ||
| "suggest": f"Use unity_instance='{inst.id}'", | ||
| } | ||
| for inst in instances | ||
| ] | ||
| raise ConnectionError( | ||
| "Multiple Unity Editor instances are running. " | ||
| "Pass unity_instance explicitly or call set_active_instance first. " | ||
| f"Available instances: {suggestions}" | ||
| ) | ||
| logger.info( | ||
| f"No instance specified, using most recent: {sorted_instances[0].id}") | ||
| return sorted_instances[0] | ||
|
|
||
| identifier = instance_identifier.strip() | ||
|
|
||
|
|
||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 107
🏁 Script executed:
# Check the file size and structure wc -l Server/src/services/custom_tool_service.pyRepository: CoplayDev/unity-mcp
Length of output: 109
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 1473
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 2422
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 1674
Error message misleadingly suggests passing
unity_instancefor global custom tools.At lines 375-377, the handler tells users to "Pass unity_instance explicitly," but global custom tools registered via
self._mcp.tool(...)have signatures built only fromdefinition.parameters(via_build_signature()and_build_annotations()). The handler attempts to retrieveunity_instancefrom context, not from kwargs, so callers cannot actually provide it as a parameter. Either exposeunity_instancein the signature and filter it from forwarded params (enabling multi-editor routing), or revise the message to only mention theset_active_instanceworkaround.🤖 Prompt for AI Agents