From 2baf209331820219c4f866d11ddcbcd93984a702 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Tue, 26 May 2026 23:03:10 +0000 Subject: [PATCH] Update Codex profile config layout --- src/ucode/agents/__init__.py | 30 +++++++- src/ucode/agents/codex.py | 129 ++++++++++++++++++++++++++++------- tests/test_agent_codex.py | 72 +++++++++++++++++-- tests/test_agents_init.py | 41 +++++++++++ tests/test_e2e.py | 6 +- tests/test_e2e_user_agent.py | 2 +- 6 files changed, 246 insertions(+), 34 deletions(-) diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index 58b74b2..14cfd1b 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -21,6 +21,7 @@ install_databricks_cli, ) from ucode.state import load_state, save_state +from ucode.telemetry import agent_version from ucode.ui import ( console, print_err, @@ -86,9 +87,24 @@ def _update_installed_tool_binary(tool: str) -> bool: return False print_success(f"{spec['display']} is up to date") + agent_version.cache_clear() return bool(shutil.which(binary)) +def _minimum_version_error(tool: str) -> str | None: + checker = getattr(_MODULES[tool], "minimum_version_error", None) + if not callable(checker): + return None + return checker() + + +def _required_update_message(tool: str) -> str | None: + checker = getattr(_MODULES[tool], "required_update_message", None) + if not callable(checker): + return None + return checker() + + def _confirm_update_installed_tool_binary(tool: str) -> bool: spec = TOOL_SPECS[tool] update = _MODULES[tool].is_update_available() @@ -105,8 +121,18 @@ def install_tool_binary(tool: str, *, strict: bool = True, update_existing: bool package = spec["package"] if shutil.which(binary): - if update_existing and _confirm_update_installed_tool_binary(tool): - _update_installed_tool_binary(tool) + if update_existing: + required_update = _required_update_message(tool) + if required_update: + print_warning(required_update) + if not _update_installed_tool_binary(tool): + raise RuntimeError(_minimum_version_error(tool) or required_update) + elif _confirm_update_installed_tool_binary(tool): + _update_installed_tool_binary(tool) + + version_error = _minimum_version_error(tool) + if version_error: + raise RuntimeError(version_error) return True if not shutil.which("npm"): diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 3a1b2aa..2077bee 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -1,8 +1,9 @@ -"""Codex agent: writes ~/.codex/config.toml with a Databricks-backed model provider.""" +"""Codex agent: writes ~/.codex/ucode.config.toml for Databricks-backed Codex.""" from __future__ import annotations import os +import re from pathlib import Path from ucode.agent_updates import available_npm_package_update @@ -23,10 +24,13 @@ from ucode.telemetry import agent_version, ucode_version CODEX_CONFIG_DIR = Path.home() / ".codex" -CODEX_CONFIG_PATH = CODEX_CONFIG_DIR / "config.toml" -CODEX_BACKUP_PATH = APP_DIR / "codex-config.backup.toml" CODEX_PROFILE_NAME = "ucode" +CODEX_CONFIG_PATH = CODEX_CONFIG_DIR / f"{CODEX_PROFILE_NAME}.config.toml" +CODEX_BACKUP_PATH = APP_DIR / "codex-ucode-config.backup.toml" CODEX_MODEL_PROVIDER_NAME = "ucode-databricks" +MINIMUM_CODEX_VERSION = (0, 134, 0) +MINIMUM_CODEX_VERSION_TEXT = "0.134.0" + SPEC: ToolSpec = { "binary": "codex", @@ -37,7 +41,8 @@ } MANAGED_KEYS: list[list[str]] = [ - ["profiles", CODEX_PROFILE_NAME], + ["model_provider"], + ["model"], ["model_providers", CODEX_MODEL_PROVIDER_NAME], ["model_providers", CODEX_MODEL_PROVIDER_NAME, "http_headers"], ] @@ -47,36 +52,112 @@ def is_update_available() -> tuple[str, str] | None: return available_npm_package_update(SPEC["package"]) +def _parse_version(value: str) -> tuple[int, int, int] | None: + match = re.search(r"(\d+)\.(\d+)\.(\d+)", value) + if not match: + return None + major, minor, patch = match.groups() + return int(major), int(minor), int(patch) + + +def _installed_version_status() -> tuple[str, bool] | None: + version = agent_version(SPEC["binary"]) + parsed = _parse_version(version) + if parsed is None: + return None + return version, parsed < MINIMUM_CODEX_VERSION + + +def minimum_version_error() -> str | None: + status = _installed_version_status() + if status is None: + return None + version, is_too_old = status + if not is_too_old: + return None + return ( + f"Codex CLI {version} is too old for ucode's Codex profile config. " + f"Codex CLI must be updated to {MINIMUM_CODEX_VERSION_TEXT} or newer; " + f"run `npm install -g {SPEC['package']}` or `ucode configure`." + ) + + +def required_update_message() -> str | None: + status = _installed_version_status() + if status is None: + return None + version, is_too_old = status + if not is_too_old: + return None + return ( + f"Codex CLI {version} is older than required {MINIMUM_CODEX_VERSION_TEXT}; " + "updating Codex is required for ucode's Codex profile config." + ) + + def render_overlay( workspace: str, model: str | None = None, databricks_profile: str | None = None ) -> dict: auth_command = build_auth_shell_command(workspace, databricks_profile) base_url = build_tool_base_url("codex", workspace) - codex_profile_cfg: dict[str, str] = {"model_provider": CODEX_MODEL_PROVIDER_NAME} + overlay: dict = {"model_provider": CODEX_MODEL_PROVIDER_NAME} if model: - codex_profile_cfg["model"] = model - return { - "profiles": {CODEX_PROFILE_NAME: codex_profile_cfg}, - "model_providers": { - CODEX_MODEL_PROVIDER_NAME: { - "name": "Databricks AI Gateway", - "base_url": base_url, - "wire_api": "responses", - "http_headers": { - "User-Agent": f"ucode/{ucode_version()} codex/{agent_version('codex')}", - }, - "auth": { - "command": "sh", - "args": ["-c", auth_command], - "timeout_ms": 5000, - "refresh_interval_ms": 900000, - }, - } - }, + overlay["model"] = model + overlay["model_providers"] = { + CODEX_MODEL_PROVIDER_NAME: { + "name": "Databricks AI Gateway", + "base_url": base_url, + "wire_api": "responses", + "http_headers": { + "User-Agent": f"ucode/{ucode_version()} codex/{agent_version('codex')}", + }, + "auth": { + "command": "sh", + "args": ["-c", auth_command], + "timeout_ms": 5000, + "refresh_interval_ms": 900000, + }, + } } + return overlay + + +def _legacy_config_path() -> Path: + return CODEX_CONFIG_PATH.parent / "config.toml" + + +def _legacy_backup_path() -> Path: + return CODEX_BACKUP_PATH.with_name("codex-legacy-config.backup.toml") + + +def _remove_legacy_ucode_profile() -> None: + """Remove ucode's old [profiles.ucode] entry from shared Codex config.""" + path = _legacy_config_path() + if path == CODEX_CONFIG_PATH or not path.exists(): + return + + doc = read_toml_safe(path) + changed = False + + profiles = doc.get("profiles") + if isinstance(profiles, dict) and CODEX_PROFILE_NAME in profiles: + backup_existing_file(path, _legacy_backup_path()) + profiles.pop(CODEX_PROFILE_NAME, None) + if not profiles: + doc.pop("profiles", None) + changed = True + + if doc.get("profile") == CODEX_PROFILE_NAME: + backup_existing_file(path, _legacy_backup_path()) + doc.pop("profile", None) + changed = True + + if changed: + write_toml_file(path, doc) def write_tool_config(state: dict, model: str | None = None) -> dict: + _remove_legacy_ucode_profile() backup_existing_file(CODEX_CONFIG_PATH, CODEX_BACKUP_PATH) overlay = render_overlay( state["workspace"], model or default_model(state), state.get("profile") diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index f9cbc3f..45c709f 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -5,6 +5,7 @@ import os from ucode.agents import codex +from ucode.config_io import read_toml_safe WS = "https://example.databricks.com" @@ -21,18 +22,18 @@ def test_display(self): class TestRenderOverlay: - def test_creates_ucode_profile_without_setting_global_default(self): + def test_uses_profile_file_shape_without_legacy_profiles(self): overlay = codex.render_overlay(WS) assert "profile" not in overlay - assert "ucode" in overlay["profiles"] + assert "profiles" not in overlay def test_sets_model_provider(self): overlay = codex.render_overlay(WS) - assert overlay["profiles"]["ucode"]["model_provider"] == "ucode-databricks" + assert overlay["model_provider"] == "ucode-databricks" def test_sets_model_when_provided(self): overlay = codex.render_overlay(WS, "databricks-gpt-5") - assert overlay["profiles"]["ucode"]["model"] == "databricks-gpt-5" + assert overlay["model"] == "databricks-gpt-5" def test_provider_base_url(self): overlay = codex.render_overlay(WS) @@ -74,6 +75,69 @@ def test_managed_keys_include_http_headers(self): assert ["model_providers", "ucode-databricks", "http_headers"] in codex.MANAGED_KEYS +class TestCodexWriteConfig: + def test_writes_ucode_profile_config_file(self, tmp_path, monkeypatch): + config_path = tmp_path / ".codex" / "ucode.config.toml" + backup_path = tmp_path / "codex-ucode-config.backup.toml" + monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path) + monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path) + monkeypatch.setattr(codex, "save_state", lambda state: None) + + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + doc = read_toml_safe(config_path) + assert doc["model_provider"] == "ucode-databricks" + assert doc["model"] == "gpt-5" + assert "profiles" not in doc + + def test_removes_legacy_ucode_profile_from_shared_config(self, tmp_path, monkeypatch): + config_dir = tmp_path / ".codex" + config_dir.mkdir() + profile_path = config_dir / "ucode.config.toml" + legacy_path = config_dir / "config.toml" + legacy_path.write_text( + 'profile = "ucode"\n\n' + "[profiles.ucode]\n" + 'model_provider = "old"\n\n' + "[profiles.other]\n" + 'model_provider = "keep"\n', + encoding="utf-8", + ) + backup_path = tmp_path / "codex-ucode-config.backup.toml" + legacy_backup_path = tmp_path / "codex-legacy-config.backup.toml" + monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", profile_path) + monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path) + monkeypatch.setattr(codex, "save_state", lambda state: None) + + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + doc = read_toml_safe(legacy_path) + assert "profile" not in doc + assert "ucode" not in doc["profiles"] + assert doc["profiles"]["other"]["model_provider"] == "keep" + assert legacy_backup_path.exists() + + +class TestCodexMinimumVersion: + def test_no_error_when_codex_is_new_enough(self, monkeypatch): + monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0") + + assert codex.minimum_version_error() is None + assert codex.required_update_message() is None + + def test_errors_when_codex_is_too_old(self, monkeypatch): + monkeypatch.setattr(codex, "agent_version", lambda binary: "0.133.0") + + assert "Codex CLI must be updated to 0.134.0 or newer" in codex.minimum_version_error() + assert "updating Codex is required" in codex.required_update_message() + + def test_unknown_version_does_not_block(self, monkeypatch): + monkeypatch.setattr(codex, "agent_version", lambda binary: "unknown") + + assert codex.minimum_version_error() is None + assert codex.required_update_message() is None + + class TestCodexDefaultModel: def test_returns_first_codex_model(self): assert codex.default_model({"codex_models": ["gpt-5", "gpt-4o"]}) == "gpt-5" diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index 15d1e36..25c17e6 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -298,6 +298,47 @@ def fake_run(*args, **kwargs): assert install_tool_binary("opencode", strict=True, update_existing=True) is True + def test_existing_old_codex_raises_clear_blocker(self, monkeypatch): + def fake_which(binary: str) -> str | None: + return f"/usr/bin/{binary}" + + message = "Codex CLI must be updated to 0.134.0 or newer" + monkeypatch.setattr("ucode.agents.shutil.which", fake_which) + monkeypatch.setattr("ucode.agents.codex.minimum_version_error", lambda: message) + monkeypatch.setattr("ucode.agents.codex.required_update_message", lambda: None) + + with pytest.raises(RuntimeError, match="Codex CLI must be updated"): + install_tool_binary("codex", strict=True, update_existing=False) + + def test_configure_updates_existing_old_codex_without_optional_prompt( + self, monkeypatch, capsys + ): + calls: list[list[str]] = [] + prompt_calls: list[str] = [] + + def fake_which(binary: str) -> str | None: + return f"/usr/bin/{binary}" + + def fake_run(args, **kwargs): + calls.append(args) + return subprocess.CompletedProcess(args, 0) + + monkeypatch.setattr("ucode.agents.shutil.which", fake_which) + monkeypatch.setattr("ucode.agents.subprocess.run", fake_run) + monkeypatch.setattr( + "ucode.agents.codex.required_update_message", + lambda: "Codex CLI 0.133.0 is older than required 0.134.0", + ) + monkeypatch.setattr("ucode.agents.codex.minimum_version_error", lambda: None) + monkeypatch.setattr( + "ucode.agents.prompt_yes_no", lambda prompt: prompt_calls.append(prompt) or False + ) + + assert install_tool_binary("codex", strict=False, update_existing=True) is True + assert calls == [["npm", "install", "-g", "@openai/codex"]] + assert prompt_calls == [] + assert "older than required" in capsys.readouterr().out + def test_ensure_tool_binary_available_raises_when_missing(self, monkeypatch): monkeypatch.setattr("ucode.agents.shutil.which", lambda _: None) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 8a10182..22a8d3b 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -182,7 +182,7 @@ def _redirect_config_paths(self, monkeypatch, tmp_path): codex_dir = tmp_path / "codex_home" / ".codex" codex_dir.mkdir(parents=True, exist_ok=True) - monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", codex_dir / "config.toml") + monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", codex_dir / "ucode.config.toml") monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", tmp_path / "codex.backup.toml") monkeypatch.setattr(claude, "CLAUDE_SETTINGS_PATH", tmp_path / "claude-settings.json") @@ -200,7 +200,7 @@ def _redirect_config_paths(self, monkeypatch, tmp_path): monkeypatch.setattr(pi, "PI_CONFIG_PATH", tmp_path / "pi-models.json") monkeypatch.setattr(pi, "PI_BACKUP_PATH", tmp_path / "pi-models.backup.json") - return codex_dir / "config.toml" + return codex_dir / "ucode.config.toml" def test_only_picks_codex_writes_only_codex_config(self, tmp_path, monkeypatch, e2e_workspace): """User selects only codex → only codex's config file is written and @@ -342,7 +342,7 @@ def test_launch_codex_per_model(self, tmp_path, monkeypatch, e2e_state, e2e_work monkeypatch.setattr(config_io_mod, "APP_DIR", tmp_path) config_dir = tmp_path / "codex_home" / ".codex" config_dir.mkdir(parents=True) - config_path = config_dir / "config.toml" + config_path = config_dir / "ucode.config.toml" backup_path = tmp_path / "codex-config.backup.toml" monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path) monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path) diff --git a/tests/test_e2e_user_agent.py b/tests/test_e2e_user_agent.py index a74fb9d..cea8830 100644 --- a/tests/test_e2e_user_agent.py +++ b/tests/test_e2e_user_agent.py @@ -212,7 +212,7 @@ def test_user_agent_arrives_at_gateway(self, tmp_path, monkeypatch, capture_serv _require_binary("codex") config_dir = tmp_path / "codex_home" / ".codex" config_dir.mkdir(parents=True) - config_path = config_dir / "config.toml" + config_path = config_dir / "ucode.config.toml" monkeypatch.setattr(config_io_mod, "APP_DIR", tmp_path) monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path)