chore: bump version to 10.1.2 - #1312
Merged
Merged
Conversation
…9284763400 chore: sync main (v10.1.0) into beta
Fix spelling in website/docs/migrations/v8.md: compatability->compatibility, isntances->instances, releated->related, maintanable->maintainable, andn->and, indepdendent->independent.
) The Editor is a console-less GUI process. TerminalLauncher spawned cmd.exe with UseShellExecute=false and CreateNoWindow=true and never redirected stdin, so uvx.exe inherited an invalid stdin handle and died with "The handle is invalid. (os error 6)" before the server could start. Redirect stdin from NUL inside the cmd.exe payload so the child gets a valid handle regardless of whether the Editor has a console. Regression from #1201, shipped in v10.1.0.
EditorApplication.isCompiling conflates three states: actually compiling, compilation queued, and finished-but-reload-deferred. A project holding EditorApplication.LockReloadAssemblies sits in the third state for as long as the lock is held, with no compilation running, and isCompiling stays true the whole time. Eight call sites gated on that raw flag, so they refused work indefinitely: the stdio bridge would not start, unity_reflect and manage_scriptable_object returned "Unity is compiling", refresh_unity reported the wrong resulting state and never completed its wait, the stdio reload handler deferred its resume, and TestJobManager both mis-attributed its init timeout and reported a bogus "compiling" block reason. Route all eight through EditorStateCache.GetActualIsCompiling(), which falls back to the event-tracked CompilationPipeline flag, and drop the isPlaying gate that previously limited the workaround to play mode. Verified live: with LockReloadAssemblies held after RequestScriptCompilation, EditorApplication.isCompiling is true while the pipeline flag is false, so CompilationPipeline.compilationFinished does fire while the reload is held.
Codex does not expose MCP tools that are configured through the HTTP block, so writing an HTTP config produces a client that connects but surfaces no tools. Declare SupportsHttpTransport = false and restrict SupportedTransports to stdio, so CoerceTransportFor settles on stdio before Configure() runs.
All 18 call sites passed run_command(config, "manage_camera", params) while the signature is run_command(tool, params, config), so the entire `unity-mcp camera` command group was dead at beta HEAD. test_cli.py asserted against call_args[0][2], which encoded the bug rather than catching it; it now asserts the tool name at [0][0] and params at [0][1].
A test job orphaned by a domain reload leaves TestRunStatus pinned with a CurrentJobId that blocks every subsequent run, and there was no way to clear it from the client side. Add clear_stuck to the run_tests MCP tool and --clear-stuck to the editor CLI. Both short-circuit ahead of the init_timeout validation and preflight, because neither applies to clearing and preflight's requires_no_tests gate would reject the very call that exists to release it.
The lockfile pinned mcpforunityserver 10.0.0 while pyproject.toml declared 10.1.0, and no workflow runs `uv lock`, so every contributor's first `uv run` dirtied their working tree.
…nclusive The 18 environment guards in ManageGraphicsTests used Assume.That, which yields Inconclusive. Three consumers disagree about what that means: the Test Runner window paints it with a failure icon, run_tests drops it from summary.total while progress.total still counts it (1150 vs 1168), and failures_so_far ignores it. A clean suite therefore reads as 18 failures. Use Assert.Ignore behind an explicit condition instead, matching the existing convention in WriteToConfigTests, StdioBridgeReconnectTests and ManageSceneMultiSceneTests. The helpers are renamed Require* since Assume* named the very API being dropped. Full EditMode suite on 2021.3.45f2 before: 1150 total / 1094 passed / 0 failed / 56 skipped, with 18 inconclusive unaccounted for. After: 1168 / 1094 / 0 / 74, and the two totals reconcile.
Generated by tools/generate_docs_reference.py; the "Check docs reference is fresh" CI job flagged testing/run_tests.md as stale after clear_stuck was added.
Follow-up to the inverted-argument fix in this branch: repairing the run_command call order was necessary but not sufficient, and the group was still effectively dead. format_output(data, format_type: str = "text") returns a string. All 18 call sites called format_output(result, config) and dropped the return value, so every `unity-mcp camera` subcommand printed nothing at all. Passing the whole CLIConfig where a format string was expected also meant the branch always fell through to text, silently ignoring --format/UNITY_MCP_FORMAT. Use click.echo(format_output(result, config.format)), matching every other command module. Adds two regression tests: one asserting the group emits non-empty output, one asserting --format json yields parseable JSON. Both fail without this change. Reported by Copilot on #1293.
…ort set Two review findings on #1292. Copilot: advertising Codex as stdio-only was not enough. GetManualSnippet() calls BuildCodexServerBlock directly, which reads the global UseHttpTransport pref itself. Configure() gets that pref coerced for it by ClientConfigurationService.ConfigureWithTransportCoercion; the snippet path does not. With the HTTP pref on, the copyable snippet still rendered [features] rmcp_client = true [mcp_servers.unityMCP] url = "http://127.0.0.1:8080/mcp" reintroducing the exact silent-failure path via manual setup. Coerce to stdio around the call for any client that does not support HTTP, restoring the pref afterwards, mirroring ConfigureWithTransportCoercion. Deliberately not removing the HTTP branch from CodexConfigHelper: it is covered by BuildCodexServerBlock_HttpMode_GeneratesUrlField and is a general-purpose helper, so narrowing the caller is the smaller and more honest change. CodeRabbit: assert SupportedTransports equals exactly { Stdio } rather than contains-stdio plus not-contains-http, which would also pass if a third transport were added. Adds Codex_ManualSnippet_IsStdio_EvenWhenHttpPreferred, which fails with the url block above before this change.
…nore test: report unavailable-pipeline graphics tests as Skipped, not Inconclusive
…nd-camera fix: repair dead camera CLI, add run_tests clear_stuck (#1272), refresh uv.lock
fix: advertise Codex as stdio-only (#1193)
…stdin fix: redirect stdin from NUL when launching the server on Windows (#1279)
…ompiling fix: trust the pipeline flag when a domain reload is deferred (#1276)
…388780929 chore: update Unity package to beta version 10.1.1-beta.2
docs: fix typos in v8 migration guide
Fixes #1297. At action:"create", component_properties was accepted and coerced by the C# dispatcher (ManageGameObject.cs) but only ever consumed by the "modify" handler, so it silently did nothing. Meanwhile the shape "create" already reads directly out of each componentsToAdd entry ({typeName, properties}) was rejected before it reached Unity, because the Python schema typed components_to_add as list[str]. - GameObjectComponentHelpers.cs: factor the componentProperties loop + error aggregation out of GameObjectModify.cs into a shared ApplyComponentProperties helper, so both actions apply it identically. - GameObjectCreate.cs: call the new helper after components are added, destroying the partially-created object and returning the error if any property fails to set (matching how component-add failures are handled). - GameObjectModify.cs: switch to the shared helper (behavior-preserving refactor, no functional change on the modify path). - manage_gameobject.py: widen components_to_add to accept {"typeName": ..., "properties": {...}} objects alongside plain strings, matching what GameObjectCreate.cs already reads. - Regenerated website/docs/reference/tools/core/manage_gameobject.md via tools/generate_docs_reference.py for the updated parameter docs. Tested: Server/tests/test_manage_gameobject.py exercises the Python contract end-to-end, including a real fastmcp/pydantic schema validation run of the issue's exact repro payloads (confirmed the pre-fix ValidationError reproduces on the unmodified file, and is gone after). Added TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ ManageGameObjectCreateTests.cs coverage for the C# side, but this was not run against a live Editor.
MCP clients gate a tool behind human approval unless it is read-only or explicitly non-destructive, and destructiveHint defaults to true when omitted. PR #480 set only `title=` on read_console, manage_editor and set_active_instance despite its description claiming otherwise, so the spec default supplied destructiveHint: true and nobody noticed. Registering all 48 tools and dumping tools/list showed 34 of them serializing as neither read-only nor explicitly non-destructive. find_gameobjects emitted `annotations: null` outright. State the hints explicitly across 10 modules. Four genuinely safe tools become destructiveHint=False; the read-only set gets explicit hints instead of relying on defaults; manage_editor and manage_components get explicit destructiveHint=True, which changes no behaviour but stops them depending on the implicit default that caused this. 34 gated -> 30, and the remaining 30 all write to the project. find_gameobjects is deliberately not readOnlyHint=True: it calls preflight(refresh_if_dirty=True), which can trigger a domain reload, and a read-only promise would let a client do that unattended. Add test_tool_annotations.py as the durable guard - it requires every tool to state title and destructiveHint, and pins the auto-approvable set so a future edit cannot silently flip one. Verified it fails by replaying the #480 regression. test_tool_test_symmetry.py now excludes registry-wide guards from counting as per-tool coverage, so one such file cannot satisfy the coverage guard for every tool it happens to mention. Does not fix the whole report: manage_asset(action="search") stays gated because manage_asset can also delete. A read-only find_assets tool is the follow-up.
#1292 declared Codex stdio-only. Tested against Codex CLI 0.47.0 with an isolated CODEX_HOME, that is wrong: a bare [mcp_servers.unityMCP] url = "http://127.0.0.1:8123/mcp" reports `transport: streamable_http` from `codex mcp get`, and Codex completes a full MCP handshake against a live mcp-for-unity HTTP server - initialize 200, notifications/initialized 202, SSE GET 200, tools/list 200 - with no feature flag set at all. Adding [features] rmcp_client, the deprecated root-level experimental_use_rmcp_client, both, or a deliberately bogus feature key all give byte-identical results; unknown feature keys are silently ignored. So #1292 removed a capability Codex has, for every Codex user. Drop SupportsHttpTransport = false (the McpClient default is already true) and delete the SupportedTransports override, since the base default is already { Stdio, Http }. Delete the GetManualSnippet stdio coercion too. It was added by #1292 to stop a stdio-only client rendering a url block, and CodexConfigurator is the only subclass of CodexMcpConfigurator, so once Codex is HTTP-capable that branch is unreachable. Leave [features] rmcp_client = true alone: it is the current key name (the root experimental_use_rmcp_client form is deprecated per openai/codex#6995), it is harmless, and it enables the RMCP client that OAuth needs. Deliberately not adding the deprecated key - it does nothing on current Codex and would just linger in users' configs. Tests now assert both transports and cover the snippet in both directions. Caveat for review: this was verified against the Codex CLI. #1193 was reported against Codex Desktop on Windows 11, which is untested here. #1292's remedy was too broad, which does not mean the reporter was wrong - ask for their version and CLI-vs-Desktop before closing #1193. If Desktop genuinely cannot do HTTP, that belongs in Desktop-specific handling, not a blanket capability removal.
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 -- most resources are `category/thing` but several are flat (`mcpforunity://instances`, `mcpforunity://menu-items`, `mcpforunity://tests`). Several agent-facing strings still named resources without their URI, so an agent following them built `mcpforunity://editor_state` and got a 404: - server instructions listed resources by bare name and told the reader to "poll the `editor_state` resource's `isCompiling` field" (that field path is also wrong -- payloads are wrapped, so it is `data.compilation.is_compiling`) - `refresh_unity`'s `wait_for_ready` parameter description referred to `editor_state.advice.ready_for_tools` - the hint Unity returns in the `refresh_unity` result said "poll editor_state until ready_for_tools is true" #1244 added a warning that names and URIs are not interchangeable, but left the strings that trigger the mistake unchanged. Spell every resource reference as a full URI instead, and correct the field paths while here. Adds a regression test asserting that no agent-facing prose -- server instructions, resource descriptions, tool and parameter descriptions, and multi-word string literals under MCPForUnity/Editor -- mentions a resource by its snake_case name without also giving that resource's URI.
Review feedback on #1302. The prose rule now also runs over the surfaces that tell a reader to go read a resource: the skill agents load, and the per-tool reference pages whose example blocks this PR fixed. Reverting those four lines makes it fail. Scoped there deliberately. `website/docs/reference/resources/` is a generated catalog that puts each name in a heading and its URI on the next line, and the guides and getting-started pages name resources as the subject of a sentence rather than instructing anyone to build a URI -- a blanket scan flags 38 lines, none of them the defect. Also drops the try/except around get_type_hints: it resolves for all 48 registered tools (266 annotated strings), so the except only had the power to skip a tool's parameters silently. Without it a resolution failure surfaces as the real error.
fix: restore HTTP transport for Codex (#1193)
…sing them Copilot review on #1304. The tools fixture keyed a dict by tool name, so two tools registering the same name would drop one entry — hiding the registry bug and skipping the lost entry's annotations, in a guard whose whole purpose is catching silent regressions. No duplicates today (48 registrations, 48 unique names — the 49th @mcp_for_unity_tool occurrence is a docstring mention at services/tools/__init__.py:28, not a decoration), so this is a guard hardening rather than a fix. Also corrects the docstring grammar Copilot flagged.
…753972755 chore: update Unity package to beta version 10.1.1-beta.3
fix: stop 34 tools forcing an approval prompt on every call (#1288)
…755957460 chore: update Unity package to beta version 10.1.1-beta.4
Fork PRs touching MCPForUnity/** get green Unity checks that verified nothing. GitHub withholds secrets from pull_request runs originating in a fork, so the detect step writes unity_ok=false and every real step is gated off. Step-level `if:` produces step-conclusion `skipped`, which contributes nothing to the job conclusion, so the job reports success having compiled and tested nothing. Make the skip unmissable: both workflows now emit ::warning:: and a $GITHUB_STEP_SUMMARY block stating the check is not a pass. Retire the safe-to-test label gate. It was the intended escape hatch but never worked in practice -- actions/checkout's floating v4 tag has since rolled forward to v4.4.0, which refuses to check out fork code under pull_request_target without allow-unsafe-pr-checkout: true. Repairing it would mean running fork-authored C# through game-ci/unity-test-runner with UNITY_* secrets in scope, which is the classic pwn-request shape. Removing the trigger makes both job-level `if:` gates dead code (each began with `github.event_name != 'pull_request_target' ||`), so they go too. To test a fork PR, review the diff and push its branch into this repo; the push trigger runs the full suite in a trusted context. Known tradeoff: the full-matrix label now takes effect on the next push rather than on application, since nothing re-triggers on `labeled`. This does not give fork PRs real signal -- it stops the absence of signal from looking like success. A license-free compile job is the follow-up.
fix(server): address resources by URI in agent-facing prose
ci: retire safe-to-test gate, make skipped Unity checks visible
…756436245 chore: update Unity package to beta version 10.1.1-beta.5
…roperties fix: make component properties reachable on manage_gameobject create
…765582021 chore: update Unity package to beta version 10.1.1-beta.6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated version bump to 10.1.2.