Add CLI #606
Merged
Add CLI #606
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
- Add click-based CLI with 15+ command groups - Commands: gameobject, component, scene, asset, script, editor, prefab, material, lighting, ui, audio, animation, code - HTTP transport to communicate with Unity via MCP server - Output formats: text, json, table - Configuration via environment variables or CLI options - Comprehensive usage guide and unit tests
Small update for CoplayDev#542
* Log a message with implicit URI changes Small update for CoplayDev#542 * Log a message with implicit URI changes Small update for CoplayDev#542 * Add helper scripts to update forks * fix: improve HTTP Local URL validation UX and styling specificity - Rename CSS class from generic "error" to "http-local-url-error" for better specificity - Rename "invalid-url" class to "http-local-invalid-url" for clarity - Disable httpServerCommandField when URL is invalid or transport not HTTP Local - Clear field value and tooltip when showing validation errors - Ensure field is re-enabled when URL becomes valid
* Log a message with implicit URI changes Small update for CoplayDev#542 * Update docker container to default to stdio Replaces CoplayDev#541
- Fix RiderConfigurator to use correct GitHub Copilot config path: - Windows: %LOCALAPPDATA%\github-copilot\intellij\mcp.json - macOS: ~/Library/Application Support/github-copilot/intellij/mcp.json - Linux: ~/.config/github-copilot/intellij/mcp.json - Add mcp.json for GitHub MCP Registry support: - Enables users to install via coplaydev/unity-mcp - Uses uvx with mcpforunityserver from PyPI
…l name that already exists as a built‑in.
Contributor
Reviewer's GuideAdds a fully featured Sequence diagram for executing a unity-mcp CLI command via HTTP MCP serversequenceDiagram
actor User
participant Shell as Shell
participant CLI as unity_mcp_cli
participant Conn as cli_utils_connection
participant HTTP as FastAPI_http_server
participant Hub as PluginHub
participant Unity as Unity_instance
User->>Shell: type "unity-mcp gameobject create \n\"MyCube\" --primitive Cube"
Shell->>CLI: invoke cli.main.main()
CLI->>CLI: parse global options\n(host, port, format, instance)
CLI->>CLI: build params for manage_gameobject
CLI->>Conn: run_command("manage_gameobject", params, config)
Conn->>HTTP: POST /api/command\n{ type: manage_gameobject,\n params: {...},\n unity_instance? }
HTTP->>Hub: PluginHub.get_sessions()
Hub-->>HTTP: sessions (session_id -> details)
HTTP->>HTTP: select session_id\n(matching unity_instance or first)
HTTP->>Hub: send_command(session_id,\n command_type, params)
Hub->>Unity: execute MCP tool\n(manage_gameobject)
Unity-->>Hub: tool result JSON
Hub-->>HTTP: MCPResponse / dict
HTTP-->>Conn: 200 OK + JSON body
Conn-->>CLI: parsed result dict
CLI->>CLI: format_output(result, config.format)
CLI-->>Shell: formatted output
Shell-->>User: display result
Class diagram for CLI core configuration, connection, and output utilitiesclassDiagram
class CLIConfig {
+str host
+int port
+int timeout
+str format
+str unity_instance
+from_env() CLIConfig
}
class Context {
+CLIConfig config
+bool verbose
+__init__()
}
class ConfigModule {
-CLIConfig _config
+get_config() CLIConfig
+set_config(config CLIConfig) void
}
class ConnectionModule {
+UnityConnectionError
+warn_if_remote_host(config CLIConfig) void
+send_command(command_type str, params dict, config CLIConfig, timeout int) dict
+run_command(command_type str, params dict, config CLIConfig, timeout int) dict
+check_connection(config CLIConfig) bool
+run_check_connection(config CLIConfig) bool
+list_unity_instances(config CLIConfig) dict
+run_list_instances(config CLIConfig) dict
}
class OutputModule {
+format_output(data any, format_type str) str
+format_as_json(data any) str
+format_as_text(data any, indent int) str
+format_as_table(data any) str
+print_success(message str) void
+print_error(message str) void
+print_warning(message str) void
+print_info(message str) void
}
class MainCLI {
+cli(host str, port int, timeout int, format str, instance str, verbose bool) void
+status() void
+instances() void
+raw_command(command_type str, params str) void
+register_commands() void
+main() void
}
class GameobjectCommands {
+gameobject()
+find(search_term str, method str, include_inactive bool, limit int, cursor int) void
+create(name str, primitive str, position tuple, rotation tuple, scale tuple, parent str, tag str, layer str, components str, save_prefab bool, prefab_path str) void
+modify(target str, name str, position tuple, rotation tuple, scale tuple, parent str, tag str, layer str, active bool, add_components str, remove_components str, search_method str) void
+delete(target str, search_method str, force bool) void
+duplicate(target str, name str, offset tuple, search_method str) void
+move(target str, reference str, direction str, distance float, local bool, search_method str) void
}
class SceneCommands {
+scene()
+hierarchy(parent str, max_depth int, include_transform bool, limit int, cursor int) void
+active() void
+load(scene str, by_index bool) void
+save(path str) void
+create(name str, path str) void
+build_settings() void
+screenshot(filename str, supersize int) void
}
class EditorCommands {
+editor()
+play() void
+pause() void
+stop() void
+console(log_types tuple, count int, filter_text str, stacktrace bool, clear bool) void
+add_tag(tag_name str) void
+remove_tag(tag_name str) void
+add_layer(layer_name str) void
+remove_layer(layer_name str) void
+set_tool(tool_name str) void
+execute_menu(menu_path str) void
+run_tests(mode str, async_mode bool, wait int, details bool, failed_only bool) void
+poll_test(job_id str, wait int, details bool, failed_only bool) void
+refresh(mode str, scope str, compile bool, no_wait bool) void
+custom_tool(tool_name str, params str) void
}
class AssetCommands {
+asset()
+search(pattern str, path str, filter_type str, limit int, page int) void
+info(path str, preview bool) void
+create(path str, asset_type str, properties str) void
+delete(path str, force bool) void
+duplicate(source str, destination str) void
+move(source str, destination str) void
+rename(path str, new_name str) void
+import_asset(path str) void
+mkdir(path str) void
}
class MaterialCommands {
+material()
+info(path str) void
+create(path str, shader str, properties str) void
+set_color(path str, r float, g float, b float, a float, property str) void
+set_property(path str, property_name str, value str) void
+assign(material_path str, target str, search_method str, slot int, mode str) void
+set_renderer_color(target str, r float, g float, b float, a float, search_method str, mode str) void
}
CLIConfig <.. ConfigModule : used by
CLIConfig <.. ConnectionModule : used by
CLIConfig <.. OutputModule : used by
Context o-- CLIConfig
MainCLI ..> CLIConfig : config
MainCLI ..> Context : uses
MainCLI ..> ConnectionModule : uses
MainCLI ..> OutputModule : uses
GameobjectCommands ..> ConnectionModule : run_command
GameobjectCommands ..> OutputModule : format_output, print_*
SceneCommands ..> ConnectionModule
SceneCommands ..> OutputModule
EditorCommands ..> ConnectionModule
EditorCommands ..> OutputModule
AssetCommands ..> ConnectionModule
AssetCommands ..> OutputModule
MaterialCommands ..> ConnectionModule
MaterialCommands ..> OutputModule
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 Python Click-based CLI for Unity MCP, including CLI modules, config/connection/output utilities, server HTTP endpoints for command execution, docs, tests, and small Unity editor configurator/UI tweaks. Changes
Sequence Diagram(s)sequenceDiagram
actor User as CLI User
participant CLI as unity-mcp (Click)
participant Server as MCP HTTP Server
participant Router as Command Router
participant Tool as MCP Tool Handler
participant Unity as Unity Editor
User->>CLI: unity-mcp <command> ...
CLI->>CLI: parse args / get_config
CLI->>Server: POST /api/command (command_type, params)
Server->>Router: dispatch command
Router->>Tool: call manage_* handler
Tool->>Unity: send MCP request
Unity-->>Tool: result
Tool-->>Router: result
Router-->>Server: response
Server-->>CLI: JSON result
CLI->>CLI: format_output
CLI->>User: display
sequenceDiagram
participant Env as Environment
participant CFG as CLIConfig.from_env
participant Cache as get_config()
participant CLI as any command
Env->>CFG: UNITY_MCP_HOST/PORT/TIMEOUT/FORMAT/INSTANCE
CFG->>CFG: validate port/timeout
CFG->>Cache: create CLIConfig instance
CLI->>Cache: get_config() (reuse instance)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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
|
Based off #544 |
![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 5 issues, and left some high level feedback:
- In
Server/src/main.py's/api/commandroute, when aunity_instanceis provided but doesn't match any session you silently fall back to the first available session; consider returning a 400/404-style error instead to avoid accidentally targeting the wrong Unity instance. - In
cli/utils/config.py,CLIConfig.from_envraisesValueErroron malformedUNITY_MCP_HTTP_PORT/UNITY_MCP_TIMEOUT, which will crash the CLI; it might be safer to log a warning and fall back to defaults so a bad env var doesn't make the tool unusable. - In
cli/main.py'sregister_commands, swallowingImportErrorwithout logging can obscure real issues (e.g., typos or packaging problems); consider at least logging which command module failed to import so debugging broken installations is easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Server/src/main.py`'s `/api/command` route, when a `unity_instance` is provided but doesn't match any session you silently fall back to the first available session; consider returning a 400/404-style error instead to avoid accidentally targeting the wrong Unity instance.
- In `cli/utils/config.py`, `CLIConfig.from_env` raises `ValueError` on malformed `UNITY_MCP_HTTP_PORT`/`UNITY_MCP_TIMEOUT`, which will crash the CLI; it might be safer to log a warning and fall back to defaults so a bad env var doesn't make the tool unusable.
- In `cli/main.py`'s `register_commands`, swallowing `ImportError` without logging can obscure real issues (e.g., typos or packaging problems); consider at least logging which command module failed to import so debugging broken installations is easier.
## Individual Comments
### Comment 1
<location> `Server/src/main.py:349-360` </location>
<code_context>
+ session_id = sid
+ break
+
+ if not session_id:
+ # Use first available session
+ session_id = next(iter(sessions.sessions.keys()))
+
+ # Send command to Unity
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid silently falling back to the first Unity session when a specific `unity_instance` was requested but not found.
If a client passes a `unity_instance` that doesn’t match any session, this code will still route the command to the first available session, which can send commands to the wrong project in multi‑instance environments. Instead, when a `unity_instance` is provided but no matching `session_id` is found, return an error (e.g. 400/404 with "Unity instance '<id>' not found") and only default to the first session when no `unity_instance` was specified.
```suggestion
# Find target session
session_id = None
if unity_instance:
# Try to match by hash or project name
for sid, details in sessions.sessions.items():
if details.hash == unity_instance or details.project == unity_instance:
session_id = sid
break
# If a specific unity_instance was requested but not found, return an error
if not session_id:
return JSONResponse(
{
"success": False,
"error": f"Unity instance '{unity_instance}' not found",
},
status_code=404,
)
else:
# No specific unity_instance requested: use first available session
session_id = next(iter(sessions.sessions.keys()))
```
</issue_to_address>
### Comment 2
<location> `Server/src/cli/main.py:185-194` </location>
<code_context>
+def register_commands():
</code_context>
<issue_to_address>
**issue (bug_risk):** Swallowing all `ImportError`s when registering commands can hide real bugs in command modules.
This pattern also hides import-time errors within the command modules themselves (e.g., syntax errors or missing dependencies), making broken commands hard to detect. To support optional modules without masking real failures, catch `ModuleNotFoundError` instead and verify `e.name` matches the optional module you expect, or at least log the exception so genuine errors in `cli.commands.*` are visible.
</issue_to_address>
### Comment 3
<location> `Server/src/cli/commands/scene.py:19-28` </location>
<code_context>
[email protected]("play")
[email protected]("target")
[email protected]("state_name")
[email protected](
+ "--layer", "-l",
+ default=0,
</code_context>
<issue_to_address>
**issue (bug_risk):** Validate `supersize` argument range for `scene screenshot` to match the documented limits.
The help text says `supersize` should be `1-4`, but the code only checks `supersize > 1` and passes it straight through. Values like `0` or very large numbers would violate the documented range and could behave unpredictably in Unity. Please enforce `1 <= supersize <= 4` and either clamp or fail with a clear error when the value is out of range.
</issue_to_address>
### Comment 4
<location> `Server/src/cli/utils/config.py:19-28` </location>
<code_context>
+ unity_instance: Optional[str] = None
+
+ @classmethod
+ def from_env(cls) -> "CLIConfig":
+ port_raw = os.environ.get("UNITY_MCP_HTTP_PORT", "8080")
+ try:
+ port = int(port_raw)
+ except (ValueError, TypeError):
+ raise ValueError(
+ f"Invalid UNITY_MCP_HTTP_PORT value: {port_raw!r}")
+
+ timeout_raw = os.environ.get("UNITY_MCP_TIMEOUT", "30")
+ try:
+ timeout = int(timeout_raw)
+ except (ValueError, TypeError):
+ raise ValueError(
+ f"Invalid UNITY_MCP_TIMEOUT value: {timeout_raw!r}")
+
+ return cls(
</code_context>
<issue_to_address>
**suggestion:** Failing hard on malformed env vars may make the CLI brittle; consider falling back to defaults with a warning.
`UNITY_MCP_HTTP_PORT` and `UNITY_MCP_TIMEOUT` parsing currently raises `ValueError` in `from_env`, aborting all CLI commands when misconfigured. For a CLI, it’s usually preferable to emit a warning (e.g., via logging or `click.echo` to stderr) and continue with the default values so the tool remains usable despite imperfect env configuration.
Suggested implementation:
```python
import logging
class CLIConfig:
```
```python
@classmethod
def from_env(cls) -> "CLIConfig":
port_raw = os.environ.get("UNITY_MCP_HTTP_PORT", "8080")
try:
port = int(port_raw)
except (ValueError, TypeError):
logging.warning(
"Invalid UNITY_MCP_HTTP_PORT value %r; falling back to default 8080",
port_raw,
)
port = 8080
timeout_raw = os.environ.get("UNITY_MCP_TIMEOUT", "30")
try:
timeout = int(timeout_raw)
except (ValueError, TypeError):
logging.warning(
"Invalid UNITY_MCP_TIMEOUT value %r; falling back to default 30",
timeout_raw,
)
timeout = 30
return cls(
host=os.environ.get("UNITY_MCP_HOST", "127.0.0.1"),
port=port,
timeout=timeout,
format=os.environ.get("UNITY_MCP_FORMAT", "text"),
unity_instance=os.environ.get("UNITY_MCP_INSTANCE"),
)
```
If this module already defines imports in a grouped block (e.g., `import os`, `from typing import Optional`), you may want to move `import logging` into that existing import section rather than directly above `class CLIConfig:` to match your style. The logging configuration (handlers/format) should be set up once in your CLI entry point so that these warnings are visible to users; if you don't configure logging anywhere yet, add a basic configuration there (e.g., `logging.basicConfig(level=logging.WARNING)`).
</issue_to_address>
### Comment 5
<location> `docs/CLI_USAGE.md:3-5` </location>
<code_context>
+# Unity MCP CLI Usage Guide
+
+The Unity MCP CLI provides command-line access to control Unity Editor through the Model Context Protocol. Currently only supports local HTTP.
+
+Note: Some tools are still experimental and might fail under circumstances. Please submit an issue for us to make it better.
+
+## Installation
</code_context>
<issue_to_address>
**suggestion (typo):** Minor grammar improvements in the introductory sentences.
Suggested tweaks for clarity:
- "The Unity MCP CLI provides command-line access to control **the** Unity Editor through the Model Context Protocol. **It** currently only supports local HTTP."
- "Note: Some tools are still experimental and might fail **under some circumstances**. Please submit an issue to help us make it better."
```suggestion
The Unity MCP CLI provides command-line access to control the Unity Editor through the Model Context Protocol. It currently only supports local HTTP.
Note: Some tools are still experimental and might fail under some circumstances. Please submit an issue to help us make it better.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+19
to
+28
| @click.option( | ||
| "--parent", | ||
| default=None, | ||
| help="Parent GameObject to list children of (name, path, or instance ID)." | ||
| ) | ||
| @click.option( | ||
| "--max-depth", "-d", | ||
| default=None, | ||
| type=int, | ||
| help="Maximum depth to traverse." |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Validate supersize argument range for scene screenshot to match the documented limits.
The help text says supersize should be 1-4, but the code only checks supersize > 1 and passes it straight through. Values like 0 or very large numbers would violate the documented range and could behave unpredictably in Unity. Please enforce 1 <= supersize <= 4 and either clamp or fail with a clear error when the value is out of range.
Comment on lines
+19
to
+28
| def from_env(cls) -> "CLIConfig": | ||
| port_raw = os.environ.get("UNITY_MCP_HTTP_PORT", "8080") | ||
| try: | ||
| port = int(port_raw) | ||
| except (ValueError, TypeError): | ||
| raise ValueError( | ||
| f"Invalid UNITY_MCP_HTTP_PORT value: {port_raw!r}") | ||
|
|
||
| timeout_raw = os.environ.get("UNITY_MCP_TIMEOUT", "30") | ||
| try: |
Contributor
There was a problem hiding this comment.
suggestion: Failing hard on malformed env vars may make the CLI brittle; consider falling back to defaults with a warning.
UNITY_MCP_HTTP_PORT and UNITY_MCP_TIMEOUT parsing currently raises ValueError in from_env, aborting all CLI commands when misconfigured. For a CLI, it’s usually preferable to emit a warning (e.g., via logging or click.echo to stderr) and continue with the default values so the tool remains usable despite imperfect env configuration.
Suggested implementation:
import logging
class CLIConfig: @classmethod
def from_env(cls) -> "CLIConfig":
port_raw = os.environ.get("UNITY_MCP_HTTP_PORT", "8080")
try:
port = int(port_raw)
except (ValueError, TypeError):
logging.warning(
"Invalid UNITY_MCP_HTTP_PORT value %r; falling back to default 8080",
port_raw,
)
port = 8080
timeout_raw = os.environ.get("UNITY_MCP_TIMEOUT", "30")
try:
timeout = int(timeout_raw)
except (ValueError, TypeError):
logging.warning(
"Invalid UNITY_MCP_TIMEOUT value %r; falling back to default 30",
timeout_raw,
)
timeout = 30
return cls(
host=os.environ.get("UNITY_MCP_HOST", "127.0.0.1"),
port=port,
timeout=timeout,
format=os.environ.get("UNITY_MCP_FORMAT", "text"),
unity_instance=os.environ.get("UNITY_MCP_INSTANCE"),
)If this module already defines imports in a grouped block (e.g., import os, from typing import Optional), you may want to move import logging into that existing import section rather than directly above class CLIConfig: to match your style. The logging configuration (handlers/format) should be set up once in your CLI entry point so that these warnings are visible to users; if you don't configure logging anywhere yet, add a basic configuration there (e.g., logging.basicConfig(level=logging.WARNING)).
…c unity_instance was requested but not found. If a client passes a unity_instance that doesn’t match any session, this code will still route the command to the first available session, which can send commands to the wrong project in multi‑instance environments. Instead, when a unity_instance is provided but no matching session_id is found, return an error (e.g. 400/404 with "Unity instance '' not found") and only default to the first session when no unity_instance was specified. Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
![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: 13
🤖 Fix all issues with AI agents
In `@Server/src/cli/CLI_USAGE_GUIDE.md`:
- Around line 345-347: The example in the usage guide uses the wrong command
name; update the snippet to use the CLI group and subcommand defined in
Server/src/cli/commands/instance.py by changing "unity-mcp instances" to the
correct "unity-mcp instance list" so it matches the instance command group and
its list subcommand.
- Around line 83-89: Add a language identifier to the fenced code blocks in the
CLI usage examples so markdownlint MD040 is satisfied: change the
triple-backtick fences that contain the CLI output ("Checking connection to
127.0.0.1:8080... ✅ Connected to Unity MCP server..." block) and the usage
synopsis ("unity-mcp [GLOBAL_OPTIONS] COMMAND_GROUP [SUBCOMMAND]...") to use
```text instead of plain ``` so they are treated as non-code text blocks.
In `@Server/src/cli/commands/audio.py`:
- Around line 31-60: The play/stop CLI commands call manage_components with
action "set_property" for property "Play"/"Stop", but AudioSource.Play()/Stop()
are methods and ComponentOps.SetProperty cannot invoke methods—this will fail;
update the fix by either extending the backend manage_components to support a
new action (e.g., "invoke_method") that takes "methodName" and optional args and
calls ComponentOps.InvokeMethod (or equivalent), or special-case audio in the
CLI to call a new action like "invoke_method" with methodName "Play"/"Stop";
change the play() and stop() functions to send that action and parameters
instead of "set_property", and update any backend handler names referenced
(ComponentOps.SetProperty -> implement ComponentOps.InvokeMethod or
method-invocation path) so method invocation is performed rather than property
assignment.
In `@Server/src/cli/commands/batch.py`:
- Around line 179-184: The file-writing block that uses output, json_output, and
print_success lacks error handling; wrap the open/write in a try/except that
catches OSError/IOError (or generic Exception), and on failure log/echo a clear
error using the existing CLI utilities (e.g., print_error or click.echo)
including the exception message and the target path, then exit non‑zero (e.g.,
raise click.Abort or sys.exit(1)); also ensure the file is opened with an
explicit encoding (utf-8) to avoid encoding issues.
In `@Server/src/cli/commands/code.py`:
- Around line 59-69: The read path after calling run_command("manage_script",
params, config) should normalize nested/encoded responses like the search
implementation: first unwrap a top-level "result" key if present (i.e., use
result = result.get("result", result)), then extract the "data" payload, check
for "contents" inside that data (or inside data.get("data") if nested), and if
the contents are base64-encoded decode them before calling click.echo; otherwise
fall back to format_output(result, config.format). Update the logic around
result/data handling in the read flow (the block using run_command, result,
data, format_output) to perform this normalization and decoding so file contents
are printed correctly when nested or encoded.
In `@Server/src/cli/commands/editor.py`:
- Around line 272-335: The CLI never adds the async intent to the request
payload; update the run_tests function to set the expected async flag on the
params dict before calling run_command (e.g., if async_mode: params["async"] =
True—or whichever key the Unity backend expects, like "run_async"), so the
backend receives explicit async intent; modify the params population in
run_tests (where params: dict[str, Any] is built) to include this key when
async_mode is True.
In `@Server/src/cli/commands/material.py`:
- Around line 171-177: The CLI option "--search-method" advertises "by_id" in
examples but the click.Choice for that option omits "by_id", causing the example
to fail; update the click.option declarations that define "--search-method" (the
click.Choice(...) entries in material.py) to include "by_id" alongside
"by_name", "by_path", "by_tag", "by_layer", and "by_component" so the validator
accepts the documented value (ensure you update both occurrences referenced in
the diff).
In `@Server/src/cli/commands/ui.py`:
- Around line 37-71: The code currently ignores failures from the component
addition and render-mode steps, so update the block using
run_command("manage_components", ...) and the render-mode run_command to check
each returned result (e.g., comp_result or render_result) and fail-fast: if
result.get("success") is falsy, call print_error with a clear message including
the component name or property and result.get("error", "Unknown error"), then
sys.exit(1); ensure you still echo the original creation result via
format_output only after all steps succeed and keep the UnityConnectionError
except block unchanged.
- Around line 102-135: The add-component and set-property calls (run_command
with "manage_components") are not checked for success, so if either fails the
Text element can be left incomplete; after each run_command for component
addition and for property setting (the two calls where "componentType":
"TextMeshProUGUI" and "action": "add"/"set_property" are used) inspect the
returned result like you do for manage_gameobject (check result.get("success")
or result.get("data") or result.get("result")), echo format_output(result,
config.format) and exit with sys.exit(1) (or raise UnityConnectionError) on
failure, and only print_success("Created Text: {name}") after all steps succeed;
keep existing UnityConnectionError except block as-is.
In `@Server/src/cli/commands/vfx.py`:
- Around line 417-431: The code currently blindly merges extra_params into
request_params which will raise if --params parses to a non-object (e.g., list);
validate that extra_params is a dict before updating: after the json.loads into
extra_params in vfx.py, check isinstance(extra_params, dict) and if not call
print_error with a friendly message like "params must be a JSON object" and
sys.exit(1); only then perform request_params.update(extra_params). Reference
extra_params, request_params, print_error, and the json.JSONDecodeError handling
to place the check.
In `@Server/src/cli/utils/__init__.py`:
- Around line 18-31: The __all__ list is not sorted which triggers Ruff RUF022;
update the __all__ declaration to either be alphabetically sorted or add a
per-line or list-level noqa to silence RUF022 while preserving grouping.
Specifically modify the __all__ variable (containing "CLIConfig", "get_config",
"set_config", "run_command", "run_check_connection", "run_list_instances",
"UnityConnectionError", "format_output", "print_success", "print_error",
"print_warning", "print_info") to be alphabetically ordered OR append a comment
like # noqa: RUF022 to the __all__ assignment so the linter stops flagging it.
Ensure you keep the same exported names and only change ordering or add the noqa
suppression.
In `@Server/src/cli/utils/output.py`:
- Around line 193-195: The info icon in print_info triggers Ruff RUF001 for
ambiguous Unicode; update the print_info function to either use an ASCII
alternative (e.g., "INFO:" or "i:") instead of the ℹ emoji, or add a local Ruff
suppression comment (e.g., # noqa: RUF001) directly above the print_info
definition to silence the rule; locate the function print_info in output.py and
apply the chosen change so the linter no longer flags the line.
In `@Server/src/main.py`:
- Around line 328-368: The cli_command_route handler currently catches a bare
Exception and returns the raw exception to the client; change it to catch
specific expected errors (e.g., json.JSONDecodeError, KeyError, ValueError) and
handle them with logger.exception(...) returning a 400 with a safe message (like
"Invalid request payload"), then add a final broad except Exception as e that
uses logger.exception("CLI command unexpected error") and returns a 500 with a
generic message (e.g., "Internal server error") without including str(e); update
imports to reference json.JSONDecodeError if needed and leave
PluginHub.get_sessions and PluginHub.send_command usage unchanged.
🧹 Nitpick comments (11)
Server/src/cli/__init__.py (1)
1-3: Consider single-sourcing version in the future.The CLI module defines its own version (
1.0.0) separate from the server version (9.0.8inpyproject.toml). This is acceptable for a new feature, but maintaining two independent versions may lead to confusion over time. Consider single-sourcing frompyproject.tomlusingimportlib.metadataif you want to keep versions aligned.prune_tool_results.py (1)
6-10: Consider catchingjson.JSONDecodeErrorinstead ofException.The broad
except Exceptioncatch works as a fallback, but catchingjson.JSONDecodeErrorwould be more precise and avoids masking unexpected errors (e.g.,MemoryError).♻️ Suggested refinement
def summarize(txt): try: obj = json.loads(txt) - except Exception: + except json.JSONDecodeError: return f"tool_result: {len(txt)} bytes"Server/src/main.py (1)
370-386: Consider consistent error handling for the instances endpoint.Similar to the command endpoint, this catches bare
Exception. While less critical for a read-only endpoint, consider logging withlogger.exceptionfor better debugging.Proposed improvement
except Exception as e: + logger.exception("Error listing Unity instances") return JSONResponse({"success": False, "error": str(e)}, status_code=500)Server/src/cli/commands/animation.py (1)
84-87: Remove unusedconfigvariable in placeholder command.The
configvariable is assigned but never used since this is a placeholder implementation. Either remove it or use it if formatting is intended for the output.Proposed fix
def set_parameter(target: str, param_name: str, value: str, param_type: str): """Set an Animator parameter. \b Examples: unity-mcp animation set-parameter "Player" "Speed" 5.0 unity-mcp animation set-parameter "Player" "IsRunning" true --type bool unity-mcp animation set-parameter "Player" "Jump" "" --type trigger """ - config = get_config() print_info( "Animation parameter command - requires custom Unity implementation") click.echo(f"Would set {param_name}={value} ({param_type}) on {target}")Server/src/cli/commands/audio.py (1)
8-8: Remove unused importprint_info.The
print_infofunction is imported but never used in this module.Proposed fix
-from cli.utils.output import format_output, print_error, print_info +from cli.utils.output import format_output, print_errorServer/src/cli/commands/scene.py (1)
219-255: Consider validating the supersize range.The help text indicates supersize should be 1-4, but there's no validation enforcing this. Invalid values could cause unexpected behavior on the Unity side.
♻️ Suggested fix using Click's IntRange
`@click.option`( "--supersize", "-s", default=1, - type=int, + type=click.IntRange(1, 4), help="Supersize multiplier (1-4)." )Server/src/cli/utils/config.py (1)
18-40: Consider adding exception chaining for better debugging.The static analysis correctly identifies that using
raise ... from errpreserves the original exception traceback, which aids debugging when users provide invalid environment variable values.♻️ Suggested fix with exception chaining
try: port = int(port_raw) except (ValueError, TypeError): - raise ValueError( - f"Invalid UNITY_MCP_HTTP_PORT value: {port_raw!r}") + raise ValueError( + f"Invalid UNITY_MCP_HTTP_PORT value: {port_raw!r}") from None timeout_raw = os.environ.get("UNITY_MCP_TIMEOUT", "30") try: timeout = int(timeout_raw) except (ValueError, TypeError): - raise ValueError( - f"Invalid UNITY_MCP_TIMEOUT value: {timeout_raw!r}") + raise ValueError( + f"Invalid UNITY_MCP_TIMEOUT value: {timeout_raw!r}") from NoneUsing
from Noneexplicitly suppresses the original exception chain (since the new message is self-explanatory), which is cleaner than an implicit chain.Server/src/cli/commands/ui.py (2)
1-1: Update the module docstring - this is a full implementation, not a placeholder.The docstring says "placeholder for future implementation" but the module contains a complete implementation with four functional commands.
♻️ Suggested fix
-"""UI CLI commands - placeholder for future implementation.""" +"""UI CLI commands - create and modify UI elements."""
179-185: Remove or use the unusedlabel_resultvariable.Static analysis correctly identified that
label_resultis assigned but never used. Either remove the assignment or use it for error checking.♻️ Option 1: Remove unused assignment
# Step 3: Create child label GameObject label_name = f"{name}_Label" - label_result = run_command("manage_gameobject", { + run_command("manage_gameobject", { "action": "create", "name": label_name, "parent": name, }, config)♻️ Option 2: Use for error checking (preferred)
# Step 3: Create child label GameObject label_name = f"{name}_Label" label_result = run_command("manage_gameobject", { "action": "create", "name": label_name, "parent": name, }, config) + + if not label_result.get("success"): + print_error(f"Failed to create button label: {label_result.get('error', 'Unknown error')}") + sys.exit(1)Server/src/cli/utils/connection.py (1)
80-96: Preserve exception context when wrapping httpx errors.
Currently the root cause is lost; chaining will keep traceback intact.♻️ Suggested fix
- except httpx.ConnectError as e: - raise UnityConnectionError( + except httpx.ConnectError as e: + raise UnityConnectionError( f"Cannot connect to Unity MCP server at {cfg.host}:{cfg.port}. " f"Make sure the server is running and Unity is connected.\n" f"Error: {e}" - ) + ) from e - except httpx.TimeoutException: - raise UnityConnectionError( + except httpx.TimeoutException as e: + raise UnityConnectionError( f"Connection to Unity timed out after {timeout or cfg.timeout}s. " f"Unity may be busy or unresponsive." - ) - except httpx.HTTPStatusError as e: - raise UnityConnectionError( + ) from e + except httpx.HTTPStatusError as e: + raise UnityConnectionError( f"HTTP error from server: {e.response.status_code} - {e.response.text}" - ) - except Exception as e: - raise UnityConnectionError(f"Unexpected error: {e}") + ) from e + except Exception as e: + raise UnityConnectionError(f"Unexpected error: {e}") from eServer/src/cli/commands/gameobject.py (1)
183-195: Consider using a unique identifier when adding components post-create.
Unity allows duplicate names; targeting bynamecan attach components to the wrong object. Ifmanage_gameobject createreturns an ID, prefer that formanage_components.
Comment on lines
+83
to
+89
| ``` | ||
| Checking connection to 127.0.0.1:8080... | ||
| ✅ Connected to Unity MCP server at 127.0.0.1:8080 | ||
| Connected Unity instances: | ||
| • MyProject (Unity 6000.2.10f1) [09abcc51] | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced blocks (MD040).
Markdownlint will flag these; use a text fence for non-code output/diagrams.
🧩 Suggested fix
-```
+```text
Checking connection to 127.0.0.1:8080...
✅ Connected to Unity MCP server at 127.0.0.1:8080
Connected Unity instances:
• MyProject (Unity 6000.2.10f1) [09abcc51]
```diff
-```
+```text
unity-mcp [GLOBAL_OPTIONS] COMMAND_GROUP [SUBCOMMAND] [ARGUMENTS] [OPTIONS]
</details>
Also applies to: 129-131
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @Server/src/cli/CLI_USAGE_GUIDE.md around lines 83 - 89, Add a language
identifier to the fenced code blocks in the CLI usage examples so markdownlint
MD040 is satisfied: change the triple-backtick fences that contain the CLI
output ("Checking connection to 127.0.0.1:8080... ✅ Connected to Unity MCP
server..." block) and the usage synopsis ("unity-mcp [GLOBAL_OPTIONS]
COMMAND_GROUP [SUBCOMMAND]...") to use text instead of plain so they are
treated as non-code text blocks.
</details>
<!-- fingerprinting:phantom:poseidon:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+345
to
+347
| # List connected Unity instances | ||
| unity-mcp instances | ||
| ``` |
Contributor
There was a problem hiding this comment.
Command name typo: instances vs instance list.
The CLI group is instance (see Server/src/cli/commands/instance.py). Update the example to avoid confusion.
🧩 Suggested fix
-# List connected Unity instances
-unity-mcp instances
+# List connected Unity instances
+unity-mcp instance list🤖 Prompt for AI Agents
In `@Server/src/cli/CLI_USAGE_GUIDE.md` around lines 345 - 347, The example in the
usage guide uses the wrong command name; update the snippet to use the CLI group
and subcommand defined in Server/src/cli/commands/instance.py by changing
"unity-mcp instances" to the correct "unity-mcp instance list" so it matches the
instance command group and its list subcommand.
Comment on lines
+31
to
+60
| def play(target: str, clip: Optional[str], search_method: Optional[str]): | ||
| """Play audio on a target's AudioSource. | ||
| \b | ||
| Examples: | ||
| unity-mcp audio play "MusicPlayer" | ||
| unity-mcp audio play "SFXSource" --clip "Assets/Audio/explosion.wav" | ||
| """ | ||
| config = get_config() | ||
|
|
||
| params: dict[str, Any] = { | ||
| "action": "set_property", | ||
| "target": target, | ||
| "componentType": "AudioSource", | ||
| "property": "Play", | ||
| "value": True, | ||
| } | ||
|
|
||
| if clip: | ||
| params["clip"] = clip | ||
|
|
||
| if search_method: | ||
| params["searchMethod"] = search_method | ||
|
|
||
| try: | ||
| result = run_command("manage_components", params, config) | ||
| click.echo(format_output(result, config.format)) | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the manage_components implementation
fd -t f -n "manage_components" --exec cat {} \;Repository: CoplayDev/unity-mcp
Length of output: 293
🏁 Script executed:
# Search for manage_components function/handler in the codebase
rg -n "def manage_components|manage_components" --type py -B 2 -A 10 | head -100Repository: CoplayDev/unity-mcp
Length of output: 6717
🏁 Script executed:
# Look for AudioSource handling in the backend
rg -n "AudioSource" --type py -C 5 | head -100Repository: CoplayDev/unity-mcp
Length of output: 4762
🏁 Script executed:
# Find all Python files that define manage_components function
rg -t py "def manage_components" -B 2 -A 20Repository: CoplayDev/unity-mcp
Length of output: 2114
🏁 Script executed:
# Search for backend implementation - look for C# or non-Python manage_components
fd -t f "manage_components"Repository: CoplayDev/unity-mcp
Length of output: 160
🏁 Script executed:
# Look in common backend directories
find . -type d -name "backend" -o -type d -name "server" -o -type d -name "handlers" | head -20Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
# Search for where manage_components is handled - likely RPC handler
rg "manage_components" --type py -lRepository: CoplayDev/unity-mcp
Length of output: 495
🏁 Script executed:
wc -l Server/src/services/tools/manage_components.pyRepository: CoplayDev/unity-mcp
Length of output: 113
🏁 Script executed:
cat -n Server/src/services/tools/manage_components.pyRepository: CoplayDev/unity-mcp
Length of output: 6004
🏁 Script executed:
# Also check the test file for audio-related tests
cat -n Server/tests/integration/test_manage_components.pyRepository: CoplayDev/unity-mcp
Length of output: 8543
🏁 Script executed:
# Find C# or backend files that might handle manage_components
find . -type f \( -name "*.cs" -o -name "*.csproj" \) | head -20Repository: CoplayDev/unity-mcp
Length of output: 1337
🏁 Script executed:
# Look for any documentation or comment about manage_components
rg "manage_components" -t cs -C 5Repository: CoplayDev/unity-mcp
Length of output: 1351
🏁 Script executed:
# Check if there's a separate backend directory
find . -type d -name "*backend*" -o -type d -name "*unity*" -o -type d -name "*handler*"Repository: CoplayDev/unity-mcp
Length of output: 219
🏁 Script executed:
wc -l MCPForUnity/Editor/Tools/ManageComponents.csRepository: CoplayDev/unity-mcp
Length of output: 111
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/ManageComponents.csRepository: CoplayDev/unity-mcp
Length of output: 15008
🏁 Script executed:
find . -type f -name "*ComponentOps*"Repository: CoplayDev/unity-mcp
Length of output: 157
🏁 Script executed:
rg "class ComponentOps" -B 2 -A 50Repository: CoplayDev/unity-mcp
Length of output: 4367
🏁 Script executed:
wc -l MCPForUnity/Editor/Helpers/ComponentOps.csRepository: CoplayDev/unity-mcp
Length of output: 109
🏁 Script executed:
cat -n MCPForUnity/Editor/Helpers/ComponentOps.cs | head -250Repository: CoplayDev/unity-mcp
Length of output: 11669
🏁 Script executed:
cat -n MCPForUnity/Editor/Helpers/ComponentOps.cs | tail -150Repository: CoplayDev/unity-mcp
Length of output: 7399
🏁 Script executed:
# Search for any special handling of method invocation like "Play" or "Stop"
rg "Play\|Stop" MCPForUnity/Editor/Helpers/ -C 3Repository: CoplayDev/unity-mcp
Length of output: 45
🌐 Web query:
Unity AudioSource Play method documentation API reference
💡 Result:
AudioSource.Play — signature and key notes
- Signature: public void Play(ulong delay = 0);. [1]
- What it does: starts playback of the AudioSource's clip. If the same clip is already assigned and playing, calling Play restarts it. [1][2]
- Deprecated detail: the delay parameter (number of samples at 44.1 kHz) is deprecated — use AudioSource.PlayDelayed (seconds) or AudioSource.PlayScheduled for more accurate timing. [1][2]
References:
[1] Unity Scripting API — AudioSource (method list and Play link).
[2] Unity Scripting API — AudioSource.Play documentation.
🏁 Script executed:
# Search for any method invocation handling in the codebase
rg "MethodInfo\|Invoke" MCPForUnity/Editor/ -lRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
# Check if there's any alternative approach for invoking methods
rg "Play|Stop" MCPForUnity/ --type cs -lRepository: CoplayDev/unity-mcp
Length of output: 1695
🏁 Script executed:
# Search for any indication of method invocation support
rg "invoke\|Invoke|MethodInfo" MCPForUnity/Editor/Tools/ -i -C 2Repository: CoplayDev/unity-mcp
Length of output: 1555
🏁 Script executed:
# Check the audio.py implementation more carefully
cat -n Server/src/cli/commands/audio.pyRepository: CoplayDev/unity-mcp
Length of output: 4326
The current implementation cannot work because Play() and Stop() are methods, not properties.
In Unity, AudioSource.Play() and AudioSource.Stop() are public methods, not properties. The backend's ComponentOps.SetProperty only handles property and field assignments via reflection—it has no mechanism to invoke methods. When this code executes, it will fail with an error like "Property or field 'Play' not found on component 'AudioSource'."
To fix this, the code needs to either:
- Add method invocation support to the
manage_componentsbackend (recommended for flexibility) - Implement audio-specific handlers that invoke Play/Stop directly
- Use a different action type that routes to method invocation instead of property assignment
The same issue affects the stop() command.
🤖 Prompt for AI Agents
In `@Server/src/cli/commands/audio.py` around lines 31 - 60, The play/stop CLI
commands call manage_components with action "set_property" for property
"Play"/"Stop", but AudioSource.Play()/Stop() are methods and
ComponentOps.SetProperty cannot invoke methods—this will fail; update the fix by
either extending the backend manage_components to support a new action (e.g.,
"invoke_method") that takes "methodName" and optional args and calls
ComponentOps.InvokeMethod (or equivalent), or special-case audio in the CLI to
call a new action like "invoke_method" with methodName "Play"/"Stop"; change the
play() and stop() functions to send that action and parameters instead of
"set_property", and update any backend handler names referenced
(ComponentOps.SetProperty -> implement ComponentOps.InvokeMethod or
method-invocation path) so method invocation is performed rather than property
assignment.
Comment on lines
+179
to
+184
| if output: | ||
| with open(output, 'w') as f: | ||
| f.write(json_output) | ||
| print_success(f"Template written to: {output}") | ||
| else: | ||
| click.echo(json_output) |
Contributor
There was a problem hiding this comment.
Add error handling for file write operation.
The template command writes to a file without handling potential I/O errors. This could fail silently or crash if the path is invalid or permissions are denied.
Proposed fix
if output:
- with open(output, 'w') as f:
- f.write(json_output)
- print_success(f"Template written to: {output}")
+ try:
+ with open(output, 'w') as f:
+ f.write(json_output)
+ print_success(f"Template written to: {output}")
+ except IOError as e:
+ print_error(f"Error writing file: {e}")
+ sys.exit(1)
else:
click.echo(json_output)🤖 Prompt for AI Agents
In `@Server/src/cli/commands/batch.py` around lines 179 - 184, The file-writing
block that uses output, json_output, and print_success lacks error handling;
wrap the open/write in a try/except that catches OSError/IOError (or generic
Exception), and on failure log/echo a clear error using the existing CLI
utilities (e.g., print_error or click.echo) including the exception message and
the target path, then exit non‑zero (e.g., raise click.Abort or sys.exit(1));
also ensure the file is opened with an explicit encoding (utf-8) to avoid
encoding issues.
Comment on lines
+59
to
+69
| try: | ||
| result = run_command("manage_script", params, config) | ||
| # For read, output content directly if available | ||
| if result.get("success") and result.get("data"): | ||
| data = result.get("data", {}) | ||
| if isinstance(data, dict) and "contents" in data: | ||
| click.echo(data["contents"]) | ||
| else: | ||
| click.echo(format_output(result, config.format)) | ||
| else: | ||
| click.echo(format_output(result, config.format)) |
Contributor
There was a problem hiding this comment.
Normalize read response before printing contents.
read assumes success/data at the top level, but search shows responses can be nested under result and/or base64-encoded. In those cases, read won’t print file contents as intended.
🐛 Suggested fix (align with search)
try:
result = run_command("manage_script", params, config)
- # For read, output content directly if available
- if result.get("success") and result.get("data"):
- data = result.get("data", {})
- if isinstance(data, dict) and "contents" in data:
- click.echo(data["contents"])
- else:
- click.echo(format_output(result, config.format))
- else:
- click.echo(format_output(result, config.format))
+ # Normalize response shape (some endpoints return {status, result:{...}})
+ inner_result = result.get("result", result)
+ if inner_result.get("success") and inner_result.get("data"):
+ data = inner_result.get("data", {})
+ contents = data.get("contents")
+ if not contents and data.get("encodedContents"):
+ try:
+ import base64
+ contents = base64.b64decode(
+ data["encodedContents"]
+ ).decode("utf-8", "replace")
+ except (ValueError, TypeError):
+ contents = None
+ if contents is not None:
+ click.echo(contents)
+ return
+ click.echo(format_output(result, config.format))
except UnityConnectionError as e:
print_error(str(e))
sys.exit(1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| result = run_command("manage_script", params, config) | |
| # For read, output content directly if available | |
| if result.get("success") and result.get("data"): | |
| data = result.get("data", {}) | |
| if isinstance(data, dict) and "contents" in data: | |
| click.echo(data["contents"]) | |
| else: | |
| click.echo(format_output(result, config.format)) | |
| else: | |
| click.echo(format_output(result, config.format)) | |
| try: | |
| result = run_command("manage_script", params, config) | |
| # Normalize response shape (some endpoints return {status, result:{...}}) | |
| inner_result = result.get("result", result) | |
| if inner_result.get("success") and inner_result.get("data"): | |
| data = inner_result.get("data", {}) | |
| contents = data.get("contents") | |
| if not contents and data.get("encodedContents"): | |
| try: | |
| import base64 | |
| contents = base64.b64decode( | |
| data["encodedContents"] | |
| ).decode("utf-8", "replace") | |
| except (ValueError, TypeError): | |
| contents = None | |
| if contents is not None: | |
| click.echo(contents) | |
| return | |
| click.echo(format_output(result, config.format)) |
🤖 Prompt for AI Agents
In `@Server/src/cli/commands/code.py` around lines 59 - 69, The read path after
calling run_command("manage_script", params, config) should normalize
nested/encoded responses like the search implementation: first unwrap a
top-level "result" key if present (i.e., use result = result.get("result",
result)), then extract the "data" payload, check for "contents" inside that data
(or inside data.get("data") if nested), and if the contents are base64-encoded
decode them before calling click.echo; otherwise fall back to
format_output(result, config.format). Update the logic around result/data
handling in the read flow (the block using run_command, result, data,
format_output) to perform this normalization and decoding so file contents are
printed correctly when nested or encoded.
Comment on lines
+102
to
+135
| try: | ||
| # Step 1: Create empty GameObject with parent | ||
| result = run_command("manage_gameobject", { | ||
| "action": "create", | ||
| "name": name, | ||
| "parent": parent, | ||
| "position": list(position), | ||
| }, config) | ||
|
|
||
| if not (result.get("success") or result.get("data") or result.get("result")): | ||
| click.echo(format_output(result, config.format)) | ||
| return | ||
|
|
||
| # Step 2: Add RectTransform and TextMeshProUGUI | ||
| run_command("manage_components", { | ||
| "action": "add", | ||
| "target": name, | ||
| "componentType": "TextMeshProUGUI", | ||
| }, config) | ||
|
|
||
| # Step 3: Set text content | ||
| run_command("manage_components", { | ||
| "action": "set_property", | ||
| "target": name, | ||
| "componentType": "TextMeshProUGUI", | ||
| "property": "text", | ||
| "value": text, | ||
| }, config) | ||
|
|
||
| click.echo(format_output(result, config.format)) | ||
| print_success(f"Created Text: {name}") | ||
| except UnityConnectionError as e: | ||
| print_error(str(e)) | ||
| sys.exit(1) |
Contributor
There was a problem hiding this comment.
Same intermediate error handling gap as create_canvas.
The component addition and property setting steps don't check for failures, potentially leaving the text element in an incomplete state.
🤖 Prompt for AI Agents
In `@Server/src/cli/commands/ui.py` around lines 102 - 135, The add-component and
set-property calls (run_command with "manage_components") are not checked for
success, so if either fails the Text element can be left incomplete; after each
run_command for component addition and for property setting (the two calls where
"componentType": "TextMeshProUGUI" and "action": "add"/"set_property" are used)
inspect the returned result like you do for manage_gameobject (check
result.get("success") or result.get("data") or result.get("result")), echo
format_output(result, config.format) and exit with sys.exit(1) (or raise
UnityConnectionError) on failure, and only print_success("Created Text: {name}")
after all steps succeed; keep existing UnityConnectionError except block as-is.
Comment on lines
+193
to
+195
| def print_info(message: str) -> None: | ||
| """Print an info message.""" | ||
| click.echo(f"ℹ️ {message}") |
Contributor
There was a problem hiding this comment.
Ruff RUF001: ambiguous Unicode in info icon.
If Ruff is enforced, the ℹ emoji will trigger RUF001. Either replace with ASCII or suppress the rule locally.
🧩 Minimal suppression
def print_info(message: str) -> None:
"""Print an info message."""
- click.echo(f"ℹ️ {message}")
+ click.echo(f"ℹ️ {message}") # noqa: RUF001📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def print_info(message: str) -> None: | |
| """Print an info message.""" | |
| click.echo(f"ℹ️ {message}") | |
| def print_info(message: str) -> None: | |
| """Print an info message.""" | |
| click.echo(f"ℹ️ {message}") # noqa: RUF001 |
🧰 Tools
🪛 Ruff (0.14.13)
195-195: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
🤖 Prompt for AI Agents
In `@Server/src/cli/utils/output.py` around lines 193 - 195, The info icon in
print_info triggers Ruff RUF001 for ambiguous Unicode; update the print_info
function to either use an ASCII alternative (e.g., "INFO:" or "i:") instead of
the ℹ emoji, or add a local Ruff suppression comment (e.g., # noqa: RUF001)
directly above the print_info definition to silence the rule; locate the
function print_info in output.py and apply the chosen change so the linter no
longer flags the line.
Comment on lines
328
to
368
| @mcp.custom_route("/api/command", methods=["POST"]) | ||
| async def cli_command_route(request: Request) -> JSONResponse: | ||
| """REST endpoint for CLI commands to Unity.""" | ||
| try: | ||
| body = await request.json() | ||
|
|
||
| command_type = body.get("type") | ||
| params = body.get("params", {}) | ||
| unity_instance = body.get("unity_instance") | ||
|
|
||
| if not command_type: | ||
| return JSONResponse({"success": False, "error": "Missing 'type' field"}, status_code=400) | ||
|
|
||
| # Get available sessions | ||
| sessions = await PluginHub.get_sessions() | ||
| if not sessions.sessions: | ||
| return JSONResponse({ | ||
| "success": False, | ||
| "error": "No Unity instances connected. Make sure Unity is running with MCP plugin." | ||
| }, status_code=503) | ||
|
|
||
| # Find target session | ||
| session_id = None | ||
| if unity_instance: | ||
| # Try to match by hash or project name | ||
| for sid, details in sessions.sessions.items(): | ||
| if details.hash == unity_instance or details.project == unity_instance: | ||
| session_id = sid | ||
| break | ||
|
|
||
| if not session_id: | ||
| # Use first available session | ||
| session_id = next(iter(sessions.sessions.keys())) | ||
|
|
||
| # Send command to Unity | ||
| result = await PluginHub.send_command(session_id, command_type, params) | ||
| return JSONResponse(result) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"CLI command error: {e}") | ||
| return JSONResponse({"success": False, "error": str(e)}, status_code=500) |
Contributor
There was a problem hiding this comment.
Improve exception handling and logging in the CLI command endpoint.
The static analysis correctly flags two issues:
- Catching bare
Exceptionis overly broad and may mask unexpected errors logging.exceptionis preferred overlogging.errorwhen logging exceptions as it automatically includes the traceback
Additionally, the error response exposes the raw exception message to the client, which could leak internal details.
Proposed fix
except Exception as e:
- logger.error(f"CLI command error: {e}")
- return JSONResponse({"success": False, "error": str(e)}, status_code=500)
+ logger.exception("CLI command error")
+ return JSONResponse({"success": False, "error": "Internal server error"}, status_code=500)🧰 Tools
🪛 Ruff (0.14.13)
366-366: Do not catch blind exception: Exception
(BLE001)
367-367: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🤖 Prompt for AI Agents
In `@Server/src/main.py` around lines 328 - 368, The cli_command_route handler
currently catches a bare Exception and returns the raw exception to the client;
change it to catch specific expected errors (e.g., json.JSONDecodeError,
KeyError, ValueError) and handle them with logger.exception(...) returning a 400
with a safe message (like "Invalid request payload"), then add a final broad
except Exception as e that uses logger.exception("CLI command unexpected error")
and returns a 500 with a generic message (e.g., "Internal server error") without
including str(e); update imports to reference json.JSONDecodeError if needed and
leave PluginHub.get_sessions and PluginHub.send_command usage unchanged.
… modules and to surface real import-time failures, so broken command modules don’t get silently ignored.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Server/src/cli/commands/vfx.py`:
- Around line 211-215: After parsing positions with json.loads into
positions_list, validate that positions_list is an actual list (use
isinstance(positions_list, list)); if it is not a list, call print_error with a
clear message like "positions must be a JSON list" and sys.exit(1). Update the
try/except block around json.loads in the vfx command (where positions and
positions_list are used) to perform this type check immediately after successful
parse and before sending to Unity.
♻️ Duplicate comments (1)
Server/src/main.py (1)
375-377: Improve exception logging and avoid exposing internal error details.Per static analysis and the previous review comment (still applicable), use
logger.exceptioninstead oflogger.errorto capture the full traceback, and avoid returning raw exception messages to clients.🛡️ Proposed fix
except Exception as e: - logger.error(f"CLI command error: {e}") - return JSONResponse({"success": False, "error": str(e)}, status_code=500) + logger.exception("CLI command error") + return JSONResponse({"success": False, "error": "Internal server error"}, status_code=500)
🧹 Nitpick comments (2)
Server/src/cli/main.py (1)
171-175: Validate thatparamsis a dict to match the type contract.The
run_commandfunction signature expectsparams: Dict[str, Any], but this code doesn't validate that the parsed JSON is actually a dict. For consistency with thevfx rawcommand and type safety, add validation.🛡️ Proposed fix
try: params_dict = json.loads(params) except json.JSONDecodeError as e: print_error(f"Invalid JSON params: {e}") sys.exit(1) + if not isinstance(params_dict, dict): + print_error("Invalid JSON params: expected an object") + sys.exit(1)Server/src/main.py (1)
394-395: Apply consistent error handling.For consistency with the
/api/commandendpoint fix, avoid exposing raw exception messages.🛡️ Proposed fix
except Exception as e: - return JSONResponse({"success": False, "error": str(e)}, status_code=500) + logger.exception("Failed to list instances") + return JSONResponse({"success": False, "error": "Internal server error"}, status_code=500)
Comment on lines
+211
to
+215
| try: | ||
| positions_list = json.loads(positions) | ||
| except json.JSONDecodeError as e: | ||
| print_error(f"Invalid JSON for positions: {e}") | ||
| sys.exit(1) |
Contributor
There was a problem hiding this comment.
Validate that positions parses to a list.
Unlike the vfx_raw command which validates that params is a dict, this code doesn't verify that the parsed JSON is actually a list. If a user passes a non-list JSON (e.g., --positions '{"x": 1}'), it will be sent to Unity and likely cause a confusing error.
🛡️ Proposed fix
try:
positions_list = json.loads(positions)
except json.JSONDecodeError as e:
print_error(f"Invalid JSON for positions: {e}")
sys.exit(1)
+ if not isinstance(positions_list, list):
+ print_error("Invalid JSON for positions: expected an array")
+ sys.exit(1)🤖 Prompt for AI Agents
In `@Server/src/cli/commands/vfx.py` around lines 211 - 215, After parsing
positions with json.loads into positions_list, validate that positions_list is
an actual list (use isinstance(positions_list, list)); if it is not a list, call
print_error with a clear message like "positions must be a JSON list" and
sys.exit(1). Update the try/except block around json.loads in the vfx command
(where positions and positions_list are used) to perform this type check
immediately after successful parse and before sending to Unity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Builds of @Scriptwonder's CLI, but formats the files and include some fixes for the
mainbranchSummary by Sourcery
Introduce a dedicated Unity MCP command-line interface and corresponding HTTP endpoints for issuing commands and managing instances, while tightening handling of configuration and built‑in tools.
New Features:
unity-mcpCLI with grouped commands for game objects, components, scenes, assets, scripts, shaders, materials, editor control, VFX, audio, lighting, UI, batch execution, and instance management.Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.