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.
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.
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.
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.
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.
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].
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.
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.
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.
Shutong Wu