Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion MCPForUnity/Editor/Clients/IMcpClientConfigurator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,20 @@ public interface IMcpClientConfigurator
/// <summary>Checks and updates status; returns current status.</summary>
McpStatus CheckStatus(bool attemptAutoRewrite = true);

/// <summary>Runs auto-configuration (register/write file/CLI etc.).</summary>
/// <summary>Runs auto-configuration (register/write file/CLI etc.). Always idempotent
/// — calling twice with the same settings is safe and is what the bulk "Configure All"
/// path relies on to refresh transport / server-version drift across every detected
/// client.</summary>
void Configure();

/// <summary>
/// Removes UnityMCP from this client's config (JSON entry, CLI registration, etc.).
/// Default is a no-op for client types that don't yet implement removal (Codex TOML);
/// callers should treat this as best-effort. The UI's per-client button routes here
/// when <see cref="Status"/> is <see cref="McpStatus.Configured"/>.
/// </summary>
void Unregister();

/// <summary>Returns the manual configuration snippet (JSON/TOML/commands).</summary>
string GetManualSnippet();

Expand Down
63 changes: 62 additions & 1 deletion MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ protected McpClientConfiguratorBase(McpClient client)
public abstract string GetConfigPath();
public abstract McpStatus CheckStatus(bool attemptAutoRewrite = true);
public abstract void Configure();

/// <summary>Default Unregister is a no-op. Override in JsonFileMcpConfigurator /
/// ClaudeCliMcpConfigurator etc. where removal has a concrete implementation.</summary>
public virtual void Unregister() { }
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public abstract string GetManualSnippet();
public abstract IList<string> GetInstallationSteps();

Expand Down Expand Up @@ -334,6 +339,9 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true)

public override void Configure()
{
// Always idempotent-write. The per-client UI button routes through Unregister
// when the user clicks while the client is already Configured; the bulk
// "Configure All" path calls this directly and expects an unconditional write.
string path = GetConfigPath();
McpConfigurationHelper.EnsureConfigDirectoryExists(path);
string result = McpConfigurationHelper.WriteMcpConfiguration(path, client);
Expand All @@ -348,6 +356,59 @@ public override void Configure()
}
}

public override string GetConfigureActionLabel()
=> client.status == McpStatus.Configured ? "Unregister" : "Configure";

/// <summary>
/// Removes the unityMCP entry from the client's JSON config (both VS Code-style
/// `servers` / `mcp.servers` layouts and the standard `mcpServers` layout). Leaves
/// the file in place so we don't clobber other servers the user has configured.
/// </summary>
public override void Unregister()
{
string path = GetConfigPath();
try
{
if (!File.Exists(path))
{
client.SetStatus(McpStatus.NotConfigured);
client.configuredTransport = Models.ConfiguredTransport.Unknown;
return;
}

var root = JsonConvert.DeserializeObject<JToken>(File.ReadAllText(path)) as JObject;
if (root == null)
{
client.SetStatus(McpStatus.NotConfigured);
client.configuredTransport = Models.ConfiguredTransport.Unknown;
return;
}

bool removed = false;
if (client.IsVsCodeLayout)
{
if ((root["servers"] as JObject)?.Remove("unityMCP") == true) removed = true;
if ((root["mcp"]?["servers"] as JObject)?.Remove("unityMCP") == true) removed = true;
}
else
{
if ((root["mcpServers"] as JObject)?.Remove("unityMCP") == true) removed = true;
}

if (removed)
{
File.WriteAllText(path, root.ToString(Formatting.Indented));
}

client.SetStatus(McpStatus.NotConfigured);
client.configuredTransport = Models.ConfiguredTransport.Unknown;
}
catch (Exception ex)
{
throw new InvalidOperationException($"Failed to unregister: {ex.Message}", ex);
}
}

public override string GetManualSnippet()
{
try
Expand Down Expand Up @@ -964,7 +1025,7 @@ private void Register()
client.configuredTransport = HttpEndpointUtility.GetCurrentServerTransport();
}

private void Unregister()
public override void Unregister()
{
var pathService = MCPServiceLocator.Paths;
string claudePath = pathService.GetClaudeCliPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,15 @@ private void InitializeUI()
manualConfigFoldout.value = false;
}

// Restore the "Configure a single client" foldout state. Defaults to collapsed
// so the prominent "Configure All Detected Clients" path is what users see first.
// Default the per-client setup foldout to expanded. The Configure / Unregister
// button for the currently-selected client lives inside it, and the 9.7.0
// default-collapsed behavior made users believe the Unregister action had been
// removed (community report: "the Unregister button was removed in 9.7.0,
// something is stuck on stdio and I can't switch to local"). The state still
// persists per-user, so anyone who explicitly collapses it keeps that preference.
if (clientDetailsFoldout != null)
{
clientDetailsFoldout.value = EditorPrefs.GetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, false);
clientDetailsFoldout.value = EditorPrefs.GetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, true);
clientDetailsFoldout.RegisterValueChangedCallback(evt =>
{
EditorPrefs.SetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, evt.newValue);
Expand Down Expand Up @@ -308,6 +312,13 @@ private void OnConfigureAllClientsClicked()

EditorUtility.DisplayDialog("Configure Detected Clients", message, "OK");

// The bulk path mutated state for every detected client. Clear the per-client
// status cache so any subsequent dropdown switch (or the currently-selected
// client refresh below) reads fresh status from disk instead of the pre-bulk
// value — otherwise every other client in the dropdown looks like it still
// has Missing/IncorrectPath status until 45 s elapses on the throttle.
lastStatusChecks.Clear();

if (selectedClientIndex >= 0 && selectedClientIndex < configurators.Count)
{
UpdateClientStatus();
Expand Down Expand Up @@ -336,7 +347,18 @@ private void OnConfigureClicked()

try
{
MCPServiceLocator.Client.ConfigureClient(client);
// Per-client toggle: Configure→register, Unregister→remove. The Configure path
// on JsonFile / Codex configurators is always idempotent-write so the bulk
// "Configure All" can call it unconditionally; the toggle lives here in the UI
// handler so the manual button still does what its label says.
if (client.Status == McpStatus.Configured)
{
client.Unregister();
}
else
{
MCPServiceLocator.Client.ConfigureClient(client);
}
lastStatusChecks.Remove(client);
RefreshClientStatus(client, forceImmediate: true);
UpdateManualConfiguration();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<ui:Label text="Client Configuration" class="section-title" />
<ui:VisualElement class="section-content">
<ui:Button name="configure-all-button" text="Configure All Detected Clients" class="primary-button" />
<ui:Foldout name="client-details-foldout" text="Per-client setup" value="false" class="client-details-foldout">
<ui:Foldout name="client-details-foldout" text="Per-client setup" value="true" class="client-details-foldout">
<ui:VisualElement class="setting-row">
<ui:Label text="Client:" class="setting-label" />
<ui:DropdownField name="client-dropdown" class="setting-dropdown-inline" />
Expand Down
Loading