diff --git a/.github/workflows/e2e-bridge.yml b/.github/workflows/e2e-bridge.yml index e6efaf9d0..c67c77a11 100644 --- a/.github/workflows/e2e-bridge.yml +++ b/.github/workflows/e2e-bridge.yml @@ -45,7 +45,17 @@ jobs: echo "unity_ok=true" >> "$GITHUB_OUTPUT" else echo "unity_ok=false" >> "$GITHUB_OUTPUT" - echo "::warning::Unity license secrets absent; E2E bridge smoke will be skipped (not failed)." + echo "::warning::E2E bridge smoke SKIPPED - no license secrets in scope (normal for fork PRs). This check is NOT a pass: nothing was booted or exercised." + # Every step below is gated on unity_ok, so the job reports a green check + # having run nothing at all. Say so plainly on the run page. + { + echo "## :warning: E2E bridge smoke was SKIPPED" + echo + echo "No Unity license secrets were in scope, so **no Editor was booted and no tool call was exercised**." + echo "The green check means the job exited cleanly - **not** that the bridge works." + echo + echo "GitHub withholds repository secrets from workflow runs triggered by a fork's pull request." + } >> "$GITHUB_STEP_SUMMARY" fi - uses: actions/checkout@v4 diff --git a/.github/workflows/unity-tests.yml b/.github/workflows/unity-tests.yml index 05b337280..9a5462c15 100644 --- a/.github/workflows/unity-tests.yml +++ b/.github/workflows/unity-tests.yml @@ -25,9 +25,14 @@ on: # Same-repo PRs get a unity-tests status check on every open / push via this trigger # (mirrors python-tests.yml). Fork PRs ALSO fire this trigger but run in the fork's # context without secrets — the detect step downstream writes unity_ok=false and the - # job exits clean with a "missing license secrets" notice so the status check still - # appears. Maintainers apply 'safe-to-test' to invoke pull_request_target below for - # a real fork-PR test run. + # job reports a green check having compiled and tested nothing. That skip is stated + # loudly in the job's step summary so it is never mistaken for a pass. + # + # There is deliberately no pull_request_target trigger here. Running fork-authored + # C# through game-ci/unity-test-runner with UNITY_* secrets in scope is the classic + # "pwn request" shape — an [InitializeOnLoad] script in the PR is enough to read + # them. To test a fork PR, review the diff and push its branch into this repo; the + # push trigger above then runs the full suite in a genuinely trusted context. pull_request: branches: [main, beta] paths: @@ -35,20 +40,8 @@ on: - MCPForUnity/Editor/** - MCPForUnity/Runtime/** - .github/workflows/unity-tests.yml - # Fork PRs: maintainer applies the 'safe-to-test' label after reviewing - # the diff. The workflow runs with UNITY_LICENSE in scope against the - # PR's head SHA. Re-pushed commits do NOT auto-trigger — maintainer must - # remove and re-apply the label to re-run after additional review. - pull_request_target: - types: [labeled] - branches: [main, beta] - paths: - - TestProjects/UnityMCPTests/** - - MCPForUnity/Editor/** - - MCPForUnity/Runtime/** - - .github/workflows/unity-tests.yml -# Dedup runs for the same branch across push / pull_request / pull_request_target / workflow_call. +# Dedup runs for the same branch across push / pull_request / workflow_call. # Same-repo PRs would otherwise fire both push (on the branch SHA) AND pull_request (on the PR); # concurrency keeps only the newer in-flight run per branch. concurrency: @@ -61,23 +54,6 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - # Gate (mirrored by testAllModes below): - # - Always run for non-PR triggers (push / workflow_call / workflow_dispatch). - # - Fork PRs: require 'safe-to-test' to be applied (existing secret-safety gate); - # 'full-matrix' may be added on top to opt into the full 4-version matrix. - # - In-repo PRs: only re-run via pull_request_target when 'full-matrix' is the - # label that just fired (the push-event run already covered the default leg). - if: > - github.event_name != 'pull_request_target' || - ( - github.event.pull_request.head.repo.full_name != github.repository && - contains(github.event.pull_request.labels.*.name, 'safe-to-test') && - (github.event.label.name == 'safe-to-test' || github.event.label.name == 'full-matrix') - ) || - ( - github.event.pull_request.head.repo.full_name == github.repository && - github.event.label.name == 'full-matrix' - ) outputs: versions: ${{ steps.set.outputs.versions }} steps: @@ -97,12 +73,13 @@ jobs: run: | set -euo pipefail # Full matrix on: beta push, workflow_call (release pipelines), workflow_dispatch, - # or any PR (pull_request OR pull_request_target) labeled with 'full-matrix'. + # or a PR carrying the 'full-matrix' label. Note the label is only read when the + # workflow fires, so applying it to an open PR takes effect on the next push. # Default (single defaultVersion from tools/unity-versions.json) otherwise — fast PR feedback. if [[ "$EVENT_NAME" == "workflow_dispatch" ]] || \ [[ "$EVENT_NAME" == "workflow_call" ]] || \ { [[ "$EVENT_NAME" == "push" ]] && [[ "$GH_REF" == "refs/heads/beta" ]]; } || \ - { { [[ "$EVENT_NAME" == "pull_request" ]] || [[ "$EVENT_NAME" == "pull_request_target" ]]; } && [[ "$FULL_MATRIX_LABEL" == "true" ]]; }; then + { [[ "$EVENT_NAME" == "pull_request" ]] && [[ "$FULL_MATRIX_LABEL" == "true" ]]; }; then versions=$(jq -c '[.versions[].id]' tools/unity-versions.json) echo "Trigger '$EVENT_NAME' on ref '$GH_REF' (full_matrix_label=$FULL_MATRIX_LABEL) → full matrix: $versions" else @@ -117,17 +94,6 @@ jobs: runs-on: ubuntu-latest permissions: contents: read - if: > - github.event_name != 'pull_request_target' || - ( - github.event.pull_request.head.repo.full_name != github.repository && - contains(github.event.pull_request.labels.*.name, 'safe-to-test') && - (github.event.label.name == 'safe-to-test' || github.event.label.name == 'full-matrix') - ) || - ( - github.event.pull_request.head.repo.full_name == github.repository && - github.event.label.name == 'full-matrix' - ) strategy: fail-fast: false matrix: @@ -159,10 +125,23 @@ jobs: echo "unity_ok=false" >> "$GITHUB_OUTPUT" fi + # A skipped run and a real pass both report a green check, because step-level + # `if:` conditions produce step-conclusion `skipped`, which contributes nothing + # to the job conclusion. Make the difference unmissable on the run page so a + # reviewer never reads this green check as "the code compiled". - name: Skip Unity tests (missing license secrets) if: steps.detect.outputs.unity_ok != 'true' run: | - echo "Unity license secrets missing; skipping Unity tests." + echo "::warning::Unity tests SKIPPED - no license secrets in scope (normal for fork PRs). This check is NOT a pass: nothing was compiled or tested." + { + echo "## :warning: Unity tests were SKIPPED" + echo + echo "No Unity license secrets were in scope for this run, so **no C# was compiled and no test was executed**." + echo "The green check means the job exited cleanly - **not** that this code works." + echo + echo "GitHub withholds repository secrets from workflow runs triggered by a fork's pull request." + echo "To get real signal, a maintainer must run the suite against this code from a trusted context." + } >> "$GITHUB_STEP_SUMMARY" - uses: actions/cache@v4 with: @@ -218,8 +197,8 @@ jobs: fi python3 - "$RESULTS_XML" <<'PY' import sys, xml.etree.ElementTree as ET - # Escape workflow-command payloads so test-controlled XML (under pull_request_target this - # is fork-supplied) can't break annotation rendering or inject extra workflow commands. + # Escape workflow-command payloads so test-controlled XML can't break annotation + # rendering or inject extra workflow commands. # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions def esc_data(s): return s.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") diff --git a/MCPForUnity/Editor/Services/EditorStateCache.cs b/MCPForUnity/Editor/Services/EditorStateCache.cs index 54625c2e4..d02b26528 100644 --- a/MCPForUnity/Editor/Services/EditorStateCache.cs +++ b/MCPForUnity/Editor/Services/EditorStateCache.cs @@ -260,8 +260,8 @@ static EditorStateCache() EditorApplication.playModeStateChanged += _ => ForceUpdate("playmode"); // Tracks whether an assembly compilation is actually running, for - // GetActualIsCompiling's Play-mode check. Statics reset on domain reload - // and this [InitializeOnLoad] ctor re-subscribes, so the flag is per-domain. + // GetActualIsCompiling. Statics reset on domain reload and this + // [InitializeOnLoad] ctor re-subscribes, so the flag is per-domain. UnityEditor.Compilation.CompilationPipeline.compilationStarted += _ => _pipelineCompilationRunning = true; UnityEditor.Compilation.CompilationPipeline.compilationFinished += _ => _pipelineCompilationRunning = false; @@ -288,7 +288,7 @@ private static void OnUpdate() { // Throttle to reduce overhead while keeping the snapshot fresh enough for polling clients. double now = EditorApplication.timeSinceStartup; - // Use GetActualIsCompiling() to avoid Play mode false positives (issue #582) + // Use GetActualIsCompiling() to avoid isCompiling false positives (issues #549, #1276) bool isCompiling = GetActualIsCompiling(); // Check for compilation edge transitions (always update on these) @@ -543,10 +543,12 @@ public static JObject GetSnapshot() private static bool _pipelineCompilationRunning; /// - /// Returns the actual compilation state, working around a known Unity quirk where - /// EditorApplication.isCompiling can return false positives in Play mode (e.g. a - /// recompile deferred by Recompile-After-Finished-Playing keeps it true for the - /// whole play session). See: https://github.com/CoplayDev/unity-mcp/issues/549 + /// Returns the actual compilation state, working around known Unity quirks where + /// EditorApplication.isCompiling reports false positives while no compilation is + /// running: a recompile deferred by Recompile-After-Finished-Playing keeps it true + /// for the whole play session (issue #549), and a project holding + /// EditorApplication.LockReloadAssemblies keeps it true until the lock is released + /// (issue #1276). In both cases the event-tracked pipeline flag is authoritative. /// internal static bool GetActualIsCompiling() { @@ -556,16 +558,9 @@ internal static bool GetActualIsCompiling() return false; } - // In Play mode, trust the event-tracked pipeline state instead: a deferred - // recompile keeps EditorApplication.isCompiling true without any compilation - // actually running. - if (EditorApplication.isPlaying) - { - return _pipelineCompilationRunning; - } - - // Outside Play mode the raw signal is reliable. - return true; + // Otherwise trust the event-tracked pipeline state: isCompiling stays true for as + // long as an assembly reload is deferred, with no compilation actually running. + return _pipelineCompilationRunning; } } } diff --git a/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs b/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs index 764c82f6d..0d2c23616 100644 --- a/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs +++ b/MCPForUnity/Editor/Services/Server/TerminalLauncher.cs @@ -42,10 +42,13 @@ public System.Diagnostics.ProcessStartInfo CreateHeadlessProcessStartInfo(string } #if UNITY_EDITOR_WIN - // cmd.exe /c " >> "" 2>&1" + // cmd.exe /c " < NUL >> "" 2>&1" // The whole payload after /c is wrapped in one outer pair of quotes; cmd strips the // outermost quotes, so inner quotes around the log path survive for paths with spaces. - string winRedirect = $"{command} >> \"{logFilePath}\" 2>&1"; + // stdin is redirected from NUL because the Editor is a console-less GUI process: with + // CreateNoWindow and no console handle, uvx.exe would inherit an invalid stdin and die + // with "The handle is invalid. (os error 6)" before launching the server. + string winRedirect = $"{command} < NUL >> \"{logFilePath}\" 2>&1"; return new System.Diagnostics.ProcessStartInfo { FileName = "cmd.exe", diff --git a/MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs b/MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs index cfa4d7286..4d9bb1ab1 100644 --- a/MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs +++ b/MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs @@ -114,14 +114,8 @@ private static void OnAfterAssemblyReload() } // If the editor is not compiling, attempt an immediate restart without relying on editor focus. - bool isCompiling = EditorApplication.isCompiling; - try - { - var pipeline = Type.GetType("UnityEditor.Compilation.CompilationPipeline, UnityEditor"); - var prop = pipeline?.GetProperty("isCompiling", System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static); - if (prop != null) isCompiling |= (bool)prop.GetValue(null); - } - catch { } + // Routed through EditorStateCache so a deferred reload (issue #1276) does not block resume. + bool isCompiling = EditorStateCache.GetActualIsCompiling(); if (!isCompiling) { diff --git a/MCPForUnity/Editor/Services/TestJobManager.cs b/MCPForUnity/Editor/Services/TestJobManager.cs index d162476e8..bdf626036 100644 --- a/MCPForUnity/Editor/Services/TestJobManager.cs +++ b/MCPForUnity/Editor/Services/TestJobManager.cs @@ -502,7 +502,7 @@ internal static TestJob GetJob(string jobId) { long now = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); long initTimeout = job.InitTimeoutMs > 0 ? job.InitTimeoutMs : DefaultInitializationTimeoutMs; - if (!EditorApplication.isCompiling && !EditorApplication.isUpdating && now - job.StartedUnixMs > initTimeout) + if (!EditorStateCache.GetActualIsCompiling() && !EditorApplication.isUpdating && now - job.StartedUnixMs > initTimeout) { McpLog.Warn($"[TestJobManager] Job {jobId} failed to initialize within {initTimeout}ms, auto-failing"); job.Status = TestJobStatus.Failed; @@ -589,7 +589,7 @@ private static string GetBlockedReason(TestJob job) return "editor_unfocused"; } - if (EditorApplication.isCompiling) + if (EditorStateCache.GetActualIsCompiling()) { return "compiling"; } diff --git a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs index c4fb2438d..a6a0559d0 100644 --- a/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs +++ b/MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs @@ -237,24 +237,10 @@ private static void EnsureStartedOnEditorIdle() } } - private static bool IsCompiling() - { - if (EditorApplication.isCompiling) - { - return true; - } - try - { - Type pipeline = Type.GetType("UnityEditor.Compilation.CompilationPipeline, UnityEditor"); - var prop = pipeline?.GetProperty("isCompiling", System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Static); - if (prop != null) - { - return (bool)prop.GetValue(null); - } - } - catch { } - return false; - } + // Routed through EditorStateCache so a deferred domain reload (issue #1276) does not + // pin the bridge off: raw EditorApplication.isCompiling stays true for as long as the + // reload is held, and this gates bridge startup. + private static bool IsCompiling() => EditorStateCache.GetActualIsCompiling(); public static void Start() { diff --git a/MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs b/MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs index 1e7063ae6..1ffa16bf7 100644 --- a/MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs +++ b/MCPForUnity/Editor/Tools/GameObjects/GameObjectComponentHelpers.cs @@ -143,6 +143,79 @@ internal static object RemoveComponentInternal(GameObject targetGo, string typeN } } + /// + /// Applies a "componentProperties" object (as accepted by 'modify') to every named + /// component already present on . Shared by 'create' and + /// 'modify' so the argument behaves identically on both actions. + /// + /// Set to true if at least one property was set successfully. + /// An ErrorResponse aggregating any per-component failures, or null if all + /// (or none) of the requested properties were applied successfully. + internal static object ApplyComponentProperties(GameObject targetGo, JObject componentPropertiesObj, out bool modified) + { + modified = false; + if (componentPropertiesObj == null) + { + return null; + } + + var componentErrors = new List(); + foreach (var prop in componentPropertiesObj.Properties()) + { + string compName = prop.Name; + JObject propertiesToSet = prop.Value as JObject; + if (propertiesToSet != null) + { + var setResult = SetComponentPropertiesInternal(targetGo, compName, propertiesToSet); + if (setResult != null) + { + componentErrors.Add(setResult); + } + else + { + modified = true; + } + } + } + + if (componentErrors.Count == 0) + { + return null; + } + + var aggregatedErrors = new List(); + foreach (var errorObj in componentErrors) + { + try + { + var dataProp = errorObj?.GetType().GetProperty("data"); + var dataVal = dataProp?.GetValue(errorObj); + if (dataVal != null) + { + var errorsProp = dataVal.GetType().GetProperty("errors"); + var errorsEnum = errorsProp?.GetValue(dataVal) as System.Collections.IEnumerable; + if (errorsEnum != null) + { + foreach (var item in errorsEnum) + { + var s = item?.ToString(); + if (!string.IsNullOrEmpty(s)) aggregatedErrors.Add(s); + } + } + } + } + catch (Exception ex) + { + McpLog.Warn($"[ManageGameObject] Error aggregating component errors: {ex.Message}"); + } + } + + return new ErrorResponse( + $"One or more component property operations failed on '{targetGo.name}'.", + new { componentErrors = componentErrors, errors = aggregatedErrors } + ); + } + internal static object SetComponentPropertiesInternal(GameObject targetGo, string componentTypeName, JObject properties, Component targetComponentInstance = null) { Component targetComponent = targetComponentInstance; diff --git a/MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs b/MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs index 6954a26ba..d1c963241 100644 --- a/MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs +++ b/MCPForUnity/Editor/Tools/GameObjects/GameObjectCreate.cs @@ -269,6 +269,16 @@ internal static object Handle(JObject @params) } } + // Set Component Properties (same "componentProperties" argument 'modify' consumes, + // applied here so it also works at creation time) + var componentPropertiesError = GameObjectComponentHelpers.ApplyComponentProperties( + newGo, @params["componentProperties"] as JObject, out _); + if (componentPropertiesError != null) + { + UnityEngine.Object.DestroyImmediate(newGo); + return componentPropertiesError; + } + // Save as Prefab ONLY if we *created* a new object AND saveAsPrefab is true GameObject finalInstance = newGo; if (createdNewObject && saveAsPrefab) diff --git a/MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.cs b/MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.cs index 0ec608812..2bfce076c 100644 --- a/MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.cs +++ b/MCPForUnity/Editor/Tools/GameObjects/GameObjectModify.cs @@ -1,6 +1,5 @@ #nullable disable using System; -using System.Collections.Generic; using System.Linq; using MCPForUnity.Editor.Helpers; using Newtonsoft.Json.Linq; @@ -223,61 +222,15 @@ internal static object Handle(JObject @params, JToken targetToken, string search } } - var componentErrors = new List(); - if (@params["componentProperties"] is JObject componentPropertiesObj) + var componentPropertiesError = GameObjectComponentHelpers.ApplyComponentProperties( + targetGo, @params["componentProperties"] as JObject, out bool componentPropertiesModified); + if (componentPropertiesError != null) { - foreach (var prop in componentPropertiesObj.Properties()) - { - string compName = prop.Name; - JObject propertiesToSet = prop.Value as JObject; - if (propertiesToSet != null) - { - var setResult = GameObjectComponentHelpers.SetComponentPropertiesInternal(targetGo, compName, propertiesToSet); - if (setResult != null) - { - componentErrors.Add(setResult); - } - else - { - modified = true; - } - } - } + return componentPropertiesError; } - - if (componentErrors.Count > 0) + if (componentPropertiesModified) { - var aggregatedErrors = new List(); - foreach (var errorObj in componentErrors) - { - try - { - var dataProp = errorObj?.GetType().GetProperty("data"); - var dataVal = dataProp?.GetValue(errorObj); - if (dataVal != null) - { - var errorsProp = dataVal.GetType().GetProperty("errors"); - var errorsEnum = errorsProp?.GetValue(dataVal) as System.Collections.IEnumerable; - if (errorsEnum != null) - { - foreach (var item in errorsEnum) - { - var s = item?.ToString(); - if (!string.IsNullOrEmpty(s)) aggregatedErrors.Add(s); - } - } - } - } - catch (Exception ex) - { - McpLog.Warn($"[GameObjectModify] Error aggregating component errors: {ex.Message}"); - } - } - - return new ErrorResponse( - $"One or more component property operations failed on '{targetGo.name}'.", - new { componentErrors = componentErrors, errors = aggregatedErrors } - ); + modified = true; } if (!modified) diff --git a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs index c508e9fc2..2ddd021c3 100644 --- a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs +++ b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Text.RegularExpressions; using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Services; using Newtonsoft.Json.Linq; using UnityEditor; using UnityEngine; @@ -44,7 +45,7 @@ public static object HandleCommand(JObject @params) return new ErrorResponse(CodeInvalidParams); } - if (EditorApplication.isCompiling || EditorApplication.isUpdating) + if (EditorStateCache.GetActualIsCompiling() || EditorApplication.isUpdating) { // Unity is transient; treat as retryable on the client side. return new ErrorResponse(CodeCompilingOrReloading, new { hint = "retry" }); diff --git a/MCPForUnity/Editor/Tools/RefreshUnity.cs b/MCPForUnity/Editor/Tools/RefreshUnity.cs index 537472ac0..e35237d80 100644 --- a/MCPForUnity/Editor/Tools/RefreshUnity.cs +++ b/MCPForUnity/Editor/Tools/RefreshUnity.cs @@ -109,7 +109,7 @@ await WaitForUnityReadyAsync( } } - string resultingState = EditorApplication.isCompiling + string resultingState = EditorStateCache.GetActualIsCompiling() ? "compiling" : (EditorApplication.isUpdating ? "asset_import" : "idle"); @@ -120,7 +120,7 @@ await WaitForUnityReadyAsync( resulting_state = resultingState, hint = shouldWaitForReady ? "Unity refresh completed; editor should be ready." - : "If Unity enters compilation/domain reload, poll editor_state until ready_for_tools is true." + : "If Unity enters compilation/domain reload, poll the mcpforunity://editor/state resource until data.advice.ready_for_tools is true." }); } @@ -146,7 +146,7 @@ void Tick() return; } - if (!EditorApplication.isCompiling + if (!EditorStateCache.GetActualIsCompiling() && !EditorApplication.isUpdating && !TestRunStatus.IsRunning && !EditorApplication.isPlayingOrWillChangePlaymode) diff --git a/MCPForUnity/Editor/Tools/UnityReflect.cs b/MCPForUnity/Editor/Tools/UnityReflect.cs index 06aee0d7f..236b74342 100644 --- a/MCPForUnity/Editor/Tools/UnityReflect.cs +++ b/MCPForUnity/Editor/Tools/UnityReflect.cs @@ -6,6 +6,7 @@ using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Services; using MCPForUnity.Runtime.Helpers; using Newtonsoft.Json.Linq; using UnityEditor; @@ -99,7 +100,7 @@ private static Dictionary GetAssemblyTypeCache() public static object HandleCommand(JObject @params) { - if (EditorApplication.isCompiling) + if (EditorStateCache.GetActualIsCompiling()) return new ErrorResponse("Cannot reflect while Unity is compiling. Wait for domain reload to complete."); if (@params == null) diff --git a/MCPForUnity/package.json b/MCPForUnity/package.json index 15bd9f5ff..9b25f2333 100644 --- a/MCPForUnity/package.json +++ b/MCPForUnity/package.json @@ -1,6 +1,6 @@ { "name": "com.coplaydev.unity-mcp", - "version": "10.1.0", + "version": "10.1.2", "displayName": "MCP for Unity", "description": "A bridge that connects AI assistants to Unity via the MCP (Model Context Protocol). Allows AI clients like Claude Code, Cursor, and VSCode to directly control your Unity Editor for enhanced development workflows.\n\nFeatures automated setup wizard, cross-platform support, and seamless integration with popular AI development tools.\n\nJoin Our Discord: https://discord.gg/y4p8KfzrN4", "unity": "2021.3", diff --git a/Server/README.md b/Server/README.md index 4637becc5..3761a12b4 100644 --- a/Server/README.md +++ b/Server/README.md @@ -71,7 +71,7 @@ Use this to run the latest released version from the repository. Change the vers "command": "uvx", "args": [ "--from", - "git+https://github.com/CoplayDev/unity-mcp@v10.1.0#subdirectory=Server", + "git+https://github.com/CoplayDev/unity-mcp@v10.1.2#subdirectory=Server", "mcp-for-unity", "--transport", "stdio" diff --git a/Server/pyproject.toml b/Server/pyproject.toml index a17976376..759d3d058 100644 --- a/Server/pyproject.toml +++ b/Server/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcpforunityserver" -version = "10.1.0" +version = "10.1.2" description = "MCP for Unity Server: A Unity package for Unity Editor integration via the Model Context Protocol (MCP)." readme = "README.md" license = "MIT" diff --git a/Server/src/cli/commands/camera.py b/Server/src/cli/commands/camera.py index 5f4f82718..e2d7f98c2 100644 --- a/Server/src/cli/commands/camera.py +++ b/Server/src/cli/commands/camera.py @@ -51,8 +51,8 @@ def ping(): unity-mcp camera ping """ config = get_config() - result = run_command(config, "manage_camera", {"action": "ping"}) - format_output(result, config) + result = run_command("manage_camera", {"action": "ping"}, config) + click.echo(format_output(result, config.format)) @camera.command("list") @@ -65,8 +65,8 @@ def list_cameras(): unity-mcp camera list """ config = get_config() - result = run_command(config, "manage_camera", {"action": "list_cameras"}) - format_output(result, config) + result = run_command("manage_camera", {"action": "list_cameras"}, config) + click.echo(format_output(result, config.format)) @camera.command("brain-status") @@ -79,8 +79,8 @@ def brain_status(): unity-mcp camera brain-status """ config = get_config() - result = run_command(config, "manage_camera", {"action": "get_brain_status"}) - format_output(result, config) + result = run_command("manage_camera", {"action": "get_brain_status"}, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -125,8 +125,8 @@ def create(name, preset, follow, look_at, priority, fov): if props: params["properties"] = props - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("ensure-brain") @@ -155,8 +155,8 @@ def ensure_brain(camera_ref, blend_style, blend_duration): if props: params["properties"] = props - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -189,8 +189,8 @@ def set_target(target, search_method, follow, look_at): "searchMethod": search_method, "properties": props if props else None, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("set-lens") @@ -228,8 +228,8 @@ def set_lens(target, search_method, fov, near, far, ortho_size, dutch): "searchMethod": search_method, "properties": props if props else None, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("set-priority") @@ -251,8 +251,8 @@ def set_priority(target, search_method, priority): "searchMethod": search_method, "properties": {"priority": priority}, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -286,8 +286,8 @@ def set_body(target, search_method, body_type, props): "searchMethod": search_method, "properties": properties if properties else None, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("set-aim") @@ -316,8 +316,8 @@ def set_aim(target, search_method, aim_type, props): "searchMethod": search_method, "properties": properties if properties else None, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("set-noise") @@ -346,8 +346,8 @@ def set_noise(target, search_method, amplitude, frequency): "searchMethod": search_method, "properties": props if props else None, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -379,8 +379,8 @@ def add_extension(target, extension_type, search_method, props): "searchMethod": search_method, "properties": properties, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("remove-extension") @@ -402,8 +402,8 @@ def remove_extension(target, extension_type, search_method): "searchMethod": search_method, "properties": {"extensionType": extension_type}, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -432,8 +432,8 @@ def set_blend(style, duration): if props: params["properties"] = props - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("force") @@ -453,8 +453,8 @@ def force_camera(target, search_method): "target": target, "searchMethod": search_method, }) - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("release") @@ -467,8 +467,8 @@ def release_override(): unity-mcp camera release """ config = get_config() - result = run_command(config, "manage_camera", {"action": "release_override"}) - format_output(result, config) + result = run_command("manage_camera", {"action": "release_override"}, config) + click.echo(format_output(result, config.format)) # ============================================================================= @@ -523,8 +523,8 @@ def screenshot(camera_ref, file_name, super_size, include_image, max_resolution, params["viewTarget"] = view_target if output_folder: params["outputFolder"] = output_folder - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) @camera.command("screenshot-multiview") @@ -550,5 +550,5 @@ def screenshot_multiview(max_resolution, view_target, output_folder): params["viewTarget"] = view_target if output_folder: params["outputFolder"] = output_folder - result = run_command(config, "manage_camera", params) - format_output(result, config) + result = run_command("manage_camera", params, config) + click.echo(format_output(result, config.format)) diff --git a/Server/src/cli/commands/editor.py b/Server/src/cli/commands/editor.py index 8b7746657..5b0dce795 100644 --- a/Server/src/cli/commands/editor.py +++ b/Server/src/cli/commands/editor.py @@ -325,8 +325,13 @@ def execute_menu(menu_path: str): is_flag=True, help="Include details for failed/skipped tests only." ) +@click.option( + "--clear-stuck", + is_flag=True, + help="Clear an orphaned running job that is blocking new runs, instead of starting a run." +) @handle_unity_errors -def run_tests(mode: str, async_mode: bool, wait: Optional[int], details: bool, failed_only: bool): +def run_tests(mode: str, async_mode: bool, wait: Optional[int], details: bool, failed_only: bool, clear_stuck: bool): """Run Unity tests. \b @@ -335,9 +340,15 @@ def run_tests(mode: str, async_mode: bool, wait: Optional[int], details: bool, f unity-mcp editor tests --mode PlayMode unity-mcp editor tests --async unity-mcp editor tests --wait 60 --failed-only + unity-mcp editor tests --clear-stuck """ config = get_config() + if clear_stuck: + result = run_command("run_tests", {"clear_stuck": True}, config) + click.echo(format_output(result, config.format)) + return + params: dict[str, Any] = {"mode": mode} if wait is not None: params["wait_timeout"] = wait diff --git a/Server/src/main.py b/Server/src/main.py index 7bbb57afd..a3f20cbee 100644 --- a/Server/src/main.py +++ b/Server/src/main.py @@ -311,19 +311,19 @@ def _build_instructions(project_scoped_tools: bool) -> str: Important Workflows: Resources vs Tools: -- Use RESOURCES to read editor state (editor_state, project_info, project_tags, tests, etc) +- Use RESOURCES to read editor state (mcpforunity://editor/state, mcpforunity://project/info, mcpforunity://project/tags, mcpforunity://tests, etc) - Use TOOLS to perform actions and mutations (manage_editor for play mode control, tag/layer management, etc) - Always check related resources before modifying the engine state with tools Reading resources (read this before using ANY resource named below): - Resources are addressed by URI, never by name. A resource's name and URI are NOT interchangeable: names use underscores (e.g. editor_state) while URIs use slashes (e.g. mcpforunity://editor/state). Do NOT build a URI by swapping separators in the name — you will 404. -- Always read the exact URI from your MCP client's resource listing (resources/list). Where these instructions mention a resource by name, look up its URI in that listing rather than guessing it. +- These instructions always spell resources as full mcpforunity:// URIs — read one exactly as written. If you only have a name (from resources/list or another tool's output), look its URI up in resources/list rather than guessing it. - Resource payloads are wrapped: the content lives under a top-level `data` object, so field paths are `data.
.` (e.g. `data.advice.ready_for_tools`), not bare top-level fields. Script Management: - After creating or modifying scripts (by your own tools or the `manage_script` tool) use `read_console` to check for compilation errors before proceeding - Only after successful compilation can new components/types be used -- You can poll the `editor_state` resource's `isCompiling` field to check if the domain reload is complete +- You can poll mcpforunity://editor/state and read `data.compilation.is_compiling` to check if the domain reload is complete, or `data.advice.ready_for_tools` for overall readiness Scene Setup: - Always include a Camera and main Light (Directional Light) in new scenes @@ -339,7 +339,7 @@ def _build_instructions(project_scoped_tools: bool) -> str: - Filter by log type (Error, Warning, Log) to focus on specific issues Menu Items: -- Use `execute_menu_item` when you have read the menu items resource +- Use `execute_menu_item` when you have read the mcpforunity://menu-items resource - This lets you interact with Unity's menu system and third-party tools Unity API Verification (requires 'docs' tool group): diff --git a/Server/src/services/tools/debug_request_context.py b/Server/src/services/tools/debug_request_context.py index f997a42f2..297fb1ead 100644 --- a/Server/src/services/tools/debug_request_context.py +++ b/Server/src/services/tools/debug_request_context.py @@ -19,6 +19,9 @@ annotations=ToolAnnotations( title="Debug Request Context", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def debug_request_context(ctx: Context) -> dict[str, Any]: diff --git a/Server/src/services/tools/find_gameobjects.py b/Server/src/services/tools/find_gameobjects.py index d1e9af015..606a45a12 100644 --- a/Server/src/services/tools/find_gameobjects.py +++ b/Server/src/services/tools/find_gameobjects.py @@ -5,6 +5,7 @@ from typing import Annotated, Any, Literal from fastmcp import Context +from mcp.types import ToolAnnotations from pydantic import Field from services.registry import mcp_for_unity_tool from services.tools import get_unity_instance_from_context @@ -21,7 +22,16 @@ "Then use mcpforunity://scene/gameobject/{id} resource for full data, " "or mcpforunity://scene/gameobject/{id}/components for component details. " "For CRUD operations (create/modify/delete), use manage_gameobject instead." - ) + ), + annotations=ToolAnnotations( + title="Find GameObjects", + # Not readOnly: preflight(refresh_if_dirty=True) below can trigger an + # asset refresh and domain reload. + readOnlyHint=False, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ) async def find_gameobjects( ctx: Context, diff --git a/Server/src/services/tools/find_in_file.py b/Server/src/services/tools/find_in_file.py index 97d0ef1db..362440b0d 100644 --- a/Server/src/services/tools/find_in_file.py +++ b/Server/src/services/tools/find_in_file.py @@ -71,6 +71,9 @@ def _split_uri(uri: str) -> tuple[str, str]: annotations=ToolAnnotations( title="Find in File", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def find_in_file( diff --git a/Server/src/services/tools/manage_components.py b/Server/src/services/tools/manage_components.py index da2cdba18..8cddd1987 100644 --- a/Server/src/services/tools/manage_components.py +++ b/Server/src/services/tools/manage_components.py @@ -5,6 +5,7 @@ from typing import Annotated, Any, Literal, Optional from fastmcp import Context +from mcp.types import ToolAnnotations from services.registry import mcp_for_unity_tool from services.tools import get_unity_instance_from_context from transport.unity_transport import send_with_unity_instance @@ -20,7 +21,12 @@ "For READING component data, use the mcpforunity://scene/gameobject/{id}/components resource " "or mcpforunity://scene/gameobject/{id}/component/{name} for a single component. " "For creating/deleting GameObjects themselves, use manage_gameobject instead." - ) + ), + annotations=ToolAnnotations( + title="Manage Components", + readOnlyHint=False, + destructiveHint=True, + ), ) async def manage_components( ctx: Context, diff --git a/Server/src/services/tools/manage_editor.py b/Server/src/services/tools/manage_editor.py index 31142073b..ef8fc667f 100644 --- a/Server/src/services/tools/manage_editor.py +++ b/Server/src/services/tools/manage_editor.py @@ -13,6 +13,8 @@ description="Controls and queries the Unity editor's state and settings. Read-only actions: telemetry_status, telemetry_ping. Modifying actions: play, pause, stop, set_active_tool, add_tag, remove_tag, add_layer, remove_layer, deploy_package, restore_package, undo, redo. For prefab editing (open/save/close prefab stage), use manage_prefabs. deploy_package copies the configured MCPForUnity source folder into the project's installed package location (triggers recompile, no confirmation dialog). restore_package reverts to the pre-deployment backup. undo/redo perform Unity editor undo/redo and return the affected group name.", annotations=ToolAnnotations( title="Manage Editor", + readOnlyHint=False, + destructiveHint=True, ), ) async def manage_editor( diff --git a/Server/src/services/tools/manage_gameobject.py b/Server/src/services/tools/manage_gameobject.py index 9d6e8a139..6ddd67f19 100644 --- a/Server/src/services/tools/manage_gameobject.py +++ b/Server/src/services/tools/manage_gameobject.py @@ -11,6 +11,88 @@ from services.tools.preflight import preflight +def _normalize_components_to_add(value: Any) -> tuple[list[str | dict[str, Any]] | None, str | None]: + """ + Normalize components_to_add, accepting both plain type-name strings and + {"typeName": ..., "properties": {...}} objects for setting initial component + properties at creation time (matching what the C# 'create' handler reads out + of each componentsToAdd entry). + + Handles various input formats from MCP clients/LLMs: + - None -> (None, None) + - list of strings and/or {"typeName": str, "properties": dict} objects -> validated list + - a single object entry -> wrapped in a list + - JSON string encoding either form -> parsed and normalized + - Plain non-JSON string "foo" -> treated as ["foo"] + + Returns: + Tuple of (parsed_list, error_message). If error_message is set, parsed_list is None. + """ + def _validate_items(items: list[Any]) -> tuple[list[str | dict[str, Any]] | None, str | None]: + normalized: list[str | dict[str, Any]] = [] + for item in items: + if isinstance(item, str): + normalized.append(item) + continue + if isinstance(item, dict): + type_name = item.get("typeName") or item.get("type_name") + if not isinstance(type_name, str) or not type_name: + return None, ( + "components_to_add object entries must include a string 'typeName', " + f"got: {item}" + ) + entry: dict[str, Any] = {"typeName": type_name} + properties = item.get("properties") + if properties is not None: + if not isinstance(properties, dict): + return None, ( + f"components_to_add entry for '{type_name}' has a non-object " + f"'properties': {properties}" + ) + entry["properties"] = properties + normalized.append(entry) + continue + return None, f"components_to_add entries must be strings or objects, got: {item!r}" + return normalized, None + + if value is None: + return None, None + + if isinstance(value, (list, tuple)): + return _validate_items(list(value)) + + if isinstance(value, dict): + # A single {"typeName": ..., "properties": ...} entry without list wrapping. + return _validate_items([value]) + + if isinstance(value, str): + val_trimmed = value.strip() + if val_trimmed in ("[object Object]", "undefined", "null", ""): + return None, ( + f"components_to_add received invalid value: '{value}'. Expected a JSON array " + 'like ["Item1", {"typeName": "Item2", "properties": {...}}]' + ) + + looks_like_json = val_trimmed.startswith("[") or val_trimmed.startswith("{") + parsed = parse_json_payload(value) + if isinstance(parsed, list): + return _validate_items(parsed) + if isinstance(parsed, dict): + return _validate_items([parsed]) + if parsed == value and looks_like_json: + return None, ( + f"components_to_add has invalid JSON syntax: '{value}'. Expected a valid JSON " + 'array like ["item1", "item2"]' + ) + if parsed == value: + # Treat as single-element list + return [value], None + + return None, f"components_to_add must be a JSON array (list), got string that parsed to {type(parsed).__name__}" + + return None, f"components_to_add must be a list, object, or JSON string, got {type(value).__name__}" + + def _normalize_component_properties(value: Any) -> tuple[dict[str, dict[str, Any]] | None, str | None]: """ Robustly normalize component_properties to a dict. @@ -74,8 +156,12 @@ async def manage_gameobject( "Rotation as [x, y, z] euler angles array, {x, y, z} object, or JSON string"] | None = None, scale: Annotated[list[float] | dict[str, float] | str, "Scale as [x, y, z] array, {x, y, z} object, or JSON string"] | None = None, - components_to_add: Annotated[list[str] | str, - "List of component names to add during 'create' or 'modify'"] | None = None, + components_to_add: Annotated[list[str | dict[str, Any]] | dict[str, Any] | str, + """List of components to add during 'create' or 'modify'. Each entry is either + a plain type name string (e.g. "BoxCollider") or an object + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} that adds the + component with initial properties applied in the same call. Mixing both forms + in one list is fine."""] | None = None, primitive_type: Annotated[str, "Primitive type for 'create' action"] | None = None, save_as_prefab: Annotated[bool | str, @@ -92,7 +178,9 @@ async def manage_gameobject( components_to_remove: Annotated[list[str] | str, "List of component names to remove"] | None = None, component_properties: Annotated[dict[str, dict[str, Any]] | str, - """Dictionary of component names to their properties to set. For example: + """Dictionary of component names to their properties to set. Works for both + 'create' (applied to components already present on the new GameObject - add + them via components_to_add first) and 'modify'. For example: `{"MyScript": {"otherObject": {"find": "Player", "method": "by_name"}}}` assigns GameObject `{"MyScript": {"playerHealth": {"find": "Player", "component": "HealthComponent"}}}` assigns Component Example set nested property: @@ -158,7 +246,7 @@ async def manage_gameobject( return {"success": False, "message": comp_props_error} # --- Normalize components_to_add and components_to_remove --- - components_to_add, add_error = normalize_string_list(components_to_add, "components_to_add") + components_to_add, add_error = _normalize_components_to_add(components_to_add) if add_error: return {"success": False, "message": add_error} diff --git a/Server/src/services/tools/manage_script.py b/Server/src/services/tools/manage_script.py index 648174504..2bd62f8ea 100644 --- a/Server/src/services/tools/manage_script.py +++ b/Server/src/services/tools/manage_script.py @@ -480,6 +480,9 @@ async def _verify_delete(): annotations=ToolAnnotations( title="Validate Script", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def validate_script( @@ -626,6 +629,9 @@ async def _verify_mutation(): annotations=ToolAnnotations( title="Manage Script Capabilities", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def manage_script_capabilities(ctx: Context) -> dict[str, Any]: @@ -658,6 +664,9 @@ async def manage_script_capabilities(ctx: Context) -> dict[str, Any]: annotations=ToolAnnotations( title="Get SHA", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def get_sha( diff --git a/Server/src/services/tools/manage_tools.py b/Server/src/services/tools/manage_tools.py index 8160aba46..b8875e4d5 100644 --- a/Server/src/services/tools/manage_tools.py +++ b/Server/src/services/tools/manage_tools.py @@ -36,6 +36,9 @@ annotations=ToolAnnotations( title="Manage Tools", readOnlyHint=False, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def manage_tools( diff --git a/Server/src/services/tools/read_console.py b/Server/src/services/tools/read_console.py index 54c933ecc..6a642dcec 100644 --- a/Server/src/services/tools/read_console.py +++ b/Server/src/services/tools/read_console.py @@ -24,6 +24,12 @@ def _strip_stacktrace_from_list(items: list) -> None: description="Gets messages from or clears the Unity Editor console. Defaults to 10 most recent entries. Use page_size/cursor for paging. Note: For maximum client compatibility, pass count as a quoted string (e.g., '5'). The 'get' action is read-only; 'clear' modifies ephemeral UI state (not project data).", annotations=ToolAnnotations( title="Read Console", + # 'clear' wipes the ephemeral Editor console buffer only — Unity still + # mirrors every entry to the Editor log file — so nothing is destroyed. + readOnlyHint=False, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def read_console( diff --git a/Server/src/services/tools/refresh_unity.py b/Server/src/services/tools/refresh_unity.py index bc147dc09..2c92831b9 100644 --- a/Server/src/services/tools/refresh_unity.py +++ b/Server/src/services/tools/refresh_unity.py @@ -180,7 +180,7 @@ async def refresh_unity( compile: Annotated[Literal["none", "request"], "Whether to request compilation"] = "none", wait_for_ready: Annotated[bool, - "If true, wait until editor_state.advice.ready_for_tools is true"] = True, + "If true, wait until mcpforunity://editor/state reports data.advice.ready_for_tools true"] = True, ) -> MCPResponse | dict[str, Any]: unity_instance = await get_unity_instance_from_context(ctx) diff --git a/Server/src/services/tools/run_tests.py b/Server/src/services/tools/run_tests.py index 0426e63b5..9e94db32d 100644 --- a/Server/src/services/tools/run_tests.py +++ b/Server/src/services/tools/run_tests.py @@ -170,12 +170,29 @@ async def run_tests( init_timeout: Annotated[int | None, "Initialization timeout in milliseconds. PlayMode tests may need longer " "due to domain reload (default: 15000). Recommended: 120000 for PlayMode."] = None, + clear_stuck: Annotated[bool, + "Clear an orphaned running job instead of starting a run. Use when a job " + "was lost to a domain reload and is blocking every subsequent run."] = False, ) -> RunTestsStartResponse | MCPResponse: + unity_instance = await get_unity_instance_from_context(ctx) + + # Runs before both the init_timeout check and preflight on purpose: neither is relevant to + # clearing, and requires_no_tests would reject the very call that exists to clear the + # orphaned job blocking it. + if clear_stuck: + response = await unity_transport.send_with_unity_instance( + async_send_command_with_retry, + unity_instance, + "run_tests", + {"clear_stuck": True}, + ) + if isinstance(response, dict): + return MCPResponse(**response) + return MCPResponse(success=False, error=str(response)) + if init_timeout is not None and init_timeout <= 0: return MCPResponse(success=False, error="init_timeout must be a positive integer (milliseconds) or None") - unity_instance = await get_unity_instance_from_context(ctx) - gate = await preflight(ctx, requires_no_tests=True, wait_for_no_compile=True, refresh_if_dirty=True) if isinstance(gate, MCPResponse): return gate @@ -226,6 +243,9 @@ def _coerce_string_list(value) -> list[str] | None: annotations=ToolAnnotations( title="Get Test Job", readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def get_test_job( diff --git a/Server/src/services/tools/set_active_instance.py b/Server/src/services/tools/set_active_instance.py index 586724f09..b11530d48 100644 --- a/Server/src/services/tools/set_active_instance.py +++ b/Server/src/services/tools/set_active_instance.py @@ -17,6 +17,11 @@ description="Set the active Unity instance for this client/session. Accepts Name@hash, hash prefix, or port number (stdio only).", annotations=ToolAnnotations( title="Set Active Instance", + # Changes session-local routing only; touches nothing in the project. + readOnlyHint=False, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, ), ) async def set_active_instance( diff --git a/Server/tests/integration/test_run_tests_async.py b/Server/tests/integration/test_run_tests_async.py index a8098ea6c..79bd24af7 100644 --- a/Server/tests/integration/test_run_tests_async.py +++ b/Server/tests/integration/test_run_tests_async.py @@ -93,6 +93,90 @@ async def test_run_tests_rejects_zero_init_timeout(): assert "init_timeout" in resp.error +@pytest.mark.asyncio +async def test_run_tests_clear_stuck_forwards_only_the_flag(monkeypatch): + from services.tools.run_tests import run_tests + + captured = {} + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + captured["command_type"] = command_type + captured["params"] = params + return {"success": True, "message": "Stuck job cleared.", "data": {"cleared": True}} + + import services.tools.run_tests as mod + monkeypatch.setattr( + mod.unity_transport, "send_with_unity_instance", fake_send_with_unity_instance) + + resp = await run_tests(DummyContext(), clear_stuck=True) + + # C# reads @params["clear_stuck"] verbatim (RunTests.cs:23), so the key must stay snake_case. + assert captured["command_type"] == "run_tests" + assert captured["params"] == {"clear_stuck": True} + assert resp.success is True + assert resp.data == {"cleared": True} + + +@pytest.mark.asyncio +async def test_run_tests_clear_stuck_bypasses_preflight(monkeypatch): + """#1272: preflight(requires_no_tests=True) would reject the call that clears the job blocking it.""" + from services.tools.run_tests import run_tests + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + return {"success": True, "message": "Stuck job cleared.", "data": {"cleared": True}} + + async def exploding_preflight(*args, **kwargs): + raise AssertionError("clear_stuck must short-circuit before preflight") + + import services.tools.run_tests as mod + monkeypatch.setattr( + mod.unity_transport, "send_with_unity_instance", fake_send_with_unity_instance) + monkeypatch.setattr(mod, "preflight", exploding_preflight) + + resp = await run_tests(DummyContext(), clear_stuck=True) + assert resp.success is True + + +@pytest.mark.asyncio +async def test_run_tests_clear_stuck_ignores_invalid_init_timeout(monkeypatch): + """Recovery must be unconditional: an unrelated bad arg must not block clearing.""" + from services.tools.run_tests import run_tests + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + return {"success": True, "message": "Stuck job cleared.", "data": {"cleared": True}} + + import services.tools.run_tests as mod + monkeypatch.setattr( + mod.unity_transport, "send_with_unity_instance", fake_send_with_unity_instance) + + resp = await run_tests(DummyContext(), clear_stuck=True, init_timeout=0) + assert resp.success is True + + +@pytest.mark.asyncio +async def test_run_tests_without_clear_stuck_still_preflights(monkeypatch): + from services.tools.run_tests import run_tests + + calls = [] + + async def fake_send_with_unity_instance(send_fn, unity_instance, command_type, params, **kwargs): + return {"success": True, "data": {"job_id": "abc123", "status": "running", "mode": "EditMode"}} + + async def recording_preflight(*args, **kwargs): + calls.append(kwargs) + return None + + import services.tools.run_tests as mod + monkeypatch.setattr( + mod.unity_transport, "send_with_unity_instance", fake_send_with_unity_instance) + monkeypatch.setattr(mod, "preflight", recording_preflight) + + resp = await run_tests(DummyContext(), mode="EditMode") + assert len(calls) == 1 + assert calls[0]["requires_no_tests"] is True + assert resp.success is True + + @pytest.mark.asyncio async def test_get_test_job_forwards_job_id(monkeypatch): from services.tools.run_tests import get_test_job diff --git a/Server/tests/test_cli.py b/Server/tests/test_cli.py index ccaebaa26..65f64f781 100644 --- a/Server/tests/test_cli.py +++ b/Server/tests/test_cli.py @@ -470,11 +470,29 @@ def test_camera_screenshot_scene_view(self, runner, mock_unity_response): ]) assert result.exit_code == 0 mock_run.assert_called_once() - params = mock_run.call_args[0][2] + assert mock_run.call_args[0][0] == "manage_camera" + params = mock_run.call_args[0][1] assert params["captureSource"] == "scene_view" assert params["viewTarget"] == "Canvas" assert params["includeImage"] is True + def test_camera_ping_prints_output(self, runner, mock_unity_response): + """The camera group must actually emit its result. + + Asserting only on run_command's arguments is what let the group ship + while formatting to a discarded string and printing nothing at all. + """ + with patch("cli.commands.camera.run_command", return_value=mock_unity_response): + result = runner.invoke(cli, ["camera", "ping"]) + assert result.exit_code == 0 + assert result.output.strip() != "" + + def test_camera_respects_json_format(self, runner, mock_unity_response): + """--format json must reach format_output, not be swallowed by a config object.""" + with patch("cli.commands.camera.run_command", return_value=mock_unity_response): + result = runner.invoke(cli, ["--format", "json", "camera", "ping"]) + assert result.exit_code == 0 + json.loads(result.output) # ============================================================================= diff --git a/Server/tests/test_manage_gameobject.py b/Server/tests/test_manage_gameobject.py new file mode 100644 index 000000000..9c0e77d30 --- /dev/null +++ b/Server/tests/test_manage_gameobject.py @@ -0,0 +1,313 @@ +"""Tests for manage_gameobject tool. + +Covers the fixes for https://github.com/CoplayDev/unity-mcp/issues/1297: +manage_gameobject's 'create' action had no reachable way to set component +properties. component_properties was forwarded to Unity but only consumed by +the 'modify' handler, and the {typeName, properties} object shape the 'create' +handler does read out of components_to_add was rejected by Pydantic validation +because components_to_add was typed as a list of strings. + +These tests exercise the Python-side contract only: what gets forwarded to +Unity as componentsToAdd / componentProperties, and what gets rejected before +a call is ever sent. The C# consumption side (GameObjectCreate.cs, +GameObjectComponentHelpers.ApplyComponentProperties) is not exercised here - +see TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs +for that half of the coverage. +""" + +import asyncio +import inspect +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from services.tools.manage_gameobject import manage_gameobject +from services.registry import get_registered_tools + + +# ── Fixture ────────────────────────────────────────────────────────── + + +@pytest.fixture +def mock_unity(monkeypatch): + captured: dict[str, object] = {} + + async def fake_send(send_fn, unity_instance, tool_name, params): + captured["unity_instance"] = unity_instance + captured["tool_name"] = tool_name + captured["params"] = params + return {"success": True, "message": "ok", "data": {}} + + monkeypatch.setattr( + "services.tools.manage_gameobject.get_unity_instance_from_context", + AsyncMock(return_value="unity-instance-1"), + ) + monkeypatch.setattr( + "services.tools.manage_gameobject.send_with_unity_instance", + fake_send, + ) + monkeypatch.setattr( + "services.tools.manage_gameobject.preflight", + AsyncMock(return_value=None), + ) + return captured + + +# ── component_properties forwarding (Fix 1 groundwork) ─────────────── + + +class TestManageGameObjectComponentProperties: + """component_properties should reach Unity for 'create', not just 'modify'.""" + + def test_component_properties_parameter_exists(self): + sig = inspect.signature(manage_gameobject) + assert "component_properties" in sig.parameters + + def test_tool_description_mentions_component_properties(self): + tool = next( + (t for t in get_registered_tools() if t["name"] == "manage_gameobject"), None + ) + assert tool is not None + desc = tool.get("description") or tool.get("kwargs", {}).get("description", "") + # The top-level tool description doesn't need to mention it, but the + # parameter's own annotation must - checked via the signature instead. + assert desc # sanity: tool is registered with a description at all + + def test_component_properties_forwarded_on_create(self, mock_unity): + """component_properties must be forwarded as componentProperties when action='create'. + + Before the fix this value reached Unity too (the Python layer never + gated it on action), but nothing on the C# side consumed it. This test + pins the Python-side half of the contract: the argument is not + silently dropped before the call is even made. + """ + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=["BoxCollider"], + component_properties={"BoxCollider": {"size": [2, 2, 2]}}, + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentProperties"] == { + "BoxCollider": {"size": [2, 2, 2]} + } + + def test_component_properties_json_string_forwarded_on_create(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + component_properties='{"BoxCollider": {"size": [2, 2, 2]}}', + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentProperties"] == { + "BoxCollider": {"size": [2, 2, 2]} + } + + def test_invalid_component_properties_rejected_before_send(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + component_properties="not json", + ) + ) + assert result["success"] is False + assert "component_properties" not in mock_unity + + +# ── components_to_add object entries (Fix 2) ────────────────────────── + + +class TestManageGameObjectComponentsToAdd: + """components_to_add must accept {typeName, properties} objects, matching + what GameObjectCreate.cs already reads out of each entry.""" + + def test_components_to_add_parameter_exists(self): + sig = inspect.signature(manage_gameobject) + assert "components_to_add" in sig.parameters + + def test_plain_string_list_still_forwarded(self, mock_unity): + """Regression: the original list[str] form must keep working.""" + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=["BoxCollider", "Rigidbody"], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == ["BoxCollider", "Rigidbody"] + + def test_single_plain_string_still_forwarded(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add="BoxCollider", + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == ["BoxCollider"] + + def test_object_entry_with_properties_accepted(self, mock_unity): + """This is the shape the issue's reproduction #2 shows being rejected + by Pydantic validation before the fix.""" + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ] + + def test_object_entry_without_properties_accepted(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[{"typeName": "Rigidbody"}], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [{"typeName": "Rigidbody"}] + + def test_mixed_string_and_object_entries_accepted(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[ + "Rigidbody", + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}}, + ], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [ + "Rigidbody", + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}}, + ] + + def test_single_object_entry_without_list_wrapping_accepted(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add={"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}}, + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ] + + def test_json_string_of_object_entries_accepted(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add='[{"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}}]', + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ] + + def test_snake_case_type_name_key_normalized(self, mock_unity): + """Convenience accepted alongside the documented camelCase 'typeName'.""" + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[{"type_name": "BoxCollider"}], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [{"typeName": "BoxCollider"}] + + def test_object_entry_missing_type_name_rejected(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[{"properties": {"size": [2, 2, 2]}}], + ) + ) + assert result["success"] is False + assert "typeName" in result["message"] + assert "params" not in mock_unity + + def test_object_entry_non_object_properties_rejected(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[{"typeName": "BoxCollider", "properties": "not-an-object"}], + ) + ) + assert result["success"] is False + assert "params" not in mock_unity + + def test_invalid_entry_type_rejected(self, mock_unity): + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + components_to_add=[123], + ) + ) + assert result["success"] is False + assert "params" not in mock_unity + + def test_none_omitted_from_params(self, mock_unity): + asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="create", + name="Probe", + ) + ) + assert "componentsToAdd" not in mock_unity["params"] + + def test_components_to_add_with_object_entries_works_on_modify_too(self, mock_unity): + """GameObjectModify.cs reads the same {typeName, properties} shape.""" + result = asyncio.run( + manage_gameobject( + SimpleNamespace(), + action="modify", + target="Probe", + components_to_add=[ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ], + ) + ) + assert result["success"] is True + assert mock_unity["params"]["componentsToAdd"] == [ + {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} + ] diff --git a/Server/tests/test_resource_uri_references.py b/Server/tests/test_resource_uri_references.py new file mode 100644 index 000000000..29402037d --- /dev/null +++ b/Server/tests/test_resource_uri_references.py @@ -0,0 +1,170 @@ +"""Agent-facing prose must address resources by URI, never by bare name. + +A resource's name and its URI are deliberately different (`editor_state` vs +`mcpforunity://editor/state`), and the URI scheme is not derivable from the +name. Any instruction, description or tool-result hint that mentions a resource +by name alone sends the reader to build `mcpforunity://`, which 404s. +""" +import os +import re +import typing +from pathlib import Path + +import pytest + +import services.resources as resources_pkg +import services.tools as tools_pkg +from services.registry import get_registered_resources, get_registered_tools +from utils.module_discovery import discover_modules + +REPO_ROOT = Path(__file__).resolve().parents[2] +UNITY_EDITOR_DIR = REPO_ROOT / "MCPForUnity" / "Editor" + +# C# string literal, honouring backslash escapes. +CSHARP_STRING = re.compile(r'"((?:[^"\\]|\\.)*)"') + + +@pytest.fixture(scope="module") +def build_instructions(): + """Import `main` without leaking the env vars it sets at import time.""" + before = dict(os.environ) + from main import _build_instructions + os.environ.clear() + os.environ.update(before) + return _build_instructions + + +@pytest.fixture(scope="module") +def resource_uris_by_name() -> dict[str, str]: + """Registered resources whose name is an unambiguous snake_case identifier. + + Single-word names (`tests`, `cameras`, `volumes`) are ordinary English and + would match prose that is not referring to the resource at all. + """ + list(discover_modules(Path(resources_pkg.__file__).parent, + resources_pkg.__package__)) + registered = get_registered_resources() + assert registered, "no resources registered — discovery failed" + return {r["name"]: r["uri"] for r in registered if "_" in r["name"]} + + +def _offenders(text: str | None, where: str, uris_by_name: dict[str, str]) -> list[str]: + """Lines that name a resource without also giving that resource's URI.""" + if not text: + return [] + found = [] + for line in str(text).splitlines(): + for name, uri in uris_by_name.items(): + if re.search(rf"\b{re.escape(name)}\b", line) and uri not in line: + found.append(f"{where}: '{name}' without '{uri}' in: {line.strip()}") + return found + + +@pytest.mark.parametrize("project_scoped_tools", [True, False]) +def test_server_instructions_reference_resources_by_uri( + project_scoped_tools: bool, build_instructions, resource_uris_by_name: dict[str, str] +): + offenders = _offenders( + build_instructions(project_scoped_tools), + f"instructions(project_scoped_tools={project_scoped_tools})", + resource_uris_by_name, + ) + assert not offenders, "\n".join(offenders) + + +def test_resource_descriptions_reference_resources_by_uri( + resource_uris_by_name: dict[str, str] +): + offenders = [] + for resource in get_registered_resources(): + offenders += _offenders( + resource.get("description"), + f"resource '{resource['name']}' description", + resource_uris_by_name, + ) + assert not offenders, "\n".join(offenders) + + +def test_tool_descriptions_reference_resources_by_uri( + resource_uris_by_name: dict[str, str] +): + list(discover_modules(Path(tools_pkg.__file__).parent, tools_pkg.__package__)) + registered = get_registered_tools() + assert registered, "no tools registered — discovery failed" + + offenders = [] + for tool in registered: + name = tool["name"] + func = tool["func"] + offenders += _offenders( + tool.get("kwargs", {}).get("description"), + f"tool '{name}' description", + resource_uris_by_name, + ) + offenders += _offenders( + func.__doc__, f"tool '{name}' docstring", resource_uris_by_name) + + hints = typing.get_type_hints(func, include_extras=True) + for param, hint in hints.items(): + for meta in getattr(hint, "__metadata__", ()): + if isinstance(meta, str): + offenders += _offenders( + meta, f"tool '{name}' parameter '{param}'", resource_uris_by_name) + + assert not offenders, "\n".join(offenders) + + +def test_unity_tool_result_strings_reference_resources_by_uri( + resource_uris_by_name: dict[str, str] +): + """Hints Unity returns in tool payloads are read by the same agents. + + Only multi-word literals are checked: a bare token such as "get_tests" is a + command name, not prose telling the reader to go read a resource. + """ + assert UNITY_EDITOR_DIR.is_dir(), f"{UNITY_EDITOR_DIR} not found" + + offenders = [] + for source in sorted(UNITY_EDITOR_DIR.rglob("*.cs")): + text = source.read_text(encoding="utf-8", errors="replace") + for line_no, line in enumerate(text.splitlines(), 1): + for match in CSHARP_STRING.finditer(line): + literal = match.group(1) + if " " not in literal.strip(): + continue + offenders += _offenders( + literal, + f"{source.relative_to(REPO_ROOT).as_posix()}:{line_no}", + resource_uris_by_name, + ) + assert not offenders, "\n".join(offenders) + + +def test_agent_facing_markdown_references_resources_by_uri( + resource_uris_by_name: dict[str, str] +): + """Docs that tell a reader to go read a resource must give its URI. + + Scoped to the surfaces that issue that instruction: the skill agents load, + and the per-tool reference pages. Excluded deliberately — + `website/docs/reference/resources/` is a generated catalog that puts each + name in a heading and its URI on the following line, and the guides and + getting-started pages name resources as subjects of a sentence rather than + telling anyone to construct a URI. + """ + targets = [REPO_ROOT / "unity-mcp-skill" / "SKILL.md"] + targets += sorted((REPO_ROOT / "website" / "docs" / + "reference" / "tools").rglob("*.md")) + assert len(targets) > 1, "expected the skill and the tool reference pages" + + offenders = [] + for doc in targets: + assert doc.is_file(), f"{doc} not found" + text = doc.read_text(encoding="utf-8", errors="replace") + for line_no, line in enumerate(text.splitlines(), 1): + offenders += _offenders( + line, + f"{doc.relative_to(REPO_ROOT).as_posix()}:{line_no}", + resource_uris_by_name, + ) + assert not offenders, "\n".join(offenders) diff --git a/Server/tests/test_tool_annotations.py b/Server/tests/test_tool_annotations.py new file mode 100644 index 000000000..933259ce4 --- /dev/null +++ b/Server/tests/test_tool_annotations.py @@ -0,0 +1,158 @@ +"""Annotation guard: every MCP tool must state its safety hints explicitly. + +MCP clients gate a tool behind a human approval prompt when it is not read-only +and not explicitly non-destructive. ``destructiveHint`` defaults to *true* when +omitted, so forgetting it is the dangerous direction -- a harmless read gets an +approval prompt on every call. + +That is exactly how #1288 happened: PR #480's own description claimed hints for +``read_console``, ``manage_editor`` and ``set_active_instance``, but the merged +diff set only ``title``. The spec default silently supplied ``destructiveHint: +true`` and nobody noticed for a year. + +So this guard requires ``title`` and ``destructiveHint`` to be stated outright, +and pins the set of tools that are safe to auto-approve. ``readOnlyHint`` is not +required: omitting it means *false*, which is the safe direction and is already +correct for every tool that leaves it unset. +""" +from pathlib import Path + +import pytest + +import services.tools as tools_package +from services.registry import get_registered_tools +from utils.module_discovery import discover_modules + + +# Tools a client may run without prompting. Two kinds live here: +# - read-only: observe Unity, change nothing. +# - non-destructive: change only ephemeral or session-local state. +# Adding a name here asserts that an agent may call it unattended. Do not add a +# tool that writes to the project, and do not set readOnlyHint on a tool whose +# body calls preflight(refresh_if_dirty=True) -- that can trigger a domain reload. +READ_ONLY = { + "debug_request_context", + "find_in_file", + "get_sha", + "get_test_job", + "manage_script_capabilities", + "unity_docs", + "unity_reflect", + "validate_script", +} + +NON_DESTRUCTIVE = { + # preflight(refresh_if_dirty=True) at find_gameobjects.py can refresh assets, + # so it is not read-only -- but it never destroys anything. + "find_gameobjects", + # 'clear' empties the ephemeral Editor console buffer; Unity still mirrors + # every entry to the Editor log file on disk. + "read_console", + # Session-local routing only. + "set_active_instance", + # Toggles which tools are visible to this session. + "manage_tools", + # Reads counters / starts a profiler session; writes no project asset. + "manage_profiler", + # Generate into a staging area; the import step is a separate tool. + "generate_audio", + "generate_image", + "generate_model", + "import_model", + "import_model_file", +} + +AUTO_APPROVABLE = READ_ONLY | NON_DESTRUCTIVE + + +def _hint(annotations, field: str): + """Read one hint, treating 'not stated' as None. + + The real ToolAnnotations is a pydantic model with every field defaulting to + None, but tests/integration/conftest.py substitutes a stub that only sets + the kwargs actually passed. getattr with a default reads the same answer + from either, so this guard means the same thing whatever ran before it. + """ + return getattr(annotations, field, None) + + +@pytest.fixture(scope="module") +def tools() -> dict: + # Import every tool module so its @mcp_for_unity_tool decorator runs. Going + # through discover_modules rather than register_all_tools keeps this off + # FastMCP, which tests/integration/conftest.py replaces with a stub for the + # whole session. + list(discover_modules(Path(tools_package.__file__).parent, tools_package.__package__)) + registered = get_registered_tools() + + # Keying by name would silently drop a duplicate registration, hiding both the + # registry bug and the annotations of whichever entry lost. Fail on it instead. + names = [t["name"] for t in registered] + duplicates = sorted({n for n in names if names.count(n) > 1}) + assert not duplicates, f"Duplicate tool registrations: {duplicates}" + + return {t["name"]: t for t in registered} + + +def test_every_tool_declares_its_hints(tools): + missing = [] + for name, tool in sorted(tools.items()): + annotations = tool["kwargs"].get("annotations") + if annotations is None: + missing.append(f"{name}: no annotations= at all") + continue + if not _hint(annotations, "title"): + missing.append(f"{name}: no title") + if _hint(annotations, "destructiveHint") is None: + missing.append(f"{name}: destructiveHint not stated (defaults to True)") + assert not missing, ( + "Every tool must state title and destructiveHint explicitly:\n " + + "\n ".join(missing) + ) + + +def test_auto_approvable_tools_are_not_gated(tools): + """A tool in AUTO_APPROVABLE must actually serialize as auto-approvable.""" + gated = [] + for name in sorted(AUTO_APPROVABLE): + assert name in tools, f"{name} is in AUTO_APPROVABLE but is not a registered tool" + annotations = tools[name]["kwargs"]["annotations"] + read_only = _hint(annotations, "readOnlyHint") + destructive = _hint(annotations, "destructiveHint") + if not read_only and destructive is not False: + gated.append(f"{name}: readOnlyHint={read_only} destructiveHint={destructive}") + assert not gated, ( + "These tools are listed as safe to auto-approve but a spec-compliant " + "client would still prompt for them:\n " + "\n ".join(gated) + ) + + +def test_read_only_set_is_exact(tools): + """readOnlyHint=True is a promise the tool cannot mutate anything. Pin it.""" + actual = { + name + for name, tool in tools.items() + if _hint(tool["kwargs"]["annotations"], "readOnlyHint") is True + } + assert actual == READ_ONLY, ( + "The read-only tool set changed. Newly read-only: " + f"{sorted(actual - READ_ONLY)}; no longer read-only: {sorted(READ_ONLY - actual)}. " + "Update READ_ONLY only after confirming the tool truly mutates nothing -- " + "including via preflight(refresh_if_dirty=True), which can trigger a domain reload." + ) + + +def test_mutating_tools_stay_gated(tools): + """Anything outside AUTO_APPROVABLE must keep prompting.""" + ungated = [] + for name, tool in sorted(tools.items()): + if name in AUTO_APPROVABLE: + continue + annotations = tool["kwargs"]["annotations"] + if _hint(annotations, "readOnlyHint") or _hint(annotations, "destructiveHint") is False: + ungated.append(name) + assert not ungated, ( + "These tools write to the Unity project but are marked auto-approvable: " + f"{ungated}. Either they belong in AUTO_APPROVABLE with a comment saying why, " + "or the annotation is wrong." + ) diff --git a/Server/tests/test_tool_test_symmetry.py b/Server/tests/test_tool_test_symmetry.py index 60cba03d5..e51ce850f 100644 --- a/Server/tests/test_tool_test_symmetry.py +++ b/Server/tests/test_tool_test_symmetry.py @@ -43,10 +43,17 @@ def _tool_modules() -> list[str]: return mods +# Registry-wide guards name many tools at once to assert one cross-cutting +# property. That is not coverage of any individual tool, so they don't count +# here -- otherwise one such file would silently satisfy this guard for every +# tool it happens to mention. +NOT_COVERAGE = {"test_tool_test_symmetry.py", "test_tool_annotations.py"} + + def _is_referenced(stem: str) -> bool: pattern = re.compile(rf"\b{re.escape(stem)}\b") for test_file in TESTS_DIR.rglob("test_*.py"): - if test_file.resolve() == Path(__file__).resolve(): + if test_file.name in NOT_COVERAGE: continue if pattern.search(test_file.read_text(encoding="utf-8")): return True diff --git a/Server/uv.lock b/Server/uv.lock index a7e843476..81a005bf9 100644 --- a/Server/uv.lock +++ b/Server/uv.lock @@ -858,7 +858,7 @@ wheels = [ [[package]] name = "mcpforunityserver" -version = "10.0.0" +version = "10.1.0" source = { editable = "." } dependencies = [ { name = "click" }, diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Clients/SupportedTransportsTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Clients/SupportedTransportsTests.cs index 865561f0f..75263c81c 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Clients/SupportedTransportsTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Clients/SupportedTransportsTests.cs @@ -2,6 +2,7 @@ using MCPForUnity.Editor.Clients; using MCPForUnity.Editor.Clients.Configurators; using MCPForUnity.Editor.Models; +using MCPForUnity.Editor.Services; using NUnit.Framework; namespace MCPForUnityTests.Editor.Clients @@ -24,6 +25,57 @@ public void ClaudeDesktop_SupportsStdioOnly() CollectionAssert.DoesNotContain(claude.SupportedTransports.ToList(), ConfiguredTransport.Http); } + [Test] + public void Codex_SupportsBothTransports() + { + // Verified against Codex CLI 0.47.0: a bare `[mcp_servers.x] url = "..."` entry reports + // `transport: streamable_http` from `codex mcp get` and completes a full MCP handshake + // against a live server, with no feature flag set. #1193 was not Codex lacking HTTP. + var codex = new CodexConfigurator(); + var list = codex.SupportedTransports.ToList(); + CollectionAssert.Contains(list, ConfiguredTransport.Stdio); + CollectionAssert.Contains(list, ConfiguredTransport.Http); + Assert.IsTrue(codex.Client.SupportsHttpTransport, "Codex is HTTP-capable"); + } + + [Test] + public void Codex_ManualSnippet_RendersUrl_WhenHttpPreferred() + { + var cache = EditorConfigurationCache.Instance; + bool original = cache.UseHttpTransport; + try + { + cache.SetUseHttpTransport(true); + string snippet = new CodexConfigurator().GetManualSnippet(); + + StringAssert.Contains("url =", snippet, "Codex snippet must configure the HTTP url"); + Assert.IsTrue(cache.UseHttpTransport, "The global transport pref must be restored"); + } + finally + { + cache.SetUseHttpTransport(original); + } + } + + [Test] + public void Codex_ManualSnippet_RendersCommand_WhenStdioPreferred() + { + var cache = EditorConfigurationCache.Instance; + bool original = cache.UseHttpTransport; + try + { + cache.SetUseHttpTransport(false); + string snippet = new CodexConfigurator().GetManualSnippet(); + + StringAssert.Contains("command", snippet, "Codex snippet must configure stdio"); + Assert.IsFalse(snippet.Contains("url ="), "Stdio snippet must not carry an HTTP url"); + } + finally + { + cache.SetUseHttpTransport(original); + } + } + [Test] public void Cursor_SupportsBothTransports() { diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs index 8426643d5..63f17071b 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Services/Server/TerminalLauncherTests.cs @@ -212,6 +212,19 @@ public void CreateHeadlessProcessStartInfo_RedirectsOutputToLogFile() StringAssert.Contains(">>", startInfo.Arguments, "output should be appended to the log via >>"); } +#if UNITY_EDITOR_WIN + [Test] + public void CreateHeadlessProcessStartInfo_RedirectsStdinFromNul() + { + // Regression guard for #1279: the Editor is a console-less GUI process, so a child + // launched with CreateNoWindow inherits an invalid stdin and uvx.exe fails with + // "The handle is invalid. (os error 6)". stdin must come from NUL instead. + var startInfo = _launcher.CreateHeadlessProcessStartInfo("uvx run-server", LogPath()); + + StringAssert.Contains("< NUL", startInfo.Arguments, "stdin should be redirected from NUL"); + } +#endif + [Test] public void CreateHeadlessProcessStartInfo_LogPathWithSpaces_IsQuoted() { diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs index 7785d8971..ae31fb9a7 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs @@ -433,6 +433,128 @@ public void Create_WithNewTag_AutoCreatesTag() #endregion + #region Component Add and Property Tests + // Regression coverage for https://github.com/CoplayDev/unity-mcp/issues/1297: + // 'create' had no reachable way to set component properties. componentProperties + // was accepted but never consumed by the create path, and the {typeName, properties} + // shape componentsToAdd already reads was rejected by the Python schema before it + // ever reached here (see Server/tests/test_manage_gameobject.py for that half). + + [Test] + public void Create_WithComponentProperties_AppliesPropertiesToAddedComponent() + { + var p = new JObject + { + ["action"] = "create", + ["name"] = "TestComponentProperties", + ["componentsToAdd"] = new JArray { "BoxCollider" }, + ["componentProperties"] = new JObject + { + ["BoxCollider"] = new JObject { ["size"] = new JArray(2, 2, 2) } + } + }; + + var result = ManageGameObject.HandleCommand(p); + var resultObj = result as JObject ?? JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), resultObj.ToString()); + + var created = FindAndTrack("TestComponentProperties"); + Assert.IsNotNull(created); + var collider = created.GetComponent(); + Assert.IsNotNull(collider, "BoxCollider should have been added"); + Assert.AreEqual(new Vector3(2f, 2f, 2f), collider.size, + "componentProperties should be applied at create time, not silently ignored"); + } + + [Test] + public void Create_WithComponentsToAddObjectEntry_AppliesPropertiesInSameEntry() + { + // The alternate shape the create path has always read directly out of + // componentsToAdd entries (typeName + properties), now reachable through + // the Python schema too. + var p = new JObject + { + ["action"] = "create", + ["name"] = "TestComponentsToAddObjectEntry", + ["componentsToAdd"] = new JArray + { + new JObject + { + ["typeName"] = "BoxCollider", + ["properties"] = new JObject { ["size"] = new JArray(3, 3, 3) } + } + } + }; + + var result = ManageGameObject.HandleCommand(p); + var resultObj = result as JObject ?? JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), resultObj.ToString()); + + var created = FindAndTrack("TestComponentsToAddObjectEntry"); + Assert.IsNotNull(created); + var collider = created.GetComponent(); + Assert.IsNotNull(collider, "BoxCollider should have been added"); + Assert.AreEqual(new Vector3(3f, 3f, 3f), collider.size); + } + + [Test] + public void Create_WithComponentPropertiesForComponentNotAdded_ReturnsErrorAndDestroysObject() + { + var p = new JObject + { + ["action"] = "create", + ["name"] = "TestComponentPropertiesMissingComponent", + ["componentProperties"] = new JObject + { + ["BoxCollider"] = new JObject { ["size"] = new JArray(2, 2, 2) } + } + }; + + var result = ManageGameObject.HandleCommand(p); + var resultObj = result as JObject ?? JObject.FromObject(result); + + Assert.IsFalse(resultObj.Value("success"), + "Setting properties on a component that was never added should fail, not silently succeed"); + + var created = GameObject.Find("TestComponentPropertiesMissingComponent"); + Assert.IsNull(created, "The partially-created GameObject should have been cleaned up"); + } + + [Test] + public void Create_WithMixedStringAndObjectComponentEntries_AddsAllAndAppliesProperties() + { + var p = new JObject + { + ["action"] = "create", + ["name"] = "TestMixedComponentEntries", + ["componentsToAdd"] = new JArray + { + "Rigidbody", + new JObject + { + ["typeName"] = "BoxCollider", + ["properties"] = new JObject { ["size"] = new JArray(4, 4, 4) } + } + } + }; + + var result = ManageGameObject.HandleCommand(p); + var resultObj = result as JObject ?? JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), resultObj.ToString()); + + var created = FindAndTrack("TestMixedComponentEntries"); + Assert.IsNotNull(created); + Assert.IsNotNull(created.GetComponent(), "Rigidbody should have been added"); + var collider = created.GetComponent(); + Assert.IsNotNull(collider, "BoxCollider should have been added"); + Assert.AreEqual(new Vector3(4f, 4f, 4f), collider.size); + } + + #endregion + #region Response Structure Tests [Test] diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs index 814098c60..83767b785 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGraphicsTests.cs @@ -106,15 +106,18 @@ public void Ping_ReturnsPipelineInfo() // Volume Actions // ===================================================================== - private void AssumeVolumeSystem() + // Assert.Ignore, not Assume.That: Assume yields Inconclusive, which the Test Runner + // window renders as a failure and which leaves the run's resultState non-Passed. + private void RequireVolumeSystem() { - Assume.That(_hasVolumeSystem, "Volume system not available — skipping."); + if (!_hasVolumeSystem) + Assert.Ignore("Volume system not available — skipping."); } [Test] public void VolumeCreate_Global_CreatesVolume() { - AssumeVolumeSystem(); + RequireVolumeSystem(); var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "volume_create", @@ -130,7 +133,7 @@ public void VolumeCreate_Global_CreatesVolume() [Test] public void VolumeCreate_WithEffects_AddsEffects() { - AssumeVolumeSystem(); + RequireVolumeSystem(); var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "volume_create", @@ -150,7 +153,7 @@ public void VolumeCreate_WithEffects_AddsEffects() [Test] public void VolumeCreate_Local_CreatesNonGlobal() { - AssumeVolumeSystem(); + RequireVolumeSystem(); var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "volume_create", @@ -164,7 +167,7 @@ public void VolumeCreate_Local_CreatesNonGlobal() [Test] public void VolumeAddEffect_AddsEffect() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_AddFx"); var result = ToJObject(ManageGraphics.HandleCommand(new JObject @@ -181,7 +184,7 @@ public void VolumeAddEffect_AddsEffect() [Test] public void VolumeAddEffect_Duplicate_ReturnsError() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_DupFx"); ManageGraphics.HandleCommand(new JObject { @@ -203,7 +206,7 @@ public void VolumeAddEffect_Duplicate_ReturnsError() [Test] public void VolumeAddEffect_InvalidEffect_ReturnsError() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_BadFx"); var result = ToJObject(ManageGraphics.HandleCommand(new JObject @@ -219,7 +222,7 @@ public void VolumeAddEffect_InvalidEffect_ReturnsError() [Test] public void VolumeSetEffect_SetsParameters() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_SetFx"); ManageGraphics.HandleCommand(new JObject { @@ -245,7 +248,7 @@ public void VolumeSetEffect_SetsParameters() [Test] public void VolumeSetEffect_InvalidParam_ReportsFailed() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_BadParam"); ManageGraphics.HandleCommand(new JObject { @@ -270,7 +273,7 @@ public void VolumeSetEffect_InvalidParam_ReportsFailed() [Test] public void VolumeRemoveEffect_RemovesEffect() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_RmFx"); ManageGraphics.HandleCommand(new JObject { @@ -301,7 +304,7 @@ public void VolumeRemoveEffect_RemovesEffect() [Test] public void VolumeRemoveEffect_NonExistent_ReturnsError() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_RmMissing"); var result = ToJObject(ManageGraphics.HandleCommand(new JObject @@ -317,7 +320,7 @@ public void VolumeRemoveEffect_NonExistent_ReturnsError() [Test] public void VolumeGetInfo_ReturnsEffectList() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_Info"); ManageGraphics.HandleCommand(new JObject { @@ -343,7 +346,7 @@ public void VolumeGetInfo_ReturnsEffectList() [Test] public void VolumeGetInfo_NonExistentTarget_ReturnsError() { - AssumeVolumeSystem(); + RequireVolumeSystem(); var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "volume_get_info", @@ -355,7 +358,7 @@ public void VolumeGetInfo_NonExistentTarget_ReturnsError() [Test] public void VolumeSetProperties_UpdatesWeightAndPriority() { - AssumeVolumeSystem(); + RequireVolumeSystem(); CreateTestVolume("GfxTest_Props"); var result = ToJObject(ManageGraphics.HandleCommand(new JObject @@ -383,7 +386,7 @@ public void VolumeSetProperties_UpdatesWeightAndPriority() [Test] public void VolumeListEffects_ReturnsAvailableTypes() { - AssumeVolumeSystem(); + RequireVolumeSystem(); var result = ToJObject(ManageGraphics.HandleCommand( new JObject { ["action"] = "volume_list_effects" })); Assert.IsTrue(result.Value("success"), result.ToString()); @@ -396,7 +399,7 @@ public void VolumeListEffects_ReturnsAvailableTypes() [Test] public void VolumeCreateProfile_CreatesAsset() { - AssumeVolumeSystem(); + RequireVolumeSystem(); string path = $"{TempRoot}/TestProfile"; var result = ToJObject(ManageGraphics.HandleCommand(new JObject { @@ -626,7 +629,8 @@ public void PipelineGetInfo_ReturnsPipelineName() [Test] public void PipelineGetSettings_ReturnsSettings() { - Assume.That(_hasURP || _hasHDRP, "Built-in pipeline has no settings asset — skipping."); + if (!_hasURP && !_hasHDRP) + Assert.Ignore("Built-in pipeline has no settings asset — skipping."); var result = ToJObject(ManageGraphics.HandleCommand( new JObject { ["action"] = "pipeline_get_settings" })); Assert.IsTrue(result.Value("success"), result.ToString()); @@ -651,15 +655,16 @@ public void PipelineSetQuality_InvalidLevel_ReturnsError() // Renderer Feature Actions (URP only) // ===================================================================== - private void AssumeURP() + private void RequireURP() { - Assume.That(_hasURP, "URP not available — skipping."); + if (!_hasURP) + Assert.Ignore("URP not available — skipping."); } [Test] public void FeatureList_ReturnsFeatures() { - AssumeURP(); + RequireURP(); var result = ToJObject(ManageGraphics.HandleCommand( new JObject { ["action"] = "feature_list" })); Assert.IsTrue(result.Value("success"), result.ToString()); @@ -670,7 +675,7 @@ public void FeatureList_ReturnsFeatures() [Test] public void FeatureAdd_InvalidType_ReturnsError() { - AssumeURP(); + RequireURP(); var result = ToJObject(ManageGraphics.HandleCommand(new JObject { ["action"] = "feature_add", diff --git a/manifest.json b/manifest.json index f53af1e65..bcf47559f 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": "0.3", "name": "Unity MCP", - "version": "10.1.0", + "version": "10.1.2", "description": "AI-powered Unity Editor automation via MCP - manage GameObjects, scripts, materials, scenes, prefabs, VFX, and run tests", "author": { "name": "Coplay", diff --git a/unity-mcp-skill/SKILL.md b/unity-mcp-skill/SKILL.md index 905561cc7..a4a979f6f 100644 --- a/unity-mcp-skill/SKILL.md +++ b/unity-mcp-skill/SKILL.md @@ -129,7 +129,7 @@ read_console( ) ``` -### 5. Always Check `editor_state` Before Complex Operations +### 5. Always Check `mcpforunity://editor/state` Before Complex Operations ```python # Read mcpforunity://editor/state to check: @@ -268,7 +268,7 @@ set_active_instance(instance="MyProject@abc123") | Symptom | Cause | Solution | |---------|-------|----------| -| Tools return "busy" | Compilation in progress | Wait, check `editor_state` | +| Tools return "busy" | Compilation in progress | Wait, check `mcpforunity://editor/state` | | "stale_file" error | File changed since SHA | Re-fetch SHA with `get_sha`, retry | | Connection lost | Domain reload | Wait ~5s, reconnect | | Commands fail silently | Wrong instance | Check `set_active_instance` | diff --git a/website/docs/migrations/v8.md b/website/docs/migrations/v8.md index 8edaea54f..7aad4fe01 100644 --- a/website/docs/migrations/v8.md +++ b/website/docs/migrations/v8.md @@ -89,7 +89,7 @@ mcp.run(transport=transport, host=host, port=port) And that's pretty much it in terms of HTTP support between the MCP server and client. Things get more interesting for the connection to the Unity plugin. -Backward compatability with stdio connections was maintained, but we did make some small performance optimisations. Namely, we have an in-memory cache of unity isntances using the `StdioPortRegistry` class. +Backward compatibility with stdio connections was maintained, but we did make some small performance optimisations. Namely, we have an in-memory cache of unity instances using the `StdioPortRegistry` class. It still calls `PortDiscovery.discover_all_unity_instances()`, but we add a lock when calling it, so multiple attempts to retrieve the instances do not cause our app to run multiple file scans at the same time. @@ -212,7 +212,7 @@ Relevant commits: ### Window logic has been split into separate classes -The main `MCPForUnityEditorWindow.cs` class, and the releated uxml and uss files, were getting quite long. We had a similar problem with the last immediate UI version of it. To keep it maintanable, we split the logic into 3 separate view classes: Settings, Connection andn ClientConfig. They correspond to the 3 visual sections the window has. +The main `MCPForUnityEditorWindow.cs` class, and the related uxml and uss files, were getting quite long. We had a similar problem with the last immediate UI version of it. To keep it maintainable, we split the logic into 3 separate view classes: Settings, Connection and ClientConfig. They correspond to the 3 visual sections the window has. Each section has its own C#, uxml and uss files, but we use a common uss file for shared styles. @@ -252,7 +252,7 @@ This was a big change, and it touches all the repo. So a lot of inefficiencies a - Loose types in Python. A lot of the new code would use dictionaries for structured data, which works, but we can benefit much more from using Pydantic classes with proper type checking. We always want to know when data is not being transferred in the format we expect it to. Plus, strong types make the code easier for humans and LLMs to reason about. - A lot of tools define a `_coerce_int` function, why? Why are we redefining a function that's the same across files? Can we use a shared function, or maybe use it as middleware? -- Similarly, the `DummyMCP` class is defined in 10 server tests, we could set this up in `conftest.py`. These tests were originally indepdendent of the `Server` project, but in v7 they became integration tests we run with `pytest`. With `pytest` being the default test runner, we can relook at how the tests are structured and optimize their setup. +- Similarly, the `DummyMCP` class is defined in 10 server tests, we could set this up in `conftest.py`. These tests were originally independent of the `Server` project, but in v7 they became integration tests we run with `pytest`. With `pytest` being the default test runner, we can relook at how the tests are structured and optimize their setup. - `server_version.txt` is used in one place, but the server can now read its own pyproject.toml to get the version, so we can remove this. - ~~Think about a structure of the MCP server some more. The `tools`, `resources` and `registry` folders make sense, but everything else just forms part of the high level repo. It's growing, so some thought about how we create modules will help with scalability.~~ - This was done, Server folder is much more hierarchical and structured. diff --git a/website/docs/reference/tools/core/manage_gameobject.md b/website/docs/reference/tools/core/manage_gameobject.md index db00ac02e..d8cffebb5 100644 --- a/website/docs/reference/tools/core/manage_gameobject.md +++ b/website/docs/reference/tools/core/manage_gameobject.md @@ -27,7 +27,7 @@ Performs CRUD operations on GameObjects. Actions: create, modify, delete, duplic | `position` | `list[float] \| dict[str, float] \| str \| None` | — | Position as [x, y, z] array, {x, y, z} object, or JSON string | | `rotation` | `list[float] \| dict[str, float] \| str \| None` | — | Rotation as [x, y, z] euler angles array, {x, y, z} object, or JSON string | | `scale` | `list[float] \| dict[str, float] \| str \| None` | — | Scale as [x, y, z] array, {x, y, z} object, or JSON string | -| `components_to_add` | `list[str] \| str \| None` | — | List of component names to add during 'create' or 'modify' | +| `components_to_add` | `list[str \| dict[str, Any]] \| dict[str, Any] \| str \| None` | — | List of components to add during 'create' or 'modify'. Each entry is either a plain type name string (e.g. "BoxCollider") or an object {"typeName": "BoxCollider", "properties": {"size": [2, 2, 2]}} that adds the component with initial properties applied in the same call. Mixing both forms in one list is fine. | | `primitive_type` | `str \| None` | — | Primitive type for 'create' action | | `save_as_prefab` | `bool \| str \| None` | — | If True, saves the created GameObject as a prefab (accepts true/false or 'true'/'false') | | `prefab_path` | `str \| None` | — | Path for prefab creation | @@ -36,7 +36,7 @@ Performs CRUD operations on GameObjects. Actions: create, modify, delete, duplic | `layer` | `str \| None` | — | Layer name | | `is_static` | `bool \| str \| None` | — | Set the GameObject's static flag. true = all StaticEditorFlags, false = none (accepts true/false or 'true'/'false') | | `components_to_remove` | `list[str] \| str \| None` | — | List of component names to remove | -| `component_properties` | `dict[str, dict[str, Any]] \| str \| None` | — | Dictionary of component names to their properties to set. For example: `{"MyScript": {"otherObject": {"find": "Player", "method": "by_name"}}}` assigns GameObject `{"MyScript": {"playerHealth": {"find": "Player", "component": "HealthComponent"}}}` assigns Component Example set nested property: - Access shared material: `{"MeshRenderer": {"sharedMaterial.color": [1, 0, 0, 1]}}` | +| `component_properties` | `dict[str, dict[str, Any]] \| str \| None` | — | Dictionary of component names to their properties to set. Works for both 'create' (applied to components already present on the new GameObject - add them via components_to_add first) and 'modify'. For example: `{"MyScript": {"otherObject": {"find": "Player", "method": "by_name"}}}` assigns GameObject `{"MyScript": {"playerHealth": {"find": "Player", "component": "HealthComponent"}}}` assigns Component Example set nested property: - Access shared material: `{"MeshRenderer": {"sharedMaterial.color": [1, 0, 0, 1]}}` | | `new_name` | `str \| None` | — | New name for the duplicated object (default: SourceName_Copy) | | `offset` | `list[float] \| str \| None` | — | Offset from original/reference position as [x, y, z] array (list or JSON string) | | `reference_object` | `str \| None` | — | Reference object for relative movement (required for move_relative) | diff --git a/website/docs/reference/tools/core/manage_script.md b/website/docs/reference/tools/core/manage_script.md index e5c0ab852..6f8e82d94 100644 --- a/website/docs/reference/tools/core/manage_script.md +++ b/website/docs/reference/tools/core/manage_script.md @@ -84,6 +84,6 @@ Returns the full file. For just a SHA (to detect drift between reads and writes) ### After every create / delete -Unity needs a domain reload to compile the new file (or notice the old one is gone). Poll the `editor_state` resource's `isCompiling` field until it flips back to `false`, then run [`read_console`](./read_console) to catch any compile errors before relying on the new types. +Unity needs a domain reload to compile the new file (or notice the old one is gone). Poll the `mcpforunity://editor/state` resource until `data.compilation.is_compiling` flips back to `false`, then run [`read_console`](./read_console) to catch any compile errors before relying on the new types. diff --git a/website/docs/reference/tools/core/refresh_unity.md b/website/docs/reference/tools/core/refresh_unity.md index e898712b0..7149d4eb5 100644 --- a/website/docs/reference/tools/core/refresh_unity.md +++ b/website/docs/reference/tools/core/refresh_unity.md @@ -21,7 +21,7 @@ Request a Unity asset database refresh and optionally a script compilation. Can | `mode` | `Literal['if_dirty', 'force']` | — | Refresh mode | | `scope` | `Literal['assets', 'scripts', 'all']` | — | Refresh scope | | `compile` | `Literal['none', 'request']` | — | Whether to request compilation | -| `wait_for_ready` | `bool` | — | If true, wait until editor_state.advice.ready_for_tools is true | +| `wait_for_ready` | `bool` | — | If true, wait until mcpforunity://editor/state reports data.advice.ready_for_tools true | ## Returns diff --git a/website/docs/reference/tools/core/script_apply_edits.md b/website/docs/reference/tools/core/script_apply_edits.md index d6440540e..d7cdcd2e2 100644 --- a/website/docs/reference/tools/core/script_apply_edits.md +++ b/website/docs/reference/tools/core/script_apply_edits.md @@ -181,6 +181,6 @@ Anchor ops are great for adding instrumentation near stable comment markers with ### After every edit -Poll `editor_state.isCompiling` until it flips back to `false`, then run [`read_console`](./read_console) to catch any compile errors before relying on the new types. +Poll the `mcpforunity://editor/state` resource until `data.compilation.is_compiling` flips back to `false`, then run [`read_console`](./read_console) to catch any compile errors before relying on the new types. diff --git a/website/docs/reference/tools/testing/run_tests.md b/website/docs/reference/tools/testing/run_tests.md index 07e90da52..1fcf8a1e3 100644 --- a/website/docs/reference/tools/testing/run_tests.md +++ b/website/docs/reference/tools/testing/run_tests.md @@ -26,6 +26,7 @@ Starts a Unity test run asynchronously and returns a job_id immediately. Poll wi | `include_failed_tests` | `bool` | — | Include details for failed/skipped tests only (default: false) | | `include_details` | `bool` | — | Include details for all tests (default: false) | | `init_timeout` | `int \| None` | — | Initialization timeout in milliseconds. PlayMode tests may need longer due to domain reload (default: 15000). Recommended: 120000 for PlayMode. | +| `clear_stuck` | `bool` | — | Clear an orphaned running job instead of starting a run. Use when a job was lost to a domain reload and is blocking every subsequent run. | ## Returns