diff --git a/README.md b/README.md index 217222712..82ef67bbb 100644 --- a/README.md +++ b/README.md @@ -201,10 +201,10 @@ subscription token and Databricks gateway token on macOS, Windows, and Linux. ## MCP Servers -Register Databricks MCP servers for configured MCP-capable agents. Cursor Agent -is MCP-only and is included when `cursor-agent` is installed: +Use `ug mcp add` to register Databricks MCP servers for coding agents. Cursor +Agent supports MCP only and requires `cursor-agent` on PATH. -Run `ug mcp add` without selectors to browse a table of services by name, UC location, +Run `ug mcp add` without options to browse services by name, UC location, and status. ✓ means configured (or managed by the workspace); ✗ means not configured. Configured and managed services stay visible as read-only rows. `[ ]`/`[x]` indicate what you select for this operation; `[-]` marks a read-only row. @@ -224,11 +224,15 @@ ug mcp add --agents cursor --names system.ai.github --workspace https://first.da ug mcp add --agents codex --names system.ai.github --profile DEFAULT ``` -With `--agents`, installed agents can be registered for MCP without configuring their -models. `--workspace` or `--profile` selects the workspace on a fresh setup. If another -workspace is already active, run `ug configure --workspace ` to switch first; -the switch replaces the agent's active MCP registrations with those saved for the -selected workspace. Switching back restores the earlier workspace's saved servers. +`--agents` adds MCP servers without changing an agent's models. Claude Code and +Codex work even if their CLI is not on PATH: UG writes their user MCP config +directly. If that file already exists, UG detects the client without its CLI. +This works on macOS, Windows, and Linux; other agents need their CLI on PATH. +Without a CLI, status shows the saved config but cannot check the connection. + +`--workspace` or `--profile` selects the workspace on a fresh setup. To switch +an existing setup, run `ug configure --workspace ` first. The switch uses +that workspace's saved MCP servers; switching back restores the previous ones. Remove configured servers: diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index cd4f4c4bf..b6b99cf55 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -1059,8 +1059,8 @@ def write_user_mcp_servers(add: dict[str, dict], remove: set[str]) -> set[str]: developer's own servers and every other key in the file are preserved. Returns the subset of ``remove`` names that were actually present (so callers can report only real removals). - If the file exists but can't be parsed as a JSON object, we must not clobber it, so we defer to - the per-server ``claude`` CLI (which edits the file in place) for exactly the changed entries.""" + If the file exists but can't be parsed as a JSON object, we must not clobber it. Defer to the + CLI when available; otherwise report the invalid config.""" removed: set[str] = set() def mutate(config: dict) -> bool: @@ -1074,7 +1074,12 @@ def mutate(config: dict) -> bool: config["mcpServers"] = servers return True - if not _update_claude_config(claude_code_config_path(), mutate): + path = claude_code_config_path() + if not _update_claude_config(path, mutate): + if shutil.which("claude") is None: + raise RuntimeError( + f"Claude Code config at {path} is invalid. Fix it or install the CLI." + ) for name in remove: # Clean every scope (not short-circuited), recording the name if any scope had it. if [scope for scope in MCP_CLEANUP_SCOPES if remove_claude_mcp_server(name, scope)]: diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 44d30d131..4c7fa4bbb 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -7,6 +7,7 @@ import os import plistlib import re +import shutil import signal import socket import sys @@ -1035,6 +1036,8 @@ def write_user_mcp_servers(add: dict[str, dict], remove: set[str]) -> set[str]: path = user_mcp_config_path() doc = _read_user_config_for_rewrite(path) if doc is None: + if shutil.which("codex") is None: + raise RuntimeError(f"Codex config at {path} is invalid. Fix it or install the CLI.") if any("url" in entry for entry in add.values()): raise RuntimeError( f"Cannot register native OAuth MCP server: Codex config is invalid TOML at {path}." diff --git a/src/ucode/cli.py b/src/ucode/cli.py index ac0c48e37..0e7691bbb 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -138,6 +138,7 @@ resolve_state, ) from ucode.mcp import ( + CONFIG_WRITABLE_MCP_CLIENTS, MCP_CLIENTS, McpServiceListingRateLimited, add_mcp_command, @@ -1965,7 +1966,8 @@ def _configure_agents_for_mcp( """Select MCP clients for one workspace without changing agent model settings.""" scope = {a if a == "cursor" else normalize_tool(a) for a in requested} installed = available_mcp_clients() - not_installed = sorted(scope - set(installed)) + # A named client whose user MCP config UG writes directly needs no CLI on PATH. + not_installed = sorted(scope - set(installed) - CONFIG_WRITABLE_MCP_CLIENTS.keys()) if not_installed: raise RuntimeError(_not_installed_message(not_installed)) previous_workspace = current_workspace_key(load_full_state()) @@ -2072,7 +2074,8 @@ def mcp_add( typer.Option( "--agents", help="Comma-separated coding agents to register the server(s) for (e.g. " - "claude,codex,cursor). MCP registration is independent of model setup. Without --agents, " + "claude,codex,cursor). Claude/Codex user config can be written even without a CLI on " + "PATH. MCP registration is independent of model setup. Without --agents, " "the server is registered for every already-configured agent.", ), ] = None, @@ -2095,11 +2098,11 @@ def mcp_add( ), ] = None, ) -> None: - """Add Databricks MCP servers to installed coding tools. + """Add Databricks MCP servers to supported coding tools. Purely additive: it never removes MCP servers that are already configured, only registers new - ones (use `ug mcp remove` to remove). Pass --agents to target (and, if needed, set up) specific - agents. Explicit MCP setup never configures agent models or machine-wide settings. + ones (use `ug mcp remove` to remove). Pass --agents to target specific clients. Explicit MCP + setup never configures agent models or machine-wide settings. """ selected = None if names is None else {s.strip() for s in names.split(",") if s.strip()} requested_agents = ( diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 57c647e7a..dd7d26cf5 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -9,6 +9,7 @@ import threading from collections.abc import Callable, Mapping from concurrent.futures import ThreadPoolExecutor, as_completed +from pathlib import Path from typing import Any from urllib.parse import urlparse @@ -124,6 +125,12 @@ "list_command": "agy mcp list", }, } +# These clients read their user MCP files on every supported OS. UG can write those files +# directly, including when a desktop app exists without a CLI on PATH. +CONFIG_WRITABLE_MCP_CLIENTS: dict[str, Callable[[], Path]] = { + "claude": lambda: claude.claude_code_config_path(), + "codex": lambda: codex.user_mcp_config_path(), +} SKILLS_MCP_KIND = "skills" SKILLS_MCP_SERVER_NAME = "databricks-skill-registry" SKILL_LOCATIONS_BY_CLIENT_KEY = "skill_locations_by_client" @@ -255,7 +262,21 @@ def remove_gemini_mcp_server(name: str) -> bool: def available_mcp_clients() -> list[str]: - return [client for client, spec in MCP_CLIENTS.items() if shutil.which(str(spec["binary"]))] + return [ + client + for client in MCP_CLIENTS + if _mcp_cli_available(client) or _config_only_mcp_client(client) + ] + + +def _mcp_cli_available(client: str) -> bool: + return shutil.which(str(MCP_CLIENTS[client]["binary"])) is not None + + +def _config_only_mcp_client(client: str) -> bool: + """A client with a readable user config and no CLI; presence is not live health.""" + path = CONFIG_WRITABLE_MCP_CLIENTS.get(client) + return path is not None and path().is_file() def configured_mcp_clients(state: dict, installed_clients: list[str]) -> list[str]: @@ -363,6 +384,20 @@ def configure_client_mcp_server( use_pat: bool = False, always_load: bool = False, ) -> list[str]: + # The desktop apps read these files on every OS. An explicitly selected client can be + # configured before its CLI is on PATH; the writer creates the user config if needed. + if client in CONFIG_WRITABLE_MCP_CLIENTS and not _mcp_cli_available(client): + entry = _managed_mcp_entry( + client, + url, + workspace, + profile, + use_pat=use_pat, + always_load=always_load, + http_client=_oauth_http_client(client, workspace, use_pat=use_pat), + ) + removed = _MCP_CLIENT_MODULES[client].write_user_mcp_servers({name: entry}, {name}) + return [MCP_USER_SCOPE] if removed else [] # Connection-backed services use native HTTP OAuth for CIMD-capable agents. # Other cases keep the stdio proxy: no-login services, unrelated URLs, PAT, # unsupported agents, and workspaces without CIMD support. @@ -429,6 +464,9 @@ def configure_client_mcp_server( def remove_client_mcp_server(client: str, name: str) -> list[str]: + if client in CONFIG_WRITABLE_MCP_CLIENTS and not _mcp_cli_available(client): + removed = _MCP_CLIENT_MODULES[client].write_user_mcp_servers({}, {name}) + return [MCP_USER_SCOPE] if removed else [] if client == "claude": return [ scope for scope in MCP_CLEANUP_SCOPES if claude.remove_claude_mcp_server(name, scope) @@ -1944,12 +1982,18 @@ def setup_mcp_clients( purge_cross_workspace_mcp_residue(state, workspace) installed_clients = available_mcp_clients() - if not installed_clients: + explicit_direct = ( + set(state.get("mcp_clients") or []) & (agents or set()) & CONFIG_WRITABLE_MCP_CLIENTS.keys() + ) + if not installed_clients and not explicit_direct: raise RuntimeError( - "No supported MCP clients are installed. Install Claude, Codex, Gemini, OpenCode, " - "or GitHub Copilot CLI." + "No supported MCP clients are available. Install a client CLI or select " + "Claude Code or Codex with `ug mcp add --agents`." ) clients = configured_mcp_clients(state, installed_clients) + clients.extend( + client for client in MCP_CLIENTS if client in explicit_direct and client not in clients + ) if agents is not None: missing = sorted(a for a in agents if a not in clients) if missing: @@ -2607,9 +2651,31 @@ def _run_mcp_list(client: str) -> str | None: def query_live_mcp_status(client: str) -> dict[str, str]: - """MCP ``{server_name: state}`` for one installed agent, including ug's Copilot config.""" - output = _run_mcp_list(client) + """MCP ``{server_name: state}`` for one client, including config-only status.""" + config_only = _config_only_mcp_client(client) and not _mcp_cli_available(client) + output = None if config_only else _run_mcp_list(client) statuses = parse_mcp_list_output(client, output) if output is not None else {} + if config_only: + # A config-only client has no CLI to probe. Report config presence, not connection + # health. Codex exposes a local enabled bit; Claude's connection remains unknown. + if client == "claude": + servers = read_json_safe(claude.claude_code_config_path()).get("mcpServers") + managed = claude.read_managed_mcp_urls() + else: + servers = read_toml_safe(codex.user_mcp_config_path()).get("mcp_servers") + managed = codex.read_managed_mcp_urls() + if isinstance(servers, Mapping): + for name, entry in servers.items(): + state = LIVE_UNKNOWN + if client == "codex": + state = ( + LIVE_DISABLED + if isinstance(entry, Mapping) and entry.get("enabled") is False + else LIVE_ENABLED + ) + statuses.setdefault(name, state) + for name in managed: + statuses.setdefault(name, LIVE_UNKNOWN) if client == "copilot": # Copilot reads ug's additional MCP config only on an agent launch. Its bare `mcp list` # omits these servers, so report their configured state without claiming a live probe. @@ -2841,8 +2907,18 @@ def list_mcp_command(agents: set[str] | None = None) -> int: if other_summary: print_note(f"Other MCP servers not configured by ug — {other_summary}.") - live_note = "Status is from each agent's `mcp list`" - if "codex" in probe_clients: + config_only = { + client + for client in probe_clients + if _config_only_mcp_client(client) and not _mcp_cli_available(client) + } + live_note = ( + "Status uses each CLI's `mcp list` when available; config-only status is from config, " + "not live health" + if config_only + else "Status is from each agent's `mcp list`" + ) + if "codex" in probe_clients and "codex" not in config_only: live_note += "; Codex reports enabled/disabled" if "copilot" in probe_clients: live_note += "; ug-added Copilot servers report configured/enabled, not live health" diff --git a/tests/README.md b/tests/README.md index ff4bacfb3..a3aec0443 100644 --- a/tests/README.md +++ b/tests/README.md @@ -75,6 +75,11 @@ native installer, `agy mcp list` parsing, `ug antigravity`/`ug agy`, and eight journeys in `integration/` (blocking Linux CI lane, Databricks Hosted and the Gemini Enterprise MPS; see `integration/README.md`). +`test_mcp.py` and `test_cli.py` cover Claude/Codex MCP setup without their CLIs: +finding an existing config, creating one with `--agents`, and reporting status +from the file. They also check malformed config errors and stale client choices. +They use temporary files; real desktop apps remain untested. + `test_integration_contract.py` checks that isolated test sessions set Antigravity's `AGY_CLI_DISABLE_AUTO_UPDATE` to the required literal `true`, overriding an inherited `1`. This component check does not run the native updater; the integration runner retains its diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 233b4206e..250dbdf0a 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -4495,6 +4495,7 @@ def test_unparseable_file_falls_back_to_cli_and_does_not_clobber(self, tmp_path, path = tmp_path / ".claude.json" path.write_text("{ this is not valid json") monkeypatch.setattr(claude, "CLAUDE_CODE_CONFIG_PATH", path) + monkeypatch.setattr(claude.shutil, "which", lambda binary: "/fake/claude") added: list[tuple[str, object]] = [] removed: list[tuple[str, str]] = [] monkeypatch.setattr(claude, "add_claude_mcp_server", lambda n, e, s: added.append((n, e))) diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index c061f3a55..3ddc40ac7 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -2699,6 +2699,7 @@ def test_unparseable_file_falls_back_to_cli_and_does_not_clobber(self, tmp_path, path = tmp_path / "config.toml" path.write_text("this is = = not valid toml [[[") monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", path) + monkeypatch.setattr(codex.shutil, "which", lambda binary: "/fake/codex") added: list[tuple[str, list]] = [] removed: list[str] = [] import ucode.mcp as mcp_mod @@ -2722,6 +2723,7 @@ def test_unparseable_file_does_not_start_native_oauth_login(self, tmp_path, monk path = tmp_path / "config.toml" path.write_text("this is = = not valid toml [[[", encoding="utf-8") monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", path) + monkeypatch.setattr(codex.shutil, "which", lambda binary: "/fake/codex") with pytest.raises(RuntimeError, match="invalid TOML"): codex.write_user_mcp_servers( {"github": codex.managed_mcp_http_entry("https://ws/mcp", "https://client/id")}, diff --git a/tests/test_cli.py b/tests/test_cli.py index eb5760ef8..6db1c286a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3274,14 +3274,16 @@ def test_missing_mcp_only_cli_errors_instead_of_prompting(self): mock_prompt.assert_not_called() - def test_agent_still_unready_after_setup_errors(self): + def test_other_clients_without_cli_still_fail_before_prompt(self): with ( patch("ucode.cli.load_state", return_value={}), patch("ucode.cli.available_mcp_clients", return_value=[]), - patch("ucode.cli.configure_workspace_command"), - pytest.raises(RuntimeError, match="Not installed or not set up: Codex"), + patch("ucode.cli._prompt_for_configuration") as prompt, + pytest.raises(RuntimeError, match="Not installed or not set up: Gemini CLI"), ): - cli_mod._configure_agents_for_mcp(["codex"]) + cli_mod._configure_agents_for_mcp(["gemini"]) + + prompt.assert_not_called() class TestMcpAddWorkspace: @@ -3360,6 +3362,39 @@ def _real_state_writes(self): def _save_workspace(workspace: str, **extra) -> None: real_save_state({"workspace": workspace, "available_tools": ["codex"], **extra}) + @pytest.mark.parametrize("agent", ["claude", "codex"]) + def test_explicit_config_writable_client_needs_no_cli(self, agent): + from ucode.mcp import setup_mcp_clients + from ucode.state import load_state + + selected = [] + + def register(**kwargs): + selected.append( + setup_mcp_clients( + load_state(), + "MCP servers", + agents=kwargs["agents"], + require_auth=False, + purge_foreign=False, + )[2] + ) + + with ( + patch("ucode.cli.available_mcp_clients", return_value=[]), + patch("ucode.mcp.available_mcp_clients", return_value=[]), + patch("ucode.cli.add_mcp_command", side_effect=register), + patch("ucode.cli._sync_claude_cowork_plugin"), + ): + result = runner.invoke( + app, + ["mcp", "add", "--agents", agent, "--names", "a.b.c", "--workspace", self.WS_B], + ) + + assert result.exit_code == 0, result.output + assert selected == [[agent]] + assert load_state()["mcp_clients"] == [agent] + def test_first_mcp_add_selects_its_workspace(self): from ucode.state import load_full_state diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 57ec834a6..7791286a6 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re import threading from unittest.mock import MagicMock @@ -110,6 +111,95 @@ def test_use_pat_appends_flag_and_profile_optional(self): assert "--profile" not in no_profile +class TestConfigOnlyMcpClients: + @pytest.mark.parametrize("client", ["claude", "codex"]) + def test_detects_existing_config_without_cli(self, tmp_path, monkeypatch, client): + monkeypatch.setattr(mcp.shutil, "which", lambda binary: None) + path = tmp_path / (".claude.json" if client == "claude" else "config.toml") + if client == "claude": + monkeypatch.setattr(mcp.claude, "claude_code_config_path", lambda: path) + else: + monkeypatch.setattr(mcp.codex, "user_mcp_config_path", lambda: path) + path.write_text("{}" if client == "claude" else "") + + assert client in mcp.available_mcp_clients() + assert client in mcp.configured_mcp_clients( + {"available_tools": [client]}, mcp.available_mcp_clients() + ) + + path.unlink() + assert client not in mcp.available_mcp_clients() + + @pytest.mark.parametrize("client", ["claude", "codex"]) + def test_config_only_registration_creates_file_and_reports_no_live_health( + self, tmp_path, monkeypatch, client + ): + monkeypatch.setattr(mcp.shutil, "which", lambda binary: None) + monkeypatch.setattr(mcp, "_oauth_http_client", lambda *a, **kw: None) + monkeypatch.setattr(mcp, "_native_oauth_http_entry", lambda *a, **kw: None) + monkeypatch.setattr(mcp.claude, "read_managed_mcp_urls", lambda: {}) + monkeypatch.setattr(mcp.codex, "read_managed_mcp_urls", lambda: {}) + monkeypatch.setattr(mcp.subprocess_cross_os, "run", lambda *a, **kw: pytest.fail("CLI ran")) + monkeypatch.setattr(mcp, "_run_mcp_list", lambda selected: None) + if client == "claude": + path = tmp_path / ".claude.json" + monkeypatch.setattr(mcp.claude, "claude_code_config_path", lambda: path) + else: + path = tmp_path / "config.toml" + monkeypatch.setattr(mcp.codex, "user_mcp_config_path", lambda: path) + + assert mcp.configure_client_mcp_server(client, "github", GH_URL, WS, "p") == [] + assert path.exists() + status = mcp.query_live_mcp_status(client) + assert status["github"] == (mcp.LIVE_UNKNOWN if client == "claude" else mcp.LIVE_ENABLED) + if client == "codex": + path.write_text(path.read_text() + "enabled = false\n") + assert mcp.query_live_mcp_status(client)["github"] == mcp.LIVE_DISABLED + assert mcp.remove_client_mcp_server(client, "github") == [mcp.MCP_USER_SCOPE] + assert "github" not in mcp.query_live_mcp_status(client) + + @pytest.mark.parametrize( + ("client", "invalid"), + [("claude", "[]"), ("codex", 'command = "x')], + ) + @pytest.mark.parametrize("action", ["add", "remove"]) + def test_invalid_config_without_cli_reports_error_and_preserves_file( + self, tmp_path, monkeypatch, client, invalid, action + ): + monkeypatch.setattr(mcp.shutil, "which", lambda binary: None) + monkeypatch.setattr(mcp, "_oauth_http_client", lambda *args, **kwargs: None) + monkeypatch.setattr( + mcp.subprocess_cross_os, "run", lambda *args, **kwargs: pytest.fail("CLI ran") + ) + path = tmp_path / (".claude.json" if client == "claude" else "config.toml") + if client == "claude": + monkeypatch.setattr(mcp.claude, "claude_code_config_path", lambda: path) + else: + monkeypatch.setattr(mcp.codex, "user_mcp_config_path", lambda: path) + path.write_text(invalid) + + with pytest.raises(RuntimeError, match=rf"{re.escape(str(path))} is invalid"): + if action == "add": + mcp.configure_client_mcp_server(client, "github", GH_URL, WS, "p") + else: + mcp.remove_client_mcp_server(client, "github") + + assert path.read_text() == invalid + + def test_saved_client_without_cli_or_config_needs_explicit_selection(self, monkeypatch): + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: []) + state = {"workspace": WS, "mcp_clients": ["codex"]} + + assert mcp.configured_mcp_clients(state, []) == [] + with pytest.raises(RuntimeError, match="No supported MCP clients are available"): + mcp.setup_mcp_clients(state, "MCP servers", require_auth=False, purge_foreign=False) + + _, _, clients = mcp.setup_mcp_clients( + state, "MCP servers", agents={"codex"}, require_auth=False, purge_foreign=False + ) + assert clients == ["codex"] + + class TestAddCodexMcpServer: def test_registers_stdio_proxy_command(self, monkeypatch): calls: list[dict] = [] @@ -347,6 +437,8 @@ def test_configures_copilot_with_proxy_argv(self, monkeypatch): def _capture_claude(self, monkeypatch, *, claude_code_available: bool): http_calls: list[tuple[str, str]] = [] proxy_calls: list[tuple[str, list[str]]] = [] + # These tests exercise CLI registration; CI need not have Claude on PATH. + monkeypatch.setattr(mcp, "_mcp_cli_available", lambda client: True) monkeypatch.setattr(mcp, "workspace_supports_cimd", lambda ws: claude_code_available) monkeypatch.setattr( mcp, "oauth_client_available", lambda ws, client_id: claude_code_available @@ -4571,6 +4663,15 @@ def test_codex_caveat_shown_only_when_codex_reported(self, monkeypatch, capsys): assert mcp.list_mcp_command() == 0 assert "Codex reports enabled/disabled" in _unwrap(capsys.readouterr().out) + def test_config_only_status_note_does_not_claim_live_probe(self, monkeypatch, capsys): + self._patch(monkeypatch, installed=("claude", "codex")) + monkeypatch.setattr(mcp.shutil, "which", lambda binary: None) + monkeypatch.setattr(mcp, "_config_only_mcp_client", lambda client: True) + assert mcp.list_mcp_command() == 0 + out = _unwrap(capsys.readouterr().out) + assert "config-only status is from config, not live health" in out + assert "Codex reports enabled/disabled" not in out + def test_codex_caveat_omitted_when_scoped_out(self, monkeypatch, capsys): # Scoping Codex out drops the Codex clause from the note. self._patch(monkeypatch, live={"claude": {}, "codex": {}})