feat(manage_editor): add wait_for_compilation action#815
Open
smuhlaci wants to merge 5 commits intoCoplayDev:betafrom
Open
feat(manage_editor): add wait_for_compilation action#815smuhlaci wants to merge 5 commits intoCoplayDev:betafrom
smuhlaci wants to merge 5 commits intoCoplayDev:betafrom
Conversation
Poll editor_state until compilation and domain reload finish so the AI can wait for script changes to compile instead of using fixed sleep. - manage_editor(action="wait_for_compilation", timeout=30) on server - CLI: unity-mcp editor wait-compile [--timeout N] - Reuses wait_for_editor_ready from refresh_unity; no C# changes Co-authored-by: Cursor <cursoragent@cursor.com>
… references and guides
Contributor
Reviewer's GuideAdds a new wait_for_compilation manage_editor action implemented entirely on the Python server, exposes it via a new CLI subcommand, wires it into existing editor readiness polling, and documents the new workflow along with integration tests covering readiness, polling, and timeout behavior. Sequence diagram for the new wait_for_compilation manage_editor actionsequenceDiagram
actor Agent
participant CLI as CLI_unity_mcp
participant Server as Python_Server
participant ManageEditor as manage_editor_tool
participant WaitReady as wait_for_editor_ready
participant Unity as Unity_Editor
Agent->>CLI: unity-mcp editor wait-compile --timeout N
CLI->>Server: run_command manage_editor action=wait_for_compilation timeout=N
Server->>ManageEditor: manage_editor(ctx, action, timeout)
ManageEditor->>ManageEditor: coerce_bool(wait_for_completion)
ManageEditor->>ManageEditor: detect action == wait_for_compilation
ManageEditor->>ManageEditor: _wait_for_compilation(ctx, timeout)
ManageEditor->>WaitReady: wait_for_editor_ready(ctx, timeout_s)
loop Poll editor_state until ready or timeout
WaitReady->>Unity: query editor_state (compilation, domain_reload)
Unity-->>WaitReady: editor_state snapshot
end
WaitReady-->>ManageEditor: ready_flag, elapsed_seconds
alt Compilation finished before timeout
ManageEditor-->>Server: {success: True, data: {waited_seconds, ready: True}}
Server-->>CLI: formatted success result
CLI->>CLI: print_success("Compilation complete (waited Xs)")
CLI-->>Agent: result
else Timed out waiting for compilation
ManageEditor-->>Server: {success: False, data: {waited_seconds, ready: False, timeout_seconds}}
Server-->>CLI: formatted failure result
CLI->>CLI: print_error("Compilation wait timed out")
CLI-->>Agent: timeout result
end
Updated class diagram for manage_editor tool and editor wait_compile CLI commandclassDiagram
class ManageEditorTool {
+manage_editor(ctx, action, wait_for_completion, timeout, tool_name, tag_name, layer_name) dict
+_wait_for_compilation(ctx, timeout) dict
}
class RefreshUnityTool {
+wait_for_editor_ready(ctx, timeout_s) tuple
}
class EditorCLI {
+wait_compile(timeout)
+play()
+pause()
+stop()
}
class UnityConnection {
+async_send_command_with_retry(instance, command, params)
}
class TransportHelpers {
+send_with_unity_instance(sender, unity_instance, command, params)
+get_unity_instance_from_context(ctx)
}
class TelemetryService {
+is_telemetry_enabled() bool
+record_tool_usage(event_name, success, duration, metadata)
}
ManageEditorTool --> RefreshUnityTool : uses wait_for_editor_ready
ManageEditorTool --> TransportHelpers : uses
ManageEditorTool --> UnityConnection : sends manage_editor commands
ManageEditorTool --> TelemetryService : telemetry_status, telemetry_ping
EditorCLI --> ManageEditorTool : calls via run_command manage_editor
EditorCLI --> TransportHelpers : uses config and formatting helpers
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 1 issue, and left some high level feedback:
- _WAIT_FOR_COMPILATION_ACTIONS is declared but never used in manage_editor.py; either remove it or wire it into any action classification/validation logic you intended it for.
- In the CLI wait-compile command, the error message is hard-coded to 'Compilation wait timed out' regardless of the actual failure cause; consider surfacing result['message'] (and/or checking a timeout flag) so non-timeout errors are reported accurately.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- _WAIT_FOR_COMPILATION_ACTIONS is declared but never used in manage_editor.py; either remove it or wire it into any action classification/validation logic you intended it for.
- In the CLI wait-compile command, the error message is hard-coded to 'Compilation wait timed out' regardless of the actual failure cause; consider surfacing result['message'] (and/or checking a timeout flag) so non-timeout errors are reported accurately.
## Individual Comments
### Comment 1
<location> `Server/src/cli/commands/editor.py:47` </location>
<code_context>
+ waited = result.get("data", {}).get("waited_seconds", 0)
+ print_success(f"Compilation complete (waited {waited}s)")
+ else:
+ print_error("Compilation wait timed out")
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** CLI error message assumes timeout even if the failure is due to another server-side error.
Any non-success from `manage_editor` currently maps to `print_error("Compilation wait timed out")`, but `_wait_for_compilation`/`manage_editor` can fail for other reasons (e.g., Python error, connectivity). Consider either checking a specific `ready`/`timeout` indicator in `result['data']`, or using `result.get("message")` (or similar) for non-timeout failures so users see the real cause.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
📝 WalkthroughWalkthroughAdded a new "wait_for_compilation" editor action plus a CLI command to block until Unity script compilation (and domain reload) finishes with an optional timeout; service logic, tests, CLI help, and docs/reference entries were added or updated accordingly. Changes
Sequence Diagram(s)mermaid 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
docs/guides/CLI_USAGE.md (1)
344-344:⚠️ Potential issue | 🟡 Minor
wait-compileis missing from the Complete Command Reference table.The new
wait-compilesubcommand was added to the Editor Controls section above (line 154) but not listed in the summary table on this line. Add it to keep the documentation consistent.📝 Proposed fix
-| `editor` | `play`, `pause`, `stop`, `refresh`, `console`, `menu`, `tool`, `add-tag`, `remove-tag`, `add-layer`, `remove-layer`, `tests`, `poll-test`, `custom-tool` | +| `editor` | `play`, `pause`, `stop`, `wait-compile`, `refresh`, `console`, `menu`, `tool`, `add-tag`, `remove-tag`, `add-layer`, `remove-layer`, `tests`, `poll-test`, `custom-tool` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guides/CLI_USAGE.md` at line 344, Add the missing `wait-compile` subcommand to the Editor Controls list in the Complete Command Reference table by inserting `wait-compile` into the backtick-enclosed list for `editor` (the same list that currently contains `play`, `pause`, `stop`, `refresh`, `console`, `menu`, `tool`, `add-tag`, `remove-tag`, `add-layer`, `remove-layer`, `tests`, `poll-test`, `custom-tool`) so the summary matches the detailed Editor Controls section.
🧹 Nitpick comments (3)
Server/src/cli/commands/editor.py (1)
21-23: Minor:-tshort option shadows the global-t(--timeout).The global CLI option
-t/--timeoutcontrols the HTTP request timeout (line 38 in CLI_USAGE.md), while this subcommand's-t/--timeoutcontrols the compilation wait duration. Click scopes them correctly so there's no runtime conflict, but it could confuse users. Consider using a different short flag (e.g.,-wfor "wait") or dropping the short option entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/cli/commands/editor.py` around lines 21 - 23, The subcommand in Server/src/cli/commands/editor.py defines a Click option click.option("--timeout", "-t", type=float) that reuses the global short flag -t; change the short flag to avoid user confusion (e.g., use "-w" for wait or remove the short option entirely). Update the option declaration for the editor command (the click.option for "--timeout") and adjust any code/comments that reference the short flag so the new short name (or no short name) is used consistently.Server/src/services/tools/manage_editor.py (2)
78-78: Unused variablestart.
start = time.monotonic()is assigned but never read —elapsedcomes from the return value ofwait_for_editor_ready. This also makesimport timeat line 1 unnecessary.♻️ Proposed fix
- start = time.monotonic() ready, elapsed = await wait_for_editor_ready(ctx, timeout_s=timeout_s)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/manage_editor.py` at line 78, Remove the unused timing variable and redundant import: delete the line assigning start = time.monotonic() and remove the top-level import time if no other code in manage_editor.py uses it; confirm that wait_for_editor_ready’s return value (elapsed) is used instead and that no other references to start or time.monotonic remain (look for start and wait_for_editor_ready in this module).
14-14: Unused constant_WAIT_FOR_COMPILATION_ACTIONS.This
frozensetis defined but never referenced. The routing on line 49 uses a direct string comparison instead. Remove it or use it in the routing logic.♻️ Option A: Remove unused constant
-_WAIT_FOR_COMPILATION_ACTIONS = frozenset({"wait_for_compilation"}) -♻️ Option B: Use the constant in routing
- if action == "wait_for_compilation": + if action in _WAIT_FOR_COMPILATION_ACTIONS:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/manage_editor.py` at line 14, The constant _WAIT_FOR_COMPILATION_ACTIONS is defined but never used; either remove that unused frozenset or change the routing logic that currently does a direct string comparison (the action routing in this module) to use a membership check against _WAIT_FOR_COMPILATION_ACTIONS (e.g., replace the direct "wait_for_compilation" equality check with "if action in _WAIT_FOR_COMPILATION_ACTIONS"). Update or delete the _WAIT_FOR_COMPILATION_ACTIONS symbol accordingly so there are no unused definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/guides/CLI_USAGE.md`:
- Line 344: Add the missing `wait-compile` subcommand to the Editor Controls
list in the Complete Command Reference table by inserting `wait-compile` into
the backtick-enclosed list for `editor` (the same list that currently contains
`play`, `pause`, `stop`, `refresh`, `console`, `menu`, `tool`, `add-tag`,
`remove-tag`, `add-layer`, `remove-layer`, `tests`, `poll-test`, `custom-tool`)
so the summary matches the detailed Editor Controls section.
---
Nitpick comments:
In `@Server/src/cli/commands/editor.py`:
- Around line 21-23: The subcommand in Server/src/cli/commands/editor.py defines
a Click option click.option("--timeout", "-t", type=float) that reuses the
global short flag -t; change the short flag to avoid user confusion (e.g., use
"-w" for wait or remove the short option entirely). Update the option
declaration for the editor command (the click.option for "--timeout") and adjust
any code/comments that reference the short flag so the new short name (or no
short name) is used consistently.
In `@Server/src/services/tools/manage_editor.py`:
- Line 78: Remove the unused timing variable and redundant import: delete the
line assigning start = time.monotonic() and remove the top-level import time if
no other code in manage_editor.py uses it; confirm that wait_for_editor_ready’s
return value (elapsed) is used instead and that no other references to start or
time.monotonic remain (look for start and wait_for_editor_ready in this module).
- Line 14: The constant _WAIT_FOR_COMPILATION_ACTIONS is defined but never used;
either remove that unused frozenset or change the routing logic that currently
does a direct string comparison (the action routing in this module) to use a
membership check against _WAIT_FOR_COMPILATION_ACTIONS (e.g., replace the direct
"wait_for_compilation" equality check with "if action in
_WAIT_FOR_COMPILATION_ACTIONS"). Update or delete the
_WAIT_FOR_COMPILATION_ACTIONS symbol accordingly so there are no unused
definitions.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.claude/skills/unity-mcp-skill/references/tools-reference.mdServer/src/cli/CLI_USAGE_GUIDE.mdServer/src/cli/commands/editor.pyServer/src/services/tools/manage_editor.pyServer/tests/integration/test_manage_editor_wait.pydocs/guides/CLI_EXAMPLE.mddocs/guides/CLI_USAGE.mdunity-mcp-skill/references/tools-reference.md
…wait-compile
- Remove unused _WAIT_FOR_COMPILATION_ACTIONS from manage_editor.py
- Use result.get('message', ...) in editor wait-compile so non-timeout
errors (e.g. Python/connectivity) are reported accurately
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Server/src/cli/commands/editor.py`:
- Line 41: The call to run_command("manage_editor", {"action":
"wait_for_compilation", "timeout": timeout}, config) doesn’t pass a transport
timeout so long waits can be cut off; update the invocation of run_command (from
editor CLI) to include a transport timeout parameter equal to or greater than
the requested timeout (e.g., pass transport_timeout=timeout or set it on the
config) so the RPC/transport uses the same timeout when invoking manage_editor
with action "wait_for_compilation".
In `@Server/src/services/tools/manage_editor.py`:
- Line 1: Remove the dead start = time.monotonic() assignment and the now-unused
import time in manage_editor.py: delete the start variable (the one set
immediately before calling wait_for_editor_ready) since elapsed is returned
directly by wait_for_editor_ready, and remove the top-level import time because
it becomes unused after dropping that assignment.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Server/src/cli/commands/editor.pyServer/src/services/tools/manage_editor.pydocs/guides/CLI_USAGE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/guides/CLI_USAGE.md
| @@ -1,3 +1,4 @@ | |||
| import time | |||
Contributor
There was a problem hiding this comment.
Remove unused start variable and the now-dead import time.
start = time.monotonic() (Line 76) is flagged by Ruff (F841) and has no effect — elapsed is already returned directly by wait_for_editor_ready. Once Line 76 is removed, the import time at Line 1 becomes unused as well.
🧹 Proposed fix
-import time
from typing import Annotated, Any, Literal timeout_s = max(1.0, min(timeout_s, 120.0))
- start = time.monotonic()
ready, elapsed = await wait_for_editor_ready(ctx, timeout_s=timeout_s)Also applies to: 76-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/services/tools/manage_editor.py` at line 1, Remove the dead start
= time.monotonic() assignment and the now-unused import time in
manage_editor.py: delete the start variable (the one set immediately before
calling wait_for_editor_ready) since elapsed is returned directly by
wait_for_editor_ready, and remove the top-level import time because it becomes
unused after dropping that assignment.
… not cut off Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Server/src/cli/commands/editor.py (1)
41-43: Transport timeout buffer correctly implemented.The 10-second buffer ensures the transport layer outlasts the server-side compilation wait regardless of float truncation by
int(). Consistent with the addressed past review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/cli/commands/editor.py` around lines 41 - 43, The current buffer implementation is correct — keep the transport_timeout calculation and usage as-is: compute transport_timeout = int(timeout) + 10 and pass it into run_command(...) while leaving the command payload timeout as the original timeout; the relevant symbols are transport_timeout, timeout, and the run_command("manage_editor", {"action": "wait_for_compilation", "timeout": timeout}, ..., timeout=transport_timeout) call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Server/src/cli/commands/editor.py`:
- Around line 45-49: The success message breaks when "waited_seconds" is present
but null (producing None) and shows verbose raw floats; change how waited is
obtained and formatted in Server/src/cli/commands/editor.py: replace
result.get("data", {}).get("waited_seconds", 0) with a retrieval that coerces
null to 0 (e.g., waited = result.get("data", {}).get("waited_seconds") or 0) and
then format the value before passing to print_success (e.g., round or format to
2 decimal places with f-string) so print_success(f"Compilation complete (waited
{formatted}s)") always receives a sane string; keep references to result,
waited, and print_success.
---
Duplicate comments:
In `@Server/src/cli/commands/editor.py`:
- Around line 41-43: The current buffer implementation is correct — keep the
transport_timeout calculation and usage as-is: compute transport_timeout =
int(timeout) + 10 and pass it into run_command(...) while leaving the command
payload timeout as the original timeout; the relevant symbols are
transport_timeout, timeout, and the run_command("manage_editor", {"action":
"wait_for_compilation", "timeout": timeout}, ..., timeout=transport_timeout)
call.
Comment on lines
+45
to
+49
| if result.get("success"): | ||
| waited = result.get("data", {}).get("waited_seconds", 0) | ||
| print_success(f"Compilation complete (waited {waited}s)") | ||
| else: | ||
| print_error(result.get("message", "Compilation wait timed out")) |
Contributor
There was a problem hiding this comment.
waited_seconds: null from the server bypasses the default and produces a malformed message.
dict.get(key, default) only applies the default when the key is absent. If the server ever returns "waited_seconds": null, the expression result.get("data", {}).get("waited_seconds", 0) evaluates to None, and the success message becomes "Compilation complete (waited Nones)".
Additionally, a raw float like 2.3456 would render verbosely in the output string.
🛡️ Proposed fix
- waited = result.get("data", {}).get("waited_seconds", 0)
- print_success(f"Compilation complete (waited {waited}s)")
+ waited = result.get("data", {}).get("waited_seconds") or 0
+ print_success(f"Compilation complete (waited {waited:.1f}s)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/cli/commands/editor.py` around lines 45 - 49, The success message
breaks when "waited_seconds" is present but null (producing None) and shows
verbose raw floats; change how waited is obtained and formatted in
Server/src/cli/commands/editor.py: replace result.get("data",
{}).get("waited_seconds", 0) with a retrieval that coerces null to 0 (e.g.,
waited = result.get("data", {}).get("waited_seconds") or 0) and then format the
value before passing to print_success (e.g., round or format to 2 decimal places
with f-string) so print_success(f"Compilation complete (waited {formatted}s)")
always receives a sane string; keep references to result, waited, and
print_success.
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
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.
Poll editor_state until compilation and domain reload finish so the AI can wait for script changes to compile instead of using fixed sleep.
Description
Issue #814 describes the problem. Agents using long sleeps after script changes) and proposes
wait_for_compilation.Type of Change
Changes Made
Adds a
wait_for_compilationaction tomanage_editorso AI agents can wait for Unity compilation (and domain reload) to finish instead of using fixedsleepdurations or custom polling ofeditor_state. This makes script-edit → play → verify workflows faster and more reliable.Fixes the workflow issue where agents had to use ~5–12 seconds of sleep after script changes before playing or running other editor operations.
1.
Server/src/services/tools/manage_editor.pywait_for_compilationaction:wait_for_editor_ready(or equivalent) to polleditor_stateuntil compilation and domain reload are done.timeoutparameter (default 30s, max 120s).waited_secondsfor observability.2.
Server/src/cli/commands/editor.pyunity-mcp editor wait-compile--timeoutfor the same timeout behavior.3.
Server/tests/integration/test_manage_editor_wait.py(new file)wait_for_compilation:manage_editorand behavior after domain reload.4. C# / Unity side
editor_stateand the existing “wait for ready” infrastructure uses it.Testing/Screenshots/Recordings
test_manage_editor_wait.py(e.g. 6 cases covering ready, poll, timeout, CLI).Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdChecklist
wait_for_compilationimplemented inmanage_editor.pyunity-mcp editor wait-compileaddedRelated Issues
Fixes #814
Additional Notes
Summary by Sourcery
Add a server-side manage_editor action and CLI command to wait for Unity script compilation to complete before proceeding with editor operations.
New Features:
unity-mcp editor wait-compilewith optional timeout to wait for scripts to compile from the command line.Documentation:
wait_for_compilationmanage_editor action in tools references.unity-mcp editor wait-compilecommand.Tests:
Summary by CodeRabbit
New Features
Documentation
Tests