Commit Graph

10 Commits (edd7817d4010032b47e6874c165c8d0b37680407)

Author SHA1 Message Date
Marcus Sanatan 2649d9c379
Move Get commands to editor resources + Run Python tests every update (#368)
* Add a function to reload the domain

Closes #357

* feat: restructure server instructions into workflow-focused format

- Reorganized instructions from flat bullet list into categorized workflow sections
- Emphasized critical script management workflow with numbered steps
- Improved readability and scannability for AI agents using the MCP server

It doesn't make sense to repeat the fucnction tools, they're already parsed

* docs: reorder tool list alphabetically in README + add reload_domain tool

* feat: add Unity editor state and project info resources

- Implemented resources for querying active tool, editor state, prefab stage, selection, and open windows
- Added project configuration resources for layers and project metadata
- Organized new resources into Editor and Project namespaces for better structure

* feat: clarify script management workflow in system prompt

- Expanded guidance to include scripts created by any tool, not just manage_script
- Added "etc" to tools examples for better clarity

* refactor: remove reload_domain tool and update script management workflow

- Removed reload_domain tool as Unity automatically recompiles scripts when modified
- Updated script management instructions to rely on editor_state polling and console checking instead of manual domain reload
- Simplified workflow by removing unnecessary manual recompilation step

* Change name of menu items resource as the LLM seems it

* refactor: reorganize tests into src/tests/integration directory

- Moved all test files from root tests/ to MCPForUnity/UnityMcpServer~/src/tests/integration/ for better organization
- Added conftest.py with telemetry and dependency stubs to simplify test setup
- Removed redundant path manipulation and module loading code from individual test files

* feat: expand Unity test workflow triggers

- Run tests on all branches instead of only main
- Add pull request trigger to catch issues before merge
- Maintain path filtering to run only when relevant files change

* chore: add GitHub Actions workflow for Python tests

- Configured automated testing on push and pull requests using pytest
- Set up uv for dependency management and Python 3.10 environment
- Added test results artifact upload for debugging failed runs

* refactor: update import path for fastmcp Context

* docs: update development setup instructions to use uv

- Changed installation commands from pip to uv pip for better dependency management
- Updated test running instructions to use uv run pytest
- Added examples for running integration and unit tests separately

* Formatting [skip ci]

* refactor: optimize CI workflow with path filters and dependency installation

- Added path filters to only trigger tests when Python source or workflow files change
- Split dependency installation into sync and dev install steps for better clarity
- Fixed YAML indentation for improved readability

* Update .github/workflows/python-tests.yml

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix: standardize test mode values to match Unity's naming convention

- Changed default mode from "edit" to "EditMode" in C# code
- Updated Python tool to use "EditMode" and "PlayMode" instead of lowercase variants

* refactor: convert test imports to relative imports

- Changed absolute imports to relative imports in integration tests for better package structure
- Removed test packages from pyproject.toml package list

* refactor: use Field with default_factory for mutable default in TagsResponse

* refactor: remove duplicate PrefabStageUtility call

* Update this as well [skip ci]

* Update MCPForUnity/UnityMcpServer~/src/tests/integration/test_script_tools.py

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* chore: remove pull_request triggers from test workflows [skip ci]

It's already covered by pushes

* refactor: update resource function return types to include MCPResponse union

* refactor: remove manual domain reload tool

- Removed reload_domain tool as Unity handles script recompilation automatically
- Updated documentation to reflect automatic compilation workflow
- Simplified script management workflow instructions in server description

* refactor: add context support to resource handlers

- Updated all resource handlers to accept Context parameter for Unity instance routing
- Replaced direct async_send_command_with_retry calls with async_send_with_unity_instance wrapper
- Added imports for get_unity_instance_from_context and async_send_with_unity_instance helpers

* fix: correct grammar in menu items documentation

* docs: update README with expanded tools and resources documentation

- Added new tools: manage_prefabs, create_script, delete_script, get_sha
- Added new resources: editor state, windows, project info, layers, and tags
- Clarified manage_script as compatibility router with recommendation to use newer edit tools
- Fixed run_test to run_tests for consistency

* refactor: convert unity_instances function to async [skip ci]

- Changed function signature from synchronous to async
- Added await keywords to ctx.info() and ctx.error() calls to properly handle async context methods

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2025-11-05 16:06:48 -04:00
dsarno f667582505
Feature/session based instance routing (#369)
* Add support for multiple Unity instances

* fix port detection

* add missing unity_instance parameter

* add instance params for resources

* Fix CodeRabbit review feedback

- Fix partial framed response handling in port discovery
  Add _recv_exact() helper to ensure complete frame reading
  Prevents healthy Unity instances from being misidentified as offline

- Remove unused default_conn variables in server.py (2 files)
  Fixes Ruff F841 lint error that would block CI/CD

- Preserve sync/async nature of resources in wrapper
  Check if original function is coroutine before wrapping
  Prevents 'dict object is not awaitable' runtime errors

- Fix reconnection to preserve instance_id
  Add instance_id tracking to UnityConnection dataclass
  Reconnection now targets the same Unity instance instead of any available one
  Prevents operations from being applied to wrong project

- Add instance logging to manage_asset for debugging
  Helps troubleshoot multi-instance scenarios

🤖 Generated with Claude Code
Co-Authored-By: Claude <noreply@anthropic.com>

* Fix CodeRabbit feedback: reconnection fallback and annotations safety

Address 3 CodeRabbit review comments:

1. Critical: Guard reconnection fallback to prevent wrong instance routing
   - When instance_id is set but rediscovery fails, now raises ConnectionError
   - Added 'from e' to preserve exception chain for better debugging
   - Prevents silently connecting to different Unity instance
   - Ensures multi-instance routing integrity

2. Minor: Guard __annotations__ access in resource registration
   - Use getattr(func, '__annotations__', {}) instead of direct access
   - Prevents AttributeError for functions without type hints

3. Minor: Remove unused get_type_hints import
   - Clean up unused import in resources/__init__.py

All changes applied to both Server/ and MCPForUnity/ directories.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix instance sorting and logging issues

- Fix sorting logic for instances without heartbeat data: use epoch timestamp instead of current time to properly deprioritize instances with None last_heartbeat
- Use logger.exception() instead of logger.error() in disconnect_all() to include stack traces for better debugging

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* update uv.lock to prepare for merging into main

* Restore Python 3.10 lockfiles and package list_unity_instances tool

* Deduplicate Unity instance discovery by port

* Scope status-file reload checks to the active instance

* refactor: implement FastMCP middleware for session-based instance routing

Replaces module-level session_state.py with UnityInstanceMiddleware class
that follows FastMCP best practices. Middleware intercepts all tool calls
via on_call_tool hook and injects active Unity instance into request state.

Key changes:
- Add UnityInstanceMiddleware class with on_call_tool hook
- Tools now use ctx.get_state("unity_instance") instead of direct session_state calls
- Remove unity_instance parameter from all tool schemas to prevent LLM hallucination
- Convert list_unity_instances tool to unity_instances resource (read-only data)
- Update error messages to reference unity://instances resource
- Add set_state/get_state methods to DummyContext test helper
- All 67 tests passing (55 passed, 5 skipped, 7 xpassed)

Architecture benefits:
- Centralized session management in middleware
- Standard FastMCP patterns (middleware + request state)
- Cleaner separation of concerns
- Prevents AI hallucination of invalid instance IDs

* fix: convert resource templates to static resources for discoverability

Convert MCP resources from URI templates with query parameters to static
resources to fix discoverability in MCP clients like Claude Code.

Changes:
- Remove {?force_refresh} from unity://instances
- Remove {?unity_instance} from mcpforunity://menu-items
- Remove {?unity_instance} from mcpforunity://tests
- Keep {mode} path parameter in mcpforunity://tests/{mode} (legitimate)

Root cause: Query parameters {?param} trigger ResourceTemplate registration,
which are listed via resources/templates/list instead of resources/list.
Claude Code's ListMcpResourcesTool only queries resources/list, making
templates undiscoverable.

Solution: Remove optional query parameters from URIs. Instance routing is
handled by middleware/context, and force_refresh was cache control that
doesn't belong in resource identity.

Impact: Resources now discoverable via standard resources/list endpoint and
work with all MCP clients including Claude Code and Cursor.

Requires FastMCP >=2.13.0 for proper RFC 6570 query parameter support.

* feat: improve material properties and sync Server resources

Material Property Improvements (ManageAsset.cs):
- Add GetMainColorPropertyName() helper that auto-detects shader color properties
- Tries _BaseColor (URP), _Color (Standard), _MainColor, _Tint, _TintColor
- Update both named and array color property handling to use auto-detection
- Add warning messages when color properties don't exist on materials
- Split HasProperty check from SetColor to enable error reporting

This fixes the issue where simple color array format [r,g,b,a] defaulted to
_Color property, causing silent failures with URP Lit shader which uses _BaseColor.

Server Resource Sync:
- Sync Server/resources with MCPForUnity/UnityMcpServer~/src/resources
- Remove query parameters from resource URIs for discoverability
- Use session-based instance routing via get_unity_instance_from_context()

* fix: repair instance routing and simplify get_unity_instance_from_context

PROBLEM:
Instance routing was failing - scripts went to wrong Unity instances.
Script1 (intended: ramble) -> went to UnityMCPTests 
Script2 (intended: UnityMCPTests) -> went to ramble 

ROOT CAUSE:
Two incompatible approaches for accessing active instance:
1. Middleware: ctx.set_state() / ctx.get_state() - used by most tools
2. Legacy: ctx.request_context.meta - used by script tools
Script tools were reading from wrong location, middleware had no effect.

FIX:
1. Updated get_unity_instance_from_context() to read from ctx.get_state()
2. Removed legacy request_context.meta code path (98 lines removed)
3. Single source of truth: middleware state only

TESTING:
- Added comprehensive test suite (21 tests) covering all scenarios
- Tests middleware state management, session isolation, race conditions
- Tests reproduce exact 4-script failure scenario
- All 88 tests pass (76 passed + 5 skipped + 7 xpassed)
- Verified fix with live 4-script test: 100% success rate

Files changed:
- Server/tools/__init__.py: Simplified from 75 lines to 15 lines
- MCPForUnity/UnityMcpServer~/src/tools/__init__.py: Same simplification
- tests/test_instance_routing_comprehensive.py: New comprehensive test suite

* refactor: standardize instance extraction and remove dead imports

- Standardize all 18 tools to use get_unity_instance_from_context() helper
  instead of direct ctx.get_state() calls for consistency
- Remove dead session_state imports from with_unity_instance decorator
  that would cause ModuleNotFoundError at runtime
- Update README.md with concise instance routing documentation

* fix: critical timezone and import bugs from code review

- Remove incorrect port safety check that treated reclaimed ports as errors
  (GetPortWithFallback may legitimately return same port if it became available)
- Fix timezone-aware vs naive datetime mixing in unity_connection.py sorting
  (use timestamp() for comparison to avoid TypeError)
- Normalize all datetime comparisons in port_discovery.py to UTC
  (file_mtime and last_heartbeat now consistently timezone-aware)
- Add missing send_with_unity_instance import in Server/tools/manage_script.py
  (was causing NameError at runtime on lines 108 and 488)

All 88 tests pass (76 passed + 5 skipped + 7 xpassed)

---------

Co-authored-by: Sakura <sakurachan@qq.com>
Co-authored-by: Claude <noreply@anthropic.com>
2025-11-05 09:43:36 -08:00
Shutong Wu 8f227ff5be
Update .Bat file and Bug fix on ManageScript (#355)
* Update .Bat file and Bug fix on ManageScript

* Update the .Bat file to include runtime folder
* Fix the inconsistent EditorPrefs variable so the GUI change on Script Validation could cause real change.

* Further changes

String to Int for consistency
2025-10-25 12:27:07 -04:00
dsarno faf9affc36
fix: JSON material property handling + tests (manage_asset) #90 (#349)
* feat: add JSON property handling for materials; add tests for JSON coercion and end-to-end; update test project manifest and ProjectVersion

* fix(manage_asset): support structured texture blocks case-insensitively; resolve _BaseMap/_MainTex automatically and apply when missing name

* Update MCPForUnity/Editor/Tools/ManageAsset.cs

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* refactor(manage_asset): remove goto; reuse alias resolver for structured texture (supports 'albedo'); tests: self-sufficient texture asset and _BaseColor/_Color guards; python: assert success in invalid JSON case

* chore(manage_asset): remove duplicate return in modify case

* tests: fix mocks/patching for manage_asset/manage_gameobject; make invalid-json case tolerant; ensure prefab modify test patches transport correctly

* ci: allow manual dispatch for Unity EditMode tests (workflow_dispatch)

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
2025-10-24 11:43:26 -07:00
David Sarno 9796f8eda9 fix: add missing JsonUtil.cs.meta file to resolve package import errors 2025-10-23 19:42:58 -07:00
dsarno 075d68dba3
Material tools: support direct shader property keys + add EditMode coverage (#344)
* Add TDD tests for MCP material management issues

- MCPMaterialTests.cs: Tests for material creation, assignment, and data reading
- MCPParameterHandlingTests.cs: Tests for JSON parameter parsing issues
- SphereMaterialWorkflowTests.cs: Tests for complete sphere material workflow

These tests document the current issues with:
- JSON parameter parsing in manage_asset and manage_gameobject tools
- Material creation with properties
- Material assignment to GameObjects
- Material component data reading

All tests currently fail (Red phase of TDD) and serve as specifications
for what needs to be fixed in the MCP system.

* Refine TDD tests to focus on actual MCP tool parameter parsing issues

- Removed redundant tests that verify working functionality (GameObjectSerializer, Unity APIs)
- Kept focused tests that document the real issue: MCP tool parameter validation
- Tests now clearly identify the root cause: JSON string parsing in MCP tools
- Tests specify exactly what needs to be fixed: parameter type flexibility

The issue is NOT in Unity APIs or serialization (which work fine),
but in MCP tool parameter validation being too strict.

* Fix port discovery protocol mismatch

- Update _try_probe_unity_mcp to recognize Unity bridge welcome message
- Unity bridge sends 'WELCOME UNITY-MCP' instead of JSON pong response
- Maintains backward compatibility with JSON pong format
- Fixes MCP server connection to Unity Editor

* Resolve merge: unify manage_gameobject param coercion and schema widening

* Tests: add MaterialParameterToolTests; merge port probe fix; widen tool schemas for JSON-string params

* feat(material): support direct shader property keys and texture paths; add EditMode tests

* chore(tests): track required .meta files; remove ephemeral Assets/Editor test helper

* fix(manage_gameobject): validate parsed component_properties is a dict; return clear error
2025-10-23 18:25:29 -07:00
dsarno a0287afbbc
Harden MCP tool parameter handling + add material workflow tests (TDD) (#343)
* Add TDD tests for MCP material management issues

- MCPMaterialTests.cs: Tests for material creation, assignment, and data reading
- MCPParameterHandlingTests.cs: Tests for JSON parameter parsing issues
- SphereMaterialWorkflowTests.cs: Tests for complete sphere material workflow

These tests document the current issues with:
- JSON parameter parsing in manage_asset and manage_gameobject tools
- Material creation with properties
- Material assignment to GameObjects
- Material component data reading

All tests currently fail (Red phase of TDD) and serve as specifications
for what needs to be fixed in the MCP system.

* Refine TDD tests to focus on actual MCP tool parameter parsing issues

- Removed redundant tests that verify working functionality (GameObjectSerializer, Unity APIs)
- Kept focused tests that document the real issue: MCP tool parameter validation
- Tests now clearly identify the root cause: JSON string parsing in MCP tools
- Tests specify exactly what needs to be fixed: parameter type flexibility

The issue is NOT in Unity APIs or serialization (which work fine),
but in MCP tool parameter validation being too strict.

* Fix port discovery protocol mismatch

- Update _try_probe_unity_mcp to recognize Unity bridge welcome message
- Unity bridge sends 'WELCOME UNITY-MCP' instead of JSON pong response
- Maintains backward compatibility with JSON pong format
- Fixes MCP server connection to Unity Editor

* Resolve merge: unify manage_gameobject param coercion and schema widening

* Tests: add MaterialParameterToolTests; merge port probe fix; widen tool schemas for JSON-string params

* refactor: extract JSON coercion helper; docs + exception narrowing\nfix: fail fast on bad component_properties JSON\ntest: unify setup, avoid test-to-test calls\nchore: use relative MCP package path

* chore(tests): track MCPToolParameterTests.cs.meta to keep GUID stable

* test: decouple MaterialParameterToolTests with helpers (no inter-test calls)
2025-10-23 17:57:27 -07:00
Marcus Sanatan f2c57ca91e
Add testing and move menu items to resources (#316)
* deps: add tomli>=2.3.0 dependency to UnityMcpServer package

* feat: dynamically fetch package version from pyproject.toml for telemetry

* Add pydantic

* feat: add resource registry for MCP resource auto-discovery

* feat: add telemetry decorator for tracking MCP resource usage

* feat: add auto-discovery and registration system for MCP resources

* feat: add resource registration to MCP server initialization

* feat: add MCPResponse model class for standardized API responses

* refactor: replace Debug.Log calls with McpLog wrapper for consistent logging

* feat: add test discovery endpoints for Unity Test Framework integration

We haven't connected them as yet, still thinking about how to do this neatly

* Fix server setup

* refactor: reduce log verbosity by changing individual resource/tool registration logs to debug level

* chore: bump mcp[cli] dependency from 1.15.0 to 1.17.0

* refactor: remove Context parameter and add uri keyword argument in resource decorator

The Context parameter doesn't work on our version of FastMCP

* chore: upgrade Python base image to 3.13 and simplify Dockerfile setup

* fix: apply telemetry decorator before mcp.tool to ensure proper wrapping order

* fix: swap order of telemetry and resource decorators to properly wrap handlers

* fix: update log prefixes for consistency in logging methods

* Fix compile errors

* feat: extend command registry to support both tools and resources

* Run get tests as a coroutine because it doesn't return results immediately

This works but it spams logs like crazy, maybe there's a better/simpler way

* refactor: migrate from coroutines to async/await for test retrieval and command execution

* feat: add optional error field to MCPResponse model

* Increased timeout because loading tests can take some time

* Make message optional so error responses that only have success and error don't cause Pydantic errors

* Set max_retries to 5

This connection module needs a lookover. The retries should be an exponential backoff and we could structure why it's failing so much

* Use pydantic model to structure the error output

* fix: initialize data field in GetTestsResponse to avoid potential errors

* Don't return path parameter

* feat: add Unity test runner execution with structured results and Python bindings

* refactor: simplify GetTests by removing mode filtering and related parsing logic

* refactor: move test runner functionality into dedicated service interface

* feat: add resource retrieval telemetry tracking with new record type and helper function

* fix: convert tool functions to async and await ctx.info calls

* refactor: reorganize menu item functionality into separate execute and get commands

An MCP resource for retrieval, and a simple command to execute. Because it's a resource, it's easier for the user to see what's in the menu items

* refactor: rename manage_menu_item to execute_menu_item and update tool examples to use async/await

We'll eventually put a section for resources

* Revert "fix: convert tool functions to async and await ctx.info calls"

This reverts commit 012ea6b7439bd1f2593864d98d03d9d95d7bdd03.

* fix: replace tomllib with tomli for Python 3.10 compatibility in telemetry module

* Remove confusing comment

* refactor: improve error handling and simplify test retrieval logic in GetTests commands

* No cache by default

* docs: remove redundant comment for HandleCommand method in ExecuteMenuItem
2025-10-13 11:16:43 -04:00
Marcus Sanatan 1d6d8c67af
New UI and work without MCP server embedded (#313)
* First pass at new UI

* Point to new UI

* Refactor: New Service-Based MCP Editor Window Architecture

We separate the business logic from the UI rendering of the new editor window with new services.
I didn't go full Dependency Injection, not sure if I want to add those deps to the install as yet, but service location is fairly straightforward.

Some differences with the old window:

- No more Auto-Setup, users will manually decide what they want to do
- Removed Python detection warning, we have a setup wizard now
- Added explicit path overrides for `uv` and the MCP server itself

* style: add flex-shrink and overflow handling to improve UI element scaling

* fix: update UI configuration and visibility when client status changes

* feat: add menu item to open legacy MCP configuration window

* refactor: improve editor window lifecycle handling with proper update subscription

* feat: add auto-verification of bridge health when connected

* fix: update Claude Code MCP server registration to use lowercase unityMCP name and correct the manual installation instructions

* fix: add Claude CLI directory to PATH for node/nvm environments

* Clarify how users will see MCP tools

* Add a keyboard shortcut to open the window

* feat: add server download UI and improve installation status messaging

This is needed for the Unity Asset Store, which doesn't have the Python server embedded.

* feat: add dynamic asset path detection to support both Package Manager and Asset Store installations

* fix: replace unicode emojis with escaped characters in status messages

* feat: add server package creation and GitHub release publishing to version bump workflow

* fix: add v prefix to server package filename in release workflow

* Fix download location

* style: improve dropdown and settings layout responsiveness with flex-shrink and max-width

* feat: add package.json version detection and refactor path utilities

* refactor: simplify imports and use fully qualified names in ServerInstaller.cs

* refactor: replace Unity Debug.Log calls with custom McpLog class

* fix: extract server files to temp directory before moving to final location

* docs: add v6 UI documentation and screenshots with service architecture overview

* docs: add new UI Toolkit-based editor window with service architecture and path overrides

* feat: improve package path resolution to support Package Manager and Asset Store installations

* Change Claude Code's casing back to "UnityMCP"

There's no need to break anything as yet

* fix: update success dialog text to clarify manual bridge start requirement

* refactor: move RefreshDebounce and ManageScriptRefreshHelpers classes inside namespace

* feat: add Asset Store fallback path detection for package root lookup

* fix: update server installation success message to be more descriptive

* refactor: replace Unity Debug.Log calls with custom McpLog utility

* fix: add file existence check before opening configuration file

* refactor: simplify asset path handling and remove redundant helper namespace references

* docs: update code block syntax highlighting in UI changes doc

* docs: add code block syntax highlighting for file structure example

* feat: import UnityEditor.UIElements namespace for UI components for Unity 2021 compatibility

* refactor: rename Python server references to MCP server for consistency

* fix: reset client status label color after error state is cleared

* Replace the phrase "Python server" with "MCP server"

* MInor doc clarification

* docs: add path override methods for UV and Claude CLI executables

* docs: update service locator registration method name from SetCustomImplementation to Register
2025-10-11 03:08:16 -04:00
Marcus Sanatan e9b1ae44c5
Rename plugin folder to MCPForUnity (#303)
* Copy UnityMcpBridge into a new MCPForUnity folder

This is to close #284

* refactor: rename UnityMcpBridge directory to MCPForUnity in docs

* chore: rename UnityMcpBridge directory to MCPForUnity across workflow files

* chore: rename UnityMcpBridge directory to MCPForUnity across all files

* refactor: update import paths from UnityMcpBridge to MCPForUnity across test files

* fix: update module import paths to use MCPForUnity instead of UnityMcpBridge

* chore: update unity-mcp package path to MCPForUnity directory

* feat: add OneTimeSetUp to initialize CommandRegistry before tests run

Hopefully fix the CI failures

* Apply recent fix to new folder

* Temporarily trigger tests to see if CI works

* Revert "Temporarily trigger tests to see if CI works"

It works!

This reverts commit 8c6eaaad07545cef047769f2c52fe506545a8161.
2025-10-03 20:23:28 -04:00