Improve port probing and script edit tooling #19
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR enhances the Unity MCP Bridge with important improvements to port discovery probing and script editing functionality. The changes focus on robustness, security, and cross-platform compatibility across two key files.
In the port discovery module (port_discovery.py), the PR refactors the _read_exact helper function from being redefined inside _try_probe_unity_mcp on every call to a reusable module-level function. This eliminates redundancy and improves performance. The PR also introduces security hardening by adding frame size validation with a 1MB cap to prevent potential DoS attacks from malicious servers sending unreasonably large frame sizes. The non-framed protocol path is improved with bounded reading that accumulates chunks up to 1024 bytes while actively searching for the expected pong response.
In the script management module (ManageScript.cs), the PR adds cross-platform symlink detection using .NET 6.0+'s LinkTarget property to prevent potential security vulnerabilities from malicious symlinked scripts. The using statement regex is enhanced to support modern C# features like 'global using' and 'using static' declarations. Method detection heuristics are improved with a fallback to FindEnclosingClassSpan when TryComputeClassSpan fails, making structured editing more robust for complex class hierarchies. The PR also adds proper support for Windows-style CRLF line endings in text parsing, addressing cross-platform compatibility issues between Windows and Unix-based systems.
These changes integrate well with the existing Unity MCP Bridge architecture by strengthening the communication layer's security and reliability while maintaining backward compatibility. The improvements address real-world scenarios that development teams encounter when working across different platforms and with modern C# language features.
Important Files Changed
Files Modified
| Filename | Score | Overview |
|---|---|---|
| UnityMcpBridge/UnityMcpServer~/src/port_discovery.py | 4/5 | Refactored _read_exact to module level and added security guards for frame size validation |
| UnityMcpBridge/Editor/Tools/ManageScript.cs | 4/5 | Enhanced C# parsing with symlink detection, CRLF support, and improved method detection heuristics |
Confidence score: 4/5
- This PR is safe to merge with minimal risk of breaking existing functionality
- Score reflects well-structured security improvements and cross-platform compatibility fixes with clear defensive programming practices
- Pay close attention to the symlink detection logic in ManageScript.cs for potential .NET version compatibility issues
Sequence Diagram
sequenceDiagram
participant User
participant MCP_Client as MCP Client
participant Unity_Bridge as Unity MCP Bridge
participant ManageScript as ManageScript Tool
participant FileSystem as File System
participant Port_Discovery as Port Discovery
participant Unity_Server as Unity MCP Server
Note over User, Unity_Server: Script Management Flow
User->>MCP_Client: Request script operation (create/update/edit)
MCP_Client->>Port_Discovery: Discover Unity port
Port_Discovery->>FileSystem: Scan registry files (unity-mcp-port-*.json)
FileSystem-->>Port_Discovery: Return candidate files
Port_Discovery->>Unity_Server: Probe ports with ping/pong
Unity_Server-->>Port_Discovery: Return pong response
Port_Discovery-->>MCP_Client: Return active port
MCP_Client->>Unity_Bridge: Send script command via discovered port
Unity_Bridge->>ManageScript: Route to HandleCommand()
alt Script Creation/Update
ManageScript->>ManageScript: Validate script name and path
ManageScript->>ManageScript: Resolve path under Assets/
ManageScript->>ManageScript: Check for symlinks (security)
ManageScript->>FileSystem: Check if script exists
FileSystem-->>ManageScript: Return file status
alt Base64 encoded content
ManageScript->>ManageScript: Decode base64 contents
end
ManageScript->>ManageScript: Validate syntax (Roslyn if available)
alt Validation fails
ManageScript-->>Unity_Bridge: Return validation errors
Unity_Bridge-->>MCP_Client: Return error response
MCP_Client-->>User: Show validation errors
else Validation passes
ManageScript->>FileSystem: Write script atomically (.tmp then move)
FileSystem-->>ManageScript: Confirm write
ManageScript->>ManageScript: Schedule asset refresh
ManageScript-->>Unity_Bridge: Return success
Unity_Bridge-->>MCP_Client: Return success response
MCP_Client-->>User: Confirm script created/updated
end
else Text Edits (apply_text_edits)
ManageScript->>FileSystem: Read current script
FileSystem-->>ManageScript: Return script contents
ManageScript->>ManageScript: Compute SHA256 of current content
ManageScript->>ManageScript: Validate precondition SHA256
alt SHA256 mismatch
ManageScript-->>Unity_Bridge: Return "stale_file" error
Unity_Bridge-->>MCP_Client: Return error with current SHA
MCP_Client-->>User: Request fresh content
else SHA256 matches
ManageScript->>ManageScript: Apply edits (line/col to index)
ManageScript->>ManageScript: Check balanced delimiters
ManageScript->>ManageScript: Validate result syntax
ManageScript->>FileSystem: Write updated script atomically
FileSystem-->>ManageScript: Confirm write
ManageScript-->>Unity_Bridge: Return success with new SHA256
Unity_Bridge-->>MCP_Client: Return success response
MCP_Client-->>User: Confirm edits applied
end
else Structured Edits (edit)
ManageScript->>FileSystem: Read script content
FileSystem-->>ManageScript: Return content
ManageScript->>ManageScript: Parse class/method spans
ManageScript->>ManageScript: Apply replace/delete/insert operations
ManageScript->>ManageScript: Validate final result
ManageScript->>FileSystem: Write modified script
FileSystem-->>ManageScript: Confirm write
ManageScript-->>Unity_Bridge: Return success
Unity_Bridge-->>MCP_Client: Return response
MCP_Client-->>User: Show edit results
end
Note over ManageScript, Unity_Server: Asset Refresh (Async)
ManageScript->>Unity_Bridge: Schedule debounced refresh
Unity_Bridge->>Unity_Bridge: Trigger asset database import
Unity_Bridge->>Unity_Bridge: Request script compilation
2 files reviewed, 1 comment
| if (!TryComputeClassSpan(original, name, null, out var clsStart, out var clsLen, out _)) | ||
| { | ||
| FindEnclosingClassSpan(original, sp.start, out clsStart, out clsLen); |
There was a problem hiding this comment.
logic: The fallback logic here changes behavior - if TryComputeClassSpan fails, it now tries FindEnclosingClassSpan instead of failing immediately. This could mask legitimate errors where the specified class doesn't exist.
1 participant
Summary
_read_exactand add frame size guards in port discovery probesManageScriptheader scanning and method replace heuristics\rnewlines and detect symlinks cross-platformTesting
pytesthttps://chatgpt.com/codex/tasks/task_e_68a2a98f2a8c832780677072a8c0d8d5