diff --git a/README.md b/README.md index 94fcacf8..c1c94221 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,23 @@ models for Claude Code's `/model` picker. Discovery defaults to `system.ai` when no provider or model location is selected. Use `--provider` or `--model-location` to select another model source; managed workspace configs control their own sources. +`ug codex` validates discovered models with the installed Codex binary and publishes +them to `~/.ucode/codex-model-catalog.json`, referenced by shared `~/.codex/config.toml` +for Codex App. Managed static lists use the same path during `ug configure`. The +latest refresh supplies the app's catalog; custom catalogs (including Isaac's) and +custom providers are preserved. The app's gateway provider and authentication must +already be configured. Validation covers the local Codex binary. + +Codex loads the catalog at app-server startup. When ug reports a catalog change, +finish active tasks, restart the app server on the **connected host**, then reconnect. +Use `codex app-server daemon restart` for a standalone managed daemon; otherwise +restart the process or application that owns the server. Reconnecting or reopening +the desktop app can reuse a remote server with the old list. + +ug removes its shared reference on discovery/validation failure, reconfiguration, +revert, or before installing/updating Codex. After an update, run `ug codex` to refresh +discovery or `ug configure` for a managed static list, then restart the app server. + ## Configure ```bash @@ -166,7 +183,7 @@ with `ug configure` to control installation. | Tool | Managed files | |------|---------------| -| Codex | `~/.codex/ucode.config.toml`, legacy `~/.codex/config.toml`, `/etc/codex/managed_config.toml` (Linux and macOS) | +| Codex | `~/.codex/ucode.config.toml`, shared catalog reference in `~/.codex/config.toml`, `~/.ucode/codex-model-catalog.json`, `/etc/codex/managed_config.toml` (Linux and macOS) | | Claude Code | `~/.claude/ucode-settings.json`, `~/.claude.json`, `/etc/claude-code/managed-settings.json` (Linux), `/Library/Application Support/ClaudeCode/managed-settings.json` (macOS) | | 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` | diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index a70d93c3..7013cc2e 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -134,6 +134,9 @@ def _update_installed_tool_binary(tool: str, version: str | None = None) -> bool command = ["npm", "install", "-g", target] print_note(f"Upgrading {spec['display']}...") + if tool == "codex": + # Detach potentially incompatible metadata until the next validated refresh. + codex.detach_app_model_catalog() try: subprocess.run(command, check=True, timeout=300) except (OSError, subprocess.CalledProcessError, subprocess.TimeoutExpired): @@ -224,6 +227,8 @@ def install_tool_binary( print_section("Bootstrap") print_warning(f"`{binary}` was not found. Installing {spec['display']}...") + if tool == "codex": + codex.detach_app_model_catalog() try: subprocess.run(["npm", "install", "-g", package], check=True, timeout=300) except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc: diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 340fc5ee..595f894b 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -31,6 +31,7 @@ deep_merge_dict, is_dry_run, prune_key_paths, + read_json_safe, read_toml_safe, write_json_file, write_toml_file, @@ -82,7 +83,7 @@ from ucode.ui import print_warning_err from .args import LaunchOptions -from .codex_catalog import prepare_codex_catalog +from .codex_catalog import prepare_codex_catalog, validate_codex_catalog CODEX_CONFIG_DIR = Path.home() / ".codex" CODEX_PROFILE_NAME = "ucode" @@ -369,9 +370,14 @@ def revert_legacy_shared_config() -> bool: through the workspace gateway. ``ucode revert`` only restored the per-profile file, leaving those edits in place. Surgically strip them here. + Also remove the shared app catalog reference installed by modern ucode. Returns True if anything was removed. """ - return _strip_legacy_ucode_entries(_legacy_config_path()) + legacy_changed = _strip_legacy_ucode_entries(_legacy_config_path()) + app_catalog_changed = detach_app_model_catalog() + if app_catalog_changed and CODEX_MODEL_CATALOG_PATH.exists(): + CODEX_MODEL_CATALOG_PATH.unlink() + return legacy_changed or app_catalog_changed def configured_paths(state: dict) -> list[str]: @@ -448,11 +454,15 @@ def write_tool_config( catalog_path = str(CODEX_MODEL_CATALOG_PATH) if static_models and not provider else None # Build and validate before modifying config so failure cannot leave a stale # catalog enabled or partially rewrite the user's configuration. - catalog = ( - prepare_codex_catalog(SPEC["binary"], static_models) - if static_models and not provider - else None - ) + try: + catalog = ( + prepare_codex_catalog(SPEC["binary"], static_models) + if static_models and not provider + else None + ) + except RuntimeError: + _detach_app_catalog_after_failure() + raise _remove_legacy_ucode_profile() # Back up only a file that predates ucode's management of the tool. A @@ -487,9 +497,11 @@ def compose(base: dict, *, include_catalog: bool = True) -> dict: return base if catalog is not None: - write_json_file(CODEX_MODEL_CATALOG_PATH, catalog) - elif CODEX_MODEL_CATALOG_PATH.exists() and not is_dry_run(): - CODEX_MODEL_CATALOG_PATH.unlink() + sync_app_model_catalog(catalog) + elif not is_dry_run(): + detach_app_model_catalog() + if CODEX_MODEL_CATALOG_PATH.exists(): + CODEX_MODEL_CATALOG_PATH.unlink() doc = read_toml_safe(CODEX_CONFIG_PATH) compose(doc) @@ -783,6 +795,9 @@ def _model_catalog_path(workspace: str, scope: str) -> Path: def _write_model_catalog(path: Path, catalog: dict) -> None: + if is_dry_run(): + write_json_file(path, catalog) + return temp_path = None try: path.parent.mkdir(parents=True, exist_ok=True) @@ -803,6 +818,87 @@ def _write_model_catalog(path: Path, catalog: dict) -> None: pass +def _is_ucode_catalog_reference(value: object) -> bool: + return isinstance(value, str) and Path(value).expanduser() == CODEX_MODEL_CATALOG_PATH + + +def _read_app_config() -> tomlkit.TOMLDocument: + path = _legacy_config_path() + try: + return tomlkit.parse(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return tomlkit.document() + except (OSError, UnicodeError, ParseError) as exc: + raise RuntimeError(f"Cannot update Codex App settings at {path}: {exc}") from exc + + +def detach_app_model_catalog() -> bool: + """Remove only a shared catalog reference owned by ucode.""" + if is_dry_run(): + return False + doc = _read_app_config() + if not _is_ucode_catalog_reference(doc.get("model_catalog_json")): + return False + doc.pop("model_catalog_json", None) + write_toml_file(_legacy_config_path(), doc) + _print_app_catalog_restart_notice() + return True + + +def _detach_app_catalog_after_failure() -> None: + """Keep the original catalog error when shared settings cannot be edited.""" + try: + detach_app_model_catalog() + except RuntimeError as exc: + print_warning_err(str(exc)) + + +def _print_app_catalog_restart_notice() -> None: + # A desktop reconnect can reuse a daemon whose model manager still holds + # the startup catalog. Never restart it here: it may be running tasks. + print_warning_err( + "Codex App model catalog changed. Existing app servers keep their startup " + "model list. After active tasks finish, restart the app server on the connected " + "host, then reconnect. For a Codex standalone daemon, run " + "`codex app-server daemon restart`; otherwise restart the process or application " + "that owns the app server. Reconnecting alone does not reload the catalog." + ) + + +def sync_app_model_catalog(catalog: dict) -> None: + """Publish a validated catalog without overwriting unreadable app settings.""" + if is_dry_run(): + return + doc = _read_app_config() + catalog_changed = read_json_safe(CODEX_MODEL_CATALOG_PATH) != catalog + _write_model_catalog(CODEX_MODEL_CATALOG_PATH, catalog) + + existing = doc.get("model_catalog_json") + if existing is not None and not _is_ucode_catalog_reference(existing): + print_warning_err( + f"Codex App already uses the custom model catalog {existing}; leaving it unchanged." + ) + return + + catalog_path = str(CODEX_MODEL_CATALOG_PATH) + provider = doc.get("model_provider") + if provider not in (None, CODEX_MODEL_PROVIDER_NAME, LEGACY_CODEX_MODEL_PROVIDER_NAME): + print_warning_err( + f"Codex App uses the custom provider {provider}; leaving its model catalog unmanaged." + ) + catalog_path = None + + reference_changed = existing != catalog_path + if reference_changed: + if catalog_path is None: + doc.pop("model_catalog_json", None) + else: + doc["model_catalog_json"] = catalog_path + write_toml_file(_legacy_config_path(), doc) + if reference_changed or (catalog_path is not None and catalog_changed): + _print_app_catalog_restart_notice() + + def _launch_token(state: dict, workspace: str, *, force_refresh: bool = False) -> str: """The token Codex authenticates with: a custom-OAuth client token when configured, else the CLI profile. The single auth-selection point — the OTLP proxy's token @@ -905,6 +1001,9 @@ def _run_codex( workspace: str | None, ) -> None: """Launch Codex — via the loopback proxy when OTLP tracing is on, else exec-replace.""" + if tool_args[:1] == ["update"]: + # exec replaces ug, so reattach only on a later validated refresh. + detach_app_model_catalog() if otel_tracing and workspace: _launch_codex_with_otel_proxy(state, base_argv, tool_args, workspace) else: @@ -973,7 +1072,10 @@ def launch( ) _set_provider_header(profile_doc, provider) _set_parent_schema_header(profile_doc, parent_schema if not provider else None) - if workspace and token and (provider or parent_schema): + updating = tool_args[:1] == ["update"] + if updating and _is_ucode_catalog_reference(profile_doc.get("model_catalog_json")): + profile_doc.pop("model_catalog_json") + if workspace and token and (provider or parent_schema) and not updating: try: if provider is not None: catalog_source = CodexCatalogSource.PROVIDER @@ -991,11 +1093,18 @@ def launch( source=catalog_source, identifier=catalog_identifier, ) + validate_codex_catalog(binary, catalog) except CodexMpsModelCatalogUnavailable: - pass + detach_app_model_catalog() + except RuntimeError: + # A failed discovery/validation must not leave a previous workspace's + # catalog active in independently launched app servers. + _detach_app_catalog_after_failure() + raise else: catalog_path = _model_catalog_path(workspace, catalog_scope) _write_model_catalog(catalog_path, catalog) + sync_app_model_catalog(catalog) profile_doc["model_catalog_json"] = str(catalog_path) # Codex otherwise boots on its bundled default model (e.g. gpt-5.6-sol), # which an MPS's allowlist doesn't route, so the first request 403s. Pin diff --git a/src/ucode/agents/codex_catalog.py b/src/ucode/agents/codex_catalog.py index c6954340..17955215 100644 --- a/src/ucode/agents/codex_catalog.py +++ b/src/ucode/agents/codex_catalog.py @@ -124,6 +124,48 @@ def build_codex_catalog( return {"models": models} +def _run_catalog_command( + binary: str, args: list[str], home: str +) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [binary, *args], + env={**os.environ, "CODEX_HOME": home}, + cwd=home, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=_TIMEOUT_SECONDS, + check=True, + ) + + +def _validate_codex_catalog_in_home(binary: str, catalog: dict, home: str) -> None: + candidate = Path(home) / "catalog.json" + candidate.write_text(json.dumps(catalog), encoding="utf-8") + _run_catalog_command( + binary, + [ + "-c", + f"model_catalog_json={json.dumps(str(candidate))}", + "debug", + "models", + ], + home, + ) + + +def validate_codex_catalog(binary: str, catalog: dict) -> None: + """Validate a candidate catalog with the selected Codex binary in isolation.""" + try: + with tempfile.TemporaryDirectory(prefix="ug-codex-catalog-") as home: + _validate_codex_catalog_in_home(binary, catalog, home) + except (OSError, TypeError, ValueError, subprocess.SubprocessError): + raise RuntimeError( + "Could not load the Codex model catalog with the selected Codex binary. " + "Update Codex and retry `ug codex`." + ) from None + + def prepare_codex_catalog(binary: str, names: list[str]) -> dict: """Extract and validate with the launch binary; never fetch or reuse stale metadata. @@ -133,20 +175,7 @@ def prepare_codex_catalog(binary: str, names: list[str]) -> dict: """ try: with tempfile.TemporaryDirectory(prefix="ug-codex-catalog-") as home: - env = {**os.environ, "CODEX_HOME": home} - - def run(args: list[str]) -> subprocess.CompletedProcess[str]: - return subprocess.run( - [binary, *args], - env=env, - cwd=home, - capture_output=True, - text=True, - timeout=_TIMEOUT_SECONDS, - check=True, - ) - - result = run(["debug", "models", "--bundled"]) + result = _run_catalog_command(binary, ["debug", "models", "--bundled"], home) payload = json.loads(result.stdout) bundled = payload.get("models") if isinstance(payload, dict) else None if ( @@ -160,23 +189,14 @@ def run(args: list[str]) -> subprocess.CompletedProcess[str]: raise ValueError("invalid bundled model catalog") fallback_warnings: list[str] = [] catalog = build_codex_catalog(bundled, names, warn=fallback_warnings.append) - candidate = Path(home) / "catalog.json" - candidate.write_text(json.dumps(catalog), encoding="utf-8") - run( - [ - "-c", - f"model_catalog_json={json.dumps(str(candidate))}", - "debug", - "models", - ] - ) - except (OSError, ValueError, subprocess.SubprocessError) as exc: + _validate_codex_catalog_in_home(binary, catalog, home) + except (OSError, TypeError, ValueError, subprocess.SubprocessError): raise RuntimeError( "Could not build the managed Codex model catalog locally. " "Upgrade the active Codex installation and verify " "`codex debug models --bundled` works, then retry configuration. " "Gateway discovery was not used." - ) from exc + ) from None for message in fallback_warnings: print_warning(message) return catalog diff --git a/tests/README.md b/tests/README.md index a2f7832e..73dcba4b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -59,10 +59,10 @@ All tests live directly in `integration/`; shared mechanics live in `utils/`. | `test_ug_configure_claude_repeat_and_revert`, `test_ug_configure_codex_repeat_and_revert` | Configure twice over user settings; complete a task; revert twice | Settings preserved; no bearer in ug state; generated config removed; status unconfigured | | `test_ug_configure_claude_cleans_stale_skills_mcp_on_workspace_switch` | Configure the first workspace, register its skills MCP, switch to a second real workspace, and use Claude | Old registration removed from Claude and the new workspace state; old workspace bucket preserved; repeat configure stays clean; real file task completes on the second workspace | | `test_ug_configure_claude_rejects_invalid_credentials`, `test_ug_configure_codex_rejects_invalid_credentials` | Configure with a rejected bearer against the real workspace | Authentication failure; no successful saved setup | -| `test_ug_configure_managed_claude`, `test_ug_configure_managed_codex` | Configure against a workspace that publishes a managed CodingAgentConfig | No agent selector; each agent's generated config exposes exactly the admin's static model_services; real gateway prompt on launch | +| `test_ug_configure_managed_claude`, `test_ug_configure_managed_codex` | Configure against a workspace that publishes a managed CodingAgentConfig | No agent selector; each agent's generated config exposes exactly the admin's static model_services; real gateway prompt on launch. The Codex case also checks the shared catalog pointer, restart guidance, and a fresh bare app-server's visible model list | | `test_case_01_*` | Launch managed Claude after configure and from fresh state | Claude receives the admin MPS header, caches exactly the independently fetched provider model IDs, and shows a cached model in a numbered picker row | | `test_case_03_*`, `test_case_05_*` | Pass a provider or model-location override to managed Claude after configure and from fresh state | ug rejects the override before Claude starts and preserves agent-owned state | -| `test_case_02_*` | Launch managed Codex after configure and from fresh state | Codex's generated catalog and app-server list match the independently fetched admin MPS-scoped model IDs | +| `test_case_02_*` | Launch managed Codex after configure and from fresh state | The scoped and stable catalogs, ug-launched app server, and fresh bare app server match the independently fetched admin MPS model IDs. The configured case uses real `ug revert` to remove ug's shared pointer and stable file while preserving a user setting | | `test_case_04_*`, `test_case_06_*` | Pass a provider or model-location override to managed Codex after configure and from fresh state | ug rejects the override before Codex starts and preserves agent-owned state | | `test_ug_configure_managed_codex_catalog_fallback` | Configure from an injected managed response containing a GPT model absent from Codex's bundled catalog | Actionable metadata warning; conservative catalog entry for the unknown model; real Codex prompt on the valid default model | | `test_managed_fixture_codex_http_headers_in_managed_file` | Interactive PTY configure with injected managed `http_headers` for Codex | The specified header (`x-databricks-workspace`) lands in `model_providers.Databricks.http_headers` in `/etc/codex/managed_config.toml` with the exact admin value | diff --git a/tests/conftest.py b/tests/conftest.py index a1a7419f..467906ee 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -50,6 +50,10 @@ def _isolate_ucode_state(tmp_path, monkeypatch): managed_files_mod, "MANAGED_BACKUP_MANIFEST_PATH", backup_dir / "manifest.json" ) monkeypatch.setattr(codex_mod, "codex_managed_config_path", lambda: None) + monkeypatch.setattr( + 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") def reject_privileged_write(path, _desired_text): pytest.fail( diff --git a/tests/integration/README.md b/tests/integration/README.md index 3e93b442..32acf61a 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -112,7 +112,7 @@ test_ug_claude_managed_model_discovery.py # fetched/reused Claude MPS policy cas test_ug_codex_managed_model_discovery.py # fetched/reused Codex MPS policy cases test_ug_claude_model_discovery.py # unmanaged scenarios 7, 9, 11, 13 test_ug_codex_model_discovery.py # unmanaged scenarios 8, 10, 12, 14 -test_ug_configure_managed.py # managed workspace: static model list, no agent selector +test_ug_configure_managed.py # managed workspace: static model list/catalog pointer, no agent selector test_ug_configure_managed_models.py # injected model sources, smart-routing banner, Codex fallback metadata test_ug_configure_managed_mcp.py # injected managed MCP list test_ug_configure_managed_skills.py # injected managed skills: download, coexist, reconcile away @@ -371,7 +371,11 @@ cannot still be running when that gate passes. Full coverage on PRs needs no lab **Managed config** jobs against a second workspace that publishes an admin CodingAgentConfig, whereas unmanaged live cases require a workspace without one. `ug configure` applies the admin config with no agent selector, and each agent's generated config exposes exactly the admin's static -`model_services` (Claude's `availableModels`/`modelPicker`, Codex's model catalog). +`model_services` (Claude's `availableModels`/`modelPicker`, Codex's model catalog). The managed +Codex case also checks stderr guidance to restart the daemon after publication, +that the shared app config points at the stable catalog, and that a fresh +bare Codex app-server returns the expected visible model before the existing TUI prompt/input +assertion. It does not claim GUI rendering or inference coverage. Treat that published CodingAgentConfig as shared CI fixture state. The managed lanes assert its exact model ids and its both-agent enablement, so editing the managed workspace's config (models, @@ -386,9 +390,12 @@ across all configured/fresh scenarios. The Codex module does the same with `main.default.ci_e2e_openai_mps`. Separate read-only, provider-scoped model-list requests establish expected IDs independently of the generated agent files. The tests require Claude's native cache to match those IDs and a cached model to appear in a numbered picker row -(including native Haiku 4.5, Opus 5, and Sonnet 5 deduplication), alongside its admin header. Codex's generated catalog -and app-server list must match its independently fetched IDs. These requests send no inference -prompts. Both agents must reject personal source overrides. +(including native Haiku 4.5, Opus 5, and Sonnet 5 deduplication), alongside its admin header. +Codex's scoped and stable catalogs, ug-launched app server, and fresh bare app server must match +its independently fetched IDs. The configured Codex journey subsequently runs real `ug revert`, +verifies that the shared pointer and stable catalog are gone, and checks that a user-owned setting +survives. These requests send no inference prompts. Both agents must reject personal source +overrides. This checks desktop startup configuration and cleanup, not GUI rendering or inference. Codex state comparisons exclude `.codex/tmp/arg0`, the disposable executable links recreated by version checks, while continuing to compare persistent agent files. In addition, `test_ug_configure_managed_codex_catalog_fallback` injects the intentionally nonexistent diff --git a/tests/integration/test_ug_codex_managed_model_discovery.py b/tests/integration/test_ug_codex_managed_model_discovery.py index b8c3ba9f..980f6908 100644 --- a/tests/integration/test_ug_codex_managed_model_discovery.py +++ b/tests/integration/test_ug_codex_managed_model_discovery.py @@ -21,6 +21,7 @@ fetch_codex_provider_catalog, parse_codex_provider_catalog, ) +from utils.terminal import TerminalProcess pytestmark = [pytest.mark.managed_fixture, pytest.mark.codex] @@ -83,6 +84,10 @@ def _assert_managed_provider_catalog(session, models, expected: CodexProviderCat # in Codex's generated profile. assert "model_catalog_json" not in config, config + shared_config = tomllib.loads((session.home / ".codex" / "config.toml").read_text()) + app_catalog_path = session.home / ".ucode" / "codex-model-catalog.json" + assert shared_config.get("model_catalog_json") == str(app_catalog_path), shared_config + managed_cache = json.loads((session.home / ".ucode/managed-config.json").read_text()) raw_config = managed_cache.get("config") enabled_agents = raw_config.get("enabled_agents") if isinstance(raw_config, dict) else None @@ -103,6 +108,7 @@ def _assert_managed_provider_catalog(session, models, expected: CodexProviderCat catalog_paths = list((session.home / ".ucode").glob("codex-model-catalog-*.json")) assert len(catalog_paths) == 1, catalog_paths catalog = json.loads(catalog_paths[0].read_text()) + assert json.loads(app_catalog_path.read_text()) == catalog catalog_ids = parse_codex_provider_catalog(catalog) session.record( "managed-provider-catalog.json", @@ -127,10 +133,18 @@ def test_case_02_managed_codex_uses_admin_discovery_after_configure( ): """Scenario: configure managed Codex, then launch its app server. - Expected: the independently fetched provider catalog exactly matches both the generated - catalog and the app-server model list. + Expected: the independently fetched provider catalog exactly matches the generated catalog, + ug-launched app server, and fresh bare app server. Real `ug revert` then removes the ug-owned + shared catalog pointer and stable catalog while preserving a user-owned Codex setting. """ session = live_session + user_config = session.home / ".codex" / "config.toml" + original_user_config = ( + "# user-owned Codex settings\n[notice]\nhide_rate_limit_model_nudge = true\n" + ) + user_config.parent.mkdir(parents=True) + user_config.write_text(original_user_config) + configured = session.run( "configure", "--workspace", @@ -144,6 +158,19 @@ def test_case_02_managed_codex_uses_admin_discovery_after_configure( models = session.codex_model_ids(["app-server", "--listen", "stdio://"]) _assert_managed_provider_catalog(session, models, _managed_codex_provider_catalog) + app_models = session.codex_model_ids( + ["app-server", "--listen", "stdio://"], name="bare-codex-models", binary="codex" + ) + assert app_models == models, (app_models, models) + + with TerminalProcess( + session, "ug", [str(session.binary), "revert"], "managed-discovery-revert" + ) as terminal: + terminal.finish() + shared_config = tomllib.loads(user_config.read_text()) + assert "model_catalog_json" not in shared_config, shared_config + assert tomllib.loads(user_config.read_text()) == tomllib.loads(original_user_config) + assert not (session.home / ".ucode" / "codex-model-catalog.json").exists() def test_case_02_managed_codex_uses_admin_discovery_from_fresh_state( @@ -151,8 +178,8 @@ def test_case_02_managed_codex_uses_admin_discovery_from_fresh_state( ): """Scenario: launch managed Codex with --workspace from fresh state. - Expected: the independently fetched provider catalog exactly matches both the generated - catalog and the app-server model list. + Expected: the independently fetched provider catalog exactly matches the generated catalog, + ug-launched app server, and fresh bare app server. """ session = live_session models = session.codex_model_ids( @@ -160,6 +187,10 @@ def test_case_02_managed_codex_uses_admin_discovery_from_fresh_state( ) _assert_managed_provider_catalog(session, models, _managed_codex_provider_catalog) + app_models = session.codex_model_ids( + ["app-server", "--listen", "stdio://"], name="bare-codex-models", binary="codex" + ) + assert app_models == models, (app_models, models) def test_case_04_managed_codex_rejects_provider_override_after_configure( diff --git a/tests/integration/test_ug_configure_managed.py b/tests/integration/test_ug_configure_managed.py index 759f2fd7..98989dca 100644 --- a/tests/integration/test_ug_configure_managed.py +++ b/tests/integration/test_ug_configure_managed.py @@ -9,6 +9,7 @@ """ import json +import tomllib import pytest from utils.terminal import AgentTerminal @@ -54,12 +55,15 @@ def test_ug_configure_managed_codex(live_session, workspace): Expected: ug applies the admin config to every enabled agent without showing the personal agent selector, Codex's generated model catalog lists exactly the admin's static - model_services, and launching Codex reaches a real gateway prompt rather than the - account-login flow. + model_services, the shared Codex App config points at that stable catalog, and configure + reports the daemon restart step on stderr. A fresh bare + Codex app-server returns the expected visible model, while the existing TUI assertion reaches + a prompt, accepts input, and exits normally. GUI rendering and inference are not covered. """ session = live_session result = session.run("configure", "--workspace", workspace, "--skip-upgrade", timeout=240) assert "Select coding agents to configure:" not in result.stdout, result.stdout + assert "codex app-server daemon restart" in " ".join(result.stderr.split()), result.stderr catalog = json.loads((session.home / ".ucode" / "codex-model-catalog.json").read_text()) listed = [ @@ -69,6 +73,17 @@ def test_ug_configure_managed_codex(live_session, workspace): ] assert listed == [MANAGED_CODEX_MODEL], catalog + shared_config = tomllib.loads((session.home / ".codex" / "config.toml").read_text()) + assert shared_config.get("model_catalog_json") == str( + session.home / ".ucode" / "codex-model-catalog.json" + ), shared_config + bare_models = session.codex_model_ids( + ["app-server", "--listen", "stdio://"], + name="bare-managed-codex-models", + binary="codex", + ) + assert bare_models == [MANAGED_CODEX_MODEL], bare_models + with AgentTerminal(session, "codex", [str(session.binary), "codex"], "managed-codex") as tui: tui.boot() tui.check_input_and_exit() diff --git a/tests/integration/utils/harness.py b/tests/integration/utils/harness.py index e9dc3728..101c4771 100644 --- a/tests/integration/utils/harness.py +++ b/tests/integration/utils/harness.py @@ -220,9 +220,10 @@ def app_server_handshake( timeout: int = 120, request: tuple[str, dict] | None = None, name: str = "app-server", + binary: str | None = None, ) -> dict: """Speak the real Codex stdio protocol and require an initialize response.""" - command = [str(self.binary), "codex", *args] + command = [binary, *args] if binary else [str(self.binary), "codex", *args] messages: queue.Queue = queue.Queue() transcript: list[str] = [] diagnostics: list[str] = [] @@ -309,7 +310,9 @@ def wait_for_response(request_id, description): f"{name}.json", {"argv": command, "stdout": transcript, "stderr": diagnostics} ) - def codex_model_ids(self, args: list[str], name: str = "codex-models") -> list[str]: + def codex_model_ids( + self, args: list[str], name: str = "codex-models", *, binary: str | None = None + ) -> list[str]: """Ask the real Codex app-server for the catalog its model picker uses.""" response = self.app_server_handshake( args, @@ -318,6 +321,7 @@ def codex_model_ids(self, args: list[str], name: str = "codex-models") -> list[s {"cursor": None, "limit": 1000, "includeHidden": False}, ), name=f"{name}-app-server", + binary=binary, ) result = response["result"] models = result.get("data") diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 55012323..618b845a 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -758,6 +758,150 @@ def test_returns_false_when_no_shared_config(self, tmp_path, monkeypatch): assert codex.revert_legacy_shared_config() is False + def test_strips_ucode_app_catalog_reference(self, capsys): + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + shared_path.parent.mkdir() + catalog_path = codex.CODEX_MODEL_CATALOG_PATH + catalog_path.write_text("{}", encoding="utf-8") + shared_path.write_text( + f'model_catalog_json = "{catalog_path}"\npersonality = "friendly"\n', + encoding="utf-8", + ) + assert codex.revert_legacy_shared_config() is True + + assert read_toml_safe(shared_path) == {"personality": "friendly"} + assert not catalog_path.exists() + assert "codex app-server daemon restart" in " ".join(capsys.readouterr().err.split()) + + +class TestCodexAppCatalog: + def test_publish_refresh_and_reattach_preserve_settings_and_report_restart(self, capsys): + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + shared_path.parent.mkdir() + original = '# User settings\nmodel = "gpt-user"\n' + shared_path.write_text(original, encoding="utf-8") + first_catalog = {"models": [{"slug": "first-model"}]} + second_catalog = {"models": [{"slug": "second-model"}]} + + codex.sync_app_model_catalog(first_catalog) + first_config = shared_path.read_text() + assert "# User settings" in first_config + assert read_toml_safe(shared_path) == { + "model": "gpt-user", + "model_catalog_json": str(codex.CODEX_MODEL_CATALOG_PATH), + } + assert json.loads(codex.CODEX_MODEL_CATALOG_PATH.read_text()) == first_catalog + published = capsys.readouterr() + assert published.out == "" + assert "codex app-server daemon restart" in " ".join(published.err.split()) + assert "After active tasks finish" in published.err + + codex.sync_app_model_catalog(first_catalog) + unchanged = capsys.readouterr() + assert unchanged.out == unchanged.err == "" + + codex.sync_app_model_catalog(second_catalog) + assert shared_path.read_text() == first_config + assert json.loads(codex.CODEX_MODEL_CATALOG_PATH.read_text()) == second_catalog + refreshed = capsys.readouterr() + assert refreshed.out == "" + assert "codex app-server daemon restart" in " ".join(refreshed.err.split()) + + # Reattaching an unchanged catalog also requires a server restart. + shared_path.write_text(original, encoding="utf-8") + codex.sync_app_model_catalog(second_catalog) + assert shared_path.read_text() == first_config + assert "codex app-server daemon restart" in " ".join(capsys.readouterr().err.split()) + + def test_invalid_shared_config_is_not_overwritten(self): + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + shared_path.parent.mkdir() + original = 'model = "unfinished\n' + shared_path.write_text(original, encoding="utf-8") + codex.CODEX_MODEL_CATALOG_PATH.write_text("previous catalog", encoding="utf-8") + + with pytest.raises(RuntimeError, match="Cannot update Codex App settings"): + codex.sync_app_model_catalog({"models": [{"slug": "gpt-mps"}]}) + + assert shared_path.read_text() == original + assert codex.CODEX_MODEL_CATALOG_PATH.read_text() == "previous catalog" + + @pytest.mark.parametrize("previous_catalog", [False, True]) + def test_custom_provider_keeps_its_own_model_discovery(self, previous_catalog, capsys): + if previous_catalog: + codex.sync_app_model_catalog({"models": [{"slug": "previous"}]}) + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + shared_path.parent.mkdir(exist_ok=True) + original = '# User settings\nmodel_provider = "custom"\nmodel = "user-model"\n' + shared_path.write_text( + original + + ( + f'model_catalog_json = "{codex.CODEX_MODEL_CATALOG_PATH}"\n' + if previous_catalog + else "" + ), + encoding="utf-8", + ) + + capsys.readouterr() + codex.sync_app_model_catalog({"models": [{"slug": "gpt-mps"}]}) + + assert shared_path.read_text() == original + output = capsys.readouterr().err + if previous_catalog: + assert "daemon restart" in " ".join(output.split()) + else: + assert "daemon restart" not in output + + def test_dry_run_leaves_config_and_catalog_unchanged(self, monkeypatch): + codex.sync_app_model_catalog({"models": [{"slug": "previous"}]}) + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + original = shared_path.read_bytes() + catalog_before = codex.CODEX_MODEL_CATALOG_PATH.read_bytes() + monkeypatch.setattr(codex, "is_dry_run", lambda: True) + + codex.sync_app_model_catalog({"models": [{"slug": "new"}]}) + + assert shared_path.read_bytes() == original + assert codex.CODEX_MODEL_CATALOG_PATH.read_bytes() == catalog_before + + def test_reconfigure_clears_discovered_app_catalog(self, monkeypatch): + codex.sync_app_model_catalog({"models": [{"slug": "previous-workspace"}]}) + monkeypatch.setattr(codex, "agent_version", lambda _: "0.154.0") + monkeypatch.setattr(codex, "save_state", lambda _: None) + + codex.write_tool_config({"workspace": WS}) + + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + assert "model_catalog_json" not in read_toml_safe(shared_path) + assert not codex.CODEX_MODEL_CATALOG_PATH.exists() + + def test_failed_static_validation_detaches_previous_app_catalog(self, monkeypatch): + codex.sync_app_model_catalog({"models": [{"slug": "old-model"}]}) + original_catalog = codex.CODEX_MODEL_CATALOG_PATH.read_bytes() + monkeypatch.setattr(codex, "agent_version", lambda _: "0.154.0") + + def reject(*args): + raise RuntimeError("catalog is incompatible") + + monkeypatch.setattr(codex, "prepare_codex_catalog", reject) + with pytest.raises(RuntimeError, match="incompatible"): + codex.write_tool_config({"workspace": WS, "codex_static_models": ["new-model"]}) + + assert "model_catalog_json" not in read_toml_safe( + codex.CODEX_CONFIG_PATH.parent / "config.toml" + ) + assert codex.CODEX_MODEL_CATALOG_PATH.read_bytes() == original_catalog + + def test_revert_preserves_subsequent_user_catalog(self, tmp_path): + codex.sync_app_model_catalog({"models": [{"slug": "gpt-mps"}]}) + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + original = 'model_catalog_json = "/user/models.json"\n' + shared_path.write_text(original, encoding="utf-8") + + assert codex.revert_legacy_shared_config() is False + assert shared_path.read_text() == original + class TestCodexDefaultModel: @pytest.fixture(autouse=True) @@ -834,6 +978,9 @@ def _patch(tmp_path, monkeypatch): lambda workspace, profile=None, force_refresh=False: "tok", ) monkeypatch.setattr(codex, "clear_model_preferences", lambda state: False) + # These launch tests isolate the real-binary validation boundary; its + # subprocess contract is covered in test_codex_catalog.py. + monkeypatch.setattr(codex, "validate_codex_catalog", lambda binary, catalog: None) return launches def test_sets_oauth_token(self, tmp_path, monkeypatch): @@ -849,11 +996,55 @@ def test_sets_oauth_token(self, tmp_path, monkeypatch): assert os.environ["OAUTH_TOKEN"] == "fresh-token" assert launches[0][-1] == "--search" + @pytest.mark.parametrize("custom_catalog", [None, "/user/isaac-app-model-catalog.json"]) + def test_native_update_detaches_catalog_without_discovery( + self, tmp_path, monkeypatch, custom_catalog + ): + self._patch(tmp_path, monkeypatch) + codex.sync_app_model_catalog({"models": [{"slug": "old-model"}]}) + shared_path = tmp_path / "config.toml" + if custom_catalog: + shared_path.write_text(f'model_catalog_json = "{custom_catalog}"\n') + profile_path = codex.CODEX_CONFIG_PATH + profile_path.write_text( + f'model_catalog_json = "{codex.CODEX_MODEL_CATALOG_PATH}"\n' + profile_path.read_text() + ) + monkeypatch.setattr( + codex, + "_fetch_codex_model_catalog", + lambda *a, **k: pytest.fail("update must not depend on gateway discovery"), + ) + monkeypatch.setattr( + codex, + "validate_codex_catalog", + lambda *a, **k: pytest.fail("update must not load an old catalog"), + ) + launches = [] + + def execute(argv): + assert read_toml_safe(shared_path).get("model_catalog_json") == custom_catalog + assert not any(arg.startswith("model_catalog_json=") for arg in argv) + launches.append(argv) + + monkeypatch.setattr(codex, "exec_or_spawn", execute) + codex.launch( + {"workspace": WS, "_codex_launch_provider": "main.default.openai"}, + ["update"], + options=LaunchOptions(), + ) + + assert len(launches) == 1 + assert launches[0][-1] == "update" + assert read_toml_safe(shared_path).get("model_catalog_json") == custom_catalog + def test_provider_discovery_uses_authoritative_catalog(self, tmp_path, monkeypatch): launches = self._patch(tmp_path, monkeypatch) catalog_path = tmp_path / "models.json" - catalog = {"models": [{"slug": "gpt-mps"}]} + app_catalog_path = tmp_path / "codex-model-catalog.json" + catalog = {"models": [{"slug": "gpt-mps", "future_metadata": {"tools": True}}]} fetch_kwargs = {} + validations = [] + monkeypatch.setattr(codex, "CODEX_MODEL_CATALOG_PATH", app_catalog_path) monkeypatch.setattr(codex, "_model_catalog_path", lambda workspace, provider: catalog_path) monkeypatch.setattr( codex, @@ -861,13 +1052,24 @@ def test_provider_discovery_uses_authoritative_catalog(self, tmp_path, monkeypat lambda workspace, token, **kwargs: fetch_kwargs.update(kwargs) or catalog, ) + def validate(binary, candidate): + assert not catalog_path.exists() + assert not app_catalog_path.exists() + validations.append((binary, candidate)) + + monkeypatch.setattr(codex, "validate_codex_catalog", validate) codex.launch( {"workspace": WS, "_codex_launch_provider": "main.default.openai"}, [], options=LaunchOptions(), ) - assert catalog_path.exists() + assert validations == [(codex.SPEC["binary"], catalog)] + assert json.loads(catalog_path.read_text()) == catalog + assert json.loads(app_catalog_path.read_text()) == catalog + assert read_toml_safe(tmp_path / "config.toml")["model_catalog_json"] == str( + app_catalog_path + ) assert f'model_catalog_json="{catalog_path}"' in launches[0] assert fetch_kwargs == { "source": codex.CodexCatalogSource.PROVIDER, @@ -881,6 +1083,63 @@ def test_provider_discovery_uses_authoritative_catalog(self, tmp_path, monkeypat ) assert 'Databricks-Model-Provider-Service = "main.default.openai"' in provider_arg + @pytest.mark.parametrize("custom_catalog", [None, "/user/isaac-app-model-catalog.json"]) + def test_incompatible_discovery_removes_only_ug_reference( + self, tmp_path, monkeypatch, custom_catalog + ): + launches = self._patch(tmp_path, monkeypatch) + codex.sync_app_model_catalog({"models": [{"slug": "old-model"}]}) + original_catalog = codex.CODEX_MODEL_CATALOG_PATH.read_bytes() + shared_path = tmp_path / "config.toml" + if custom_catalog: + shared_path.write_text(f'model_catalog_json = "{custom_catalog}"\n') + monkeypatch.setattr( + codex, "_fetch_codex_model_catalog", lambda *a, **k: {"models": [{"slug": "new"}]} + ) + + def reject(binary, catalog): + raise RuntimeError("The installed Codex cannot load this model catalog") + + monkeypatch.setattr(codex, "validate_codex_catalog", reject) + + with pytest.raises(RuntimeError, match="cannot load this model catalog"): + codex.launch( + {"workspace": WS, "_codex_launch_provider": "main.default.openai"}, + [], + options=LaunchOptions(), + ) + + assert not launches + assert read_toml_safe(shared_path).get("model_catalog_json") == custom_catalog + assert codex.CODEX_MODEL_CATALOG_PATH.read_bytes() == original_catalog + assert not list(codex.CODEX_MODEL_CATALOG_PATH.parent.glob("codex-model-catalog-*.json")) + + def test_provider_discovery_preserves_custom_app_catalog(self, tmp_path, monkeypatch, capsys): + launches = self._patch(tmp_path, monkeypatch) + catalog_path = tmp_path / "models.json" + app_catalog_path = tmp_path / "codex-model-catalog.json" + shared_path = tmp_path / "config.toml" + shared_path.write_text('model_catalog_json = "/user/models.json"\n', encoding="utf-8") + monkeypatch.setattr(codex, "CODEX_MODEL_CATALOG_PATH", app_catalog_path) + monkeypatch.setattr(codex, "_model_catalog_path", lambda workspace, scope: catalog_path) + monkeypatch.setattr( + codex, + "_fetch_codex_model_catalog", + lambda workspace, token, **kwargs: {"models": [{"slug": "gpt-mps"}]}, + ) + + codex.launch( + {"workspace": WS, "_codex_launch_provider": "main.default.openai"}, + [], + options=LaunchOptions(), + ) + + assert launches + assert read_toml_safe(shared_path)["model_catalog_json"] == "/user/models.json" + output = capsys.readouterr() + assert "leaving it unchanged" in " ".join(output.err.split()) + assert "daemon restart" not in output.err + def test_provider_pins_first_catalog_model(self, tmp_path, monkeypatch): launches = self._patch(tmp_path, monkeypatch) catalog = {"models": [{"slug": "gpt-primary"}, {"slug": "gpt-secondary"}]} @@ -1035,12 +1294,19 @@ def fetch(workspace, token, **kwargs): ] assert fetched == ["main.first", "main.second"] assert catalog_args[0] != catalog_args[1] + assert json.loads(codex.CODEX_MODEL_CATALOG_PATH.read_text()) == { + "models": [{"slug": "main.second"}] + } + assert read_toml_safe(tmp_path / "config.toml")["model_catalog_json"] == str( + codex.CODEX_MODEL_CATALOG_PATH + ) @pytest.mark.parametrize("tool_args", [[], ["--model", "gpt-mps"]]) def test_provider_launches_when_discovery_is_unavailable( self, tmp_path, monkeypatch, tool_args ): launches = self._patch(tmp_path, monkeypatch) + codex.sync_app_model_catalog({"models": [{"slug": "previous-workspace"}]}) monkeypatch.setattr( codex, "_fetch_codex_model_catalog", @@ -1061,6 +1327,7 @@ def test_provider_launches_when_discovery_is_unavailable( if tool_args: assert launches[0][-len(tool_args) :] == tool_args assert not any(arg.startswith("model_catalog_json=") for arg in launches[0]) + assert "model_catalog_json" not in read_toml_safe(tmp_path / "config.toml") provider_arg = next( arg for arg in launches[0] if arg.startswith("model_providers.Databricks=") ) @@ -1083,6 +1350,27 @@ def test_provider_keeps_other_discovery_failures_fatal(self, tmp_path, monkeypat assert launches == [] + def test_discovery_error_survives_unreadable_shared_config(self, tmp_path, monkeypatch, capsys): + launches = self._patch(tmp_path, monkeypatch) + shared_path = tmp_path / "config.toml" + original = 'model = "unfinished\n' + shared_path.write_text(original) + + def fail_discovery(*args, **kwargs): + raise RuntimeError("HTTP 403 Forbidden") + + monkeypatch.setattr(codex, "_fetch_codex_model_catalog", fail_discovery) + with pytest.raises(RuntimeError, match="HTTP 403 Forbidden"): + codex.launch( + {"workspace": WS, "_codex_launch_provider": "main.default.openai"}, + [], + options=LaunchOptions(), + ) + + assert not launches + assert shared_path.read_text() == original + assert "Cannot update Codex App settings" in " ".join(capsys.readouterr().err.split()) + def test_provider_rejects_managed_model_catalog(self, tmp_path, monkeypatch): launches = self._patch(tmp_path, monkeypatch) managed_path = tmp_path / "managed_config.toml" diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index 99caa179..1c731103 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -588,6 +588,15 @@ def _boom(*a, **k): # pragma: no cover - must never run class TestInstallToolBinary: + @staticmethod + def _seed_codex_catalog_reference(catalog_ref: str | None = None): + codex = agents_mod.codex + shared_path = codex.CODEX_CONFIG_PATH.parent / "config.toml" + shared_path.parent.mkdir(parents=True, exist_ok=True) + reference = catalog_ref or str(codex.CODEX_MODEL_CATALOG_PATH) + shared_path.write_text(f'model_catalog_json = "{reference}"\n', encoding="utf-8") + return shared_path + def test_non_strict_returns_false_when_npm_missing(self, monkeypatch): monkeypatch.setattr("ucode.agents.shutil.which", lambda _: None) @@ -656,6 +665,88 @@ def test_required_update_prompts_and_rechecks(self, monkeypatch, tool, command): assert calls == [command] assert prompts == [(f"Upgrade {TOOL_SPECS[tool]['display']} if available?", True)] + @pytest.mark.parametrize( + "catalog_ref", + [None, "/user/isaac-app-model-catalog.json"], + ids=["ug-catalog", "custom-catalog"], + ) + def test_codex_update_detaches_only_ug_catalog_before_mutation(self, monkeypatch, catalog_ref): + shared_path = self._seed_codex_catalog_reference(catalog_ref) + calls = [] + + monkeypatch.setattr("ucode.agents.shutil.which", lambda binary: f"/usr/bin/{binary}") + monkeypatch.setattr("ucode.agents._too_new_downgrade", lambda _: None) + monkeypatch.setattr("ucode.agents._minimum_version_error", lambda _: None) + + def fake_run(args, **kwargs): + calls.append(args) + contents = shared_path.read_text(encoding="utf-8") + if catalog_ref is None: + assert "model_catalog_json" not in contents + else: + assert f'model_catalog_json = "{catalog_ref}"' in contents + return subprocess.CompletedProcess(args, 0) + + monkeypatch.setattr("ucode.agents.subprocess.run", fake_run) + + assert agents_mod._update_installed_tool_binary("codex") is True + assert calls == [["codex", "update"]] + + @pytest.mark.parametrize("installed", [False, True], ids=["install", "update"]) + def test_codex_install_or_update_failure_leaves_catalog_detached(self, monkeypatch, installed): + shared_path = self._seed_codex_catalog_reference() + + monkeypatch.setattr( + "ucode.agents.shutil.which", + lambda binary: f"/usr/bin/{binary}" if installed or binary == "npm" else None, + ) + monkeypatch.setattr("ucode.agents._minimum_version_error", lambda _: "must upgrade") + monkeypatch.setattr("ucode.agents._too_new_downgrade", lambda _: None) + + def fail_run(args, **kwargs): + assert "model_catalog_json" not in shared_path.read_text(encoding="utf-8") + raise subprocess.CalledProcessError(1, args) + + monkeypatch.setattr("ucode.agents.subprocess.run", fail_run) + + if installed: + assert agents_mod._update_installed_tool_binary("codex") is False + else: + assert install_tool_binary("codex", strict=False) is False + assert "model_catalog_json" not in shared_path.read_text(encoding="utf-8") + + def test_catalog_detach_failure_blocks_codex_binary_mutation(self, monkeypatch): + calls = [] + + monkeypatch.setattr("ucode.agents.shutil.which", lambda binary: f"/usr/bin/{binary}") + monkeypatch.setattr( + agents_mod.codex, + "detach_app_model_catalog", + lambda: (_ for _ in ()).throw(RuntimeError("cannot detach catalog")), + ) + monkeypatch.setattr( + "ucode.agents.subprocess.run", lambda args, **kwargs: calls.append(args) + ) + + with pytest.raises(RuntimeError, match="cannot detach catalog"): + agents_mod._update_installed_tool_binary("codex") + assert calls == [] + + def test_claude_update_does_not_detach_codex_catalog(self, monkeypatch): + shared_path = self._seed_codex_catalog_reference() + calls = [] + + monkeypatch.setattr("ucode.agents.shutil.which", lambda binary: f"/usr/bin/{binary}") + monkeypatch.setattr("ucode.agents._minimum_version_error", lambda _: None) + monkeypatch.setattr( + "ucode.agents.subprocess.run", + lambda args, **kwargs: calls.append(args) or subprocess.CompletedProcess(args, 0), + ) + + assert agents_mod._update_installed_tool_binary("claude") is True + assert calls == [["claude", "upgrade"]] + assert "model_catalog_json" in shared_path.read_text(encoding="utf-8") + @pytest.mark.parametrize("tool", ["claude", "codex"]) def test_required_update_declined_blocks_launch(self, monkeypatch, tool): monkeypatch.setattr("ucode.agents.shutil.which", lambda binary: f"/usr/bin/{binary}") diff --git a/tests/test_codex_catalog.py b/tests/test_codex_catalog.py index be93b154..6f00ee13 100644 --- a/tests/test_codex_catalog.py +++ b/tests/test_codex_catalog.py @@ -166,6 +166,7 @@ def run(argv, **kwargs): homes.append(home) assert kwargs["cwd"] == str(home) assert home.exists() + assert kwargs["stdin"] is subprocess.DEVNULL assert kwargs["timeout"] == 30 assert kwargs["check"] is True if len(calls) == 1: @@ -186,6 +187,75 @@ def run(argv, **kwargs): assert not homes[0].exists() +def test_validate_discovered_catalog_with_same_binary_in_isolation(monkeypatch): + discovered = { + "models": [ + { + "slug": "system.ai.gpt-6-astra", + "future_field": {"nested": [1, "keep"]}, + "visibility": "list", + } + ], + "future_catalog_field": {"enabled": True}, + } + original = copy.deepcopy(discovered) + calls = [] + homes = [] + + def run(argv, **kwargs): + calls.append(argv) + home = Path(kwargs["env"]["CODEX_HOME"]) + homes.append(home) + assert argv[0] == "/selected/codex" + assert argv[-2:] == ["debug", "models"] + assert kwargs["cwd"] == str(home) + assert kwargs["env"]["CODEX_HOME"] != "/user/codex-home" + assert kwargs["stdin"] is subprocess.DEVNULL + assert kwargs["timeout"] == 30 + assert kwargs["check"] is True + assert kwargs["capture_output"] is True + assert kwargs["text"] is True + setting = json.loads(argv[2].split("=", 1)[1]) + candidate = Path(setting) + assert candidate.parent == home + assert json.loads(candidate.read_text()) == discovered + return subprocess.CompletedProcess(argv, 0, "") + + monkeypatch.setenv("CODEX_HOME", "/user/codex-home") + monkeypatch.setattr(catalog.subprocess, "run", run) + + assert catalog.validate_codex_catalog("/selected/codex", discovered) is None + + assert len(calls) == 1 + assert calls[0][1] == "-c" + assert discovered == original + assert not homes[0].exists() + + +@pytest.mark.parametrize( + "error", + [ + FileNotFoundError("codex not found"), + subprocess.TimeoutExpired("codex", 30), + subprocess.CalledProcessError(1, ["codex"], stderr="catalog secret"), + ], + ids=["missing-binary", "timeout", "rejected-catalog"], +) +def test_validate_discovered_catalog_failure_is_actionable_without_payload(monkeypatch, error): + catalog_payload = {"models": [{"slug": "system.ai.secret-model"}]} + + def run(*args, **kwargs): + raise error + + monkeypatch.setattr(catalog.subprocess, "run", run) + + with pytest.raises(RuntimeError, match="Update Codex") as raised: + catalog.validate_codex_catalog("/selected/codex", catalog_payload) + + assert "secret-model" not in str(raised.value) + assert "catalog secret" not in str(raised.value) + + def test_missing_gpt_warning_is_printed_after_validation(monkeypatch, bundled): calls = [] warnings = []