-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Large Cleanup and Refactor + Many new Tests added #642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 28 commits
Commits
Show all changes
114 commits
Select commit
Hold shift + click to select a range
a3ead02
docs: Add codebase overview and comprehensive refactor plan
dsarno c1c0c30
More stuff for cleanup
dsarno 13d85bb
docs: Document null parameter handling inconsistency and test validatβ¦
dsarno 7a2a95f
fix: Prevent characterization tests from mutating editor state
dsarno 6278420
docs: Mark characterization test validation complete
dsarno 3481bb1
fix: Mark StopLocalHttpServer test as Explicit - kills MCP connection
dsarno a4f2df4
docs: Update progress with complete characterization test validation
dsarno ac531f6
test: Add 29 Windows/UI domain characterization tests
dsarno ae443c8
docs: Update progress with Windows characterization tests complete
dsarno 4f386e6
test: Add 53 Models domain characterization tests
dsarno 2895789
docs: Update progress with Models tests complete and bug documentation
dsarno e90453f
feat: Add pagination and filtering to tests resource
dsarno 2fe83b0
fix: Simplify tests resource to work with fastmcp URI constraints
dsarno 0282d23
docs: Complete pre-refactor utility audit
dsarno b80488c
refactor: QW-1 Delete dead code
dsarno 497d48b
docs: Update progress with QW-1 complete
dsarno 2e6d6d4
refactor: QW-2 Create JSON parser utility
dsarno 9aaa7cd
docs: Update progress with QW-2 complete
dsarno 514f420
refactor: QW-3 Patch in AssetPathUtility for path normalization
dsarno 0718f3c
docs: Update progress with QW-3 complete
dsarno 2b9e7c4
refactor: QW-4 Create search method constants for CLI commands
dsarno 56415fe
docs: Update REFACTOR_PLAN with QW-4 completion status
dsarno 88dfd2a
refactor: QW-5 Create confirmation dialog utility for CLI commands
dsarno 8d6234d
docs: Add QW-5 completion and comprehensive verification summary
dsarno 86d3718
docs: Add URI to all 21 MCP resource descriptions for better discoverβ¦
dsarno cedea74
refactor: P1-1 Create ToolParams validation wrapper
dsarno 225a604
refactor: P1-1 Apply ToolParams to ManageScript and ReadConsole
dsarno 8cf2596
fix: Resolve compilation errors in ToolParams implementation
dsarno e4972dd
test: Update ManageEditor null params test to reflect P1-1 fix
dsarno de82134
docs: Add P1-1.5 Python MCP Parameter Aliasing plan
dsarno 5b25758
fix: Address PR #642 CodeRabbit review feedback
dsarno ed63d95
fix: Address remaining PR #642 CodeRabbit feedback
dsarno ea38301
fix: Address final PR #642 CodeRabbit feedback for tests
dsarno bc0c254
docs: Integrate CodeRabbit feedback into P1-1.5 plan
dsarno d8a7fe2
feat: P1-1.5 Add parameter normalization middleware for camelCase supβ¦
dsarno 08cfa9e
refactor: P1-1.5 Use Pydantic AliasChoices instead of middleware
dsarno 016cabe
docs: Update P1-1.5 status - pattern established, expansion bookmarked
dsarno 8c8c043
refactor: P1-6 Consolidate duplicate test fixtures
dsarno 17c7448
docs: Update progress - P1-6 done, P1-2 and P2-3 skipped
dsarno 631cd25
refactor: P2-1 Add handle_unity_errors decorator for CLI commands
dsarno cb4fb7a
docs: Update progress - P2-1 in progress
dsarno 2bbee03
refactor: P2-1 Complete - Apply handle_unity_errors decorator to all β¦
dsarno 7e3ebf8
docs: Update progress - P2-1 complete
dsarno 7a709ea
docs: Add P2-8 CLI Consistency Pass to refactor plan
dsarno 3ea1c9f
refactor: P1-3 Add nullable coercion methods and consolidate TryParseβ¦
dsarno 0ddc679
docs: Update progress - P1-3 complete
dsarno ff7762d
docs: Add P2-9 focus nudge improvements task to refactor plan
dsarno c76ffba
refactor(P2-8): Add --force flag to texture delete command
dsarno f3525c7
docs: Update P2-8 CLI Consistency Pass status
dsarno d3a8995
fix: Address CodeRabbit PR feedback
dsarno 7fa808e
test(P3-1): Add ServerManagementService characterization tests
dsarno f3c3921
refactor(P3-1): Add Server component interfaces
dsarno 9d78108
refactor(P3-1): Extract ProcessDetector from ServerManagementService
dsarno a8cb6f9
refactor(P3-1): Extract PidFileManager from ServerManagementService
dsarno 1908846
refactor(P3-1): Extract ProcessTerminator from ServerManagementService
dsarno dac96e8
refactor(P3-1): Extract ServerCommandBuilder from ServerManagementSerβ¦
dsarno 9989467
refactor(P3-1): Extract TerminalLauncher from ServerManagementService
dsarno 530a6b0
refactor(P3-1): Complete ServerManagementService decomposition
dsarno 7782077
fix(critical): Prevent ProcessTerminator from killing all processes
dsarno e03a404
fix(tests): Correct characterization tests to document actual behavior
dsarno 6ec08c1
refactor(P1-5): Add EditorConfigurationCache to eliminate scattered Eβ¦
dsarno 3bba507
docs: Mark P1-5 Configuration Cache as complete
dsarno f07fd91
docs: Fix misleading parameter documentation in tests.py resources
dsarno 0fbe228
docs: Update REFACTOR_PROGRESS.md with P3-1 and P1-5 completions
dsarno f372019
docs: Update P2-6 plan with detailed VFX split + utility consolidation
dsarno bcbd786
refactor(P2-6): Consolidate string case utilities
dsarno eae5812
refactor(P2-6): Extract VFX Graph code from ManageVFX
dsarno f6ab62c
docs: Update REFACTOR_PROGRESS.md with P2-6 completion
dsarno 16ca408
fix(P1-5): Add cache refresh when toggling HTTP/STDIO transport
dsarno aaf08ca
refactor(P2-9): Improve focus nudge timing for better test reliability
dsarno e90ac6b
docs: Mark refactor plan complete - all items evaluated
dsarno 089b57d
fix: Add conftest.py to fix Python path for pytest
dsarno f610484
test: Enable domain reload resilience tests
dsarno 2589e71
refactor(P2-9): Implement exponential backoff for focus nudges
dsarno 6d36473
fix(P2-9): Wait for window switch and use exponential focus duration
dsarno 7d3bf0e
feat(P2-9): Add PID-based focus nudging for multi-instance support
dsarno 225ef5b
fix(P2-9): Use bundle ID activation to fully wake Unity on macOS
dsarno 161920f
fix(tests): Fix asyncio event loop issues in transport tests
dsarno 9114435
fix(tests): Fix telemetry mock patches to use correct import location
dsarno b63af09
fix(tests): Add telemetry singleton cleanup to prevent Python crashes
dsarno 7c57b07
fix(tests): Reorder test collection to run characterization tests befβ¦
dsarno c8c3cf4
docs: Clean up refactor artifacts and rewrite developer guide
dsarno f961d81
fix: Remove absolute path from docstring example
dsarno 0939008
fix: Remove orphaned .meta files for deleted markdown docs
dsarno 8a6c8ba
fix: Gate MCP startup logs behind debug mode toggle
dsarno 798ead6
fix: Use relative path for MCP package in test project manifest
dsarno f57c6d8
fix: Remove personal Claude settings and gitignore it
dsarno 84e4b3a
fix: Remove orphaned test README files referencing deleted docs
dsarno 128d7b5
fix: Remove test artifact Materials and Prefabs
dsarno 78d2bdc
fix: Remove test artifacts (QW3 scene, screenshots, textures, models β¦
dsarno 605e38a
fix: Remove file with corrupted filename
dsarno 14718d8
docs: Remove redundant OVERVIEW.md (covered by CLAUDE.md)
dsarno b6d5705
fix: Address CodeRabbit review feedback
dsarno 2d5f605
fix(coderabbit): Address critical CodeRabbit feedback issues
dsarno d67fdf9
fix(coderabbit): Address linting and validation feedback
dsarno 6d15fee
debug: Use McpLog.Warn for guaranteed LineRenderer debug visibility
dsarno 4e2274c
cleanup: Remove debug logging from LineWrite (tool verified working)
dsarno 4c30f1c
fix(coderabbit): Safe float parsing and unused import cleanup
dsarno 4bb752f
fix: OpenCode configurator preserves existing config
dsarno 845a8f6
security: Fix AppleScript injection vulnerability in focus_nudge.py
dsarno 2da4c7c
fix: Fix middleware job state cleanup and improve test error handling
dsarno adeca0e
fix: Mark slow process inspection tests as [Explicit]
dsarno 210ca08
fix: Only increment consecutive nudges counter after focus attempt
dsarno f4325f0
fix: Address remaining CodeRabbit feedback
dsarno 77f9bfa
fix: Final CodeRabbit feedback - VFX and telemetry hardening
dsarno 4a2cd9a
cleanup: Remove orphaned .meta files for deleted empty folders
dsarno 91d8175
feat: Add dict/hex format support for vectors and colors
dsarno 3b0b14f
Fix VFX graph asset handling and harden CI GO merge
dsarno 904f370
Fix VFX graph asset handling and harden CI GO merge
dsarno c33a761
Merge remote changes
dsarno 8e9c123
Deduplicate VFX template listing
dsarno dbfdb17
Avoid duplicate GO fragment merges
dsarno 3ed6e2e
Harden test job handling and tool validation
dsarno 57ad589
Relax VFX version checks and harden VFX tools
dsarno File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| # Unity-MCP Repository Overview | ||
|
|
||
| **Purpose**: Dual-language MCP (Model Context Protocol) bridge enabling AI assistants (Claude, Cursor, etc.) to control Unity Editor automation. | ||
|
|
||
| **Lead Maintainer**: David Sarno | ||
|
|
||
| **Languages**: Python (Server), C# (Unity Editor Client), Shell (Build scripts) | ||
|
|
||
| --- | ||
|
|
||
| ## Directory Structure | ||
|
|
||
| ``` | ||
| unity-mcp/ | ||
| βββ MCPForUnity/ # Unity Editor Package (C#, 163 files) | ||
| β βββ Editor/ # Editor tools and UI | ||
| β βββ Runtime/ # Runtime components (minimal) | ||
| β βββ UnityMcpServer~/ # Embedded server (submodule) | ||
| βββ Server/ # Python MCP Server (94 files) | ||
| β βββ src/ # Main source code | ||
| β βββ tests/ # Test suite | ||
| βββ tools/ # Utility/build scripts (7 files) | ||
| βββ TestProjects/ # Sample Unity projects | ||
| βββ UnityMcpBridge/ # Legacy bridge code | ||
| βββ CustomTools/ # Custom tool templates | ||
| βββ docs/ # Documentation | ||
| βββ manifest.json # MCP manifest (v0.3) | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### High-Level Flow | ||
| ``` | ||
| Client (AI/IDE) | ||
| β (MCP Protocol) | ||
| MCP Server (Python, HTTP/Stdio, Starlette) | ||
| β (HTTP Bridge) | ||
| Unity Editor (C# Plugin) | ||
| β (Editor API) | ||
| Editor State & Assets | ||
| ``` | ||
|
|
||
| ### Layers | ||
|
|
||
| | Layer | Location | Purpose | | ||
| |-------|----------|---------| | ||
| | **MCP Transport** | `Server/src/transport/` | Protocol handling, instance routing, plugin discovery | | ||
| | **CLI Commands** | `Server/src/cli/commands/` | 20 domain-specific command modules | | ||
| | **Services** | `Server/src/services/` | Custom tools, resource handlers | | ||
| | **Core Infrastructure** | `Server/src/core/` | Logging, telemetry, configuration | | ||
| | **Models** | `Server/src/models/` | Request/response structures | | ||
| | **Unity Bridge** | `MCPForUnity/Editor/Services/` | Bridge control, state management | | ||
| | **Editor Tools** | `MCPForUnity/Editor/Tools/` | 42 C# implementations (mirrored from CLI) | | ||
| | **Helpers** | `MCPForUnity/Editor/Helpers/` | 27 utility modules | | ||
| | **UI/Configuration** | `MCPForUnity/Editor/Windows/`, `Clients/` | Editor windows and MCP config | | ||
| | **Build/Release** | `tools/` | Version mgmt, stress testing, releases | | ||
|
|
||
| --- | ||
|
|
||
| ## Major Components (10 Domains) | ||
|
|
||
| ### 1. **Transport & Communication** | ||
| - **Files**: `Server/src/transport/` (7 files) | ||
| - **Purpose**: MCP protocol handling, Unity instance routing, plugin discovery | ||
| - **Key Files**: `unity_instance_middleware.py`, `plugin_hub.py`, `http_server.py` | ||
|
|
||
| ### 2. **CLI Commands (Server-Side Tools)** | ||
| - **Files**: `Server/src/cli/commands/` (20 domain modules) | ||
| - **Purpose**: Tool implementations for asset, animation, audio, components, scenes, etc. | ||
| - **Pattern**: Each domain gets a dedicated module (e.g., `commands/prefabs.py`, `commands/materials.py`) | ||
|
|
||
| ### 3. **Editor Tools Implementation** | ||
| - **Files**: `MCPForUnity/Editor/Tools/` (42 files) | ||
| - **Purpose**: C# mirror implementations of Python CLI commands | ||
| - **Pattern**: Tools are symmetrical across Python/C# (same domain, same functionality) | ||
|
|
||
| ### 4. **Unity Editor Integration** | ||
| - **Files**: `MCPForUnity/Editor/Services/` (34 files) | ||
| - **Purpose**: Bridge control, state caching, communication with server | ||
| - **Key Services**: Bridge control, config management, health monitoring | ||
|
|
||
| ### 5. **Client Configuration & Setup** | ||
| - **Files**: `MCPForUnity/Editor/Clients/` (18 files), `Models/` (6 files) | ||
| - **Purpose**: MCP client configurators, registry, initial setup | ||
| - **Key Files**: `McpClientConfigurator.cs`, `McpClient.cs`, `McpConfig.cs` | ||
|
|
||
| ### 6. **Core Infrastructure** | ||
| - **Files**: `Server/src/core/` (5 files) | ||
| - **Purpose**: Telemetry, logging, configuration management (cross-cutting concerns) | ||
|
|
||
| ### 7. **Helper Utilities** | ||
| - **Files**: `MCPForUnity/Editor/Helpers/` (27 files) + `Server/src/utils/` (3 files) | ||
| - **Purpose**: Asset path helpers, component operations, configuration builders | ||
| - **Pattern**: Reusable functions supporting main tool implementations | ||
|
|
||
| ### 8. **UI & Windows** | ||
| - **Files**: `MCPForUnity/Editor/Windows/` (8 files) | ||
| - **Purpose**: Editor windows, preferences, UI components | ||
| - **Pattern**: Each window typically handles one configuration or setup area | ||
|
|
||
| ### 9. **Models & Data Structures** | ||
| - **Files**: `Server/src/models/` (3 files) + `MCPForUnity/Editor/Models/` (6 files) | ||
| - **Purpose**: Request/response structures, configuration schemas | ||
| - **Pattern**: Shared data definitions across Python/C# | ||
|
|
||
| ### 10. **Build, Release & Testing** | ||
| - **Files**: `tools/` (7 files), `Server/tests/`, `MCPForUnity/Editor/Tests/` | ||
| - **Purpose**: Version management, stress testing, asset store packaging, test suites | ||
|
|
||
| --- | ||
|
|
||
| ## Key Patterns & Principles | ||
|
|
||
| 1. **Domain-Driven Symmetry**: Each domain (Prefabs, Materials, Scripts, etc.) exists in both Python (CLI) and C# (Editor Tools) | ||
| 2. **Multi-Instance Support**: Can target multiple Unity Editor instances simultaneously | ||
| 3. **Extensible Plugin System**: Custom tools can be registered without modifying core | ||
| 4. **Bidirectional Communication**: Server polls/controls Editor; Editor can push state updates | ||
| 5. **Cross-Cutting Concerns**: Logging, telemetry, configuration centralized in core infrastructure | ||
|
|
||
| --- | ||
|
|
||
| ## Total Stats | ||
|
|
||
| | Metric | Count | | ||
| |--------|-------| | ||
| | Python Source Files | 94 | | ||
| | C# Source Files | 163 | | ||
| | CLI Command Domains | 20 | | ||
| | Editor Tool Modules | 42 | | ||
| | Service Modules (Unity) | 34 | | ||
| | Helper Modules | 30+ | | ||
| | MCP Tools Exposed | 27 | | ||
| | Entry Points | 3 (main.py, Editor Windows, Editor Menu) | | ||
|
|
||
| --- | ||
|
|
||
| ## Common Improvement Areas (Known/Suspected) | ||
|
|
||
| - Repeated patterns across domain-specific tool implementations | ||
| - Possible dead code or legacy CLI commands | ||
| - Over-engineering in helper utilities | ||
| - Bloated configuration or setup workflows | ||
| - Asymmetries between Python and C# implementations | ||
|
|
||
| --- | ||
|
|
||
| ## For Future Cleanup Passes | ||
|
|
||
| Use the 10 domains listed above as your primary analysis units. Each domain is self-contained enough for parallel review and refactoring recommendations. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "mcpServers": { | ||
| "unity-mcp": { | ||
| "type": "stdio", | ||
| "command": "uv", | ||
| "args": [ | ||
| "run", | ||
| "--directory", | ||
| "/Users/davidsarno/unity-mcp-refactor/Server", | ||
| "src/main.py", | ||
| "--transport", | ||
| "stdio" | ||
| ] | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ | |
| "MultiEdit(reports/**)" | ||
| ], | ||
| "deny": [ | ||
| "Bash", | ||
| "WebFetch", | ||
| "WebSearch", | ||
| "Task", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| { | ||
| "permissions": { | ||
| "allow": [ | ||
| "Bash", | ||
| "mcp__unity-mcp__get_test_job", | ||
| "mcp__UnityMCP__get_test_job", | ||
| "mcp__UnityMCP__run_tests", | ||
| "mcp__UnityMCP__read_console" | ||
| ], | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| "ask": [ | ||
| "Bash(git push*)" | ||
| ] | ||
| }, | ||
| "enabledPlugins": { | ||
| "frontend-design@claude-plugins-official": true | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.