Address CodeRabbit review on #1320:
- _env_float now requires math.isfinite(value): "inf"/"Infinity"/"1e309"/
"nan" are positive-or-parseable but would produce unusable socket/timeout
behaviour, so they now fall back to the default like other invalid input.
- test_config_default_values clears the two env vars first so ambient env
can't mask the defaults.
- Added coverage for valid overrides and for invalid/zero/negative/
non-finite values falling back to defaults.
Long-running tool calls (asset imports, test runs, batched edits) were
cut off ~30-90s into execution, so the task could never finish. On the
stdio transport this was governed by hardcoded values on both hops:
- Unity side: StdioBridgeHost.FrameIOTimeoutMs (30s const) capped every
command's execution and frame I/O; on timeout the client reconnected
and re-sent, which force-closed the prior client and made the bridge
restart on a new port (the repeated "StdioBridgeHost started on port
6400/6402" churn).
- Server side: ServerConfig.connection_timeout (30s socket recv) and
command_total_timeout (90s cross-retry ceiling) cut the command off
first.
Unlike the WebSocket transport (WebSocketTransportClient reads a per-call
timeout off the wire), the stdio bridge had no way to raise these.
Make all three configurable with a 5-minute default:
- FrameIOTimeoutMs: 30s -> 300s, env UNITY_MCP_STDIO_COMMAND_TIMEOUT_MS.
ReceiveTimeout now scales with it (max(60s, timeout)).
- connection_timeout: 30s -> 300s, env UNITY_MCP_CONNECTION_TIMEOUT.
- command_total_timeout: 90s -> 600s, env UNITY_MCP_COMMAND_TOTAL_TIMEOUT.
Invalid/non-positive env values fall back to the default so a bad
override can't disable the timeout. Updates the config characterization
test to the new defaults.
Every PR to this repo comes from a fork -- including maintainers', who
work from Scriptwonder/unity-mcp. GitHub withholds secrets from runs
triggered by a fork's pull request, so unity-tests.yml skips and reports
a green check having compiled nothing. Across the last 60 runs of that
workflow: 51 pull_request (all skipped), 9 pull_request_target, and zero
push/workflow_call. No PR has ever been compile-verified.
Unity refuses to open a project without an activated license, so there is
no way to compile via the Editor without secrets. This sidesteps that: it
never launches Unity. It pulls the PUBLIC unityci/editor image purely to
read reference assemblies out of it, and drives Roslyn -- Unity's own
bundled csc -- directly. A compiler needs no license.
Compiles MCPForUnity.Runtime and MCPForUnity.Editor for win, osx and
linux. The existing Unity matrix is linux-only, so the UNITY_EDITOR_WIN
and UNITY_EDITOR_OSX branches have never been compiled anywhere in CI.
Runtime targets netstandard2.1 and Editor targets .NET 4.8, matching what
Unity does; feeding both BCLs to one compile double-defines System.Object.
Reference manifests and defines are captured from Unity's generated
csprojs rather than globbed: Editor/Data holds the whole .NET 4.8 BCL plus
vendored libraries Unity deliberately does not reference (ExCSS.Unity
redefines System.Tuple, cscompmgd.dll redefines Microsoft.CSharp.
CompilerError). Both need regenerating when defaultVersion changes.
Verified locally against 2021.3.45f2: all six compiles pass, and a probe
error behind #if UNITY_EDITOR_OSX fails the osx pass with exit 1 while
win and linux stay green.
This does not replace unity-tests.yml -- it compiles, it does not run
tests.
Measured four concurrent agents against one Editor over the HTTP
transport (527 client calls, 1498 commands through the bridge at
97.9% success). The Editor stayed healthy throughout, so the notes
below are about degradation modes, not failure.
Guide gains a section on running several agents at once: a single
Editor executes one command at a time, so reads stretched from ~5s
to ~17s behind another agent's writes and batching bought nothing
(~2-3s per call regardless of batch size). Also documents that
'Available: none' is a transient reconnect artifact rather than an
absent Editor, and tabulates when a failed call is safe to retry.
Architecture doc records two gaps found while measuring. Instance
resolution runs before dispatch and hard-fails while the registry
is briefly empty during a domain reload, which was roughly seven in
ten client-visible errors. And a timed-out command is not cancelled
in Unity: send_command pops the pending future, so a command already
executing runs to completion and its result is discarded, reporting
failure for an effect that landed. Commands still queued when the
socket drops are lost cleanly instead.
Also corrects the claim that throughput had not been measured; the
numbers support the existing no-broker decision, since serialization
comes from Unity's single-threaded main loop rather than the layer a
broker would occupy.
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.
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.
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.
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.
#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.
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.
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.
github-actions[bot]