feat: add command gateway for multi-agent concurrent access#826
Open
Lint111 wants to merge 33 commits intoCoplayDev:betafrom
Open
feat: add command gateway for multi-agent concurrent access#826Lint111 wants to merge 33 commits intoCoplayDev:betafrom
Lint111 wants to merge 33 commits intoCoplayDev:betafrom
Conversation
Add ExecutionTier enum (Instant/Smooth/Heavy) to classify tool execution cost. Extend McpForUnityToolAttribute with Tier property (default Smooth). Annotate built-in tools: Heavy for manage_script, manage_shader, refresh_unity, run_tests. Instant for find_gameobjects, read_console, get_test_job. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CommandClassifier.Classify() inspects action parameters to override the declared tool tier (e.g., manage_scene.get_hierarchy -> Instant, manage_scene.load -> Heavy). ClassifyBatch() returns the highest tier across a set of commands for queue scheduling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
BatchJob tracks ticket, agent, commands, results, and status. TicketStore manages CRUD operations, ticket generation, expiry cleanup, and per-agent statistics. Foundation for the command gateway queue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CommandQueue provides FIFO dispatch with tier awareness: instant jobs execute inline, smooth jobs run when no heavy is active, heavy jobs get exclusive access. CommandGatewayState hooks into EditorApplication .update for per-frame queue processing. CommandRegistry extended with GetToolTier() and tier storage in HandlerInfo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When async=true is passed, BatchExecute now routes through CommandGatewayState.Queue instead of executing inline. Returns a ticket for non-instant batches (poll via poll_job) or results directly for instant batches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PollJob (Instant tier) returns job status with position/progress/results. QueueStatus (Instant tier) returns queue depth, active heavy job info, smooth in-flight count, and per-agent statistics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
45 tests covering: - ExecutionTier enum ordering and attribute defaults (5) - CommandClassifier action-level overrides and batching (13) - TicketStore CRUD, expiry, agent stats (10) - CommandQueue submit, poll, cancel, queue depth (10) - BatchExecute async path, PollJob, QueueStatus integration (7) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Design for two fixes to the command gateway: 1. Domain-reload-causing operations (refresh_unity, play mode) are held in queue while tests are running or compilation is active 2. Queue state persists across domain reloads via SessionState Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7-task TDD plan for making the command gateway aware of async Unity operations (test runs, compilation) so domain-reload-causing commands are held until safe, and queue state survives domain reloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only refresh_unity (compile!=none) and manage_editor (play) trigger domain reload. Script/shader file ops are just disk I/O. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hJob Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Domain-reload-causing jobs (refresh_unity compile, manage_editor play) are held in queue while TestJobManager.HasRunningJob or EditorApplication.isCompiling is true. Non-reload heavy jobs proceed normally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a queued domain-reload job is blocked by editor activity (tests running, compiling), the poll_job response now includes a blocked_by field indicating the reason. Non-reload jobs return null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TicketStore now supports ToJson/FromJson serialization. Running jobs are marked failed on restore (interrupted by reload), queued jobs survive and re-enter the heavy queue. CommandGatewayState hooks AssemblyReloadEvents to persist via SessionState. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When run_tests completes instantly (fire-and-forget) but the TestRunner is still executing, the heavy slot was released immediately, allowing domain-reload jobs like refresh_unity to be dequeued before the guard could catch them. Three fixes: 1. Hold the heavy slot after batch completion while IsEditorBusy() is true — prevents next heavy dequeue until async operations settle 2. One-frame cooldown between heavy job completions — gives async state one editor frame to propagate before the guard check 3. Add TestRunStatus.IsRunning to IsEditorBusy predicate for defense-in- depth (covers manual UI test runs alongside TestJobManager tracking) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instant tier (never queued, non-blocking). Returns all tickets with compact field names (t/s/a/l/p/b/e) for token-efficient batch polling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New Queue tab for MCP editor window with real-time job list, status bar, and 1s auto-refresh. UIElements-based section component. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6 tasks: expose GetAllJobs, UXML layout, USS styles, C# controller, wire into editor window with 1s refresh, integration verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…job list Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…refresh Adds Queue as 6th tab in the MCP For Unity editor window. Loads McpQueueSection UXML/USS/controller, wires tab toggle and panel switching, adds independent 1-second refresh timer that only fires while the Queue tab is visible (before the 2-second connection status throttle). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Queue tab was blank because UIAssetSync didn't copy the new UXML/USS files from WSL to Assets/MCPForUnityUI/. Added both files to the sync list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When an agent polls a completed/failed/cancelled job, the ticket is removed from the queue after building the response. If the GatewayJobLogging EditorPref is enabled, job data is serialized to Logs/mcp-gateway-jobs.jsonl before removal. - Add Remove(ticket) to TicketStore and CommandQueue - Add GatewayJobLogger utility (JSON-lines file output) - Add GatewayJobLogging EditorPref key - Modify PollJob to auto-remove terminal jobs after response Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add Unity .meta files for Queue UI components and UIAssetSync - AssetPathUtility returns UIAssetSync.SyncedBasePath when WSL detected Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Toggle to enable/disable gateway job logging - Text field showing current log file path (default: Logs/mcp-gateway-jobs.jsonl) - Browse button opens file dialog to choose custom log path - Path row hidden when logging is disabled - GatewayJobLogger now supports configurable path via EditorPrefs - Light/dark theme support for logging box Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, a batch containing run_tests would show status "done" immediately after the command returned, even though the test runner was still executing asynchronously. Now: - Heavy jobs stay in Running status if IsEditorBusy() is true after all commands dispatch - ProcessTick transitions them to Done once the editor settles - Remove() clears _activeHeavyTicket if the removed job was active - ProcessTick handles null jobs (removed by auto-cleanup) gracefully Tested: Agent 1 run_tests shows "running" during test execution, transitions to "done" only after tests finish. Agent 2 refresh_unity correctly blocked with "tests_running" until Agent 1 completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Jobs that caused a domain reload (e.g., refresh_unity with compile) and had all commands dispatched are now restored as Done instead of Failed when the editor reloads. The reload IS the success signal for these jobs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Reviewer's GuideImplements a tier-aware command gateway with async batch execution, persistent job queue, and an in-editor Queue tab, including new tooling for job polling, queue inspection, domain-reload safety, and optional JSONL job logging. Sequence diagram for async batch_execute submission via command gatewaysequenceDiagram
actor Agent
participant MCPClient
participant BatchExecute
participant CommandRegistry
participant CommandClassifier
participant CommandGatewayState
participant CommandQueue
Agent->>MCPClient: Send batch_execute(async=true, commands)
MCPClient->>BatchExecute: HandleCommand(params)
BatchExecute->>BatchExecute: Validate commands, atomic, agent, label
BatchExecute->>CommandRegistry: GetToolTier(toolName) for each command
CommandRegistry-->>BatchExecute: ExecutionTier
loop For_each_command
BatchExecute->>CommandClassifier: Classify(toolName, toolTier, cmdParams)
CommandClassifier-->>BatchExecute: effectiveTier
BatchExecute->>CommandClassifier: CausesDomainReload(toolName, cmdParams)
CommandClassifier-->>BatchExecute: causesDomainReload
BatchExecute->>BatchExecute: Build BatchCommand(Tool, Params, Tier, CausesDomainReload)
end
BatchExecute->>CommandGatewayState: Queue.Submit(agent, label, atomic, commands)
CommandGatewayState->>CommandQueue: Submit(agent, label, atomic, commands)
CommandQueue->>CommandQueue: ClassifyBatch(commands)
CommandQueue->>CommandQueue: CreateJob + enqueue heavy if needed
CommandGatewayState-->>BatchExecute: BatchJob(job)
alt job.Tier == Instant
BatchExecute->>CommandRegistry: InvokeCommandAsync for each command
CommandRegistry-->>BatchExecute: results
BatchExecute->>BatchExecute: job.Results, Status=Done
BatchExecute-->>MCPClient: SuccessResponse(ticket, results)
else Non_instant (Smooth or Heavy)
BatchExecute->>CommandQueue: GetAheadOf(job.Ticket)
CommandQueue-->>BatchExecute: jobsAhead
BatchExecute-->>MCPClient: PendingResponse(ticket, status=queued, position, tier, agent, label)
end
MCPClient-->>Agent: Return ticket for later polling
Sequence diagram for poll_job lifecycle and auto-cleanupsequenceDiagram
actor Agent
participant MCPClient
participant PollJob
participant CommandGatewayState
participant CommandQueue
participant TicketStore
participant GatewayJobLogger
Agent->>MCPClient: Send poll_job(ticket)
MCPClient->>PollJob: HandleCommand(params)
PollJob->>CommandGatewayState: Queue.Poll(ticket)
CommandGatewayState->>CommandQueue: Poll(ticket)
CommandQueue->>TicketStore: GetJob(ticket)
TicketStore-->>CommandQueue: BatchJob(job)
CommandQueue-->>PollJob: BatchJob(job)
alt job.Status == Queued
PollJob->>CommandQueue: GetAheadOf(ticket)
CommandQueue-->>PollJob: aheadJobs
PollJob->>CommandQueue: GetBlockedReason(ticket)
CommandQueue-->>PollJob: blockedBy
PollJob-->>MCPClient: PendingResponse(status=queued, position, blocked_by)
else job.Status == Running
PollJob-->>MCPClient: PendingResponse(status=running, progress)
else job.Status == Done
PollJob-->>MCPClient: SuccessResponse(status=done, results)
PollJob->>GatewayJobLogger: Log(job) [if enabled]
PollJob->>CommandGatewayState: Queue.Remove(ticket)
CommandGatewayState->>CommandQueue: Remove(ticket)
CommandQueue->>TicketStore: Remove(ticket)
TicketStore-->>CommandQueue: removedJob
else job.Status == Failed or Cancelled
PollJob-->>MCPClient: ErrorResponse(status, error, results)
PollJob->>GatewayJobLogger: Log(job) [if enabled]
PollJob->>CommandGatewayState: Queue.Remove(ticket)
CommandGatewayState->>CommandQueue: Remove(ticket)
CommandQueue->>TicketStore: Remove(ticket)
TicketStore-->>CommandQueue: removedJob
end
MCPClient-->>Agent: Present job status or results
Updated class diagram for the command gateway and queueing modelclassDiagram
direction LR
class ExecutionTier {
<<enum>>
Instant
Smooth
Heavy
}
class JobStatus {
<<enum>>
Queued
Running
Done
Failed
Cancelled
}
class McpForUnityToolAttribute {
+string CommandName
+string PollAction
+ExecutionTier Tier
}
class CommandClassifier {
+ExecutionTier Classify(toolName, attributeTier, @params)
+ExecutionTier ClassifyBatch(commands)
+bool CausesDomainReload(toolName, @params)
}
class BatchCommand {
+string Tool
+JObject Params
+ExecutionTier Tier
+bool CausesDomainReload
}
class BatchJob {
+string Ticket
+string Agent
+string Label
+bool Atomic
+ExecutionTier Tier
+bool CausesDomainReload
+JobStatus Status
+List~BatchCommand~ Commands
+List~object~ Results
+int CurrentIndex
+string Error
+DateTime CreatedAt
+DateTime CompletedAt
+int UndoGroup
}
class AgentStats {
+int Active
+int Queued
+int Completed
}
class TicketStore {
-Dictionary~string,BatchJob~ _jobs
-int _nextId
+BatchJob CreateJob(agent, label, atomic, tier)
+BatchJob GetJob(ticket)
+BatchJob Remove(ticket)
+void CleanExpired(expiry)
+List~BatchJob~ GetQueuedJobs()
+List~BatchJob~ GetRunningJobs()
+Dictionary~string,AgentStats~ GetAgentStats()
+List~BatchJob~ GetAllJobs()
+int QueueDepth
+string ToJson()
+void FromJson(json)
}
class CommandQueue {
-TicketStore _store
-Queue~string~ _heavyQueue
-List~string~ _smoothInFlight
-string _activeHeavyTicket
+Func~bool~ IsEditorBusy
+bool HasActiveHeavy
+int QueueDepth
+int SmoothInFlight
+BatchJob Submit(agent, label, atomic, commands)
+BatchJob Poll(ticket)
+bool Cancel(ticket, agent)
+List~BatchJob~ GetAheadOf(ticket)
+string GetBlockedReason(ticket)
+BatchJob Remove(ticket)
+object GetSummary()
+List~BatchJob~ GetAllJobs()
+object GetStatus()
+string PersistToJson()
+void RestoreFromJson(json)
+void ProcessTick(executeCommand)
-Task ExecuteJob(job, executeCommand)
}
class CommandGatewayState {
<<static>>
+CommandQueue Queue
-const string SessionKey
-static void OnUpdate()
}
class GatewayJobLogger {
<<static>>
+string DefaultLogPath
+bool IsEnabled
+string LogPath
+void Log(job)
}
class PollJob {
<<tool>>
+object HandleCommand(@params)
}
class QueueStatus {
<<tool>>
+object HandleCommand(@params)
}
class BatchExecute {
+Task~object~ HandleCommand(@params)
-object HandleAsyncSubmit(@params, commandsToken)
}
class CommandRegistry {
+ExecutionTier GetToolTier(commandName)
+Task~object~ InvokeCommandAsync(commandName, @params)
}
class McpQueueSection {
+VisualElement Root
+McpQueueSection(root)
+void Refresh()
-void CacheUIElements()
-void SetupLoggingControls()
-void UpdateStatusBar(queue, allJobs)
-void UpdateJobList(allJobs, queue)
-VisualElement CreateJobRow(job, queue)
}
class MCPForUnityEditorWindow {
-McpQueueSection queueSection
-ToolbarToggle queueTabToggle
-VisualElement queuePanel
-double _lastQueueRefreshTime
-ActivePanel _currentPanel
-void OnEditorUpdate()
-void SetupTabs()
-void SwitchPanel(panel)
-void CreateGUI()
}
class ExecutionTierTests
class CommandClassifierTests
class CommandQueueTests
class TicketStoreTests
class BatchExecuteAsyncTests
McpForUnityToolAttribute --> ExecutionTier
CommandClassifier --> ExecutionTier
CommandClassifier --> BatchCommand
BatchJob --> ExecutionTier
BatchJob --> JobStatus
BatchJob --> BatchCommand
TicketStore --> BatchJob
TicketStore --> AgentStats
CommandQueue --> TicketStore
CommandQueue --> BatchJob
CommandQueue --> BatchCommand
CommandGatewayState --> CommandQueue
GatewayJobLogger --> BatchJob
PollJob --> CommandGatewayState
PollJob --> GatewayJobLogger
QueueStatus --> CommandGatewayState
BatchExecute --> CommandRegistry
BatchExecute --> CommandClassifier
BatchExecute --> CommandGatewayState
McpQueueSection --> CommandGatewayState
MCPForUnityEditorWindow --> McpQueueSection
ExecutionTierTests --> ExecutionTier
CommandClassifierTests --> CommandClassifier
CommandQueueTests --> CommandQueue
TicketStoreTests --> TicketStore
BatchExecuteAsyncTests --> BatchExecute
Updated class diagram for the Queue tab UI componentsclassDiagram
direction LR
class MCPForUnityEditorWindow {
-McpQueueSection queueSection
-ToolbarToggle queueTabToggle
-VisualElement queuePanel
-double _lastQueueRefreshTime
-double QueueRefreshIntervalSeconds
-double _lastEditorUpdateTime
-double EditorUpdateIntervalSeconds
-ActivePanel _currentPanel
+void CreateGUI()
-void SetupTabs()
-void SwitchPanel(panel)
-void OnEditorUpdate()
}
class McpQueueSection {
+VisualElement Root
-Label heavyTicketLabel
-Label queuedCount
-Label runningCount
-Label doneCount
-Label failedCount
-VisualElement jobListContainer
-VisualElement jobListHeader
-Label emptyLabel
-Toggle loggingToggle
-TextField logPathField
-Button browseLogPathBtn
-VisualElement loggingPathRow
+McpQueueSection(root)
+void Refresh()
-void CacheUIElements()
-void SetupLoggingControls()
-void UpdateLoggingPathVisibility()
-void UpdateStatusBar(queue, allJobs)
-void UpdateJobList(allJobs, queue)
-VisualElement CreateJobRow(job, queue)
-string Truncate(text, maxLength)
}
class GatewayJobLogger {
<<static>>
+string DefaultLogPath
+bool IsEnabled
+string LogPath
+void Log(job)
}
class CommandGatewayState {
<<static>>
+CommandQueue Queue
}
class CommandQueue {
+List~BatchJob~ GetAllJobs()
+bool HasActiveHeavy
+int QueueDepth
+int SmoothInFlight
+string GetBlockedReason(ticket)
+Func~bool~ IsEditorBusy
}
class TicketStore {
+List~BatchJob~ GetAllJobs()
}
class BatchJob {
+string Ticket
+string Agent
+string Label
+ExecutionTier Tier
+bool CausesDomainReload
+JobStatus Status
+List~BatchCommand~ Commands
+List~object~ Results
+int CurrentIndex
+string Error
}
class UIAssetSync {
<<static>>
+string SyncedBasePath
+bool NeedsSync()
}
class AssetPathUtility {
+string GetMcpPackageRootPath()
}
MCPForUnityEditorWindow --> McpQueueSection
MCPForUnityEditorWindow --> UIAssetSync
UIAssetSync --> AssetPathUtility
McpQueueSection --> CommandGatewayState
CommandGatewayState --> CommandQueue
CommandQueue --> TicketStore
CommandQueue --> BatchJob
McpQueueSection --> GatewayJobLogger
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 3 issues, and left some high level feedback:
- In
CommandQueue,GetBlockedReasonreaches intoTestJobManager/TestRunStatus/EditorApplicationdirectly even thoughIsEditorBusyis already injected; consider routing both the boolean and reason through a single delegate/interface so the queue remains decoupled from editor/test services and easier to fake in tests. - The
HandleAsyncSubmitpath inBatchExecuteruns "instant" commands synchronously on the editor thread viaInvokeCommandAsync(...).GetAwaiter().GetResult(), which risks UI stalls or deadlocks for long-running async tools; consider making this path fully async (usingawait) or explicitly constraining/documenting the tools that can be treated asInstant.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CommandQueue`, `GetBlockedReason` reaches into `TestJobManager`/`TestRunStatus`/`EditorApplication` directly even though `IsEditorBusy` is already injected; consider routing both the boolean and reason through a single delegate/interface so the queue remains decoupled from editor/test services and easier to fake in tests.
- The `HandleAsyncSubmit` path in `BatchExecute` runs "instant" commands synchronously on the editor thread via `InvokeCommandAsync(...).GetAwaiter().GetResult()`, which risks UI stalls or deadlocks for long-running async tools; consider making this path fully async (using `await`) or explicitly constraining/documenting the tools that can be treated as `Instant`.
## Individual Comments
### Comment 1
<location path="TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs" line_range="59" />
<code_context>
+ [Test]
+ public void BatchExecuteMaxCommands_DefaultIs25()
</code_context>
<issue_to_address>
**suggestion (testing):** Add targeted tests for the new async batch submission path (instant vs queued behaviour and error conditions).
The new `BatchExecute.HandleAsyncSubmit` path is core to this gateway, but it isn’t covered by the current tests. Please add tests that drive the async path via the `batch_execute` tool, including:
- An `async=true` batch with only `ExecutionTier.Instant` commands, asserting a `SuccessResponse` (`status="done"`, `ticket`, and populated `results`) and that the `BatchJob` is completed rather than queued.
- An `async=true` batch that includes `Smooth`/`Heavy` commands, asserting a `PendingResponse` (`ticket`, `status="queued"`, `position`, `tier`, `agent`, `label`, `atomic`), and ideally verifying the queued job state (e.g. via `CommandGatewayState.Queue.Poll(ticket)`).
- Error cases: only invalid commands (e.g. missing `tool`) should return `ErrorResponse("No valid commands in async batch.")`, and a failing command in an atomic instant batch should fail with partial `results` and the `ticket` present in the error data.
This will exercise the new async semantics end-to-end and validate the instant-vs-queued behaviour.
</issue_to_address>
### Comment 2
<location path="docs/plans/2026-02-25-gateway-async-awareness-plan.md" line_range="394-395" />
<code_context>
+ }
+
+ _activeHeavyTicket = ticket;
+ _ = ExecuteJob(job, executeCommand);
+ return;
+ }
</code_context>
<issue_to_address>
**nitpick (typo):** Clarify grammar in the inline comment about re-enqueuing peeked items.
The inline comment `// Re-enqueue any remaining peeked items were already handled by the loop` is grammatically awkward. Please rephrase, e.g. `// Remaining peeked items are already handled by the loop` or similar.
Suggested implementation:
```
if (job == null || job.Status == JobStatus.Cancelled) continue;
// Guard: domain-reload jobs must wait when editor is busy
if (job.CausesDomainReload && editorBusy)
{
_heavyQueue.Enqueue(ticket); // put back at end
continue;
}
// Remaining peeked items are already handled by the loop
_activeHeavyTicket = ticket;
```
If the original inline comment you referenced (`// Re-enqueue any remaining peeked items were already handled by the loop`) appears elsewhere in the file or in related files, apply a similar replacement there:
<<<<<<< SEARCH
// Re-enqueue any remaining peeked items were already handled by the loop
=======
// Remaining peeked items are already handled by the loop
>>>>>>> REPLACE
Adjust capitalization or wording to match your team's commenting style if needed.
</issue_to_address>
### Comment 3
<location path="docs/plans/2026-02-25-queue-summary-ui-design.md" line_range="34-36" />
<code_context>
+
+### Status bar
+- Active heavy ticket with agent + label (or "No active heavy job")
+- Three counters: Queued / Running / Done+Failed
+- Uses existing `.section` and `.setting-row` styles
+
</code_context>
<issue_to_address>
**suggestion:** Align the documented counter list with the implemented four counters.
This line still refers to “Three counters: Queued / Running / Done+Failed”, but the UI and UXML/USS show four separate counters: queued, running, done, and failed. Please either list all four here or reword this bullet so it accurately matches the final UI behaviour.
```suggestion
### Status bar
- Active heavy ticket with agent + label (or "No active heavy job")
- Four counters: Queued / Running / Done / Failed
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a sophisticated command gateway system that enables safe concurrent access to Unity from multiple MCP clients. The implementation addresses the critical problem of domain reloads interrupting parallel agent operations by classifying tools into execution tiers (Instant/Smooth/Heavy) and implementing intelligent queue scheduling with domain-reload awareness.
Changes:
- Core gateway infrastructure with tier-based command classification, queue management, and SessionState persistence for domain-reload survival
- New
poll_jobandqueue_statusMCP tools for asynchronous batch execution with ticket-based polling - Real-time Queue tab in the MCP editor window with auto-refresh, job logging, and status visualization
- WSL compatibility workaround (UIAssetSync) for UXML/USS assets
- Extended batch_execute with async=true parameter for non-blocking command submission
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| ExecutionTier.cs | Enum defining three-tier classification system (Instant/Smooth/Heavy) |
| CommandClassifier.cs | Action-level tier refinement logic with domain-reload detection |
| BatchJob.cs | Job model with lifecycle tracking, undo support, and domain-reload flag |
| TicketStore.cs | Job storage with ticket generation, expiry, and SessionState serialization |
| CommandQueue.cs | Tier-aware FIFO dispatcher with exclusive heavy scheduling and editor-busy guards |
| CommandGatewayState.cs | Singleton wiring queue to EditorApplication.update with persistence hooks |
| PollJob.cs | MCP tool for polling job status with auto-cleanup of terminal jobs |
| QueueStatus.cs | Compressed queue introspection tool |
| GatewayJobLogger.cs | Optional JSON-lines audit logging with configurable path |
| McpQueueSection.uxml/uss/cs | Queue tab UI with status bar, job list, and logging controls |
| BatchExecute.cs | Extended with async gateway path alongside synchronous execution |
| UIAssetSync.cs | WSL UNC path workaround for Unity's UXML/USS importer limitations |
| 5 test files | 47 unit tests covering classifier, queue, store, and async tools |
| Tool files (7) | ExecutionTier annotations for proper queue dispatch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs
Show resolved
Hide resolved
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
📝 WalkthroughWalkthroughAdds an async-aware, tiered batch execution system to the Unity editor MCP gateway: new ExecutionTier model, per-command classification (including domain-reload detection), a persistent command queue with ticketing, polling endpoints, gateway job logging, editor Queue UI, WSL UI-asset sync, and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant BatchExec as BatchExecute
participant Classifier as CommandClassifier
participant Queue as CommandQueue
participant Store as TicketStore
participant EditorLoop as Editor.update
participant Poll as PollJob
Client->>BatchExec: async submit(commands)
BatchExec->>Classifier: classify each command
Classifier-->>BatchExec: (tier, causesDomainReload)
BatchExec->>Queue: Submit(agent,label,atomic,commands)
Queue->>Store: CreateJob(...)
Store-->>Queue: BatchJob(ticket)
Queue-->>BatchExec: PendingResponse(ticket)
Client->>Poll: poll_job(ticket)
Poll->>Queue: Poll(ticket)
Queue->>Store: GetJob(ticket)
Store-->>Queue: BatchJob(status)
Queue-->>Poll: Pending/Success/Error
EditorLoop->>Queue: ProcessTick(executeCommand)
Queue->>EditorLoop: executeCommand(tool,params)
EditorLoop-->>Queue: command result
Queue->>Store: Update job status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Tools/BatchExecute.cs (1)
35-66: 🛠️ Refactor suggestion | 🟠 MajorUse
ToolParamsfor top-level batch parameter parsing.Line 56 and Lines 250-253 parse tool inputs directly from
JObject. Please switch these toToolParamsso required/optional validation is consistent across editor tools.As per coding guidelines
MCPForUnity/Editor/Tools/*.cs: UseToolParamsfor consistent parameter validation in C# Editor tools with methods likeGetInt(),RequireString().Also applies to: 248-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/BatchExecute.cs` around lines 35 - 66, The top-level parameter parsing in HandleCommand is using JObject.Value/ indexing directly; replace this with a ToolParams instance (e.g. new ToolParams(`@params`)) and call its helpers (GetInt(), GetBool() / GetString() / RequireString() as appropriate) to read "async", "failFast", "parallel", "maxParallelism" and the "commands" array so validation is consistent with other editor tools; specifically, change the commandsToken extraction to use ToolParams to require/validate the "commands" entry and replace usages of `@params.Value`<int?>("maxParallelism") and `@params.Value`<bool?>("async") etc. with the matching ToolParams methods while preserving the async vs legacy flow inside HandleCommand and when calling HandleAsyncSubmit.
🟡 Minor comments (13)
docs/plans/2026-02-25-gateway-async-awareness-plan.md-90-94 (1)
90-94:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks (MD040).
Lines 90, 121, 886, 893, and 904 start unlabeled fences. Please add a language (
text,bash,csharp,json, etc.) to keep markdownlint clean.Suggested fix pattern
-``` +```text run_tests(mode=EditMode, test_names=[ "MCPForUnity.Tests.Editor.CommandClassifierTests.CausesDomainReload_RefreshUnity_CompileRequest_ReturnsTrue" ]) -``` +```Also applies to: 121-123, 886-889, 893-896, 904-933
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-gateway-async-awareness-plan.md` around lines 90 - 94, The markdown contains unlabeled fenced code blocks (e.g., the run_tests(...) snippet and other blocks referenced at lines 90, 121, 886, 893, 904) causing MD040; update each triple-backtick fence to include an appropriate language identifier (for the run_tests snippet use text or bash, and for other blocks choose csharp/json/text as appropriate) so every code fence is labeled (e.g., ```text or ```csharp) while keeping the block contents unchanged.docs/plans/2026-02-25-gateway-async-awareness-plan.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorFix heading level jump to satisfy markdown linting.
Line 13 jumps from
#to###; use##here to avoid MD001 violations.Suggested fix
-### Task 1: Add `CausesDomainReload` to CommandClassifier +## Task 1: Add `CausesDomainReload` to CommandClassifier🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-gateway-async-awareness-plan.md` at line 13, Heading level jumps from H1 to H3 for the section titled "Task 1: Add `CausesDomainReload` to CommandClassifier"; change the heading token from "### Task 1: Add `CausesDomainReload` to CommandClassifier" to "## Task 1: Add `CausesDomainReload` to CommandClassifier" so the document uses a proper H2 and satisfies markdown lint MD001.docs/plans/2026-02-25-gateway-async-awareness-design.md-27-31 (1)
27-31:⚠️ Potential issue | 🟡 MinorSpecify a language on the fenced block to satisfy markdownlint.
Line 27 opens an unlabeled fence; add a language token (for example
text).Suggested fix
-``` +```text if job.CausesDomainReload AND (TestJobManager.HasRunningJob OR EditorApplication.isCompiling) → skip, keep queued -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-gateway-async-awareness-design.md` around lines 27 - 31, The fenced code block containing the conditional snippet with symbols job.CausesDomainReload, TestJobManager.HasRunningJob, and EditorApplication.isCompiling is missing a language specifier; update the opening fence to include a language token (for example "text") so the block becomes ```text ... ``` to satisfy markdownlint while preserving the exact contents of the snippet.docs/plans/2026-02-25-queue-summary-ui-design.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorAdd language specifiers to fenced code blocks to resolve MD040 lint warnings.
Both ASCII-art/pseudo-code blocks are missing a language tag (e.g.,
```text).Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-queue-summary-ui-design.md` at line 13, Add explicit language specifiers to the fenced code blocks containing ASCII-art/pseudo-code (the two triple-backtick blocks flagged by MD040) by changing their opening fences from ``` to a tagged form like ```text (or ```ascii) so the linter recognizes them; update both code fences so each opening triple-backtick includes the chosen language tag.MCPForUnity/Editor/Tools/PollJob.cs-57-68 (1)
57-68:⚠️ Potential issue | 🟡 MinorGuard against
nullCommandsin theRunningbranch.
job.Commands.Countandjob.Commands.Countat Lines 59 and 65 will throw aNullReferenceExceptionifCommandsis evernullon aRunningjob. While the current execution flow guarantees it's set before a job entersRunning, defensive guarding avoids a silent regression if that invariant is weakened later.🛡️ Proposed fix
- response = new PendingResponse( - $"Running command {job.CurrentIndex + 1}/{job.Commands.Count}.", + int total = job.Commands?.Count ?? 0; + response = new PendingResponse( + $"Running command {job.CurrentIndex + 1}/{total}.", pollIntervalSeconds: 1.0, data: new { ticket = job.Ticket, status = "running", - progress = $"{job.CurrentIndex + 1}/{job.Commands.Count}", + progress = $"{job.CurrentIndex + 1}/{total}", agent = job.Agent, label = job.Label });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/PollJob.cs` around lines 57 - 68, Guard against job.Commands being null in the JobStatus.Running branch: before using job.Commands.Count or constructing the progress string, check for null (or use a safe fallback) and compute safeCount = job.Commands?.Count ?? 0 and safeIndex = Math.Min(job.CurrentIndex + 1, safeCount); then build the PendingResponse using safeCount and safeIndex for the message and progress fields (update the Running case that creates the PendingResponse to reference these safe values rather than job.Commands.Count directly).TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/TicketStoreTests.cs-113-121 (1)
113-121:⚠️ Potential issue | 🟡 MinorPotential flaky ordering —
CreatedAttimestamps may collide.
GetQueuedJobs()sorts byCreatedAt. If both jobs are created in the same millisecond (common in unit tests),OrderByis not guaranteed to be stable between runs, and the positional assertions on lines 119–120 can fail non-deterministically.Consider setting
CreatedAtexplicitly or asserting on ticket presence without relying on index position:🔧 Proposed fix
[Test] public void GetQueuedJobs_OrderedByCreationTime() { var j1 = _store.CreateJob("a", "first", false, ExecutionTier.Heavy); + j1.CreatedAt = System.DateTime.UtcNow.AddSeconds(-1); var j2 = _store.CreateJob("b", "second", false, ExecutionTier.Smooth); var queued = _store.GetQueuedJobs(); Assert.That(queued[0].Ticket, Is.EqualTo(j1.Ticket)); Assert.That(queued[1].Ticket, Is.EqualTo(j2.Ticket)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/TicketStoreTests.cs` around lines 113 - 121, The test GetQueuedJobs_OrderedByCreationTime is flaky because CreateJob may assign identical CreatedAt timestamps; update the test to avoid relying on millisecond ordering by either explicitly setting distinct CreatedAt values on j1 and j2 after creation (e.g., j1.CreatedAt = now; j2.CreatedAt = now.AddMilliseconds(1)) before calling _store.GetQueuedJobs(), or change the assertions to check that both tickets are present without requiring specific positions (e.g., Assert.That(queued.Select(q => q.Ticket), Is.EquivalentTo(new[] { j1.Ticket, j2.Ticket }))); modify only the test logic around CreateJob, j1/j2, and the assertions so GetQueuedJobs and _store remain unchanged.MCPForUnity/Editor/Tools/PollJob.cs-18-21 (1)
18-21:⚠️ Potential issue | 🟡 MinorUse
RequireString()per theToolParamscoding guideline.
p.GetRequired("ticket")deviates from the establishedToolParamsAPI convention. As per coding guidelines, C# Editor tools should use methods likeRequireString()for string parameters.🔧 Proposed fix
- var ticketResult = p.GetRequired("ticket"); - if (!ticketResult.IsSuccess) - return new ErrorResponse(ticketResult.ErrorMessage); - - string ticket = ticketResult.Value; + var ticketResult = p.RequireString("ticket"); + if (!ticketResult.IsSuccess) + return new ErrorResponse(ticketResult.ErrorMessage); + + string ticket = ticketResult.Value;As per coding guidelines: "Use
ToolParamsfor consistent parameter validation in C# Editor tools with methods likeGetInt(),RequireString()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/PollJob.cs` around lines 18 - 21, The code is using p.GetRequired("ticket") instead of the ToolParams guideline method; replace the call with p.RequireString("ticket") (or the equivalent RequireString method on ToolParams) and update handling to use the returned result (e.g., check success and use its Value or return new ErrorResponse(result.ErrorMessage)) so the Ticket extraction uses ToolParams.RequireString(), referenced variable p, and the existing ErrorResponse/ticketResult flow.docs/plans/2026-02-25-queue-summary-ui-plan.md-35-38 (1)
35-38:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks.
These fenced blocks are unlabeled and trigger MD040. Please annotate each block with an explicit language (for example
bash,json, ortext).Also applies to: 491-494, 767-770, 797-799, 811-813
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-queue-summary-ui-plan.md` around lines 35 - 38, The Markdown fenced code blocks like the one containing "refresh_unity(scope=all, compile=request, wait_for_ready=true)" and the "read_console(types=[\"error\"]) → 0 errors" output are unlabeled and trigger MD040; edit each such fenced block (including the other occurrences mentioned) to add an explicit language identifier (e.g., ```bash, ```text or ```console) after the opening backticks so the block is annotated and the linter no longer flags MD040.MCPForUnity/Editor/Helpers/UIAssetSync.cs-71-72 (1)
71-72:⚠️ Potential issue | 🟡 MinorAnchor destination path to project root explicitly.
Line 71 builds a full path from a relative base (
Assets/...) using current working directory semantics. UsePath.GetDirectoryName(Application.dataPath)as the root so writes are always deterministic to the active project.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Helpers/UIAssetSync.cs` around lines 71 - 72, The code currently computes destPath with Path.GetFullPath(Path.Combine(SyncedBasePath, relativePath)) which uses the process CWD; change the construction to anchor against the Unity project root by using Path.GetDirectoryName(Application.dataPath) as the base before combining with SyncedBasePath and relativePath so destPath and destDir are deterministic. Update the logic that sets destPath and destDir (the variables destPath, destDir and the expression using SyncedBasePath/relativePath) to first compute projectRoot = Path.GetDirectoryName(Application.dataPath) and then combine projectRoot with SyncedBasePath and relativePath to produce the final paths.docs/plans/2026-02-25-queue-summary-ui-plan.md-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorFix heading-level jump in task sections.
Line 15 jumps from H1 to H3. Use H2 for
Task 1(and peers) to satisfy MD001.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-queue-summary-ui-plan.md` at line 15, The markdown heading "Task 1: Expose GetAllJobs on CommandQueue" is currently H3 (###) causing a jump from the document H1; change this and all peer task headings ("Task 1", "Task 2", etc.) from H3 to H2 (##) so the section hierarchy is H1 -> H2 -> H3 and MD001 is satisfied; update the lines with the exact heading texts to use "##" instead of "###".MCPForUnity/Editor/Tools/TicketStore.cs-39-45 (1)
39-45:⚠️ Potential issue | 🟡 Minor
CleanExpiredshould also clean cancelled terminal jobs.The expiry filter currently excludes
JobStatus.Cancelled, so cancelled jobs can accumulate when they are not removed immediately.Proposed fix
- .Where(kvp => (kvp.Value.Status == JobStatus.Done || kvp.Value.Status == JobStatus.Failed) + .Where(kvp => (kvp.Value.Status == JobStatus.Done + || kvp.Value.Status == JobStatus.Failed + || kvp.Value.Status == JobStatus.Cancelled) && kvp.Value.CompletedAt.HasValue && DateTime.UtcNow - kvp.Value.CompletedAt.Value > expiry)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/TicketStore.cs` around lines 39 - 45, The CleanExpired logic in TicketStore.CleanExpired currently filters terminal jobs by JobStatus.Done or JobStatus.Failed and thus misses cancelled terminal jobs; update the expiry predicate used when building expired (the LINQ Where over _jobs entries) to also include kvp.Value.Status == JobStatus.Cancelled so cancelled jobs with CompletedAt older than expiry are selected and removed from _jobs; ensure the selection still checks CompletedAt.HasValue and the time-based comparison remains unchanged.MCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.cs-216-221 (1)
216-221:⚠️ Potential issue | 🟡 MinorQueued jobs show incorrect progress (
1/N) before execution starts.Line 219 uses
job.CurrentIndex + 1for non-done jobs, so queued jobs render as if one command already ran. Render queued as0/N.Proposed fix
- int idx = job.Status == JobStatus.Done ? job.Commands.Count : job.CurrentIndex + 1; + int idx = job.Status switch + { + JobStatus.Queued => 0, + JobStatus.Done => job.Commands.Count, + _ => System.Math.Clamp(job.CurrentIndex + 1, 1, job.Commands.Count) + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.cs` around lines 216 - 221, The progress calculation currently uses job.CurrentIndex + 1 for non-done jobs which makes queued jobs show "1/N"; change the logic in McpQueueSection (the block that builds progressText) to use job.CurrentIndex for non-Done jobs and leave Done as job.Commands.Count, i.e. set idx = job.Status == JobStatus.Done ? job.Commands.Count : job.CurrentIndex so queued jobs render as "0/N".MCPForUnity/Editor/Tools/CommandQueue.cs-144-146 (1)
144-146:⚠️ Potential issue | 🟡 MinorDone-job progress is off by one in summary output.
Completed jobs currently report progress from
CurrentIndexdirectly, which yieldsN-1/Nafter a normal full run.Proposed fix
if (j.Commands != null && j.Commands.Count > 0) - entry["p"] = $"{j.CurrentIndex + (j.Status == JobStatus.Done ? 0 : 1)}/{j.Commands.Count}"; +{ + var completed = j.Status == JobStatus.Done + ? j.Commands.Count + : Math.Min(j.CurrentIndex + 1, j.Commands.Count); + entry["p"] = $"{completed}/{j.Commands.Count}"; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/CommandQueue.cs` around lines 144 - 146, The progress string in CommandQueue is off by one for completed jobs because it uses j.CurrentIndex + (j.Status == JobStatus.Done ? 0 : 1); change the assignment that sets entry["p"] so it always adds 1 to j.CurrentIndex (e.g. $"{j.CurrentIndex + 1}/{j.Commands.Count}") to ensure a finished job reports N/N; update the line inside the block that checks j.Commands to use j.CurrentIndex + 1 unconditionally.
🧹 Nitpick comments (10)
MCPForUnity/Editor/Tools/QueueStatus.cs (1)
14-18: Consider wrapping@paramswithToolParamsfor consistency with other tools.This tool doesn't require any input parameters, so there's nothing to validate. However, some tools in this directory use
ToolParamsto wrap the incomingJObject. Adopting the same pattern here — even if justvar tp = new ToolParams(@params);— keeps the tool surface consistent and makes it trivial to add optional parameters (e.g., filters) later.Per coding guidelines for
MCPForUnity/Editor/Tools/*.cs: "UseToolParamsfor consistent parameter validation in C# Editor tools with methods likeGetInt(),RequireString()".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/QueueStatus.cs` around lines 14 - 18, Wrap the incoming JObject in a ToolParams instance inside HandleCommand to match other tools' patterns: create a local variable like tp = new ToolParams(`@params`) at the start of the method and use tp (even if not used further) to preserve consistency and make future optional parameter handling trivial; update the HandleCommand method signature/body so it constructs ToolParams before calling CommandGatewayState.Queue.GetSummary() and returning the SuccessResponse.MCPForUnity/Editor/Tools/CommandClassifier.cs (2)
48-58:CausesDomainReloadreturnstruewhencompileparam is absent — worth documenting explicitly.
@params.Value<string>("compile")returnsnullwhen the key is missing, andnull != "none"evaluates totrue. This meansrefresh_unitywith nocompilekey is treated as domain-reload-causing, which is the correct conservative default — but it's a non-obvious null comparison. A short inline comment would prevent future readers from misreading it as a bug.♻️ Proposed comment
- "refresh_unity" => `@params.Value`<string>("compile") != "none", + // null (omitted) or any value other than "none" triggers a reload + "refresh_unity" => `@params.Value`<string>("compile") != "none",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/CommandClassifier.cs` around lines 48 - 58, The CausesDomainReload method treats a missing "compile" key as causing a domain reload because `@params.Value`<string>("compile") returns null and null != "none" is true; add a brief inline comment above the "refresh_unity" case (referencing CausesDomainReload and the `@params.Value`<string>("compile") expression) explaining that returning true on missing/unknown compile values is an intentional conservative default to avoid silently skipping reloads.
33-43:ClassifyBatchon an empty array returnsInstant— consider documenting this sentinel.Initialising
maxtoExecutionTier.Instantmeans a zero-command batch resolves toInstant. This is likely intentional (no commands = no restriction), but a brief comment guards against someone assuming a "neutral" default should beSmooth.♻️ Proposed comment
- var max = ExecutionTier.Instant; + // Start at the lowest tier; an empty batch resolves to Instant (no restriction). + var max = ExecutionTier.Instant;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/CommandClassifier.cs` around lines 33 - 43, ClassifyBatch currently initializes max to ExecutionTier.Instant so an empty commands array returns Instant; add a short comment above ClassifyBatch explaining that ExecutionTier.Instant is being used intentionally as the sentinel/default for "no commands" (and reference that ClassifyBatch calls Classify for each command), so future readers understand that an empty batch represents no restrictions rather than a neutral Smooth value.MCPForUnity/Editor/Tools/CommandGatewayState.cs (1)
37-41: Cache theProcessTickdelegate to avoid a per-frame allocation.
OnUpdateconstructs a newasynclambda every editor tick (potentially 100 + calls/second), causing continuous heap pressure. The lambda closes over no mutable instance state, so it can be lifted to a static field.♻️ Proposed fix
+ static readonly Func<string, JObject, Task<object>> _executeCommand = + (tool, `@params`) => CommandRegistry.InvokeCommandAsync(tool, `@params`); + static void OnUpdate() { - Queue.ProcessTick(async (tool, `@params`) => - await CommandRegistry.InvokeCommandAsync(tool, `@params`)); + Queue.ProcessTick(_executeCommand); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/CommandGatewayState.cs` around lines 37 - 41, OnUpdate currently creates a new async lambda each tick which allocates; pull that lambda out into a static readonly delegate and pass the cached delegate to Queue.ProcessTick instead. Create a static readonly Func<ToolType, ParamType, Task> (or matching delegate signature used by Queue.ProcessTick) field (name e.g. s_processTickHandler) that calls CommandRegistry.InvokeCommandAsync(tool, `@params`) and replace the inline async lambda in OnUpdate with that field to avoid per-frame allocations; keep the call to CommandRegistry.InvokeCommandAsync unchanged.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandClassifierTests.cs (1)
27-40: Consider adding coverage forcreate(manage_scene) andpause/stop(manage_editor).
ClassifyManageSceneshares theHeavyarm across"create" or "load" or "save", butcreatehas no dedicated test. LikewiseClassifyManageEditormaps"pause"and"stop"toHeavywith no test coverage. If the switch arms are ever edited, the missing cases won't be caught by the suite.Also applies to: 59-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandClassifierTests.cs` around lines 27 - 40, Add unit tests to cover the missing branches by invoking CommandClassifier.Classify with the same patterns used in existing tests: add a test Classify_ManageScene_Create_ReturnsHeavy that calls CommandClassifier.Classify("manage_scene", ExecutionTier.Smooth, new JObject { ["action"] = "create" }) and asserts ExecutionTier.Heavy, and add tests Classify_ManageEditor_Pause_ReturnsHeavy and Classify_ManageEditor_Stop_ReturnsHeavy that call CommandClassifier.Classify("manage_editor", ExecutionTier.Smooth, new JObject { ["action"] = "pause" }) and new JObject { ["action"] = "stop" } respectively and assert ExecutionTier.Heavy to ensure the "create", "pause", and "stop" switch arms are covered.MCPForUnity/Editor/Tools/CommandRegistry.cs (1)
198-207:GetToolTiersilently returnsSmoothfor unregistered commands — consider whether that's always correct.Callers (e.g.,
BatchExecutebuilding a command list beforeCommandRegistry.Initialize()has run) will silently receiveSmoothrather than an error. If this sentinel is intentional, a brief XML-doc note that "unregistered or not-yet-initialized tools default to Smooth" would prevent future confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/CommandRegistry.cs` around lines 198 - 207, GetToolTier currently returns ExecutionTier.Smooth when a command isn't found in _handlers, which can hide callers' initialization/order issues (e.g., BatchExecute calling GetToolTier before CommandRegistry.Initialize); update the XML doc summary for GetToolTier to explicitly state that unregistered or not-yet-initialized commands will default to ExecutionTier.Smooth so callers are aware of the sentinel behavior and can change their flow or call Initialize first.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
332-349: Add explicit warning when Queue section assets fail to load.The Queue load block has no
elsediagnostic, unlike Tools/Resources. A warning here makes empty Queue-tab failures much easier to diagnose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs` around lines 332 - 349, The Queue section load lacks diagnostic warnings when assets fail to load; update the block that references queueTree and queueStyleSheet (and the creation of McpQueueSection) to log explicit warnings when queueTree is null and when queueStyleSheet is null so failures are visible—use the same logging strategy as the Tools/Resources block (e.g., call Debug.LogWarning or an existing logger) to report "McpQueueSection.uxml not found" and "McpQueueSection.uss not found" and ensure you do not attempt to instantiate McpQueueSection or add a null stylesheet to rootVisualElement when the assets are missing.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs (1)
26-52: Strengthen response assertions to avoid false positives.These tests currently only check
Is.Not.Null, so contract regressions can slip through. Please assert expected response kind/status fields (e.g., error for missing/invalid ticket, structured status payload for queue status).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs` around lines 26 - 52, Update the tests to assert on the response contract instead of only non-null: for PollJob.HandleCommand in PollJob_NullParams_ReturnsError, PollJob_MissingTicket_ReturnsError, and PollJob_InvalidTicket_ReturnsNotFound assert the returned JObject contains an error/ kind/status field (e.g., result["kind"] == "error" or result["status"] == "error") and for PollJob_InvalidTicket also assert an appropriate not-found indicator (status code or message). For QueueStatus.HandleCommand in QueueStatus_ReturnsStatusObject assert the response is a structured status payload (e.g., contains expected keys like "queue", "length", or "items") and validate their types (JObject/JArray or numeric) to prevent false positives.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandQueueTests.cs (2)
79-89: Add a null/empty-agent cancellation authorization test.Given cancellation is ownership-sensitive, add a regression test that
Cancel(ticket, null)andCancel(ticket, "")are rejected.Proposed additional test
+ [Test] + public void Cancel_NullOrEmptyAgent_Fails() + { + var cmds = new List<BatchCommand> + { + new() { Tool = "refresh_unity", Params = new JObject(), Tier = ExecutionTier.Heavy } + }; + var job = _queue.Submit("agent-1", "heavy", false, cmds); + + Assert.That(_queue.Cancel(job.Ticket, null), Is.False); + Assert.That(_queue.Cancel(job.Ticket, ""), Is.False); + Assert.That(job.Status, Is.EqualTo(JobStatus.Queued)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandQueueTests.cs` around lines 79 - 89, Update the Cancel_WrongAgent_Fails test to also assert cancellation is rejected when the caller agent is null or empty: call _queue.Cancel(job.Ticket, null) and _queue.Cancel(job.Ticket, "") and assert both return false and that job.Status remains JobStatus.Queued; keep the existing wrong-agent assertion. Use the same job created in Cancel_WrongAgent_Fails so you validate authorization logic in _queue.Cancel for null and empty caller values.
203-204: Tighten “proceeds” assertions to avoid false positives.
Is.Not.EqualTo(JobStatus.Queued)can still pass if a job fails or is cancelled. These tests should assert expected terminal/running states explicitly (and preferablyErroris null).Suggested assertion pattern
- Assert.That(job.Status, Is.Not.EqualTo(JobStatus.Queued)); + Assert.That(job.Status, Is.EqualTo(JobStatus.Running).Or.EqualTo(JobStatus.Done)); + Assert.That(job.Error, Is.Null);Also applies to: 217-218, 237-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandQueueTests.cs` around lines 203 - 204, Replace the loose negative assertion "Assert.That(job.Status, Is.Not.EqualTo(JobStatus.Queued))" with explicit positive assertions of the expected state (e.g., Assert.That(job.Status, Is.EqualTo(JobStatus.Running)) or Assert.That(job.Status, Is.EqualTo(JobStatus.Completed)) depending on the test) and add an assertion that the job has no error (e.g., Assert.IsNull(job.Error)). Update all occurrences in CommandQueueTests.cs where the loose check appears (the instance using the local variable job at the shown lines and the other two occurrences) so each test asserts the precise terminal or running state and verifies Error is null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MCPForUnity/Editor/Helpers/AssetPathUtility.cs`:
- Around line 106-112: GetMcpPackageRootPath currently returns
UIAssetSync.SyncedBasePath when UIAssetSync.NeedsSync() is true, which changes
the package root semantics and breaks downstream callers like GetPackageJson
that expect the real package root (so package.json can be found); instead,
preserve the true package root return value from GetMcpPackageRootPath and limit
the WSL workaround to UI-specific lookups only: revert the early return in
GetMcpPackageRootPath to always return the actual package root, and update any
code that loads UXML/USS (or GetPackageJson) to check UIAssetSync.NeedsSync()
and use UIAssetSync.SyncedBasePath only for UI asset resolution while keeping
GetPackageJson pointed at the real package root.
In `@MCPForUnity/Editor/Helpers/UIAssetSync.cs`:
- Around line 52-87: The static initializer UIAssetSync.UIAssetSync() currently
performs unguarded file I/O (File.ReadAllText, File.WriteAllText,
Directory.CreateDirectory) which can throw and break type initialization; wrap
the per-file operations inside a try/catch that catches IOException and general
Exception, log the error with Debug.LogError or Debug.LogException including the
relativePath and exception, skip that file and continue so the static ctor never
throws, and ensure the existing anyUpdated/AssetDatabase.Refresh logic remains
intact; keep the early returns from NeedsSync() and GetPackagePhysicalRoot()
unchanged.
In `@MCPForUnity/Editor/Tools/BatchExecute.cs`:
- Around line 255-273: The async batch loop currently skips malformed or
missing-tool entries and doesn't enforce disabled-tool policy; change it to
validate every command token and fail the entire batch if any invalid entry or a
disabled tool is found: for each token parsed in the loop (the block using
NormalizeParameterKeys, CommandRegistry.GetToolTier, CommandClassifier.Classify,
CommandClassifier.CausesDomainReload and creating new BatchCommand) detect
missing/empty toolName or non-JObject params and collect/return an ErrorResponse
listing the invalid entries instead of continuing, and check the
tool-enabled/disabled status the same way the sync path does (use the same
CommandRegistry/CommandClassifier call used in the sync path) and return an
ErrorResponse if any tool is disabled before adding commands to the commands
list.
- Around line 280-304: The current loop in BatchExecute (using
CommandRegistry.InvokeCommandAsync and collecting job.Results) unconditionally
sets job.Status = JobStatus.Done and returns a SuccessResponse even if some
commands produced ErrorResponse; after the foreach, inspect job.Results for any
ErrorResponse (or exceptions collected), and if any exist set job.Status =
JobStatus.Failed, populate job.Error/CompletedAt and return an ErrorResponse
(including ticket and results); only set Done and return SuccessResponse when no
ErrorResponse entries are present. Ensure you reference the existing symbols:
CommandRegistry.InvokeCommandAsync, ErrorResponse, SuccessResponse, job.Results,
JobStatus, atomic, and cmd.Tool when building the error payload.
- Around line 284-286: The blocking call in HandleAsyncSubmit uses
.ConfigureAwait(true).GetAwaiter().GetResult() which can deadlock the Unity
editor; change HandleAsyncSubmit to be async (e.g., Task or Task<TResult>
return) and replace that blocking call with await
CommandRegistry.InvokeCommandAsync(cmd.Tool, cmd.Params) so the call yields to
the Unity synchronization context; update any callers of HandleAsyncSubmit to
await the new async signature (or otherwise handle the returned Task), and keep
job.Results.Add(result) after the awaited call.
In `@MCPForUnity/Editor/Tools/CommandQueue.cs`:
- Around line 117-123: The Remove method currently clears _activeHeavyTicket
directly which bypasses the busy-hold and cooldown logic in ProcessTick; to fix
this, stop clearing _activeHeavyTicket inside Remove (leave
_smoothInFlight.Remove(ticket) and return _store.Remove(ticket) only) and let
ProcessTick handle clearing the active heavy slot and enforcing
cooldown/busy-hold, or if immediate clearing is absolutely required add a guard
that only clears _activeHeavyTicket when not in the editor-busy state/cooldown
(use the same busy/cooldown checks used by ProcessTick) so the
busy-hold/cooldown semantics remain consistent.
- Around line 61-66: In the Cancel method, the current check lets agent==null
bypass owner checks; change the authorization so a null agent cannot cancel
others: after retrieving job via _store.GetJob(ticket) and confirming job.Status
== JobStatus.Queued, require agent to be non-null and equal to job.Agent before
proceeding (i.e., return false if agent is null or agent != job.Agent). Update
the conditional that currently reads "if (job.Agent != agent && agent != null)
return false;" to enforce this stricter check using Cancel, job.Agent, and
JobStatus.
- Around line 199-208: RestoreFromJson currently repopulates _heavyQueue from
_store but doesn't clear transient runtime state first, causing duplicates on
repeated restores; before calling _store.FromJson or before re-enqueuing jobs in
RestoreFromJson, clear/reset the runtime-only fields: call _heavyQueue.Clear()
(or recreate it), clear the _smoothInFlight collection (e.g.,
_smoothInFlight.Clear() or new empty instance), and reset _activeHeavyTicket to
its default/null value, then proceed to call _store.FromJson and re-enqueue
tickets from _store.GetQueuedJobs() as you already do.
- Around line 37-45: Submit currently assigns the caller-owned list to
job.Commands which allows external mutation; instead make a defensive copy in
Submit by creating a new List<BatchCommand> and deep-cloning each element before
assigning to job.Commands (e.g., construct new BatchCommand instances copying
primitive fields and using JToken/JObject.DeepClone() or a BatchCommand.Clone
method for Params), so references to the original list or JObjects cannot modify
the queued job; update Submit in CommandQueue and add/use a Clone method on
BatchCommand if needed.
In `@MCPForUnity/Editor/Tools/GatewayJobLogger.cs`:
- Around line 82-87: The GatewayJobLogger.Log() call performs a synchronous
File.AppendAllText using LogPath on the Unity editor main thread (invoked from
PollJob.HandleCommand), which can block; change Log to perform non-blocking
background writes (e.g., move the file write and directory-creation logic into
Task.Run or an internal background writer/queue) so the main thread only
enqueues the log entry and returns immediately; ensure you reference and use
LogPath, the existing entry.ToString(Formatting.None) payload, and preserve
directory creation semantics (create dir inside the background task or use a
thread-safe check) and handle exceptions from the background task so failures
are logged without throwing on the main thread.
---
Outside diff comments:
In `@MCPForUnity/Editor/Tools/BatchExecute.cs`:
- Around line 35-66: The top-level parameter parsing in HandleCommand is using
JObject.Value/ indexing directly; replace this with a ToolParams instance (e.g.
new ToolParams(`@params`)) and call its helpers (GetInt(), GetBool() / GetString()
/ RequireString() as appropriate) to read "async", "failFast", "parallel",
"maxParallelism" and the "commands" array so validation is consistent with other
editor tools; specifically, change the commandsToken extraction to use
ToolParams to require/validate the "commands" entry and replace usages of
`@params.Value`<int?>("maxParallelism") and `@params.Value`<bool?>("async") etc.
with the matching ToolParams methods while preserving the async vs legacy flow
inside HandleCommand and when calling HandleAsyncSubmit.
---
Minor comments:
In `@docs/plans/2026-02-25-gateway-async-awareness-design.md`:
- Around line 27-31: The fenced code block containing the conditional snippet
with symbols job.CausesDomainReload, TestJobManager.HasRunningJob, and
EditorApplication.isCompiling is missing a language specifier; update the
opening fence to include a language token (for example "text") so the block
becomes ```text ... ``` to satisfy markdownlint while preserving the exact
contents of the snippet.
In `@docs/plans/2026-02-25-gateway-async-awareness-plan.md`:
- Around line 90-94: The markdown contains unlabeled fenced code blocks (e.g.,
the run_tests(...) snippet and other blocks referenced at lines 90, 121, 886,
893, 904) causing MD040; update each triple-backtick fence to include an
appropriate language identifier (for the run_tests snippet use text or bash, and
for other blocks choose csharp/json/text as appropriate) so every code fence is
labeled (e.g., ```text or ```csharp) while keeping the block contents unchanged.
- Line 13: Heading level jumps from H1 to H3 for the section titled "Task 1: Add
`CausesDomainReload` to CommandClassifier"; change the heading token from "###
Task 1: Add `CausesDomainReload` to CommandClassifier" to "## Task 1: Add
`CausesDomainReload` to CommandClassifier" so the document uses a proper H2 and
satisfies markdown lint MD001.
In `@docs/plans/2026-02-25-queue-summary-ui-design.md`:
- Line 13: Add explicit language specifiers to the fenced code blocks containing
ASCII-art/pseudo-code (the two triple-backtick blocks flagged by MD040) by
changing their opening fences from ``` to a tagged form like ```text (or
```ascii) so the linter recognizes them; update both code fences so each opening
triple-backtick includes the chosen language tag.
In `@docs/plans/2026-02-25-queue-summary-ui-plan.md`:
- Around line 35-38: The Markdown fenced code blocks like the one containing
"refresh_unity(scope=all, compile=request, wait_for_ready=true)" and the
"read_console(types=[\"error\"]) → 0 errors" output are unlabeled and trigger
MD040; edit each such fenced block (including the other occurrences mentioned)
to add an explicit language identifier (e.g., ```bash, ```text or ```console)
after the opening backticks so the block is annotated and the linter no longer
flags MD040.
- Line 15: The markdown heading "Task 1: Expose GetAllJobs on CommandQueue" is
currently H3 (###) causing a jump from the document H1; change this and all peer
task headings ("Task 1", "Task 2", etc.) from H3 to H2 (##) so the section
hierarchy is H1 -> H2 -> H3 and MD001 is satisfied; update the lines with the
exact heading texts to use "##" instead of "###".
In `@MCPForUnity/Editor/Helpers/UIAssetSync.cs`:
- Around line 71-72: The code currently computes destPath with
Path.GetFullPath(Path.Combine(SyncedBasePath, relativePath)) which uses the
process CWD; change the construction to anchor against the Unity project root by
using Path.GetDirectoryName(Application.dataPath) as the base before combining
with SyncedBasePath and relativePath so destPath and destDir are deterministic.
Update the logic that sets destPath and destDir (the variables destPath, destDir
and the expression using SyncedBasePath/relativePath) to first compute
projectRoot = Path.GetDirectoryName(Application.dataPath) and then combine
projectRoot with SyncedBasePath and relativePath to produce the final paths.
In `@MCPForUnity/Editor/Tools/CommandQueue.cs`:
- Around line 144-146: The progress string in CommandQueue is off by one for
completed jobs because it uses j.CurrentIndex + (j.Status == JobStatus.Done ? 0
: 1); change the assignment that sets entry["p"] so it always adds 1 to
j.CurrentIndex (e.g. $"{j.CurrentIndex + 1}/{j.Commands.Count}") to ensure a
finished job reports N/N; update the line inside the block that checks
j.Commands to use j.CurrentIndex + 1 unconditionally.
In `@MCPForUnity/Editor/Tools/PollJob.cs`:
- Around line 57-68: Guard against job.Commands being null in the
JobStatus.Running branch: before using job.Commands.Count or constructing the
progress string, check for null (or use a safe fallback) and compute safeCount =
job.Commands?.Count ?? 0 and safeIndex = Math.Min(job.CurrentIndex + 1,
safeCount); then build the PendingResponse using safeCount and safeIndex for the
message and progress fields (update the Running case that creates the
PendingResponse to reference these safe values rather than job.Commands.Count
directly).
- Around line 18-21: The code is using p.GetRequired("ticket") instead of the
ToolParams guideline method; replace the call with p.RequireString("ticket") (or
the equivalent RequireString method on ToolParams) and update handling to use
the returned result (e.g., check success and use its Value or return new
ErrorResponse(result.ErrorMessage)) so the Ticket extraction uses
ToolParams.RequireString(), referenced variable p, and the existing
ErrorResponse/ticketResult flow.
In `@MCPForUnity/Editor/Tools/TicketStore.cs`:
- Around line 39-45: The CleanExpired logic in TicketStore.CleanExpired
currently filters terminal jobs by JobStatus.Done or JobStatus.Failed and thus
misses cancelled terminal jobs; update the expiry predicate used when building
expired (the LINQ Where over _jobs entries) to also include kvp.Value.Status ==
JobStatus.Cancelled so cancelled jobs with CompletedAt older than expiry are
selected and removed from _jobs; ensure the selection still checks
CompletedAt.HasValue and the time-based comparison remains unchanged.
In `@MCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.cs`:
- Around line 216-221: The progress calculation currently uses job.CurrentIndex
+ 1 for non-done jobs which makes queued jobs show "1/N"; change the logic in
McpQueueSection (the block that builds progressText) to use job.CurrentIndex for
non-Done jobs and leave Done as job.Commands.Count, i.e. set idx = job.Status ==
JobStatus.Done ? job.Commands.Count : job.CurrentIndex so queued jobs render as
"0/N".
In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/TicketStoreTests.cs`:
- Around line 113-121: The test GetQueuedJobs_OrderedByCreationTime is flaky
because CreateJob may assign identical CreatedAt timestamps; update the test to
avoid relying on millisecond ordering by either explicitly setting distinct
CreatedAt values on j1 and j2 after creation (e.g., j1.CreatedAt = now;
j2.CreatedAt = now.AddMilliseconds(1)) before calling _store.GetQueuedJobs(), or
change the assertions to check that both tickets are present without requiring
specific positions (e.g., Assert.That(queued.Select(q => q.Ticket),
Is.EquivalentTo(new[] { j1.Ticket, j2.Ticket }))); modify only the test logic
around CreateJob, j1/j2, and the assertions so GetQueuedJobs and _store remain
unchanged.
---
Nitpick comments:
In `@MCPForUnity/Editor/Tools/CommandClassifier.cs`:
- Around line 48-58: The CausesDomainReload method treats a missing "compile"
key as causing a domain reload because `@params.Value`<string>("compile") returns
null and null != "none" is true; add a brief inline comment above the
"refresh_unity" case (referencing CausesDomainReload and the
`@params.Value`<string>("compile") expression) explaining that returning true on
missing/unknown compile values is an intentional conservative default to avoid
silently skipping reloads.
- Around line 33-43: ClassifyBatch currently initializes max to
ExecutionTier.Instant so an empty commands array returns Instant; add a short
comment above ClassifyBatch explaining that ExecutionTier.Instant is being used
intentionally as the sentinel/default for "no commands" (and reference that
ClassifyBatch calls Classify for each command), so future readers understand
that an empty batch represents no restrictions rather than a neutral Smooth
value.
In `@MCPForUnity/Editor/Tools/CommandGatewayState.cs`:
- Around line 37-41: OnUpdate currently creates a new async lambda each tick
which allocates; pull that lambda out into a static readonly delegate and pass
the cached delegate to Queue.ProcessTick instead. Create a static readonly
Func<ToolType, ParamType, Task> (or matching delegate signature used by
Queue.ProcessTick) field (name e.g. s_processTickHandler) that calls
CommandRegistry.InvokeCommandAsync(tool, `@params`) and replace the inline async
lambda in OnUpdate with that field to avoid per-frame allocations; keep the call
to CommandRegistry.InvokeCommandAsync unchanged.
In `@MCPForUnity/Editor/Tools/CommandRegistry.cs`:
- Around line 198-207: GetToolTier currently returns ExecutionTier.Smooth when a
command isn't found in _handlers, which can hide callers' initialization/order
issues (e.g., BatchExecute calling GetToolTier before
CommandRegistry.Initialize); update the XML doc summary for GetToolTier to
explicitly state that unregistered or not-yet-initialized commands will default
to ExecutionTier.Smooth so callers are aware of the sentinel behavior and can
change their flow or call Initialize first.
In `@MCPForUnity/Editor/Tools/QueueStatus.cs`:
- Around line 14-18: Wrap the incoming JObject in a ToolParams instance inside
HandleCommand to match other tools' patterns: create a local variable like tp =
new ToolParams(`@params`) at the start of the method and use tp (even if not used
further) to preserve consistency and make future optional parameter handling
trivial; update the HandleCommand method signature/body so it constructs
ToolParams before calling CommandGatewayState.Queue.GetSummary() and returning
the SuccessResponse.
In `@MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs`:
- Around line 332-349: The Queue section load lacks diagnostic warnings when
assets fail to load; update the block that references queueTree and
queueStyleSheet (and the creation of McpQueueSection) to log explicit warnings
when queueTree is null and when queueStyleSheet is null so failures are
visible—use the same logging strategy as the Tools/Resources block (e.g., call
Debug.LogWarning or an existing logger) to report "McpQueueSection.uxml not
found" and "McpQueueSection.uss not found" and ensure you do not attempt to
instantiate McpQueueSection or add a null stylesheet to rootVisualElement when
the assets are missing.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.cs`:
- Around line 26-52: Update the tests to assert on the response contract instead
of only non-null: for PollJob.HandleCommand in PollJob_NullParams_ReturnsError,
PollJob_MissingTicket_ReturnsError, and PollJob_InvalidTicket_ReturnsNotFound
assert the returned JObject contains an error/ kind/status field (e.g.,
result["kind"] == "error" or result["status"] == "error") and for
PollJob_InvalidTicket also assert an appropriate not-found indicator (status
code or message). For QueueStatus.HandleCommand in
QueueStatus_ReturnsStatusObject assert the response is a structured status
payload (e.g., contains expected keys like "queue", "length", or "items") and
validate their types (JObject/JArray or numeric) to prevent false positives.
In
`@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandClassifierTests.cs`:
- Around line 27-40: Add unit tests to cover the missing branches by invoking
CommandClassifier.Classify with the same patterns used in existing tests: add a
test Classify_ManageScene_Create_ReturnsHeavy that calls
CommandClassifier.Classify("manage_scene", ExecutionTier.Smooth, new JObject {
["action"] = "create" }) and asserts ExecutionTier.Heavy, and add tests
Classify_ManageEditor_Pause_ReturnsHeavy and
Classify_ManageEditor_Stop_ReturnsHeavy that call
CommandClassifier.Classify("manage_editor", ExecutionTier.Smooth, new JObject {
["action"] = "pause" }) and new JObject { ["action"] = "stop" } respectively and
assert ExecutionTier.Heavy to ensure the "create", "pause", and "stop" switch
arms are covered.
In `@TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandQueueTests.cs`:
- Around line 79-89: Update the Cancel_WrongAgent_Fails test to also assert
cancellation is rejected when the caller agent is null or empty: call
_queue.Cancel(job.Ticket, null) and _queue.Cancel(job.Ticket, "") and assert
both return false and that job.Status remains JobStatus.Queued; keep the
existing wrong-agent assertion. Use the same job created in
Cancel_WrongAgent_Fails so you validate authorization logic in _queue.Cancel for
null and empty caller values.
- Around line 203-204: Replace the loose negative assertion
"Assert.That(job.Status, Is.Not.EqualTo(JobStatus.Queued))" with explicit
positive assertions of the expected state (e.g., Assert.That(job.Status,
Is.EqualTo(JobStatus.Running)) or Assert.That(job.Status,
Is.EqualTo(JobStatus.Completed)) depending on the test) and add an assertion
that the job has no error (e.g., Assert.IsNull(job.Error)). Update all
occurrences in CommandQueueTests.cs where the loose check appears (the instance
using the local variable job at the shown lines and the other two occurrences)
so each test asserts the precise terminal or running state and verifies Error is
null.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
MCPForUnity/Editor/Constants/EditorPrefKeys.csMCPForUnity/Editor/Helpers/AssetPathUtility.csMCPForUnity/Editor/Helpers/UIAssetSync.csMCPForUnity/Editor/Helpers/UIAssetSync.cs.metaMCPForUnity/Editor/Tools/BatchExecute.csMCPForUnity/Editor/Tools/BatchJob.csMCPForUnity/Editor/Tools/BatchJob.cs.metaMCPForUnity/Editor/Tools/CommandClassifier.csMCPForUnity/Editor/Tools/CommandClassifier.cs.metaMCPForUnity/Editor/Tools/CommandGatewayState.csMCPForUnity/Editor/Tools/CommandGatewayState.cs.metaMCPForUnity/Editor/Tools/CommandQueue.csMCPForUnity/Editor/Tools/CommandQueue.cs.metaMCPForUnity/Editor/Tools/CommandRegistry.csMCPForUnity/Editor/Tools/ExecutionTier.csMCPForUnity/Editor/Tools/ExecutionTier.cs.metaMCPForUnity/Editor/Tools/FindGameObjects.csMCPForUnity/Editor/Tools/GatewayJobLogger.csMCPForUnity/Editor/Tools/GatewayJobLogger.cs.metaMCPForUnity/Editor/Tools/GetTestJob.csMCPForUnity/Editor/Tools/ManageScript.csMCPForUnity/Editor/Tools/ManageShader.csMCPForUnity/Editor/Tools/McpForUnityToolAttribute.csMCPForUnity/Editor/Tools/PollJob.csMCPForUnity/Editor/Tools/PollJob.cs.metaMCPForUnity/Editor/Tools/QueueStatus.csMCPForUnity/Editor/Tools/QueueStatus.cs.metaMCPForUnity/Editor/Tools/ReadConsole.csMCPForUnity/Editor/Tools/RefreshUnity.csMCPForUnity/Editor/Tools/RunTests.csMCPForUnity/Editor/Tools/TicketStore.csMCPForUnity/Editor/Tools/TicketStore.cs.metaMCPForUnity/Editor/Windows/Components/Queue.metaMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.csMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.cs.metaMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.ussMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.uss.metaMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.uxmlMCPForUnity/Editor/Windows/Components/Queue/McpQueueSection.uxml.metaMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxmlTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteAsyncTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandClassifierTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandQueueTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ExecutionTierTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/TicketStoreTests.csdocs/plans/2026-02-25-gateway-async-awareness-design.mddocs/plans/2026-02-25-gateway-async-awareness-plan.mddocs/plans/2026-02-25-queue-summary-ui-design.mddocs/plans/2026-02-25-queue-summary-ui-plan.md
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 50 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Author
|
@copilot open a new pull request to apply changes based on the comments in in the thread in general |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/plans/2026-02-25-queue-summary-ui-design.md (2)
53-60: Add a language specifier to the fenced code block.The data-flow block has no language tag, triggering MD040. Use
textorplaintext.✏️ Proposed fix
-``` +```text CommandGatewayState.Queue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-queue-summary-ui-design.md` around lines 53 - 60, The fenced code block containing the data-flow diagram (starting with CommandGatewayState.Queue and lines referencing TicketStore.GetAllJobs, Queue.HasActiveHeavy, Queue.QueueDepth, Queue.SmoothInFlight, Queue.IsEditorBusy) is missing a language specifier, which triggers MD040; fix it by adding a language tag (e.g., text or plaintext) to the opening fence so the block reads ```text followed by the existing lines and the closing ``` to satisfy the linter.
13-30: Add a language specifier to the fenced code block.The ASCII art layout block has no language tag, triggering MD040. Use
textorplaintextto silence the warning.✏️ Proposed fix
-``` +```text ┌───────────────────────────────────────────────┐🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-25-queue-summary-ui-design.md` around lines 13 - 30, The fenced ASCII-art code block is missing a language specifier, triggering MD040; update the opening triple-backticks for the ASCII diagram (the block starting with the box diagram/Status Bar and Job List) to include a language tag like ```text or ```plaintext so the linter accepts it and the diagram remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-02-25-queue-summary-ui-design.md`:
- Line 19: The ASCII layout at the given diagram is missing the "Failed" counter
declared in the spec (line 36); update the diagram to include all four counters
— "Queued", "Running", "Done", and "Failed" — e.g., change the line showing
"Queued: 2 │ Running: 1 │ Done: 5" to include "│ Failed: 0" (or the appropriate
example value) and ensure separators/spacing match the existing style, or if the
design intentionally omits failures, instead update the component spec to remove
"Failed" so both the spec and the ASCII layout are consistent.
---
Nitpick comments:
In `@docs/plans/2026-02-25-queue-summary-ui-design.md`:
- Around line 53-60: The fenced code block containing the data-flow diagram
(starting with CommandGatewayState.Queue and lines referencing
TicketStore.GetAllJobs, Queue.HasActiveHeavy, Queue.QueueDepth,
Queue.SmoothInFlight, Queue.IsEditorBusy) is missing a language specifier, which
triggers MD040; fix it by adding a language tag (e.g., text or plaintext) to the
opening fence so the block reads ```text followed by the existing lines and the
closing ``` to satisfy the linter.
- Around line 13-30: The fenced ASCII-art code block is missing a language
specifier, triggering MD040; update the opening triple-backticks for the ASCII
diagram (the block starting with the box diagram/Status Bar and Job List) to
include a language tag like ```text or ```plaintext so the linter accepts it and
the diagram remains unchanged.
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
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.
Summary
batch_executewith anasync=truepath that queues commands and returns tickets for pollingMotivation
When multiple AI agents interact with Unity simultaneously (e.g., parallel coding agents via Claude Code), they compete for the single-threaded editor. Without coordination, domain reloads from one agent break another's in-flight operations, causing cascading failures. This gateway provides safe concurrent access by:
Architecture
Execution Tiers
find_gameobjects,read_console,get_test_jobmanage_gameobject,manage_materialmanage_script.create,refresh_unity,run_testsAction-Level Overrides
The
CommandClassifierdowngrades tool tiers based on specific actions. For example:manage_sceneis Heavy (can create scenes), butmanage_scene.get_hierarchyis Instantmanage_assetis Smooth (can modify assets), butmanage_asset.searchis InstantQueue Scheduling
Domain Reload Safety
CausesDomainReloadflag propagated from classifier through commands and jobsSessionStatebefore assembly reload, restored afterDone(the reload IS the success signal); other interrupted jobs are markedFailed: "Interrupted by domain reload"Job Lifecycle & Auto-Cleanup
Terminal jobs (Done, Failed, Cancelled) are removed from the store when polled.
If gateway job logging is enabled, job metadata is appended to a JSON-lines file before removal.
Uncollected jobs expire after 5 minutes as a safety net.
New Tools
poll_job— Poll a queued job by ticket. Returns status, progress, position in queue, blocked reason, and results when done. Auto-removes terminal jobs after returning data.queue_status— Compressed queue summary: per-job status, active heavy ticket, queue depth, smooth in-flight count.Queue Tab (Editor UI)
Real-time dashboard in the MCP For Unity editor window:
Files Changed (50 files, +4,721 lines)
New Production Code
ExecutionTier.cs— Enum +[ExecutionTier]attribute for tool registrationCommandClassifier.cs— Action-level tier overrides (tool + action -> effective tier)BatchJob.cs— Job model with status lifecycle, undo group tracking, results collectionTicketStore.cs— Job storage with ticket-based lookup, expiry, removal, and domain-reload-aware restoreCommandQueue.cs— Tier-aware FIFO dispatcher with exclusive heavy scheduling, domain-reload guards, editor-busy holdCommandGatewayState.cs—[InitializeOnLoad]singleton wiring queue toEditorApplication.updatewith SessionState persistencePollJob.cs— MCP tool for polling job status with auto-cleanup of terminal jobsQueueStatus.cs— MCP tool for compressed queue introspectionGatewayJobLogger.cs— JSON-lines audit logger with configurable path via EditorPrefsQueue Tab UI
McpQueueSection.uxml— Layout: status bar, logging box, job list header, dynamic rows, empty stateMcpQueueSection.uss— Styles: status dots, columns, blocked badge, logging box, light/dark themesMcpQueueSection.cs— Controller: refresh from queue state, logging toggle/path wiring, file browserUIAssetSync.cs— WSL UNC path workaround: copies UXML/USS to Assets/ on domain reloadAssetPathUtility.cs— Returns synced base path when WSL detectedModified Production Code
BatchExecute.cs— Addedasync=truegateway path alongside existing synchronous pathMcpForUnityToolAttribute.cs— Extended withExecutionTierpropertyCommandRegistry.cs— AddedGetToolTier()helperMCPForUnityEditorWindow.cs— Wired Queue as 6th tab with 1s independent refresh timerMCPForUnityEditorWindow.uxml— Added queue-tab toggle and queue-panel scroll viewEditorPrefKeys.cs— AddedGatewayJobLoggingandGatewayJobLogPathkeys[ExecutionTier(Heavy)]annotationsTests (5 test files, 47 tests)
ExecutionTierTests.cs— 5 tests: tier enum values, attribute defaults, attribute with tier parameterCommandClassifierTests.cs— 13 tests: action-level classification, override logic, null/missing params, default fallbackTicketStoreTests.cs— 12 tests: job creation, ticket lookup, expiry cleanup, agent stats, queue orderingCommandQueueTests.cs— 10 tests: submit, poll, cancel, queue depth, agent validation, heavy schedulingBatchExecuteAsyncTests.cs— 7 tests: gateway state singleton, PollJob null/missing/invalid params, QueueStatus response, BatchExecute constantsDocumentation
2026-02-25-gateway-async-awareness-design.md— Domain-reload safety design doc2026-02-25-gateway-async-awareness-plan.md— Implementation plan for async guards2026-02-25-queue-summary-ui-design.md— Queue tab UI design doc2026-02-25-queue-summary-ui-plan.md— Queue tab implementation planUsage
Async batch submission
{ "tool": "batch_execute", "params": { "async": true, "agent": "agent-1", "label": "Create player scripts", "atomic": true, "commands": [ {"tool": "manage_script", "params": {"action": "create", "name": "Player", "path": "Assets/Scripts"}}, {"tool": "manage_script", "params": {"action": "create", "name": "Enemy", "path": "Assets/Scripts"}} ] } }Poll for results
{"tool": "poll_job", "params": {"ticket": "t-000001"}}Returns full results on completion, then auto-removes the ticket from the queue.
Check queue status
{"tool": "queue_status", "params": {}}Job log output (when enabled)
{"ticket":"t-000010","agent":"agent-1","label":"run-tests","tier":"heavy","status":"done","created_at":"2026-02-25T11:09:14Z","completed_at":"2026-02-25T11:09:14Z","command_count":1,"tools":["run_tests"]}Test Plan
run_testsstays "running" during async test execution, Agent 2refresh_unityblocked withtests_runninguntil Agent 1 completes, then executesrefresh_unityjob correctly shows Done (not Failed) after the domain reload it triggeredSummary by Sourcery
Introduce a tier-aware command gateway with async batch execution, queue persistence, and an in-editor Queue tab for monitoring and logging multi-agent MCP jobs.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Tests
Documentation