fix: Multi-session UI improvements and HTTP instance recognition (#517)

* fix: Multi-session UI improvements and HTTP instance recognition

- Separate server and session lifecycle in HTTP Local mode
  - Show 'Start Server' / 'Stop Server' button for server control
  - Show 'Start Session' / 'End Session' button when server is running
  - No auto-join on server start (requires manual session start)
- Show instance name instead of port in session status (e.g. 'Session Active (ramble)')
- Use native project_hash for HTTP instance recognition instead of computed SHA256
- Fix test expectations for manage_asset JSON parsing error messages

* fix: Multi-session UI improvements and HTTP instance recognition

- Separate server and session lifecycle in HTTP Local mode
  - Show 'Start Server' / 'Stop Server' button for server control
  - Show 'Start Session' / 'End Session' button when server is running
  - Auto-start session when THIS instance starts the server
  - Require manual session start when connecting to external server
  - Auto-detect and end orphaned sessions when server stops
- Show instance name instead of port in session status
- Use native project_hash for HTTP instance recognition
  - Guard against empty hash with warning log
- Remove dead code (duplicate _safe_response)
- Add defensive path handling for instance name extraction
main
dsarno 2026-01-05 16:05:38 -08:00 committed by GitHub
parent d6a1a9b9e3
commit 2a1e56a5ee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 109 additions and 190 deletions

View File

@ -47,7 +47,6 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
private Button testConnectionButton; private Button testConnectionButton;
private bool connectionToggleInProgress; private bool connectionToggleInProgress;
private bool autoStartInProgress;
private bool httpServerToggleInProgress; private bool httpServerToggleInProgress;
private Task verificationTask; private Task verificationTask;
private string lastHealthStatus; private string lastHealthStatus;
@ -269,16 +268,28 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
bool debugMode = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false); bool debugMode = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false);
bool stdioSelected = transportDropdown != null && (TransportProtocol)transportDropdown.value == TransportProtocol.Stdio; bool stdioSelected = transportDropdown != null && (TransportProtocol)transportDropdown.value == TransportProtocol.Stdio;
// Keep the consolidated Start/Stop Server button label in sync even when the session is not running // Keep the Start/Stop Server button label in sync even when the session is not running
// (e.g., orphaned server after a domain reload). // (e.g., orphaned server after a domain reload).
// NOTE: This also updates lastLocalServerRunning which is used below for session toggle visibility.
UpdateStartHttpButtonState(); UpdateStartHttpButtonState();
// If local-server controls are active, hide the manual session toggle controls and // Detect orphaned session: if HTTP Local session thinks it's running but the server is gone,
// rely on the Start/Stop Server button. We still keep the session status text visible // automatically end the session to keep UI in sync with reality.
// next to the dot for clarity. if (showLocalServerControls && isRunning && !lastLocalServerRunning && !connectionToggleInProgress)
{
McpLog.Info("Server no longer running; ending orphaned session.");
_ = EndOrphanedSessionAsync();
isRunning = false; // Update local state for the rest of this method
}
// For HTTP Local: show session toggle button only when server is running (so user can manually start/end session).
// For Stdio/HTTP Remote: always show the session toggle button.
// This separates server lifecycle from session lifecycle for multi-instance scenarios.
// We use lastLocalServerRunning which was just refreshed by UpdateStartHttpButtonState() above.
if (connectionToggleButton != null) if (connectionToggleButton != null)
{ {
connectionToggleButton.style.display = showLocalServerControls ? DisplayStyle.None : DisplayStyle.Flex; bool showSessionToggle = !showLocalServerControls || lastLocalServerRunning;
connectionToggleButton.style.display = showSessionToggle ? DisplayStyle.Flex : DisplayStyle.None;
} }
// Hide "Test" buttons unless Debug Mode is enabled. // Hide "Test" buttons unless Debug Mode is enabled.
@ -297,7 +308,14 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
if (isRunning) if (isRunning)
{ {
connectionStatusLabel.text = "Session Active"; // Show instance name (project folder name) for better identification in multi-instance scenarios.
// Defensive: handle edge cases where path parsing might return null/empty.
string projectDir = System.IO.Path.GetDirectoryName(Application.dataPath);
string instanceName = !string.IsNullOrEmpty(projectDir)
? System.IO.Path.GetFileName(projectDir)
: "Unity";
if (string.IsNullOrEmpty(instanceName)) instanceName = "Unity";
connectionStatusLabel.text = $"Session Active ({instanceName})";
statusIndicator.RemoveFromClassList("disconnected"); statusIndicator.RemoveFromClassList("disconnected");
statusIndicator.AddToClassList("connected"); statusIndicator.AddToClassList("connected");
connectionToggleButton.text = "End Session"; connectionToggleButton.text = "End Session";
@ -429,7 +447,6 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
bool httpLocalSelected = IsHttpLocalSelected(); bool httpLocalSelected = IsHttpLocalSelected();
bool canStartLocalServer = httpLocalSelected && MCPServiceLocator.Server.IsLocalUrl(); bool canStartLocalServer = httpLocalSelected && MCPServiceLocator.Server.IsLocalUrl();
bool sessionRunning = MCPServiceLocator.Bridge.IsRunning;
bool localServerRunning = false; bool localServerRunning = false;
// Avoid running expensive port/PID checks every UI tick. // Avoid running expensive port/PID checks every UI tick.
@ -444,16 +461,15 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
localServerRunning = lastLocalServerRunning; localServerRunning = lastLocalServerRunning;
} }
// Single consolidated button: Start Server (launch local server + start session) or // Server button only controls server lifecycle (Start/Stop Server).
// Stop Server (end session + attempt to stop local server). // Session lifecycle is handled by the separate connectionToggleButton.
bool shouldShowStop = sessionRunning || localServerRunning; bool shouldShowStop = localServerRunning;
startHttpServerButton.text = shouldShowStop ? "Stop Server" : "Start Server"; startHttpServerButton.text = shouldShowStop ? "Stop Server" : "Start Server";
// Note: Server logs may contain transient HTTP 400s on /mcp during startup probing and // Note: Server logs may contain transient HTTP 400s on /mcp during startup probing and
// CancelledError stack traces on shutdown when streaming requests are cancelled; this is expected. // CancelledError stack traces on shutdown when streaming requests are cancelled; this is expected.
startHttpServerButton.EnableInClassList("server-running", shouldShowStop); startHttpServerButton.EnableInClassList("server-running", localServerRunning);
startHttpServerButton.SetEnabled( startHttpServerButton.SetEnabled(
(canStartLocalServer && !httpServerToggleInProgress && !autoStartInProgress) || !httpServerToggleInProgress && (shouldShowStop || canStartLocalServer));
(shouldShowStop && !httpServerToggleInProgress));
startHttpServerButton.tooltip = httpLocalSelected startHttpServerButton.tooltip = httpLocalSelected
? (canStartLocalServer ? string.Empty : "HTTP Local requires a localhost URL (localhost/127.0.0.1/0.0.0.0/::1).") ? (canStartLocalServer ? string.Empty : "HTTP Local requires a localhost URL (localhost/127.0.0.1/0.0.0.0/::1).")
: string.Empty; : string.Empty;
@ -481,34 +497,36 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
try try
{ {
// If a session is active, treat this as "Stop Server" (end session first, then try to stop server). // Check if a local server is running.
if (bridgeService.IsRunning) bool serverRunning = IsHttpLocalSelected() && MCPServiceLocator.Server.IsLocalHttpServerRunning();
if (serverRunning)
{ {
await bridgeService.StopAsync(); // Stop Server: end session first (if active), then stop the server.
if (bridgeService.IsRunning)
{
await bridgeService.StopAsync();
}
bool stopped = MCPServiceLocator.Server.StopLocalHttpServer(); bool stopped = MCPServiceLocator.Server.StopLocalHttpServer();
if (!stopped) if (!stopped)
{ {
McpLog.Warn("Failed to stop HTTP server or no server was running"); McpLog.Warn("Failed to stop HTTP server or no server was running");
} }
return;
} }
else
// If the session isn't running but the local server is, allow stopping the server directly.
if (IsHttpLocalSelected() && MCPServiceLocator.Server.IsLocalHttpServerRunning())
{ {
bool stopped = MCPServiceLocator.Server.StopLocalHttpServer(); // Start Server: launch the local HTTP server.
if (!stopped) // When WE start the server, auto-start our session (we clearly want to use it).
// This differs from detecting an already-running server, where we require manual session start.
bool serverStarted = MCPServiceLocator.Server.StartLocalHttpServer();
if (serverStarted)
{ {
McpLog.Warn("Failed to stop HTTP server or no server was running"); await TryAutoStartSessionAsync();
}
else
{
McpLog.Warn("Failed to start local HTTP server");
} }
return;
}
// Otherwise, "Start Server" and then auto-start the session.
bool started = MCPServiceLocator.Server.StartLocalHttpServer();
if (started)
{
await TryAutoStartSessionAfterHttpServerAsync();
} }
} }
catch (Exception ex) catch (Exception ex)
@ -524,6 +542,40 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
} }
} }
private async Task TryAutoStartSessionAsync()
{
// Wait briefly for the HTTP server to become ready, then start the session.
// This is called when THIS instance starts the server (not when detecting an external server).
var bridgeService = MCPServiceLocator.Bridge;
const int maxAttempts = 10;
var delay = TimeSpan.FromSeconds(1);
for (int attempt = 0; attempt < maxAttempts; attempt++)
{
// Check if server is actually accepting connections
if (!MCPServiceLocator.Server.IsLocalHttpServerRunning())
{
await Task.Delay(delay);
continue;
}
bool started = await bridgeService.StartAsync();
if (started)
{
await VerifyBridgeConnectionAsync();
UpdateConnectionStatus();
return;
}
if (attempt < maxAttempts - 1)
{
await Task.Delay(delay);
}
}
McpLog.Warn("Failed to auto-start session after launching the HTTP server.");
}
private void PersistUnityPortFromField() private void PersistUnityPortFromField()
{ {
if (unityPortField == null) if (unityPortField == null)
@ -605,159 +657,28 @@ namespace MCPForUnity.Editor.Windows.Components.Connection
await VerifyBridgeConnectionAsync(); await VerifyBridgeConnectionAsync();
} }
private async Task TryAutoStartSessionAfterHttpServerAsync() private async Task EndOrphanedSessionAsync()
{ {
if (autoStartInProgress) // Fire-and-forget cleanup of orphaned session when server is no longer running.
{ // This prevents the UI from showing "Session Active" when the underlying server is gone.
return;
}
var bridgeService = MCPServiceLocator.Bridge;
if (bridgeService.IsRunning)
{
return;
}
autoStartInProgress = true;
connectionToggleButton?.SetEnabled(false);
const int maxAttempts = 10;
var delay = TimeSpan.FromSeconds(1);
try try
{ {
// Wait until the HTTP server is actually accepting connections to reduce transient "unable to connect then recovers" connectionToggleInProgress = true;
// behavior (session start attempts can race the server startup). connectionToggleButton?.SetEnabled(false);
// Dev mode uses --no-cache --refresh which causes uvx to rebuild the package, taking significantly longer. await MCPServiceLocator.Bridge.StopAsync();
bool devModeEnabled = false; }
try { devModeEnabled = EditorPrefs.GetBool(EditorPrefKeys.DevModeForceServerRefresh, false); } catch { } catch (Exception ex)
var startupTimeout = devModeEnabled ? TimeSpan.FromSeconds(45) : TimeSpan.FromSeconds(10); {
McpLog.Warn($"Failed to end orphaned session: {ex.Message}");
if (devModeEnabled)
{
McpLog.Info("Dev mode enabled: server startup may take longer while uvx rebuilds the package...");
}
bool serverReady = await WaitForHttpServerAcceptingConnectionsAsync(startupTimeout);
if (!serverReady)
{
McpLog.Warn($"HTTP server did not become reachable within {startupTimeout.TotalSeconds}s; will still attempt to start the session.");
}
for (int attempt = 0; attempt < maxAttempts; attempt++)
{
bool started = await bridgeService.StartAsync();
if (started)
{
await VerifyBridgeConnectionAsync();
UpdateConnectionStatus();
return;
}
if (attempt < maxAttempts - 1)
{
await Task.Delay(delay);
}
}
McpLog.Warn("Failed to auto-start MCP session after launching the HTTP server.");
} }
finally finally
{ {
autoStartInProgress = false; connectionToggleInProgress = false;
connectionToggleButton?.SetEnabled(true); connectionToggleButton?.SetEnabled(true);
UpdateConnectionStatus(); UpdateConnectionStatus();
} }
} }
private static async Task<bool> WaitForHttpServerAcceptingConnectionsAsync(TimeSpan timeout)
{
try
{
string baseUrl = HttpEndpointUtility.GetBaseUrl();
if (!Uri.TryCreate(baseUrl, UriKind.Absolute, out var uri) || uri.Port <= 0)
{
return true; // Don't block if URL cannot be parsed
}
string host = uri.Host;
int port = uri.Port;
// Normalize wildcard/bind-all hosts to loopback for readiness checks.
// When the server binds to 0.0.0.0 or ::, clients typically connect via localhost/127.0.0.1.
string normalizedHost;
if (string.IsNullOrWhiteSpace(host)
|| string.Equals(host, "0.0.0.0", StringComparison.OrdinalIgnoreCase)
|| string.Equals(host, "::", StringComparison.OrdinalIgnoreCase)
|| string.Equals(host, "*", StringComparison.OrdinalIgnoreCase))
{
normalizedHost = "localhost";
}
else
{
normalizedHost = host;
}
var deadline = DateTime.UtcNow + timeout;
while (DateTime.UtcNow < deadline)
{
try
{
using var client = new TcpClient();
var connectTask = client.ConnectAsync(normalizedHost, port);
var completed = await Task.WhenAny(connectTask, Task.Delay(250));
if (completed != connectTask)
{
// Avoid leaving a long-running ConnectAsync in-flight (default OS connect timeout can be very long),
// which can accumulate across retries and impact overall editor/network responsiveness.
// Closing the client will cause ConnectAsync to complete quickly (typically with an exception),
// which we then attempt to observe (bounded) by awaiting.
try { client.Close(); } catch { }
}
try
{
// Even after Close(), some platforms may take a moment to complete the connect task.
// Keep this bounded so the readiness loop can't hang here.
var connectCompleted = await Task.WhenAny(connectTask, Task.Delay(250));
if (connectCompleted == connectTask)
{
await connectTask;
}
else
{
_ = connectTask.ContinueWith(
t => _ = t.Exception,
System.Threading.CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted,
TaskScheduler.Default);
}
}
catch
{
// Ignore connection exceptions and retry until timeout.
}
if (client.Connected)
{
return true;
}
}
catch
{
// Ignore and retry until timeout
}
await Task.Delay(150);
}
return false;
}
catch
{
return false;
}
}
public async Task VerifyBridgeConnectionAsync() public async Task VerifyBridgeConnectionAsync()
{ {
// Prevent concurrent verification calls // Prevent concurrent verification calls

View File

@ -266,15 +266,14 @@ class CustomToolService:
return None return None
return {"message": str(response)} return {"message": str(response)}
def _safe_response(self, response):
if isinstance(response, dict):
return response
if response is None:
return None
return {"message": str(response)}
def compute_project_id(project_name: str, project_path: str) -> str: def compute_project_id(project_name: str, project_path: str) -> str:
"""
DEPRECATED: Computes a SHA256-based project ID.
This function is no longer used as of the multi-session fix.
Unity instances now use their native project_hash (SHA1-based) for consistency
across stdio and WebSocket transports.
"""
combined = f"{project_name}:{project_path}" combined = f"{project_name}:{project_path}"
return sha256(combined.encode("utf-8")).hexdigest().upper()[:16] return sha256(combined.encode("utf-8")).hexdigest().upper()[:16]
@ -307,7 +306,12 @@ def resolve_project_id_for_unity_instance(unity_instance: str | None) -> str | N
) )
if target: if target:
return compute_project_id(target.name, target.path) # Return the project_hash from Unity (not a computed SHA256 hash).
# This matches the hash Unity uses when registering tools via WebSocket.
if target.hash:
return target.hash
logger.warning(f"Unity instance {target.id} has empty hash; cannot resolve project ID")
return None
except Exception: except Exception:
logger.debug( logger.debug(
f"Failed to resolve project id via connection pool for {unity_instance}") f"Failed to resolve project id via connection pool for {unity_instance}")

View File

@ -32,13 +32,7 @@ class TestManageAssetJsonParsing:
properties='{"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]}' properties='{"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]}'
) )
# Verify JSON parsing was logged # Verify the result - JSON string was successfully parsed and passed to Unity
assert any(
"manage_asset: coerced properties using centralized parser" in msg
for msg in ctx.log_info
)
# Verify the result
assert result["success"] is True assert result["success"] is True
assert "Asset created successfully" in result["message"] assert "Asset created successfully" in result["message"]
@ -63,7 +57,7 @@ class TestManageAssetJsonParsing:
# Verify behavior: parsing fails with a clear error # Verify behavior: parsing fails with a clear error
assert result.get("success") is False assert result.get("success") is False
assert "failed to parse properties string" in result.get("message", "") assert "failed to parse properties" in result.get("message", "")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_properties_dict_unchanged(self, monkeypatch): async def test_properties_dict_unchanged(self, monkeypatch):