Bug fix and batch customization#727
Merged
Scriptwonder merged 3 commits intoCoplayDev:betafrom Feb 12, 2026
Merged
Conversation
1. Fix the bug where for certain MacOS user (like me), the GitURLoverride is set to the unity-mcp folder rather than the unity-mcp/Server 2. Customize batch for it to take 0-100 requests. Tested on MacOS.
Contributor
Reviewer's GuideMakes batch_execute’s max-commands limit configurable (up to 100) and exposes it to both Unity and server-side logic, adds extensive UI creation batch templates to the skill docs, and fixes GitUrlOverride so local unity-mcp paths are auto-corrected to the Server subdirectory when needed. Sequence diagram for configurable batch_execute limit between Unity and serversequenceDiagram
actor LlmUser
participant ClaudeSkill
participant ServerBatchExecute as Server_batch_execute_py
participant ServerEditorState as Server_editor_state_service
participant UnityEditor
participant EditorStateCache
participant BatchExecuteCs as BatchExecute_CSharp
participant EditorPrefs
LlmUser->>ClaudeSkill: Call batch_execute with commands
ClaudeSkill->>ServerBatchExecute: batch_execute(ctx, commands, fail_fast)
ServerBatchExecute->>ServerBatchExecute: Validate commands list not empty
ServerBatchExecute->>ServerEditorState: _get_max_commands_from_editor_state(ctx)
ServerEditorState->>UnityEditor: HTTP request for editor state
UnityEditor->>EditorStateCache: BuildSnapshot(reason)
EditorStateCache->>BatchExecuteCs: GetMaxCommandsPerBatch()
BatchExecuteCs->>EditorPrefs: GetInt(BatchExecuteMaxCommands, DefaultMaxCommandsPerBatch)
EditorPrefs-->>BatchExecuteCs: configured_or_default
BatchExecuteCs-->>EditorStateCache: clamped_max_commands
EditorStateCache-->>UnityEditor: EditorStateSnapshot with Settings.BatchExecuteMaxCommands
UnityEditor-->>ServerEditorState: Serialized editor state response
ServerEditorState-->>ServerBatchExecute: EditorStateData with settings.batch_execute_max_commands
ServerBatchExecute->>ServerBatchExecute: Cache max_commands
ServerBatchExecute->>ServerBatchExecute: Compare len(commands) to max_commands
alt Too_many_commands
ServerBatchExecute-->>ClaudeSkill: Error value: exceeds max_commands
ClaudeSkill-->>LlmUser: Report batch_execute limit error
else Within_limit
ServerBatchExecute->>ServerBatchExecute: Normalize commands
ServerBatchExecute->>ServerBatchExecute: Execute commands via Unity transport
ServerBatchExecute-->>ClaudeSkill: Aggregated tool results
ClaudeSkill-->>LlmUser: Return batch results
end
Updated class diagram for batch_execute configuration and editor state modelsclassDiagram
class EditorPrefKeys {
<<static>>
+string GitUrlOverride
+string BatchExecuteMaxCommands
}
class BatchExecute_CSharp {
<<static>>
+int DefaultMaxCommandsPerBatch
+int AbsoluteMaxCommandsPerBatch
+int GetMaxCommandsPerBatch()
+Task~object~ HandleCommand(JObject params)
}
class EditorStateCache {
<<static>>
+JObject BuildSnapshot(string reason)
}
class EditorStateSnapshot {
+EditorStateTransport Transport
+EditorStateSettings Settings
}
class EditorStateSettings {
+int BatchExecuteMaxCommands
}
class EditorStateTransport {
+bool? UnityBridgeConnected
+long? LastMessageUnixMs
}
class EditorStateSettingsPy {
+int~nullable~ batch_execute_max_commands
}
class EditorStateDataPy {
+EditorStateSettingsPy~nullable~ settings
+EditorStateTransportPy~nullable~ transport
}
class EditorStateTransportPy {
+bool~nullable~ unity_bridge_connected
+int~nullable~ last_message_unix_ms
}
class BatchExecute_Py {
+int DEFAULT_MAX_COMMANDS_PER_BATCH
+int ABSOLUTE_MAX_COMMANDS_PER_BATCH
+int~nullable~ _cached_max_commands
+async int _get_max_commands_from_editor_state(Context ctx)
+void invalidate_cached_max_commands()
+async Any batch_execute(Context ctx, list commands, bool fail_fast)
}
EditorStateCache --> EditorStateSnapshot
EditorStateSnapshot --> EditorStateTransport
EditorStateSnapshot --> EditorStateSettings
EditorStateDataPy --> EditorStateSettingsPy
EditorStateDataPy --> EditorStateTransportPy
BatchExecute_CSharp ..> EditorPrefKeys : uses
EditorStateCache ..> BatchExecute_CSharp : calls GetMaxCommandsPerBatch
BatchExecute_Py ..> EditorStateDataPy : reads settings.batch_execute_max_commands
BatchExecute_Py ..> BatchExecute_CSharp : shares limits via editor state
EditorPrefKeys <.. McpToolsSection : BatchExecuteMaxCommands key used
class McpToolsSection {
+VisualElement CreateToolRow(ToolMetadata tool)
-VisualElement CreateBatchExecuteSettings()
-bool IsBatchExecuteTool(ToolMetadata tool)
}
Flow diagram for auto-correcting local MCP server path overridesflowchart TD
Start[[Start]] --> CheckEmpty{Path empty?}
CheckEmpty -->|Yes| ReturnOriginal1[Return original path]
CheckEmpty -->|No| CheckRemote{Starts with http, https, git+, ssh?}
CheckRemote -->|Yes| ReturnOriginal2[Return original path]
CheckRemote -->|No| StripFilePrefix{Starts with file://?}
StripFilePrefix -->|Yes| SetPrefix[Store prefix and strip from path]
StripFilePrefix -->|No| NoPrefix[Use path as checkPath; prefix empty]
SetPrefix --> CheckPyproject
NoPrefix --> CheckPyproject
CheckPyproject{pyproject.toml exists in checkPath?} -->|Yes| ReturnOriginal3[Return original path]
CheckPyproject -->|No| CheckServerSubdir
CheckServerSubdir{pyproject.toml exists in checkPath/Server?} -->|Yes| BuildCorrected[Return prefix + checkPath/Server]
CheckServerSubdir -->|No| ReturnOriginal4[Return original path]
ReturnOriginal1 --> End[[End]]
ReturnOriginal2 --> End
ReturnOriginal3 --> End
ReturnOriginal4 --> End
BuildCorrected --> End
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR makes the batch_execute per-batch command limit configurable from the Unity editor (default 25, hard cap 100). Changes touch docs, editor prefs/UI, editor state snapshots, local server path resolution, and server-side batch_execute validation and caching. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Unity Editor (UI/Prefs)
participant StateSvc as EditorStateSnapshot
participant Server as MCP Server (batch_execute)
participant Skill as unity-mcp-skill / Client
Skill->>Server: POST batch_execute(commands[])
Server->>StateSvc: Read latest editor state (settings)
StateSvc-->>Server: settings.batch_execute_max_commands (or null)
Server->>Server: determine max_commands (cached or default, clamp to 100)
Server->>Server: validate len(commands) <= max_commands
alt valid
Server-->>Skill: execute commands (non-transactional)
else too many
Server-->>Skill: error - exceed configured max
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The local server path resolution logic is duplicated between AssetPathUtility.ResolveLocalServerPath and McpAdvancedSection.ResolveServerPath; consider centralizing this into a single shared helper to avoid future divergence in behavior.
- ResolveServerPath hardcodes the "file://" prefix while ResolveLocalServerPath preserves the original casing/prefix substring; aligning these behaviors would reduce subtle inconsistencies when users enter differently cased file URLs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The local server path resolution logic is duplicated between AssetPathUtility.ResolveLocalServerPath and McpAdvancedSection.ResolveServerPath; consider centralizing this into a single shared helper to avoid future divergence in behavior.
- ResolveServerPath hardcodes the "file://" prefix while ResolveLocalServerPath preserves the original casing/prefix substring; aligning these behaviors would reduce subtle inconsistencies when users enter differently cased file URLs.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Helpers/AssetPathUtility.cs:256-257` </location>
<code_context>
+ return path;
+ }
+
+ // If it looks like a PyPI package reference (no path separators), skip
+ if (!path.Contains('/') && !path.Contains('\\') && !path.StartsWith("file:", StringComparison.OrdinalIgnoreCase))
+ {
+ return path;
</code_context>
<issue_to_address>
**suggestion:** Relative local paths without separators are treated as PyPI references and skipped.
This logic treats any string without `/`, `\`, or `file:` as a PyPI reference, so relative directories like `Server` or `my_package` won’t be auto-corrected even if they contain a valid `pyproject.toml`. If users are likely to enter such relative paths, consider tightening the heuristic (e.g. require a more PyPI-specific pattern or explicitly distinguish absolute vs relative paths) so these local directories can still be handled.
</issue_to_address>
### Comment 2
<location> `MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs:353-362` </location>
<code_context>
+ private static string ResolveServerPath(string path)
</code_context>
<issue_to_address>
**suggestion:** Path resolution logic is duplicated between ResolveServerPath and ResolveLocalServerPath and may drift over time.
These two methods share most of the same concerns (local vs URL detection, `file://` handling, `Server` subdirectory probing) but differ in details like the PyPI-style heuristic and prefix preservation. Consider extracting a shared helper for the common resolution logic that both methods delegate to, so any future changes are made in one place and behavior stays consistent across the UI, editor prefs, and `GetMcpServerPackageSource`.
Suggested implementation:
```csharp
/// <summary>
/// Shared normalization logic for resolving server paths (both local and remote).
/// Handles scheme detection, `file://` URIs, and normalization of local filesystem
/// paths before any call-site specific heuristics are applied.
/// </summary>
/// <param name="path">Raw user-provided path or URL.</param>
/// <param name="normalizedPath">
/// Normalized local filesystem path (if applicable). When this returns false,
/// this will be null and the original <paramref name="path" /> should be used.
/// </param>
/// <returns>
/// True when <paramref name="path" /> represents a local filesystem location that
/// should be further resolved (e.g. probing for a "Server" subdirectory); false
/// when the caller should treat the original string as-is (URLs, PyPI-style refs, etc.).
/// </returns>
private static bool TryNormalizeLocalServerPath(string path, out string normalizedPath)
{
normalizedPath = null;
if (string.IsNullOrEmpty(path))
return false;
// Non-local sources (git URLs, PyPI refs, etc.) are returned as-is at call sites.
if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase))
{
return false;
}
// Support `file://` style URIs, which can come from Unity file pickers on some platforms.
if (path.StartsWith("file://", StringComparison.OrdinalIgnoreCase))
{
if (Uri.TryCreate(path, UriKind.Absolute, out var uri) && uri.IsFile)
normalizedPath = uri.LocalPath;
else
normalizedPath = path;
return true;
}
// For plain paths, just normalize separators; call sites can perform additional
// probing (e.g. for "Server" subdirectories or pyproject.toml detection).
normalizedPath = path.Replace('\\', Path.DirectorySeparatorChar)
.Replace('/', Path.DirectorySeparatorChar);
return true;
}
/// <summary>
/// Validates and auto-corrects a local server path to ensure it points to the directory
/// containing pyproject.toml (the Python package root). If the user selects a parent
/// directory (e.g. the repo root), this checks for a "Server" subdirectory with
/// pyproject.toml and returns that instead.
/// </summary>
private static string ResolveServerPath(string path)
```
To complete the refactor and fully address the duplication / drift risk, you should:
1. Update `ResolveServerPath` to:
- Call `TryNormalizeLocalServerPath(path, out var normalizedPath)`.
- If it returns false, immediately return the original `path` (covers URLs, PyPI-style refs, etc.).
- Use `normalizedPath` for all subsequent filesystem probing logic (including the `"Server"` subdirectory and `pyproject.toml` detection) instead of re-implementing scheme checks.
2. Update `ResolveLocalServerPath` in the same file to:
- Call `TryNormalizeLocalServerPath` up-front and branch the same way.
- Remove any duplicated scheme / `file://` / separator normalization logic that is now handled centrally.
3. If there are other call sites that need consistent behavior (e.g. `GetMcpServerPackageSource` or editor prefs code that performs similar path handling), consider routing them through either `TryNormalizeLocalServerPath` directly or one of the two higher-level resolvers, rather than re-implementing normalization in-line.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+256
to
+257
| // If it looks like a PyPI package reference (no path separators), skip | ||
| if (!path.Contains('/') && !path.Contains('\\') && !path.StartsWith("file:", StringComparison.OrdinalIgnoreCase)) |
Contributor
There was a problem hiding this comment.
suggestion: Relative local paths without separators are treated as PyPI references and skipped.
This logic treats any string without /, \, or file: as a PyPI reference, so relative directories like Server or my_package won’t be auto-corrected even if they contain a valid pyproject.toml. If users are likely to enter such relative paths, consider tightening the heuristic (e.g. require a more PyPI-specific pattern or explicitly distinguish absolute vs relative paths) so these local directories can still be handled.
Comment on lines
+353
to
+362
| private static string ResolveServerPath(string path) | ||
| { | ||
| if (string.IsNullOrEmpty(path)) | ||
| return path; | ||
|
|
||
| // If path is not a local filesystem path, return as-is (git URLs, PyPI refs, etc.) | ||
| if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase)) |
Contributor
There was a problem hiding this comment.
suggestion: Path resolution logic is duplicated between ResolveServerPath and ResolveLocalServerPath and may drift over time.
These two methods share most of the same concerns (local vs URL detection, file:// handling, Server subdirectory probing) but differ in details like the PyPI-style heuristic and prefix preservation. Consider extracting a shared helper for the common resolution logic that both methods delegate to, so any future changes are made in one place and behavior stays consistent across the UI, editor prefs, and GetMcpServerPackageSource.
Suggested implementation:
/// <summary>
/// Shared normalization logic for resolving server paths (both local and remote).
/// Handles scheme detection, `file://` URIs, and normalization of local filesystem
/// paths before any call-site specific heuristics are applied.
/// </summary>
/// <param name="path">Raw user-provided path or URL.</param>
/// <param name="normalizedPath">
/// Normalized local filesystem path (if applicable). When this returns false,
/// this will be null and the original <paramref name="path" /> should be used.
/// </param>
/// <returns>
/// True when <paramref name="path" /> represents a local filesystem location that
/// should be further resolved (e.g. probing for a "Server" subdirectory); false
/// when the caller should treat the original string as-is (URLs, PyPI-style refs, etc.).
/// </returns>
private static bool TryNormalizeLocalServerPath(string path, out string normalizedPath)
{
normalizedPath = null;
if (string.IsNullOrEmpty(path))
return false;
// Non-local sources (git URLs, PyPI refs, etc.) are returned as-is at call sites.
if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) ||
path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase))
{
return false;
}
// Support `file://` style URIs, which can come from Unity file pickers on some platforms.
if (path.StartsWith("file://", StringComparison.OrdinalIgnoreCase))
{
if (Uri.TryCreate(path, UriKind.Absolute, out var uri) && uri.IsFile)
normalizedPath = uri.LocalPath;
else
normalizedPath = path;
return true;
}
// For plain paths, just normalize separators; call sites can perform additional
// probing (e.g. for "Server" subdirectories or pyproject.toml detection).
normalizedPath = path.Replace('\\', Path.DirectorySeparatorChar)
.Replace('/', Path.DirectorySeparatorChar);
return true;
}
/// <summary>
/// Validates and auto-corrects a local server path to ensure it points to the directory
/// containing pyproject.toml (the Python package root). If the user selects a parent
/// directory (e.g. the repo root), this checks for a "Server" subdirectory with
/// pyproject.toml and returns that instead.
/// </summary>
private static string ResolveServerPath(string path)To complete the refactor and fully address the duplication / drift risk, you should:
- Update
ResolveServerPathto:- Call
TryNormalizeLocalServerPath(path, out var normalizedPath). - If it returns false, immediately return the original
path(covers URLs, PyPI-style refs, etc.). - Use
normalizedPathfor all subsequent filesystem probing logic (including the"Server"subdirectory andpyproject.tomldetection) instead of re-implementing scheme checks.
- Call
- Update
ResolveLocalServerPathin the same file to:- Call
TryNormalizeLocalServerPathup-front and branch the same way. - Remove any duplicated scheme /
file:/// separator normalization logic that is now handled centrally.
- Call
- If there are other call sites that need consistent behavior (e.g.
GetMcpServerPackageSourceor editor prefs code that performs similar path handling), consider routing them through eitherTryNormalizeLocalServerPathdirectly or one of the two higher-level resolvers, rather than re-implementing normalization in-line.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Server/src/services/tools/batch_execute.py`:
- Around line 23-53: The module cache _cached_max_commands is never invalidated
causing stale limits; modify _get_max_commands_from_editor_state to use a
time-based TTL: add a module-level timestamp (e.g.,
_cached_max_commands_last_updated: float | None) and a TTL constant (e.g.,
CACHE_TTL_SECONDS = 60), and when called check if _cached_max_commands is set
and (time.time() - _cached_max_commands_last_updated) < CACHE_TTL_SECONDS then
return the cached value; otherwise fetch from get_editor_state, update both
_cached_max_commands and _cached_max_commands_last_updated when the fetched
limit is valid, and fall back to DEFAULT_MAX_COMMANDS_PER_BATCH on errors; keep
the existing invalidate_cached_max_commands() but no longer required if TTL is
used.
🧹 Nitpick comments (3)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
214-221: Side-effect in a getter-like method — consider documenting or renaming.
GetMcpServerPackageSource()now mutatesEditorPrefsas a side-effect (persisting the corrected path). While this is useful for self-healing, callers may not expect a "Get" method to write state. The inline comment helps, but a brief<remarks>in the XML doc would make the contract clearer for future maintainers.MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs (1)
347-393: Duplicate logic — delegate toAssetPathUtility.ResolveLocalServerPathinstead.This method is nearly identical to
AssetPathUtility.ResolveLocalServerPath(lines 242–286) but with subtle differences: it's missing the PyPI-reference guard and hardcodes the"file://"prefix instead of preserving original casing. Maintaining two copies invites drift.♻️ Proposed refactor
- private static string ResolveServerPath(string path) - { - if (string.IsNullOrEmpty(path)) - return path; - - // If path is not a local filesystem path, return as-is (git URLs, PyPI refs, etc.) - if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) || - path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) || - path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) || - path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase)) - { - return path; - } - - // Strip file:// prefix for filesystem checks, but preserve it for the return value - string checkPath = path; - string prefix = string.Empty; - if (checkPath.StartsWith("file://", StringComparison.OrdinalIgnoreCase)) - { - prefix = "file://"; - checkPath = checkPath.Substring(7); - } - - // Already points to a directory with pyproject.toml — correct path - if (File.Exists(Path.Combine(checkPath, "pyproject.toml"))) - { - return path; - } - - // Check if "Server" subdirectory contains pyproject.toml (common repo structure) - string serverSubDir = Path.Combine(checkPath, "Server"); - if (File.Exists(Path.Combine(serverSubDir, "pyproject.toml"))) - { - string corrected = prefix + serverSubDir; - McpLog.Info($"Auto-corrected server path to 'Server' subdirectory: {corrected}"); - return corrected; - } - - // Return as-is; uvx will report the error if the path is invalid - return path; - } + private static string ResolveServerPath(string path) + { + string resolved = AssetPathUtility.ResolveLocalServerPath(path); + if (resolved != path) + { + McpLog.Info($"Auto-corrected server path to 'Server' subdirectory: {resolved}"); + } + return resolved; + }MCPForUnity/Editor/Tools/BatchExecute.cs (1)
33-51: Consider usingToolParamsfor parameter validation.The new null-check and existing parameter access (
@params["commands"],@params.Value<bool?>()) bypassToolParams, which provides consistent validation via methods likeGetInt(),RequireString(). Since this is a pre-existing pattern, converting the whole method is outside the scope of this PR, but worth noting for a follow-up.As per coding guidelines:
MCPForUnity/Editor/Tools/*.cs: UseToolParamsfor consistent parameter validation in C# Editor tools with methods likeGetInt(),RequireString().
Comment on lines
+23
to
+53
| # Module-level cache for the Unity-configured limit (populated from editor state). | ||
| _cached_max_commands: int | None = None | ||
|
|
||
|
|
||
| async def _get_max_commands_from_editor_state(ctx: Context) -> int: | ||
| """ | ||
| Attempt to read the configured batch limit from the Unity editor state. | ||
| Falls back to DEFAULT_MAX_COMMANDS_PER_BATCH if unavailable. | ||
| """ | ||
| global _cached_max_commands | ||
| if _cached_max_commands is not None: | ||
| return _cached_max_commands | ||
|
|
||
| try: | ||
| from services.resources.editor_state import get_editor_state | ||
|
|
||
| state_resp = await get_editor_state(ctx) | ||
| data = state_resp.data if hasattr(state_resp, "data") else ( | ||
| state_resp.get("data") if isinstance(state_resp, dict) else None | ||
| ) | ||
| if isinstance(data, dict): | ||
| settings = data.get("settings") | ||
| if isinstance(settings, dict): | ||
| limit = settings.get("batch_execute_max_commands") | ||
| if isinstance(limit, int) and 1 <= limit <= ABSOLUTE_MAX_COMMANDS_PER_BATCH: | ||
| _cached_max_commands = limit | ||
| return limit | ||
| except Exception as exc: | ||
| logger.debug("Could not read batch limit from editor state: %s", exc) | ||
|
|
||
| return DEFAULT_MAX_COMMANDS_PER_BATCH |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'invalidate_cached_max_commands' --type=pyRepository: CoplayDev/unity-mcp
Length of output: 154
🏁 Script executed:
# Also check for any TTL or time-based cache invalidation mechanisms
rg -n 'cache.*expire|ttl|time.*cache|datetime.*cache' -i --type=pyRepository: CoplayDev/unity-mcp
Length of output: 3797
🏁 Script executed:
# Check the full context of batch_execute.py to see if there are cache expiration patterns
fd batch_execute.py --type=f | head -1 | xargs wc -lRepository: CoplayDev/unity-mcp
Length of output: 109
🏁 Script executed:
# Read the full batch_execute.py to see all cache-related code
cat -n Server/src/services/tools/batch_execute.pyRepository: CoplayDev/unity-mcp
Length of output: 6414
🏁 Script executed:
# Search for places where invalidate_cached_max_commands might be called implicitly or where cache is reset
rg -n 'invalidate|_cached_max_commands' --type=py -B2 -A2Repository: CoplayDev/unity-mcp
Length of output: 3742
🏁 Script executed:
# Check if there's any event-based mechanism or editor state change listener that might trigger cache invalidation
rg -n 'editor.*state|on_.*change|listener|subscribe|event' --type=py | grep -i 'batch_execute\|max_commands\|cache'Repository: CoplayDev/unity-mcp
Length of output: 1101
🏁 Script executed:
# Final verification: search for any imports of invalidate_cached_max_commands
rg -n 'from.*batch_execute.*import|import.*batch_execute' --type=pyRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
# Check if there's any test coverage that exercises cache invalidation
fd '.*batch.*execute.*test' -i --type=fRepository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
# Broader search: look for any mention of batch_execute in test files
fd --type f -e py | xargs grep -l 'batch_execute' | head -20Repository: CoplayDev/unity-mcp
Length of output: 228
🏁 Script executed:
# Check if invalidate_cached_max_commands is called via string reference or dynamic import
rg -n 'batch_execute\|invalidate' --type=py Server/tests/ 2>/dev/null | head -30Repository: CoplayDev/unity-mcp
Length of output: 45
🏁 Script executed:
# Check the files that mention batch_execute to see if any call invalidate_cached_max_commands
for file in "Server/src/cli/commands/batch.py" "Server/src/services/resources/editor_state.py" "Server/src/services/resources/gameobject.py"; do
echo "=== $file ==="
grep -n 'invalidate_cached_max_commands' "$file" 2>/dev/null || echo "No matches"
doneRepository: CoplayDev/unity-mcp
Length of output: 242
🏁 Script executed:
# Check if batch.py or any other file imports or uses invalidate_cached_max_commands
grep -r 'invalidate_cached_max_commands' Server/ --include="*.py"Repository: CoplayDev/unity-mcp
Length of output: 151
Cache is never invalidated — setting changes require server restart.
_cached_max_commands is populated on the first batch_execute call and never cleared. If a user changes the max commands in the Unity MCP Tools window, the server continues using the stale cached value until restart. The invalidate_cached_max_commands() function exists but is not called anywhere in the codebase.
Consider either:
- Wiring
invalidate_cached_max_commands()to editor state change events, or - Adding a TTL so the cache expires after a reasonable interval (e.g., 30–60 seconds).
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 50-50: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In `@Server/src/services/tools/batch_execute.py` around lines 23 - 53, The module
cache _cached_max_commands is never invalidated causing stale limits; modify
_get_max_commands_from_editor_state to use a time-based TTL: add a module-level
timestamp (e.g., _cached_max_commands_last_updated: float | None) and a TTL
constant (e.g., CACHE_TTL_SECONDS = 60), and when called check if
_cached_max_commands is set and (time.time() -
_cached_max_commands_last_updated) < CACHE_TTL_SECONDS then return the cached
value; otherwise fetch from get_editor_state, update both _cached_max_commands
and _cached_max_commands_last_updated when the fetched limit is valid, and fall
back to DEFAULT_MAX_COMMANDS_PER_BATCH on errors; keep the existing
invalidate_cached_max_commands() but no longer required if TTL is used.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes local server-source override path selection to reliably target the Python package root (Server/), and makes the batch_execute command limit configurable up to a hard max of 100 (surfaced from Unity editor state and enforced on both Unity + server). It also expands the Unity-MCP skill documentation with UI creation workflows and updated batch-limit guidance.
Changes:
- Auto-correct local
GitUrlOverride/server-source paths to theServersubdirectory whenpyproject.tomlis found there. - Add a Unity Editor preference UI for
batch_executemax commands (default 25, hard max 100) and expose it viaeditor_state.settings. - Update skill docs: new “UI Creation Workflows” section and updated batch limit guidance.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs |
Auto-corrects server path input and improves browse dialog text for selecting the Server folder. |
MCPForUnity/Editor/Helpers/AssetPathUtility.cs |
Resolves and persists corrected local server package source paths (prefers Server/ when applicable). |
MCPForUnity/Editor/Constants/EditorPrefKeys.cs |
Adds an EditorPrefs key for the configurable batch limit. |
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs |
Adds UI control in the Tools window to set max commands per batch. |
MCPForUnity/Editor/Tools/BatchExecute.cs |
Enforces the configurable batch limit on the Unity side (default 25, max 100). |
MCPForUnity/Editor/Services/EditorStateCache.cs |
Publishes the configured batch limit into the editor_state snapshot under settings. |
Server/src/services/resources/editor_state.py |
Extends the editor_state schema to include settings.batch_execute_max_commands. |
Server/src/services/tools/batch_execute.py |
Reads batch limit from editor_state (with fallback/defaults) and updates validation + tool description. |
unity-mcp-skill/references/workflows.md |
Adds “UI Creation Workflows” templates and references batch limit configurability. |
unity-mcp-skill/SKILL.md |
Updates batch limit docs and adds a UI capability row linking to the UI workflows section. |
.claude/skills/unity-mcp-skill/references/workflows.md |
Mirrors the UI workflows documentation update for the Claude skill copy. |
.claude/skills/unity-mcp-skill/SKILL.md |
Mirrors the SKILL.md documentation update for the Claude skill copy. |
Comments suppressed due to low confidence (1)
MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs:343
OnBrowseGitUrlClicked()setsgitUrlOverride.value, which triggers the registered value-changed callback that already persists to EditorPrefs and invokesOnGitUrlChanged/OnHttpServerCommandUpdateRequested. The method then repeats those side effects, causing duplicate pref writes and double event invocation. UseSetValueWithoutNotify()here and keep the explicit persistence/events, or set.valueand remove the explicitEditorPrefs.SetString(...)+ event invocations.
private void OnBrowseGitUrlClicked()
{
string picked = EditorUtility.OpenFolderPanel("Select Server folder (containing pyproject.toml)", string.Empty, string.Empty);
if (!string.IsNullOrEmpty(picked))
{
picked = ResolveServerPath(picked);
gitUrlOverride.value = picked;
EditorPrefs.SetString(EditorPrefKeys.GitUrlOverride, picked);
OnGitUrlChanged?.Invoke();
OnHttpServerCommandUpdateRequested?.Invoke();
McpLog.Info($"Server source override set to: {picked}");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+49
| # Module-level cache for the Unity-configured limit (populated from editor state). | ||
| _cached_max_commands: int | None = None | ||
|
|
||
|
|
||
| async def _get_max_commands_from_editor_state(ctx: Context) -> int: | ||
| """ | ||
| Attempt to read the configured batch limit from the Unity editor state. | ||
| Falls back to DEFAULT_MAX_COMMANDS_PER_BATCH if unavailable. | ||
| """ | ||
| global _cached_max_commands | ||
| if _cached_max_commands is not None: | ||
| return _cached_max_commands | ||
|
|
||
| try: | ||
| from services.resources.editor_state import get_editor_state | ||
|
|
||
| state_resp = await get_editor_state(ctx) | ||
| data = state_resp.data if hasattr(state_resp, "data") else ( | ||
| state_resp.get("data") if isinstance(state_resp, dict) else None | ||
| ) | ||
| if isinstance(data, dict): | ||
| settings = data.get("settings") | ||
| if isinstance(settings, dict): | ||
| limit = settings.get("batch_execute_max_commands") | ||
| if isinstance(limit, int) and 1 <= limit <= ABSOLUTE_MAX_COMMANDS_PER_BATCH: | ||
| _cached_max_commands = limit | ||
| return limit |
There was a problem hiding this comment.
_cached_max_commands is cached at module scope and not keyed by Unity instance. Since the server supports multiple Unity instances via unity_instance context state, a limit read from one project can incorrectly apply to another (rejecting valid batches or allowing batches Unity will reject). Cache this per unity_instance (e.g., dict keyed by instance id) or remove the cache and read editor_state each call.
Comment on lines
+27
to
+59
| async def _get_max_commands_from_editor_state(ctx: Context) -> int: | ||
| """ | ||
| Attempt to read the configured batch limit from the Unity editor state. | ||
| Falls back to DEFAULT_MAX_COMMANDS_PER_BATCH if unavailable. | ||
| """ | ||
| global _cached_max_commands | ||
| if _cached_max_commands is not None: | ||
| return _cached_max_commands | ||
|
|
||
| try: | ||
| from services.resources.editor_state import get_editor_state | ||
|
|
||
| state_resp = await get_editor_state(ctx) | ||
| data = state_resp.data if hasattr(state_resp, "data") else ( | ||
| state_resp.get("data") if isinstance(state_resp, dict) else None | ||
| ) | ||
| if isinstance(data, dict): | ||
| settings = data.get("settings") | ||
| if isinstance(settings, dict): | ||
| limit = settings.get("batch_execute_max_commands") | ||
| if isinstance(limit, int) and 1 <= limit <= ABSOLUTE_MAX_COMMANDS_PER_BATCH: | ||
| _cached_max_commands = limit | ||
| return limit | ||
| except Exception as exc: | ||
| logger.debug("Could not read batch limit from editor state: %s", exc) | ||
|
|
||
| return DEFAULT_MAX_COMMANDS_PER_BATCH | ||
|
|
||
|
|
||
| def invalidate_cached_max_commands() -> None: | ||
| """Reset the cached limit so the next call re-reads from editor state.""" | ||
| global _cached_max_commands | ||
| _cached_max_commands = None |
There was a problem hiding this comment.
The max-commands limit is cached indefinitely after the first read. If the user changes the setting in the Unity MCP Tools window while the server is running, the server will keep enforcing the old limit until invalidate_cached_max_commands() is called (currently unused). Consider adding a TTL/refresh mechanism, invalidating on editor_state changes, or just re-reading editor_state each request to keep behavior consistent with the UI.
Comment on lines
+355
to
+392
| if (string.IsNullOrEmpty(path)) | ||
| return path; | ||
|
|
||
| // If path is not a local filesystem path, return as-is (git URLs, PyPI refs, etc.) | ||
| if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) || | ||
| path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return path; | ||
| } | ||
|
|
||
| // Strip file:// prefix for filesystem checks, but preserve it for the return value | ||
| string checkPath = path; | ||
| string prefix = string.Empty; | ||
| if (checkPath.StartsWith("file://", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| prefix = "file://"; | ||
| checkPath = checkPath.Substring(7); | ||
| } | ||
|
|
||
| // Already points to a directory with pyproject.toml — correct path | ||
| if (File.Exists(Path.Combine(checkPath, "pyproject.toml"))) | ||
| { | ||
| return path; | ||
| } | ||
|
|
||
| // Check if "Server" subdirectory contains pyproject.toml (common repo structure) | ||
| string serverSubDir = Path.Combine(checkPath, "Server"); | ||
| if (File.Exists(Path.Combine(serverSubDir, "pyproject.toml"))) | ||
| { | ||
| string corrected = prefix + serverSubDir; | ||
| McpLog.Info($"Auto-corrected server path to 'Server' subdirectory: {corrected}"); | ||
| return corrected; | ||
| } | ||
|
|
||
| // Return as-is; uvx will report the error if the path is invalid | ||
| return path; |
There was a problem hiding this comment.
ResolveServerPath() duplicates the local-path auto-correction logic that was also added to AssetPathUtility.ResolveLocalServerPath(). Keeping two implementations increases the chance of drift (e.g., handling of file:/// URIs, future supported schemes). Prefer reusing the helper from AssetPathUtility (and/or extracting a single shared utility) so both the UI and command-building paths behave identically.
Suggested change
| if (string.IsNullOrEmpty(path)) | |
| return path; | |
| // If path is not a local filesystem path, return as-is (git URLs, PyPI refs, etc.) | |
| if (path.StartsWith("http://", StringComparison.OrdinalIgnoreCase) || | |
| path.StartsWith("https://", StringComparison.OrdinalIgnoreCase) || | |
| path.StartsWith("git+", StringComparison.OrdinalIgnoreCase) || | |
| path.StartsWith("ssh://", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| return path; | |
| } | |
| // Strip file:// prefix for filesystem checks, but preserve it for the return value | |
| string checkPath = path; | |
| string prefix = string.Empty; | |
| if (checkPath.StartsWith("file://", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| prefix = "file://"; | |
| checkPath = checkPath.Substring(7); | |
| } | |
| // Already points to a directory with pyproject.toml — correct path | |
| if (File.Exists(Path.Combine(checkPath, "pyproject.toml"))) | |
| { | |
| return path; | |
| } | |
| // Check if "Server" subdirectory contains pyproject.toml (common repo structure) | |
| string serverSubDir = Path.Combine(checkPath, "Server"); | |
| if (File.Exists(Path.Combine(serverSubDir, "pyproject.toml"))) | |
| { | |
| string corrected = prefix + serverSubDir; | |
| McpLog.Info($"Auto-corrected server path to 'Server' subdirectory: {corrected}"); | |
| return corrected; | |
| } | |
| // Return as-is; uvx will report the error if the path is invalid | |
| return path; | |
| // Delegate to shared helper to keep path resolution logic centralized | |
| return AssetPathUtility.ResolveLocalServerPath(path); |
Scriptwonder
added a commit
that referenced
this pull request
Feb 12, 2026
This reverts commit ad3756e.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Type of Change
Save your change type
Changes Made
Testing/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
Additional Notes
Summary by Sourcery
Fix MCP server path overrides and make batch execution limits configurable while documenting UI creation workflows.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Documentation