diff --git a/README.md b/README.md index 7030ef97..a5fb6379 100644 --- a/README.md +++ b/README.md @@ -243,12 +243,12 @@ ucode publish # publish it to the workspace ``` `ucode setup` walks through the agents to enable and which one bare `ucode` launches, then per agent: -Databricks-hosted models or an external Model Provider Service, the models to expose, and (for Codex) -whether the config writes the agent's own OS-level settings file or a ucode-only one. Interactive -Claude Code configuration installs its gateway configuration in the OS-managed settings scope so -enterprise settings cannot silently override ucode. Non-interactive and CI runs use the local file -without invoking `sudo`, and stop with an actionable error if an existing managed value conflicts. -Claude subscription relay is local-only because its loopback proxy exists only for that session. +Databricks-hosted models or an external Model Provider Service and the models to expose. Interactive +Claude Code and Codex configuration installs gateway-critical values in the OS-managed settings +scope so enterprise settings cannot silently override ucode. Non-interactive and CI runs use local +files without invoking `sudo`, and stop with an actionable error if an existing managed value +conflicts. Claude subscription relay is local-only because its loopback proxy exists only for that +session. Claude Code is asked one model per family (opus/sonnet/haiku/fable), since it selects models by family alias; any family can be skipped. @@ -386,6 +386,7 @@ control the installation. | `~/.codex/ucode.config.toml` (or legacy `~/.codex/config.toml`) | Codex | | `~/.claude/ucode-settings.json` | Claude Code settings generated by ucode | | `/etc/claude-code/managed-settings.json` (Linux) or `/Library/Application Support/ClaudeCode/managed-settings.json` (macOS) | Claude Code OS-managed settings | +| `/etc/codex/managed_config.toml` | Codex OS-managed settings | | `~/.gemini/.env` | Gemini CLI | | `~/.config/opencode/opencode.json` | OpenCode | | `~/.copilot/.env` | GitHub Copilot CLI | diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index 16a0c8f1..4f75e6bc 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -3,15 +3,14 @@ ## Summary Claude Code and Codex give OS-managed settings higher precedence than user settings. Previously, -ucode wrote only its local configuration unless an administrator enabled -`use_as_global_settings`. An existing machine-managed file could therefore silently override the -gateway endpoint, authentication helper, provider headers, or model selected by ucode. +ucode could write only its local configuration while an existing machine-managed file silently +overrode the gateway endpoint, authentication helper, provider headers, or model. The two stacked PRs make precedence handling deterministic: 1. The Claude PR adds the shared managed-file lifecycle and applies it to Claude Code. -2. The Codex PR reuses that lifecycle for TOML, applies it to Codex, and removes - `use_as_global_settings`. +2. The Codex PR reuses that lifecycle for TOML, applies it to Codex, and removes the old optional + managed-settings path. After both PRs merge, interactive configuration reconciles the agent's OS-managed file by default. Non-interactive and CI execution never elevates privileges and instead uses local settings when the @@ -59,7 +58,7 @@ request administrator permission. A first-time non-interactive launch remains lo For each agent, ucode: 1. Strictly parses the existing managed JSON or TOML document. -2. Produces the desired document by applying the same overlay used for the local ucode file. +2. Produces the desired document by applying the same gateway overlay used for the local ucode file. 3. Preserves settings outside the paths owned by ucode. 4. Preserves enterprise Claude permission-deny entries while adding ucode-required entries. 5. Records the original baseline before the first change. @@ -204,19 +203,6 @@ Write, verification, parse, symlink, and managed-conflict failures block the age information always identifies the file and recommends either an interactive configure/revert or administrator help. -## Removal of `use_as_global_settings` - -The final behavior has no managed-config scope choice: - -- Claude and Codex reconcile OS-managed settings automatically during interactive configuration. -- Other agents continue using their existing local configuration because they do not have the same - supported self-refreshing managed-file path. -- The Codex PR removes `use_as_global_settings` from schemas, resolution, setup prompts, summaries, - documentation, and tests. - -The Claude PR temporarily leaves Codex's legacy interpretation in place so that the first PR is -independently safe. The stacked Codex PR removes the remaining field and transitional code. - ## PR Boundaries ### PR 1: Claude Code @@ -227,7 +213,7 @@ independently safe. The stacked Codex PR removes the remaining field and transit - Add non-interactive local fallback with conflict detection. - Add Claude relay-specific safety checks. - Add Claude managed status and revert output. -- Remove Claude from the legacy `use_as_global_settings` setup choice. +- Remove Claude's old managed-settings scope choice. ### PR 2: Codex @@ -235,4 +221,6 @@ independently safe. The stacked Codex PR removes the remaining field and transit - Make interactive Codex configuration reconcile OS-managed TOML by default. - Add non-interactive local fallback with conflict detection. - Add Codex fingerprinted cached launches, status, and revert behavior. -- Remove all remaining `use_as_global_settings` code and documentation. +- Keep opt-in smart-routing hooks in the local Codex config rather than adding them by default to + machine-managed policy. +- Remove the remaining managed-settings scope schema, resolution, setup prompt, summary, and tests. diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index 0cd4d2ce..85a898c9 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -71,11 +71,7 @@ DEFAULT_TOOL = "codex" BUNDLE_VERSION = 1 -_MANAGED_SETTINGS_TOOLS = {"claude"} - -# Codex still honors the legacy managed-config opt-in until its follow-up migration. Claude always -# reconciles its OS-managed settings and therefore no longer appears in the setup prompt. -GLOBAL_SETTINGS_AGENTS = frozenset({"codex"}) +_MANAGED_SETTINGS_TOOLS = {"claude", "codex"} # ucode tool -> `databricks aitools` agent id. gemini/pi aren't supported. AITOOLS_AGENT_TOKENS = { diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 3f80bb3c..766b6852 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -2,14 +2,17 @@ from __future__ import annotations +import copy import os import re import subprocess import sys import time +from collections.abc import Callable from pathlib import Path import tomlkit +from tomlkit.exceptions import ParseError from ucode.config_io import ( APP_DIR, @@ -25,7 +28,18 @@ get_databricks_token, ) from ucode.launcher import exec_or_spawn -from ucode.managed_files import OS, current_os, write_managed_file +from ucode.managed_files import ( + OS, + current_os, + managed_file_conflicts, + managed_file_is_verified, + managed_file_status, + managed_writes_allowed, + mark_managed_file_verified, + read_managed_file, + reconcile_managed_file, + revert_managed_file, +) from ucode.smart_routing.codex_hooks import ( remove_smart_routing_hooks, routing_models, @@ -335,27 +349,24 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non use_pat=bool(state.get("use_pat")), provider=provider, ) + + def compose(base: dict) -> dict: + deep_merge_dict(base, copy.deepcopy(overlay)) + # deep_merge can't drop keys, so clear model preferences from an earlier run. + if chosen_model is None: + for key in ("model", "model_reasoning_effort"): + base.pop(key, None) + return base + doc = read_toml_safe(CODEX_CONFIG_PATH) - deep_merge_dict(doc, overlay) - # deep_merge can't drop keys, so clear model preferences from an earlier run. - if chosen_model is None: - for key in ("model", "model_reasoning_effort"): - doc.pop(key, None) + compose(doc) sync_smart_routing_hooks( doc, state, enabled=smart_routing_enabled(state) and provider is None, ) write_toml_file(CODEX_CONFIG_PATH, doc) - # use_as_global_settings: also write the modern overlay to Codex's OS managed config - # (/etc/codex/managed_config.toml), the highest-precedence scope a bare `codex` reads — so it - # defaults to the gateway without `--profile ucode`. codex auth self-refreshes via - # `ucode auth-token`, so the file keeps working. The write goes through the sudo path in - # `managed_files`. - if state.get("write_managed_config"): - _write_managed_config( - workspace, None, databricks_profile, bool(state.get("use_pat")), provider - ) + _reconcile_managed_config(state, compose) state = mark_tool_managed(state, "codex", MANAGED_KEYS) save_state(state) return state @@ -370,44 +381,85 @@ def _is_gpt_family(model: str) -> bool: def _managed_config_path() -> Path | None: - """OS-level Codex managed config file, or None on unsupported platforms. - - Linux and macOS use ``/etc/codex/managed_config.toml`` (root-owned, highest precedence). See - https://learn.chatgpt.com/docs/enterprise/managed-configuration. Codex also supports a - ``~/.codex/managed_config.toml`` on Windows, but ucode's write path is sudo/Unix-only - (see :func:`managed_files.managed_files_supported`), so Windows returns None here too. - """ + """Return Codex's managed config path on platforms supported by ucode's sudo writer.""" if current_os() in (OS.LINUX, OS.MACOS): return Path("/etc/codex/managed_config.toml") return None -def _write_managed_config( - workspace: str, - model: str | None, - databricks_profile: str | None, - use_pat: bool, - provider: str | None, -) -> None: - """Merge the modern overlay into Codex's OS managed_config.toml, preserving any other keys there. +def _parse_managed_config(text: str) -> dict: + try: + return tomlkit.parse(text) + except ParseError as exc: + raise RuntimeError(f"invalid TOML: {exc}") from exc - Written via the sudo path in `managed_files` (drift-suppressed). - """ + +def managed_config_is_current(state: dict) -> bool: + path = _managed_config_path() + if path is None: + return True + required_scope = "managed" if managed_writes_allowed() else None + return managed_file_is_verified(state, "codex", path, required_scope=required_scope) + + +def managed_config_status(state: dict) -> tuple[Path | None, str, str]: + path = _managed_config_path() + status, backup = managed_file_status(state, "codex", path, parser=_parse_managed_config) + return path, status, backup + + +def revert_managed_config() -> str: + return revert_managed_file( + "codex", + display="Codex", + parser=_parse_managed_config, + dumper=tomlkit.dumps, + ) + + +def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> None: + """Reconcile Codex's highest-precedence config while preserving unrelated policy.""" path = _managed_config_path() if path is None: print_warning_err( "Machine-wide Codex settings aren't supported on this platform; skipped the managed " - "config write." + "config." ) return - overlay = render_overlay( - workspace, model, databricks_profile, use_pat=use_pat, provider=provider + if path.is_symlink(): + raise RuntimeError( + f"Refusing to use Codex managed settings through symlink {path}. Replace it with a " + "regular file or contact your administrator." + ) + current_text = read_managed_file(path) + try: + existing = _parse_managed_config(current_text) if current_text is not None else {} + except RuntimeError as exc: + raise RuntimeError( + f"Cannot safely update Codex managed settings at {path}: {exc}. ucode did not modify " + "the file. Repair it or contact your administrator." + ) from exc + managed_before = copy.deepcopy(existing) + desired_doc = compose(existing) + if not managed_writes_allowed(): + conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) + if conflicts: + raise RuntimeError( + "Codex configuration cannot be applied non-interactively because OS-managed " + f"settings at {path} override ucode values: {', '.join(conflicts)}. Run `ucode " + "configure --agent codex` from an interactive terminal or contact your " + "administrator." + ) + mark_managed_file_verified(state, "codex", path, scope="local-compatible") + return + reconcile_managed_file( + path, + tomlkit.dumps(desired_doc), + tool="codex", + display="Codex", + owned_paths=MANAGED_KEYS, ) - doc = read_toml_safe(path) - deep_merge_dict(doc, overlay) - # deep_merge can't drop keys, so clear a model pinned by an earlier run. - doc.pop("model", None) - write_managed_file(path, tomlkit.dumps(doc), display="Codex") + mark_managed_file_verified(state, "codex", path) def default_model(state: dict) -> str | None: @@ -501,8 +553,8 @@ def _app_server_start_model() -> str: # since execvp replaces this process. print_warning_err( "ucode's `--profile` isn't accepted here (error above). Retrying " - f"without it: this run uses {LEGACY_CODEX_CONFIG_PATH}, NOT the " - "Databricks gateway." + f"without it: Codex will resolve {LEGACY_CODEX_CONFIG_PATH} and any OS-managed " + "settings instead of the ucode profile." ) exec_or_spawn([binary, *tool_args]) return # unreachable in production (exec replaces the process) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index a94eaa54..569cdf0f 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1049,6 +1049,11 @@ def status() -> int: print_kv("OS-managed settings", managed_status) print_kv("Managed settings file", str(managed_path) if managed_path else "unsupported") print_kv("Managed settings backup", backup_status) + elif tool == "codex": + managed_path, managed_status, backup_status = codex_agent.managed_config_status(state) + print_kv("OS-managed settings", managed_status) + print_kv("Managed settings file", str(managed_path) if managed_path else "unsupported") + print_kv("Managed settings backup", backup_status) console.print() print_heading("Skills") @@ -1105,6 +1110,7 @@ def revert() -> int: managed_configs = state.get("managed_configs") or {} mcp_results = revert_mcp_configs(state) claude_managed_result = claude_agent.revert_managed_settings() + codex_managed_result = codex_agent.revert_managed_config() results: dict[str, bool] = { tool: restore_file( @@ -1127,6 +1133,7 @@ def revert() -> int: if legacy_codex_stripped: print_kv("Codex shared config", "ucode entries removed") print_kv("Claude Code OS-managed settings", claude_managed_result) + print_kv("Codex OS-managed settings", codex_managed_result) print_kv("Pi settings", "restored" if pi_settings_restored else "unchanged") for client, spec in MCP_CLIENTS.items(): print_kv( @@ -1872,7 +1879,7 @@ def _can_launch_from_cached_config( claude_agent.CLAUDE_SETTINGS_PATH.exists() and claude_agent.managed_settings_are_current(state) ) - return codex_agent.has_ucode_config() + return codex_agent.has_ucode_config() and codex_agent.managed_config_is_current(state) def _launch_tool( diff --git a/src/ucode/managed_config.py b/src/ucode/managed_config.py index afb3248c..7bbf512b 100644 --- a/src/ucode/managed_config.py +++ b/src/ucode/managed_config.py @@ -150,8 +150,6 @@ def _normalize_enabled_agent(entry: object) -> tuple[str, dict] | None: return None config_in = _as_dict(entry_dict.get("config")) agent_config: dict = {} - if isinstance(config_in.get("use_as_global_settings"), bool): - agent_config["use_as_global_settings"] = config_in["use_as_global_settings"] headers = config_in.get("custom_headers") if isinstance(headers, dict): clean = { diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 1d78e5e1..500cb0c7 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -10,7 +10,6 @@ import hashlib import json import os -import shlex import subprocess import sys import tempfile @@ -22,7 +21,7 @@ from typing import Any, cast from ucode.config_io import APP_DIR, is_dry_run -from ucode.ui import console, print_err, print_note, print_success, print_warning +from ucode.ui import console, print_note, print_success, print_warning # Absolute path so a stripped PATH (desktop/GUI launchers) still finds it. _SUDO = "/usr/bin/sudo" @@ -67,14 +66,6 @@ def managed_files_supported() -> bool: return current_os() in (OS.LINUX, OS.MACOS) -def _read_existing(path: Path) -> str: - """Current file contents, or "" when absent. No sudo — the managed file is world-readable.""" - try: - return path.read_text(encoding="utf-8") if path.exists() else "" - except OSError: - return "" - - def read_managed_file(path: Path) -> str | None: """Read a managed file strictly, returning ``None`` only when it is absent.""" try: @@ -622,45 +613,6 @@ def _sudo_remove(path: Path) -> None: _restore_immutable(path, original_flags) -def write_managed_file(path: Path, desired_text: str, *, display: str) -> str: - """Write ``desired_text`` to a root-owned managed file, only when it differs (drift check). - - Returns ``"written"``, ``"unchanged"``, or ``"skipped"``. Never raises: a permission or immutable - failure is surfaced as an actionable message and reported as ``"skipped"`` so the launch still - proceeds (the private ucode config already lets ``ucode `` work). - """ - if not managed_files_supported(): - print_warning( - f"{display}: machine-wide managed settings aren't supported on this platform; " - f"skipped {path}." - ) - return "skipped" - # Drift check first — reading is unprivileged, so an unchanged file never triggers a sudo prompt. - if _read_existing(path) == desired_text: - return "unchanged" - if is_dry_run(): - console.print(f"\n[bold]\\[dry run] {path} (via sudo)[/bold]\n{desired_text}") - return "written" - if not managed_writes_allowed(): - print_warning( - f"{display}: skipped the OS-managed settings update at {path} because the command " - "is non-interactive." - ) - return "skipped" - try: - _sudo_replace(path, desired_text) - except PermissionError as exc: - print_err( - f"{display}: cannot write {path} without root ({exc}). Re-run with `sudo ucode ...` to " - "apply the config machine-wide." - ) - return "skipped" - except subprocess.CalledProcessError as exc: - _report_sudo_failure(path, display, exc) - return "skipped" - return "written" - - def _sudo_replace(path: Path, desired_text: str) -> None: """Atomically replace ``path`` via sudo while preserving metadata and file flags.""" if not managed_writes_allowed(): @@ -789,23 +741,6 @@ def _restore_immutable(path: Path, flags: tuple[str, ...]) -> None: print_warning(f"Could not restore the immutable flag on {path}.") -def _report_sudo_failure(path: Path, display: str, exc: subprocess.CalledProcessError) -> None: - """Surface a sudo helper failure with a concrete fix. An immutable destination is the common - cause — cp fails with EPERM even under root — so point at the OS-specific clear command.""" - stderr = (exc.stderr or "").strip() if isinstance(exc.stderr, str) else "" - cmd = exc.cmd or [] - cp_failed = "cp" in cmd[1:3] - if cp_failed and "Operation not permitted" in stderr: - quoted = shlex.quote(str(path)) - clear_cmd = f"sudo {'chflags noschg' if current_os() is OS.MACOS else 'chattr -i'} {quoted}" - print_err( - f"{display}: {path} appears to be immutable. Clear the immutable attribute and re-run:\n" - f" {clear_cmd}\n ucode ..." - ) - else: - print_err(f"{display}: failed to write managed settings at {path}: {stderr or exc}") - - def _sudo_failure_message(path: Path, display: str, exc: subprocess.CalledProcessError) -> str: stderr = (exc.stderr or "").strip() if isinstance(exc.stderr, str) else "" cmd = exc.cmd or [] diff --git a/src/ucode/managed_resolve.py b/src/ucode/managed_resolve.py index 811b9f1f..b6658d4e 100644 --- a/src/ucode/managed_resolve.py +++ b/src/ucode/managed_resolve.py @@ -21,7 +21,6 @@ from typing import cast -from ucode.agents import GLOBAL_SETTINGS_AGENTS from ucode.databricks import ANTHROPIC_FAMILIES, classify_model_family from ucode.state import MANAGED_OVERLAY_KEY @@ -177,21 +176,6 @@ def managed_provider_service(managed: dict, tool: str) -> str | None: return _str(_agent_model_config(managed, tool).get("model_provider_service")) -def managed_use_as_global_settings(managed: dict, tool: str) -> bool: - """True when the admin marked ``tool`` machine-wide AND ``tool`` can support it. - - ``use_as_global_settings`` means: also write the agent's OS-level managed settings file - (``/etc/claude-code/managed-settings.json``, ``/etc/codex/managed_config.toml``) so a bare - ``claude`` / ``codex`` picks up the gateway config. Only agents in - :data:`~ucode.agents.GLOBAL_SETTINGS_AGENTS` have such a file, so the flag is ignored for any - other agent — a hand-written ``--from-file`` config can't turn it on for an agent that has no - managed settings path. - """ - if tool not in GLOBAL_SETTINGS_AGENTS: - return False - return bool(_agent_entry(managed, tool).get("use_as_global_settings")) - - def managed_default_model(managed: dict, tool: str) -> str | None: """Return the model the managed config wants ``tool`` to launch on, if it names one. @@ -290,12 +274,6 @@ def resolve_state(managed: dict, state: dict, tool: str) -> dict: overlay["provider_services"] = state.get("provider_services") providers[tool] = provider resolved["provider_services"] = providers - if managed_use_as_global_settings(managed, tool): - # Transient: recorded in the overlay so `save_state` strips it before persisting. It exists - # only for this config-write, telling the agent's `write_tool_config` to also write the OS - # managed settings file. A non-managed launch never sets it, so default behavior is unchanged. - overlay["write_managed_config"] = state.get("write_managed_config") - resolved["write_managed_config"] = True if overlay: resolved[MANAGED_OVERLAY_KEY] = overlay return resolved diff --git a/src/ucode/managed_setup.py b/src/ucode/managed_setup.py index 243ed0b3..3c52f909 100644 --- a/src/ucode/managed_setup.py +++ b/src/ucode/managed_setup.py @@ -218,9 +218,6 @@ def _model_config_payload(tool: str, model_config: dict) -> dict: def _enabled_agent_payload(tool: str, agent_config: dict) -> dict: """Build one ``EnabledAgent`` entry (agent enum + its ``AgentConfig``).""" config: dict = {} - use_as_global = agent_config.get("use_as_global_settings") - if isinstance(use_as_global, bool): - config["use_as_global_settings"] = use_as_global headers = agent_config.get("custom_headers") if isinstance(headers, dict): clean = {k: v for k, v in headers.items() if isinstance(k, str) and isinstance(v, str)} diff --git a/src/ucode/managed_wizard.py b/src/ucode/managed_wizard.py index 3710c68b..6854f833 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -25,7 +25,7 @@ from typing import cast from ucode import config_io -from ucode.agents import GLOBAL_SETTINGS_AGENTS, TOOL_SPECS, check_gateway_endpoint +from ucode.agents import TOOL_SPECS, check_gateway_endpoint from ucode.databricks import ( ANTHROPIC_FAMILIES, all_users_can_use_schema, @@ -81,14 +81,6 @@ spinner, ) -# The OS-level managed settings file `use_as_global_settings` writes for each agent — named in the -# prompt so an admin sees exactly what answering "yes" touches. Yes writes this file (needs sudo -# once) so a bare `claude`/`codex` reaches the gateway on its own; no keeps it ucode-only. -GLOBAL_SETTINGS_FILES = { - "claude": "managed-settings.json", - "codex": "managed_config.toml", -} - # Shown whenever the workspace's coding-agent-config APIs return FEATURE_DISABLED. CODING_AGENT_CONFIGS_DISABLED_MESSAGE = ( "Workspace-managed coding agent configuration is not available on this workspace. Use " @@ -429,9 +421,6 @@ def _confirm_agent(tool: str, agent_config: dict) -> None: provider = model_config.get("model_provider_service") if provider: detail = f"{detail} via {provider}" - if tool in GLOBAL_SETTINGS_AGENTS: - scope = "global settings" if agent_config.get("use_as_global_settings") else "ucode-only" - detail = f"{detail} · {scope}" print_success(f"{display} configured — {detail}") @@ -1085,14 +1074,7 @@ def _render_summary(workspace: str, manifest: dict) -> None: provider = model_config.get("model_provider_service") if provider: detail = f"{detail} via {provider}" - # Only agents that can use global settings carry the scope label; for the rest it's not a choice. - if tool in GLOBAL_SETTINGS_AGENTS: - scope = ( - "global settings" if agent_config.get("use_as_global_settings") else "ucode-only" - ) - lines.append(kv_line(display, f"{detail} ({scope})")) - else: - lines.append(kv_line(display, detail)) + lines.append(kv_line(display, detail)) # Spell out the per-family slots and model lists: the one-line default alone doesn't show # which families an admin configured, which is most of what they chose for claude. models = model_config.get("models") @@ -1170,12 +1152,6 @@ def _config_facts(manifest: dict) -> list[tuple[str, str, str]]: facts.append((f"agent:{tool}:model:{family}", f"{display} ({family})", str(model))) elif isinstance(models, list) and len(models) > 1: facts.append((f"agent:{tool}:models", f"{display} models", ", ".join(map(str, models)))) - if tool in GLOBAL_SETTINGS_AGENTS: - scope = ( - "global settings" if agent_config.get("use_as_global_settings") else "ucode-only" - ) - facts.append((f"agent:{tool}:scope", f"{display} settings", scope)) - for server in manifest.get("mcp_servers") or []: name = str(server.get("name")) facts.append((f"mcp:{name}", f"MCP server {name}", str(server.get("type") or ""))) @@ -1650,16 +1626,6 @@ def setup_command( agent_config: dict = { "model_config": _prompt_models_for_agent(tool, state, provider_service) } - # Only claude and codex have an OS-level managed settings file that a bare `claude`/`codex` - # reads (`/etc/claude-code/managed-settings.json`, `/etc/codex/managed_config.toml`); the - # other agents don't, so we don't offer them the choice. - if tool in GLOBAL_SETTINGS_AGENTS: - binary = TOOL_SPECS[tool]["binary"] - agent_config["use_as_global_settings"] = prompt_yes_no_default( - f"Route `{binary}` through the gateway too, not just `ucode {binary}`? " - f"(writes {GLOBAL_SETTINGS_FILES[tool]}, needs sudo once)", - default=False, - ) enabled_agents[tool] = agent_config _confirm_agent(tool, agent_config) diff --git a/tests/conftest.py b/tests/conftest.py index 34f8a7de..04d9638b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,6 +28,7 @@ def _isolate_ucode_state(tmp_path, monkeypatch): import ucode.databricks as databricks_mod import ucode.managed_files as managed_files_mod import ucode.state as state_mod + from ucode.agents import codex as codex_mod state_dir = tmp_path / ".ucode" state_dir.mkdir() @@ -38,6 +39,7 @@ def _isolate_ucode_state(tmp_path, monkeypatch): monkeypatch.setattr( managed_files_mod, "MANAGED_BACKUP_MANIFEST_PATH", backup_dir / "manifest.json" ) + monkeypatch.setattr(codex_mod, "_managed_config_path", lambda: None) def reject_privileged_write(path, _desired_text): pytest.fail( diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 948ff400..87868090 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -694,9 +694,10 @@ def test_fallback_warns_on_stderr_before_handoff(self, monkeypatch, capsys): # and points at codex's own error so it doesn't read as their mistake. assert "ucode's `--profile`" in err assert "error above" in err - # Names the fallback config and that Databricks routing is lost. + # Names both config scopes Codex will resolve without the ucode profile. assert str(codex.LEGACY_CODEX_CONFIG_PATH) in err - assert "NOT the Databricks gateway" in err + assert "OS-managed settings" in err + assert "instead of the ucode profile" in err assert captured.out == "" def test_slow_failure_does_not_retry(self, monkeypatch): @@ -719,7 +720,7 @@ def test_fast_success_does_not_retry(self, monkeypatch): class TestCodexManagedConfig: - """use_as_global_settings: also write Codex's OS managed_config.toml (via sudo, mocked).""" + """Every normal configuration also reconciles Codex's OS-managed config.""" def _patch(self, tmp_path, monkeypatch): config_path = tmp_path / ".codex" / "ucode.config.toml" @@ -728,21 +729,22 @@ def _patch(self, tmp_path, monkeypatch): monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", tmp_path / "codex-ucode-config.backup.toml") monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0") monkeypatch.setattr(codex, "save_state", lambda state: None) + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: True) # Deterministic managed path + a mocked sudo writer that writes straight to disk, so the test # can read the TOML back and NO real sudo/`/etc` write ever happens. monkeypatch.setattr(codex, "_managed_config_path", lambda: managed_path) - def fake_write_managed(path, text, *, display): + def fake_write_managed(path, text, **kwargs): Path(path).parent.mkdir(parents=True, exist_ok=True) Path(path).write_text(text, encoding="utf-8") return "written" - monkeypatch.setattr(codex, "write_managed_file", fake_write_managed) + monkeypatch.setattr(codex, "reconcile_managed_file", fake_write_managed) return config_path, managed_path - def test_writes_managed_config_when_flagged(self, tmp_path, monkeypatch): + def test_writes_managed_config_by_default(self, tmp_path, monkeypatch): _, managed_path = self._patch(tmp_path, monkeypatch) - state = {"workspace": WS, "codex_models": ["gpt-5"], "write_managed_config": True} + state = {"workspace": WS, "codex_models": ["gpt-5"]} codex.write_tool_config(state) doc = read_toml_safe(managed_path) @@ -756,7 +758,7 @@ def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): managed_path.write_text( 'model = "my-own"\napproval_policy = "on-request"\n', encoding="utf-8" ) - state = {"workspace": WS, "codex_models": ["gpt-5"], "write_managed_config": True} + state = {"workspace": WS, "codex_models": ["gpt-5"]} codex.write_tool_config(state) doc = read_toml_safe(managed_path) @@ -764,8 +766,41 @@ def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): assert doc["approval_policy"] == "on-request" assert "model" not in doc - def test_no_managed_write_by_default(self, tmp_path, monkeypatch): + def test_noninteractive_uses_local_config_when_managed_config_is_compatible( + self, tmp_path, monkeypatch + ): _, managed_path = self._patch(tmp_path, monkeypatch) + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) state = {"workspace": WS, "codex_models": ["gpt-5"]} codex.write_tool_config(state) assert not managed_path.exists() + + def test_noninteractive_preserves_unrelated_managed_config(self, tmp_path, monkeypatch): + _, managed_path = self._patch(tmp_path, monkeypatch) + managed_path.parent.mkdir(parents=True, exist_ok=True) + original = 'approval_policy = "on-request"\n' + managed_path.write_text(original, encoding="utf-8") + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) + + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + assert managed_path.read_text(encoding="utf-8") == original + + def test_noninteractive_fails_when_managed_config_conflicts(self, tmp_path, monkeypatch): + _, managed_path = self._patch(tmp_path, monkeypatch) + managed_path.parent.mkdir(parents=True, exist_ok=True) + managed_path.write_text('model_provider = "enterprise"\n', encoding="utf-8") + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) + + with pytest.raises(RuntimeError, match="cannot be applied non-interactively"): + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + def test_invalid_managed_toml_is_not_modified(self, tmp_path, monkeypatch): + _, managed_path = self._patch(tmp_path, monkeypatch) + managed_path.parent.mkdir(parents=True, exist_ok=True) + managed_path.write_text("[invalid", encoding="utf-8") + + with pytest.raises(RuntimeError, match="Cannot safely update Codex managed settings"): + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + assert managed_path.read_text(encoding="utf-8") == "[invalid" diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index a7692df0..5977cc0c 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -536,7 +536,7 @@ def capture_batch(displays): configure_selected_tools({}, ["codex", "claude"]) - assert batches == [["Claude Code"]] + assert batches == [["Codex", "Claude Code"]] def test_merges_with_existing_available_tools(self, monkeypatch): """Configuring a new tool should not drop previously-configured tools diff --git a/tests/test_cli.py b/tests/test_cli.py index d1bf98bf..c8fc5dbe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1417,12 +1417,23 @@ def test_accepts_configured_codex_launch(self): with ( patch("ucode.cli.managed_agent_config_enabled", return_value=False), patch("ucode.cli.codex_agent.has_ucode_config", return_value=True), + patch("ucode.cli.codex_agent.managed_config_is_current", return_value=True), ): assert ( cli_mod._can_launch_from_cached_config("codex", MINIMAL_STATE, **self._kwargs()) is True ) + with ( + patch("ucode.cli.managed_agent_config_enabled", return_value=False), + patch("ucode.cli.codex_agent.has_ucode_config", return_value=True), + patch("ucode.cli.codex_agent.managed_config_is_current", return_value=False), + ): + assert ( + cli_mod._can_launch_from_cached_config("codex", MINIMAL_STATE, **self._kwargs()) + is False + ) + def test_accepts_claude_only_when_managed_settings_are_verified(self, tmp_path): import ucode.cli as cli_mod @@ -1460,6 +1471,7 @@ def test_accepts_codex_v2_launch_with_complete_model_cache(self, monkeypatch): with ( patch("ucode.cli.managed_agent_config_enabled", return_value=False), patch("ucode.cli.codex_agent.has_ucode_config", return_value=True), + patch("ucode.cli.codex_agent.managed_config_is_current", return_value=True), ): assert cli_mod._can_launch_from_cached_config("codex", state, **self._kwargs()) is True diff --git a/tests/test_managed_config.py b/tests/test_managed_config.py index 8a1f26bd..05c179fc 100644 --- a/tests/test_managed_config.py +++ b/tests/test_managed_config.py @@ -30,7 +30,6 @@ { "agent": "CODING_AGENT_CLAUDE_CODE", "config": { - "use_as_global_settings": True, "custom_headers": {"x-databricks-workspace": "eng-ml-inference"}, "tracing_config": {"table": "main.default.ucode_traces"}, "model_config": { @@ -91,7 +90,6 @@ def test_full_manifest_maps_enums_to_tool_names(self): def test_claude_agent_config_fields(self): claude = normalize_managed_config(RAW_MANIFEST)["enabled_agents"]["claude"] - assert claude["use_as_global_settings"] is True assert claude["custom_headers"] == {"x-databricks-workspace": "eng-ml-inference"} assert claude["tracing_table"] == "main.default.ucode_traces" assert claude["model_config"]["default_model"] == "system.ai.claude-opus-4-8" diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index ba4b6b5e..80860b2a 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -25,14 +25,6 @@ def _supported(monkeypatch): monkeypatch.setattr(managed_files.sys.stdin, "isatty", lambda: True) -def _capture_sudo(monkeypatch): - calls: list = [] - monkeypatch.setattr( - managed_files, "_sudo_replace", lambda path, text: calls.append((str(path), text)) - ) - return calls - - @pytest.fixture def backup_dir(tmp_path, monkeypatch): path = tmp_path / "managed-backups" @@ -41,62 +33,6 @@ def backup_dir(tmp_path, monkeypatch): return path -class TestWriteManagedFile: - def test_unchanged_content_does_not_sudo(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - path.write_text("same", encoding="utf-8") - calls = _capture_sudo(monkeypatch) - assert managed_files.write_managed_file(path, "same", display="X") == "unchanged" - # The whole point: an unchanged file never prompts for a password. - assert calls == [] - - def test_changed_content_sudo_writes(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - path.write_text("old", encoding="utf-8") - calls = _capture_sudo(monkeypatch) - assert managed_files.write_managed_file(path, "new", display="X") == "written" - assert calls == [(str(path), "new")] - - def test_absent_file_sudo_writes(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - calls = _capture_sudo(monkeypatch) - assert managed_files.write_managed_file(path, "new", display="X") == "written" - assert calls == [(str(path), "new")] - - def test_dry_run_does_not_sudo(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - calls = _capture_sudo(monkeypatch) - config_io.set_dry_run(True) - assert managed_files.write_managed_file(path, "new", display="X") == "written" - assert calls == [] - - def test_unsupported_platform_skips(self, tmp_path, monkeypatch): - monkeypatch.setattr(managed_files, "managed_files_supported", lambda: False) - calls = _capture_sudo(monkeypatch) - path = tmp_path / "managed.json" - assert managed_files.write_managed_file(path, "new", display="X") == "skipped" - assert calls == [] - - def test_permission_error_is_skipped_not_raised(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - - def boom(path, text): - raise PermissionError("no root") - - monkeypatch.setattr(managed_files, "_sudo_replace", boom) - # Never raises — the launch proceeds; the private ucode config still works. - assert managed_files.write_managed_file(path, "new", display="X") == "skipped" - - def test_sudo_failure_is_skipped_not_raised(self, tmp_path, monkeypatch): - path = tmp_path / "managed.json" - - def boom(path, text): - raise subprocess.CalledProcessError(1, ["/usr/bin/sudo", "cp"], stderr="denied") - - monkeypatch.setattr(managed_files, "_sudo_replace", boom) - assert managed_files.write_managed_file(path, "new", display="X") == "skipped" - - class TestClearImmutableStatDenied: def test_stat_denied_path_returns_no_flags_without_raising(self, monkeypatch): # Regression: `_clear_immutable` ran an unguarded path.exists() inside the sudo write; under a @@ -155,6 +91,58 @@ def run(command, **kwargs): class TestManagedFileLifecycle: + def test_dry_run_does_not_write_or_backup(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + config_io.set_dry_run(True) + monkeypatch.setattr( + managed_files, "_sudo_replace", lambda *args: pytest.fail("must not write") + ) + + result = managed_files.reconcile_managed_file( + path, + '{"ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + ) + + assert result == "written" + assert not backup_dir.exists() + + def test_unsupported_platform_skips(self, tmp_path, monkeypatch): + monkeypatch.setattr(managed_files, "managed_files_supported", lambda: False) + monkeypatch.setattr( + managed_files, "_sudo_replace", lambda *args: pytest.fail("must not write") + ) + + result = managed_files.reconcile_managed_file( + tmp_path / "managed.json", + '{"ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + ) + + assert result == "unsupported" + + def test_permission_failure_is_actionable(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + + def deny_write(path, text): + raise PermissionError("no root") + + monkeypatch.setattr(managed_files, "_sudo_replace", deny_write) + + with pytest.raises(RuntimeError, match="could not update"): + managed_files.reconcile_managed_file( + path, + '{"ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + ) + assert (backup_dir / "manifest.json").exists() + def test_reconcile_refuses_symlink_target(self, tmp_path, backup_dir, monkeypatch): target = tmp_path / "real.json" target.write_text("{}", encoding="utf-8") diff --git a/tests/test_managed_resolve.py b/tests/test_managed_resolve.py index 3c912678..cf5d5cdb 100644 --- a/tests/test_managed_resolve.py +++ b/tests/test_managed_resolve.py @@ -18,11 +18,10 @@ managed_state_overrides, managed_supplies_models, managed_unservable_models, - managed_use_as_global_settings, recommended_agent, resolve_state, ) -from ucode.state import MANAGED_OVERLAY_KEY, _without_managed_overlay +from ucode.state import MANAGED_OVERLAY_KEY WORKSPACE = "https://ws.example.com" @@ -32,7 +31,6 @@ "default_agent": "claude", "enabled_agents": { "claude": { - "use_as_global_settings": True, "model_config": { "default_model": "system.ai.claude-opus-5", "models": { @@ -184,37 +182,6 @@ def test_layers_provider_without_dropping_other_tools(self): } -class TestGlobalSettings: - def test_only_codex_keeps_the_legacy_global_settings_flag(self): - from ucode.agents import GLOBAL_SETTINGS_AGENTS - - assert GLOBAL_SETTINGS_AGENTS == frozenset({"codex"}) - - def test_claude_no_longer_honors_the_legacy_flag(self): - assert managed_use_as_global_settings(MANAGED, "claude") is False - - def test_flag_false_when_not_opted_in(self): - # codex is enabled but never marked machine-wide. - assert managed_use_as_global_settings(MANAGED, "codex") is False - - def test_flag_ignored_for_unsupported_agent(self): - # A hand-written --from-file config can't turn it on for an agent whose token can't refresh. - managed = {"enabled_agents": {"gemini": {"use_as_global_settings": True}}} - assert managed_use_as_global_settings(managed, "gemini") is False - - def test_resolve_does_not_set_transient_flag_for_claude(self): - resolved = resolve_state(MANAGED, _state(), "claude") - assert "write_managed_config" not in resolved - - def test_resolve_omits_flag_when_not_opted_in(self): - resolved = resolve_state(MANAGED, _state(), "codex") - assert "write_managed_config" not in resolved - - def test_removed_claude_flag_is_not_persisted(self): - resolved = resolve_state(MANAGED, _state(), "claude") - assert "write_managed_config" not in _without_managed_overlay(resolved) - - class TestStateFileIsNotRewritten: """The managed config must win by precedence, not by overwriting the developer's state file. @@ -408,7 +375,7 @@ def test_true_for_a_flat_model_list(self): def test_false_when_the_config_names_no_models(self): # Discovery still has to run, or the launch has nothing to pin. - managed = {"enabled_agents": {"claude": {"use_as_global_settings": True}}} + managed = {"enabled_agents": {"claude": {}}} assert managed_supplies_models(managed, "claude") is False def test_false_for_an_agent_the_config_does_not_cover(self): diff --git a/tests/test_managed_setup.py b/tests/test_managed_setup.py index d4425a64..140bc3d9 100644 --- a/tests/test_managed_setup.py +++ b/tests/test_managed_setup.py @@ -64,7 +64,6 @@ def _full_manifest() -> dict: "default_agent": "claude", "enabled_agents": { "claude": { - "use_as_global_settings": True, "custom_headers": {"x-databricks-workspace": "eng-ml-inference"}, "tracing_table": "main.default.claude-traces", "model_config": { @@ -76,7 +75,6 @@ def _full_manifest() -> dict: }, }, "codex": { - "use_as_global_settings": False, "model_config": {"default_model": "system.ai.gpt-5-6"}, }, "opencode": { @@ -308,21 +306,6 @@ def test_name_is_carried_through_when_present(self): ) assert payload["name"] == "coding-agent-configs/abc" - def test_use_as_global_settings_false_is_preserved(self): - # `False` is meaningful (write to the user-level file), so it must not be dropped as falsy. - payload = serialize_managed_config( - { - "default_agent": "codex", - "enabled_agents": { - "codex": { - "use_as_global_settings": False, - "model_config": {"default_model": "m"}, - } - }, - } - ) - assert payload["enabled_agents"][0]["config"]["use_as_global_settings"] is False - class TestModelOptions: def test_claude_only_sees_claude_models(self): @@ -524,7 +507,7 @@ def test_default_agent_must_be_enabled(self): def test_default_agent_needs_a_default_model(self): manifest = { "default_agent": "claude", - "enabled_agents": {"claude": {"use_as_global_settings": True}}, + "enabled_agents": {"claude": {}}, } errors = validate_manifest(manifest) assert any("model_config.default_model" in e for e in errors) diff --git a/tests/test_managed_wizard.py b/tests/test_managed_wizard.py index d14d8f20..013cdc9d 100644 --- a/tests/test_managed_wizard.py +++ b/tests/test_managed_wizard.py @@ -1841,24 +1841,18 @@ def test_single_model_agent_needs_no_extra_line(self, capsys): assert "system.ai.gemini-3-flash" in out assert "models:" not in out - def test_scope_label_only_for_global_capable_agents(self, capsys): - # Codex retains the legacy scope choice; Claude now always installs managed settings. + def test_summary_has_no_settings_scope_choice(self, capsys): manifest = { "default_agent": "codex", "enabled_agents": { - "codex": { - "model_config": {"default_model": "system.ai.gpt-5"}, - "use_as_global_settings": True, - }, + "codex": {"model_config": {"default_model": "system.ai.gpt-5"}}, "gemini": {"model_config": {"default_model": "system.ai.gemini-3-flash"}}, }, } wizard._render_summary(WORKSPACE, manifest) out = capsys.readouterr().out - assert "global settings" in out - # The gemini line names its model but carries no global-settings/ucode-only scope. - gemini_line = next(line for line in out.splitlines() if "gemini-3-flash" in line) - assert "ucode-only" not in gemini_line and "global settings" not in gemini_line + assert "global settings" not in out + assert "ucode-only" not in out class TestSetupFromFile: