Codex Support Legacy setup for versions < 0.134

src/ucode/agents/codex.py

@@ -27,6 +27,8 @@
 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"
+LEGACY_CODEX_CONFIG_PATH = CODEX_CONFIG_DIR / "config.toml"
+LEGACY_CODEX_BACKUP_PATH = APP_DIR / "codex-config.backup.toml"
 CODEX_MODEL_PROVIDER_NAME = "ucode-databricks"
 MINIMUM_CODEX_VERSION = (0, 134, 0)
 MINIMUM_CODEX_VERSION_TEXT = "0.134.0"
@@ -47,6 +49,13 @@
     ["model_providers", CODEX_MODEL_PROVIDER_NAME, "http_headers"],
 ]
 
+LEGACY_MANAGED_KEYS: list[list[str]] = [
+    ["profile"],
+    ["profiles", CODEX_PROFILE_NAME],
+    ["model_providers", CODEX_MODEL_PROVIDER_NAME],
+    ["model_providers", CODEX_MODEL_PROVIDER_NAME, "http_headers"],
+]
+
 
 def is_update_available() -> tuple[str, str] | None:
     return available_npm_package_update(SPEC["package"])
@@ -68,60 +77,72 @@ def _installed_version_status() -> tuple[str, bool] | 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 _use_legacy_layout() -> bool:
+    """Return True when the installed Codex CLI predates per-profile config files.
 
+    Codex 0.134.0 introduced support for `--profile <name>` resolving to
+    `~/.codex/<name>.config.toml`. Older releases only honor a single
+    `~/.codex/config.toml` with `[profiles.<name>]` sections. When the version
+    is unknown we keep the new layout (matches the prior "unknown does not
+    block" semantic).
+    """
+    parsed = _parse_version(agent_version(SPEC["binary"]))
+    if parsed is None:
+        return False
+    return parsed < MINIMUM_CODEX_VERSION
 
-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 _provider_block(workspace: str, databricks_profile: str | None) -> dict:
+    auth_command = build_auth_shell_command(workspace, databricks_profile)
+    base_url = build_tool_base_url("codex", workspace)
+    return {
+        "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,
+        },
+    }
 
 
 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)
     overlay: dict = {"model_provider": CODEX_MODEL_PROVIDER_NAME}
     if model:
         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,
-            },
-        }
+        CODEX_MODEL_PROVIDER_NAME: _provider_block(workspace, databricks_profile),
     }
     return overlay
 
 
+def render_legacy_overlay(
+    workspace: str, model: str | None = None, databricks_profile: str | None = None
+) -> dict:
+    """Overlay for Codex CLI < 0.134.0, which only reads `~/.codex/config.toml`.
+
+    The shared file uses `profile = "ucode"` to select `[profiles.ucode]`, which
+    points at the shared `[model_providers.ucode-databricks]` block.
+    """
+    profile_block: dict = {"model_provider": CODEX_MODEL_PROVIDER_NAME}
+    if model:
+        profile_block["model"] = model
+    return {
+        "profile": CODEX_PROFILE_NAME,
+        "profiles": {CODEX_PROFILE_NAME: profile_block},
+        "model_providers": {
+            CODEX_MODEL_PROVIDER_NAME: _provider_block(workspace, databricks_profile),
+        },
+    }
+
+
 def _legacy_config_path() -> Path:
     return CODEX_CONFIG_PATH.parent / "config.toml"
 
@@ -157,11 +178,27 @@ def _remove_legacy_ucode_profile() -> None:
 
 
 def write_tool_config(state: dict, model: str | None = None) -> dict:
+    workspace = state["workspace"]
+    chosen_model = model or default_model(state)
+    databricks_profile = state.get("profile")
+
+    if _use_legacy_layout():
+        # Codex < 0.134.0 only reads ~/.codex/config.toml. Write the shared
+        # config with [profiles.ucode] + shared [model_providers.ucode-databricks]
+        # and skip the per-profile-file cleanup that would normally strip
+        # ucode's entry from the shared file.
+        backup_existing_file(LEGACY_CODEX_CONFIG_PATH, LEGACY_CODEX_BACKUP_PATH)
+        overlay = render_legacy_overlay(workspace, chosen_model, databricks_profile)
+        doc = read_toml_safe(LEGACY_CODEX_CONFIG_PATH)
+        deep_merge_dict(doc, overlay)
+        write_toml_file(LEGACY_CODEX_CONFIG_PATH, doc)
+        state = mark_tool_managed(state, "codex", LEGACY_MANAGED_KEYS)
+        save_state(state)
+        return state
+
     _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")
-    )
+    overlay = render_overlay(workspace, chosen_model, databricks_profile)
     doc = read_toml_safe(CODEX_CONFIG_PATH)
     deep_merge_dict(doc, overlay)
     write_toml_file(CODEX_CONFIG_PATH, doc)

tests/test_agent_codex.py

@@ -81,6 +81,7 @@ def test_writes_ucode_profile_config_file(self, tmp_path, monkeypatch):
         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, "agent_version", lambda binary: "0.134.0")
         monkeypatch.setattr(codex, "save_state", lambda state: None)
 
         codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]})
@@ -107,6 +108,7 @@ def test_removes_legacy_ucode_profile_from_shared_config(self, tmp_path, monkeyp
         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, "agent_version", lambda binary: "0.134.0")
         monkeypatch.setattr(codex, "save_state", lambda state: None)
 
         codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]})
@@ -117,25 +119,71 @@ def test_removes_legacy_ucode_profile_from_shared_config(self, tmp_path, monkeyp
         assert doc["profiles"]["other"]["model_provider"] == "keep"
         assert legacy_backup_path.exists()
 
+    def test_writes_legacy_shared_config_when_codex_too_old(self, tmp_path, monkeypatch):
+        config_dir = tmp_path / ".codex"
+        legacy_path = config_dir / "config.toml"
+        profile_path = config_dir / "ucode.config.toml"
+        backup_path = tmp_path / "codex-ucode-config.backup.toml"
+        legacy_backup_path = tmp_path / "codex-config.backup.toml"
+        monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", profile_path)
+        monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path)
+        monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", legacy_path)
+        monkeypatch.setattr(codex, "LEGACY_CODEX_BACKUP_PATH", legacy_backup_path)
+        monkeypatch.setattr(codex, "agent_version", lambda binary: "0.133.0")
+        monkeypatch.setattr(codex, "save_state", lambda state: None)
+
+        codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]})
+
+        # Per-profile file must not be written for old Codex.
+        assert not profile_path.exists()
+        doc = read_toml_safe(legacy_path)
+        assert doc["profile"] == "ucode"
+        assert doc["profiles"]["ucode"]["model_provider"] == "ucode-databricks"
+        assert doc["profiles"]["ucode"]["model"] == "gpt-5"
+        provider = doc["model_providers"]["ucode-databricks"]
+        assert provider["base_url"] == f"{WS}/ai-gateway/codex/v1"
+        assert provider["wire_api"] == "responses"
+
+    def test_legacy_write_preserves_other_profiles_in_shared_config(self, tmp_path, monkeypatch):
+        config_dir = tmp_path / ".codex"
+        config_dir.mkdir()
+        legacy_path = config_dir / "config.toml"
+        legacy_path.write_text(
+            '[profiles.other]\nmodel_provider = "keep"\n',
+            encoding="utf-8",
+        )
+        profile_path = config_dir / "ucode.config.toml"
+        backup_path = tmp_path / "codex-ucode-config.backup.toml"
+        legacy_backup_path = tmp_path / "codex-config.backup.toml"
+        monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", profile_path)
+        monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", backup_path)
+        monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", legacy_path)
+        monkeypatch.setattr(codex, "LEGACY_CODEX_BACKUP_PATH", legacy_backup_path)
+        monkeypatch.setattr(codex, "agent_version", lambda binary: "0.133.0")
+        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 doc["profiles"]["other"]["model_provider"] == "keep"
+        assert doc["profiles"]["ucode"]["model_provider"] == "ucode-databricks"
+
 
-class TestCodexMinimumVersion:
-    def test_no_error_when_codex_is_new_enough(self, monkeypatch):
+class TestCodexLegacyLayoutDetection:
+    def test_new_codex_uses_modern_layout(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
+        assert codex._use_legacy_layout() is False
 
-    def test_errors_when_codex_is_too_old(self, monkeypatch):
+    def test_old_codex_uses_legacy_layout(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()
+        assert codex._use_legacy_layout() is True
 
-    def test_unknown_version_does_not_block(self, monkeypatch):
+    def test_unknown_version_uses_modern_layout(self, monkeypatch):
         monkeypatch.setattr(codex, "agent_version", lambda binary: "unknown")
 
-        assert codex.minimum_version_error() is None
-        assert codex.required_update_message() is None
+        assert codex._use_legacy_layout() is False
 
 
 class TestCodexDefaultModel:

tests/test_agents_init.py

@@ -298,47 +298,6 @@ 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)
 

tests/test_e2e.py

@@ -93,6 +93,21 @@ def _run_gemini_gateway_smoke(workspace: str, model: str, token: str) -> str:
     return data.get("candidates", [{}])[0].get("content", {}).get("parts", [{}])[0].get("text", "")
 
 
+E2E_EXCLUDED_MODEL_IDS = {
+    # Listed by AI Gateway model discovery, but currently rejected at launch
+    # with "The provided model identifier is invalid."
+    "databricks-claude-opus-4-8",
+}
+
+
+def _launchable_model_items(models: dict) -> list[tuple[str, str]]:
+    return [
+        (family, model_id)
+        for family, model_id in models.items()
+        if model_id and model_id not in E2E_EXCLUDED_MODEL_IDS
+    ]
+
+
 # ---------------------------------------------------------------------------
 # Databricks auth / token
 # ---------------------------------------------------------------------------
@@ -424,6 +439,9 @@ def test_launch_claude_per_model(
         claude_models: dict = e2e_state.get("claude_models") or {}
         if not claude_models:
             pytest.skip("No Claude models available on this workspace")
+        launchable_models = _launchable_model_items(claude_models)
+        if not launchable_models:
+            pytest.skip("No launchable Claude models available on this workspace")
 
         # Use an isolated config dir so the claude subprocess never reads or
         # writes ~/.claude/settings.json during this test.
@@ -436,7 +454,7 @@ def test_launch_claude_per_model(
         base_url = build_tool_base_url("claude", e2e_workspace)
 
         failures = []
-        for family, model_id in claude_models.items():
+        for family, model_id in launchable_models:
             with pytest.MonkeyPatch().context() as mp:
                 mp.setattr("ucode.state.save_state", lambda s: None)
                 claude.write_tool_config({**e2e_state, "workspace": e2e_workspace}, model_id)
@@ -619,9 +637,8 @@ def _all_models(self, e2e_state: dict) -> list[tuple[str, str]]:
         """Return [(family, model_id), ...] for every model copilot can talk to."""
         out: list[tuple[str, str]] = []
         claude_models: dict = e2e_state.get("claude_models") or {}
-        for family, model_id in claude_models.items():
-            if model_id:
-                out.append((f"claude-{family}", model_id))
+        for family, model_id in _launchable_model_items(claude_models):
+            out.append((f"claude-{family}", model_id))
         for model in e2e_state.get("codex_models") or []:
             if any(frag in model for frag in self.COPILOT_INCOMPATIBLE_MODEL_FRAGMENTS):
                 continue
@@ -680,9 +697,8 @@ class TestPiLaunch:
     def _all_models(self, e2e_state: dict) -> list[tuple[str, str]]:
         out: list[tuple[str, str]] = []
         claude_models: dict = e2e_state.get("claude_models") or {}
-        for family, model_id in claude_models.items():
-            if model_id:
-                out.append((f"claude-{family}", model_id))
+        for family, model_id in _launchable_model_items(claude_models):
+            out.append((f"claude-{family}", model_id))
         for model in e2e_state.get("codex_models") or []:
             out.append(("codex", model))
         for model in e2e_state.get("gemini_models") or []: