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.
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.
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.
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.
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.
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].
Phases 2-3. Adds a curated AssetGenModelCatalog (image/3D/audio models with
price/duration/use-case metadata; defaults reference the adapter constants so the
panel default equals what an omitted model resolves to). Adds the generate_audio
MCP tool + CLI, threads a per-(kind,provider) selected-model pref so a GUI choice
becomes the default generate_* uses when no model is passed, and makes
Tripo/Meshy consume req.Model. Per-provider prefs (not per-type) so disjoint model
lists never clobber. Compiles on 2021.3 floor; Python + adapter/catalog tests green.
Claude-Session: https://claude.ai/code/session_01GCxmdd4qo7MG6J4M6WcT9Y
A rigged/animated FBX imported through import_model_file always arrived
with zero AnimationClips because the import pipeline hard-coded
ModelImporterAnimationType.None. Thread an optional animation_type param
(none|generic|humanoid|legacy) through the MCP tool, the CLI command, and
the C# pipeline so callers can surface a model's clips; the default stays
None, so existing imports are unchanged. glTF/GLB ignores the knob
(glTFast imports animation itself).
Also fold the tested Blender-handoff findings into the blender-to-unity
skill: GLB-vs-FBX fidelity guidance (with the new
references/bridge-fidelity.md matrix, force-added since .claude is
gitignored), FBX emission restoration steps, and the animation_type knob.
Sync uv.lock with the pyproject 10.0.0 version already on beta.
Claude-Session: https://claude.ai/code/session_01Cya1SZmg7CJgJS61nhjLH4
Live-testing follow-ups from the two-client isolation pass:
- The HTTP refusal only logged the available instances server-side while the
stdio guard lists them inline — an agent hitting the HTTP path needed an
extra mcpforunity://instances hop to act. InstanceSelectionRequiredError now
carries available_instances structurally and appends them to the message,
fetched from the registry at raise time (error path only).
- The transport wrapper's blanket except turned the refusal into hint="retry",
which is misleading: a blind retry fails identically. Selection errors now
return hint="select_instance" with reason and available_instances in data,
so clients can route straight to set_active_instance.
The orphaned-session detector ended an active session on a SINGLE stale
reachability reading, and the reading came from a lone 50ms TCP connect
cached for 0.75s — trivially false-negative on a machine busy with test
runs or domain reloads. Evidence bundles in #1207 show 13 teardowns and
147 socket closures in one session from exactly this loop, wedging the
bridge in no_unity_session churn until manual recovery.
- Require 3 consecutive failed polls (0.75s cadence) before declaring a
session orphaned; probe readings taken while the editor is compiling or
importing don't count toward teardown, and detection is skipped entirely
while busy.
- Raise the probe's connect wait 50ms -> 250ms, as an overall budget shared
across candidate hosts so the worst-case main-thread wait cannot multiply.
- Honor UNITY_MCP_SESSION_RESOLVE_MAX_WAIT_S above 20s (ceiling now 120s;
default unchanged): the old ceiling equalled the default, silently
neutering the documented escape hatch for projects whose reloads or test
boundaries legitimately exceed 20s. Same treatment for
UNITY_MCP_SESSION_READY_WAIT_SECONDS, and both now share one bounded
env-read helper.
The remaining piece of #1207 (keepalive reload-awareness in
WebSocketTransportClient) is untouched here: the resume machinery reworked
in #1234 already covers reload boundaries, and the detector debounce
removes the dominant churn source dsarno identified.
The stdio connection pool's _resolve_instance_id silently returned the
most-recently-heartbeated editor when no unity_instance and no default
were supplied, letting an unbound session retarget another project's
Unity. Mirror the HTTP "multiple connected, no active set" guard: select
the sole instance when there is exactly one, otherwise raise ConnectionError
listing the available ids and requiring an explicit selection.
Pairs with the cherry-picked middleware fix that keys active-instance
state by ctx.session_id instead of the peer-supplied client_id.
UnityInstanceMiddleware previously kept its own dict (`_active_by_key`) keyed
by `client_id | user:{user_id} | "global"`. The MCP protocol's identity is
session-based, not client-based:
- Streamable HTTP uses `Mcp-Session-Id` per the 2025-11-25 spec
- stdio is intrinsically 1:1 (one subprocess = one session)
- `client_id` is the peer-declared Implementation name, which is not
unique — two Cursor instances both report `"cursor"` and collide
The `"global"` fallback compounded this: anonymous clients all collapsed onto
one record, so client A's `set_active_instance` retargeted client B (#1023).
FastMCP already exposes session-isolated state via `ctx.set_state` /
`ctx.get_state`, prefixed by `ctx.session_id` (verified in fastmcp 3.x
`server/context.py:1181-1238` — reads `mcp-session-id` header on HTTP,
generates a per-session UUID on stdio, caches on the session object).
Switching to it removes a whole concurrency surface and closes the bug class
by construction.
Changes:
- Replace `_active_by_key + _lock + get_session_key` with two-liner
set/get/clear_active_instance backed by `ctx.set_state` under
`mcpforunity.active_instance`.
- `set_active_instance` tool now returns `session_id` (the real key) instead
of the bogus `session_key` field.
- `debug_request_context` drops `derived_key` and `all_keys_in_store`
(no global dict to enumerate; per-session reads still report
`active_instance` and the real `session_id`).
- Drop characterization/integration tests that pinned the removed
`client_id → key → "global"` behaviour. Add a regression test that
proves two ctxs with private state dicts can't read each other's
active instance.
No protocol or wire-format change. The behavior shift is invisible to
single-client setups and removes the cross-client leak for multi-client
HTTP deployments.
Add glTFast (com.unity.cloud.gltfast) as an optional dependency in the Dependencies tab
(detect + Install/Remove + bulk Install-All), so GLB generation/import is one click away.
README 'AI Asset Generation' section (providers, BYO-key in the Asset Gen tab, OS secure
store, manage_tools to enable, async tool usage). docs/asset-gen-manual-verification.md
checklist for live validation (real keys + licensed editor; Hunyuan TC3 two-header
caveat). Drop an incidental 'manage_tools' mention from a Phase 0 test comment so the
tool-symmetry quarantine guard stays honest.
Claude-Session: https://claude.ai/code/session_01Tjpb5gYgUe2AUJuRdXr7Lv
Thin key-free pass-throughs in group asset_gen mirroring the C# command names:
camelCase param mapping, None-strip, async status/job_id (manage_packages shape).
CLI group 'asset-gen' registered in cli/main.py. No key/secret param exists on any
tool; tests assert sent payloads carry no key/secret/token. 34 tests pass.
Claude-Session: https://claude.ai/code/session_01Tjpb5gYgUe2AUJuRdXr7Lv
- Register 'asset_gen' tool group (off by default, parity with vfx/animation)
- EditorPrefKeys.AssetGen.* consts (non-secret config only; keys go to secure store)
- AssetGenPrefs helper (provider/format/output-root/normalize/enabled) + EditMode tests
- Python scaffold test asserts group present and disabled by default
Claude-Session: https://claude.ai/code/session_01Tjpb5gYgUe2AUJuRdXr7Lv
- local_harness: fail fast (exit 2) on empty --legs and on --ci without
UNITY_IMAGE, instead of silently exiting green / crashing with an
unhandled CalledProcessError
- local_harness: correct the nearest-patch `best` type annotation
(key is (patch:int, suffix:str), not a 3-tuple)
- test_tool_test_symmetry: fix the module docstring to match behavior
(only test_*.py files are scanned; non-test_*.py scripts like
tests/e2e/bridge_smoke.py do not count toward coverage)
Deferred CodeRabbit's SHA-pin suggestion for e2e-bridge.yml: the repo pins
actions by tag (@v4/@v6), not SHA, so pinning one workflow would be
inconsistent — left as a repo-wide policy decision.
- Server/tests/test_tool_test_symmetry.py discovers every module exposing an
@mcp_for_unity_tool and fails CI if it is not referenced by any test
- A KNOWN_UNTESTED quarantine lets pre-existing gaps (execute_menu_item,
manage_shader, manage_tools) skip rather than fail; a second test fails if a
quarantine entry is stale, so the list can only shrink
Operationalizes the CLAUDE.md rule that every new tool ships with a test.
A one-command, no-API-key end-to-end gate for the Python<->Unity bridge,
runnable locally and in CI.
- tools/local_harness.py: boots a headless Hub-licensed Editor (or attaches
with --reuse) and runs smoke + EditMode + PlayMode legs over the bridge,
aggregating JUnit; exit codes 0-5 (pass / regression / unreachable /
no-compile / no-license / no-editor)
- Server/tests/e2e/bridge_smoke.py: deterministic no-LLM contract driver over
the real wire path; the no-LLM counterpart to claude-nl-suite.yml
- .github/workflows/e2e-bridge.yml: PR gate booting headless Unity in CI;
self-skips (warns) when Unity license secrets are absent
- .github/workflows/python-tests.yml: run the hermetic harness unit tests and
add tools/** to the path triggers
- tools/tests/test_local_harness.py: 69 hermetic unit tests for the harness's
Unity-free decision logic (discovery, version resolution, exit-code mapping)
- Document the harness in CLAUDE.md and the contributor docs
targetGo.isStatic returns true when *any* flag is set, so a partially
static object (e.g. only Navigation) would skip the update when
is_static=true was requested. Compare GetStaticEditorFlags() against
the desired flags instead.
Also adds a "false" string coercion test per review feedback.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the old 5-action read-only profiler with a comprehensive tool
covering session control, generic counter reads, memory snapshots, and
Frame Debugger. Adds "profiling" as a new opt-in tool group.
Adds save_prefab_stage to manage_editor to complete the prefab stage
workflow alongside the existing open_prefab_stage and close_prefab_stage.
- C#: SavePrefabStage() uses EditorSceneManager.MarkSceneDirty +
SaveScene on the prefab stage scene, returns ErrorResponse when no
stage is open or save fails
- Python: adds save_prefab_stage to action Literal and tool description
- Tests: 3 new tests covering forwarding, description, and clean params
open_prefab_stage was already merged in #968. This PR only adds
save_prefab_stage.
Andriy Mykhaylyshyn