ucode MCP cleanup: dedup cursor mcp.json writers (follow-up to #560)

src/ucode/agents/claude.py

@@ -26,6 +26,8 @@
 )
 from ucode.constants import (
     LOOPBACK_HOST,
+    MCP_CLEANUP_SCOPES,
+    MCP_USER_SCOPE,
     MODEL_PROVIDER_SERVICE_HEADER,
     MODEL_SERVICE_PARENT_SCHEMA_HEADER,
 )
@@ -49,6 +51,7 @@
     reconcile_managed_file,
     revert_managed_file,
 )
+from ucode.mcp_oauth import CLAUDE_CODE_OAUTH_CLIENT_ID, MCP_OAUTH_CALLBACK_PORT
 from ucode.smart_routing import v2 as smart_routing_v2
 from ucode.smart_routing.claude_hooks import (
     remove_smart_routing_hooks,
@@ -502,6 +505,106 @@ def _enforce_model_default_hierarchy(
     return selected_default_model
 
 
+def add_claude_mcp_server(
+    name: str,
+    server: list[str] | dict,
+    scope: str = MCP_USER_SCOPE,
+    *,
+    always_load: bool = False,
+) -> None:
+    # Three registration shapes share this helper. The plain proxy path passes an
+    # argv list (`ucode mcp-proxy ...`), registered via `claude mcp add ... -- <argv>`
+    # where `--` fences the proxy's own flags off from claude's parser. The
+    # web_search server passes a full stdio entry dict with its own env, which only
+    # `add-json` can express — so a dict routes there. Finally, `always_load` (the
+    # skills registry) needs `alwaysLoad: true`, which plain `mcp add` can't set, so
+    # build a stdio entry dict and route it to add-json too.
+    if isinstance(server, dict):
+        cmd = ["claude", "mcp", "add-json", name, json.dumps(server), "-s", scope]
+    elif always_load:
+        entry = {
+            "type": "stdio",
+            "command": server[0],
+            "args": list(server[1:]),
+            "alwaysLoad": True,
+        }
+        cmd = ["claude", "mcp", "add-json", name, json.dumps(entry), "-s", scope]
+    else:
+        cmd = ["claude", "mcp", "add", name, "-s", scope, "--", *server]
+    try:
+        subprocess.run(
+            cmd,
+            check=True,
+            capture_output=True,
+            text=True,
+            timeout=30,
+        )
+    except subprocess.CalledProcessError as exc:
+        raise RuntimeError(f"Failed to add MCP server '{name}' via claude CLI.") from exc
+
+
+def add_claude_http_mcp_server(
+    name: str,
+    url: str,
+    scope: str = MCP_USER_SCOPE,
+    *,
+    client_id: str = CLAUDE_CODE_OAUTH_CLIENT_ID,
+    callback_port: int = MCP_OAUTH_CALLBACK_PORT,
+) -> None:
+    """Register a Databricks MCP endpoint as a **direct HTTP** server so Claude
+    Code is the OAuth client and drives the RFC 8707 connection login itself.
+
+    Unlike the stdio proxy (which injects a plain workspace token and hides the
+    per-user connection state), a direct HTTP server lets Claude Code do MCP OAuth
+    against ``/oidc`` with the ``resource`` indicator: on a missing/expired
+    connection credential, ``/mcp`` shows "needs authentication" and Authenticate
+    runs the login (``/oidc`` -> ``/mcp-service-login``). ``client_id`` is the
+    published ``claude-code`` app (it has the loopback ``/callback`` redirect
+    registered); the callback port is arbitrary because ``/oidc`` ignores the port
+    for loopback redirects."""
+    cmd = [
+        "claude",
+        "mcp",
+        "add",
+        "--transport",
+        "http",
+        "-s",
+        scope,
+        "--client-id",
+        client_id,
+        "--callback-port",
+        str(callback_port),
+        name,
+        url,
+    ]
+    try:
+        subprocess.run(cmd, check=True, capture_output=True, text=True, timeout=30)
+    except subprocess.CalledProcessError as exc:
+        raise RuntimeError(f"Failed to add HTTP MCP server '{name}' via claude CLI.") from exc
+
+
+def remove_claude_mcp_server(name: str, scope: str) -> bool:
+    # Imported lazily: `_is_missing_mcp_server_output` is a shared CLI-output matcher
+    # in ucode.mcp (used by the codex/gemini removers too), and ucode.mcp imports
+    # this module at load time — a function-level import avoids that cycle.
+    from ucode.mcp import _is_missing_mcp_server_output
+
+    try:
+        subprocess.run(
+            ["claude", "mcp", "remove", name, "-s", scope],
+            check=True,
+            capture_output=True,
+            text=True,
+            timeout=30,
+        )
+        return True
+    except subprocess.CalledProcessError as exc:
+        output = f"{exc.stderr or ''}\n{exc.stdout or ''}"
+        if _is_missing_mcp_server_output(output):
+            return False
+        raise RuntimeError(f"Failed to remove MCP server '{name}' via claude CLI.") from exc
+
+
 def _register_web_search_mcp(workspace: str, search_model: str, profile: str | None = None) -> bool:
     """Register (or replace) the web_search MCP server in Claude Code's user
     scope via `claude mcp add-json`. Removes any prior entry first so re-runs
@@ -510,13 +613,6 @@ def _register_web_search_mcp(workspace: str, search_model: str, profile: str | N
     Returns True if registration succeeded. Failures are non-blocking: we warn
     and return False so the rest of `ucode claude` setup can complete.
     """
-    # Imported lazily to avoid a circular import via ucode.mcp -> ucode.agents.
-    from ucode.mcp import (
-        MCP_CLEANUP_SCOPES,
-        add_claude_mcp_server,
-        remove_claude_mcp_server,
-    )
-
     for scope in MCP_CLEANUP_SCOPES:
         try:
             remove_claude_mcp_server(WEB_SEARCH_MCP_NAME, scope)
@@ -548,8 +644,6 @@ def _web_search_mcp_is_current(state: dict, entry: dict) -> bool:
 
 def _unregister_web_search_mcp() -> None:
     """Remove the web_search MCP server from all scopes. Used by revert."""
-    from ucode.mcp import MCP_CLEANUP_SCOPES, remove_claude_mcp_server
-
     for scope in MCP_CLEANUP_SCOPES:
         try:
             remove_claude_mcp_server(WEB_SEARCH_MCP_NAME, scope)

src/ucode/agents/cursor.py

@@ -36,23 +36,26 @@ def build_mcp_server_entry(argv: list[str]) -> dict:
     }
 
 
-def write_mcp_server_config(name: str, argv: list[str]) -> bool:
-    """Add (or replace) one MCP server entry in ~/.cursor/mcp.json.
-
-    Merges into the existing `mcpServers` map so unrelated entries the user
-    already configured survive. Returns True when an entry with this name was
-    already present (i.e. this was a replacement)."""
+def _upsert_mcp_server(name: str, entry: dict) -> bool:
+    """Add (or replace) one entry in ~/.cursor/mcp.json's `mcpServers`, merging so
+    unrelated entries the user already configured survive. Returns True when an
+    entry with this name was already present (i.e. this was a replacement)."""
     existing = read_json_safe(CURSOR_MCP_CONFIG_PATH)
     mcp_servers = existing.get("mcpServers")
     if not isinstance(mcp_servers, dict):
         mcp_servers = {}
     removed = name in mcp_servers
-    mcp_servers[name] = build_mcp_server_entry(argv)
+    mcp_servers[name] = entry
     existing["mcpServers"] = mcp_servers
     write_json_file(CURSOR_MCP_CONFIG_PATH, existing)
     return removed
 
 
+def write_mcp_server_config(name: str, argv: list[str]) -> bool:
+    """Add (or replace) a stdio (`ucode mcp-proxy`) MCP server in ~/.cursor/mcp.json."""
+    return _upsert_mcp_server(name, build_mcp_server_entry(argv))
+
+
 def build_http_mcp_server_entry(url: str, client_id: str) -> dict:
     # Cursor's remote-MCP-with-OAuth schema: a `url` server plus an `auth` object
     # naming a pre-registered OAuth client. Cursor drives the OAuth itself (to its
@@ -67,21 +70,10 @@ def build_http_mcp_server_entry(url: str, client_id: str) -> dict:
 
 
 def write_http_mcp_server_config(name: str, url: str, client_id: str) -> bool:
-    """Add (or replace) one **OAuth HTTP** MCP server entry in ~/.cursor/mcp.json.
-
-    Used for connection-backed AI Gateway services when the workspace has Cursor's
-    OAuth client published: Cursor authenticates directly rather than going through
-    the token-injecting stdio proxy. Merges into `mcpServers` like the stdio path;
-    returns True when an entry with this name was already present."""
-    existing = read_json_safe(CURSOR_MCP_CONFIG_PATH)
-    mcp_servers = existing.get("mcpServers")
-    if not isinstance(mcp_servers, dict):
-        mcp_servers = {}
-    removed = name in mcp_servers
-    mcp_servers[name] = build_http_mcp_server_entry(url, client_id)
-    existing["mcpServers"] = mcp_servers
-    write_json_file(CURSOR_MCP_CONFIG_PATH, existing)
-    return removed
+    """Add (or replace) an **OAuth HTTP** MCP server (url + pre-registered client) in
+    ~/.cursor/mcp.json, so Cursor drives the connection login itself instead of the
+    token-injecting stdio proxy."""
+    return _upsert_mcp_server(name, build_http_mcp_server_entry(url, client_id))
 
 
 def remove_mcp_server_config(name: str) -> bool:

src/ucode/constants.py

@@ -5,3 +5,9 @@
 
 MODEL_PROVIDER_SERVICE_HEADER = "Databricks-Model-Provider-Service"
 MODEL_SERVICE_PARENT_SCHEMA_HEADER = "Databricks-Model-Service-Parent-Schema"
+
+# MCP server registration scopes. Claude Code supports local/project/user; the
+# other CLIs only take the user-scope name. Kept here (a leaf module) so both
+# `ucode.mcp` and `ucode.agents.claude` can import them without an import cycle.
+MCP_USER_SCOPE = "user"
+MCP_CLEANUP_SCOPES = ("local", "project", MCP_USER_SCOPE)

src/ucode/mcp.py

@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import json
 import os
 import shutil
 import string
@@ -28,8 +27,9 @@
 from questionary.question import Question
 from questionary.styles import merge_styles_default
 
-from ucode.agents import copilot, cursor, gemini, opencode
+from ucode.agents import claude, copilot, cursor, gemini, opencode
 from ucode.config_io import restore_file
+from ucode.constants import MCP_CLEANUP_SCOPES, MCP_USER_SCOPE
 from ucode.databricks import (
     PermissionDeniedError,
     apply_pat_environment,
@@ -46,7 +46,6 @@
 from ucode.mcp_oauth import (
     CLAUDE_CODE_OAUTH_CLIENT_ID,
     CURSOR_OAUTH_CLIENT_ID,
-    MCP_OAUTH_CALLBACK_PORT,
     oauth_client_available,
 )
 from ucode.state import load_full_state, load_state, save_state
@@ -60,8 +59,6 @@
     spinner,
 )
 
-MCP_USER_SCOPE = "user"
-MCP_CLEANUP_SCOPES = ("local", "project", MCP_USER_SCOPE)
 MCP_PICKER_VISIBLE_ROWS = 10
 
 # AI Gateway MCP-services endpoints carry this path segment. These are the
@@ -142,84 +139,6 @@ class _Back:
 MCP_ADD_PREFIX = "add:"
 
 
-def add_claude_mcp_server(
-    name: str,
-    server: list[str] | dict,
-    scope: str = MCP_USER_SCOPE,
-    *,
-    always_load: bool = False,
-) -> None:
-    # Three registration shapes share this helper. The plain proxy path passes an
-    # argv list (`ucode mcp-proxy ...`), registered via `claude mcp add ... -- <argv>`
-    # where `--` fences the proxy's own flags off from claude's parser. The
-    # web_search server (agents/claude.py) passes a full stdio entry dict with its
-    # own env, which only `add-json` can express — so a dict routes there. Finally,
-    # `always_load` (the skills registry) needs `alwaysLoad: true`, which plain
-    # `mcp add` can't set, so build a stdio entry dict and route it to add-json too.
-    if isinstance(server, dict):
-        cmd = ["claude", "mcp", "add-json", name, json.dumps(server), "-s", scope]
-    elif always_load:
-        entry = {
-            "type": "stdio",
-            "command": server[0],
-            "args": list(server[1:]),
-            "alwaysLoad": True,
-        }
-        cmd = ["claude", "mcp", "add-json", name, json.dumps(entry), "-s", scope]
-    else:
-        cmd = ["claude", "mcp", "add", name, "-s", scope, "--", *server]
-    try:
-        subprocess.run(
-            cmd,
-            check=True,
-            capture_output=True,
-            text=True,
-            timeout=30,
-        )
-    except subprocess.CalledProcessError as exc:
-        raise RuntimeError(f"Failed to add MCP server '{name}' via claude CLI.") from exc
-
-
-def add_claude_http_mcp_server(
-    name: str,
-    url: str,
-    scope: str = MCP_USER_SCOPE,
-    *,
-    client_id: str = CLAUDE_CODE_OAUTH_CLIENT_ID,
-    callback_port: int = MCP_OAUTH_CALLBACK_PORT,
-) -> None:
-    """Register a Databricks MCP endpoint as a **direct HTTP** server so Claude
-    Code is the OAuth client and drives the RFC 8707 connection login itself.
-
-    Unlike the stdio proxy (which injects a plain workspace token and hides the
-    per-user connection state), a direct HTTP server lets Claude Code do MCP OAuth
-    against ``/oidc`` with the ``resource`` indicator: on a missing/expired
-    connection credential, ``/mcp`` shows "needs authentication" and Authenticate
-    runs the login (``/oidc`` -> ``/mcp-service-login``). ``client_id`` is the
-    published ``claude-code`` app (it has the loopback ``/callback`` redirect
-    registered); the callback port is arbitrary because ``/oidc`` ignores the port
-    for loopback redirects."""
-    cmd = [
-        "claude",
-        "mcp",
-        "add",
-        "--transport",
-        "http",
-        "-s",
-        scope,
-        "--client-id",
-        client_id,
-        "--callback-port",
-        str(callback_port),
-        name,
-        url,
-    ]
-    try:
-        subprocess.run(cmd, check=True, capture_output=True, text=True, timeout=30)
-    except subprocess.CalledProcessError as exc:
-        raise RuntimeError(f"Failed to add HTTP MCP server '{name}' via claude CLI.") from exc
-
-
 def _is_missing_mcp_server_output(output: str) -> bool:
     normalized = output.lower()
     return (
@@ -230,23 +149,6 @@ def _is_missing_mcp_server_output(output: str) -> bool:
     )
 
 
-def remove_claude_mcp_server(name: str, scope: str) -> bool:
-    try:
-        subprocess.run(
-            ["claude", "mcp", "remove", name, "-s", scope],
-            check=True,
-            capture_output=True,
-            text=True,
-            timeout=30,
-        )
-        return True
-    except subprocess.CalledProcessError as exc:
-        output = f"{exc.stderr or ''}\n{exc.stdout or ''}"
-        if _is_missing_mcp_server_output(output):
-            return False
-        raise RuntimeError(f"Failed to remove MCP server '{name}' via claude CLI.") from exc
-
-
 def add_codex_mcp_server(name: str, argv: list[str]) -> None:
     # `--` fences the proxy argv off from codex's own flag parser, registering
     # it as a stdio server (codex spawns the command and speaks MCP over it).
@@ -378,9 +280,11 @@ def configure_client_mcp_server(
     ):
         if client == "claude":
             removed_scopes = [
-                scope for scope in MCP_CLEANUP_SCOPES if remove_claude_mcp_server(name, scope)
+                scope
+                for scope in MCP_CLEANUP_SCOPES
+                if claude.remove_claude_mcp_server(name, scope)
             ]
-            add_claude_http_mcp_server(name, url, client_id=oauth_client)
+            claude.add_claude_http_mcp_server(name, url, client_id=oauth_client)
             return removed_scopes
         if client == "cursor":
             removed = cursor.write_http_mcp_server_config(name, url, client_id=oauth_client)
@@ -394,9 +298,9 @@ def configure_client_mcp_server(
     argv = build_mcp_proxy_argv(url, workspace, profile, use_pat=use_pat)
     if client == "claude":
         removed_scopes = [
-            scope for scope in MCP_CLEANUP_SCOPES if remove_claude_mcp_server(name, scope)
+            scope for scope in MCP_CLEANUP_SCOPES if claude.remove_claude_mcp_server(name, scope)
         ]
-        add_claude_mcp_server(name, argv, MCP_USER_SCOPE, always_load=always_load)
+        claude.add_claude_mcp_server(name, argv, MCP_USER_SCOPE, always_load=always_load)
         return removed_scopes
     if client == "codex":
         removed = remove_codex_mcp_server(name)
@@ -420,7 +324,9 @@ def configure_client_mcp_server(
 
 def remove_client_mcp_server(client: str, name: str) -> list[str]:
     if client == "claude":
-        return [scope for scope in MCP_CLEANUP_SCOPES if remove_claude_mcp_server(name, scope)]
+        return [
+            scope for scope in MCP_CLEANUP_SCOPES if claude.remove_claude_mcp_server(name, scope)
+        ]
     if client == "codex":
         return [MCP_USER_SCOPE] if remove_codex_mcp_server(name) else []
     if client == "gemini":