From b07397c7240aea603f8245ae8d32fe4ac3d6faf9 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Wed, 12 Aug 2026 17:12:02 +0000 Subject: [PATCH 01/10] Write agents' native config under use_as_global_settings (claude, codex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an admin marks an agent machine-wide in `ucode setup`, also write the agent's own native config file (~/.claude/settings.json, ~/.codex/config.toml) so a bare `claude`/`codex` hits the gateway, not just `ucode `. Only claude and codex qualify — their auth self-refreshes — so the machine-wide prompt is now only asked for those two. Revert surgically prunes just ucode's keys from the native file, and hydrate_state no longer drops the tracking. Co-authored-by: Isaac --- src/ucode/agents/__init__.py | 10 +++ src/ucode/agents/claude.py | 146 ++++++++++++++++++++++++++-------- src/ucode/agents/codex.py | 80 ++++++++++++++++++- src/ucode/cli.py | 9 +++ src/ucode/config_io.py | 36 ++++++++- src/ucode/managed_resolve.py | 22 +++++ src/ucode/managed_wizard.py | 24 ++++-- src/ucode/state.py | 24 +++++- tests/test_agent_claude.py | 94 ++++++++++++++++++++++ tests/test_agent_codex.py | 76 ++++++++++++++++++ tests/test_config_io.py | 39 +++++++++ tests/test_managed_resolve.py | 38 ++++++++- tests/test_managed_wizard.py | 19 +++++ tests/test_state.py | 24 ++++++ 14 files changed, 592 insertions(+), 49 deletions(-) diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index ecb57f7d..b54e1360 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -70,6 +70,16 @@ DEFAULT_TOOL = "codex" BUNDLE_VERSION = 1 +# Agents that can mirror ucode's managed config into the tool's NATIVE default config file, so a +# bare `claude` / `codex` (not just `ucode `) picks up the gateway settings. This is gated by +# the admin's `use_as_global_settings` choice in `ucode setup`. Only agents whose gateway auth +# self-refreshes qualify: claude's `apiKeyHelper` and codex's `ucode auth-token` command both re-mint +# tokens on their own, so the native file keeps working indefinitely. The other agents bake a +# short-lived bearer token with no bare-launch refresher (opencode/pi/gemini) or expose no native +# config file at all (copilot is env-var only), so they're excluded — and `ucode setup` doesn't even +# ask them the machine-wide question. +GLOBAL_SETTINGS_AGENTS = frozenset({"claude", "codex"}) + # ucode tool -> `databricks aitools` agent id. gemini/pi aren't supported. AITOOLS_AGENT_TOKENS = { "claude": "claude-code", diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 2ad47402..7ab0ada8 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -12,6 +12,7 @@ import subprocess import sys import threading +from collections.abc import Callable from pathlib import Path from typing import cast @@ -21,6 +22,7 @@ ToolSpec, backup_existing_file, deep_merge_dict, + prune_key_paths, read_json_safe, write_json_file, ) @@ -42,6 +44,10 @@ CLAUDE_CONFIG_DIR = Path.home() / ".claude" CLAUDE_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "ucode-settings.json" CLAUDE_BACKUP_PATH = APP_DIR / "claude-ucode-settings.backup.json" +# Claude Code's own user-scope settings file, read by a bare `claude` with no flags. ucode writes +# here (in addition to the private file above) only when the managed config sets +# use_as_global_settings, so a developer who launches `claude` directly still hits the gateway. +CLAUDE_NATIVE_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "settings.json" SPEC: ToolSpec = { "binary": "claude", @@ -516,43 +522,50 @@ def write_tool_config( "to install the Claude Stop hook — traces won't be emitted. Re-run " "`ucode configure tracing`." ) - - existing = read_json_safe(CLAUDE_SETTINGS_PATH) - merged = deep_merge_dict(existing, overlay) - # Drop any apiKeyHelper a prior non-relayed launch left in the file; relayed - # must not carry one (it would outrank the subscription OAuth). - if relayed: - merged.pop("apiKeyHelper", None) - if tracing_env_vars and stop_hook_command: - _upsert_tracing_stop_hook(merged, stop_hook_command) - if not tracing_env_vars: - env_block = merged.get("env") - if isinstance(env_block, dict): - for key in CLAUDE_TRACING_ENV_KEYS: - env_block.pop(key, None) - # Strip only ucode's tracing Stop hook so user hooks stay intact. - _remove_tracing_stop_hook(merged) - # Prune ucode-managed model env keys we deliberately don't write this run - # (e.g. ANTHROPIC_MODEL — see render_overlay). - overlay_env = overlay.get("env", {}) - merged_env = merged.get("env") - if isinstance(merged_env, dict): - for key in CLAUDE_MANAGED_MODEL_ENV_KEYS: - if key not in overlay_env: - merged_env.pop(key, None) - # deep_merge_dict keeps keys already in the file, so drop the ones ucode no - # longer writes. - if isinstance(merged_env, dict): - for key in CLAUDE_REMOVED_ENV_KEYS: - merged_env.pop(key, None) - # Smart-routing hooks: install ucode's PreToolUse/SessionStart/ - # SubagentStart hooks when routing is enabled (and not under a provider, - # which pins no Databricks model), else surgically strip only ucode's own. + # Smart-routing hooks: install ucode's PreToolUse/SessionStart/SubagentStart hooks when routing + # is enabled (and not under a provider, which pins no Databricks model), else surgically strip + # only ucode's own. Applied per file inside _compose_claude_settings. routing_enabled = smart_routing_enabled(state) and provider is None - sync_smart_routing_hooks(merged, state, enabled=routing_enabled) if routing_enabled: managed_keys = managed_keys + [["hooks", event] for event in CLAUDE_ROUTING_HOOK_EVENTS] - write_json_file(CLAUDE_SETTINGS_PATH, merged) + + def _compose(base: dict) -> dict: + # deepcopy the overlay per file so merging into one base can't alias nested dicts into + # the other (deep_merge_dict grafts overlay's own dict objects onto a base missing the key). + merged = deep_merge_dict(base, copy.deepcopy(overlay)) + # Drop any apiKeyHelper a prior non-relayed launch left in the file; relayed + # must not carry one (it would outrank the subscription OAuth). + if relayed: + merged.pop("apiKeyHelper", None) + if tracing_env_vars and stop_hook_command: + _upsert_tracing_stop_hook(merged, stop_hook_command) + if not tracing_env_vars: + env_block = merged.get("env") + if isinstance(env_block, dict): + for key in CLAUDE_TRACING_ENV_KEYS: + env_block.pop(key, None) + # Strip only ucode's tracing Stop hook so user hooks stay intact. + _remove_tracing_stop_hook(merged) + # Prune ucode-managed model env keys we deliberately don't write this run + # (e.g. ANTHROPIC_MODEL — see render_overlay). + overlay_env = overlay.get("env", {}) + merged_env = merged.get("env") + if isinstance(merged_env, dict): + for key in CLAUDE_MANAGED_MODEL_ENV_KEYS: + if key not in overlay_env: + merged_env.pop(key, None) + # deep_merge_dict keeps keys already in the file, so drop the ones ucode no + # longer writes. + for key in CLAUDE_REMOVED_ENV_KEYS: + merged_env.pop(key, None) + sync_smart_routing_hooks(merged, state, enabled=routing_enabled) + return merged + + write_json_file(CLAUDE_SETTINGS_PATH, _compose(read_json_safe(CLAUDE_SETTINGS_PATH))) + + native_configs = None + if state.get("write_native_config"): + native_configs = _write_native_settings(_compose, managed_keys, relayed) if web_search_model: _register_web_search_mcp(state["workspace"], web_search_model, state.get("profile")) @@ -564,11 +577,74 @@ def write_tool_config( else: state.pop("claude_relayed", None) state.pop("relayed_proxy_port", None) - state = mark_tool_managed(state, "claude", managed_keys) + state = mark_tool_managed(state, "claude", managed_keys, native=native_configs) save_state(state) return state +def _write_native_settings( + compose: Callable[[dict], dict], managed_keys: list[list[str]], relayed: bool +) -> list[dict] | None: + """Mirror ucode's managed config into Claude Code's native ~/.claude/settings.json. + + Runs only under use_as_global_settings so a bare `claude` reaches the gateway. The same compose + (merge overlay + prune stale keys) that produced the private file is applied to the user's own + settings, so their unrelated keys survive. Returns the native descriptor for revert tracking, or + None when nothing was written. + + Relayed launches are skipped: they depend on a per-session loopback refresh proxy that only runs + during `ucode claude`, so a bare `claude` could not reach the gateway anyway. + """ + if relayed: + print_warning( + "Claude subscription-relay launches use a per-session proxy a bare `claude` can't " + "reach, so ucode did not write ~/.claude/settings.json for machine-wide use. Launch " + "with `ucode claude` to use the relay." + ) + return None + write_json_file( + CLAUDE_NATIVE_SETTINGS_PATH, compose(read_json_safe(CLAUDE_NATIVE_SETTINGS_PATH)) + ) + # Enterprise managed settings outrank the user scope per key and can't be excluded, so warn when + # they'd shadow what we just wrote — a bare `claude` would silently ignore ucode's config there. + conflict = _managed_relayed_conflicts() + overrides = managed_settings_model_overrides() + if conflict: + conflict_path, keys = conflict + print_warning( + f"Enterprise managed settings at {conflict_path} set {', '.join(keys)}, which override " + "~/.claude/settings.json — a bare `claude` may not route through the gateway." + ) + elif overrides: + print_warning( + f"Enterprise managed settings at {overrides} pin a model, which overrides the workspace " + "default a bare `claude` would otherwise use." + ) + return [{"path": str(CLAUDE_NATIVE_SETTINGS_PATH), "format": "json", "keys": managed_keys}] + + +def revert_native_config(state: dict) -> str | None: + """Surgically strip ucode's keys from native config files it wrote under use_as_global_settings. + + Returns a short status ("ucode entries removed" / "unchanged") for the revert summary, or None + when ucode never wrote a native file for claude. + """ + native = ((state.get("managed_configs") or {}).get("claude") or {}).get("native") + if not isinstance(native, list) or not native: + return None + changed = False + for descriptor in native: + path = Path(descriptor.get("path", "")) + keys = descriptor.get("keys") or [] + if not path or not path.exists(): + continue + doc = read_json_safe(path) + if prune_key_paths(doc, keys): + write_json_file(path, doc) + changed = True + return "ucode entries removed" if changed else "unchanged" + + def _is_tracing_stop_hook(hook: object) -> bool: if not isinstance(hook, dict): return False diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index f64747d4..06690f05 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -354,11 +354,89 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non enabled=smart_routing_enabled(state) and provider is None, ) write_toml_file(CODEX_CONFIG_PATH, doc) - state = mark_tool_managed(state, "codex", MANAGED_KEYS) + # use_as_global_settings: also write the modern overlay to the native ~/.codex/config.toml so a + # bare `codex` (no `--profile ucode`) defaults to the gateway. Written last, after + # `_remove_legacy_ucode_profile` above stripped the legacy entries, so the modern provider block + # survives. codex auth self-refreshes via `ucode auth-token`, so the native file keeps working. + native_configs = None + if state.get("write_native_config"): + native_configs = _write_native_config( + workspace, chosen_model, databricks_profile, bool(state.get("use_pat")), provider + ) + state = mark_tool_managed(state, "codex", MANAGED_KEYS, native=native_configs) save_state(state) return state +def _write_native_config( + workspace: str, + model: str | None, + databricks_profile: str | None, + use_pat: bool, + provider: str | None, +) -> list[dict]: + """Merge the modern overlay into the native ~/.codex/config.toml, preserving the user's keys. + + Returns the native descriptor for revert tracking. + """ + overlay = render_overlay( + workspace, model, databricks_profile, use_pat=use_pat, provider=provider + ) + doc = read_toml_safe(LEGACY_CODEX_CONFIG_PATH) + deep_merge_dict(doc, overlay) + if provider: + # deep_merge can't drop keys; clear a `model` a prior non-provider run pinned. + doc.pop("model", None) + write_toml_file(LEGACY_CODEX_CONFIG_PATH, doc) + return [ + { + "path": str(LEGACY_CODEX_CONFIG_PATH), + "format": "toml", + "keys": [list(k) for k in MANAGED_KEYS], + } + ] + + +def _strip_modern_ucode_entries(path: Path) -> bool: + """Surgically remove ucode's *modern* keys from a shared Codex config. + + Drops the top-level ``model_provider = "ucode-databricks"`` selector (and the ``model`` pinned + alongside it) and the ``[model_providers.ucode-databricks]`` block, leaving the user's other keys + intact. Mirrors :func:`_strip_legacy_ucode_entries` for the modern native write. Returns True if + anything was removed. + """ + if not path.exists(): + return False + doc = read_toml_safe(path) + changed = False + if doc.get("model_provider") == CODEX_MODEL_PROVIDER_NAME: + doc.pop("model_provider", None) + # ucode pins `model` only alongside its own provider, so remove it when the provider is ours. + doc.pop("model", None) + changed = True + providers = doc.get("model_providers") + if isinstance(providers, dict) and CODEX_MODEL_PROVIDER_NAME in providers: + providers.pop(CODEX_MODEL_PROVIDER_NAME, None) + if not providers: + doc.pop("model_providers", None) + changed = True + if changed: + write_toml_file(path, doc) + return changed + + +def revert_native_config(state: dict) -> str | None: + """Strip ucode's modern keys from ~/.codex/config.toml if it was written under global settings. + + Returns a short status for the revert summary, or None when ucode never wrote the native file. + """ + native = ((state.get("managed_configs") or {}).get("codex") or {}).get("native") + if not isinstance(native, list) or not native: + return None + changed = _strip_modern_ucode_entries(LEGACY_CODEX_CONFIG_PATH) + return "ucode entries removed" if changed else "unchanged" + + def default_model(state: dict) -> str | None: """Pick the newest GPT model when multiple are available. diff --git a/src/ucode/cli.py b/src/ucode/cli.py index a78c68a1..46dd6e96 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -999,6 +999,12 @@ def revert() -> int: # Older Codex (< 0.134.0) had ucode edit the shared ~/.codex/config.toml in # place; restoring the per-profile file above does not undo that. legacy_codex_stripped = revert_legacy_shared_config() + # Native config files written under use_as_global_settings (a bare `claude` / `codex` reading + # the tool's own config): surgically strip ucode's keys, never touching the user's own settings. + native_reverts = { + "claude": claude_agent.revert_native_config(state), + "codex": codex_agent.revert_native_config(state), + } clear_state() print_heading("Revert") @@ -1007,6 +1013,9 @@ def revert() -> int: print_kv(f"{spec['display']} config", "restored" if results[tool] else "unchanged") if legacy_codex_stripped: print_kv("Codex shared config", "ucode entries removed") + for tool, status in native_reverts.items(): + if status: + print_kv(f"{TOOL_SPECS[tool]['display']} native config", status) print_kv("Pi settings", "restored" if pi_settings_restored else "unchanged") for client, spec in MCP_CLIENTS.items(): print_kv( diff --git a/src/ucode/config_io.py b/src/ucode/config_io.py index 06446b9a..3ca67ec8 100644 --- a/src/ucode/config_io.py +++ b/src/ucode/config_io.py @@ -4,7 +4,7 @@ import json from pathlib import Path -from typing import TypedDict +from typing import TypedDict, cast import tomlkit import tomlkit.exceptions @@ -107,6 +107,40 @@ def deep_merge_dict(base: dict, overlay: dict) -> dict: return base +def prune_key_paths(doc: dict, key_paths: list[list[str]]) -> bool: + """Surgically remove each ``key_path`` from a nested dict, dropping emptied parents. + + ``key_paths`` is a list of paths like ``[["env", "ANTHROPIC_BASE_URL"], ["apiKeyHelper"]]``. + Only the exact leaves are removed; sibling keys the user set themselves stay untouched, and a + parent dict left empty by the removal is dropped too. Returns True if anything changed. + + Used to undo ucode's writes to an agent's *native* config file (which it shares with the + user's own settings), where restoring a backup would clobber edits made since ucode first ran. + """ + changed = False + for path in key_paths: + if _prune_one(doc, list(path)): + changed = True + return changed + + +def _prune_one(node: object, path: list[str]) -> bool: + if not path or not isinstance(node, dict): + return False + mapping = cast("dict[str, object]", node) + key = path[0] + if key not in mapping: + return False + if len(path) == 1: + mapping.pop(key, None) + return True + child = mapping.get(key) + removed = _prune_one(child, path[1:]) + if removed and isinstance(child, dict) and not child: + mapping.pop(key, None) + return removed + + def read_json_safe(path: Path) -> dict: if not path.exists(): return {} diff --git a/src/ucode/managed_resolve.py b/src/ucode/managed_resolve.py index b6658d4e..509b9b30 100644 --- a/src/ucode/managed_resolve.py +++ b/src/ucode/managed_resolve.py @@ -176,6 +176,22 @@ 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 own native config file so a bare + ``claude`` / ``codex`` picks up the gateway config. Only agents in + :data:`~ucode.agents.GLOBAL_SETTINGS_AGENTS` can honor it (their auth self-refreshes); the flag + is ignored for any other agent, so a hand-written ``--from-file`` config can't turn it on for an + agent that would silently break after the token expires. + """ + from ucode.agents import GLOBAL_SETTINGS_AGENTS + + 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. @@ -274,6 +290,12 @@ 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 + # native file. A non-managed launch never sets it, so default behavior is unchanged. + overlay["write_native_config"] = state.get("write_native_config") + resolved["write_native_config"] = True if overlay: resolved[MANAGED_OVERLAY_KEY] = overlay return resolved diff --git a/src/ucode/managed_wizard.py b/src/ucode/managed_wizard.py index e1755505..7c86a8dd 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -18,7 +18,7 @@ from pathlib import Path from typing import cast -from ucode.agents import TOOL_SPECS, check_gateway_endpoint +from ucode.agents import GLOBAL_SETTINGS_AGENTS, TOOL_SPECS, check_gateway_endpoint from ucode.databricks import ( ANTHROPIC_FAMILIES, all_users_can_use_schema, @@ -827,8 +827,12 @@ def _render_summary(workspace: str, manifest: dict) -> None: provider = model_config.get("model_provider_service") if provider: detail = f"{detail} via {provider}" - scope = "global settings" if agent_config.get("use_as_global_settings") else "ucode-only" - lines.append(kv_line(display, f"{detail} ({scope})")) + # 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)) # 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") @@ -1086,11 +1090,15 @@ def setup_command( agent_config: dict = { "model_config": _prompt_models_for_agent(tool, state, provider_service) } - agent_config["use_as_global_settings"] = prompt_yes_no_default( - f"Write {TOOL_SPECS[tool]['display']}'s config to its global settings file? " - f"({GLOBAL_SETTINGS_BLURB})", - default=False, - ) + # 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: + agent_config["use_as_global_settings"] = prompt_yes_no_default( + f"Write {TOOL_SPECS[tool]['display']}'s config to its global settings file? " + f"({GLOBAL_SETTINGS_BLURB})", + default=False, + ) enabled_agents[tool] = agent_config manifest: dict = {"default_agent": default_agent, "enabled_agents": enabled_agents} diff --git a/src/ucode/state.py b/src/ucode/state.py index 0031bc4d..82f98632 100644 --- a/src/ucode/state.py +++ b/src/ucode/state.py @@ -124,7 +124,14 @@ def hydrate_state(state: dict) -> dict: for tool, entry in managed_configs.items(): if isinstance(entry, dict): keys = entry.get("keys") if isinstance(entry.get("keys"), list) else [] - normalized[tool] = {"keys": keys} + norm: dict = {"keys": keys} + # Preserve native-file tracking so `ucode revert` can surgically prune the agent's + # own config file (e.g. ~/.claude/settings.json) it wrote under use_as_global_settings. + # Dropping it here would silently strand ucode's keys in the user's file forever. + native = entry.get("native") + if isinstance(native, list): + norm["native"] = native + normalized[tool] = norm elif entry: normalized[tool] = {"keys": []} hydrated["managed_configs"] = normalized @@ -236,9 +243,20 @@ def clear_state() -> None: raise RuntimeError(f"Failed to clear state file: {STATE_PATH}") from exc -def mark_tool_managed(state: dict, tool: str, managed_keys: list) -> dict: +def mark_tool_managed( + state: dict, tool: str, managed_keys: list, native: list[dict] | None = None +) -> dict: + """Record which config keys ucode manages for ``tool``. + + ``native`` optionally describes the agent's own native config file(s) ucode also wrote under + ``use_as_global_settings`` — each ``{"path": str, "format": "json"|"toml", "keys": [...]}`` — + so ``ucode revert`` can surgically prune only ucode's keys from the user's shared file. + """ managed_configs = dict(state.get("managed_configs") or {}) - managed_configs[tool] = {"keys": list(managed_keys)} + entry: dict = {"keys": list(managed_keys)} + if native: + entry["native"] = native + managed_configs[tool] = entry state["managed_configs"] = managed_configs state["last_tool"] = tool return state diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 10f5815b..050d020d 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -468,6 +468,100 @@ def test_strips_stale_disable_experimental_betas(self, monkeypatch): assert written[0]["env"]["CLAUDE_CODE_USE_GATEWAY"] == "1" +class TestWriteToolConfigNativeSettings: + """use_as_global_settings: also write Claude Code's own ~/.claude/settings.json.""" + + def _patch(self, monkeypatch, writes, existing_by_path=None): + existing_by_path = existing_by_path or {} + monkeypatch.setattr(claude, "backup_existing_file", lambda *a, **kw: True) + monkeypatch.setattr( + claude, "read_json_safe", lambda path: dict(existing_by_path.get(str(path), {})) + ) + monkeypatch.setattr( + claude, "write_json_file", lambda path, payload: writes.append((str(path), payload)) + ) + monkeypatch.setattr(claude, "save_state", lambda state: None) + monkeypatch.setattr(claude, "_register_web_search_mcp", lambda *a, **kw: True) + # Keep the enterprise managed-settings checks deterministic regardless of the host machine. + monkeypatch.setattr(claude, "_managed_relayed_conflicts", lambda: None) + monkeypatch.setattr(claude, "managed_settings_model_overrides", lambda: None) + + def test_writes_native_file_when_flagged(self, monkeypatch): + writes: list = [] + self._patch(monkeypatch, writes) + state = {"workspace": WS, "codex_models": [], "write_native_config": True} + result = claude.write_tool_config(state, "databricks-claude-sonnet-4") + paths = [p for p, _ in writes] + assert str(claude.CLAUDE_SETTINGS_PATH) in paths + assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) in paths + native = result["managed_configs"]["claude"]["native"] + assert native[0]["path"] == str(claude.CLAUDE_NATIVE_SETTINGS_PATH) + assert native[0]["format"] == "json" + + def test_native_file_preserves_user_keys(self, monkeypatch): + writes: list = [] + existing = {str(claude.CLAUDE_NATIVE_SETTINGS_PATH): {"env": {"MY_OWN": "keep"}}} + self._patch(monkeypatch, writes, existing) + state = {"workspace": WS, "codex_models": [], "write_native_config": True} + claude.write_tool_config(state, "databricks-claude-sonnet-4") + native = next(p for path, p in writes if path == str(claude.CLAUDE_NATIVE_SETTINGS_PATH)) + # The user's own key survives; ucode's gateway keys are merged in alongside it. + assert native["env"]["MY_OWN"] == "keep" + assert native["env"]["ANTHROPIC_BASE_URL"] + assert native["apiKeyHelper"] + + def test_no_native_write_by_default(self, monkeypatch): + writes: list = [] + self._patch(monkeypatch, writes) + state = {"workspace": WS, "codex_models": []} + result = claude.write_tool_config(state, "databricks-claude-sonnet-4") + paths = [p for p, _ in writes] + assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) not in paths + assert "native" not in result["managed_configs"]["claude"] + + def test_relayed_skips_native_write(self, monkeypatch): + writes: list = [] + warns: list = [] + self._patch(monkeypatch, writes) + monkeypatch.setattr(claude, "print_warning", lambda msg: warns.append(msg)) + monkeypatch.setattr(claude, "relayed_proxy_base_url", lambda state: "http://127.0.0.1:9999") + state = {"workspace": WS, "codex_models": [], "write_native_config": True} + result = claude.write_tool_config(state, "databricks-claude-sonnet-4", relayed=True) + paths = [p for p, _ in writes] + assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) not in paths + assert result["managed_configs"]["claude"].get("native") is None + assert any("bare `claude`" in w for w in warns) + + +class TestClaudeRevertNativeConfig: + def test_prunes_only_tracked_keys(self, tmp_path): + native_path = tmp_path / "settings.json" + native_path.write_text( + json.dumps({"env": {"ANTHROPIC_BASE_URL": "x", "MY": "keep"}, "apiKeyHelper": "h"}), + encoding="utf-8", + ) + state = { + "managed_configs": { + "claude": { + "keys": [], + "native": [ + { + "path": str(native_path), + "format": "json", + "keys": [["env", "ANTHROPIC_BASE_URL"], ["apiKeyHelper"]], + } + ], + } + } + } + assert claude.revert_native_config(state) == "ucode entries removed" + assert json.loads(native_path.read_text()) == {"env": {"MY": "keep"}} + + def test_returns_none_without_native_tracking(self): + state = {"managed_configs": {"claude": {"keys": []}}} + assert claude.revert_native_config(state) is None + + class TestRegisterWebSearchMcp: def test_clears_existing_then_adds(self, monkeypatch): import ucode.mcp as mcp_mod diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index fa8894ed..cf781584 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -652,3 +652,79 @@ def test_fast_success_does_not_retry(self, monkeypatch): codex.launch({"workspace": WS}, []) assert exc.value.code == 0 assert fallbacks == [] + + +class TestCodexNativeConfig: + """use_as_global_settings: also write the native ~/.codex/config.toml so a bare `codex` works.""" + + def _patch(self, tmp_path, monkeypatch): + config_path = tmp_path / ".codex" / "ucode.config.toml" + native_path = tmp_path / ".codex" / "config.toml" + monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path) + monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", tmp_path / "codex-ucode-config.backup.toml") + monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", native_path) + monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0") + monkeypatch.setattr(codex, "save_state", lambda state: None) + return config_path, native_path + + def test_writes_native_config_when_flagged(self, tmp_path, monkeypatch): + _, native_path = self._patch(tmp_path, monkeypatch) + state = {"workspace": WS, "codex_models": ["gpt-5"], "write_native_config": True} + result = codex.write_tool_config(state) + + doc = read_toml_safe(native_path) + assert doc["model_provider"] == "ucode-databricks" + assert doc["model"] == "gpt-5" + assert "ucode-databricks" in doc["model_providers"] + native = result["managed_configs"]["codex"]["native"] + assert native[0]["path"] == str(native_path) + assert native[0]["format"] == "toml" + + def test_native_config_preserves_user_keys(self, tmp_path, monkeypatch): + _, native_path = self._patch(tmp_path, monkeypatch) + native_path.parent.mkdir(parents=True, exist_ok=True) + native_path.write_text( + 'model = "my-own"\napproval_policy = "on-request"\n', encoding="utf-8" + ) + state = {"workspace": WS, "codex_models": ["gpt-5"], "write_native_config": True} + codex.write_tool_config(state) + + doc = read_toml_safe(native_path) + # ucode pins its own model, but the user's unrelated keys survive. + assert doc["approval_policy"] == "on-request" + assert doc["model"] == "gpt-5" + + def test_no_native_write_by_default(self, tmp_path, monkeypatch): + _, native_path = self._patch(tmp_path, monkeypatch) + state = {"workspace": WS, "codex_models": ["gpt-5"]} + result = codex.write_tool_config(state) + assert not native_path.exists() + assert "native" not in result["managed_configs"]["codex"] + + def test_revert_strips_native_entries(self, tmp_path, monkeypatch): + _, native_path = self._patch(tmp_path, monkeypatch) + native_path.parent.mkdir(parents=True, exist_ok=True) + native_path.write_text( + 'model = "gpt-5"\nmodel_provider = "ucode-databricks"\napproval_policy = "on-request"\n' + '\n[model_providers.ucode-databricks]\nbase_url = "x"\n', + encoding="utf-8", + ) + state = { + "managed_configs": { + "codex": { + "keys": [], + "native": [{"path": str(native_path), "format": "toml", "keys": []}], + } + } + } + assert codex.revert_native_config(state) == "ucode entries removed" + doc = read_toml_safe(native_path) + assert "model_provider" not in doc + assert "model" not in doc + assert "model_providers" not in doc + # The user's own key is left intact. + assert doc["approval_policy"] == "on-request" + + def test_revert_returns_none_without_native(self): + state = {"managed_configs": {"codex": {"keys": []}}} + assert codex.revert_native_config(state) is None diff --git a/tests/test_config_io.py b/tests/test_config_io.py index 1d59c33c..d15bc3cc 100644 --- a/tests/test_config_io.py +++ b/tests/test_config_io.py @@ -14,6 +14,7 @@ ensure_parent_dir, is_dry_run, parse_dotenv, + prune_key_paths, read_json_safe, read_toml_safe, restore_file, @@ -313,3 +314,41 @@ def test_mutates_and_returns_base(self): base = {"a": 1} result = deep_merge_dict(base, {"b": 2}) assert result is base + + +# --------------------------------------------------------------------------- +# prune_key_paths +# --------------------------------------------------------------------------- + + +class TestPruneKeyPaths: + def test_removes_leaf_and_leaves_siblings(self): + doc = {"env": {"MANAGED": "1", "USER": "2"}, "apiKeyHelper": "x"} + changed = prune_key_paths(doc, [["env", "MANAGED"], ["apiKeyHelper"]]) + assert changed is True + assert doc == {"env": {"USER": "2"}} + + def test_drops_emptied_parent(self): + doc = {"env": {"ONLY": "1"}} + assert prune_key_paths(doc, [["env", "ONLY"]]) is True + assert doc == {} + + def test_keeps_parent_with_remaining_siblings(self): + doc = {"env": {"A": "1", "B": "2"}} + assert prune_key_paths(doc, [["env", "A"]]) is True + assert doc == {"env": {"B": "2"}} + + def test_missing_path_is_noop(self): + doc = {"env": {"A": "1"}} + assert prune_key_paths(doc, [["env", "NOPE"], ["absent"]]) is False + assert doc == {"env": {"A": "1"}} + + def test_removes_top_level_key(self): + doc = {"model_provider": "ucode", "model": "gpt-5", "other": True} + assert prune_key_paths(doc, [["model_provider"], ["model"]]) is True + assert doc == {"other": True} + + def test_partial_path_through_non_dict_is_noop(self): + doc = {"model": "scalar"} + assert prune_key_paths(doc, [["model", "nested"]]) is False + assert doc == {"model": "scalar"} diff --git a/tests/test_managed_resolve.py b/tests/test_managed_resolve.py index 5dbf41d3..6058b4a9 100644 --- a/tests/test_managed_resolve.py +++ b/tests/test_managed_resolve.py @@ -18,10 +18,11 @@ 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 +from ucode.state import MANAGED_OVERLAY_KEY, _without_managed_overlay WORKSPACE = "https://ws.example.com" @@ -183,6 +184,41 @@ def test_layers_provider_without_dropping_other_tools(self): } +class TestGlobalSettings: + def test_only_claude_and_codex_support_global_settings(self): + # This set gates both the write path AND the `ucode setup` machine-wide prompt. Adding an + # agent whose token can't self-refresh here would re-introduce a config that breaks in ~1h. + from ucode.agents import GLOBAL_SETTINGS_AGENTS + + assert GLOBAL_SETTINGS_AGENTS == frozenset({"claude", "codex"}) + + def test_flag_true_for_opted_in_supported_agent(self): + assert managed_use_as_global_settings(MANAGED, "claude") is True + + 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_sets_transient_write_native_config(self): + resolved = resolve_state(MANAGED, _state(), "claude") + assert resolved["write_native_config"] is True + + def test_resolve_omits_flag_when_not_opted_in(self): + resolved = resolve_state(MANAGED, _state(), "codex") + assert "write_native_config" not in resolved + + def test_write_native_config_is_not_persisted(self): + # It lives only for the config-write; save_state (via _without_managed_overlay) drops it so + # a later non-managed launch never writes native files. + resolved = resolve_state(MANAGED, _state(), "claude") + assert "write_native_config" not in _without_managed_overlay(resolved) + + class TestStateFileIsNotRewritten: """The managed config must win by precedence, not by overwriting the developer's state file. diff --git a/tests/test_managed_wizard.py b/tests/test_managed_wizard.py index c165f5f0..603a2d3a 100644 --- a/tests/test_managed_wizard.py +++ b/tests/test_managed_wizard.py @@ -1514,6 +1514,25 @@ 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): + # claude/codex can be machine-wide, so they carry the scope; gemini can't, so it doesn't. + manifest = { + "default_agent": "claude", + "enabled_agents": { + "claude": { + "model_config": {"default_model": "system.ai.claude-opus-4-8"}, + "use_as_global_settings": True, + }, + "gemini": {"model_config": {"default_model": "system.ai.gemini-3-flash"}}, + }, + } + wizard._render_summary(WORKSPACE, manifest) + out = capsys.readouterr().out + assert "machine-wide" in out + # The gemini line names its model but carries no per-user/machine-wide scope. + gemini_line = next(line for line in out.splitlines() if "gemini-3-flash" in line) + assert "per-user" not in gemini_line and "machine-wide" not in gemini_line + class TestSetupFromFile: def _write(self, tmp_path, payload): diff --git a/tests/test_state.py b/tests/test_state.py index 4555af67..ec3a970d 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -246,6 +246,20 @@ def test_drops_falsy_managed_configs(self): assert "codex" not in result["managed_configs"] assert "claude" not in result["managed_configs"] + def test_preserves_native_tracking(self): + native = [ + {"path": "/home/u/.claude/settings.json", "format": "json", "keys": [["env", "X"]]} + ] + state = {"managed_configs": {"claude": {"keys": [["env", "X"]], "native": native}}} + result = hydrate_state(state) + # Without preserving `native`, `ucode revert` would strand ucode's keys in the user's file. + assert result["managed_configs"]["claude"]["native"] == native + + def test_drops_non_list_native(self): + state = {"managed_configs": {"claude": {"keys": [], "native": "bogus"}}} + result = hydrate_state(state) + assert "native" not in result["managed_configs"]["claude"] + class TestBuildAgentState: def test_returns_empty_without_workspace(self): @@ -289,3 +303,13 @@ def test_preserves_existing_managed_configs(self): result = mark_tool_managed(state, "codex", [["profile"]]) assert "gemini" in result["managed_configs"] assert "codex" in result["managed_configs"] + + def test_records_native_descriptor(self): + native = [{"path": "/x/config.toml", "format": "toml", "keys": [["model_provider"]]}] + result = mark_tool_managed({}, "codex", [["model"]], native=native) + assert result["managed_configs"]["codex"] == {"keys": [["model"]], "native": native} + + def test_no_native_key_when_none(self): + result = mark_tool_managed({}, "claude", [["env", "X"]]) + assert result["managed_configs"]["claude"] == {"keys": [["env", "X"]]} + assert "native" not in result["managed_configs"]["claude"] From 765208b8913df76cc0ad953f4e5da7df24008f1b Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Wed, 12 Aug 2026 17:56:55 +0000 Subject: [PATCH 02/10] ci: retrigger (transient CLI-download network flake) From b10272269fa6a4cb453ddcd0c65e4839449739c1 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Wed, 12 Aug 2026 19:12:48 +0000 Subject: [PATCH 03/10] Fix native-descriptor stranding and revert destroying user hooks Addresses Isaac Review findings on this PR: - mark_tool_managed dropped the persisted native descriptor whenever a re-launch wrote no native file (use_as_global_settings unset, a relayed Claude launch, or a legacy-layout Codex launch). ucode's keys stayed in the user's shared file but revert could no longer find them. Preserve the prior descriptor when native is None (revert wipes state wholesale, so no stale descriptor lingers). - claude revert_native_config path-pruned whole hook-event arrays (hooks.Stop, hooks.PreToolUse/SessionStart/SubagentStart), deleting the user's own hooks. Route hook-event keys through the marker-matched removers (remove_smart_routing_hooks / _remove_tracing_stop_hook), symmetric with the write path; only plain keys go to prune_key_paths. Co-authored-by: Isaac --- src/ucode/agents/claude.py | 25 ++++++++++++++++- src/ucode/state.py | 11 ++++++++ tests/test_agent_claude.py | 57 ++++++++++++++++++++++++++++++++++++++ tests/test_state.py | 21 ++++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 7ab0ada8..c042b733 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -639,7 +639,30 @@ def revert_native_config(state: dict) -> str | None: if not path or not path.exists(): continue doc = read_json_safe(path) - if prune_key_paths(doc, keys): + # Hook-event keys ([`hooks`, ]) address the user's own shared hook arrays. Pruning the + # whole path would delete every hook they registered under that event, not just ucode's — so + # route those through the same marker-matched removers the write path uses (symmetric with + # `sync_smart_routing_hooks` / `_upsert_tracing_stop_hook` in `write_tool_config`). Only plain, + # ucode-owned key paths go to `prune_key_paths`. + plain_keys: list[list[str]] = [] + touches_routing_hooks = False + touches_tracing_stop_hook = False + for key in keys: + if len(key) == 2 and key[0] == "hooks" and key[1] in CLAUDE_ROUTING_HOOK_EVENTS: + touches_routing_hooks = True + elif len(key) == 2 and key[0] == "hooks" and key[1] == "Stop": + touches_tracing_stop_hook = True + else: + plain_keys.append(key) + file_changed = prune_key_paths(doc, plain_keys) + if touches_routing_hooks and remove_smart_routing_hooks(doc): + file_changed = True + if touches_tracing_stop_hook: + before = json.dumps(doc.get("hooks"), sort_keys=True) + _remove_tracing_stop_hook(doc) + if json.dumps(doc.get("hooks"), sort_keys=True) != before: + file_changed = True + if file_changed: write_json_file(path, doc) changed = True return "ucode entries removed" if changed else "unchanged" diff --git a/src/ucode/state.py b/src/ucode/state.py index 82f98632..7059d109 100644 --- a/src/ucode/state.py +++ b/src/ucode/state.py @@ -251,11 +251,22 @@ def mark_tool_managed( ``native`` optionally describes the agent's own native config file(s) ucode also wrote under ``use_as_global_settings`` — each ``{"path": str, "format": "json"|"toml", "keys": [...]}`` — so ``ucode revert`` can surgically prune only ucode's keys from the user's shared file. + + ``native=None`` means "this launch wrote no native file", not "clear the tracking": a later + launch that skips the native write (the admin unset ``use_as_global_settings``, a relayed Claude + launch, or a legacy-layout Codex launch) leaves ucode's keys sitting in the user's shared file, so + the descriptor from the launch that *did* write them must be preserved or ``ucode revert`` can no + longer find and prune them. ``ucode revert`` wipes state wholesale (``clear_state``), so a stale + descriptor never lingers past a revert. """ managed_configs = dict(state.get("managed_configs") or {}) entry: dict = {"keys": list(managed_keys)} if native: entry["native"] = native + else: + prior_native = (managed_configs.get(tool) or {}).get("native") + if isinstance(prior_native, list) and prior_native: + entry["native"] = prior_native managed_configs[tool] = entry state["managed_configs"] = managed_configs state["last_tool"] = tool diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 050d020d..ccfadf8a 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -561,6 +561,63 @@ def test_returns_none_without_native_tracking(self): state = {"managed_configs": {"claude": {"keys": []}}} assert claude.revert_native_config(state) is None + def test_preserves_user_hooks_under_managed_events(self, tmp_path): + # Regression: the descriptor's `keys` include whole hook-event paths (["hooks","PreToolUse"], + # ["hooks","Stop"], ...). Path-pruning those deleted the user's own hooks registered under the + # same events. Revert must surgically strip only ucode's marker-matched hooks, symmetric with + # the write path. + user_pre = {"matcher": "Bash", "hooks": [{"type": "command", "command": "my-linter"}]} + ucode_pre = { + "matcher": "Agent|Task", + "hooks": [{"type": "command", "command": "auth claude-router-hook route-subagent"}], + } + user_stop = {"hooks": [{"type": "command", "command": "my-notify"}]} + ucode_stop = {"hooks": [{"type": "command", "command": "mlflow autolog claude stop-hook"}]} + native_path = tmp_path / "settings.json" + native_path.write_text( + json.dumps( + { + "env": {"ANTHROPIC_BASE_URL": "x", "MY": "keep"}, + "hooks": { + "PreToolUse": [user_pre, ucode_pre], + "SessionStart": [ + {"hooks": [{"type": "command", "command": "auth claude-router-hook s"}]} + ], + "Stop": [user_stop, ucode_stop], + }, + } + ), + encoding="utf-8", + ) + state = { + "managed_configs": { + "claude": { + "keys": [], + "native": [ + { + "path": str(native_path), + "format": "json", + "keys": [ + ["env", "ANTHROPIC_BASE_URL"], + ["hooks", "PreToolUse"], + ["hooks", "SessionStart"], + ["hooks", "Stop"], + ], + } + ], + } + } + } + assert claude.revert_native_config(state) == "ucode entries removed" + result = json.loads(native_path.read_text()) + # ucode's env key gone, the user's kept. + assert result["env"] == {"MY": "keep"} + # The user's own hooks survive; ucode's marker-matched hooks and the now-empty + # SessionStart event are gone. + assert result["hooks"]["PreToolUse"] == [user_pre] + assert result["hooks"]["Stop"] == [user_stop] + assert "SessionStart" not in result["hooks"] + class TestRegisterWebSearchMcp: def test_clears_existing_then_adds(self, monkeypatch): diff --git a/tests/test_state.py b/tests/test_state.py index ec3a970d..d507d408 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -313,3 +313,24 @@ def test_no_native_key_when_none(self): result = mark_tool_managed({}, "claude", [["env", "X"]]) assert result["managed_configs"]["claude"] == {"keys": [["env", "X"]]} assert "native" not in result["managed_configs"]["claude"] + + def test_preserves_prior_native_when_native_is_none(self): + # A re-launch that writes no native file (use_as_global_settings unset, a relayed Claude + # launch, or a legacy-layout Codex launch) must not drop the descriptor from the launch that + # did write ucode's keys — otherwise `ucode revert` can no longer prune them. + native = [{"path": "/x/config.toml", "format": "toml", "keys": [["model_provider"]]}] + state = mark_tool_managed({}, "codex", [["model"]], native=native) + result = mark_tool_managed(state, "codex", [["model"]], native=None) + assert result["managed_configs"]["codex"]["native"] == native + + def test_native_none_without_prior_leaves_no_native(self): + state = mark_tool_managed({}, "claude", [["env", "X"]]) + result = mark_tool_managed(state, "claude", [["env", "Y"]], native=None) + assert "native" not in result["managed_configs"]["claude"] + + def test_new_native_replaces_prior(self): + first = [{"path": "/a", "format": "json", "keys": [["a"]]}] + second = [{"path": "/b", "format": "json", "keys": [["b"]]}] + state = mark_tool_managed({}, "claude", [["env", "X"]], native=first) + result = mark_tool_managed(state, "claude", [["env", "X"]], native=second) + assert result["managed_configs"]["claude"]["native"] == second From 712c8d72d44e92cf523eae704cbd09d4656f043e Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Thu, 13 Aug 2026 21:56:57 +0000 Subject: [PATCH 04/10] Write agents' OS managed settings file (claude, codex), isaac-style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under use_as_global_settings, write the agent's OS-level managed settings file so a bare `claude`/`codex` picks up the gateway, not just `ucode `: - claude: /etc/claude-code/managed-settings.json (mac: /Library/…) — JSON - codex: /etc/codex/managed_config.toml — TOML New managed_files.py mirrors isaac (devtools/ai/llm_lib/core/config.py): a drift check reads the world-readable file with no sudo and no-ops when unchanged (so no password prompt on the common launch), else temp-file -> `sudo cp` with chattr/chflags immutable handling and actionable errors. Revert surgically prunes only ucode's keys via the same sudo path. Renames the transient flag write_native_config -> write_managed_config. Co-authored-by: Isaac --- src/ucode/agents/claude.py | 73 +++++++-------- src/ucode/agents/codex.py | 92 ++++++++++++------- src/ucode/cli.py | 15 +-- src/ucode/managed_files.py | 167 ++++++++++++++++++++++++++++++++++ src/ucode/managed_resolve.py | 17 ++-- src/ucode/managed_wizard.py | 4 +- tests/test_agent_claude.py | 131 +++++++++++++++----------- tests/test_agent_codex.py | 66 ++++++++------ tests/test_managed_files.py | 101 ++++++++++++++++++++ tests/test_managed_resolve.py | 12 +-- tests/test_managed_wizard.py | 8 +- 11 files changed, 509 insertions(+), 177 deletions(-) create mode 100644 src/ucode/managed_files.py create mode 100644 tests/test_managed_files.py diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index c042b733..0aa182df 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -32,6 +32,7 @@ get_databricks_token, ) from ucode.launcher import exec_or_spawn +from ucode.managed_files import prune_managed_file, write_managed_file from ucode.smart_routing.claude_hooks import ( remove_smart_routing_hooks, sync_smart_routing_hooks, @@ -44,10 +45,6 @@ CLAUDE_CONFIG_DIR = Path.home() / ".claude" CLAUDE_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "ucode-settings.json" CLAUDE_BACKUP_PATH = APP_DIR / "claude-ucode-settings.backup.json" -# Claude Code's own user-scope settings file, read by a bare `claude` with no flags. ucode writes -# here (in addition to the private file above) only when the managed config sets -# use_as_global_settings, so a developer who launches `claude` directly still hits the gateway. -CLAUDE_NATIVE_SETTINGS_PATH = CLAUDE_CONFIG_DIR / "settings.json" SPEC: ToolSpec = { "binary": "claude", @@ -563,9 +560,9 @@ def _compose(base: dict) -> dict: write_json_file(CLAUDE_SETTINGS_PATH, _compose(read_json_safe(CLAUDE_SETTINGS_PATH))) - native_configs = None - if state.get("write_native_config"): - native_configs = _write_native_settings(_compose, managed_keys, relayed) + managed_descriptors = None + if state.get("write_managed_config"): + managed_descriptors = _write_managed_settings(_compose, managed_keys, relayed) if web_search_model: _register_web_search_mcp(state["workspace"], web_search_model, state.get("profile")) @@ -577,20 +574,22 @@ def _compose(base: dict) -> dict: else: state.pop("claude_relayed", None) state.pop("relayed_proxy_port", None) - state = mark_tool_managed(state, "claude", managed_keys, native=native_configs) + state = mark_tool_managed(state, "claude", managed_keys, native=managed_descriptors) save_state(state) return state -def _write_native_settings( +def _write_managed_settings( compose: Callable[[dict], dict], managed_keys: list[list[str]], relayed: bool ) -> list[dict] | None: - """Mirror ucode's managed config into Claude Code's native ~/.claude/settings.json. + """Write ucode's config into Claude Code's OS managed-settings.json so a bare `claude` works. - Runs only under use_as_global_settings so a bare `claude` reaches the gateway. The same compose - (merge overlay + prune stale keys) that produced the private file is applied to the user's own - settings, so their unrelated keys survive. Returns the native descriptor for revert tracking, or - None when nothing was written. + Runs only under use_as_global_settings. The managed file is root-owned and the highest-precedence + scope, so it applies whether or not `ucode` launches `claude`. The same compose (merge overlay + + prune stale keys) that produced the private file is applied to the existing managed file, so any + real IT-authored keys already there survive. The write goes through the isaac-style sudo path + (drift-suppressed, so no password prompt when unchanged). Returns the descriptor for revert + tracking, or None when nothing was written. Relayed launches are skipped: they depend on a per-session loopback refresh proxy that only runs during `ucode claude`, so a bare `claude` could not reach the gateway anyway. @@ -598,36 +597,30 @@ def _write_native_settings( if relayed: print_warning( "Claude subscription-relay launches use a per-session proxy a bare `claude` can't " - "reach, so ucode did not write ~/.claude/settings.json for machine-wide use. Launch " - "with `ucode claude` to use the relay." + "reach, so ucode did not write the managed settings file. Launch with `ucode claude` " + "to use the relay." ) return None - write_json_file( - CLAUDE_NATIVE_SETTINGS_PATH, compose(read_json_safe(CLAUDE_NATIVE_SETTINGS_PATH)) - ) - # Enterprise managed settings outrank the user scope per key and can't be excluded, so warn when - # they'd shadow what we just wrote — a bare `claude` would silently ignore ucode's config there. - conflict = _managed_relayed_conflicts() - overrides = managed_settings_model_overrides() - if conflict: - conflict_path, keys = conflict - print_warning( - f"Enterprise managed settings at {conflict_path} set {', '.join(keys)}, which override " - "~/.claude/settings.json — a bare `claude` may not route through the gateway." - ) - elif overrides: + path = _managed_settings_path() + if path is None: print_warning( - f"Enterprise managed settings at {overrides} pin a model, which overrides the workspace " - "default a bare `claude` would otherwise use." + "Machine-wide Claude settings aren't supported on this platform; skipped the managed " + "settings write." ) - return [{"path": str(CLAUDE_NATIVE_SETTINGS_PATH), "format": "json", "keys": managed_keys}] + return None + desired = json.dumps(compose(read_json_safe(path)), indent=2) + status = write_managed_file(path, desired, display="Claude Code") + if status == "skipped": + return None + return [{"path": str(path), "format": "json", "keys": managed_keys}] -def revert_native_config(state: dict) -> str | None: - """Surgically strip ucode's keys from native config files it wrote under use_as_global_settings. +def revert_managed_config(state: dict) -> str | None: + """Surgically strip ucode's keys from the managed settings file it wrote under + use_as_global_settings, writing the pruned file back via sudo. Returns a short status ("ucode entries removed" / "unchanged") for the revert summary, or None - when ucode never wrote a native file for claude. + when ucode never wrote a managed file for claude. """ native = ((state.get("managed_configs") or {}).get("claude") or {}).get("native") if not isinstance(native, list) or not native: @@ -663,8 +656,12 @@ def revert_native_config(state: dict) -> str | None: if json.dumps(doc.get("hooks"), sort_keys=True) != before: file_changed = True if file_changed: - write_json_file(path, doc) - changed = True + # Root-owned managed file: write the pruned content back via sudo (drift-suppressed). + if ( + prune_managed_file(path, json.dumps(doc, indent=2), display="Claude Code") + != "skipped" + ): + changed = True return "ucode entries removed" if changed else "unchanged" diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 06690f05..05f29d6c 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -9,6 +9,8 @@ import time from pathlib import Path +import tomlkit + from ucode.agent_updates import available_npm_package_update from ucode.config_io import ( APP_DIR, @@ -24,6 +26,7 @@ get_databricks_token, ) from ucode.launcher import exec_or_spawn +from ucode.managed_files import write_managed_file from ucode.smart_routing.codex_hooks import ( remove_smart_routing_hooks, sync_smart_routing_hooks, @@ -354,60 +357,74 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non 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 the native ~/.codex/config.toml so a - # bare `codex` (no `--profile ucode`) defaults to the gateway. Written last, after - # `_remove_legacy_ucode_profile` above stripped the legacy entries, so the modern provider block - # survives. codex auth self-refreshes via `ucode auth-token`, so the native file keeps working. - native_configs = None - if state.get("write_native_config"): - native_configs = _write_native_config( + # 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 isaac-style sudo path. + managed_descriptors = None + if state.get("write_managed_config"): + managed_descriptors = _write_managed_config( workspace, chosen_model, databricks_profile, bool(state.get("use_pat")), provider ) - state = mark_tool_managed(state, "codex", MANAGED_KEYS, native=native_configs) + state = mark_tool_managed(state, "codex", MANAGED_KEYS, native=managed_descriptors) save_state(state) return state -def _write_native_config( +def _managed_config_path() -> Path | None: + """OS-level Codex managed config file, or None on unsupported platforms. + + Linux and macOS both use ``/etc/codex/managed_config.toml`` (root-owned, highest precedence); + Windows uses ``~/.codex/managed_config.toml``. See + https://learn.chatgpt.com/docs/enterprise/managed-configuration. + """ + if sys.platform == "darwin" or sys.platform.startswith("linux"): + return Path("/etc/codex/managed_config.toml") + if sys.platform.startswith("win"): + return Path.home() / ".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, -) -> list[dict]: - """Merge the modern overlay into the native ~/.codex/config.toml, preserving the user's keys. +) -> list[dict] | None: + """Merge the modern overlay into Codex's OS managed_config.toml, preserving any other keys there. - Returns the native descriptor for revert tracking. + Written via the isaac-style sudo path (drift-suppressed). Returns the descriptor for revert + tracking, or None when nothing was written. """ + 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." + ) + return None overlay = render_overlay( workspace, model, databricks_profile, use_pat=use_pat, provider=provider ) - doc = read_toml_safe(LEGACY_CODEX_CONFIG_PATH) + doc = read_toml_safe(path) deep_merge_dict(doc, overlay) if provider: # deep_merge can't drop keys; clear a `model` a prior non-provider run pinned. doc.pop("model", None) - write_toml_file(LEGACY_CODEX_CONFIG_PATH, doc) - return [ - { - "path": str(LEGACY_CODEX_CONFIG_PATH), - "format": "toml", - "keys": [list(k) for k in MANAGED_KEYS], - } - ] + status = write_managed_file(path, tomlkit.dumps(doc), display="Codex") + if status == "skipped": + return None + return [{"path": str(path), "format": "toml", "keys": [list(k) for k in MANAGED_KEYS]}] -def _strip_modern_ucode_entries(path: Path) -> bool: - """Surgically remove ucode's *modern* keys from a shared Codex config. +def _strip_modern_ucode_entries(doc: tomlkit.TOMLDocument) -> bool: + """Surgically remove ucode's *modern* keys from an in-memory Codex config document. Drops the top-level ``model_provider = "ucode-databricks"`` selector (and the ``model`` pinned alongside it) and the ``[model_providers.ucode-databricks]`` block, leaving the user's other keys - intact. Mirrors :func:`_strip_legacy_ucode_entries` for the modern native write. Returns True if - anything was removed. + intact. Mirrors :func:`_strip_legacy_ucode_entries`. Returns True if anything was removed. """ - if not path.exists(): - return False - doc = read_toml_safe(path) changed = False if doc.get("model_provider") == CODEX_MODEL_PROVIDER_NAME: doc.pop("model_provider", None) @@ -420,20 +437,27 @@ def _strip_modern_ucode_entries(path: Path) -> bool: if not providers: doc.pop("model_providers", None) changed = True - if changed: - write_toml_file(path, doc) return changed -def revert_native_config(state: dict) -> str | None: - """Strip ucode's modern keys from ~/.codex/config.toml if it was written under global settings. +def revert_managed_config(state: dict) -> str | None: + """Strip ucode's modern keys from Codex's managed config if it was written under global settings, + writing the pruned file back via sudo. - Returns a short status for the revert summary, or None when ucode never wrote the native file. + Returns a short status for the revert summary, or None when ucode never wrote the managed file. """ native = ((state.get("managed_configs") or {}).get("codex") or {}).get("native") if not isinstance(native, list) or not native: return None - changed = _strip_modern_ucode_entries(LEGACY_CODEX_CONFIG_PATH) + changed = False + for descriptor in native: + path = Path(descriptor.get("path", "")) + if not path or not path.exists(): + continue + doc = read_toml_safe(path) + if _strip_modern_ucode_entries(doc): + if write_managed_file(path, tomlkit.dumps(doc), display="Codex") != "skipped": + changed = True return "ucode entries removed" if changed else "unchanged" diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 46dd6e96..bb19e890 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -999,11 +999,12 @@ def revert() -> int: # Older Codex (< 0.134.0) had ucode edit the shared ~/.codex/config.toml in # place; restoring the per-profile file above does not undo that. legacy_codex_stripped = revert_legacy_shared_config() - # Native config files written under use_as_global_settings (a bare `claude` / `codex` reading - # the tool's own config): surgically strip ucode's keys, never touching the user's own settings. - native_reverts = { - "claude": claude_agent.revert_native_config(state), - "codex": codex_agent.revert_native_config(state), + # OS managed settings files written under use_as_global_settings (the highest-precedence config a + # bare `claude` / `codex` reads): surgically strip ucode's keys via sudo, never touching other + # keys. Runs before clear_state so the tracked descriptors are still available. + managed_reverts = { + "claude": claude_agent.revert_managed_config(state), + "codex": codex_agent.revert_managed_config(state), } clear_state() @@ -1013,9 +1014,9 @@ def revert() -> int: print_kv(f"{spec['display']} config", "restored" if results[tool] else "unchanged") if legacy_codex_stripped: print_kv("Codex shared config", "ucode entries removed") - for tool, status in native_reverts.items(): + for tool, status in managed_reverts.items(): if status: - print_kv(f"{TOOL_SPECS[tool]['display']} native config", status) + print_kv(f"{TOOL_SPECS[tool]['display']} managed config", status) print_kv("Pi settings", "restored" if pi_settings_restored else "unchanged") for client, spec in MCP_CLIENTS.items(): print_kv( diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py new file mode 100644 index 00000000..df0f75f2 --- /dev/null +++ b/src/ucode/managed_files.py @@ -0,0 +1,167 @@ +"""Write agent config into OS-level *managed settings* files. + +These files are root-owned and the highest-precedence config scope for their agent — a bare +``claude`` / ``codex`` (launched directly, without ucode) reads them, so writing here is what makes +the gateway config apply outside ``ucode ``: + +- Claude Code: ``/etc/claude-code/managed-settings.json`` (Linux), + ``/Library/Application Support/ClaudeCode/managed-settings.json`` (macOS) +- Codex: ``/etc/codex/managed_config.toml`` (Linux + macOS) + +The write mirrors isaac's approach (``devtools/ai/llm_lib/core/config.py`` in universe): a **drift +check** reads the world-readable file WITHOUT sudo and does nothing when it already matches, so the +common no-op launch never prompts for a password; only a real change shells out to ``sudo`` (temp +file → ``sudo cp``), clearing and restoring the immutable flag (``chattr``/``chflags``) that a fleet +golden image may have set. Writing needs root, so the first write (or one after the config changes) +prompts for the developer's sudo password — the same tradeoff isaac makes. +""" + +from __future__ import annotations + +import os +import shlex +import subprocess +import sys +import tempfile +from pathlib import Path + +from ucode.config_io import is_dry_run +from ucode.ui import console, print_err, print_warning + +# Absolute path so a stripped PATH (desktop/GUI launchers) still finds it; matches isaac. +_SUDO = "/usr/bin/sudo" + + +def managed_files_supported() -> bool: + """True on the platforms whose managed-settings write path is implemented (Linux, macOS).""" + return sys.platform == "darwin" or sys.platform.startswith("linux") + + +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 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" + 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: + """Replace ``path`` with ``desired_text`` via sudo (temp file → ``sudo cp``), handling immutability. + + Writes the payload to a user-owned temp file first (no sudo), then copies it into place with + ``sudo`` and makes it world-readable — the same sequence isaac uses so the file it lays down is + readable by the agent binary regardless of who launched it. + """ + subprocess.run([_SUDO, "mkdir", "-p", str(path.parent)], check=True) + with tempfile.NamedTemporaryFile( + mode="w", suffix=path.suffix or ".tmp", delete=False, encoding="utf-8" + ) as tmp: + tmp.write(desired_text) + tmp_path = tmp.name + try: + restore_immutable = _clear_immutable(path) + try: + # capture_output so the CalledProcessError on failure (e.g. still-immutable dest) carries + # cp's stderr for an actionable message. + subprocess.run( + [_SUDO, "cp", tmp_path, str(path)], capture_output=True, text=True, check=True + ) + subprocess.run([_SUDO, "chmod", "a+rx", str(path.parent)], check=True) + subprocess.run([_SUDO, "chmod", "a+r", str(path)], check=True) + finally: + if restore_immutable: + _restore_immutable(path) + finally: + os.unlink(tmp_path) + + +def _clear_immutable(path: Path) -> bool: + """Clear an immutable flag a fleet golden image may have set. Returns whether to restore it. + + macOS: preserve JAMF's system-immutable ``schg`` across the update — inspect, unlock only when + set, and report that it must be restored. Linux: best-effort ``chattr -i`` (not every filesystem + supports it), never restored — matching isaac. + """ + if not path.exists(): + return False + if sys.platform == "darwin": + result = subprocess.run( + ["/usr/bin/stat", "-f", "%Sf", str(path)], capture_output=True, text=True, check=False + ) + if result.returncode == 0 and "schg" in result.stdout.strip().split(","): + subprocess.run( + [_SUDO, "chflags", "noschg", str(path)], capture_output=True, text=True, check=True + ) + return True + return False + subprocess.run([_SUDO, "chattr", "-i", str(path)], capture_output=True, text=True, check=False) + return False + + +def _restore_immutable(path: Path) -> None: + """Re-set macOS's ``schg`` flag after a write. Best-effort so it never masks the write result.""" + result = subprocess.run( + [_SUDO, "chflags", "schg", str(path)], capture_output=True, text=True, check=False + ) + if result.returncode != 0: + 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 = len(cmd) >= 2 and cmd[1] == "cp" + if cp_failed and "Operation not permitted" in stderr: + quoted = shlex.quote(str(path)) + clear_cmd = f"sudo {'chflags noschg' if sys.platform == 'darwin' 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 prune_managed_file(path: Path, pruned_text: str, *, display: str) -> str: + """Write back a managed file with ucode's keys removed (used by ``ucode revert``). + + ``pruned_text`` is the file's content with ucode's entries stripped. Goes through the same + drift-suppressed sudo write, so when ucode's keys weren't present the write is a no-op with no + password prompt. + """ + return write_managed_file(path, pruned_text, display=display) diff --git a/src/ucode/managed_resolve.py b/src/ucode/managed_resolve.py index 509b9b30..73aaad00 100644 --- a/src/ucode/managed_resolve.py +++ b/src/ucode/managed_resolve.py @@ -179,11 +179,12 @@ def managed_provider_service(managed: dict, tool: str) -> str | None: 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 own native config file so a bare + ``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` can honor it (their auth self-refreshes); the flag - is ignored for any other agent, so a hand-written ``--from-file`` config can't turn it on for an - agent that would silently break after the token expires. + :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. """ from ucode.agents import GLOBAL_SETTINGS_AGENTS @@ -292,10 +293,10 @@ def resolve_state(managed: dict, state: dict, tool: str) -> dict: 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 - # native file. A non-managed launch never sets it, so default behavior is unchanged. - overlay["write_native_config"] = state.get("write_native_config") - resolved["write_native_config"] = True + # 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_wizard.py b/src/ucode/managed_wizard.py index 7c86a8dd..e9e189b9 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -829,7 +829,9 @@ def _render_summary(workspace: str, manifest: dict) -> None: 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" + 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)) diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index ccfadf8a..37b9575a 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -4,6 +4,7 @@ import json import os +from pathlib import Path import pytest @@ -468,75 +469,101 @@ def test_strips_stale_disable_experimental_betas(self, monkeypatch): assert written[0]["env"]["CLAUDE_CODE_USE_GATEWAY"] == "1" -class TestWriteToolConfigNativeSettings: - """use_as_global_settings: also write Claude Code's own ~/.claude/settings.json.""" +FAKE_MANAGED_PATH = Path("/tmp/ucode-test/managed-settings.json") - def _patch(self, monkeypatch, writes, existing_by_path=None): + +class TestWriteToolConfigManagedSettings: + """use_as_global_settings: also write Claude Code's OS managed-settings.json (via sudo, mocked).""" + + def _patch(self, monkeypatch, private_writes, managed_writes, existing_by_path=None): existing_by_path = existing_by_path or {} monkeypatch.setattr(claude, "backup_existing_file", lambda *a, **kw: True) + # Deep-copy the seeded existing content so the compose step can't mutate the fixture. monkeypatch.setattr( - claude, "read_json_safe", lambda path: dict(existing_by_path.get(str(path), {})) + claude, + "read_json_safe", + lambda path: json.loads(json.dumps(existing_by_path.get(str(path), {}))), ) monkeypatch.setattr( - claude, "write_json_file", lambda path, payload: writes.append((str(path), payload)) + claude, + "write_json_file", + lambda path, payload: private_writes.append((str(path), payload)), ) monkeypatch.setattr(claude, "save_state", lambda state: None) monkeypatch.setattr(claude, "_register_web_search_mcp", lambda *a, **kw: True) - # Keep the enterprise managed-settings checks deterministic regardless of the host machine. - monkeypatch.setattr(claude, "_managed_relayed_conflicts", lambda: None) - monkeypatch.setattr(claude, "managed_settings_model_overrides", lambda: None) - - def test_writes_native_file_when_flagged(self, monkeypatch): - writes: list = [] - self._patch(monkeypatch, writes) - state = {"workspace": WS, "codex_models": [], "write_native_config": True} + # Deterministic managed path, and a mocked sudo writer so NO real sudo/`/etc` write happens. + monkeypatch.setattr(claude, "_managed_settings_path", lambda: FAKE_MANAGED_PATH) + + def fake_write_managed(path, text, *, display): + managed_writes.append((str(path), text)) + return "written" + + monkeypatch.setattr(claude, "write_managed_file", fake_write_managed) + + def test_writes_managed_file_when_flagged(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + self._patch(monkeypatch, private_writes, managed_writes) + state = {"workspace": WS, "codex_models": [], "write_managed_config": True} result = claude.write_tool_config(state, "databricks-claude-sonnet-4") - paths = [p for p, _ in writes] - assert str(claude.CLAUDE_SETTINGS_PATH) in paths - assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) in paths + # Private file still written; managed file written too. + assert str(claude.CLAUDE_SETTINGS_PATH) in [p for p, _ in private_writes] + assert [p for p, _ in managed_writes] == [str(FAKE_MANAGED_PATH)] native = result["managed_configs"]["claude"]["native"] - assert native[0]["path"] == str(claude.CLAUDE_NATIVE_SETTINGS_PATH) + assert native[0]["path"] == str(FAKE_MANAGED_PATH) assert native[0]["format"] == "json" - def test_native_file_preserves_user_keys(self, monkeypatch): - writes: list = [] - existing = {str(claude.CLAUDE_NATIVE_SETTINGS_PATH): {"env": {"MY_OWN": "keep"}}} - self._patch(monkeypatch, writes, existing) - state = {"workspace": WS, "codex_models": [], "write_native_config": True} + def test_managed_file_preserves_other_keys(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + # An IT-authored key already in the managed file must survive the merge. + existing = {str(FAKE_MANAGED_PATH): {"env": {"MY_OWN": "keep"}}} + self._patch(monkeypatch, private_writes, managed_writes, existing) + state = {"workspace": WS, "codex_models": [], "write_managed_config": True} claude.write_tool_config(state, "databricks-claude-sonnet-4") - native = next(p for path, p in writes if path == str(claude.CLAUDE_NATIVE_SETTINGS_PATH)) - # The user's own key survives; ucode's gateway keys are merged in alongside it. - assert native["env"]["MY_OWN"] == "keep" - assert native["env"]["ANTHROPIC_BASE_URL"] - assert native["apiKeyHelper"] - - def test_no_native_write_by_default(self, monkeypatch): - writes: list = [] - self._patch(monkeypatch, writes) + _, text = managed_writes[0] + written = json.loads(text) + assert written["env"]["MY_OWN"] == "keep" + assert written["env"]["ANTHROPIC_BASE_URL"] + assert written["apiKeyHelper"] + + def test_no_managed_write_by_default(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + self._patch(monkeypatch, private_writes, managed_writes) state = {"workspace": WS, "codex_models": []} result = claude.write_tool_config(state, "databricks-claude-sonnet-4") - paths = [p for p, _ in writes] - assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) not in paths + assert managed_writes == [] assert "native" not in result["managed_configs"]["claude"] - def test_relayed_skips_native_write(self, monkeypatch): - writes: list = [] + def test_relayed_skips_managed_write(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] warns: list = [] - self._patch(monkeypatch, writes) + self._patch(monkeypatch, private_writes, managed_writes) monkeypatch.setattr(claude, "print_warning", lambda msg: warns.append(msg)) monkeypatch.setattr(claude, "relayed_proxy_base_url", lambda state: "http://127.0.0.1:9999") - state = {"workspace": WS, "codex_models": [], "write_native_config": True} + state = {"workspace": WS, "codex_models": [], "write_managed_config": True} result = claude.write_tool_config(state, "databricks-claude-sonnet-4", relayed=True) - paths = [p for p, _ in writes] - assert str(claude.CLAUDE_NATIVE_SETTINGS_PATH) not in paths + assert managed_writes == [] assert result["managed_configs"]["claude"].get("native") is None assert any("bare `claude`" in w for w in warns) -class TestClaudeRevertNativeConfig: - def test_prunes_only_tracked_keys(self, tmp_path): - native_path = tmp_path / "settings.json" - native_path.write_text( +class TestClaudeRevertManagedConfig: + @staticmethod + def _mock_sudo_prune(monkeypatch): + # Route the sudo write straight to disk (the descriptor path is a tmp file) — never real sudo. + def fake_prune(path, text, *, display): + Path(path).write_text(text, encoding="utf-8") + return "written" + + monkeypatch.setattr(claude, "prune_managed_file", fake_prune) + + def test_prunes_only_tracked_keys(self, tmp_path, monkeypatch): + self._mock_sudo_prune(monkeypatch) + managed_path = tmp_path / "managed-settings.json" + managed_path.write_text( json.dumps({"env": {"ANTHROPIC_BASE_URL": "x", "MY": "keep"}, "apiKeyHelper": "h"}), encoding="utf-8", ) @@ -546,7 +573,7 @@ def test_prunes_only_tracked_keys(self, tmp_path): "keys": [], "native": [ { - "path": str(native_path), + "path": str(managed_path), "format": "json", "keys": [["env", "ANTHROPIC_BASE_URL"], ["apiKeyHelper"]], } @@ -554,18 +581,20 @@ def test_prunes_only_tracked_keys(self, tmp_path): } } } - assert claude.revert_native_config(state) == "ucode entries removed" - assert json.loads(native_path.read_text()) == {"env": {"MY": "keep"}} + assert claude.revert_managed_config(state) == "ucode entries removed" + assert json.loads(managed_path.read_text()) == {"env": {"MY": "keep"}} - def test_returns_none_without_native_tracking(self): + def test_returns_none_without_native_tracking(self, monkeypatch): + self._mock_sudo_prune(monkeypatch) state = {"managed_configs": {"claude": {"keys": []}}} - assert claude.revert_native_config(state) is None + assert claude.revert_managed_config(state) is None - def test_preserves_user_hooks_under_managed_events(self, tmp_path): + def test_preserves_user_hooks_under_managed_events(self, tmp_path, monkeypatch): # Regression: the descriptor's `keys` include whole hook-event paths (["hooks","PreToolUse"], # ["hooks","Stop"], ...). Path-pruning those deleted the user's own hooks registered under the # same events. Revert must surgically strip only ucode's marker-matched hooks, symmetric with # the write path. + self._mock_sudo_prune(monkeypatch) user_pre = {"matcher": "Bash", "hooks": [{"type": "command", "command": "my-linter"}]} ucode_pre = { "matcher": "Agent|Task", @@ -573,7 +602,7 @@ def test_preserves_user_hooks_under_managed_events(self, tmp_path): } user_stop = {"hooks": [{"type": "command", "command": "my-notify"}]} ucode_stop = {"hooks": [{"type": "command", "command": "mlflow autolog claude stop-hook"}]} - native_path = tmp_path / "settings.json" + native_path = tmp_path / "managed-settings.json" native_path.write_text( json.dumps( { @@ -608,7 +637,7 @@ def test_preserves_user_hooks_under_managed_events(self, tmp_path): } } } - assert claude.revert_native_config(state) == "ucode entries removed" + assert claude.revert_managed_config(state) == "ucode entries removed" result = json.loads(native_path.read_text()) # ucode's env key gone, the user's kept. assert result["env"] == {"MY": "keep"} diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index cf781584..b972a265 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +from pathlib import Path import pytest @@ -654,57 +655,66 @@ def test_fast_success_does_not_retry(self, monkeypatch): assert fallbacks == [] -class TestCodexNativeConfig: - """use_as_global_settings: also write the native ~/.codex/config.toml so a bare `codex` works.""" +class TestCodexManagedConfig: + """use_as_global_settings: also write Codex's OS managed_config.toml (via sudo, mocked).""" def _patch(self, tmp_path, monkeypatch): config_path = tmp_path / ".codex" / "ucode.config.toml" - native_path = tmp_path / ".codex" / "config.toml" + managed_path = tmp_path / "etc-codex" / "managed_config.toml" monkeypatch.setattr(codex, "CODEX_CONFIG_PATH", config_path) monkeypatch.setattr(codex, "CODEX_BACKUP_PATH", tmp_path / "codex-ucode-config.backup.toml") - monkeypatch.setattr(codex, "LEGACY_CODEX_CONFIG_PATH", native_path) monkeypatch.setattr(codex, "agent_version", lambda binary: "0.134.0") monkeypatch.setattr(codex, "save_state", lambda state: None) - return config_path, native_path + # 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 test_writes_native_config_when_flagged(self, tmp_path, monkeypatch): - _, native_path = self._patch(tmp_path, monkeypatch) - state = {"workspace": WS, "codex_models": ["gpt-5"], "write_native_config": True} + def fake_write_managed(path, text, *, display): + 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) + return config_path, managed_path + + def test_writes_managed_config_when_flagged(self, tmp_path, monkeypatch): + _, managed_path = self._patch(tmp_path, monkeypatch) + state = {"workspace": WS, "codex_models": ["gpt-5"], "write_managed_config": True} result = codex.write_tool_config(state) - doc = read_toml_safe(native_path) + doc = read_toml_safe(managed_path) assert doc["model_provider"] == "ucode-databricks" assert doc["model"] == "gpt-5" assert "ucode-databricks" in doc["model_providers"] native = result["managed_configs"]["codex"]["native"] - assert native[0]["path"] == str(native_path) + assert native[0]["path"] == str(managed_path) assert native[0]["format"] == "toml" - def test_native_config_preserves_user_keys(self, tmp_path, monkeypatch): - _, native_path = self._patch(tmp_path, monkeypatch) - native_path.parent.mkdir(parents=True, exist_ok=True) - native_path.write_text( + def test_managed_config_preserves_other_keys(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 = "my-own"\napproval_policy = "on-request"\n', encoding="utf-8" ) - state = {"workspace": WS, "codex_models": ["gpt-5"], "write_native_config": True} + state = {"workspace": WS, "codex_models": ["gpt-5"], "write_managed_config": True} codex.write_tool_config(state) - doc = read_toml_safe(native_path) - # ucode pins its own model, but the user's unrelated keys survive. + doc = read_toml_safe(managed_path) + # ucode pins its own model, but other keys already in the managed file survive. assert doc["approval_policy"] == "on-request" assert doc["model"] == "gpt-5" - def test_no_native_write_by_default(self, tmp_path, monkeypatch): - _, native_path = self._patch(tmp_path, monkeypatch) + def test_no_managed_write_by_default(self, tmp_path, monkeypatch): + _, managed_path = self._patch(tmp_path, monkeypatch) state = {"workspace": WS, "codex_models": ["gpt-5"]} result = codex.write_tool_config(state) - assert not native_path.exists() + assert not managed_path.exists() assert "native" not in result["managed_configs"]["codex"] - def test_revert_strips_native_entries(self, tmp_path, monkeypatch): - _, native_path = self._patch(tmp_path, monkeypatch) - native_path.parent.mkdir(parents=True, exist_ok=True) - native_path.write_text( + def test_revert_strips_managed_entries(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 = "gpt-5"\nmodel_provider = "ucode-databricks"\napproval_policy = "on-request"\n' '\n[model_providers.ucode-databricks]\nbase_url = "x"\n', encoding="utf-8", @@ -713,12 +723,12 @@ def test_revert_strips_native_entries(self, tmp_path, monkeypatch): "managed_configs": { "codex": { "keys": [], - "native": [{"path": str(native_path), "format": "toml", "keys": []}], + "native": [{"path": str(managed_path), "format": "toml", "keys": []}], } } } - assert codex.revert_native_config(state) == "ucode entries removed" - doc = read_toml_safe(native_path) + assert codex.revert_managed_config(state) == "ucode entries removed" + doc = read_toml_safe(managed_path) assert "model_provider" not in doc assert "model" not in doc assert "model_providers" not in doc @@ -727,4 +737,4 @@ def test_revert_strips_native_entries(self, tmp_path, monkeypatch): def test_revert_returns_none_without_native(self): state = {"managed_configs": {"codex": {"keys": []}}} - assert codex.revert_native_config(state) is None + assert codex.revert_managed_config(state) is None diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py new file mode 100644 index 00000000..f9ffed34 --- /dev/null +++ b/tests/test_managed_files.py @@ -0,0 +1,101 @@ +"""Tests for managed_files.py — the isaac-style sudo writer for OS managed settings files. + +Every test mocks the actual privileged step (`_sudo_replace`), so NO real `sudo` / `/etc` write +ever runs. The behavior that matters here is the drift check: an unchanged file must not shell out. +""" + +from __future__ import annotations + +import subprocess + +import pytest + +import ucode.config_io as config_io +from ucode import managed_files + + +@pytest.fixture(autouse=True) +def _reset_dry_run(): + config_io.set_dry_run(False) + yield + config_io.set_dry_run(False) + + +@pytest.fixture(autouse=True) +def _supported(monkeypatch): + # Pin platform support on so tests are deterministic on any host. + monkeypatch.setattr(managed_files, "managed_files_supported", lambda: True) + + +def _capture_sudo(monkeypatch): + calls: list = [] + monkeypatch.setattr( + managed_files, "_sudo_replace", lambda path, text: calls.append((str(path), text)) + ) + return calls + + +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 TestPruneManagedFile: + def test_prune_is_noop_when_already_absent(self, tmp_path, monkeypatch): + # Revert on a file that never held ucode's keys: pruned text == existing -> no sudo. + path = tmp_path / "managed.json" + path.write_text("pruned", encoding="utf-8") + calls = _capture_sudo(monkeypatch) + assert managed_files.prune_managed_file(path, "pruned", display="X") == "unchanged" + assert calls == [] diff --git a/tests/test_managed_resolve.py b/tests/test_managed_resolve.py index 6058b4a9..07c1eacd 100644 --- a/tests/test_managed_resolve.py +++ b/tests/test_managed_resolve.py @@ -204,19 +204,19 @@ def test_flag_ignored_for_unsupported_agent(self): managed = {"enabled_agents": {"gemini": {"use_as_global_settings": True}}} assert managed_use_as_global_settings(managed, "gemini") is False - def test_resolve_sets_transient_write_native_config(self): + def test_resolve_sets_transient_write_managed_config(self): resolved = resolve_state(MANAGED, _state(), "claude") - assert resolved["write_native_config"] is True + assert resolved["write_managed_config"] is True def test_resolve_omits_flag_when_not_opted_in(self): resolved = resolve_state(MANAGED, _state(), "codex") - assert "write_native_config" not in resolved + assert "write_managed_config" not in resolved - def test_write_native_config_is_not_persisted(self): + def test_write_managed_config_is_not_persisted(self): # It lives only for the config-write; save_state (via _without_managed_overlay) drops it so - # a later non-managed launch never writes native files. + # a later non-managed launch never writes the managed settings file. resolved = resolve_state(MANAGED, _state(), "claude") - assert "write_native_config" not in _without_managed_overlay(resolved) + assert "write_managed_config" not in _without_managed_overlay(resolved) class TestStateFileIsNotRewritten: diff --git a/tests/test_managed_wizard.py b/tests/test_managed_wizard.py index 603a2d3a..4657035b 100644 --- a/tests/test_managed_wizard.py +++ b/tests/test_managed_wizard.py @@ -1515,7 +1515,7 @@ def test_single_model_agent_needs_no_extra_line(self, capsys): assert "models:" not in out def test_scope_label_only_for_global_capable_agents(self, capsys): - # claude/codex can be machine-wide, so they carry the scope; gemini can't, so it doesn't. + # claude/codex can use global settings, so they carry the scope; gemini can't, so it doesn't. manifest = { "default_agent": "claude", "enabled_agents": { @@ -1528,10 +1528,10 @@ def test_scope_label_only_for_global_capable_agents(self, capsys): } wizard._render_summary(WORKSPACE, manifest) out = capsys.readouterr().out - assert "machine-wide" in out - # The gemini line names its model but carries no per-user/machine-wide scope. + 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 "per-user" not in gemini_line and "machine-wide" not in gemini_line + assert "ucode-only" not in gemini_line and "global settings" not in gemini_line class TestSetupFromFile: From 804297e27fd296f5e218d572ad406ea80b27d6c7 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Thu, 13 Aug 2026 22:37:08 +0000 Subject: [PATCH 05/10] Fix crashes/false-warning writing agents' managed settings on locked dirs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual testing on an enterprise-managed box surfaced three issues: - read_json_safe/read_toml_safe did an unguarded path.exists(), which raises PermissionError (not a clean "absent") when the file sits under a root-locked dir like a 750 /etc/codex — crashing the whole launch. Guard it -> treat as empty/absent. - managed_files._clear_immutable had the same unguarded path.exists() inside the sudo write, so the write aborted with a misleading "cannot write without root" before the chmod that opens the dir. Guard it; root's `cp` overwrites anyway. - The "enterprise managed settings may override your admin's config" warning fired even when ucode itself authored that managed file under use_as_global_settings. Suppress it (both sites) when ucode owns the file. Co-authored-by: Isaac --- src/ucode/cli.py | 14 +++++++++++--- src/ucode/config_io.py | 13 +++++++++---- src/ucode/managed_files.py | 8 +++++++- tests/test_config_io.py | 21 +++++++++++++++++++++ tests/test_managed_files.py | 15 +++++++++++++++ 5 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/ucode/cli.py b/src/ucode/cli.py index bb19e890..398f8977 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -75,6 +75,7 @@ managed_provider_service, managed_supplies_models, managed_unservable_models, + managed_use_as_global_settings, recommended_agent, resolve_state, ) @@ -1579,8 +1580,10 @@ def _launch_tool( ) # The enterprise scope outranks the --settings file ucode writes, so a model pinned # there quietly beats the admin's — point at the file rather than let the mismatch - # look like a ucode bug. - if tool == "claude": + # look like a ucode bug. Suppressed under use_as_global_settings: there ucode itself + # authored that managed-settings file, so its model keys are the admin's config, not an + # external override. + if tool == "claude" and not managed_use_as_global_settings(managed, "claude"): overrides = claude_agent.managed_settings_model_overrides() if overrides is not None: print_warning( @@ -1692,7 +1695,12 @@ def _launch_tool( # outranks the --settings file ucode writes AND can't be excluded with --setting-sources, # so a model pinned there silently wins over `--model`. Warn so a launch that ignores the # requested model looks like the misconfiguration it is, not a ucode bug. - if model and tool == "claude": + # Suppressed when ucode authored the managed-settings file itself (use_as_global_settings) + # — the pinned model is then ucode's own, deliberately applied, not a surprise override. + managed_owns_claude = managed is not None and managed_use_as_global_settings( + managed, "claude" + ) + if model and tool == "claude" and not managed_owns_claude: enterprise = claude_agent.managed_settings_model_overrides() if enterprise is not None: print_warning( diff --git a/src/ucode/config_io.py b/src/ucode/config_io.py index 3ca67ec8..6a92066d 100644 --- a/src/ucode/config_io.py +++ b/src/ucode/config_io.py @@ -142,9 +142,12 @@ def _prune_one(node: object, path: list[str]) -> bool: def read_json_safe(path: Path) -> dict: - if not path.exists(): - return {} + # `path.exists()` is inside the try: stat-ing a file under a root-locked dir (e.g. a + # root-owned /etc/codex) raises PermissionError, which must read as "absent/unreadable → {}" + # rather than crash a launch that only wanted to merge into it. try: + if not path.exists(): + return {} data = json.loads(path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError): return {} @@ -152,9 +155,11 @@ def read_json_safe(path: Path) -> dict: def read_toml_safe(path: Path) -> tomlkit.TOMLDocument: - if not path.exists(): - return tomlkit.document() + # See read_json_safe: keep `path.exists()` inside the try so a PermissionError on a locked + # parent directory is treated as an empty document rather than propagating. try: + if not path.exists(): + return tomlkit.document() return tomlkit.parse(path.read_text(encoding="utf-8")) except (OSError, tomlkit.exceptions.TOMLKitError): return tomlkit.document() diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index df0f75f2..5e882adc 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -115,7 +115,13 @@ def _clear_immutable(path: Path) -> bool: set, and report that it must be restored. Linux: best-effort ``chattr -i`` (not every filesystem supports it), never restored — matching isaac. """ - if not path.exists(): + try: + # `path.exists()` stats the file; under a root-locked parent dir (e.g. a 750 /etc/codex we + # haven't opened yet) that raises PermissionError. There's nothing to unlock we can see, and + # the subsequent `sudo cp` (as root) overwrites regardless, so treat it as "nothing to clear". + if not path.exists(): + return False + except OSError: return False if sys.platform == "darwin": result = subprocess.run( diff --git a/tests/test_config_io.py b/tests/test_config_io.py index d15bc3cc..d8852592 100644 --- a/tests/test_config_io.py +++ b/tests/test_config_io.py @@ -321,6 +321,27 @@ def test_mutates_and_returns_base(self): # --------------------------------------------------------------------------- +class _StatDeniedPath: + """Path stand-in whose existence check raises PermissionError, as stat-ing a file under a + root-locked directory (e.g. a root-owned /etc/codex) does for a non-root process.""" + + def exists(self): + raise PermissionError(13, "Permission denied") + + def read_text(self, encoding="utf-8"): + raise AssertionError("read_text must not be reached once exists() denies") + + +class TestReadSafePermissionDenied: + def test_read_json_safe_returns_empty_on_permission_error(self): + # Regression: a locked parent dir made path.exists() raise and crashed the whole launch. + assert read_json_safe(_StatDeniedPath()) == {} + + def test_read_toml_safe_returns_empty_on_permission_error(self): + result = read_toml_safe(_StatDeniedPath()) + assert dict(result) == {} + + class TestPruneKeyPaths: def test_removes_leaf_and_leaves_siblings(self): doc = {"env": {"MANAGED": "1", "USER": "2"}, "apiKeyHelper": "x"} diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index f9ffed34..9b680a46 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -91,6 +91,21 @@ def boom(path, text): assert managed_files.write_managed_file(path, "new", display="X") == "skipped" +class TestClearImmutableStatDenied: + def test_stat_denied_path_returns_false_without_raising(self, monkeypatch): + # Regression: `_clear_immutable` ran an unguarded path.exists() inside the sudo write; under a + # root-locked /etc/codex that raised PermissionError and aborted the write ("without root"). + class _StatDenied: + def exists(self): + raise PermissionError(13, "Permission denied") + + # Ensure no sudo subprocess is attempted if the guard ever regresses. + monkeypatch.setattr( + managed_files.subprocess, "run", lambda *a, **k: pytest.fail("should not shell out") + ) + assert managed_files._clear_immutable(_StatDenied()) is False + + class TestPruneManagedFile: def test_prune_is_noop_when_already_absent(self, tmp_path, monkeypatch): # Revert on a file that never held ucode's keys: pruned text == existing -> no sudo. From f83dda8c2701bb7841d71fd98d8c619ce1d7fafd Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Thu, 13 Aug 2026 22:40:12 +0000 Subject: [PATCH 06/10] setup: make the global-settings prompt describe actual behavior Name the exact file each answer writes (Claude Code's managed-settings.json / Codex's managed_config.toml) and spell out the payoff: "yes" means a bare `claude`/`codex` reaches the gateway on its own (no ucode needed), "no" keeps a ucode-only settings file. Co-authored-by: Isaac --- src/ucode/managed_wizard.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/ucode/managed_wizard.py b/src/ucode/managed_wizard.py index e9e189b9..d569d4c5 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -71,13 +71,19 @@ spinner, ) -# What `use_as_global_settings` actually does, in plain terms. Admins are choosing whether to write -# the agent's own global settings file (so it points at the gateway even when launched directly) or -# a ucode-specific one (so the agent only routes through the gateway when launched via ucode). +# 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. +GLOBAL_SETTINGS_FILES = { + "claude": "Claude Code's managed-settings.json", + "codex": "Codex's managed_config.toml", +} + +# What `use_as_global_settings` actually does, in plain terms. `{binary}` is filled in per agent. GLOBAL_SETTINGS_BLURB = ( - "Answer Yes to write this agent's own global settings file, so it points at the Databricks " - "gateway even when launched directly, without ucode. Answer no to write a ucode-specific " - "settings file instead, so the agent only routes through the gateway when launched via ucode." + "Yes writes the gateway config into that file (needs sudo once), so a bare `{binary}` reaches " + "the Databricks gateway on its own — you don't have to launch it through ucode. No writes a " + "ucode-only settings file instead, so `{binary}` uses the gateway only when started with " + "`ucode {binary}`." ) BUDGET_POLICY_BLURB = ( @@ -1096,9 +1102,10 @@ def setup_command( # 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"Write {TOOL_SPECS[tool]['display']}'s config to its global settings file? " - f"({GLOBAL_SETTINGS_BLURB})", + f"Write {TOOL_SPECS[tool]['display']}'s config to {GLOBAL_SETTINGS_FILES[tool]}? " + f"({GLOBAL_SETTINGS_BLURB.format(binary=binary)})", default=False, ) enabled_agents[tool] = agent_config From d7de31885fde35ec85bc9476f39f39ce2675ee7e Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Thu, 13 Aug 2026 22:41:18 +0000 Subject: [PATCH 07/10] setup: reword global-settings prompt to "answer yes/no to ..." Co-authored-by: Isaac --- src/ucode/managed_wizard.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ucode/managed_wizard.py b/src/ucode/managed_wizard.py index d569d4c5..409ba5df 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -80,10 +80,10 @@ # What `use_as_global_settings` actually does, in plain terms. `{binary}` is filled in per agent. GLOBAL_SETTINGS_BLURB = ( - "Yes writes the gateway config into that file (needs sudo once), so a bare `{binary}` reaches " - "the Databricks gateway on its own — you don't have to launch it through ucode. No writes a " - "ucode-only settings file instead, so `{binary}` uses the gateway only when started with " - "`ucode {binary}`." + "Answer yes to write the gateway config into that file (needs sudo once), so a bare `{binary}` " + "reaches the Databricks gateway on its own — you don't have to launch it through ucode. Answer " + "no to write a ucode-only settings file instead, so `{binary}` uses the gateway only when " + "started with `ucode {binary}`." ) BUDGET_POLICY_BLURB = ( From 040eb1f2516055e8dbd70e90bfb8a69fcc53e9af Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Thu, 13 Aug 2026 23:29:04 +0000 Subject: [PATCH 08/10] test(e2e): skip provider-launch tests when the account is out of credits The provider-launch e2e tests only prove routing reaches the Model Provider Service; a real "Credit balance is too low" from the provider account is an environmental condition, not a ucode bug, so skip it like the existing no-permission case rather than failing CI. Generalize _skip_if_no_permission -> _skip_if_provider_unusable to cover both. Co-authored-by: Isaac --- tests/test_e2e.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index ebd2ab3f..76b5087e 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -573,9 +573,14 @@ def _first_service(tool: str, workspace: str, token: str) -> str: return names[0] @staticmethod - def _skip_if_no_permission(combined: str, provider: str) -> None: + def _skip_if_provider_unusable(combined: str, provider: str) -> None: + # Environmental provider-account conditions, not ucode bugs: the test only proves routing + # reaches the provider, so skip (rather than fail) when the account lacks a grant on the + # connection or has run out of credits — state outside the code under test. if "USE CONNECTION" in combined or "EXECUTE" in combined: pytest.skip(f"no permission on provider {provider}: {combined[:200]}") + if "Credit balance is too low" in combined: + pytest.skip(f"provider {provider} account is out of credits: {combined[:200]}") def test_launch_claude_through_provider( self, tmp_path, monkeypatch, e2e_state, e2e_workspace, e2e_token @@ -613,7 +618,7 @@ def test_launch_claude_through_provider( } result = _run_agent(claude.validate_cmd("claude"), env=env, timeout=90) combined = (result.stdout + result.stderr).strip() - self._skip_if_no_permission(combined, provider) + self._skip_if_provider_unusable(combined, provider) assert result.returncode == 0 and combined, ( f"provider={provider} rc={result.returncode} " f"stdout={result.stdout[:300]!r} stderr={result.stderr[:300]!r}" @@ -650,7 +655,7 @@ def test_launch_codex_through_provider( except subprocess.TimeoutExpired: pytest.fail(f"provider={provider} timed out after {timeout_seconds}s") combined = (result.stdout + result.stderr).strip() - self._skip_if_no_permission(combined, provider) + self._skip_if_provider_unusable(combined, provider) assert result.returncode == 0 and combined, ( f"provider={provider} rc={result.returncode} " f"stdout={result.stdout[:300]!r} stderr={result.stderr[:300]!r}" From 026d62e66722d61c0e54d914cf03790e824dee56 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Fri, 14 Aug 2026 06:13:34 +0000 Subject: [PATCH 09/10] review: drop isaac refs, hoist import, shorten mark_tool_managed docstring --- src/ucode/agents/claude.py | 6 +++--- src/ucode/agents/codex.py | 7 ++++--- src/ucode/cli.py | 2 +- src/ucode/managed_files.py | 19 +++++++++---------- src/ucode/managed_resolve.py | 3 +-- src/ucode/state.py | 16 ++++++---------- 6 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 0aa182df..7378cccf 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -587,9 +587,9 @@ def _write_managed_settings( Runs only under use_as_global_settings. The managed file is root-owned and the highest-precedence scope, so it applies whether or not `ucode` launches `claude`. The same compose (merge overlay + prune stale keys) that produced the private file is applied to the existing managed file, so any - real IT-authored keys already there survive. The write goes through the isaac-style sudo path - (drift-suppressed, so no password prompt when unchanged). Returns the descriptor for revert - tracking, or None when nothing was written. + real IT-authored keys already there survive. The write goes through the sudo path in + `managed_files` (drift-suppressed, so no password prompt when unchanged). Returns the descriptor + for revert tracking, or None when nothing was written. Relayed launches are skipped: they depend on a per-session loopback refresh proxy that only runs during `ucode claude`, so a bare `claude` could not reach the gateway anyway. diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 05f29d6c..e3ac98b4 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -360,7 +360,8 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non # 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 isaac-style sudo path. + # `ucode auth-token`, so the file keeps working. The write goes through the sudo path in + # `managed_files`. managed_descriptors = None if state.get("write_managed_config"): managed_descriptors = _write_managed_config( @@ -394,8 +395,8 @@ def _write_managed_config( ) -> list[dict] | None: """Merge the modern overlay into Codex's OS managed_config.toml, preserving any other keys there. - Written via the isaac-style sudo path (drift-suppressed). Returns the descriptor for revert - tracking, or None when nothing was written. + Written via the sudo path in `managed_files` (drift-suppressed). Returns the descriptor for + revert tracking, or None when nothing was written. """ path = _managed_config_path() if path is None: diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 398f8977..37fc494d 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1691,7 +1691,7 @@ def _launch_tool( # the id can't ride `resolved_model` — it is threaded separately as `custom_model`. if model and tool != "claude": resolved_model = model - # Claude Code's enterprise managed-settings scope (e.g. an Isaac/dbexec install) + # Claude Code's enterprise managed-settings scope (e.g. a dbexec install) # outranks the --settings file ucode writes AND can't be excluded with --setting-sources, # so a model pinned there silently wins over `--model`. Warn so a launch that ignores the # requested model looks like the misconfiguration it is, not a ucode bug. diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 5e882adc..c4c7b59e 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -8,12 +8,11 @@ ``/Library/Application Support/ClaudeCode/managed-settings.json`` (macOS) - Codex: ``/etc/codex/managed_config.toml`` (Linux + macOS) -The write mirrors isaac's approach (``devtools/ai/llm_lib/core/config.py`` in universe): a **drift -check** reads the world-readable file WITHOUT sudo and does nothing when it already matches, so the -common no-op launch never prompts for a password; only a real change shells out to ``sudo`` (temp -file → ``sudo cp``), clearing and restoring the immutable flag (``chattr``/``chflags``) that a fleet -golden image may have set. Writing needs root, so the first write (or one after the config changes) -prompts for the developer's sudo password — the same tradeoff isaac makes. +The write is guarded by a **drift check**: it reads the world-readable file WITHOUT sudo and does +nothing when it already matches, so the common no-op launch never prompts for a password; only a +real change shells out to ``sudo`` (temp file → ``sudo cp``), clearing and restoring the immutable +flag (``chattr``/``chflags``) that a fleet golden image may have set. Writing needs root, so the +first write (or one after the config changes) prompts for the developer's sudo password. """ from __future__ import annotations @@ -28,7 +27,7 @@ from ucode.config_io import is_dry_run from ucode.ui import console, print_err, print_warning -# Absolute path so a stripped PATH (desktop/GUI launchers) still finds it; matches isaac. +# Absolute path so a stripped PATH (desktop/GUI launchers) still finds it. _SUDO = "/usr/bin/sudo" @@ -82,8 +81,8 @@ def _sudo_replace(path: Path, desired_text: str) -> None: """Replace ``path`` with ``desired_text`` via sudo (temp file → ``sudo cp``), handling immutability. Writes the payload to a user-owned temp file first (no sudo), then copies it into place with - ``sudo`` and makes it world-readable — the same sequence isaac uses so the file it lays down is - readable by the agent binary regardless of who launched it. + ``sudo`` and makes it world-readable so the file it lays down is readable by the agent binary + regardless of who launched it. """ subprocess.run([_SUDO, "mkdir", "-p", str(path.parent)], check=True) with tempfile.NamedTemporaryFile( @@ -113,7 +112,7 @@ def _clear_immutable(path: Path) -> bool: macOS: preserve JAMF's system-immutable ``schg`` across the update — inspect, unlock only when set, and report that it must be restored. Linux: best-effort ``chattr -i`` (not every filesystem - supports it), never restored — matching isaac. + supports it), never restored. """ try: # `path.exists()` stats the file; under a root-locked parent dir (e.g. a 750 /etc/codex we diff --git a/src/ucode/managed_resolve.py b/src/ucode/managed_resolve.py index 73aaad00..811b9f1f 100644 --- a/src/ucode/managed_resolve.py +++ b/src/ucode/managed_resolve.py @@ -21,6 +21,7 @@ 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 @@ -186,8 +187,6 @@ def managed_use_as_global_settings(managed: dict, tool: str) -> bool: other agent — a hand-written ``--from-file`` config can't turn it on for an agent that has no managed settings path. """ - from ucode.agents import GLOBAL_SETTINGS_AGENTS - if tool not in GLOBAL_SETTINGS_AGENTS: return False return bool(_agent_entry(managed, tool).get("use_as_global_settings")) diff --git a/src/ucode/state.py b/src/ucode/state.py index 7059d109..2b3d05cd 100644 --- a/src/ucode/state.py +++ b/src/ucode/state.py @@ -248,16 +248,12 @@ def mark_tool_managed( ) -> dict: """Record which config keys ucode manages for ``tool``. - ``native`` optionally describes the agent's own native config file(s) ucode also wrote under - ``use_as_global_settings`` — each ``{"path": str, "format": "json"|"toml", "keys": [...]}`` — - so ``ucode revert`` can surgically prune only ucode's keys from the user's shared file. - - ``native=None`` means "this launch wrote no native file", not "clear the tracking": a later - launch that skips the native write (the admin unset ``use_as_global_settings``, a relayed Claude - launch, or a legacy-layout Codex launch) leaves ucode's keys sitting in the user's shared file, so - the descriptor from the launch that *did* write them must be preserved or ``ucode revert`` can no - longer find and prune them. ``ucode revert`` wipes state wholesale (``clear_state``), so a stale - descriptor never lingers past a revert. + ``native`` optionally describes the native config file(s) ucode also wrote under + ``use_as_global_settings`` — each ``{"path": str, "format": "json"|"toml", "keys": [...]}`` — so + ``ucode revert`` can prune only ucode's keys from the user's shared file. + + ``native=None`` means "this launch wrote no native file", not "clear the tracking": the prior + descriptor is preserved so revert can still find keys an earlier launch wrote. """ managed_configs = dict(state.get("managed_configs") or {}) entry: dict = {"keys": list(managed_keys)} From 909243ec24781a2ea76ee86da1616990e7d0a8f2 Mon Sep 17 00:00:00 2001 From: AarushiShah-db Date: Fri, 14 Aug 2026 06:33:00 +0000 Subject: [PATCH 10/10] review: drop managed-file revert; add platform enum + reconcile codex/windows --- src/ucode/agents/claude.py | 79 ++++---------------------- src/ucode/agents/codex.py | 73 ++++-------------------- src/ucode/cli.py | 10 ---- src/ucode/managed_files.py | 44 ++++++++++----- src/ucode/state.py | 32 ++--------- tests/test_agent_claude.py | 109 +----------------------------------- tests/test_agent_codex.py | 36 +----------- tests/test_managed_files.py | 10 ---- tests/test_state.py | 47 +--------------- 9 files changed, 64 insertions(+), 376 deletions(-) diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 7378cccf..e818cdf9 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -10,7 +10,6 @@ import signal import socket import subprocess -import sys import threading from collections.abc import Callable from pathlib import Path @@ -22,7 +21,6 @@ ToolSpec, backup_existing_file, deep_merge_dict, - prune_key_paths, read_json_safe, write_json_file, ) @@ -32,7 +30,7 @@ get_databricks_token, ) from ucode.launcher import exec_or_spawn -from ucode.managed_files import prune_managed_file, write_managed_file +from ucode.managed_files import OS, current_os, write_managed_file from ucode.smart_routing.claude_hooks import ( remove_smart_routing_hooks, sync_smart_routing_hooks, @@ -138,9 +136,9 @@ def _resolve_web_search_model(state: dict) -> str | None: def _managed_settings_path() -> Path | None: """OS-specific location of Claude Code's enterprise managed-settings.json. Returns None on unsupported platforms.""" - if sys.platform.startswith("linux"): + if current_os() is OS.LINUX: return Path("/etc/claude-code/managed-settings.json") - if sys.platform == "darwin": + if current_os() is OS.MACOS: return Path("/Library/Application Support/ClaudeCode/managed-settings.json") return None @@ -560,9 +558,8 @@ def _compose(base: dict) -> dict: write_json_file(CLAUDE_SETTINGS_PATH, _compose(read_json_safe(CLAUDE_SETTINGS_PATH))) - managed_descriptors = None if state.get("write_managed_config"): - managed_descriptors = _write_managed_settings(_compose, managed_keys, relayed) + _write_managed_settings(_compose, relayed) if web_search_model: _register_web_search_mcp(state["workspace"], web_search_model, state.get("profile")) @@ -574,22 +571,19 @@ def _compose(base: dict) -> dict: else: state.pop("claude_relayed", None) state.pop("relayed_proxy_port", None) - state = mark_tool_managed(state, "claude", managed_keys, native=managed_descriptors) + state = mark_tool_managed(state, "claude", managed_keys) save_state(state) return state -def _write_managed_settings( - compose: Callable[[dict], dict], managed_keys: list[list[str]], relayed: bool -) -> list[dict] | None: +def _write_managed_settings(compose: Callable[[dict], dict], relayed: bool) -> None: """Write ucode's config into Claude Code's OS managed-settings.json so a bare `claude` works. Runs only under use_as_global_settings. The managed file is root-owned and the highest-precedence scope, so it applies whether or not `ucode` launches `claude`. The same compose (merge overlay + prune stale keys) that produced the private file is applied to the existing managed file, so any real IT-authored keys already there survive. The write goes through the sudo path in - `managed_files` (drift-suppressed, so no password prompt when unchanged). Returns the descriptor - for revert tracking, or None when nothing was written. + `managed_files` (drift-suppressed, so no password prompt when unchanged). Relayed launches are skipped: they depend on a per-session loopback refresh proxy that only runs during `ucode claude`, so a bare `claude` could not reach the gateway anyway. @@ -600,69 +594,16 @@ def _write_managed_settings( "reach, so ucode did not write the managed settings file. Launch with `ucode claude` " "to use the relay." ) - return None + return path = _managed_settings_path() if path is None: print_warning( "Machine-wide Claude settings aren't supported on this platform; skipped the managed " "settings write." ) - return None + return desired = json.dumps(compose(read_json_safe(path)), indent=2) - status = write_managed_file(path, desired, display="Claude Code") - if status == "skipped": - return None - return [{"path": str(path), "format": "json", "keys": managed_keys}] - - -def revert_managed_config(state: dict) -> str | None: - """Surgically strip ucode's keys from the managed settings file it wrote under - use_as_global_settings, writing the pruned file back via sudo. - - Returns a short status ("ucode entries removed" / "unchanged") for the revert summary, or None - when ucode never wrote a managed file for claude. - """ - native = ((state.get("managed_configs") or {}).get("claude") or {}).get("native") - if not isinstance(native, list) or not native: - return None - changed = False - for descriptor in native: - path = Path(descriptor.get("path", "")) - keys = descriptor.get("keys") or [] - if not path or not path.exists(): - continue - doc = read_json_safe(path) - # Hook-event keys ([`hooks`, ]) address the user's own shared hook arrays. Pruning the - # whole path would delete every hook they registered under that event, not just ucode's — so - # route those through the same marker-matched removers the write path uses (symmetric with - # `sync_smart_routing_hooks` / `_upsert_tracing_stop_hook` in `write_tool_config`). Only plain, - # ucode-owned key paths go to `prune_key_paths`. - plain_keys: list[list[str]] = [] - touches_routing_hooks = False - touches_tracing_stop_hook = False - for key in keys: - if len(key) == 2 and key[0] == "hooks" and key[1] in CLAUDE_ROUTING_HOOK_EVENTS: - touches_routing_hooks = True - elif len(key) == 2 and key[0] == "hooks" and key[1] == "Stop": - touches_tracing_stop_hook = True - else: - plain_keys.append(key) - file_changed = prune_key_paths(doc, plain_keys) - if touches_routing_hooks and remove_smart_routing_hooks(doc): - file_changed = True - if touches_tracing_stop_hook: - before = json.dumps(doc.get("hooks"), sort_keys=True) - _remove_tracing_stop_hook(doc) - if json.dumps(doc.get("hooks"), sort_keys=True) != before: - file_changed = True - if file_changed: - # Root-owned managed file: write the pruned content back via sudo (drift-suppressed). - if ( - prune_managed_file(path, json.dumps(doc, indent=2), display="Claude Code") - != "skipped" - ): - changed = True - return "ucode entries removed" if changed else "unchanged" + write_managed_file(path, desired, display="Claude Code") def _is_tracing_stop_hook(hook: object) -> bool: diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index e3ac98b4..0e153cb4 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -26,7 +26,7 @@ get_databricks_token, ) from ucode.launcher import exec_or_spawn -from ucode.managed_files import write_managed_file +from ucode.managed_files import OS, current_os, write_managed_file from ucode.smart_routing.codex_hooks import ( remove_smart_routing_hooks, sync_smart_routing_hooks, @@ -362,12 +362,11 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non # 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`. - managed_descriptors = None if state.get("write_managed_config"): - managed_descriptors = _write_managed_config( + _write_managed_config( workspace, chosen_model, databricks_profile, bool(state.get("use_pat")), provider ) - state = mark_tool_managed(state, "codex", MANAGED_KEYS, native=managed_descriptors) + state = mark_tool_managed(state, "codex", MANAGED_KEYS) save_state(state) return state @@ -375,14 +374,13 @@ def write_tool_config(state: dict, model: str | None = None, provider: str | Non def _managed_config_path() -> Path | None: """OS-level Codex managed config file, or None on unsupported platforms. - Linux and macOS both use ``/etc/codex/managed_config.toml`` (root-owned, highest precedence); - Windows uses ``~/.codex/managed_config.toml``. See - https://learn.chatgpt.com/docs/enterprise/managed-configuration. + 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. """ - if sys.platform == "darwin" or sys.platform.startswith("linux"): + if current_os() in (OS.LINUX, OS.MACOS): return Path("/etc/codex/managed_config.toml") - if sys.platform.startswith("win"): - return Path.home() / ".codex" / "managed_config.toml" return None @@ -392,11 +390,10 @@ def _write_managed_config( databricks_profile: str | None, use_pat: bool, provider: str | None, -) -> list[dict] | None: +) -> None: """Merge the modern overlay into Codex's OS managed_config.toml, preserving any other keys there. - Written via the sudo path in `managed_files` (drift-suppressed). Returns the descriptor for - revert tracking, or None when nothing was written. + Written via the sudo path in `managed_files` (drift-suppressed). """ path = _managed_config_path() if path is None: @@ -404,7 +401,7 @@ def _write_managed_config( "Machine-wide Codex settings aren't supported on this platform; skipped the managed " "config write." ) - return None + return overlay = render_overlay( workspace, model, databricks_profile, use_pat=use_pat, provider=provider ) @@ -413,53 +410,7 @@ def _write_managed_config( if provider: # deep_merge can't drop keys; clear a `model` a prior non-provider run pinned. doc.pop("model", None) - status = write_managed_file(path, tomlkit.dumps(doc), display="Codex") - if status == "skipped": - return None - return [{"path": str(path), "format": "toml", "keys": [list(k) for k in MANAGED_KEYS]}] - - -def _strip_modern_ucode_entries(doc: tomlkit.TOMLDocument) -> bool: - """Surgically remove ucode's *modern* keys from an in-memory Codex config document. - - Drops the top-level ``model_provider = "ucode-databricks"`` selector (and the ``model`` pinned - alongside it) and the ``[model_providers.ucode-databricks]`` block, leaving the user's other keys - intact. Mirrors :func:`_strip_legacy_ucode_entries`. Returns True if anything was removed. - """ - changed = False - if doc.get("model_provider") == CODEX_MODEL_PROVIDER_NAME: - doc.pop("model_provider", None) - # ucode pins `model` only alongside its own provider, so remove it when the provider is ours. - doc.pop("model", None) - changed = True - providers = doc.get("model_providers") - if isinstance(providers, dict) and CODEX_MODEL_PROVIDER_NAME in providers: - providers.pop(CODEX_MODEL_PROVIDER_NAME, None) - if not providers: - doc.pop("model_providers", None) - changed = True - return changed - - -def revert_managed_config(state: dict) -> str | None: - """Strip ucode's modern keys from Codex's managed config if it was written under global settings, - writing the pruned file back via sudo. - - Returns a short status for the revert summary, or None when ucode never wrote the managed file. - """ - native = ((state.get("managed_configs") or {}).get("codex") or {}).get("native") - if not isinstance(native, list) or not native: - return None - changed = False - for descriptor in native: - path = Path(descriptor.get("path", "")) - if not path or not path.exists(): - continue - doc = read_toml_safe(path) - if _strip_modern_ucode_entries(doc): - if write_managed_file(path, tomlkit.dumps(doc), display="Codex") != "skipped": - changed = True - return "ucode entries removed" if changed else "unchanged" + write_managed_file(path, tomlkit.dumps(doc), display="Codex") def default_model(state: dict) -> str | None: diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 37fc494d..1f7faeb2 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -1000,13 +1000,6 @@ def revert() -> int: # Older Codex (< 0.134.0) had ucode edit the shared ~/.codex/config.toml in # place; restoring the per-profile file above does not undo that. legacy_codex_stripped = revert_legacy_shared_config() - # OS managed settings files written under use_as_global_settings (the highest-precedence config a - # bare `claude` / `codex` reads): surgically strip ucode's keys via sudo, never touching other - # keys. Runs before clear_state so the tracked descriptors are still available. - managed_reverts = { - "claude": claude_agent.revert_managed_config(state), - "codex": codex_agent.revert_managed_config(state), - } clear_state() print_heading("Revert") @@ -1015,9 +1008,6 @@ def revert() -> int: print_kv(f"{spec['display']} config", "restored" if results[tool] else "unchanged") if legacy_codex_stripped: print_kv("Codex shared config", "ucode entries removed") - for tool, status in managed_reverts.items(): - if status: - print_kv(f"{TOOL_SPECS[tool]['display']} managed config", status) print_kv("Pi settings", "restored" if pi_settings_restored else "unchanged") for client, spec in MCP_CLIENTS.items(): print_kv( diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index c4c7b59e..40e58803 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -22,6 +22,7 @@ import subprocess import sys import tempfile +from enum import Enum from pathlib import Path from ucode.config_io import is_dry_run @@ -31,9 +32,34 @@ _SUDO = "/usr/bin/sudo" +class OS(Enum): + """The host OS families this module distinguishes, off `sys.platform`.""" + + LINUX = "linux" + MACOS = "macos" + WINDOWS = "windows" + OTHER = "other" + + +def current_os() -> OS: + """Map `sys.platform` onto :class:`OS` (lowercased, so a mixed-case value can't slip through).""" + platform = sys.platform.lower() + if platform.startswith("linux"): + return OS.LINUX + if platform == "darwin": + return OS.MACOS + if platform.startswith("win"): + return OS.WINDOWS + return OS.OTHER + + def managed_files_supported() -> bool: - """True on the platforms whose managed-settings write path is implemented (Linux, macOS).""" - return sys.platform == "darwin" or sys.platform.startswith("linux") + """True on the platforms whose managed-settings write path is implemented (Linux, macOS). + + The write path needs `sudo` (`sudo cp`, `chattr`/`chflags`), which is Unix-only — so Windows and + any other platform are unsupported. + """ + return current_os() in (OS.LINUX, OS.MACOS) def _read_existing(path: Path) -> str: @@ -122,7 +148,7 @@ def _clear_immutable(path: Path) -> bool: return False except OSError: return False - if sys.platform == "darwin": + if current_os() is OS.MACOS: result = subprocess.run( ["/usr/bin/stat", "-f", "%Sf", str(path)], capture_output=True, text=True, check=False ) @@ -153,20 +179,10 @@ def _report_sudo_failure(path: Path, display: str, exc: subprocess.CalledProcess cp_failed = len(cmd) >= 2 and cmd[1] == "cp" if cp_failed and "Operation not permitted" in stderr: quoted = shlex.quote(str(path)) - clear_cmd = f"sudo {'chflags noschg' if sys.platform == 'darwin' else 'chattr -i'} {quoted}" + 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 prune_managed_file(path: Path, pruned_text: str, *, display: str) -> str: - """Write back a managed file with ucode's keys removed (used by ``ucode revert``). - - ``pruned_text`` is the file's content with ucode's entries stripped. Goes through the same - drift-suppressed sudo write, so when ucode's keys weren't present the write is a no-op with no - password prompt. - """ - return write_managed_file(path, pruned_text, display=display) diff --git a/src/ucode/state.py b/src/ucode/state.py index 2b3d05cd..a73d9a2c 100644 --- a/src/ucode/state.py +++ b/src/ucode/state.py @@ -124,14 +124,7 @@ def hydrate_state(state: dict) -> dict: for tool, entry in managed_configs.items(): if isinstance(entry, dict): keys = entry.get("keys") if isinstance(entry.get("keys"), list) else [] - norm: dict = {"keys": keys} - # Preserve native-file tracking so `ucode revert` can surgically prune the agent's - # own config file (e.g. ~/.claude/settings.json) it wrote under use_as_global_settings. - # Dropping it here would silently strand ucode's keys in the user's file forever. - native = entry.get("native") - if isinstance(native, list): - norm["native"] = native - normalized[tool] = norm + normalized[tool] = {"keys": keys} elif entry: normalized[tool] = {"keys": []} hydrated["managed_configs"] = normalized @@ -243,27 +236,10 @@ def clear_state() -> None: raise RuntimeError(f"Failed to clear state file: {STATE_PATH}") from exc -def mark_tool_managed( - state: dict, tool: str, managed_keys: list, native: list[dict] | None = None -) -> dict: - """Record which config keys ucode manages for ``tool``. - - ``native`` optionally describes the native config file(s) ucode also wrote under - ``use_as_global_settings`` — each ``{"path": str, "format": "json"|"toml", "keys": [...]}`` — so - ``ucode revert`` can prune only ucode's keys from the user's shared file. - - ``native=None`` means "this launch wrote no native file", not "clear the tracking": the prior - descriptor is preserved so revert can still find keys an earlier launch wrote. - """ +def mark_tool_managed(state: dict, tool: str, managed_keys: list) -> dict: + """Record which config keys ucode manages for ``tool``.""" managed_configs = dict(state.get("managed_configs") or {}) - entry: dict = {"keys": list(managed_keys)} - if native: - entry["native"] = native - else: - prior_native = (managed_configs.get(tool) or {}).get("native") - if isinstance(prior_native, list) and prior_native: - entry["native"] = prior_native - managed_configs[tool] = entry + managed_configs[tool] = {"keys": list(managed_keys)} state["managed_configs"] = managed_configs state["last_tool"] = tool return state diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 37b9575a..b45b6303 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -505,13 +505,10 @@ def test_writes_managed_file_when_flagged(self, monkeypatch): managed_writes: list = [] self._patch(monkeypatch, private_writes, managed_writes) state = {"workspace": WS, "codex_models": [], "write_managed_config": True} - result = claude.write_tool_config(state, "databricks-claude-sonnet-4") + claude.write_tool_config(state, "databricks-claude-sonnet-4") # Private file still written; managed file written too. assert str(claude.CLAUDE_SETTINGS_PATH) in [p for p, _ in private_writes] assert [p for p, _ in managed_writes] == [str(FAKE_MANAGED_PATH)] - native = result["managed_configs"]["claude"]["native"] - assert native[0]["path"] == str(FAKE_MANAGED_PATH) - assert native[0]["format"] == "json" def test_managed_file_preserves_other_keys(self, monkeypatch): private_writes: list = [] @@ -532,9 +529,8 @@ def test_no_managed_write_by_default(self, monkeypatch): managed_writes: list = [] self._patch(monkeypatch, private_writes, managed_writes) state = {"workspace": WS, "codex_models": []} - result = claude.write_tool_config(state, "databricks-claude-sonnet-4") + claude.write_tool_config(state, "databricks-claude-sonnet-4") assert managed_writes == [] - assert "native" not in result["managed_configs"]["claude"] def test_relayed_skips_managed_write(self, monkeypatch): private_writes: list = [] @@ -544,110 +540,11 @@ def test_relayed_skips_managed_write(self, monkeypatch): monkeypatch.setattr(claude, "print_warning", lambda msg: warns.append(msg)) monkeypatch.setattr(claude, "relayed_proxy_base_url", lambda state: "http://127.0.0.1:9999") state = {"workspace": WS, "codex_models": [], "write_managed_config": True} - result = claude.write_tool_config(state, "databricks-claude-sonnet-4", relayed=True) + claude.write_tool_config(state, "databricks-claude-sonnet-4", relayed=True) assert managed_writes == [] - assert result["managed_configs"]["claude"].get("native") is None assert any("bare `claude`" in w for w in warns) -class TestClaudeRevertManagedConfig: - @staticmethod - def _mock_sudo_prune(monkeypatch): - # Route the sudo write straight to disk (the descriptor path is a tmp file) — never real sudo. - def fake_prune(path, text, *, display): - Path(path).write_text(text, encoding="utf-8") - return "written" - - monkeypatch.setattr(claude, "prune_managed_file", fake_prune) - - def test_prunes_only_tracked_keys(self, tmp_path, monkeypatch): - self._mock_sudo_prune(monkeypatch) - managed_path = tmp_path / "managed-settings.json" - managed_path.write_text( - json.dumps({"env": {"ANTHROPIC_BASE_URL": "x", "MY": "keep"}, "apiKeyHelper": "h"}), - encoding="utf-8", - ) - state = { - "managed_configs": { - "claude": { - "keys": [], - "native": [ - { - "path": str(managed_path), - "format": "json", - "keys": [["env", "ANTHROPIC_BASE_URL"], ["apiKeyHelper"]], - } - ], - } - } - } - assert claude.revert_managed_config(state) == "ucode entries removed" - assert json.loads(managed_path.read_text()) == {"env": {"MY": "keep"}} - - def test_returns_none_without_native_tracking(self, monkeypatch): - self._mock_sudo_prune(monkeypatch) - state = {"managed_configs": {"claude": {"keys": []}}} - assert claude.revert_managed_config(state) is None - - def test_preserves_user_hooks_under_managed_events(self, tmp_path, monkeypatch): - # Regression: the descriptor's `keys` include whole hook-event paths (["hooks","PreToolUse"], - # ["hooks","Stop"], ...). Path-pruning those deleted the user's own hooks registered under the - # same events. Revert must surgically strip only ucode's marker-matched hooks, symmetric with - # the write path. - self._mock_sudo_prune(monkeypatch) - user_pre = {"matcher": "Bash", "hooks": [{"type": "command", "command": "my-linter"}]} - ucode_pre = { - "matcher": "Agent|Task", - "hooks": [{"type": "command", "command": "auth claude-router-hook route-subagent"}], - } - user_stop = {"hooks": [{"type": "command", "command": "my-notify"}]} - ucode_stop = {"hooks": [{"type": "command", "command": "mlflow autolog claude stop-hook"}]} - native_path = tmp_path / "managed-settings.json" - native_path.write_text( - json.dumps( - { - "env": {"ANTHROPIC_BASE_URL": "x", "MY": "keep"}, - "hooks": { - "PreToolUse": [user_pre, ucode_pre], - "SessionStart": [ - {"hooks": [{"type": "command", "command": "auth claude-router-hook s"}]} - ], - "Stop": [user_stop, ucode_stop], - }, - } - ), - encoding="utf-8", - ) - state = { - "managed_configs": { - "claude": { - "keys": [], - "native": [ - { - "path": str(native_path), - "format": "json", - "keys": [ - ["env", "ANTHROPIC_BASE_URL"], - ["hooks", "PreToolUse"], - ["hooks", "SessionStart"], - ["hooks", "Stop"], - ], - } - ], - } - } - } - assert claude.revert_managed_config(state) == "ucode entries removed" - result = json.loads(native_path.read_text()) - # ucode's env key gone, the user's kept. - assert result["env"] == {"MY": "keep"} - # The user's own hooks survive; ucode's marker-matched hooks and the now-empty - # SessionStart event are gone. - assert result["hooks"]["PreToolUse"] == [user_pre] - assert result["hooks"]["Stop"] == [user_stop] - assert "SessionStart" not in result["hooks"] - - class TestRegisterWebSearchMcp: def test_clears_existing_then_adds(self, monkeypatch): import ucode.mcp as mcp_mod diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index b972a265..d39afd8e 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -680,15 +680,12 @@ def fake_write_managed(path, text, *, display): def test_writes_managed_config_when_flagged(self, tmp_path, monkeypatch): _, managed_path = self._patch(tmp_path, monkeypatch) state = {"workspace": WS, "codex_models": ["gpt-5"], "write_managed_config": True} - result = codex.write_tool_config(state) + codex.write_tool_config(state) doc = read_toml_safe(managed_path) assert doc["model_provider"] == "ucode-databricks" assert doc["model"] == "gpt-5" assert "ucode-databricks" in doc["model_providers"] - native = result["managed_configs"]["codex"]["native"] - assert native[0]["path"] == str(managed_path) - assert native[0]["format"] == "toml" def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): _, managed_path = self._patch(tmp_path, monkeypatch) @@ -707,34 +704,5 @@ def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): def test_no_managed_write_by_default(self, tmp_path, monkeypatch): _, managed_path = self._patch(tmp_path, monkeypatch) state = {"workspace": WS, "codex_models": ["gpt-5"]} - result = codex.write_tool_config(state) + codex.write_tool_config(state) assert not managed_path.exists() - assert "native" not in result["managed_configs"]["codex"] - - def test_revert_strips_managed_entries(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 = "gpt-5"\nmodel_provider = "ucode-databricks"\napproval_policy = "on-request"\n' - '\n[model_providers.ucode-databricks]\nbase_url = "x"\n', - encoding="utf-8", - ) - state = { - "managed_configs": { - "codex": { - "keys": [], - "native": [{"path": str(managed_path), "format": "toml", "keys": []}], - } - } - } - assert codex.revert_managed_config(state) == "ucode entries removed" - doc = read_toml_safe(managed_path) - assert "model_provider" not in doc - assert "model" not in doc - assert "model_providers" not in doc - # The user's own key is left intact. - assert doc["approval_policy"] == "on-request" - - def test_revert_returns_none_without_native(self): - state = {"managed_configs": {"codex": {"keys": []}}} - assert codex.revert_managed_config(state) is None diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index 9b680a46..dbb14f7b 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -104,13 +104,3 @@ def exists(self): managed_files.subprocess, "run", lambda *a, **k: pytest.fail("should not shell out") ) assert managed_files._clear_immutable(_StatDenied()) is False - - -class TestPruneManagedFile: - def test_prune_is_noop_when_already_absent(self, tmp_path, monkeypatch): - # Revert on a file that never held ucode's keys: pruned text == existing -> no sudo. - path = tmp_path / "managed.json" - path.write_text("pruned", encoding="utf-8") - calls = _capture_sudo(monkeypatch) - assert managed_files.prune_managed_file(path, "pruned", display="X") == "unchanged" - assert calls == [] diff --git a/tests/test_state.py b/tests/test_state.py index d507d408..ff8c3ccd 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -246,20 +246,6 @@ def test_drops_falsy_managed_configs(self): assert "codex" not in result["managed_configs"] assert "claude" not in result["managed_configs"] - def test_preserves_native_tracking(self): - native = [ - {"path": "/home/u/.claude/settings.json", "format": "json", "keys": [["env", "X"]]} - ] - state = {"managed_configs": {"claude": {"keys": [["env", "X"]], "native": native}}} - result = hydrate_state(state) - # Without preserving `native`, `ucode revert` would strand ucode's keys in the user's file. - assert result["managed_configs"]["claude"]["native"] == native - - def test_drops_non_list_native(self): - state = {"managed_configs": {"claude": {"keys": [], "native": "bogus"}}} - result = hydrate_state(state) - assert "native" not in result["managed_configs"]["claude"] - class TestBuildAgentState: def test_returns_empty_without_workspace(self): @@ -304,33 +290,6 @@ def test_preserves_existing_managed_configs(self): assert "gemini" in result["managed_configs"] assert "codex" in result["managed_configs"] - def test_records_native_descriptor(self): - native = [{"path": "/x/config.toml", "format": "toml", "keys": [["model_provider"]]}] - result = mark_tool_managed({}, "codex", [["model"]], native=native) - assert result["managed_configs"]["codex"] == {"keys": [["model"]], "native": native} - - def test_no_native_key_when_none(self): - result = mark_tool_managed({}, "claude", [["env", "X"]]) - assert result["managed_configs"]["claude"] == {"keys": [["env", "X"]]} - assert "native" not in result["managed_configs"]["claude"] - - def test_preserves_prior_native_when_native_is_none(self): - # A re-launch that writes no native file (use_as_global_settings unset, a relayed Claude - # launch, or a legacy-layout Codex launch) must not drop the descriptor from the launch that - # did write ucode's keys — otherwise `ucode revert` can no longer prune them. - native = [{"path": "/x/config.toml", "format": "toml", "keys": [["model_provider"]]}] - state = mark_tool_managed({}, "codex", [["model"]], native=native) - result = mark_tool_managed(state, "codex", [["model"]], native=None) - assert result["managed_configs"]["codex"]["native"] == native - - def test_native_none_without_prior_leaves_no_native(self): - state = mark_tool_managed({}, "claude", [["env", "X"]]) - result = mark_tool_managed(state, "claude", [["env", "Y"]], native=None) - assert "native" not in result["managed_configs"]["claude"] - - def test_new_native_replaces_prior(self): - first = [{"path": "/a", "format": "json", "keys": [["a"]]}] - second = [{"path": "/b", "format": "json", "keys": [["b"]]}] - state = mark_tool_managed({}, "claude", [["env", "X"]], native=first) - result = mark_tool_managed(state, "claude", [["env", "X"]], native=second) - assert result["managed_configs"]["claude"]["native"] == second + def test_records_only_keys(self): + result = mark_tool_managed({}, "codex", [["model"]]) + assert result["managed_configs"]["codex"] == {"keys": [["model"]]}