diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index b5911fe56..77d02654b 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -478,6 +478,63 @@ jobs: ${{ runner.temp }}/ug-integration/artifacts/ ${{ runner.temp }}/ug-integration/wheels/ + # Non-blocking like the Copilot lane: continue-on-error and absent from the cujs `needs`. + # Runs the Copilot managed_fixture cases on the managed e2e workspace with injected configs that + # enable Copilot (a service principal authenticates it, like the `managed` lane). + managed-copilot: + name: Managed config · Copilot (non-blocking) + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} + continue-on-error: true + runs-on: ubuntu-22.04 + timeout-minutes: 30 + env: + UCODE_TEST_WORKSPACE: ${{ secrets.E2E_ADMIN_WORKSPACE }} + DATABRICKS_CLIENT_ID: ${{ secrets.E2E_ADMIN_SP_CLIENT_ID }} + DATABRICKS_CLIENT_SECRET: ${{ secrets.E2E_ADMIN_SP_CLIENT_SECRET }} + DEPENDENCY: ${{ inputs.dependency }} + TEST_MARKER: managed_fixture and copilot + ARTIFACT_NAME: integration-managed-copilot + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ inputs.ref || github.sha }} + persist-credentials: false + fetch-depth: 0 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 22.19.0 + - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + with: + version: 0.9.8 + enable-cache: ${{ inputs.ref == '' }} + - uses: databricks/setup-cli@bdb89f81c11a5bd647fd55b585b7c396ec68a25a # v1.0.0 + with: + version: 1.17.0 + - name: Run the Copilot managed cases against the managed workspace + shell: bash + run: | + args=(--copilot-version "$COPILOT_VERSION") + if [[ -n "$DEPENDENCY" ]]; then + args+=(--dependency "$DEPENDENCY") + fi + uv run --no-project --python 3.12 python scripts/run_integration.py \ + --python 3.12 --ug-version "$UG_VERSION" --entry-point "$ENTRY_POINT" \ + --default-index "$PACKAGE_INDEX" --output "$RUNNER_TEMP/ug-integration" \ + "${args[@]}" -- -m "$TEST_MARKER" + - name: Upload managed Copilot evidence + if: ${{ !cancelled() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: ${{ env.ARTIFACT_NAME }} + include-hidden-files: true + path: | + ${{ runner.temp }}/ug-integration/*.json + ${{ runner.temp }}/ug-integration/*.txt + ${{ runner.temp }}/ug-integration/*.xml + ${{ runner.temp }}/ug-integration/*.log + ${{ runner.temp }}/ug-integration/artifacts/ + ${{ runner.temp }}/ug-integration/wheels/ + # managed and dedicated-cuj stay Linux-only: ug has no Windows managed-settings # writer, and the CUJ suite drives agents through a POSIX PTY. managed: diff --git a/README.md b/README.md index f3eb06598..7cf0f1f52 100644 --- a/README.md +++ b/README.md @@ -508,7 +508,7 @@ preferences are ignored and their files are left untouched. See the bundled | Claude Code | `~/.claude/ucode-settings.json`, `~/.claude.json`, `/etc/claude-code/managed-settings.json` (Linux), `/Library/Application Support/ClaudeCode/managed-settings.json` (macOS), `C:\Program Files\ClaudeCode\managed-settings.json` (Windows, written only from an elevated terminal) | | Gemini CLI | `~/.gemini/ucode.env`, `~/.ucode/.gemini-home/.gemini/settings.json` | | OpenCode | `~/.ucode/opencode-xdg/opencode/opencode.json`, `~/.ucode/opencode-xdg/opencode/plugin/ucode-auth.js` | -| GitHub Copilot CLI | `~/.copilot/ucode.env`, `~/.copilot/ucode-mcp-config.json` | +| GitHub Copilot CLI | `~/.copilot/ucode.env`, `~/.copilot/mcp-config.json` (or `$COPILOT_HOME/mcp-config.json`) | | Pi | `~/.ucode/pi-home/.pi/agent/models.json`, `~/.ucode/pi-home/.pi/agent/settings.json` | | Antigravity CLI | `~/.gemini/config/skills.json` (adds `~/.agents/skills`), `~/.gemini/config/mcp_config.json` (ug's MCP servers only); the gateway is set per launch through `AGY_LLM_GATEWAY_*` variables | | Cursor Agent | `~/.cursor/mcp.json` | diff --git a/src/ucode/agents/copilot.py b/src/ucode/agents/copilot.py index 614f6c7a4..c5909ecc0 100644 --- a/src/ucode/agents/copilot.py +++ b/src/ucode/agents/copilot.py @@ -29,6 +29,7 @@ import os import re import secrets +import subprocess import threading import time from collections.abc import Callable, Iterable, Iterator @@ -41,8 +42,8 @@ from ucode.config_io import ( APP_DIR, ToolSpec, - apply_json_mcp_diff, backup_existing_file, + is_dry_run, parse_dotenv, read_json_safe, write_dotenv, @@ -88,9 +89,11 @@ COPILOT_CONFIG_DIR = Path.home() / ".copilot" COPILOT_ENV_PATH = COPILOT_CONFIG_DIR / "ucode.env" -COPILOT_MCP_CONFIG_PATH = COPILOT_CONFIG_DIR / "ucode-mcp-config.json" +COPILOT_MCP_CONFIG_PATH = COPILOT_CONFIG_DIR / "mcp-config.json" COPILOT_BACKUP_PATH = APP_DIR / "copilot-ucode-env.backup" -COPILOT_MCP_BACKUP_PATH = APP_DIR / "copilot-ucode-mcp-config.backup.json" +# Older ug versions kept their servers in a side file loaded with `--additional-mcp-config`. +LEGACY_MCP_CONFIG_PATH = COPILOT_CONFIG_DIR / "ucode-mcp-config.json" +LEGACY_MCP_BACKUP_PATH = APP_DIR / "copilot-ucode-mcp-config.backup.json" COPILOT_PROXY_LOG_PATH = APP_DIR / "logs" / "copilot-proxy.log" _REGISTRY_PREFIX = "ug-providers-" _REGISTRY_LOCK_NAME = "ug-providers.lock" @@ -776,35 +779,128 @@ def build_mcp_server_entry(argv: list[str]) -> dict: } -def write_mcp_server_config(name: str, argv: list[str]) -> bool: - backup_existing_file(COPILOT_MCP_CONFIG_PATH, COPILOT_MCP_BACKUP_PATH) - existing = read_json_safe(COPILOT_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) - existing["mcpServers"] = mcp_servers - write_json_file(COPILOT_MCP_CONFIG_PATH, existing) - return removed +def copilot_mcp_config_path() -> Path: + """The user config Copilot reads ``mcpServers`` from (and ``copilot mcp add`` writes).""" + copilot_home = os.environ.get("COPILOT_HOME") + return Path(copilot_home) / "mcp-config.json" if copilot_home else COPILOT_MCP_CONFIG_PATH -def remove_mcp_server_config(name: str) -> bool: - existing = read_json_safe(COPILOT_MCP_CONFIG_PATH) - mcp_servers = existing.get("mcpServers") - if not isinstance(mcp_servers, dict) or name not in mcp_servers: +def _read_mcp_config_for_rewrite(path: Path) -> dict | None: + """``path`` parsed for a rewrite: ``{}`` when absent, ``None`` when it isn't a JSON object, so + a caller never overwrites it. Copilot also accepts JSON with comments, which lands here.""" + if not path.exists(): + return {} + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeError, json.JSONDecodeError): + return None + return data if isinstance(data, dict) else None + + +def remove_legacy_mcp_config() -> bool: + """Delete the legacy side file and its backup (everything in them is ug's).""" + existed = LEGACY_MCP_CONFIG_PATH.exists() or LEGACY_MCP_BACKUP_PATH.exists() + if is_dry_run(): + return existed + for path in (LEGACY_MCP_CONFIG_PATH, LEGACY_MCP_BACKUP_PATH): + try: + path.unlink(missing_ok=True) + except OSError as exc: + raise RuntimeError(f"Failed to remove {path}") from exc + return existed + + +def _read_legacy_mcp_servers() -> dict[str, dict]: + """The servers in the legacy side file. Every entry there was written by ug, so it moves into + the user config as-is.""" + servers = read_json_safe(LEGACY_MCP_CONFIG_PATH).get("mcpServers") + if not isinstance(servers, dict): + return {} + return {name: entry for name, entry in servers.items() if isinstance(entry, dict)} + + +def _remove_mcp_server_via_cli(name: str) -> bool: + try: + result = subprocess_cross_os.run( + [SPEC["binary"], "mcp", "remove", name], + check=False, + capture_output=True, + text=True, + timeout=30, + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError(f"Timed out removing MCP server '{name}' via copilot CLI.") from exc + if "not found" in f"{result.stderr or ''}\n{result.stdout or ''}".lower(): return False - mcp_servers.pop(name) - existing["mcpServers"] = mcp_servers - write_json_file(COPILOT_MCP_CONFIG_PATH, existing) + if result.returncode != 0: + raise RuntimeError(f"Failed to remove MCP server '{name}' via copilot CLI.") return True +def _add_mcp_server_via_cli(name: str, entry: dict) -> None: + # The CLI refuses an existing name, so a replace removes it first. + _remove_mcp_server_via_cli(name) + argv = [entry["command"], *entry.get("args", [])] + try: + subprocess_cross_os.run( + [SPEC["binary"], "mcp", "add", name, "--", *argv], + check=True, + capture_output=True, + text=True, + timeout=30, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc: + raise RuntimeError(f"Failed to add MCP server '{name}' via copilot CLI.") from exc + + +def _apply_mcp_diff(add: dict[str, dict], remove: set[str]) -> set[str]: + path = copilot_mcp_config_path() + config = _read_mcp_config_for_rewrite(path) + legacy = _read_legacy_mcp_servers() + upserts = {name: entry for name, entry in legacy.items() if name not in remove} | add + if config is None: + # Defer to the copilot CLI, which reads the file the way Copilot does, for exactly the + # changed entries. + removed = {name for name in remove if _remove_mcp_server_via_cli(name) or name in legacy} + for name, entry in upserts.items(): + _add_mcp_server_via_cli(name, entry) + remove_legacy_mcp_config() + return removed + + servers = config.get("mcpServers") + if not isinstance(servers, dict): + servers = {} + removed = {name for name in remove if name in servers or name in legacy} + if upserts or removed: + for name in remove: + servers.pop(name, None) + servers.update(upserts) + config["mcpServers"] = servers + write_json_file(path, config) + remove_legacy_mcp_config() + return removed + + +def migrate_legacy_mcp_config() -> None: + """Move the servers in the legacy side file into the user config, once.""" + if LEGACY_MCP_CONFIG_PATH.exists() or LEGACY_MCP_BACKUP_PATH.exists(): + _apply_mcp_diff({}, set()) + + +def write_mcp_server_config(name: str, argv: list[str]) -> bool: + """Add (or replace) a stdio (`ug mcp-proxy`) MCP server in Copilot's user config. Returns + True when an entry of that name was replaced.""" + return bool(_apply_mcp_diff({name: build_mcp_server_entry(argv)}, {name})) + + +def remove_mcp_server_config(name: str) -> bool: + return bool(_apply_mcp_diff({}, {name})) + + def write_user_mcp_servers(add: dict[str, dict], remove: set[str]) -> set[str]: - """Apply ``add``/``remove`` to Copilot's `mcpServers` in a single read-modify-write. Returns the names actually removed.""" - return apply_json_mcp_diff( - COPILOT_MCP_CONFIG_PATH, "mcpServers", add, remove, backup_path=COPILOT_MCP_BACKUP_PATH - ) + """Apply ``add``/``remove`` to Copilot's user-config ``mcpServers`` in a single + read-modify-write. Returns the names actually removed.""" + return _apply_mcp_diff(add, remove) def scrub_stored_credentials() -> None: @@ -927,6 +1023,10 @@ def launch(state: dict, tool_args: list[str], *, options: LaunchOptions) -> None managed_list=not (provider_service or model_location), ) scrub_stored_credentials() + try: + migrate_legacy_mcp_config() + except RuntimeError as exc: + print_warning(f"ug's MCP servers aren't available in this session: {exc}") # Only Copilot launches need the proxy (and its `httpx` import). from ucode import gateway_proxy @@ -962,7 +1062,7 @@ def launch(state: dict, tool_args: list[str], *, options: LaunchOptions) -> None with session_registry(config) as registry_path: env = build_runtime_env(registry_path, start_model) proc = subprocess_cross_os.popen( - [SPEC["binary"], *mcp_config_args(), *tool_args], env=env + [SPEC["binary"], *MCP_LAUNCH_ARGS, *tool_args], env=env ) returncode = _wait_for_exit(proc) finally: @@ -976,14 +1076,13 @@ def launch(state: dict, tool_args: list[str], *, options: LaunchOptions) -> None def validate_cmd(binary: str) -> list[str]: return [ binary, - *mcp_config_args(), + *MCP_LAUNCH_ARGS, "--prompt", "say hi in 5 words or less", "--allow-all-tools", ] -def mcp_config_args() -> list[str]: - if not COPILOT_MCP_CONFIG_PATH.exists(): - return [] - return ["--additional-mcp-config", f"@{COPILOT_MCP_CONFIG_PATH}"] +# Offline mode empties Copilot's allowlist of servers whose `instructions` reach the model, so +# without this the gateway's servers lose theirs. +MCP_LAUNCH_ARGS = ("--allow-all-mcp-server-instructions",) diff --git a/src/ucode/config_io.py b/src/ucode/config_io.py index c4ca3578c..85ce794ac 100644 --- a/src/ucode/config_io.py +++ b/src/ucode/config_io.py @@ -185,7 +185,7 @@ def apply_json_mcp_diff( ) -> set[str]: """Apply an ``add``/``remove`` MCP-server diff to the ``key`` map in a JSON config, in one read-modify-write. Shared by the agents whose MCP servers live in a JSON object keyed by name - (copilot/cursor/gemini/opencode); the developer's own entries and every other key are kept. + (cursor/gemini/kilo/opencode); the developer's own entries and every other key are kept. ``backup_path`` snapshots the file first when the agent keeps a revert backup. Returns the subset of ``remove`` names that were actually present (so callers can report only real removals).""" if backup_path is not None: diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index d441c345d..a244a1357 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import os import re import shutil @@ -16,7 +17,7 @@ from rich.table import Table from ucode.agents import antigravity, claude, codex, copilot, cursor, gemini, kilo, opencode -from ucode.config_io import read_json_safe, read_toml_safe, restore_file +from ucode.config_io import read_json_safe, read_toml_safe from ucode.constants import MCP_CLEANUP_SCOPES, MCP_USER_SCOPE from ucode.databricks import ( AIGW_MCP_SERVICES_SEGMENT, @@ -110,7 +111,7 @@ "copilot": { "binary": "copilot", "display": "GitHub Copilot CLI", - "list_command": "copilot mcp list", + "list_command": "copilot mcp list --json", }, "cursor": { "binary": "cursor-agent", @@ -456,6 +457,12 @@ def revert_mcp_configs(state: dict) -> dict[str, bool]: all_servers = list(state.get("mcp_servers") or []) + list( state.get("managed_mcp_servers") or [] ) + # One agent's unreadable config must not stop the revert of the others. + try: + if copilot.remove_legacy_mcp_config(): + results["copilot"] = True + except RuntimeError as exc: + print_warning(f"Could not remove ug's old Copilot MCP file: {exc}") for server in all_servers: name = server.get("name") if not isinstance(name, str) or not name: @@ -463,19 +470,14 @@ def revert_mcp_configs(state: dict) -> dict[str, bool]: for client in server.get("clients") or []: if client not in MCP_CLIENTS: continue - removed_scopes = remove_client_mcp_server(client, name) + try: + removed_scopes = remove_client_mcp_server(client, name) + except RuntimeError as exc: + display = MCP_CLIENTS[client]["display"] + print_warning(f"Could not remove MCP server `{name}` from {display}: {exc}") + continue results[client] = bool(removed_scopes) or results.get(client, False) - # OpenCode MCP entries live in the normal OpenCode config and are restored - # by the main agent config revert. Copilot stores MCP servers separately, - # so restore its original MCP file after removing per-server entries above. - results["copilot"] = restore_file( - copilot.COPILOT_MCP_CONFIG_PATH, - copilot.COPILOT_MCP_BACKUP_PATH, - any( - "copilot" in (server.get("clients") or []) for server in state.get("mcp_servers") or [] - ), - ) or results.get("copilot", False) return results @@ -2367,10 +2369,11 @@ def remove_mcp_command(agents: set[str] | None = None) -> int: # `ug mcp list`: configured MCP servers + their live per-agent connection status # --------------------------------------------------------------------------- -# Per-(server, agent) states surfaced by `ug mcp list`. Agent listings differ: some -# health-check servers, Codex reports enabled/disabled, and Copilot's ug-added entries -# are visible only in the additional config file. NOT_REGISTERED means the agent is -# installed but neither its listing nor ug's additional config contains the server. +# Per-(server, agent) live states surfaced by `ug mcp list`. Every agent's `mcp list` +# health-checks its servers and reports connected/failed, except Codex, Antigravity and +# Copilot, whose listings report only whether an entry is enabled/disabled (no probe). +# NOT_REGISTERED means the agent is installed but doesn't list the server; a server maps to +# None (rendered "agent not installed") when the agent binary isn't installed at all. LIVE_CONNECTED = "connected" LIVE_FAILED = "failed" LIVE_ENABLED = "enabled" @@ -2510,6 +2513,28 @@ def _parse_tree_mcp_list(output: str) -> dict[str, str]: return statuses +def _parse_copilot_mcp_list(output: str) -> dict[str, str]: + """Parse `copilot mcp list --json` into ``{server_name: state}``. + + Like Codex, Copilot reports config state (``enabled``), not a health probe. Only the leading + JSON object is read, since stderr follows it; output without one contributes nothing.""" + start = output.find("{") + if start < 0: + return {} + try: + data, _ = json.JSONDecoder().raw_decode(output, start) + except json.JSONDecodeError: + return {} + servers = data.get("mcpServers") if isinstance(data, dict) else None + if not isinstance(servers, dict): + return {} + return { + name: LIVE_ENABLED if entry.get("enabled", True) else LIVE_DISABLED + for name, entry in servers.items() + if isinstance(entry, dict) + } + + def parse_mcp_list_output(client: str, output: str) -> dict[str, str]: """Parse an agent's `mcp list` output into ``{server_name: live-state}`` (best-effort). @@ -2521,6 +2546,8 @@ def parse_mcp_list_output(client: str, output: str) -> dict[str, str]: if client in ("codex", "antigravity"): # Both print a columnar table with an enabled/disabled state column. parsed = _parse_codex_mcp_list(output) + elif client == "copilot": + parsed = _parse_copilot_mcp_list(output) elif client in ("kilo", "opencode"): parsed = _parse_tree_mcp_list(output) else: @@ -2564,18 +2591,11 @@ 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.""" + """Live ``{server_name: state}`` for one installed agent (empty if its listing can't be read).""" output = _run_mcp_list(client) - statuses = parse_mcp_list_output(client, output) if output is not None else {} - 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. - servers = read_json_safe(copilot.COPILOT_MCP_CONFIG_PATH).get("mcpServers") - if isinstance(servers, dict): - for name, entry in servers.items(): - if isinstance(name, str) and isinstance(entry, dict): - statuses.setdefault(name, LIVE_ENABLED) - return statuses + if output is None: + return {} + return parse_mcp_list_output(client, output) def _query_live_statuses(clients: list[str]) -> dict[str, dict[str, str]]: @@ -2799,10 +2819,13 @@ def list_mcp_command(agents: set[str] | None = None) -> int: 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: - live_note += "; Codex reports enabled/disabled" - if "copilot" in probe_clients: - live_note += "; ug-added Copilot servers report configured/enabled, not live health" + # Only mention the enabled/disabled caveat for the agents actually in the reported set. + config_state_clients = [ + MCP_CLIENTS[client]["display"] for client in ("codex", "copilot") if client in probe_clients + ] + if config_state_clients: + verb = "reports" if len(config_state_clients) == 1 else "report" + live_note += f"; {' and '.join(config_state_clients)} {verb} enabled/disabled" print_note(f"{live_note}.") # A `needs sign-in` server is registered fine but needs the one-time per-user connection login; # point the developer at it rather than leaving an opaque status. diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 0093c805f..e2fbe07da 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -93,9 +93,10 @@ exercise. and run documentation at the suite root. - Current scope is basic Claude/Codex configuration, routing, script usage, command forwarding, and configure/revert CUJs, plus Copilot CLI headless - model tasks and one TUI model-switch journey (non-blocking lane), plus Antigravity CLI - configure, TUI, headless, External Models, MCP-tool, skill-directory, and revert journeys - (blocking Linux lane). Do not add other MCP/skills functionality, + model tasks, one TUI model-switch journey, and Copilot MCP/skills journeys matching the + Claude/Codex ones (non-blocking lanes), plus Antigravity CLI configure, TUI, headless, + External Models, MCP-tool, skill-directory, and revert journeys (blocking Linux lane). + Do not add other MCP/skills functionality, tracing, or the broad configure-option matrix without a new scope request. Existing `mcp --help` checks cover dispatch only. diff --git a/tests/README.md b/tests/README.md index d0559198f..d7438fc49 100644 --- a/tests/README.md +++ b/tests/README.md @@ -678,6 +678,8 @@ integration utilities; only CUJ-specific evidence correlation stays in a test fi | `test_ug_copilot_tui_switches_from_claude_to_gpt_with_model_command` | TUI: answer folder trust, Claude task, `/model dbx-responses/`, GPT task, `/exit` | Both answers sent as the Claude then GPT model; the session records the switch; normal exit; no session registry left (non-blocking CI lane) | | `test_ug_copilot_refreshes_a_revoked_token_mid_session`, `test_ug_copilot_reports_expired_sign_in` | Sign in through `DATABRICKS_BEARER_COMMAND` minting short-lived PATs; mid-task the agent revokes them (or also signs out) | Revoked: a token is minted after the revocation and the task completes without restarting Copilot. Signed out: output shows "Databricks sign-in expired" with `databricks auth login`; the failure is in `~/.ucode/logs/copilot-proxy.log`, not the terminal (non-blocking CI lane) | | `test_ug_configure_copilot_repeat_and_revert` | Configure Copilot twice over a user-owned `~/.copilot/mcp-config.json`, run a GPT task, revert twice | Task completes; no file holds the bearer; revert removes `~/.copilot/ucode.env`, leaves the user file byte-identical and reports Not Configured. (non-blocking CI lane) | +| `test_ug_mcp_add_copilot_registers_server_in_copilot_config` | Over a developer-owned, disabled server in `~/.copilot/mcp-config.json`, configure Copilot, `ug mcp add --agents copilot --names system.ai.web_search`, run a headless task that calls the tool, then revert | `copilot mcp list --json` lists ug's server from the user config, enabled, next to the developer's still-disabled one; `ug mcp list --agents copilot` reports it enabled; Copilot's session events record a successful call loaded from the user config; revert leaves only the developer's server and no legacy `ucode-mcp-config.json` (non-blocking CI lane) | +| `test_managed_fixture_copilot_mcp_registered_in_user_config`, `test_managed_fixture_copilot_skills_lists_downloaded_skill` | Non-interactive configure with an injected config enabling Copilot with a managed MCP server or a managed skill | `copilot mcp list --json` lists the managed server from the user config as the ug mcp-proxy command; `copilot skill list --json` lists the downloaded skill as an enabled personal skill (non-blocking managed Copilot lane) | | `test_ug_copilot_upgrade_scrubs_token_saved_by_older_ug` | Configure Copilot with ug v0.2.0 (pinned commit; saves the bearer in `~/.copilot/ucode.env`), then configure with this ug, run a GPT task, revert | v0.2.0 left the bearer; after this ug's configure no file holds it; task completes; revert leaves no token and no `ucode.env` (non-blocking CI lane) | | `test_ug_opencode_headless_prompt_argument` | Run OpenCode from a script (`run --format json --auto`) with an argument prompt | Completed Read tool call; final text answer contains the file value; exit zero (non-blocking CI lane) | | `test_ug_agents_self_managed_opencode_journey` | Under the injected `managed_workspace_default` config (enables Claude/Codex, not OpenCode): bare configure, `ug agents list`, refused OpenCode launch, `ug agents add opencode`, real headless task, `ug agents remove opencode` (marker `managed_fixture and opencode`, non-blocking CI lane) | OpenCode absent from the list and launch fails with "doesn't enable OpenCode" / `ug agents add opencode`; after add it is listed self-managed and the headless Read task returns the fixture value; after remove it is hidden and refused again | diff --git a/tests/conftest.py b/tests/conftest.py index 1f3470782..a7017fab2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -40,6 +40,7 @@ def _isolate_ucode_state(tmp_path, monkeypatch): from ucode.agents import antigravity as antigravity_mod from ucode.agents import claude as claude_mod from ucode.agents import codex as codex_mod + from ucode.agents import copilot as copilot_mod from ucode.agents import vscode as vscode_mod from ucode.agents.desktop import claude as desktop_mod @@ -115,6 +116,17 @@ def install_test_policy(path, desired_text, *, interactive=False, allow_sudo=Tru codex_mod, "CODEX_MODEL_CATALOG_PATH", state_dir / "codex-model-catalog.json" ) monkeypatch.setattr(codex_mod, "CODEX_CONFIG_PATH", tmp_path / ".codex" / "ucode.config.toml") + # MCP writes and revert edit Copilot's user config and delete the legacy side file. + monkeypatch.setattr( + copilot_mod, "COPILOT_MCP_CONFIG_PATH", tmp_path / ".copilot" / "mcp-config.json" + ) + monkeypatch.setattr( + copilot_mod, "LEGACY_MCP_CONFIG_PATH", tmp_path / ".copilot" / "ucode-mcp-config.json" + ) + monkeypatch.setattr( + copilot_mod, "LEGACY_MCP_BACKUP_PATH", state_dir / "copilot-ucode-mcp-config.backup.json" + ) + monkeypatch.delenv("COPILOT_HOME", raising=False) def reject_privileged_write(path, _desired_text): pytest.fail( diff --git a/tests/fixtures/managed_config/copilot_mcp.json b/tests/fixtures/managed_config/copilot_mcp.json new file mode 100644 index 000000000..8cec6a864 --- /dev/null +++ b/tests/fixtures/managed_config/copilot_mcp.json @@ -0,0 +1,24 @@ +{ + "spec_version": 1, + "default_agent": "CODING_AGENT_COPILOT", + "enabled_agents": [ + { + "agent": "CODING_AGENT_COPILOT", + "config": { + "models": { + "model_services": [ + "system.ai.claude-sonnet-4-6" + ] + }, + "default_models": { + "default_model": "system.ai.claude-sonnet-4-6" + } + } + } + ], + "mcp_servers": { + "names": [ + "system.ai.github" + ] + } +} diff --git a/tests/fixtures/managed_config/copilot_skills_by_name.json b/tests/fixtures/managed_config/copilot_skills_by_name.json new file mode 100644 index 000000000..0d6c2d02b --- /dev/null +++ b/tests/fixtures/managed_config/copilot_skills_by_name.json @@ -0,0 +1,24 @@ +{ + "spec_version": 1, + "default_agent": "CODING_AGENT_COPILOT", + "enabled_agents": [ + { + "agent": "CODING_AGENT_COPILOT", + "config": { + "models": { + "model_services": [ + "system.ai.claude-sonnet-4-6" + ] + }, + "default_models": { + "default_model": "system.ai.claude-sonnet-4-6" + } + } + } + ], + "skills": { + "names": [ + "main.default.forkable-meals" + ] + } +} diff --git a/tests/integration/README.md b/tests/integration/README.md index ff07fd949..f57d1fea5 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -338,6 +338,12 @@ model-discovery schema and completes a file task on its Claude-compatible model `test_ug_copilot_providers.py` runs Copilot through the MPS fixtures below: a file task on the Anthropic service's model, on the OpenAI, Azure OpenAI and Microsoft Foundry services' `gpt-5-nano`, and on a named model of the Bedrock allow-all service; launching on the relayed service fails before Copilot starts. +`test_ug_copilot_mcp.py` adds `system.ai.web_search` with `ug mcp add --agents copilot` over a +developer's own disabled server, checks Copilot's `copilot mcp list --json` and `ug mcp list`, runs +a headless task whose tool call Copilot's session events record as successful, and reverts. The +Copilot cases in `test_ug_configure_managed_mcp.py` and `test_ug_configure_managed_skills.py` check +that a managed server and a managed skill reach Copilot (`copilot mcp list --json`, `copilot skill +list --json`). Antigravity CLI has five live headless journeys in `test_ug_antigravity_headless.py` and three in `test_ug_configure_antigravity.py`. Headless runs use `agy -p --output-format stream-json`; the @@ -665,6 +671,7 @@ test_ug_copilot_lifecycle.py # Copilot refresh after revocation, sign test_ug_copilot_models.py # Copilot model registry: Claude then GPT, unknown model, TUI /model switch test_ug_copilot_providers.py # Copilot via Model Provider Services: Anthropic, OpenAI, Azure OpenAI, Foundry, Bedrock allow-all; relayed refused test_ug_copilot_model_location.py # Copilot on a Unity Catalog model location: fresh --model-location file task +test_ug_copilot_mcp.py # Copilot MCP: ug mcp add, tool call, ug mcp list, revert keeps the user's server test_ug_antigravity_headless.py # Antigravity via ug's proxy: file task, --model (ug agy), Gemini MPS targets, MCP SQL tool, skill dir test_ug_configure_antigravity.py # Antigravity: configure picker + TUI task + reopen; External Models picker; revert keeps user files test_ug_codex_tracing.py # Codex OTLP export reaches the configured trace table @@ -683,8 +690,8 @@ test_ug_claude_model_discovery.py # unmanaged scenarios 9, 11, 13 test_ug_codex_model_discovery.py # unmanaged scenarios 10, 12, 14 test_ug_configure_managed.py # stubbed static model list/catalog pointer, no agent selector, saved-config replay test_ug_configure_managed_models.py # stubbed model sources, smart-routing banner, Codex fallback metadata -test_ug_configure_managed_mcp.py # stubbed managed MCP list -test_ug_configure_managed_skills.py # stubbed managed skills: download, coexist, reconcile away +test_ug_configure_managed_mcp.py # stubbed managed MCP list (Claude, Codex, Copilot) +test_ug_configure_managed_skills.py # stubbed managed skills: download, coexist, reconcile away (Claude, Codex, Copilot) test_ug_configure_managed_lifecycle.py # none -> A -> B -> MPS -> none: reconcile, clear on MPS/no-config test_installation.py # fresh installed package utils/ # process/terminal/evidence helpers and Docker files @@ -875,9 +882,9 @@ cases instead complete real file tasks through `--workspace` and `--model-locati journeys check setup and launch with existing services, without inference. There are **88 live cases** (including 12 marked TUI journeys) and **7 installation -checks** with Claude and Codex; selecting OpenCode adds one live headless case and two `managed_fixture` cases; selecting Copilot adds seventeen live cases; selecting Antigravity adds eight live cases (one TUI). One +checks** with Claude and Codex; selecting OpenCode adds one live headless case and two `managed_fixture` cases; selecting Copilot adds eighteen live cases and two `managed_fixture` cases; selecting Antigravity adds eight live cases (one TUI). One **`workspace_switch` case** uses two real workspaces and checks skills MCP cleanup and a completed -Claude task. A further **44 `managed_fixture` cases** (twelve of them also `live`) run on the +Claude task. A further **46 `managed_fixture` cases** (twelve of them also `live`) run on the managed workspace with a checked-in JSON CodingAgentConfig from `tests/fixtures/managed_config/` injected through `UCODE_MANAGED_CONFIG_STUB`; there are no cases that read a published config. Twelve explicit configured/fresh Claude and Codex discovery and source-override journeys use the @@ -1101,8 +1108,10 @@ A second non-blocking job, **Managed OpenCode self-managed** (`managed_fixture a in `test_ug_agents_self_managed.py`), installs Claude, Codex and OpenCode and runs against the managed e2e workspace (`E2E_ADMIN_WORKSPACE`) with the injected `managed_workspace_default` config, which enables Claude/Codex but not OpenCode. It is likewise outside `cujs`. -A non-blocking **Copilot** job (`live and copilot`, sixteen cases) runs on Linux and Windows the -same way and is likewise outside `cujs` until it is stable. +A non-blocking **Copilot** job (`live and copilot`, seventeen cases) runs on Linux and Windows the +same way and is likewise outside `cujs` until it is stable. A non-blocking **Managed config · Copilot** +job (`managed_fixture and copilot`, two cases) installs only Copilot and runs on the managed e2e +workspace with injected configs that enable Copilot; it is likewise outside `cujs`. A blocking **Antigravity** job (`live and antigravity`, eight cases) runs on Linux only with the pinned `ANTIGRAVITY_BUILD`; it is part of the `cujs` gate. diff --git a/tests/integration/test_ug_configure_managed_mcp.py b/tests/integration/test_ug_configure_managed_mcp.py index bd158cae2..072f32586 100644 --- a/tests/integration/test_ug_configure_managed_mcp.py +++ b/tests/integration/test_ug_configure_managed_mcp.py @@ -10,9 +10,11 @@ the developer's own ~/.codex/config.toml servers. A non-interactive configure has no sudo reconcile, so it falls back to the user-scope registration, which the real /mcp view still lists. Claude's `managedMcpServers` path is covered by unit tests; its integration coverage needs the managed -workspace to publish the `claude-code` OAuth client, which E2E_ADMIN_WORKSPACE does not. +workspace to publish the `claude-code` OAuth client, which E2E_ADMIN_WORKSPACE does not. Copilot +has no OS-managed MCP file, so its managed servers land in its user config, ~/.copilot/mcp-config.json. """ +import json import tomllib import pytest @@ -114,3 +116,26 @@ def test_managed_fixture_codex_http_headers_in_managed_file(live_session, worksp f"Expected {managed_header_key!r} = {managed_header_value!r}, " f"got {headers[managed_header_key]!r}" ) + + +@pytest.mark.managed_fixture +@pytest.mark.copilot +def test_managed_fixture_copilot_mcp_registered_in_user_config(live_session, workspace): + """Scenario: a non-interactive configure with a managed MCP server enabled for Copilot. + + Expected: Copilot's own `copilot mcp list --json` lists the server from its user config, + enabled, as the ug mcp-proxy stdio command for the server's mcp-services URL. + """ + session = live_session + use_managed_config_fixture(session, "copilot_mcp") + result = session.run("configure", "--workspace", workspace, "--skip-upgrade", timeout=240) + assert "Select coding agents to configure:" not in result.stdout, result.stdout + + listing = session.run("mcp", "list", "--json", binary=session.which("copilot"), timeout=60) + servers = json.loads(listing.stdout)["mcpServers"] + assert "system-ai-github" in servers, servers + entry = servers["system-ai-github"] + assert entry["source"] == "user", entry + assert entry["enabled"] is True, entry + assert entry["command"].endswith("ug"), entry + assert "mcp-services" in entry["args"][entry["args"].index("--url") + 1], entry diff --git a/tests/integration/test_ug_configure_managed_skills.py b/tests/integration/test_ug_configure_managed_skills.py index 9a96eda57..f650c1e4d 100644 --- a/tests/integration/test_ug_configure_managed_skills.py +++ b/tests/integration/test_ug_configure_managed_skills.py @@ -3,12 +3,14 @@ The admin CodingAgentConfig is a checked-in JSON fixture injected via UCODE_MANAGED_CONFIG_STUB so the real /skills TUI can be driven against a managed skill; only the config INPUT is stubbed (auth, the skill download, the config writers, and the agent binary stay real). These assert what the agent presents, not the bundles on disk (that is unit tests' job). See -tests/AGENTS.md rule 4. Claude reads ~/.claude/skills and Codex reads the shared ~/.agents/skills, -both of which `ug configure` writes, so a managed skill reaches either agent. The reconcile +tests/AGENTS.md rule 4. Claude reads ~/.claude/skills and Codex and Copilot read the shared +~/.agents/skills, both of which `ug configure` writes, so a managed skill reaches each agent. The reconcile lifecycle test additionally checks the on-disk bundles, since removal-on-reconcile is a disk change that has no steady-state `/skills` signal. """ +import json + import pytest from utils.managed import use_managed_config_fixture from utils.terminal import AgentTerminal @@ -79,6 +81,27 @@ def test_managed_fixture_codex_skills_lists_downloaded_skill(live_session, works ) +@pytest.mark.managed_fixture +@pytest.mark.copilot +def test_managed_fixture_copilot_skills_lists_downloaded_skill(live_session, workspace): + """Scenario: configure Copilot under an injected config that names a managed skill, then list + Copilot's skills. + + Expected: Copilot's own `copilot skill list --json` lists the downloaded managed skill as an + enabled personal skill, read from the shared ~/.agents/skills dir `ug configure` writes. + """ + session = live_session + use_managed_config_fixture(session, "copilot_skills_by_name") + result = session.run("configure", "--workspace", workspace, "--skip-upgrade", timeout=240) + assert "Select coding agents to configure:" not in result.stdout, result.stdout + + listing = session.run("skill", "list", "--json", binary=session.which("copilot"), timeout=60) + skills = [skill for skill in json.loads(listing.stdout) if skill.get("name") == SKILL_NAME] + assert skills, listing.stdout + assert all(skill["enabled"] is True for skill in skills), skills + assert any(str(skill["source"]).startswith("personal") for skill in skills), skills + + @pytest.mark.managed_fixture @pytest.mark.claude def test_managed_skills_reconcile_lifecycle(live_session, workspace): diff --git a/tests/integration/test_ug_copilot_mcp.py b/tests/integration/test_ug_copilot_mcp.py new file mode 100644 index 000000000..3dd82656a --- /dev/null +++ b/tests/integration/test_ug_copilot_mcp.py @@ -0,0 +1,94 @@ +"""CUJ for Databricks MCP servers in GitHub Copilot CLI through installed ug.""" + +import json + +import pytest +from utils.constants import CLAUDE_TEST_MODEL, COPILOT_HEADLESS_FLAGS +from utils.evidence import copilot_mcp_tool_calls +from utils.terminal import TerminalProcess + +pytestmark = [pytest.mark.live, pytest.mark.copilot] + +MCP_SERVICE = "system.ai.web_search" +# The developer's own server, disabled in Copilot's settings so the session never starts it. +USER_SERVER = "developer-own" +USER_ENTRY = {"type": "local", "command": "developer-own-mcp", "args": [], "tools": ["*"]} + + +def _copilot_mcp_servers(session) -> dict: + """Copilot's own view of its configured MCP servers (`copilot mcp list --json`).""" + result = session.run("mcp", "list", "--json", binary=session.which("copilot"), timeout=60) + return json.loads(result.stdout)["mcpServers"] + + +def test_ug_mcp_add_copilot_registers_server_in_copilot_config(live_session, workspace): + """Scenario: with a disabled server of their own in Copilot's user config, the developer + configures Copilot, adds `system.ai.web_search` with `ug mcp add --agents copilot`, asks + `ug copilot` to use it in a headless task, then runs `ug revert`. + + Expected: Copilot's own `copilot mcp list --json` lists ug's server from its user config + next to the developer's still-disabled server; `ug mcp list --agents copilot` reports it + enabled; Copilot's session events show a successful call to the server's tool, loaded from + the user config; revert removes only ug's server, so Copilot lists just the developer's. + Limits: the service needs no connection sign-in, so the sign-in path is not covered here. + """ + session = live_session + # Ordinary pre-existing user settings, not manufactured gateway state. + user_config = session.home / ".copilot/mcp-config.json" + user_config.parent.mkdir(parents=True) + user_config.write_text(json.dumps({"mcpServers": {USER_SERVER: USER_ENTRY}}), encoding="utf-8") + user_settings = session.home / ".copilot/settings.json" + user_settings.write_text(json.dumps({"disabledMcpServers": [USER_SERVER]}), encoding="utf-8") + + session.run( + "configure", + "--agents", + "copilot", + "--workspace", + workspace, + "--skip-validate", + "--skip-upgrade", + "--disable-databricks-ai-tools", + ) + session.run( + "mcp", "add", "--agents", "copilot", "--workspace", workspace, "--names", MCP_SERVICE + ) + [server] = [ + entry["name"] + for entry in session.workspace_state()["mcp_servers"] + if MCP_SERVICE in str(entry.get("url")) + ] + + servers = _copilot_mcp_servers(session) + assert servers[server]["source"] == "user", servers + assert servers[server]["enabled"] is True, servers + assert servers[USER_SERVER]["enabled"] is False, servers + listing = session.run("mcp", "list", "--agents", "copilot").stdout + rows = [line for line in listing.splitlines() if server in line] + assert rows and all("enabled" in row for row in rows), listing + + session.record("copilot-model.json", {"model": CLAUDE_TEST_MODEL}) + session.run( + "copilot", + "--model", + CLAUDE_TEST_MODEL, + "-p", + f"Call the `{server}` web_search tool once to search for 'Databricks Unity Catalog'. " + "Then reply with the single word DONE.", + *COPILOT_HEADLESS_FLAGS, + timeout=240, + ) + calls = [call for call in copilot_mcp_tool_calls(session) if call["server"] == server] + assert calls, copilot_mcp_tool_calls(session) + assert all(call["source"] == "user" for call in calls), calls + assert any(call["succeeded"] for call in calls), calls + + with TerminalProcess(session, "ug", [str(session.binary), "revert"], "revert") as terminal: + terminal.finish() + assert set(_copilot_mcp_servers(session)) == {USER_SERVER} + assert json.loads(user_config.read_text(encoding="utf-8")) == { + "mcpServers": {USER_SERVER: USER_ENTRY} + } + settings = json.loads(user_settings.read_text(encoding="utf-8")) + assert settings["disabledMcpServers"] == [USER_SERVER], settings + assert not (session.home / ".copilot/ucode-mcp-config.json").exists() diff --git a/tests/integration/utils/agents/copilot.py b/tests/integration/utils/agents/copilot.py index 2ea73799d..4244a86df 100644 --- a/tests/integration/utils/agents/copilot.py +++ b/tests/integration/utils/agents/copilot.py @@ -2,7 +2,13 @@ from . import _evidence_id -__all__ = ["EVIDENCE_KIND", "assistant_answers", "completed_task_models", "selected_models"] +__all__ = [ + "EVIDENCE_KIND", + "assistant_answers", + "completed_task_models", + "mcp_tool_calls", + "selected_models", +] SESSION_DIRECTORY = ".copilot/session-state" EVIDENCE_KIND = "assistant message model" @@ -46,3 +52,27 @@ def selected_models(records: list[dict]) -> list[str]: elif record.get("type") == "session.model_change" and data.get("newModel"): refs.append(data["newModel"]) return refs + + +def mcp_tool_calls(records: list[dict]) -> list[dict]: + """Each MCP tool call: its server, the config source Copilot loaded it from, and whether + the call completed successfully.""" + succeeded = { + (record.get("data") or {}).get("toolCallId") + for record in records + if record.get("type") == "tool.execution_complete" + and (record.get("data") or {}).get("success") is True + } + calls = [] + for record in records: + data = record.get("data") or {} + if record.get("type") == "tool.execution_start" and data.get("mcpServerName"): + calls.append( + { + "server": data["mcpServerName"], + "tool": data.get("mcpToolName"), + "source": data.get("mcpConfigSource"), + "succeeded": data.get("toolCallId") in succeeded, + } + ) + return calls diff --git a/tests/integration/utils/evidence.py b/tests/integration/utils/evidence.py index 0a1798b8e..9fb855939 100644 --- a/tests/integration/utils/evidence.py +++ b/tests/integration/utils/evidence.py @@ -156,6 +156,15 @@ def copilot_selected_models(session) -> list[str]: ] +def copilot_mcp_tool_calls(session) -> list[dict]: + """Every MCP tool call recorded by Copilot sessions under this home.""" + return [ + call + for records in agent_sessions(session, "copilot").values() + for call in copilot.mcp_tool_calls(records) + ] + + def assert_no_session_registry_left(session) -> None: """`ug copilot` removes its per-session registry, which holds the proxy secret, on exit.""" leftovers = sorted(session.home.glob(".copilot/ug-providers-*")) diff --git a/tests/test_agent_copilot.py b/tests/test_agent_copilot.py index b1ac9a0f0..24633280b 100644 --- a/tests/test_agent_copilot.py +++ b/tests/test_agent_copilot.py @@ -5,6 +5,7 @@ import contextlib import json import os +import subprocess from pathlib import Path import httpx @@ -830,9 +831,7 @@ def test_writes_mcp_server_without_clobbering_existing_config(self, tmp_path, mo monkeypatch.setattr(config_io_mod, "APP_DIR", tmp_path) config_file = tmp_path / "mcp-config.json" - backup_file = tmp_path / "copilot-mcp-backup.json" monkeypatch.setattr(cp_mod, "COPILOT_MCP_CONFIG_PATH", config_file) - monkeypatch.setattr(cp_mod, "COPILOT_MCP_BACKUP_PATH", backup_file) config_file.write_text( json.dumps( @@ -863,9 +862,7 @@ def test_reports_replaced_mcp_server(self, tmp_path, monkeypatch): monkeypatch.setattr(config_io_mod, "APP_DIR", tmp_path) config_file = tmp_path / "mcp-config.json" - backup_file = tmp_path / "copilot-mcp-backup.json" monkeypatch.setattr(cp_mod, "COPILOT_MCP_CONFIG_PATH", config_file) - monkeypatch.setattr(cp_mod, "COPILOT_MCP_BACKUP_PATH", backup_file) config_file.write_text( json.dumps({"mcpServers": {"github": {"old": True}}}), @@ -905,6 +902,123 @@ def test_removes_mcp_server_without_clobbering_others(self, tmp_path, monkeypatc assert written["other"] is True +class TestCopilotUserMcpConfig: + ENTRY = {"type": "local", "command": "ug", "args": ["mcp-proxy"], "tools": ["*"]} + + def test_honors_copilot_home(self, tmp_path, monkeypatch): + monkeypatch.setenv("COPILOT_HOME", str(tmp_path / "copilot-home")) + + copilot.write_user_mcp_servers({"svc": self.ENTRY}, set()) + + written = json.loads((tmp_path / "copilot-home" / "mcp-config.json").read_text()) + assert written["mcpServers"] == {"svc": self.ENTRY} + + @staticmethod + def _stub_cli(monkeypatch, *, fail_add=False): + calls: list[list[str]] = [] + + def run(argv, **kwargs): + calls.append(argv) + if argv[2] == "add" and fail_add: + raise subprocess.CalledProcessError(1, argv) + stderr = f'Server "{argv[3]}" not found.' if argv[2] == "remove" else "" + return subprocess.CompletedProcess(argv, 1 if stderr else 0, "", stderr) + + monkeypatch.setattr(copilot.subprocess_cross_os, "run", run) + return calls + + def test_defers_to_the_copilot_cli_for_a_config_it_cannot_parse(self, monkeypatch): + calls = self._stub_cli(monkeypatch) + path = copilot.copilot_mcp_config_path() + path.parent.mkdir(parents=True, exist_ok=True) + jsonc = '{\n // mine\n "mcpServers": {"mine": {"x": 1},},\n}\n' + path.write_text(jsonc, encoding="utf-8") + copilot.LEGACY_MCP_CONFIG_PATH.write_text(json.dumps({"mcpServers": {"old": self.ENTRY}})) + + removed = copilot.write_user_mcp_servers({"svc": self.ENTRY}, {"gone"}) + + assert removed == set() + assert calls == [ + ["copilot", "mcp", "remove", "gone"], + ["copilot", "mcp", "remove", "old"], + ["copilot", "mcp", "add", "old", "--", "ug", "mcp-proxy"], + ["copilot", "mcp", "remove", "svc"], + ["copilot", "mcp", "add", "svc", "--", "ug", "mcp-proxy"], + ] + assert path.read_text(encoding="utf-8") == jsonc + assert not copilot.LEGACY_MCP_CONFIG_PATH.exists() + + def test_failed_cli_add_keeps_the_legacy_side_file(self, monkeypatch): + self._stub_cli(monkeypatch, fail_add=True) + path = copilot.copilot_mcp_config_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("{not json", encoding="utf-8") + copilot.LEGACY_MCP_CONFIG_PATH.write_text(json.dumps({"mcpServers": {"old": self.ENTRY}})) + + with pytest.raises(RuntimeError, match="Failed to add MCP server 'old'"): + copilot.migrate_legacy_mcp_config() + + assert path.read_text(encoding="utf-8") == "{not json" + assert copilot.LEGACY_MCP_CONFIG_PATH.exists() + + def test_failed_write_keeps_the_legacy_side_file(self, monkeypatch): + def fail(path, payload): + raise RuntimeError(f"Failed to write config file: {path}") + + monkeypatch.setattr(copilot, "write_json_file", fail) + copilot.LEGACY_MCP_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) + copilot.LEGACY_MCP_CONFIG_PATH.write_text(json.dumps({"mcpServers": {"old": self.ENTRY}})) + + with pytest.raises(RuntimeError, match="Failed to write config file"): + copilot.migrate_legacy_mcp_config() + + assert copilot.LEGACY_MCP_CONFIG_PATH.exists() + + def test_a_legacy_side_file_that_cannot_be_deleted_raises_a_runtime_error(self, monkeypatch): + copilot.LEGACY_MCP_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) + copilot.LEGACY_MCP_CONFIG_PATH.write_text("{}") + + def unlink(self, missing_ok=False): + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr(Path, "unlink", unlink) + + with pytest.raises(RuntimeError, match="Failed to remove"): + copilot.remove_legacy_mcp_config() + + def test_empty_legacy_side_file_is_removed_without_a_write(self): + copilot.LEGACY_MCP_CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) + copilot.LEGACY_MCP_CONFIG_PATH.write_text("{}") + + copilot.migrate_legacy_mcp_config() + + assert not copilot.LEGACY_MCP_CONFIG_PATH.exists() + assert not copilot.copilot_mcp_config_path().exists() + + def test_moves_legacy_side_file_servers_into_the_user_config(self): + path = copilot.copilot_mcp_config_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"mcpServers": {"mine": {"x": 1}}, "other": True})) + copilot.LEGACY_MCP_CONFIG_PATH.write_text( + json.dumps({"mcpServers": {"svc": self.ENTRY, "gone": self.ENTRY}}) + ) + copilot.LEGACY_MCP_BACKUP_PATH.write_text("{}") + + removed = copilot.write_user_mcp_servers({}, {"gone"}) + + assert removed == {"gone"} + written = json.loads(path.read_text()) + assert written["other"] is True + assert written["mcpServers"] == {"mine": {"x": 1}, "svc": self.ENTRY} + assert not copilot.LEGACY_MCP_CONFIG_PATH.exists() + assert not copilot.LEGACY_MCP_BACKUP_PATH.exists() + + def test_migration_is_a_no_op_without_a_legacy_side_file(self): + copilot.migrate_legacy_mcp_config() + + assert not copilot.copilot_mcp_config_path().exists() + + def isolate_copilot_config_paths(monkeypatch, tmp_path): import ucode.config_io as config_io import ucode.state as state_mod @@ -916,7 +1030,6 @@ def isolate_copilot_config_paths(monkeypatch, tmp_path): monkeypatch.setattr(state_mod, "STATE_PATH", app_dir / "state.json") monkeypatch.setattr(copilot, "COPILOT_ENV_PATH", env_path) monkeypatch.setattr(copilot, "COPILOT_BACKUP_PATH", app_dir / "copilot-ucode-env.backup") - monkeypatch.setattr(copilot, "COPILOT_MCP_CONFIG_PATH", tmp_path / "missing-mcp.json") monkeypatch.setattr(copilot, "COPILOT_CONFIG_DIR", env_path.parent) return env_path @@ -1139,7 +1252,14 @@ def test_explicit_model_is_rewritten_to_the_registry_ref(self, tmp_path, monkeyp options=copilot.LaunchOptions(launch_model=SONNET), ) - assert seen["argv"] == ["copilot", "-p", "hi", "--model", f"dbx-responses/{GPT}"] + assert seen["argv"] == [ + "copilot", + "--allow-all-mcp-server-instructions", + "-p", + "hi", + "--model", + f"dbx-responses/{GPT}", + ] assert seen["env"]["COPILOT_MODEL"] == f"dbx-responses/{GPT}" def test_starts_on_the_requested_launch_model(self, tmp_path, monkeypatch): @@ -1320,24 +1440,20 @@ def test_has_prompt_flag(self): cmd = copilot.validate_cmd("copilot") assert "--prompt" in cmd - def test_adds_ucode_mcp_config_when_present(self, tmp_path, monkeypatch): - mcp_path = tmp_path / "ucode-mcp-config.json" - mcp_path.write_text("{}", encoding="utf-8") - monkeypatch.setattr(copilot, "COPILOT_MCP_CONFIG_PATH", mcp_path) - + def test_lets_mcp_server_instructions_reach_the_model(self): cmd = copilot.validate_cmd("copilot") - assert cmd[:3] == ["copilot", "--additional-mcp-config", f"@{mcp_path}"] + assert cmd[:2] == ["copilot", "--allow-all-mcp-server-instructions"] + assert "--additional-mcp-config" not in cmd class TestWriteUserMcpServers: def test_batched_add_remove_preserves_other_keys(self, tmp_path, monkeypatch): - path = tmp_path / "ucode-mcp-config.json" + path = tmp_path / "mcp-config.json" path.write_text( json.dumps({"other": 1, "mcpServers": {"mine": {"type": "local"}, "gone": {}}}) ) monkeypatch.setattr(copilot, "COPILOT_MCP_CONFIG_PATH", path) - monkeypatch.setattr(copilot, "COPILOT_MCP_BACKUP_PATH", tmp_path / "backup.json") copilot.write_user_mcp_servers( {"svc": copilot.build_mcp_server_entry(["ug", "mcp-proxy", "u"])}, {"gone"} diff --git a/tests/test_integration_contract.py b/tests/test_integration_contract.py index 7f714c398..9399608f1 100644 --- a/tests/test_integration_contract.py +++ b/tests/test_integration_contract.py @@ -419,6 +419,7 @@ def test_cuj_discovery_fails_for_a_file_without_a_check_name(tmp_path): "antigravity", "managed", "managed-opencode", + "managed-copilot", "dedicated-cuj-plan", "dedicated-cuj", "cujs", diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 87dac057b..6019ce09a 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -3068,22 +3068,14 @@ def test_reports_schemaless_connection(self, capsys): class TestRevertMcpConfigs: - def test_removes_cli_registered_servers_and_restores_copilot_config(self, monkeypatch): + def test_removes_cli_registered_servers_from_every_client(self, monkeypatch): removed: list[tuple[str, str]] = [] - restored: list[tuple[object, object, bool]] = [] monkeypatch.setattr( mcp, "remove_client_mcp_server", lambda client, name: removed.append((client, name)) or ["user"], ) - monkeypatch.setattr( - mcp, - "restore_file", - lambda config_path, backup_path, managed: ( - restored.append((config_path, backup_path, managed)) or True - ), - ) result = mcp.revert_mcp_configs( { @@ -3103,9 +3095,6 @@ def test_removes_cli_registered_servers_and_restores_copilot_config(self, monkey ("opencode", "github-mcp"), ("copilot", "github-mcp"), ] - assert restored == [ - (mcp.copilot.COPILOT_MCP_CONFIG_PATH, mcp.copilot.COPILOT_MCP_BACKUP_PATH, True) - ] assert result == { "claude": True, "codex": True, @@ -3114,6 +3103,44 @@ def test_removes_cli_registered_servers_and_restores_copilot_config(self, monkey "copilot": True, } + def test_one_agents_failed_removal_does_not_stop_the_others(self, monkeypatch, capsys): + removed: list[tuple[str, str]] = [] + + def remove(client, name): + if client == "copilot": + raise RuntimeError("Failed to remove MCP server via copilot CLI.") + removed.append((client, name)) + return ["user"] + + monkeypatch.setattr(mcp, "remove_client_mcp_server", remove) + + result = mcp.revert_mcp_configs( + {"mcp_servers": [{"name": "svc", "clients": ["copilot", "claude", "codex"]}]} + ) + + assert removed == [("claude", "svc"), ("codex", "svc")] + assert result == {"claude": True, "codex": True} + assert "Could not remove MCP server `svc` from GitHub Copilot CLI" in _unwrap( + capsys.readouterr().out + ) + + def test_removes_copilot_servers_from_its_user_config_and_the_legacy_side_file(self): + user_config = mcp.copilot.copilot_mcp_config_path() + user_config.parent.mkdir(parents=True, exist_ok=True) + user_config.write_text( + json.dumps({"mcpServers": {"github-mcp": {"type": "local"}, "mine": {"x": 1}}}) + ) + legacy = mcp.copilot.LEGACY_MCP_CONFIG_PATH + legacy.write_text(json.dumps({"mcpServers": {"stale-ug": {"type": "local"}}})) + + result = mcp.revert_mcp_configs( + {"mcp_servers": [{"name": "github-mcp", "clients": ["copilot"]}]} + ) + + assert result == {"copilot": True} + assert json.loads(user_config.read_text())["mcpServers"] == {"mine": {"x": 1}} + assert not legacy.exists() + def test_removes_skills_registry_across_its_clients(self, monkeypatch): removed: list[tuple[str, str]] = [] monkeypatch.setattr( @@ -3121,7 +3148,6 @@ def test_removes_skills_registry_across_its_clients(self, monkeypatch): "remove_client_mcp_server", lambda client, name: removed.append((client, name)) or ["user"], ) - monkeypatch.setattr(mcp, "restore_file", lambda *a, **kw: False) skills_entry = mcp._resolve_skills_mcp_servers( WS, ["claude", "codex"], _by_client(["claude", "codex"], ["a.b"]), [] @@ -4181,6 +4207,27 @@ def test_parses_codex_enabled_disabled_table(self): "github": mcp.LIVE_ENABLED, } + def test_parses_copilot_json_listing(self): + output = json.dumps( + { + "mcpServers": { + "system-ai-github": {"type": "local", "source": "user", "enabled": True}, + "paused": {"type": "local", "source": "user", "enabled": False}, + } + } + ) + assert mcp.parse_mcp_list_output("copilot", output) == { + "system-ai-github": mcp.LIVE_ENABLED, + "paused": mcp.LIVE_DISABLED, + } + + def test_copilot_listing_followed_by_stderr_still_parses(self): + output = json.dumps({"mcpServers": {"svc": {"enabled": True}}}) + "\nwarning: slow start\n" + assert mcp.parse_mcp_list_output("copilot", output) == {"svc": mcp.LIVE_ENABLED} + + def test_unreadable_copilot_listing_returns_no_servers(self): + assert mcp.parse_mcp_list_output("copilot", "error: something broke") == {} + def test_empty_listing_returns_no_servers(self): assert mcp.parse_mcp_list_output("gemini", "No MCP servers configured.") == {} @@ -4224,14 +4271,6 @@ def test_cursor_unapproved_server_is_not_reported_missing(self): output = "Loading MCPs…\nug-audit: not loaded (needs approval)\n" assert mcp.parse_mcp_list_output("cursor", output) == {"ug-audit": mcp.LIVE_NEEDS_APPROVAL} - def test_copilot_additional_config_is_reported_as_enabled(self, tmp_path, monkeypatch): - # Bare `copilot mcp list` does not include servers from --additional-mcp-config. - config = tmp_path / "ucode-mcp-config.json" - config.write_text('{"mcpServers":{"ug-audit":{"type":"local"}}}') - monkeypatch.setattr(mcp.copilot, "COPILOT_MCP_CONFIG_PATH", config) - monkeypatch.setattr(mcp, "_run_mcp_list", lambda client: "No MCP servers configured.") - assert mcp.query_live_mcp_status("copilot") == {"ug-audit": mcp.LIVE_ENABLED} - def test_ansi_progress_escapes_are_stripped_before_parsing(self, monkeypatch): # cursor-agent redraws progress with ANSI escapes that must not leak into server names. class _Result: @@ -4494,6 +4533,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_caveat_names_codex_and_copilot_when_both_reported(self, monkeypatch, capsys): + self._patch( + monkeypatch, installed=("claude", "codex", "copilot"), live={"codex": {}, "copilot": {}} + ) + assert mcp.list_mcp_command() == 0 + assert "Codex and GitHub Copilot CLI report enabled/disabled" in _unwrap( + capsys.readouterr().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": {}})