Merged
Conversation
Contributor
Reviewer's GuideAdds an Auto-Start on Editor Load option that can automatically start the HTTP MCP bridge (including local server) when the Unity Editor opens, and plumbs a quiet mode into server startup to support non-interactive auto-start without dialogs. Updated class diagram for HTTP auto-start and server managementclassDiagram
class IServerManagementService {
<<interface>>
+bool StartLocalHttpServer(bool quiet)
+bool StopLocalHttpServer()
+bool IsLocalHttpServerReachable()
}
class ServerManagementService {
+bool StartLocalHttpServer(bool quiet)
+bool StopLocalHttpServer()
+bool IsLocalHttpServerReachable()
-string GetPlatformSpecificPathPrepend()
-bool TryGetLocalHttpServerCommandParts(out string exePath, out string args, out string displayCommand, out string error)
-void ClearLocalServerPidTracking()
-void DeletePidFile(string pidFilePath)
-void StoreLocalHttpServerHandshake(string pidFilePath, string instanceToken)
-System.Diagnostics.ProcessStartInfo CreateTerminalProcessStartInfo(string launchCommand)
}
IServerManagementService <|.. ServerManagementService
class McpAdvancedSection {
-VisualElement Root
-TextField gitUrlOverride
-Button browseGitUrlButton
-Button clearGitUrlButton
-Toggle autoStartOnLoadToggle
-Toggle debugLogsToggle
-Toggle logRecordToggle
-Toggle devModeForceRefreshToggle
-Button deployBackupButton
-Button deployRestoreButton
+void CacheUIElements()
+void InitializeUI()
+void RegisterCallbacks()
+void UpdatePathOverrides()
}
class EditorPrefKeys {
<<static>>
+const string AutoStartOnLoad
+const string ResumeHttpAfterReload
+const string DebugLogs
+const string GitUrlOverride
}
class StdioBridgeHost {
<<static>>
-Task StartStdioBridgeAsync()
-Task StopStdioBridgeAsync()
-bool ShouldAutoStartBridge()
}
class HttpAutoStartHandler {
<<static>>
+HttpAutoStartHandler()
-void OnEditorReady()
-Task AutoStartAsync()
-Task WaitForServerAndConnectAsync()
-Task ConnectBridgeAsync()
}
class EditorConfigurationCache {
<<singleton>>
+static EditorConfigurationCache Instance
+bool UseHttpTransport
}
class MCPServiceLocator {
<<static>>
+IServerManagementService Server
+ITransportManager TransportManager
+IBridge Bridge
}
class ITransportManager {
<<interface>>
+bool IsRunning(TransportMode mode)
}
class IBridge {
<<interface>>
+Task~bool~ StartAsync()
}
class HttpEndpointUtility {
<<static>>
+bool IsRemoteScope()
+bool IsHttpLocalUrlAllowedForLaunch(string baseUrl, out string policyError)
+string GetLocalBaseUrl()
}
class MCPForUnityEditorWindow {
<<static>>
+void RequestHealthVerification()
}
class TransportMode {
<<enum>>
Http
Stdio
}
MCPServiceLocator --> IServerManagementService : Server
MCPServiceLocator --> ITransportManager : TransportManager
MCPServiceLocator --> IBridge : Bridge
HttpAutoStartHandler --> EditorPrefKeys : reads
HttpAutoStartHandler --> EditorConfigurationCache : uses
HttpAutoStartHandler --> MCPServiceLocator : uses
HttpAutoStartHandler --> HttpEndpointUtility : uses
HttpAutoStartHandler --> MCPForUnityEditorWindow : calls
StdioBridgeHost --> EditorPrefKeys : reads
StdioBridgeHost --> EditorConfigurationCache : uses
McpAdvancedSection --> EditorPrefKeys : reads/writes AutoStartOnLoad
McpAdvancedSection --> EditorPrefs : uses
class EditorPrefs {
<<static>>
+bool GetBool(string key, bool defaultValue)
+void SetBool(string key, bool value)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds an editor preference and UI toggle to control automatic HTTP MCP bridge startup on Unity editor load, introduces an HttpAutoStartHandler to perform conditional, retried startup/connect logic, and updates server management to support quiet (non‑interactive) server starts. Changes
Sequence DiagramsequenceDiagram
participant Editor as Unity Editor
participant Handler as HttpAutoStartHandler
participant ServerMgmt as ServerManagementService
participant HttpServer as Local HTTP Server
participant Bridge as MCP Bridge
Editor->>Handler: InitializeOnLoad
Handler->>Handler: Check batch mode & EditorPref AutoStartOnLoad
Handler->>Editor: Schedule OnEditorReady (delayCall)
Editor->>Handler: OnEditorReady
Handler->>Handler: Validate transport & running state
alt Auto-start enabled
Handler->>Handler: Determine local vs remote
alt Local
Handler->>ServerMgmt: StartLocalHttpServer(quiet=true)
ServerMgmt->>HttpServer: Spawn/start process
loop Retry (up to ~30 attempts)
Handler->>HttpServer: Check reachability
HttpServer-->>Handler: Respond / Timeout
end
Handler->>Bridge: ConnectBridgeAsync -> Bridge establishes connection
else Remote
Handler->>Bridge: ConnectBridgeAsync (remote URL)
end
Handler->>Bridge: Trigger health verification in UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an “Auto-Start on Editor Load” option so the MCP bridge (and for HTTP local, the server process) can be started automatically when the Unity Editor opens, addressing issue #822.
Changes:
- Add an Advanced Settings toggle backed by a new
EditorPrefskey (AutoStartOnLoad). - Introduce an
[InitializeOnLoad]handler to auto-start/connect HTTP transport on editor load (including launching the local HTTP server quietly). - Extend
StartLocalHttpServerwith aquietmode to suppress dialogs during auto-start flows.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.uxml | Adds the new auto-start toggle to the Advanced Settings UI. |
| MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs | Wires the toggle to EditorPrefs and refreshes it with other settings. |
| MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs | Makes stdio auto-start conditional on the new preference (when stdio is selected). |
| MCPForUnity/Editor/Services/ServerManagementService.cs | Adds quiet parameter to suppress dialogs for auto-start scenarios. |
| MCPForUnity/Editor/Services/IServerManagementService.cs | Updates the interface contract for StartLocalHttpServer(bool quiet = false). |
| MCPForUnity/Editor/Services/HttpAutoStartHandler.cs | New editor-load handler to auto-start HTTP (launch local server + connect bridge). |
| MCPForUnity/Editor/Services/HttpAutoStartHandler.cs.meta | Adds Unity meta for the new script asset. |
| MCPForUnity/Editor/Constants/EditorPrefKeys.cs | Adds the new EditorPrefs key constant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+28
to
+35
| // Only check lightweight EditorPrefs here — services like EditorConfigurationCache | ||
| // and MCPServiceLocator may not be initialized yet on fresh editor launch. | ||
| bool autoStartEnabled = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, true); | ||
| if (!autoStartEnabled) return; | ||
|
|
||
| // Delay to let the editor and services finish initialization. | ||
| EditorApplication.delayCall += OnEditorReady; | ||
| } |
There was a problem hiding this comment.
The auto-start preference is described as user “opt-in”, but this reads EditorPrefKeys.AutoStartOnLoad with a default of true, which opts everyone in (and will auto-launch/connect on first install when the key doesn’t exist). Consider defaulting this pref to false, or using EditorPrefs.HasKey so only explicitly-set users are enabled (while optionally preserving legacy stdio auto-start behavior).
Comment on lines
+117
to
+121
| // Abort if user changed settings while we were waiting. | ||
| if (!EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, true)) return; | ||
| if (!EditorConfigurationCache.Instance.UseHttpTransport) return; | ||
| if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) return; | ||
|
|
There was a problem hiding this comment.
This loop says it aborts if the user changed settings while waiting, but it only checks UseHttpTransport. If the user switches HTTP scope from local→remote (or changes the local URL) while waiting, the code can keep waiting on the local server and then call Bridge.StartAsync() against the new scope. Consider also aborting when HttpEndpointUtility.IsRemoteScope() becomes true (and/or when the local base URL changes).
| var autoStartLabel = autoStartOnLoadToggle.parent?.Q<Label>(); | ||
| if (autoStartLabel != null) | ||
| autoStartLabel.tooltip = autoStartOnLoadToggle.tooltip; | ||
| autoStartOnLoadToggle.SetValueWithoutNotify(EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, true)); |
There was a problem hiding this comment.
This initializes the toggle from EditorPrefKeys.AutoStartOnLoad with a default of true, which makes the new feature enabled by default even when the pref hasn’t been set. If the intent is a user-controlled opt-in, initialize with a default of false (or use EditorPrefs.HasKey to distinguish unset vs explicitly enabled).
Suggested change
| autoStartOnLoadToggle.SetValueWithoutNotify(EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, true)); | |
| bool autoStartOnLoad = false; | |
| if (EditorPrefs.HasKey(EditorPrefKeys.AutoStartOnLoad)) | |
| { | |
| autoStartOnLoad = EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad); | |
| } | |
| autoStartOnLoadToggle.SetValueWithoutNotify(autoStartOnLoad); |
Comment on lines
+243
to
+249
| if (!quiet) | ||
| { | ||
| EditorUtility.DisplayDialog( | ||
| "Cannot Start HTTP Server", | ||
| error ?? "The server command could not be constructed with the current settings.", | ||
| "OK"); | ||
| } |
There was a problem hiding this comment.
When quiet=true, this returns false without logging error if the command can’t be constructed. For auto-start, that makes failures hard to diagnose. Consider logging a warning/error with the reason even when dialogs are suppressed.
Suggested change
| if (!quiet) | |
| { | |
| EditorUtility.DisplayDialog( | |
| "Cannot Start HTTP Server", | |
| error ?? "The server command could not be constructed with the current settings.", | |
| "OK"); | |
| } | |
| var message = error ?? "The server command could not be constructed with the current settings."; | |
| if (!quiet) | |
| { | |
| EditorUtility.DisplayDialog( | |
| "Cannot Start HTTP Server", | |
| message, | |
| "OK"); | |
| } | |
| else | |
| { | |
| Debug.LogError($"Cannot Start HTTP Server: {message}"); | |
| } |
| $"{string.Join(", ", remaining)}\n\n" + | ||
| "MCP For Unity will not terminate unrelated processes. Stop the owning process manually or change the HTTP URL.", | ||
| "OK"); | ||
| } |
There was a problem hiding this comment.
When quiet=true, a port-in-use failure returns false without any log output. Auto-start will then fail silently aside from a generic message. Consider logging the port and remaining PIDs (or at least that the port is in use) when dialogs are suppressed.
Suggested change
| } | |
| } | |
| else | |
| { | |
| Debug.LogWarning( | |
| $"[MCP For Unity] Cannot start the local HTTP server because port {uri.Port} is already in use by PID(s): " + | |
| $"{string.Join(\", \", remaining)}. MCP For Unity will not terminate unrelated processes. " + | |
| "Stop the owning process manually or change the HTTP URL."); | |
| } |
| @@ -0,0 +1,2 @@ | |||
| fileFormatVersion: 2 | |||
| guid: 3d8f1790992fe0742938d8a879056ee6 No newline at end of file | |||
There was a problem hiding this comment.
This new .meta file only contains fileFormatVersion and guid, but other script .meta files in the repo include the full MonoImporter block. Please regenerate this meta via Unity (or include the full MonoImporter section) to match project conventions and avoid Unity re-serializing it differently on import.
Suggested change
| guid: 3d8f1790992fe0742938d8a879056ee6 | |
| guid: 3d8f1790992fe0742938d8a879056ee6 | |
| MonoImporter: | |
| externalObjects: {} | |
| serializedVersion: 2 | |
| defaultReferences: [] | |
| executionOrder: 0 | |
| icon: {instanceID: 0} | |
| userData: | |
| assetBundleName: | |
| assetBundleVariant: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/HttpAutoStartHandler.cs (1)
37-60: Potential race withHttpBridgeReloadHandlerduring domain reloads.The check on line 48 guards against double-start by checking
ResumeHttpAfterReload. However,HttpBridgeReloadHandler.OnAfterAssemblyReloaddeletes this flag before actually resuming the transport. IfdelayCallordering varies, there's a narrow window where both handlers could attempt to start.The
IsRunningcheck on line 52 provides a second layer of protection, but sinceIsRunningis a non-atomic snapshot of connection state (seeTransportManager.cs:119), a TOCTOU race remains theoretically possible.In practice, Unity's
delayCallqueue should process in order after allafterAssemblyReloadhandlers complete, making this unlikely. Consider this a minor edge-case awareness rather than an immediate fix requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs` around lines 37 - 60, The OnEditorReady auto-start has a TOCTOU race with HttpBridgeReloadHandler because it checks EditorPrefKeys.ResumeHttpAfterReload and TransportManager.IsRunning separately; fix by introducing an atomic start API on the transport manager (e.g., add TransportManager.TryStartHttpTransport or TryStartTransport(TransportMode) that performs the "is running" check and starts the transport under a lock/atomic operation) and call that from OnEditorReady/AutoStartAsync instead of performing separate IsRunning checks; alternatively adjust HttpBridgeReloadHandler.OnAfterAssemblyReload so it only clears ResumeHttpAfterReload after it has successfully resumed the transport, and update calls to use the new atomic method to avoid double-start races (refer to OnEditorReady, AutoStartAsync, HttpBridgeReloadHandler.OnAfterAssemblyReload, TransportManager.IsRunning/TryStartTransport).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs`:
- Around line 37-60: The OnEditorReady auto-start has a TOCTOU race with
HttpBridgeReloadHandler because it checks EditorPrefKeys.ResumeHttpAfterReload
and TransportManager.IsRunning separately; fix by introducing an atomic start
API on the transport manager (e.g., add TransportManager.TryStartHttpTransport
or TryStartTransport(TransportMode) that performs the "is running" check and
starts the transport under a lock/atomic operation) and call that from
OnEditorReady/AutoStartAsync instead of performing separate IsRunning checks;
alternatively adjust HttpBridgeReloadHandler.OnAfterAssemblyReload so it only
clears ResumeHttpAfterReload after it has successfully resumed the transport,
and update calls to use the new atomic method to avoid double-start races (refer
to OnEditorReady, AutoStartAsync, HttpBridgeReloadHandler.OnAfterAssemblyReload,
TransportManager.IsRunning/TryStartTransport).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21a033a4-a166-4665-88bb-68eb0683174d
📒 Files selected for processing (8)
MCPForUnity/Editor/Constants/EditorPrefKeys.csMCPForUnity/Editor/Services/HttpAutoStartHandler.csMCPForUnity/Editor/Services/HttpAutoStartHandler.cs.metaMCPForUnity/Editor/Services/IServerManagementService.csMCPForUnity/Editor/Services/ServerManagementService.csMCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.csMCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.csMCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.uxml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/HttpAutoStartHandler.cs (1)
105-152: Extract the wait/connect policy instead of mirroring it.The docstring says this flow mirrors
TryAutoStartSessionAsyncinMcpConnectionSection. Keeping the retry thresholds, late-connect fallback, and logging in two places will drift quickly; a shared helper/service would keep both entry points consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs` around lines 105 - 152, Extract the retry/wait-and-connect policy into a single shared helper (e.g., an AutoStartPolicy or AutoStartHelper) and have both WaitForServerAndConnectAsync and TryAutoStartSessionAsync call that helper; move the constants (maxAttempts, shortDelay, longDelay), the abort checks (EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad), EditorConfigurationCache.Instance.UseHttpTransport, MCPServiceLocator.TransportManager.IsRunning), the reachability check (MCPServiceLocator.Server.IsLocalHttpServerReachable), the late-connect fallback logic, the bridge start call (MCPServiceLocator.Bridge.StartAsync), and the logging/health-verification (McpLog.Info, McpLog.Warn, MCPForUnityEditorWindow.RequestHealthVerification) into the new shared routine so both callers simply invoke it and preserve identical behavior and messages.
🤖 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/Services/HttpAutoStartHandler.cs`:
- Around line 17-35: The static constructor for HttpAutoStartHandler is firing
on domain reloads and recompiles; scope it to the first editor session by using
SessionState (e.g., a bool key like "HttpAutoStartHandler.SessionInitialized")
so the constructor returns if SessionState.GetBool(key, false) is true, and set
SessionState.SetBool(key, true) after passing the checks; keep the existing
checks for Application.isBatchMode and
EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad) and only register
EditorApplication.delayCall += OnEditorReady when the session-scoped flag
indicates this is the first load (HttpAutoStartHandler static ctor and
OnEditorReady are the symbols to update), leaving HttpBridgeReloadHandler to
continue handling reload-resume cases.
---
Nitpick comments:
In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs`:
- Around line 105-152: Extract the retry/wait-and-connect policy into a single
shared helper (e.g., an AutoStartPolicy or AutoStartHelper) and have both
WaitForServerAndConnectAsync and TryAutoStartSessionAsync call that helper; move
the constants (maxAttempts, shortDelay, longDelay), the abort checks
(EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad),
EditorConfigurationCache.Instance.UseHttpTransport,
MCPServiceLocator.TransportManager.IsRunning), the reachability check
(MCPServiceLocator.Server.IsLocalHttpServerReachable), the late-connect fallback
logic, the bridge start call (MCPServiceLocator.Bridge.StartAsync), and the
logging/health-verification (McpLog.Info, McpLog.Warn,
MCPForUnityEditorWindow.RequestHealthVerification) into the new shared routine
so both callers simply invoke it and preserve identical behavior and messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f050e16-8456-4f0b-85bd-f0b31e416b6f
📒 Files selected for processing (2)
MCPForUnity/Editor/Services/HttpAutoStartHandler.csMCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- MCPForUnity/Editor/Windows/Components/Advanced/McpAdvancedSection.cs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
MCPForUnity/Editor/Services/HttpAutoStartHandler.cs (2)
151-152: Prefer a specific exception type over a barecatch.The empty
catch { return; }swallows all exceptions. While this is defensive,Task.Delaytypically throwsTaskCanceledExceptionorOperationCanceledExceptionif cancelled. Catching these specifically improves clarity and avoids masking unexpected errors.Suggested fix
- try { await Task.Delay(delay); } - catch { return; } + try { await Task.Delay(delay); } + catch (OperationCanceledException) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs` around lines 151 - 152, Replace the bare catch in the try around Task.Delay(delay) so only cancellation-related exceptions are swallowed: catch OperationCanceledException (which also covers TaskCanceledException) and return; avoid catching Exception or using an empty catch block. Update the try/catch surrounding Task.Delay(delay) in HttpAutoStartHandler (the method containing that await) to catch OperationCanceledException specifically and return on cancellation.
128-148: Consider extracting duplicated bridge-start logic.Lines 130-136 and 141-147 share identical logic for starting the bridge, logging success, requesting health verification, and returning. Extracting this to a local helper method would reduce duplication and make future changes easier.
Suggested refactor
private static async Task WaitForServerAndConnectAsync() { const int maxAttempts = 30; var shortDelay = TimeSpan.FromMilliseconds(500); var longDelay = TimeSpan.FromSeconds(3); + async Task<bool> TryStartBridgeAsync(string logSuffix = "") + { + bool started = await MCPServiceLocator.Bridge.StartAsync(); + if (started) + { + McpLog.Info($"[HTTP Auto-Start] Bridge started successfully{logSuffix}"); + MCPForUnityEditorWindow.RequestHealthVerification(); + } + return started; + } + for (int attempt = 0; attempt < maxAttempts; attempt++) { // Abort if user changed settings while we were waiting. if (!EditorPrefs.GetBool(EditorPrefKeys.AutoStartOnLoad, false)) return; if (!EditorConfigurationCache.Instance.UseHttpTransport) return; if (MCPServiceLocator.TransportManager.IsRunning(TransportMode.Http)) return; bool reachable = MCPServiceLocator.Server.IsLocalHttpServerReachable(); if (reachable) { - bool started = await MCPServiceLocator.Bridge.StartAsync(); - if (started) - { - McpLog.Info("[HTTP Auto-Start] Bridge started successfully"); - MCPForUnityEditorWindow.RequestHealthVerification(); - return; - } + if (await TryStartBridgeAsync()) return; } else if (attempt >= 20 && (attempt - 20) % 3 == 0) { // Last-resort: try connecting even if not detected (process detection may fail). - bool started = await MCPServiceLocator.Bridge.StartAsync(); - if (started) - { - McpLog.Info("[HTTP Auto-Start] Bridge started successfully (late connect)"); - MCPForUnityEditorWindow.RequestHealthVerification(); - return; - } + if (await TryStartBridgeAsync(" (late connect)")) return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs` around lines 128 - 148, The duplicated block that calls MCPServiceLocator.Bridge.StartAsync, logs success via McpLog.Info, calls MCPForUnityEditorWindow.RequestHealthVerification, and returns should be extracted into a small helper (e.g., TryStartBridgeAsync or StartBridgeAndVerifyAsync) that performs the await MCPServiceLocator.Bridge.StartAsync() call, logs with McpLog.Info on success (use the appropriate message variant), triggers MCPForUnityEditorWindow.RequestHealthVerification(), and returns a bool indicating success; then replace both occurrences with a single call to that helper and return when it yields true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MCPForUnity/Editor/Services/HttpAutoStartHandler.cs`:
- Around line 151-152: Replace the bare catch in the try around
Task.Delay(delay) so only cancellation-related exceptions are swallowed: catch
OperationCanceledException (which also covers TaskCanceledException) and return;
avoid catching Exception or using an empty catch block. Update the try/catch
surrounding Task.Delay(delay) in HttpAutoStartHandler (the method containing
that await) to catch OperationCanceledException specifically and return on
cancellation.
- Around line 128-148: The duplicated block that calls
MCPServiceLocator.Bridge.StartAsync, logs success via McpLog.Info, calls
MCPForUnityEditorWindow.RequestHealthVerification, and returns should be
extracted into a small helper (e.g., TryStartBridgeAsync or
StartBridgeAndVerifyAsync) that performs the await
MCPServiceLocator.Bridge.StartAsync() call, logs with McpLog.Info on success
(use the appropriate message variant), triggers
MCPForUnityEditorWindow.RequestHealthVerification(), and returns a bool
indicating success; then replace both occurrences with a single call to that
helper and return when it yields true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbbd98bf-8c36-45a6-8ae6-dac0a59c8271
📒 Files selected for processing (1)
MCPForUnity/Editor/Services/HttpAutoStartHandler.cs
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.
Auto-start server (for http local) if toggled the option.
Related Issues
#822
Additional Notes
Summary by Sourcery
Add an editor preference and handler to automatically start the MCP HTTP bridge (and local server when applicable) on Unity Editor load, with a quiet start mode for non-interactive flows.
New Features:
Enhancements:
Summary by CodeRabbit