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.
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.
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].
- McpToolsSection: add the asset_gen -> "Asset Gen" group display name so
the tool-group tab no longer falls back to the raw "Asset_gen".
- Update the asset_gen group blurb (registry + CLI) to include audio gen.
- Regenerate the tool reference docs from the Python registry: add the
missing generate_audio.md and refresh the asset_gen landscape/index.
Claude-Session: https://claude.ai/code/session_015KYy51gwBuhDuLZXXoqc98
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
CodeRabbit follow-up on #1262: the API-level default is omitting the
parameter (nothing crosses the wire); passing 'none' explicitly maps to
the same no-rig import. Say "omitted or 'none'" instead of calling
'none' the default, and regenerate the reference page.
Claude-Session: https://claude.ai/code/session_01Cya1SZmg7CJgJS61nhjLH4
Review responses (CodeRabbit + Copilot):
- Document the 'legacy' animation_type value in the C# XML summary, MCP tool
description, param help, and CLI help (Copilot x4).
- SKILL.md: drop multi-material zones as a GLB-only trigger (both formats keep
slots), define out_glb in the glTF branch example, document the full
animation_type contract, probe both Principled emission socket names
('Emission Color' 4.x / 'Emission' 3.x), keep use_active_scene=True in the
Notes GLB example, and distinguish FBX vertex-color data transfer from URP
Lit shader display.
- bridge-fidelity.md: make export_apply conditional (static-only; skinned/
shape-key meshes need False) and document export_animations' active/NLA-
stashed semantics.
Regenerate website/docs/reference for the updated tool description — fixes the
"Check docs reference is fresh" CI failure ("differs: asset_gen/import_model_file.md").
Claude-Session: https://claude.ai/code/session_01Cya1SZmg7CJgJS61nhjLH4
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.
Add a "Reading resources" section to the FastMCP server instructions
string. It states that resources are addressed by URI (slashes), never
by name (underscores); that the exact URI should be taken from the
client's resources/list rather than constructed from the name; and that
payloads are wrapped under a top-level `data` object (field paths look
like data.<section>.<field>).
Prevents the common naive failure where an agent turns a resource name
into a URI (e.g. editor_state -> mcpforunity://editor_state) and 404s,
and the matching failure of guessing a bare top-level field instead of
the nested data.* path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Security review + code review of the asset-gen feature surfaced concrete issues;
this fixes them and adds regression tests (request-shaping layer, FakeHttpTransport).
Security
- SafeZipExtractor enforces an extension allowlist; ModelImportPipeline passes an
inert model/texture allowlist so a provider archive can't drop a .cs/.dll under
Assets/ and have the Editor compile/load it (code execution on import).
- AssetGenJobManager refuses non-http(s) download URLs before fetching
(file:// SSRF / local-file read into the project).
Provider correctness
- Meshy image->3D polls /openapi/v1/image-to-3d/{id} (was the v2 text URL).
- Meshy text->3D honors texture=true via the preview->refine two-phase flow.
- OpenRouter image->image attaches the reference image (content image_url part).
- fal image->image uses the /edit endpoint + image_urls array; width/height
forwarded as image_size.
- Sketchfab search forwards categories/count/cursor/downloadable; preview doc
corrected (returns metadata, not a base64 thumbnail).
- Job import calls AssetDatabase.Refresh() before importing a freshly written file.
Local image input (image_path)
- New LocalImage helper; image_path is read and sent inline as a base64 data URI
for Meshy / fal / OpenRouter. Tripo rejects local images with a clear error
(needs a hosted image_url; its upload flow is not wired).
Cleanup (no behavior change)
- Shared AssetGenPaths + ProviderHttp helpers, HttpResult.Ok, MissingKeyMessage,
cached glTFast probe, dead-field / per-frame-alloc removal, CLI _emit.
Docs: README + manual-verification updated (image_path support; transparency is
import-flag-only; width/height fal-only).
Verified: package compiles clean; Python 1306 passed / 3 skipped. Meshy refine,
fal /edit, and image_path data-URI paths are unit-tested at the request layer
only -- live smoke per provider (real keys) still pending.
Claude-Session: https://claude.ai/code/session_015DAUrMR5UaSEzEn2wNPrEP
Drop Hunyuan (Tencent TC3-HMAC) — high effort and not testable by the maintainer:
delete HunyuanAdapter + TencentCloud3Signer (+tests), unwire from AssetGenProviders,
SecureKeyStoreConstants, the GUI provider list, and Python/CLI/README/manual-verify docs.
Refresh model defaults to current (2026) SOTA, verified against provider docs:
- fal: fal-ai/flux/dev -> fal-ai/flux-2 (FLUX.2 dev; cheaper and higher quality)
- Tripo: v2.5-20250123 -> v3.1-20260211 (current recommended model)
- Meshy: drop deprecated art_style (errors on Meshy-6); pin ai_model=meshy-6
- OpenRouter: gemini-2.5-flash-image-preview (delisted) -> gemini-2.5-flash-image
Fix two tests that asserted the now-implemented 'meshy' provider throws (repointed to the
now-removed 'hunyuan'). They were silently wrong: the headless --full runs never executed
EditMode tests (Unity -quit exits before -runTests; compile-only). A real -runTests run
now passes 62/62 AssetGen EditMode tests; Python suite 1299 passed.
Claude-Session: https://claude.ai/code/session_01Tjpb5gYgUe2AUJuRdXr7Lv
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
README-zh.md edits + .meta/uv.lock churn on brand-distribution-analytics, committed
(not stashed) so the feature branch can be checked out in the main worktree for editor
testing. Untracked .agents/ and AGENTS.md intentionally left out.
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.
Andriy Mykhaylyshyn