diff --git a/src/ucode/agents/copilot.py b/src/ucode/agents/copilot.py index 19a52b8e..7c6a6e65 100644 --- a/src/ucode/agents/copilot.py +++ b/src/ucode/agents/copilot.py @@ -6,15 +6,35 @@ to clean up; the values are also injected directly into the child process's environment at launch. -We point Copilot CLI's `openai` provider at the Databricks MLflow chat-completions -gateway, which serves Claude and codex (gpt-5) models. Gemini is intentionally -excluded — Databricks' Gemini translation layer rejects the `stream_options` -field that Copilot CLI sends, so Gemini models 400 on every request. +Copilot CLI supports two BYOK provider dialects. Claude models get its native +`anthropic` provider type, pointed at the same Messages-API gateway path +claude.py uses — Copilot's own runtime inserts `cache_control` breakpoints on +that path, so the (typically huge, shared) system/tool prefix actually caches. +Codex (gpt-5) has no native-dialect provider on Copilot's side, so it stays on +the `openai` provider against the Databricks MLflow chat-completions gateway. +Gemini is intentionally excluded from both — Databricks' Gemini translation +layer rejects the `stream_options` field that Copilot CLI sends, so Gemini +models 400 on every request. + +The `anthropic` path needs two things not obvious from `copilot help +providers`, both confirmed live: Bearer auth, not the `x-api-key` that +provider type sends by default (Databricks' gateway 401s on it); and a +well-known model id kept separate from the actual wire id, or Copilot sends +`temperature`, which current-gen Claude models reject. See render_env_overlay. + +That last part only works from Copilot 1.0.81-6 onward — verified live across +1.0.79 through 1.0.83. Below it, Copilot always sends `temperature` on the +`anthropic` path regardless of the model id, so Sonnet 5/Opus 5 (which reject +it outright) 400 on every request; Haiku 4.5 tolerates `temperature` and +would work either way, but the version gate below applies to every Claude +model for simplicity. Below 1.0.81-6, Claude models keep the old `openai` +path — uncached, but that's the status quo, not a regression. """ from __future__ import annotations import os +import re import signal import subprocess import threading @@ -32,10 +52,11 @@ ) from ucode.databricks import ( TOKEN_REFRESH_INTERVAL_SECONDS, - build_copilot_base_url, + build_copilot_base_urls, get_databricks_token, ) from ucode.state import mark_tool_managed, save_state +from ucode.telemetry import agent_version COPILOT_CONFIG_DIR = Path.home() / ".copilot" COPILOT_ENV_PATH = COPILOT_CONFIG_DIR / "ucode.env" @@ -55,6 +76,8 @@ "COPILOT_PROVIDER_TYPE", "COPILOT_PROVIDER_BASE_URL", "COPILOT_MODEL", + "COPILOT_PROVIDER_MODEL_ID", + "COPILOT_PROVIDER_WIRE_MODEL", "COPILOT_PROVIDER_BEARER_TOKEN", "COPILOT_OFFLINE", "OAUTH_TOKEN", @@ -64,6 +87,28 @@ "OPENAI_API_KEY", "COPILOT_PROVIDER_API_KEY", ] +# COPILOT_MODEL (openai) vs COPILOT_PROVIDER_MODEL_ID+COPILOT_PROVIDER_WIRE_MODEL +# (anthropic) are mutually exclusive — cleared before every write so switching +# families doesn't leave the other set stale in ~/.copilot/ucode.env. +_MODEL_SELECTION_KEYS = ( + "COPILOT_MODEL", + "COPILOT_PROVIDER_MODEL_ID", + "COPILOT_PROVIDER_WIRE_MODEL", +) + +_CANONICAL_CLAUDE_MODEL_ID_RE = re.compile(r"claude-[a-z0-9]+(?:-[a-z0-9]+)*", re.IGNORECASE) +# Same Bedrock version-marker pattern usage.py's normalize_price_key strips, +# e.g. "claude-opus-4-8-v1:0" -> "claude-opus-4-8" — Copilot's catalog doesn't +# carry the AWS version suffix, so leaving it in re-triggers the "unrecognized +# model" fallback (including the `temperature` send) this split is for. +_BEDROCK_VERSION_SUFFIX_RE = re.compile(r"-v\d+(:\d+)?$") + +# (major, minor, patch, prerelease) — see the module docstring. A version with +# no prerelease suffix (a final release) is a 4th component of _UNRELEASED so +# it always sorts after every prerelease of the same (major, minor, patch). +MINIMUM_COPILOT_ANTHROPIC_VERSION = (1, 0, 81, 6) +_UNRELEASED = 999_999 +_COPILOT_VERSION_RE = re.compile(r"(\d+)\.(\d+)\.(\d+)(?:-(\d+))?") def is_update_available() -> tuple[str, str] | None: @@ -91,10 +136,54 @@ def default_model(state: dict) -> str | None: return None +def _is_claude_model(model: str) -> bool: + # Every Claude family/model id ucode discovers or pins contains "claude" + # (canonical Anthropic names like "claude-sonnet-5", or Bedrock-style + # slugs like "us.anthropic.claude-opus-4-8") — same substring check + # `databricks.py` already uses elsewhere to special-case the family. + return "claude" in model.lower() + + +def _canonical_claude_model_id(model: str) -> str: + # e.g. "system.ai.claude-sonnet-5" -> "claude-sonnet-5" — the well-known + # name Copilot needs to recognize the model (see render_env_overlay). + # Lowercased and stripped of any Bedrock version suffix so it matches + # Copilot's catalog regardless of the input's casing or source. + match = _CANONICAL_CLAUDE_MODEL_ID_RE.search(model) + canonical = match.group(0).lower() if match else model.lower() + return _BEDROCK_VERSION_SUFFIX_RE.sub("", canonical) + + +def _parse_copilot_version(value: str) -> tuple[int, int, int, int] | None: + match = _COPILOT_VERSION_RE.search(value) + if not match: + return None + major, minor, patch, pre = match.groups() + return int(major), int(minor), int(patch), int(pre) if pre is not None else _UNRELEASED + + +def _supports_anthropic_provider() -> bool: + version = _parse_copilot_version(agent_version(SPEC["binary"])) + return version is not None and version >= MINIMUM_COPILOT_ANTHROPIC_VERSION + + def render_env_overlay(workspace: str, model: str, token: str) -> dict[str, str]: + base_urls = build_copilot_base_urls(workspace) + if _is_claude_model(model) and _supports_anthropic_provider(): + return { + "COPILOT_PROVIDER_TYPE": "anthropic", + "COPILOT_PROVIDER_BASE_URL": base_urls["anthropic"], + # Not COPILOT_MODEL: that would default both ids below to the + # unrecognized catalog id and Copilot would send `temperature`. + "COPILOT_PROVIDER_MODEL_ID": _canonical_claude_model_id(model), + "COPILOT_PROVIDER_WIRE_MODEL": model, + "COPILOT_PROVIDER_BEARER_TOKEN": token, # not API_KEY — see module docstring + "COPILOT_OFFLINE": "true", + "OAUTH_TOKEN": token, + } return { "COPILOT_PROVIDER_TYPE": "openai", - "COPILOT_PROVIDER_BASE_URL": build_copilot_base_url(workspace), + "COPILOT_PROVIDER_BASE_URL": base_urls["openai"], "COPILOT_MODEL": model, "COPILOT_PROVIDER_BEARER_TOKEN": token, "COPILOT_OFFLINE": "true", @@ -161,6 +250,8 @@ def write_tool_config( existing = parse_dotenv(COPILOT_ENV_PATH) for key in LEGACY_ENV_KEYS: existing.pop(key, None) + for key in _MODEL_SELECTION_KEYS: + existing.pop(key, None) existing.update(overlay) write_dotenv(COPILOT_ENV_PATH, existing) state = mark_tool_managed(state, "copilot", MANAGED_KEYS) diff --git a/src/ucode/databricks.py b/src/ucode/databricks.py index e98faf95..d736e65f 100644 --- a/src/ucode/databricks.py +++ b/src/ucode/databricks.py @@ -3206,12 +3206,23 @@ def build_pi_base_urls(workspace: str) -> dict[str, str]: } -def build_copilot_base_url(workspace: str) -> str: - # Copilot CLI's `openai` provider appends `/chat/completions` to the - # configured base URL. The Databricks MLflow chat-completions gateway is - # OpenAI-compatible and serves Claude, codex (gpt-5), and gemini models - # behind one URL. - return f"{workspace}/ai-gateway/mlflow/v1" +def build_copilot_base_urls(workspace: str) -> dict[str, str]: + # Copilot CLI's `anthropic` provider type speaks the native Anthropic + # Messages API directly, against the same gateway path claude.py uses. + # That's what lets Copilot's own cache_control-insertion logic run and + # cache the (typically huge, shared) system/tool prefix — the `openai` + # provider's wire format has no field to carry a cache breakpoint, so a + # Claude model proxied that way never gets a cache hit. + # + # Codex (gpt-5) has no native-dialect provider type on Copilot's side, so + # it stays on `openai` against the OpenAI-compatible MLflow + # chat-completions gateway, which serves Claude, codex, and gemini behind + # one URL. Copilot CLI's `openai` provider appends `/chat/completions` to + # this base URL. + return { + "anthropic": build_tool_base_url("claude", workspace), + "openai": f"{workspace}/ai-gateway/mlflow/v1", + } def build_shared_base_urls(workspace: str) -> dict[str, str | dict[str, str]]: @@ -3220,7 +3231,7 @@ def build_shared_base_urls(workspace: str) -> dict[str, str | dict[str, str]]: "claude": build_tool_base_url("claude", workspace), "gemini": build_tool_base_url("gemini", workspace), "opencode": build_opencode_base_urls(workspace), - "copilot": build_copilot_base_url(workspace), + "copilot": build_copilot_base_urls(workspace), "pi": build_pi_base_urls(workspace), } return urls diff --git a/tests/test_agent_copilot.py b/tests/test_agent_copilot.py index f6fd5136..a1aa9a29 100644 --- a/tests/test_agent_copilot.py +++ b/tests/test_agent_copilot.py @@ -4,9 +4,13 @@ import json +import pytest + from ucode.agents import copilot WS = "https://example.databricks.com" +# >= MINIMUM_COPILOT_ANTHROPIC_VERSION, for tests that need the anthropic path reachable. +NEW_ENOUGH_VERSION = "1.0.82" class TestCopilotSpec: @@ -24,26 +28,187 @@ def test_config_path_is_ucode_env_file(self): class TestRenderEnvOverlay: - def test_sets_provider_base_url(self): + """Claude models get Copilot's native `anthropic` provider (so Copilot's own + cache_control logic runs); everything else (codex/gpt-5) stays on `openai` + against the MLflow gateway. + + Verified live against a real workspace: Databricks' AI Gateway 401s on the + `x-api-key` auth Copilot's `anthropic` provider sends by default + (COPILOT_PROVIDER_API_KEY) — it wants `Authorization: Bearer` + (COPILOT_PROVIDER_BEARER_TOKEN, used for both provider types here). And an + unrecognized COPILOT_MODEL (a Databricks catalog id) makes Copilot fall + back to defaults that include sending `temperature`, which current-gen + Claude models 400 on — hence the separate COPILOT_PROVIDER_MODEL_ID + (canonical name) / COPILOT_PROVIDER_WIRE_MODEL (actual wire id) split. + + All of that only works on Copilot >= 1.0.81-6 (see TestSupportsAnthropicProvider), + so every test here mocks a new-enough installed version.""" + + @pytest.fixture(autouse=True) + def _new_enough_copilot(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: NEW_ENOUGH_VERSION) + + def test_claude_model_uses_anthropic_provider_type(self): env = copilot.render_env_overlay(WS, "claude-sonnet-4-6", "tok") - assert env["COPILOT_PROVIDER_BASE_URL"] == f"{WS}/ai-gateway/mlflow/v1" + assert env["COPILOT_PROVIDER_TYPE"] == "anthropic" + + def test_claude_model_uses_native_anthropic_base_url(self): + env = copilot.render_env_overlay(WS, "claude-sonnet-4-6", "tok") + assert env["COPILOT_PROVIDER_BASE_URL"] == f"{WS}/ai-gateway/anthropic" + + def test_claude_model_sets_bearer_token_not_api_key(self): + env = copilot.render_env_overlay(WS, "claude-sonnet-4-6", "tok123") + assert env["COPILOT_PROVIDER_BEARER_TOKEN"] == "tok123" + assert "COPILOT_PROVIDER_API_KEY" not in env + + def test_claude_model_splits_canonical_id_from_wire_model(self): + env = copilot.render_env_overlay(WS, "system.ai.claude-sonnet-5", "tok") + assert env["COPILOT_PROVIDER_MODEL_ID"] == "claude-sonnet-5" + assert env["COPILOT_PROVIDER_WIRE_MODEL"] == "system.ai.claude-sonnet-5" + assert "COPILOT_MODEL" not in env - def test_sets_provider_type(self): - env = copilot.render_env_overlay(WS, "m", "t") + def test_claude_model_matches_case_insensitively(self): + env = copilot.render_env_overlay(WS, "us.anthropic.Claude-Opus-4-8", "tok") + assert env["COPILOT_PROVIDER_TYPE"] == "anthropic" + assert env["COPILOT_PROVIDER_MODEL_ID"] == "claude-opus-4-8" + + def test_non_claude_model_uses_openai_provider_type(self): + env = copilot.render_env_overlay(WS, "gpt-5", "t") assert env["COPILOT_PROVIDER_TYPE"] == "openai" - def test_sets_model(self): - env = copilot.render_env_overlay(WS, "claude-sonnet-4-6", "tok") - assert env["COPILOT_MODEL"] == "claude-sonnet-4-6" + def test_non_claude_model_uses_mlflow_base_url(self): + env = copilot.render_env_overlay(WS, "gpt-5", "t") + assert env["COPILOT_PROVIDER_BASE_URL"] == f"{WS}/ai-gateway/mlflow/v1" - def test_sets_bearer_token(self): - env = copilot.render_env_overlay(WS, "m", "tok123") + def test_non_claude_model_sets_model_and_bearer_token(self): + env = copilot.render_env_overlay(WS, "gpt-5", "tok123") + assert env["COPILOT_MODEL"] == "gpt-5" assert env["COPILOT_PROVIDER_BEARER_TOKEN"] == "tok123" + assert "COPILOT_PROVIDER_API_KEY" not in env + assert "COPILOT_PROVIDER_MODEL_ID" not in env + assert "COPILOT_PROVIDER_WIRE_MODEL" not in env def test_sets_offline_true(self): - env = copilot.render_env_overlay(WS, "m", "t") + env = copilot.render_env_overlay(WS, "gpt-5", "t") assert env["COPILOT_OFFLINE"] == "true" + def test_sets_oauth_token_for_both_families(self): + assert copilot.render_env_overlay(WS, "claude-sonnet-4-6", "tok")["OAUTH_TOKEN"] == "tok" + assert copilot.render_env_overlay(WS, "gpt-5", "tok")["OAUTH_TOKEN"] == "tok" + + +class TestOldCopilotFallsBackToOpenai: + """Below 1.0.81-6, Copilot always sends `temperature` on the anthropic + path regardless of model id, and current-gen Claude models 400 on it — + verified live (1.0.79, 1.0.80, 1.0.81-0 all fail; 1.0.81-6 onward works). + So an old Copilot must keep getting the openai path even for Claude + models: uncached, but that's the pre-fix status quo, not a regression.""" + + def test_old_version_keeps_claude_on_openai(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.80") + env = copilot.render_env_overlay(WS, "system.ai.claude-sonnet-5", "tok") + assert env["COPILOT_PROVIDER_TYPE"] == "openai" + assert env["COPILOT_MODEL"] == "system.ai.claude-sonnet-5" + + def test_unknown_version_keeps_claude_on_openai(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "unknown") + env = copilot.render_env_overlay(WS, "system.ai.claude-sonnet-5", "tok") + assert env["COPILOT_PROVIDER_TYPE"] == "openai" + + def test_new_enough_version_uses_anthropic(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.81-6") + env = copilot.render_env_overlay(WS, "system.ai.claude-sonnet-5", "tok") + assert env["COPILOT_PROVIDER_TYPE"] == "anthropic" + + +class TestSupportsAnthropicProvider: + def test_false_below_the_minimum_patch(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.80") + assert copilot._supports_anthropic_provider() is False + + def test_false_just_below_the_minimum_prerelease(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.81-0") + assert copilot._supports_anthropic_provider() is False + + def test_true_at_the_exact_minimum_prerelease(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.81-6") + assert copilot._supports_anthropic_provider() is True + + def test_true_above_the_minimum_prerelease(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.81-7") + assert copilot._supports_anthropic_provider() is True + + def test_true_for_the_final_release_of_the_minimum_version(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.81") + assert copilot._supports_anthropic_provider() is True + + def test_true_for_a_newer_minor_version(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "1.0.83") + assert copilot._supports_anthropic_provider() is True + + def test_false_when_version_cannot_be_determined(self, monkeypatch): + monkeypatch.setattr(copilot, "agent_version", lambda binary: "unknown") + assert copilot._supports_anthropic_provider() is False + + +class TestParseCopilotVersion: + def test_parses_a_final_release(self): + assert copilot._parse_copilot_version("1.0.82") == (1, 0, 82, copilot._UNRELEASED) + + def test_parses_a_prerelease(self): + assert copilot._parse_copilot_version("1.0.81-6") == (1, 0, 81, 6) + + def test_a_final_release_sorts_after_its_prereleases(self): + final = copilot._parse_copilot_version("1.0.81") + prerelease = copilot._parse_copilot_version("1.0.81-14") + assert final > prerelease + + def test_returns_none_for_unparseable_input(self): + assert copilot._parse_copilot_version("unknown") is None + + +class TestIsClaudeModel: + def test_true_for_canonical_name(self): + assert copilot._is_claude_model("claude-sonnet-5") is True + + def test_true_for_bedrock_style_slug(self): + assert copilot._is_claude_model("us.anthropic.claude-opus-4-8") is True + + def test_false_for_codex(self): + assert copilot._is_claude_model("gpt-5") is False + + def test_false_for_catalog_qualified_gpt(self): + assert copilot._is_claude_model("system.ai.gpt-5") is False + + +class TestCanonicalClaudeModelId: + def test_strips_databricks_catalog_prefix(self): + assert copilot._canonical_claude_model_id("system.ai.claude-sonnet-5") == "claude-sonnet-5" + + def test_strips_bedrock_region_and_provider_prefix(self): + result = copilot._canonical_claude_model_id("us.anthropic.claude-opus-4-8") + assert result == "claude-opus-4-8" + + def test_leaves_bare_canonical_name_unchanged(self): + assert copilot._canonical_claude_model_id("claude-haiku-4-5") == "claude-haiku-4-5" + + def test_falls_back_to_the_input_when_no_match(self): + assert copilot._canonical_claude_model_id("weird-model") == "weird-model" + + def test_lowercases_a_mixed_case_id(self): + assert ( + copilot._canonical_claude_model_id("us.anthropic.Claude-Opus-4-8") == "claude-opus-4-8" + ) + + def test_lowercases_the_fallback_when_no_match(self): + assert copilot._canonical_claude_model_id("Weird-Model") == "weird-model" + + def test_strips_a_trailing_bedrock_version_suffix(self): + assert copilot._canonical_claude_model_id("claude-opus-4-8-v1:0") == "claude-opus-4-8" + + def test_strips_a_bedrock_version_suffix_without_a_colon_part(self): + assert copilot._canonical_claude_model_id("claude-opus-4-8-v1") == "claude-opus-4-8" + class TestBuildRuntimeEnv: def test_inherits_path(self): @@ -60,6 +225,32 @@ def test_sets_oauth_token_for_mcp(self): assert env["OAUTH_TOKEN"] == "tok" +class TestWriteToolConfig: + def test_switching_families_clears_the_stale_model_selection_keys(self, tmp_path, monkeypatch): + import ucode.agents.copilot as cp_mod + import ucode.config_io as config_io_mod + + monkeypatch.setattr(config_io_mod, "APP_DIR", tmp_path) + env_path = tmp_path / "ucode.env" + monkeypatch.setattr(cp_mod, "COPILOT_ENV_PATH", env_path) + monkeypatch.setattr(cp_mod, "COPILOT_BACKUP_PATH", tmp_path / "backup") + monkeypatch.setattr(cp_mod, "save_state", lambda state: None) + monkeypatch.setattr(cp_mod, "agent_version", lambda binary: NEW_ENOUGH_VERSION) + + state = {"workspace": WS} + cp_mod.write_tool_config(state, "system.ai.claude-sonnet-5", token="tok-a") + written = config_io_mod.parse_dotenv(env_path) + assert written["COPILOT_PROVIDER_MODEL_ID"] == "claude-sonnet-5" + assert written["COPILOT_PROVIDER_WIRE_MODEL"] == "system.ai.claude-sonnet-5" + assert "COPILOT_MODEL" not in written + + cp_mod.write_tool_config(state, "gpt-5", token="tok-b") + written = config_io_mod.parse_dotenv(env_path) + assert written["COPILOT_MODEL"] == "gpt-5" + assert "COPILOT_PROVIDER_MODEL_ID" not in written + assert "COPILOT_PROVIDER_WIRE_MODEL" not in written + + class TestMcpServerConfig: # ucode registers the `ucode mcp-proxy ...` bridge as a `local` (stdio) MCP # server; the proxy refreshes the token, so no URL/bearer header here. @@ -191,6 +382,8 @@ def test_includes_required_vars(self): "COPILOT_PROVIDER_TYPE", "COPILOT_PROVIDER_BASE_URL", "COPILOT_MODEL", + "COPILOT_PROVIDER_MODEL_ID", + "COPILOT_PROVIDER_WIRE_MODEL", "COPILOT_PROVIDER_BEARER_TOKEN", "COPILOT_OFFLINE", "OAUTH_TOKEN", diff --git a/tests/test_databricks.py b/tests/test_databricks.py index ffd9636b..0a083b3c 100644 --- a/tests/test_databricks.py +++ b/tests/test_databricks.py @@ -21,6 +21,7 @@ all_users_can_use_schema, build_auth_shell_command, build_auth_token_argv, + build_copilot_base_urls, build_databricks_cli_env, build_opencode_base_urls, build_shared_base_urls, @@ -115,6 +116,10 @@ def test_opencode_raises(self): with pytest.raises(RuntimeError, match="multiple base URLs"): build_tool_base_url("opencode", WS) + def test_copilot_raises(self): + with pytest.raises(RuntimeError, match="multiple base URLs"): + build_tool_base_url("copilot", WS) + def test_unsupported_tool_raises(self): with pytest.raises(RuntimeError, match="Unsupported"): build_tool_base_url("unknown", WS) @@ -128,6 +133,16 @@ def test_returns_anthropic_gemini_and_oss(self): assert urls["oss"] == f"{WS}/ai-gateway/mlflow/v1" +class TestBuildCopilotBaseUrls: + def test_anthropic_points_at_the_native_claude_gateway(self): + urls = build_copilot_base_urls(WS) + assert urls["anthropic"] == f"{WS}/ai-gateway/anthropic" + + def test_openai_points_at_the_mlflow_gateway(self): + urls = build_copilot_base_urls(WS) + assert urls["openai"] == f"{WS}/ai-gateway/mlflow/v1" + + class TestBuildSharedBaseUrls: def test_contains_all_tools(self): urls = build_shared_base_urls(WS) @@ -135,11 +150,16 @@ def test_contains_all_tools(self): assert "claude" in urls assert "gemini" in urls assert "opencode" in urls + assert "copilot" in urls def test_opencode_is_dict(self): urls = build_shared_base_urls(WS) assert isinstance(urls["opencode"], dict) + def test_copilot_is_dict(self): + urls = build_shared_base_urls(WS) + assert isinstance(urls["copilot"], dict) + def test_codex_url_format(self): urls = build_shared_base_urls(WS) assert urls["codex"] == f"{WS}/ai-gateway/codex/v1"