From 55a69b5ad007bcf1a46fe13f43d43c4fe0be3423 Mon Sep 17 00:00:00 2001 From: Anthony Ivan Date: Tue, 1 Sep 2026 23:41:08 +0800 Subject: [PATCH] Make managed settings sync explicit Signed-off-by: Anthony Ivan --- README.md | 31 +++-- docs/os-managed-settings-design.md | 99 +++++++------- src/ucode/agents/__init__.py | 60 ++++++-- src/ucode/agents/claude.py | 136 +++++++++++++----- src/ucode/agents/codex.py | 116 ++++++++++++---- src/ucode/cli.py | 101 +++++++++++++- src/ucode/managed_files.py | 31 ++++- tests/test_agent_claude.py | 145 +++++++++++++++++--- tests/test_agent_codex.py | 151 +++++++++++++++----- tests/test_agents_init.py | 25 +++- tests/test_cli.py | 213 +++++++++++++++++++++++++++-- tests/test_managed_files.py | 52 ++++++- tests/test_managed_resolve.py | 12 +- 13 files changed, 953 insertions(+), 219 deletions(-) diff --git a/README.md b/README.md index 661ab410..6cd4cdb1 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,19 @@ To configure specific tools without the picker, pass a comma-separated list: ucode configure --agents claude,codex ``` +Claude Code and Codex are configured locally by default. Their launches read any higher-precedence +OS-managed settings and block only when they conflict; launches never modify them or request +administrator access. To intentionally configure selected supported agents machine-wide, run: + +```bash +ucode configure --sync-managed-settings +``` + +The normal agent picker still runs, and synchronization applies only to the selected Claude Code +and Codex agents. This can affect agents launched outside ucode, including editor extensions. A +later plain configure or `ucode revert` restores only the managed changes previously made by ucode, +preserving administrator changes. + Available agent names are `codex`, `claude`, `gemini`, `opencode`, `copilot`, and `pi`. `cursor` is also accepted (MCP-only — it registers Databricks MCP servers but configures no models). Naming agents explicitly is treated as a request for all of them: if any one isn't available on the workspace, the run fails without configuring the others. Add `--skip-unavailable` to configure the available subset instead and skip the rest with a warning: @@ -239,11 +252,11 @@ ucode publish # publish it to the workspace `ucode setup` walks through the agents to enable and which one bare `ucode` launches, then per agent: Databricks-hosted models or an external Model Provider Service and the models to expose. Interactive -Claude Code and Codex configuration installs gateway-critical values in the OS-managed settings -scope so enterprise settings cannot silently override ucode. Non-interactive and CI runs use local -files without invoking `sudo`, and stop with an actionable error if an existing managed value -conflicts. Claude subscription relay is local-only because its loopback proxy exists only for that -session. +Claude Code and Codex configuration uses local ucode files unless the user explicitly passes +`--sync-managed-settings`. Existing managed values are always checked because they take precedence; +conflicting values stop configuration or launch without being overwritten. Configure and launch +never invoke `sudo` without the explicit flag, and launches are always read-only. Claude subscription +relay is local-only because its loopback proxy exists only for that session. Claude Code is asked one model per family (opus/sonnet/haiku/fable), since it selects models by family alias; any family can be skipped. @@ -337,6 +350,7 @@ The output looks like: | `ucode revert` | Clear saved state and restore backed-up config files | | `ucode configure --dry-run` | Preview config files without writing them | | `ucode configure --agents claude,codex` | Configure specific agents without the interactive picker | +| `ucode configure --sync-managed-settings` | Explicitly synchronize selected Claude Code and Codex OS-managed settings; may require administrator access and affect agents outside ucode | | `ucode configure --workspaces https://first.databricks.com,https://second.databricks.com` | Configure workspaces without the interactive picker | | `ucode configure --profiles DEFAULT` | Configure using existing Databricks CLI profiles (hosts come from `~/.databrickscfg`) | | `ucode configure --profiles DEFAULT --use-pat` | Authenticate with the profile's personal access token — no browser login | @@ -379,8 +393,8 @@ control the installation. |------|------| | `~/.codex/ucode.config.toml` (or legacy `~/.codex/config.toml`) | Codex | | `~/.claude/ucode-settings.json` | Claude Code settings generated by ucode | -| `/etc/claude-code/managed-settings.json` (Linux) or `/Library/Application Support/ClaudeCode/managed-settings.json` (macOS) | Claude Code OS-managed settings | -| `/etc/codex/managed_config.toml` | Codex OS-managed settings | +| `/etc/claude-code/managed-settings.json` (Linux) or `/Library/Application Support/ClaudeCode/managed-settings.json` (macOS) | Optional Claude Code OS-managed settings, changed only by explicit synchronization or restoration | +| `/etc/codex/managed_config.toml` | Optional Codex OS-managed settings, changed only by explicit synchronization or restoration | | `~/.gemini/.env` | Gemini CLI | | `~/.config/opencode/opencode.json` | OpenCode | | `~/.copilot/.env` | GitHub Copilot CLI | @@ -389,7 +403,8 @@ control the installation. | `~/.ucode/managed-state.json` | The managed config — authored by `ucode setup` (admins) and refreshed from the workspace on launch | | `~/.ucode/managed-backups/` | Baseline backups for OS-managed files changed by ucode | -Existing files are backed up before being overwritten. `ucode revert` restores backups. +Files changed by ucode are backed up first. `ucode revert` restores only ucode-owned changes and +preserves later administrator edits. ## Documentation diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index 4f75e6bc..daf2ca9e 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -12,9 +12,10 @@ The two stacked PRs make precedence handling deterministic: 2. The Codex PR reuses that lifecycle for TOML, applies it to Codex, and removes the old optional managed-settings path. -After both PRs merge, interactive configuration reconciles the agent's OS-managed file by default. -Non-interactive and CI execution never elevates privileges and instead uses local settings when the -managed file is compatible. +Claude and Codex configuration is local-only by default. OS-managed synchronization requires the +explicit `--sync-managed-settings` flag and applies to the supported agents selected by the normal +configure flow. Launches never write managed files or elevate privileges; they only verify that +existing higher-precedence values are compatible. ## Configuration Files @@ -23,37 +24,38 @@ managed file is compatible. | Claude Code | `~/.claude/ucode-settings.json` | Linux: `/etc/claude-code/managed-settings.json`; macOS: `/Library/Application Support/ClaudeCode/managed-settings.json` | | Codex | `~/.codex/ucode.config.toml` | `/etc/codex/managed_config.toml` | -The local file is always written. The OS-managed file is additionally reconciled during interactive -configuration, except for Claude subscription relay. +The local file is always written. OS-managed files are changed only by explicit synchronization or +restoration, except for Claude subscription relay, which is always local-only. -## Interactive Detection +## Explicit Synchronization -An invocation may modify OS-managed settings only when standard input is a TTY. Standard output does -not affect the decision, so piping logs does not disable an otherwise interactive configuration. -CI, pipes, cron jobs, and headless subprocesses normally have non-TTY standard input and therefore -remain local-only. +`ucode configure` uses local settings. `--sync-managed-settings` is the user's explicit consent to +configure selected Claude Code and Codex agents machine-wide. The normal picker still runs when no +agents are named, and the command fails before agent installation or configuration when none of the +final selections support synchronization. Because a write may require administrator approval, an +actual change still requires standard input to be a TTY. An already-identical or compatible external +file needs no elevation. -This is a new shared ucode distinction. The previous implementation inferred interactivity from -command shape in some flows and did not guard managed-file writes consistently. +TTY state is only a privilege boundary; it does not select configuration scope and never changes +whether a cached launch is valid. ## Behavior Matrix | Invocation | Managed file | Behavior | | --- | --- | --- | -| Interactive | Absent | Create it from the ucode configuration after recording an absent baseline. | -| Interactive | Unrelated or partially populated | Preserve unrelated values and add or update all ucode-owned values. | -| Interactive | Conflicting | Back up the baseline, replace the conflicting ucode-owned values, and verify. | -| Interactive | Already identical | Continue without a backup, write, or `sudo` invocation. | -| Non-interactive | Absent | Use the local ucode file. Do not create the managed file. | -| Non-interactive | Ucode-owned values absent or equal | Use the local ucode file. Do not modify the managed file. | -| Non-interactive | Ucode-owned value conflicts | Stop before launching because the higher-precedence value would override ucode. | +| Plain configure or launch | Absent | Use the local ucode file. Do not create the managed file. | +| Plain configure or launch | Compatible | Use the local ucode file. Do not modify the managed file. | +| Plain configure or launch | Conflicting | Stop because the higher-precedence value would override ucode. | +| Explicit sync | Absent | Create it after recording an absent baseline. | +| Explicit sync | Last version written by ucode | Update and verify it. | +| Explicit sync | Compatible external file | Leave it untouched. | +| Explicit sync | Conflicting or externally modified file | Refuse to overwrite it. | | Any | Invalid, unreadable, or symlinked | Stop without modifying the file because precedence cannot be established safely. | -`ucode configure`, first-time `ucode claude` or `ucode codex`, and later launches all use the same -agent-specific reconciliation path. A first-time launch from an interactive terminal can therefore -request administrator permission. A first-time non-interactive launch remains local-only. +First-time and later `ucode claude` and `ucode codex` launches use only the read-only compatibility +path. They never request administrator permission. -## Interactive Reconciliation +## Explicit Reconciliation For each agent, ucode: @@ -66,9 +68,9 @@ For each agent, ucode: 7. Reads the installed file back and verifies its exact contents. 8. Records the last-applied snapshot, owned paths, and a launch fingerprint. -An existing managed file is reconciled even when it does not currently conflict. This ensures every -ucode-required value exists at the highest-precedence scope and avoids separate behavior for absent, -partial, and conflicting files. +An existing file is updated only when it exactly matches ucode's last-applied snapshot. A compatible +external file remains untouched, and a conflicting or externally modified file is preserved and +reported. ## Privileged Write Transaction @@ -124,9 +126,11 @@ If ucode cannot complete a write or revert, the backup remains available for a l - Preserve externally changed values rather than replacing them with stale baseline values. - Refuse an unsafe or unparsable merge and retain the backup. -A revert that needs to change an OS-managed file must run interactively. A successful revert removes -that agent's backup record. The existing local configuration and ucode state cleanup still occur as -part of the command. +A plain `ucode configure --agent claude` performs the same restoration before writing new local +settings, making it the opt-out path after explicit synchronization. A restoration that needs to +change an OS-managed file must run interactively. If restoration fails, local configuration can +continue, the backup remains available, and launches do not retry elevation. A successful +restoration removes that agent's backup record. ## Cached Launches @@ -138,10 +142,10 @@ After a successful managed reconciliation or compatibility check, ucode stores: - size; - nanosecond modification and change times. -A cached launch performs one `stat()` call and compares this fingerprint. When it matches, ucode -does not read, parse, back up, write, or invoke `sudo`. When it changes, the normal reconciliation or -compatibility path runs again. This catches later MDM replacement without adding meaningful latency -to unchanged launches. +A missing managed file is valid without a fingerprint. For an existing file, a cached launch +compares its fingerprint. When it matches, ucode does not parse the file. When it changes, ucode +runs the read-only compatibility path again. Launches never back up, write, restore, or invoke +`sudo`. The verification scopes distinguish: @@ -149,8 +153,8 @@ The verification scopes distinguish: - a managed file verified as compatible with local settings; - a managed file verified as compatible with Claude relay. -A compatibility fingerprint created non-interactively cannot suppress the next interactive -reconciliation. +Compatibility fingerprints are valid independently of TTY state. Only the explicit synchronization +flag selects the write path. ## Claude Subscription Relay @@ -178,15 +182,14 @@ standard Databricks authentication. invalid, unreadable, missing, or unsupported; - whether a managed baseline backup is available. -Interactive updates announce the backup location, administrator-permission request, and verified -result. An identical file produces no elevation message. +Explicit updates announce the backup location, administrator-permission request, and verified +result. An identical or compatible external file produces no elevation message. Representative blockers are: ```text -Claude Code configuration cannot be applied non-interactively because OS-managed settings at - override ucode values: env.ANTHROPIC_BASE_URL. Run `ucode configure --agent claude` from an -interactive terminal or contact your administrator. +Claude Code OS-managed settings at override ucode values: env.ANTHROPIC_BASE_URL. ucode did +not modify the file during this launch. Contact your administrator. ``` ```text @@ -199,9 +202,9 @@ Cannot safely update Codex managed settings at : . ucode did Repair it or contact your administrator. ``` -Write, verification, parse, symlink, and managed-conflict failures block the agent launch. Recovery -information always identifies the file and recommends either an interactive configure/revert or -administrator help. +Parse, symlink, unreadable-file, and managed-conflict failures block launch. Write and restoration +failures occur only during explicit configure or revert operations. Recovery information identifies +the file and recommends configure, revert, or administrator help. ## PR Boundaries @@ -209,8 +212,9 @@ administrator help. - Add shared strict reads, fingerprints, compatibility checks, secure backups, atomic privileged writes, immutable-flag handling, verification, status, and three-way revert helpers. -- Make interactive Claude configuration reconcile OS-managed JSON by default. -- Add non-interactive local fallback with conflict detection. +- Make Claude configuration local-only by default and add explicit managed synchronization. +- Make every launch read-only with conflict detection. +- Restore prior ucode-managed changes during plain configure and revert. - Add Claude relay-specific safety checks. - Add Claude managed status and revert output. - Remove Claude's old managed-settings scope choice. @@ -218,8 +222,9 @@ administrator help. ### PR 2: Codex - Stack on the Claude PR and reuse the shared lifecycle with strict TOML parsing and serialization. -- Make interactive Codex configuration reconcile OS-managed TOML by default. -- Add non-interactive local fallback with conflict detection. +- Make Codex configuration local-only by default and add explicit managed synchronization. +- Make every launch read-only with conflict detection. +- Restore prior ucode-managed changes during plain configure and revert. - Add Codex fingerprinted cached launches, status, and revert behavior. - Keep opt-in smart-routing hooks in the local Codex config rather than adding them by default to machine-managed policy. diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index 578aa208..fc01edc4 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -350,10 +350,16 @@ def configure_tool( relayed: bool = False, route_root_model: str | None = None, custom_model: str | None = None, + sync_managed_settings: bool = False, ) -> dict: result: dict | tuple[dict, str] if tool == "codex": - result = codex.write_tool_config(state, model, provider=provider) + result = codex.write_tool_config( + state, + model, + provider=provider, + sync_managed_settings=sync_managed_settings, + ) elif tool == "claude": # A Model Provider Service routes by header and pins no Databricks # model, so the usual "model required" guard doesn't apply to claude. @@ -368,6 +374,7 @@ def configure_tool( relayed=relayed, route_root_model=route_root_model, custom_model=custom_model, + sync_managed_settings=sync_managed_settings, ) else: # provider routing is claude/codex-only; every other tool needs a model. @@ -433,7 +440,7 @@ def _availability_failure_detail(tool: str, state: dict) -> str: return " (" + "; ".join(parts) + ")" -def configure_single_tool(tool: str, state: dict) -> dict: +def configure_single_tool(tool: str, state: dict, *, sync_managed_settings: bool = False) -> dict: """Check availability, configure, and persist state for one tool only.""" provider = get_provider_service(state, tool) # A Model Provider Service routes through the same gateway and pins no @@ -446,31 +453,49 @@ def configure_single_tool(tool: str, state: dict) -> dict: raise RuntimeError( f"{TOOL_SPECS[tool]['display']} is not available on this workspace.{detail}" ) - with managed_write_batch(_managed_settings_displays([tool])): - state = _configure_one(tool, state, provider) + with managed_write_batch( + _managed_settings_displays([tool], sync_managed_settings=sync_managed_settings) + ): + state = _configure_one(tool, state, provider, sync_managed_settings=sync_managed_settings) available_tools = list(set((state.get("available_tools") or []) + [tool])) state["available_tools"] = available_tools save_state(state) return state -def _configure_one(tool: str, state: dict, provider: str | None) -> dict: +def _configure_one( + tool: str, + state: dict, + provider: str | None, + *, + sync_managed_settings: bool = False, +) -> dict: """Write one tool's config, routing through ``provider`` when set.""" if provider: provider_models, error, relayed = resolve_provider_models(tool, state, provider) if error: raise RuntimeError(error) return configure_tool( - tool, state, None, provider=provider, provider_models=provider_models, relayed=relayed + tool, + state, + None, + provider=provider, + provider_models=provider_models, + relayed=relayed, + sync_managed_settings=sync_managed_settings, ) if tool == "codex": - return configure_tool("codex", state) + return configure_tool("codex", state, sync_managed_settings=sync_managed_settings) state, model = resolve_launch_model(tool, state, None) - return configure_tool(tool, state, model) + return configure_tool(tool, state, model, sync_managed_settings=sync_managed_settings) def configure_selected_tools( - state: dict, tools: list[str], *, install_ai_tools: bool = True + state: dict, + tools: list[str], + *, + install_ai_tools: bool = True, + sync_managed_settings: bool = False, ) -> dict: """Configure the given tools. Caller is responsible for ensuring each tool is available on the workspace. @@ -479,9 +504,16 @@ def configure_selected_tools( replacing it, so a previously-configured tool the user didn't pick this run is preserved. """ - with managed_write_batch(_managed_settings_displays(tools)): + with managed_write_batch( + _managed_settings_displays(tools, sync_managed_settings=sync_managed_settings) + ): for tool in tools: - state = _configure_one(tool, state, get_provider_service(state, tool)) + state = _configure_one( + tool, + state, + get_provider_service(state, tool), + sync_managed_settings=sync_managed_settings, + ) existing = state.get("available_tools") or [] state["available_tools"] = sorted(set(existing) | set(tools)) @@ -491,7 +523,11 @@ def configure_selected_tools( return state -def _managed_settings_displays(tools: list[str]) -> list[str]: +def _managed_settings_displays( + tools: list[str], *, sync_managed_settings: bool = False +) -> list[str]: + if not sync_managed_settings: + return [] return [TOOL_SPECS[tool]["display"] for tool in tools if tool in _MANAGED_SETTINGS_TOOLS] diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 602c2538..0f5455c0 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -37,10 +37,12 @@ from ucode.managed_files import ( OS, current_os, + managed_file_backup_available, managed_file_conflicts, + managed_file_fingerprint, managed_file_is_verified, + managed_file_matches_last_applied, managed_file_status, - managed_writes_allowed, mark_managed_file_verified, read_managed_file, reconcile_managed_file, @@ -245,12 +247,14 @@ def managed_settings_are_current(state: dict) -> bool: path = _managed_settings_path() if path is None: return True - if state.get("claude_relayed"): - required_scope = "relay-compatible" - elif managed_writes_allowed(): - required_scope = "managed" - else: - required_scope = None + if path.is_symlink(): + return False + try: + if not managed_file_fingerprint(path).get("exists"): + return True + except RuntimeError: + return False + required_scope = "relay-compatible" if state.get("claude_relayed") else None return managed_file_is_verified(state, "claude", path, required_scope=required_scope) @@ -269,6 +273,10 @@ def revert_managed_settings() -> str: ) +def managed_settings_backup_available() -> bool: + return managed_file_backup_available("claude") + + def _managed_relayed_conflicts(path: Path) -> list[str]: """Return managed settings that would override Claude subscription relay auth.""" text = read_managed_file(path) @@ -580,6 +588,7 @@ def write_tool_config( relayed: bool = False, route_root_model: str | None = None, custom_model: str | None = None, + sync_managed_settings: bool = False, ) -> dict: backup_existing_file(CLAUDE_SETTINGS_PATH, CLAUDE_BACKUP_PATH) web_search_model = _resolve_web_search_model(state) @@ -662,7 +671,10 @@ def _compose(base: dict) -> dict: write_json_file(CLAUDE_SETTINGS_PATH, _compose(read_json_safe(CLAUDE_SETTINGS_PATH))) - _reconcile_managed_settings(state, _compose, managed_file_keys, relayed) + if sync_managed_settings: + _sync_managed_settings(state, _compose, managed_file_keys, relayed) + else: + _validate_managed_settings(state, _compose, managed_file_keys, relayed) if web_search_model: web_search_entry = _web_search_mcp_entry( @@ -690,27 +702,32 @@ def _compose(base: dict) -> dict: return state -def _reconcile_managed_settings( +def _managed_settings_documents( + path: Path, compose: Callable[[dict], dict] +) -> tuple[str | None, dict, dict]: + current_text = read_managed_file(path) + try: + existing = _parse_managed_settings(current_text) if current_text is not None else {} + except RuntimeError as exc: + raise RuntimeError( + f"Cannot safely inspect Claude Code managed settings at {path}: {exc}. " + "ucode did not modify the file. Repair it or contact your administrator." + ) from exc + managed_before = copy.deepcopy(existing) + desired_settings = compose(existing) + _preserve_permission_denies(managed_before, desired_settings) + return current_text, managed_before, desired_settings + + +def _validate_managed_settings( state: dict, compose: Callable[[dict], dict], owned_paths: list[list[str]], relayed: bool, ) -> None: - """Reconcile Claude Code's OS-managed settings so a bare ``claude`` uses the gateway. - - The managed file is root-owned and the highest-precedence scope, so every normal Claude - configuration mirrors ucode's settings there. The same compose operation that produced the - private file is applied to the existing managed file, preserving unrelated IT-authored keys. - - 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. - """ + """Read-only check that managed settings do not override ucode's local settings.""" path = _managed_settings_path() if path is None: - print_warning( - "Machine-wide Claude settings aren't supported on this platform; skipped the managed " - "settings." - ) return if path.is_symlink(): raise RuntimeError( @@ -729,31 +746,74 @@ def _reconcile_managed_settings( mark_managed_file_verified(state, "claude", path, scope="relay-compatible") return - current_text = read_managed_file(path) - try: - existing = _parse_managed_settings(current_text) if current_text is not None else {} - except RuntimeError as exc: + _, managed_before, desired_settings = _managed_settings_documents(path, compose) + conflicts = managed_file_conflicts(managed_before, desired_settings, owned_paths) + if conflicts: raise RuntimeError( - f"Cannot safely update Claude Code managed settings at {path}: {exc}. " - "ucode did not modify the file. Repair it or contact your administrator." - ) from exc - managed_before = copy.deepcopy(existing) - desired_settings = compose(existing) - _preserve_permission_denies(managed_before, desired_settings) - if not managed_writes_allowed(): + f"Claude Code OS-managed settings at {path} override ucode values: " + f"{', '.join(conflicts)}. ucode did not modify the file during this launch. If ucode " + "previously synchronized it, run `ucode configure --agent claude` or `ucode revert` " + "from an interactive terminal. Otherwise, contact your administrator." + ) + mark_managed_file_verified(state, "claude", path, scope="local-compatible") + + +def _sync_managed_settings( + state: dict, + compose: Callable[[dict], dict], + owned_paths: list[list[str]], + relayed: bool, +) -> None: + """Explicitly synchronize Claude's OS-managed settings when safe.""" + path = _managed_settings_path() + if path is None: + print_warning( + "Machine-wide Claude settings aren't supported on this platform; skipped the managed " + "settings." + ) + return + if path.is_symlink(): + raise RuntimeError( + f"Refusing to use Claude Code managed settings through symlink {path}. Replace it " + "with a regular file or contact your administrator." + ) + if relayed: + _validate_managed_settings(state, compose, owned_paths, relayed=True) + return + + print_note(f"Synchronizing Claude Code OS-managed settings at {path}.") + current_text, managed_before, desired_settings = _managed_settings_documents(path, compose) + desired_text = _dump_managed_settings(desired_settings) + backup_available = managed_file_backup_available("claude") + ucode_owned = managed_file_matches_last_applied("claude", path, current_text) + if current_text == desired_text: + mark_managed_file_verified( + state, "claude", path, scope="managed" if ucode_owned else "local-compatible" + ) + return + if current_text is not None and backup_available and not ucode_owned: + raise RuntimeError( + f"Claude Code OS-managed settings at {path} changed after ucode last synchronized " + "them. ucode preserved the newer file. Run `ucode revert` to remove only ucode-owned " + "entries, or contact your administrator." + ) + if current_text is not None and not ucode_owned: conflicts = managed_file_conflicts(managed_before, desired_settings, owned_paths) if conflicts: raise RuntimeError( - "Claude Code configuration cannot be applied non-interactively because " - f"OS-managed settings at {path} override ucode values: {', '.join(conflicts)}. " - "Run `ucode configure --agent claude` from an interactive terminal or contact " - "your administrator." + f"Claude Code OS-managed settings at {path} are externally managed and conflict " + f"with ucode values: {', '.join(conflicts)}. ucode did not modify the file; " + "contact your administrator." ) + print_note( + f"Claude Code OS-managed settings at {path} are externally managed and compatible; " + "left unchanged." + ) mark_managed_file_verified(state, "claude", path, scope="local-compatible") return reconcile_managed_file( path, - _dump_managed_settings(desired_settings), + desired_text, tool="claude", display="Claude Code", owned_paths=owned_paths, diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index b53ca113..712554e2 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -31,10 +31,12 @@ from ucode.managed_files import ( OS, current_os, + managed_file_backup_available, managed_file_conflicts, + managed_file_fingerprint, managed_file_is_verified, + managed_file_matches_last_applied, managed_file_status, - managed_writes_allowed, mark_managed_file_verified, read_managed_file, reconcile_managed_file, @@ -49,7 +51,7 @@ from ucode.smart_routing.codex_routing import codex_model_id from ucode.state import mark_tool_managed, save_state from ucode.telemetry import agent_version, ucode_version -from ucode.ui import print_warning_err +from ucode.ui import print_note, print_warning_err CODEX_CONFIG_DIR = Path.home() / ".codex" CODEX_PROFILE_NAME = "ucode" @@ -298,7 +300,12 @@ def revert_legacy_shared_config() -> bool: return _strip_legacy_ucode_entries(_legacy_config_path()) -def write_tool_config(state: dict, model: str | None = None, provider: str | None = None) -> dict: +def write_tool_config( + state: dict, + model: str | None = None, + provider: str | None = None, + sync_managed_settings: bool = False, +) -> dict: workspace = state["workspace"] # Leave model selection to Codex. The gateway still receives the configured # provider and authentication settings, while Codex uses its own default. @@ -362,7 +369,10 @@ def compose(base: dict) -> dict: enabled=False, ) write_toml_file(CODEX_CONFIG_PATH, doc) - _reconcile_managed_config(state, compose) + if sync_managed_settings: + _sync_managed_config(state, compose) + else: + _validate_managed_config(state, compose) state = mark_tool_managed(state, "codex", MANAGED_KEYS) save_state(state) return state @@ -394,8 +404,14 @@ def managed_config_is_current(state: dict) -> bool: path = _managed_config_path() if path is None: return True - required_scope = "managed" if managed_writes_allowed() else None - return managed_file_is_verified(state, "codex", path, required_scope=required_scope) + if path.is_symlink(): + return False + try: + if not managed_file_fingerprint(path).get("exists"): + return True + except RuntimeError: + return False + return managed_file_is_verified(state, "codex", path) def managed_config_status(state: dict) -> tuple[Path | None, str, str]: @@ -413,8 +429,50 @@ def revert_managed_config() -> str: ) -def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> None: - """Reconcile Codex's highest-precedence config while preserving unrelated policy.""" +def managed_config_backup_available() -> bool: + return managed_file_backup_available("codex") + + +def _managed_config_documents( + path: Path, compose: Callable[[dict], dict] +) -> tuple[str | None, dict, dict]: + current_text = read_managed_file(path) + try: + existing = _parse_managed_config(current_text) if current_text is not None else {} + except RuntimeError as exc: + raise RuntimeError( + f"Cannot safely inspect Codex managed settings at {path}: {exc}. ucode did not " + "modify the file. Repair it or contact your administrator." + ) from exc + managed_before = copy.deepcopy(existing) + desired_doc = compose(existing) + return current_text, managed_before, desired_doc + + +def _validate_managed_config(state: dict, compose: Callable[[dict], dict]) -> None: + """Read-only check that managed settings do not override ucode's local profile.""" + path = _managed_config_path() + if path is None: + return + if path.is_symlink(): + raise RuntimeError( + f"Refusing to use Codex managed settings through symlink {path}. Replace it with a " + "regular file or contact your administrator." + ) + _, managed_before, desired_doc = _managed_config_documents(path, compose) + conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) + if conflicts: + raise RuntimeError( + f"Codex OS-managed settings at {path} override ucode values: " + f"{', '.join(conflicts)}. ucode did not modify the file during this launch. If ucode " + "previously synchronized it, run `ucode configure --agent codex` or `ucode revert` " + "from an interactive terminal. Otherwise, contact your administrator." + ) + mark_managed_file_verified(state, "codex", path, scope="local-compatible") + + +def _sync_managed_config(state: dict, compose: Callable[[dict], dict]) -> None: + """Explicitly synchronize Codex's OS-managed config when safe.""" path = _managed_config_path() if path is None: print_warning_err( @@ -427,35 +485,45 @@ def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> N f"Refusing to use Codex managed settings through symlink {path}. Replace it with a " "regular file or contact your administrator." ) - current_text = read_managed_file(path) - try: - existing = _parse_managed_config(current_text) if current_text is not None else {} - except RuntimeError as exc: + + print_note(f"Synchronizing Codex OS-managed settings at {path}.") + current_text, managed_before, desired_doc = _managed_config_documents(path, compose) + desired_text = tomlkit.dumps(desired_doc) + backup_available = managed_file_backup_available("codex") + ucode_owned = managed_file_matches_last_applied("codex", path, current_text) + if current_text == desired_text: + mark_managed_file_verified( + state, "codex", path, scope="managed" if ucode_owned else "local-compatible" + ) + return + if current_text is not None and backup_available and not ucode_owned: raise RuntimeError( - f"Cannot safely update Codex managed settings at {path}: {exc}. ucode did not modify " - "the file. Repair it or contact your administrator." - ) from exc - managed_before = copy.deepcopy(existing) - desired_doc = compose(existing) - if not managed_writes_allowed(): + f"Codex OS-managed settings at {path} changed after ucode last synchronized them. " + "ucode preserved the newer file. Run `ucode revert` to remove only ucode-owned " + "entries, or contact your administrator." + ) + if current_text is not None and not ucode_owned: conflicts = managed_file_conflicts(managed_before, desired_doc, MANAGED_KEYS) if conflicts: raise RuntimeError( - "Codex configuration cannot be applied non-interactively because OS-managed " - f"settings at {path} override ucode values: {', '.join(conflicts)}. Run `ucode " - "configure --agent codex` from an interactive terminal or contact your " - "administrator." + f"Codex OS-managed settings at {path} are externally managed and conflict with " + f"ucode values: {', '.join(conflicts)}. ucode did not modify the file; contact " + "your administrator." ) + print_note( + f"Codex OS-managed settings at {path} are externally managed and compatible; left " + "unchanged." + ) mark_managed_file_verified(state, "codex", path, scope="local-compatible") return reconcile_managed_file( path, - tomlkit.dumps(desired_doc), + desired_text, tool="codex", display="Codex", owned_paths=MANAGED_KEYS, ) - mark_managed_file_verified(state, "codex", path) + mark_managed_file_verified(state, "codex", path, scope="managed") def default_model(state: dict) -> str | None: diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 9281e59b..d1260e24 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -421,6 +421,21 @@ def _parse_agents_option(agents: str) -> list[str]: return tools +_MANAGED_SETTINGS_SYNC_TOOLS = {"claude", "codex"} + + +def _validate_managed_settings_sync_selection(tools: list[str]) -> None: + if _MANAGED_SETTINGS_SYNC_TOOLS.intersection(tools): + return + supported = ", ".join( + spec["display"] for tool, spec in TOOL_SPECS.items() if tool in _MANAGED_SETTINGS_SYNC_TOOLS + ) + raise RuntimeError( + "Managed settings synchronization was requested, but none of the selected agents " + f"support it. Supported agents: {supported}." + ) + + def _parse_skill_locations(location: str | None) -> list[str]: """Parse a comma-separated `--location` into `.` refs, dropping duplicates while preserving order. `None`/empty yields `[]` (the @@ -830,6 +845,40 @@ def _use_databricks() -> dict: return state +def _restore_managed_settings_for_local_config(tool: str) -> None: + if is_dry_run(): + return + if tool == "claude": + display = "Claude Code" + revert = claude_agent.revert_managed_settings + elif tool == "codex": + display = "Codex" + revert = codex_agent.revert_managed_config + else: + return + try: + result = revert() + except RuntimeError as exc: + print_warning( + f"{display} OS-managed settings cleanup is incomplete. Local settings will still be " + f"configured. {exc}" + ) + return + if result != "unchanged": + print_note(f"{display} OS-managed settings: {result}") + + +def _print_local_managed_settings_notes(tools: list[str]) -> None: + for tool in tools: + if tool not in _MANAGED_SETTINGS_SYNC_TOOLS: + continue + display = TOOL_SPECS[tool]["display"] + print_note( + f"{display} was configured locally. To also synchronize OS-managed settings, run " + f"`ucode configure --agent {tool} --sync-managed-settings`." + ) + + def configure_workspace_command( tool: str | None = None, selected_tools: list[str] | None = None, @@ -842,9 +891,14 @@ def configure_workspace_command( fable_enabled: bool | None = None, databricks_ai_tools_enabled: bool | None = None, offer_optional_setup: bool = False, + sync_managed_settings: bool = False, + restore_managed_settings: bool = False, ) -> int: if tool is not None and selected_tools is not None: raise RuntimeError("Use either --agent or --agents, not both.") + explicitly_selected = [tool] if tool is not None else selected_tools + if sync_managed_settings and explicitly_selected is not None: + _validate_managed_settings_sync_selection(explicitly_selected) # The Databricks-vs-Model-Provider-Service picker is shown only on the fully # interactive path (`ucode configure` with no --agent/--agents). Naming agents @@ -863,7 +917,13 @@ def configure_workspace_command( databricks_ai_tools_enabled=databricks_ai_tools_enabled, ) state = states[0] - state = configure_single_tool(tool, state) + if ( + tool in _MANAGED_SETTINGS_SYNC_TOOLS + and restore_managed_settings + and not sync_managed_settings + ): + _restore_managed_settings_for_local_config(tool) + state = configure_single_tool(tool, state, sync_managed_settings=sync_managed_settings) install_databricks_ai_tools_for_agents([tool], state) spec = TOOL_SPECS[tool] console.print( @@ -876,6 +936,8 @@ def configure_workspace_command( expand=False, ) ) + if not sync_managed_settings: + _print_local_managed_settings_notes([tool]) if skip_validate: print_note(f"Skipping {spec['display']} validation (--skip-validate).") return 0 @@ -935,6 +997,8 @@ def configure_workspace_command( print_warning(f"Skipping agent(s) not available on this workspace: {displays}.") picked = [tool_name for tool_name in selected_tools if tool_name in available_on_workspace] + if sync_managed_settings: + _validate_managed_settings_sync_selection(picked) if not picked: print_note("No coding agents selected — nothing to configure.") return 0 @@ -953,10 +1017,18 @@ def configure_workspace_command( for tool_name in picked: state = _maybe_select_provider_service(tool_name, state) + if restore_managed_settings and not sync_managed_settings: + for tool_name in picked: + _restore_managed_settings_for_local_config(tool_name) if offer_optional_setup: - state = configure_selected_tools(state, picked, install_ai_tools=False) + state = configure_selected_tools( + state, + picked, + install_ai_tools=False, + sync_managed_settings=sync_managed_settings, + ) else: - state = configure_selected_tools(state, picked) + state = configure_selected_tools(state, picked, sync_managed_settings=sync_managed_settings) summary_lines = [f"[bold]Workspace:[/bold] [cyan]{state['workspace']}[/cyan]"] for tool_name in picked: @@ -973,6 +1045,8 @@ def configure_workspace_command( expand=False, ) ) + if not sync_managed_settings: + _print_local_managed_settings_notes(picked) if skip_validate: print_note("Skipping agent validation (--skip-validate).") @@ -2596,6 +2670,14 @@ def configure( "freshly discovered models.", ), ] = False, + sync_managed_settings: Annotated[ + bool, + typer.Option( + "--sync-managed-settings", + help="Also synchronize selected supported agents' OS-managed settings. This may " + "require administrator access and can affect agents launched outside ucode.", + ), + ] = False, skip_unavailable: Annotated[ bool, typer.Option( @@ -2679,6 +2761,8 @@ def configure( install_databricks_cli() if agent is not None and agents is not None: raise RuntimeError("Use either --agent or --agents, not both.") + if sync_managed_settings and agent is not None: + _validate_managed_settings_sync_selection([normalize_tool(agent)]) if workspaces is not None and profiles is not None: raise RuntimeError("Use either --workspaces or --profiles, not both.") if use_pat and profiles is None: @@ -2709,6 +2793,13 @@ def configure( # Only forward the opt-in flags when set so existing call expectations # (and defaults) stay unchanged for the common interactive path. skip_kwargs: dict = {} + if sync_managed_settings: + skip_kwargs["sync_managed_settings"] = True + elif ( + claude_agent.managed_settings_backup_available() + or codex_agent.managed_config_backup_available() + ): + skip_kwargs["restore_managed_settings"] = True if use_pat: skip_kwargs["use_pat"] = True if skip_validate: @@ -2754,6 +2845,10 @@ def configure( # only agent, do a workspace-only configure so that later `configure # mcp` run has a current workspace to target. requested = [a.strip().lower() for a in agents.split(",") if a.strip()] + if sync_managed_settings: + _validate_managed_settings_sync_selection( + [normalize_tool(name) for name in requested if name != "cursor"] + ) wants_cursor = "cursor" in requested model_agent_names = ",".join(a for a in requested if a != "cursor") if model_agent_names: diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 500cb0c7..29f7c8ba 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -242,8 +242,9 @@ def reconcile_managed_file( return "unsupported" if not managed_writes_allowed() and not is_dry_run(): raise RuntimeError( - f"Refusing to update {display} managed settings at {path} non-interactively. " - "Run the command from an interactive terminal." + f"Cannot synchronize {display} OS-managed settings at {path} non-interactively " + "because administrator approval may be required. Run the configure command from an " + "interactive terminal." ) if path.is_symlink(): raise RuntimeError( @@ -270,8 +271,8 @@ def reconcile_managed_file( _sudo_replace(path, desired_text) except PermissionError as exc: raise RuntimeError( - f"{display} cannot start because ucode could not update {path}: {exc}. " - "Run the ucode command from an interactive terminal and approve the administrator " + f"Could not synchronize {display} OS-managed settings at {path}: {exc}. Run " + "the configure command from an interactive terminal and approve the administrator " "prompt, or contact your administrator." ) from exc except subprocess.CalledProcessError as exc: @@ -501,6 +502,19 @@ def _snapshot_text(entry: dict, key: str) -> str | None: return text +def managed_file_matches_last_applied(tool: str, path: Path, current_text: str | None) -> bool: + """Whether ``current_text`` is the exact managed file last written by ucode.""" + entry = _manifest_files(_load_manifest()).get(tool) + if not isinstance(entry, dict): + return False + if entry.get("path") != str(path): + raise RuntimeError( + f"The saved {tool} managed-settings backup targets {entry.get('path')}, not {path}." + ) + last_text = _snapshot_text(entry, "last_applied_file") + return last_text is not None and current_text == last_text + + def _original_text(entry: dict) -> str | None: if not entry.get("original_existed"): return None @@ -515,6 +529,11 @@ def _backup_label(tool: str) -> str: return "available" if isinstance(entry, dict) else "none" +def managed_file_backup_available(tool: str) -> bool: + """Whether ucode has restoration metadata for ``tool``.""" + return _backup_label(tool) != "none" + + def _delete_backup(tool: str, manifest: dict, entry: dict) -> None: for key in ("backup_file", "last_applied_file"): filename = entry.get(key) @@ -751,8 +770,8 @@ def _sudo_failure_message(path: Path, display: str, exc: subprocess.CalledProces "Contact your administrator." ) return ( - f"{display} cannot start because ucode could not update {path}: {stderr or exc}. " - "Run the ucode command from an interactive terminal and approve the administrator prompt, " + f"Could not synchronize {display} OS-managed settings at {path}: {stderr or exc}. Run " + "the configure command from an interactive terminal and approve the administrator prompt, " "or contact your administrator." ) diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index c76b46e1..bec9735a 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -549,7 +549,6 @@ def _patch(self, monkeypatch, existing, written): ) monkeypatch.setattr(claude, "save_state", lambda state: None) monkeypatch.setattr(claude, "_register_web_search_mcp", lambda *a, **kw: True) - monkeypatch.setattr(claude, "managed_writes_allowed", lambda: True) def test_strips_stale_disable_experimental_betas(self, monkeypatch): existing = {"env": {"CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS": "1"}} @@ -566,10 +565,37 @@ def test_strips_stale_disable_experimental_betas(self, monkeypatch): FAKE_MANAGED_PATH = Path("/tmp/ucode-test/managed-settings.json") -class TestWriteToolConfigManagedSettings: - """Every normal configuration also writes Claude Code's OS-managed settings.""" +class TestManagedSettingsAreCurrent: + def test_absent_file_is_valid_without_fingerprint(self, tmp_path, monkeypatch): + path = tmp_path / "managed.json" + monkeypatch.setattr(claude, "_managed_settings_path", lambda: path) + + assert claude.managed_settings_are_current({}) is True + + def test_existing_file_requires_matching_fingerprint(self, tmp_path, monkeypatch): + path = tmp_path / "managed.json" + path.write_text("{}\n", encoding="utf-8") + monkeypatch.setattr(claude, "_managed_settings_path", lambda: path) + state: dict = {} + + assert claude.managed_settings_are_current(state) is False + claude.mark_managed_file_verified(state, "claude", path, scope="local-compatible") + assert claude.managed_settings_are_current(state) is True - def _patch(self, monkeypatch, private_writes, managed_writes, existing_by_path=None): + +class TestWriteToolConfigManagedSettings: + """Managed settings are read-only unless configure explicitly requests synchronization.""" + + def _patch( + self, + monkeypatch, + private_writes, + managed_writes, + existing_by_path=None, + *, + ucode_owned=False, + backup_available=False, + ): 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. @@ -585,7 +611,6 @@ def _patch(self, monkeypatch, private_writes, managed_writes, existing_by_path=N ) monkeypatch.setattr(claude, "save_state", lambda state: None) monkeypatch.setattr(claude, "_register_web_search_mcp", lambda *a, **kw: True) - monkeypatch.setattr(claude, "managed_writes_allowed", lambda: 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) monkeypatch.setattr( @@ -596,6 +621,16 @@ def _patch(self, monkeypatch, private_writes, managed_writes, existing_by_path=N ), ) monkeypatch.setattr(claude, "mark_managed_file_verified", lambda *a, **kw: None) + monkeypatch.setattr( + claude, + "managed_file_matches_last_applied", + lambda *args: ucode_owned, + ) + monkeypatch.setattr( + claude, + "managed_file_backup_available", + lambda tool: backup_available or ucode_owned, + ) def fake_write_managed(path, text, **kwargs): managed_writes.append((str(path), text)) @@ -603,37 +638,58 @@ def fake_write_managed(path, text, **kwargs): monkeypatch.setattr(claude, "reconcile_managed_file", fake_write_managed) - def test_writes_managed_file_by_default(self, monkeypatch): + def test_default_only_writes_local_settings(self, monkeypatch): private_writes: list = [] managed_writes: list = [] self._patch(monkeypatch, private_writes, managed_writes) state = {"workspace": WS, "codex_models": []} 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 managed_writes == [] + + def test_explicit_sync_writes_absent_managed_file(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + notes: list[str] = [] + self._patch(monkeypatch, private_writes, managed_writes) + monkeypatch.setattr(claude, "print_note", notes.append) + state = {"workspace": WS, "codex_models": []} + claude.write_tool_config(state, "databricks-claude-sonnet-4", sync_managed_settings=True) assert [p for p, _ in managed_writes] == [str(FAKE_MANAGED_PATH)] + assert notes == [f"Synchronizing Claude Code OS-managed settings at {FAKE_MANAGED_PATH}."] - def test_managed_file_preserves_other_keys(self, monkeypatch): + def test_sync_updates_ucode_owned_file_and_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) + self._patch( + monkeypatch, + private_writes, + managed_writes, + existing, + ucode_owned=True, + ) state = {"workspace": WS, "codex_models": []} - claude.write_tool_config(state, "databricks-claude-sonnet-4") + claude.write_tool_config(state, "databricks-claude-sonnet-4", sync_managed_settings=True) _, 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_managed_file_preserves_enterprise_permission_denies(self, monkeypatch): + def test_sync_preserves_enterprise_permission_denies(self, monkeypatch): private_writes: list = [] managed_writes: list = [] existing = {str(FAKE_MANAGED_PATH): {"permissions": {"deny": ["Bash(rm:*)"]}}} - self._patch(monkeypatch, private_writes, managed_writes, existing) + self._patch( + monkeypatch, + private_writes, + managed_writes, + existing, + ucode_owned=True, + ) state = {"workspace": WS, "codex_models": ["databricks-gpt-5"]} - claude.write_tool_config(state, "databricks-claude-sonnet-4") + claude.write_tool_config(state, "databricks-claude-sonnet-4", sync_managed_settings=True) _, text = managed_writes[0] assert json.loads(text)["permissions"]["deny"] == ["Bash(rm:*)", "WebSearch"] @@ -676,32 +732,83 @@ def test_relayed_rejects_invalid_managed_json(self, monkeypatch): assert managed_writes == [] - def test_noninteractive_uses_local_settings_when_managed_file_is_compatible(self, monkeypatch): + def test_compatible_managed_file_is_read_only(self, monkeypatch): private_writes: list = [] managed_writes: list = [] self._patch(monkeypatch, private_writes, managed_writes) - monkeypatch.setattr(claude, "managed_writes_allowed", lambda: False) state = {"workspace": WS, "codex_models": []} claude.write_tool_config(state, "databricks-claude-sonnet-4") assert managed_writes == [] - def test_noninteractive_fails_when_managed_file_conflicts(self, monkeypatch): + def test_conflicting_managed_file_fails_read_only(self, monkeypatch): private_writes: list = [] managed_writes: list = [] existing = { str(FAKE_MANAGED_PATH): {"env": {"ANTHROPIC_BASE_URL": "https://other.example.com"}} } self._patch(monkeypatch, private_writes, managed_writes, existing) - monkeypatch.setattr(claude, "managed_writes_allowed", lambda: False) state = {"workspace": WS, "codex_models": []} - with pytest.raises(RuntimeError, match="cannot be applied non-interactively"): + with pytest.raises(RuntimeError, match="override ucode values"): claude.write_tool_config(state, "databricks-claude-sonnet-4") assert managed_writes == [] + def test_explicit_sync_leaves_compatible_external_file_untouched(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + existing = {str(FAKE_MANAGED_PATH): {"env": {"MY_OWN": "keep"}}} + self._patch(monkeypatch, private_writes, managed_writes, existing) + notes: list[str] = [] + monkeypatch.setattr(claude, "print_note", notes.append) + + claude.write_tool_config( + {"workspace": WS, "codex_models": []}, + "databricks-claude-sonnet-4", + sync_managed_settings=True, + ) + + assert managed_writes == [] + assert any("externally managed and compatible" in note for note in notes) + + def test_explicit_sync_refuses_conflicting_external_file(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + existing = {str(FAKE_MANAGED_PATH): {"env": {"ANTHROPIC_BASE_URL": "https://other"}}} + self._patch(monkeypatch, private_writes, managed_writes, existing) + + with pytest.raises(RuntimeError, match="externally managed and conflict"): + claude.write_tool_config( + {"workspace": WS, "codex_models": []}, + "databricks-claude-sonnet-4", + sync_managed_settings=True, + ) + + assert managed_writes == [] + + def test_explicit_sync_refuses_file_modified_after_ucode(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + existing = {str(FAKE_MANAGED_PATH): {"enterprise": "new-policy"}} + self._patch( + monkeypatch, + private_writes, + managed_writes, + existing, + backup_available=True, + ) + + with pytest.raises(RuntimeError, match="changed after ucode last synchronized"): + claude.write_tool_config( + {"workspace": WS, "codex_models": []}, + "databricks-claude-sonnet-4", + sync_managed_settings=True, + ) + + assert managed_writes == [] + class TestRegisterWebSearchMcp: def test_skips_registration_when_entry_is_current(self, monkeypatch): diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 23a6025b..b83b8fcc 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -644,88 +644,169 @@ def test_fast_success_does_not_retry(self, monkeypatch): assert fallbacks == [] -class TestCodexManagedConfig: - """Every normal configuration also reconciles Codex's OS-managed config.""" +class TestCodexManagedConfigIsCurrent: + def test_absent_file_is_valid_without_fingerprint(self, tmp_path, monkeypatch): + path = tmp_path / "managed_config.toml" + monkeypatch.setattr(codex, "_managed_config_path", lambda: path) + + assert codex.managed_config_is_current({}) is True + + def test_existing_file_requires_matching_fingerprint(self, tmp_path, monkeypatch): + path = tmp_path / "managed_config.toml" + path.write_text("", encoding="utf-8") + monkeypatch.setattr(codex, "_managed_config_path", lambda: path) + state: dict = {} - def _patch(self, tmp_path, monkeypatch): + assert codex.managed_config_is_current(state) is False + codex.mark_managed_file_verified(state, "codex", path, scope="local-compatible") + assert codex.managed_config_is_current(state) is True + + +class TestCodexManagedConfig: + """Managed config is read-only unless configure explicitly requests synchronization.""" + + def _patch( + self, + tmp_path, + monkeypatch, + *, + ucode_owned=False, + backup_available=False, + ): config_path = tmp_path / ".codex" / "ucode.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, "agent_version", lambda binary: "0.134.0") monkeypatch.setattr(codex, "save_state", lambda state: None) - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: True) - # Deterministic managed path + a mocked sudo writer that writes straight to disk, so the test - # can read the TOML back and NO real sudo/`/etc` write ever happens. monkeypatch.setattr(codex, "_managed_config_path", lambda: managed_path) + monkeypatch.setattr( + codex, + "managed_file_matches_last_applied", + lambda *args: ucode_owned, + ) + monkeypatch.setattr( + codex, + "managed_file_backup_available", + lambda tool: backup_available or ucode_owned, + ) + managed_writes: list[Path] = [] def fake_write_managed(path, text, **kwargs): Path(path).parent.mkdir(parents=True, exist_ok=True) Path(path).write_text(text, encoding="utf-8") + managed_writes.append(Path(path)) return "written" monkeypatch.setattr(codex, "reconcile_managed_file", fake_write_managed) - return config_path, managed_path + return config_path, managed_path, managed_writes - def test_writes_managed_config_by_default(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + def test_default_only_writes_local_config(self, tmp_path, monkeypatch): + config_path, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) state = {"workspace": WS, "codex_models": ["gpt-5"]} codex.write_tool_config(state) + assert config_path.exists() + assert not managed_path.exists() + assert managed_writes == [] + + def test_explicit_sync_writes_absent_managed_config(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) + notes: list[str] = [] + monkeypatch.setattr(codex, "print_note", notes.append) + + codex.write_tool_config( + {"workspace": WS, "codex_models": ["gpt-5"]}, + sync_managed_settings=True, + ) + doc = read_toml_safe(managed_path) assert doc["model_provider"] == "ucode-databricks" assert "model" not in doc assert "ucode-databricks" in doc["model_providers"] + assert managed_writes == [managed_path] + assert notes == [f"Synchronizing Codex OS-managed settings at {managed_path}."] - def test_managed_config_preserves_other_keys(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + def test_sync_updates_ucode_owned_config_and_preserves_other_keys(self, tmp_path, monkeypatch): + _, managed_path, _ = self._patch(tmp_path, monkeypatch, ucode_owned=True) managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_text( - 'model = "my-own"\napproval_policy = "on-request"\n', encoding="utf-8" - ) + managed_path.write_text('approval_policy = "on-request"\n', encoding="utf-8") state = {"workspace": WS, "codex_models": ["gpt-5"]} - codex.write_tool_config(state) + codex.write_tool_config(state, sync_managed_settings=True) doc = read_toml_safe(managed_path) - # ucode removes its stale model pin, but other keys already in the managed file survive. assert doc["approval_policy"] == "on-request" - assert "model" not in doc - - def test_noninteractive_uses_local_config_when_managed_config_is_compatible( - self, tmp_path, monkeypatch - ): - _, managed_path = self._patch(tmp_path, monkeypatch) - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) - state = {"workspace": WS, "codex_models": ["gpt-5"]} - codex.write_tool_config(state) - assert not managed_path.exists() + assert doc["model_provider"] == "ucode-databricks" - def test_noninteractive_preserves_unrelated_managed_config(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + def test_default_preserves_compatible_external_config(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) managed_path.parent.mkdir(parents=True, exist_ok=True) original = 'approval_policy = "on-request"\n' managed_path.write_text(original, encoding="utf-8") - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) assert managed_path.read_text(encoding="utf-8") == original + assert managed_writes == [] - def test_noninteractive_fails_when_managed_config_conflicts(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + def test_default_fails_when_managed_config_conflicts(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) managed_path.parent.mkdir(parents=True, exist_ok=True) managed_path.write_text('model_provider = "enterprise"\n', encoding="utf-8") - monkeypatch.setattr(codex, "managed_writes_allowed", lambda: False) - with pytest.raises(RuntimeError, match="cannot be applied non-interactively"): + with pytest.raises(RuntimeError, match="override ucode values"): codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + assert managed_writes == [] + + def test_explicit_sync_leaves_compatible_external_config_untouched(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) + managed_path.parent.mkdir(parents=True, exist_ok=True) + original = 'approval_policy = "on-request"\n' + managed_path.write_text(original, encoding="utf-8") + + codex.write_tool_config( + {"workspace": WS, "codex_models": ["gpt-5"]}, + sync_managed_settings=True, + ) + + assert managed_path.read_text(encoding="utf-8") == original + assert managed_writes == [] + + def test_explicit_sync_refuses_conflicting_external_config(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) + managed_path.parent.mkdir(parents=True, exist_ok=True) + managed_path.write_text('model_provider = "enterprise"\n', encoding="utf-8") + + with pytest.raises(RuntimeError, match="externally managed and conflict"): + codex.write_tool_config( + {"workspace": WS, "codex_models": ["gpt-5"]}, + sync_managed_settings=True, + ) + assert managed_writes == [] + + def test_explicit_sync_refuses_config_modified_after_ucode(self, tmp_path, monkeypatch): + _, managed_path, managed_writes = self._patch( + tmp_path, + monkeypatch, + backup_available=True, + ) + managed_path.parent.mkdir(parents=True, exist_ok=True) + managed_path.write_text('approval_policy = "never"\n', encoding="utf-8") + + with pytest.raises(RuntimeError, match="changed after ucode last synchronized"): + codex.write_tool_config( + {"workspace": WS, "codex_models": ["gpt-5"]}, + sync_managed_settings=True, + ) + assert managed_writes == [] def test_invalid_managed_toml_is_not_modified(self, tmp_path, monkeypatch): - _, managed_path = self._patch(tmp_path, monkeypatch) + _, managed_path, managed_writes = self._patch(tmp_path, monkeypatch) managed_path.parent.mkdir(parents=True, exist_ok=True) managed_path.write_text("[invalid", encoding="utf-8") - with pytest.raises(RuntimeError, match="Cannot safely update Codex managed settings"): + with pytest.raises(RuntimeError, match="Cannot safely inspect Codex managed settings"): codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) assert managed_path.read_text(encoding="utf-8") == "[invalid" + assert managed_writes == [] diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index 5977cc0c..5e3db9d7 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -122,7 +122,9 @@ class TestConfigureWiresAiToolsInstall: def _stub_configure(self, monkeypatch): captured = {} - monkeypatch.setattr(agents_mod, "configure_tool", lambda tool, state, model=None: state) + monkeypatch.setattr( + agents_mod, "configure_tool", lambda tool, state, model=None, **kwargs: state + ) monkeypatch.setattr(agents_mod, "save_state", lambda state: None) monkeypatch.setattr( agents_mod, @@ -521,7 +523,7 @@ def test_ensure_tool_binary_available_raises_when_missing(self, monkeypatch): class TestConfigureSelectedTools: - def test_groups_managed_permission_notice(self, monkeypatch): + def test_groups_only_explicit_managed_syncs(self, monkeypatch): batches: list[list[str]] = [] @contextmanager @@ -530,18 +532,23 @@ def capture_batch(displays): yield monkeypatch.setattr(agents_mod, "managed_write_batch", capture_batch) - monkeypatch.setattr(agents_mod, "_configure_one", lambda tool, state, provider: state) + monkeypatch.setattr( + agents_mod, "_configure_one", lambda tool, state, provider, **kwargs: state + ) monkeypatch.setattr(agents_mod, "save_state", lambda state: None) monkeypatch.setattr(agents_mod, "install_databricks_ai_tools_for_agents", lambda *_: None) configure_selected_tools({}, ["codex", "claude"]) + configure_selected_tools({}, ["codex", "claude"], sync_managed_settings=True) - assert batches == [["Codex", "Claude Code"]] + assert batches == [[], ["Codex", "Claude Code"]] def test_merges_with_existing_available_tools(self, monkeypatch): """Configuring a new tool should not drop previously-configured tools from state['available_tools'].""" - monkeypatch.setattr("ucode.agents.configure_tool", lambda tool, state, model=None: state) + monkeypatch.setattr( + "ucode.agents.configure_tool", lambda tool, state, model=None, **kwargs: state + ) monkeypatch.setattr("ucode.agents.save_state", lambda s: None) state = { @@ -553,7 +560,9 @@ def test_merges_with_existing_available_tools(self, monkeypatch): assert set(result["available_tools"]) == {"codex", "claude"} def test_adds_new_tool_to_available_tools(self, monkeypatch): - monkeypatch.setattr("ucode.agents.configure_tool", lambda tool, state, model=None: state) + monkeypatch.setattr( + "ucode.agents.configure_tool", lambda tool, state, model=None, **kwargs: state + ) monkeypatch.setattr("ucode.agents.save_state", lambda s: None) state = { @@ -565,7 +574,9 @@ def test_adds_new_tool_to_available_tools(self, monkeypatch): assert set(result["available_tools"]) == {"codex", "claude"} def test_empty_selection_preserves_existing(self, monkeypatch): - monkeypatch.setattr("ucode.agents.configure_tool", lambda tool, state, model=None: state) + monkeypatch.setattr( + "ucode.agents.configure_tool", lambda tool, state, model=None, **kwargs: state + ) monkeypatch.setattr("ucode.agents.save_state", lambda s: None) state = {"workspace": "https://x.databricks.com", "available_tools": ["codex"]} diff --git a/tests/test_cli.py b/tests/test_cli.py index 8bb1ea9b..182f23b9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -45,6 +45,8 @@ def no_state_writes(): patch("ucode.agents.codex.save_state"), patch("ucode.agents.claude.save_state"), patch("ucode.agents.claude._managed_settings_path", return_value=None), + patch("ucode.agents.claude.managed_settings_backup_available", return_value=False), + patch("ucode.agents.codex.managed_config_backup_available", return_value=False), patch("ucode.agents.gemini.save_state"), patch("ucode.agents.opencode.save_state"), ): @@ -1577,6 +1579,74 @@ def test_no_flag_calls_configure_all(self): assert result.exit_code == 0, result.output mock_cfg.assert_called_once_with(prompt_optional_updates=True, offer_optional_setup=True) + def test_sync_managed_settings_preserves_interactive_picker(self): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary") as mock_install, + patch("ucode.cli.configure_workspace_command") as mock_cfg, + ): + result = runner.invoke(app, ["configure", "--sync-managed-settings"]) + + assert result.exit_code == 0, result.output + mock_install.assert_not_called() + mock_cfg.assert_called_once_with( + prompt_optional_updates=True, + offer_optional_setup=True, + sync_managed_settings=True, + ) + + def test_sync_managed_settings_requires_supported_agent(self): + with patch("ucode.cli.install_databricks_cli"): + result = runner.invoke( + app, ["configure", "--agent", "gemini", "--sync-managed-settings"] + ) + + assert result.exit_code == 1 + assert "Managed settings synchronization was requested" in result.output + assert "agents support it" in result.output + assert "Claude Code" in result.output + assert "Codex" in result.output + + def test_sync_managed_settings_supports_codex(self): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary") as mock_install, + patch("ucode.cli.configure_workspace_command") as mock_cfg, + ): + result = runner.invoke( + app, ["configure", "--agent", "codex", "--sync-managed-settings"] + ) + + assert result.exit_code == 0, result.output + mock_install.assert_called_once_with( + "codex", strict=True, update_existing=True, prompt_optional_updates=True + ) + mock_cfg.assert_called_once_with("codex", sync_managed_settings=True) + + def test_plain_configure_restores_previous_managed_sync(self): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary"), + patch("ucode.cli.claude_agent.managed_settings_backup_available", return_value=True), + patch("ucode.cli.configure_workspace_command") as mock_cfg, + ): + result = runner.invoke(app, ["configure", "--agent", "claude"]) + + assert result.exit_code == 0, result.output + mock_cfg.assert_called_once_with("claude", restore_managed_settings=True) + + def test_plain_codex_configure_restores_previous_managed_sync(self): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary"), + patch("ucode.cli.codex_agent.managed_config_backup_available", return_value=True), + patch("ucode.cli.configure_workspace_command") as mock_cfg, + ): + result = runner.invoke(app, ["configure", "--agent", "codex"]) + + assert result.exit_code == 0, result.output + mock_cfg.assert_called_once_with("codex", restore_managed_settings=True) + def test_optional_setup_installs_ai_tools_and_configures_mcp(self): import ucode.cli as cli_mod @@ -1974,6 +2044,96 @@ def test_mcp_rejects_bare_short_name(self): class TestConfigureAgentsSelection: + def test_plain_claude_configure_continues_when_restore_fails(self, monkeypatch): + import ucode.cli as cli_mod + + state = {**MINIMAL_STATE, "available_tools": []} + monkeypatch.setattr(cli_mod, "_configure_shared_workspace_states", lambda *a, **k: [state]) + monkeypatch.setattr( + cli_mod, + "configure_single_tool", + lambda tool, current, **kwargs: {**current, "available_tools": [tool]}, + ) + monkeypatch.setattr(cli_mod, "install_databricks_ai_tools_for_agents", lambda *a: None) + monkeypatch.setattr( + cli_mod.claude_agent, + "revert_managed_settings", + lambda: (_ for _ in ()).throw(RuntimeError("administrator access denied")), + ) + monkeypatch.setattr(cli_mod, "is_dry_run", lambda: False) + warnings: list[str] = [] + monkeypatch.setattr(cli_mod, "print_warning", warnings.append) + + result = cli_mod.configure_workspace_command( + "claude", + workspaces=[("https://example.com", None)], + skip_validate=True, + restore_managed_settings=True, + ) + + assert result == 0 + assert any("cleanup is incomplete" in warning for warning in warnings) + + def test_plain_codex_configure_restores_managed_sync(self, monkeypatch): + import ucode.cli as cli_mod + + state = {**MINIMAL_STATE, "available_tools": []} + monkeypatch.setattr(cli_mod, "_configure_shared_workspace_states", lambda *a, **k: [state]) + monkeypatch.setattr( + cli_mod, + "configure_single_tool", + lambda tool, current, **kwargs: {**current, "available_tools": [tool]}, + ) + monkeypatch.setattr(cli_mod, "install_databricks_ai_tools_for_agents", lambda *a: None) + restored: list[str] = [] + monkeypatch.setattr( + cli_mod.codex_agent, + "revert_managed_config", + lambda: restored.append("codex") or "restored", + ) + monkeypatch.setattr(cli_mod, "is_dry_run", lambda: False) + + result = cli_mod.configure_workspace_command( + "codex", + workspaces=[("https://example.com", None)], + skip_validate=True, + restore_managed_settings=True, + ) + + assert result == 0 + assert restored == ["codex"] + + def test_explicit_sync_skips_restore_and_forwards_intent(self, monkeypatch): + import ucode.cli as cli_mod + + state = {**MINIMAL_STATE, "available_tools": []} + monkeypatch.setattr(cli_mod, "_configure_shared_workspace_states", lambda *a, **k: [state]) + configured: list[tuple[str, bool]] = [] + monkeypatch.setattr( + cli_mod, + "configure_single_tool", + lambda tool, current, *, sync_managed_settings=False: ( + configured.append((tool, sync_managed_settings)) or current + ), + ) + monkeypatch.setattr(cli_mod, "install_databricks_ai_tools_for_agents", lambda *a: None) + monkeypatch.setattr( + cli_mod.claude_agent, + "revert_managed_settings", + lambda: pytest.fail("explicit sync must not restore"), + ) + + result = cli_mod.configure_workspace_command( + "claude", + workspaces=[("https://example.com", None)], + skip_validate=True, + sync_managed_settings=True, + restore_managed_settings=True, + ) + + assert result == 0 + assert configured == [("claude", True)] + def test_selected_tools_skip_picker(self, monkeypatch): import ucode.cli as cli_mod @@ -2004,7 +2164,9 @@ def test_selected_tools_skip_picker(self, monkeypatch): monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: configured.append(tools) or {**state, "available_tools": tools}, + lambda state, tools, **kwargs: ( + configured.append(tools) or {**state, "available_tools": tools} + ), ) monkeypatch.setattr(cli_mod, "validate_all_tools", lambda state: None) @@ -2012,6 +2174,31 @@ def test_selected_tools_skip_picker(self, monkeypatch): assert install_calls == ["claude", "codex"] assert configured == [["claude", "codex"]] + def test_interactive_sync_rejects_unsupported_selection_before_writes(self, monkeypatch): + import ucode.cli as cli_mod + + state = {**MINIMAL_STATE, "available_tools": []} + monkeypatch.setattr(cli_mod, "_configure_shared_workspace_states", lambda *a, **k: [state]) + monkeypatch.setattr(cli_mod, "save_state", lambda state: None) + monkeypatch.setattr(cli_mod, "check_gateway_endpoint", lambda state, tool: tool == "gemini") + monkeypatch.setattr(cli_mod, "prompt_for_tools", lambda available: ["gemini"]) + monkeypatch.setattr( + cli_mod, + "install_tool_binary", + lambda *args, **kwargs: pytest.fail("must reject before installing agents"), + ) + monkeypatch.setattr( + cli_mod, + "configure_selected_tools", + lambda *args, **kwargs: pytest.fail("must reject before writing agent configuration"), + ) + + with pytest.raises(RuntimeError, match="none of the selected agents support it"): + cli_mod.configure_workspace_command( + workspaces=[("https://example.com", None)], + sync_managed_settings=True, + ) + def test_provider_picker_gated_by_interactive_path(self, monkeypatch): import ucode.cli as cli_mod @@ -2020,7 +2207,9 @@ def test_provider_picker_gated_by_interactive_path(self, monkeypatch): monkeypatch.setattr(cli_mod, "check_gateway_endpoint", lambda s, t: t == "claude") monkeypatch.setattr(cli_mod, "install_tool_binary", lambda *a, **k: True) monkeypatch.setattr( - cli_mod, "configure_selected_tools", lambda s, tools: {**s, "available_tools": tools} + cli_mod, + "configure_selected_tools", + lambda s, tools, **kwargs: {**s, "available_tools": tools}, ) monkeypatch.setattr(cli_mod, "validate_all_tools", lambda s: None) picked_for: list[str] = [] @@ -2059,7 +2248,9 @@ def test_unavailable_selected_tool_errors_before_configure(self, monkeypatch): monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: pytest.fail("configure_selected_tools should not be called"), + lambda state, tools, **kwargs: pytest.fail( + "configure_selected_tools should not be called" + ), ) with pytest.raises(RuntimeError, match="Codex"): @@ -2098,7 +2289,9 @@ def test_skip_unavailable_configures_available_subset(self, monkeypatch): monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: configured.append(tools) or {**state, "available_tools": tools}, + lambda state, tools, **kwargs: ( + configured.append(tools) or {**state, "available_tools": tools} + ), ) monkeypatch.setattr(cli_mod, "validate_all_tools", lambda state: None) warnings: list[str] = [] @@ -2126,7 +2319,9 @@ def test_skip_unavailable_still_fails_when_none_available(self, monkeypatch): monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: pytest.fail("configure_selected_tools should not be called"), + lambda state, tools, **kwargs: pytest.fail( + "configure_selected_tools should not be called" + ), ) assert ( @@ -2173,7 +2368,7 @@ def fake_configure_shared_state( monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: {**state, "available_tools": tools}, + lambda state, tools, **kwargs: {**state, "available_tools": tools}, ) monkeypatch.setattr(cli_mod, "validate_all_tools", lambda state: None) @@ -2214,7 +2409,7 @@ def fake_configure_shared_state( monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda state, tools: ( + lambda state, tools, **kwargs: ( configured_tools.append((state["workspace"], tools)) or {**state, "available_tools": tools} ), @@ -2732,7 +2927,7 @@ def test_skip_validate_skips_agent_validation(self, monkeypatch): monkeypatch.setattr( cli_mod, "configure_selected_tools", - lambda s, tools: {**s, "available_tools": tools}, + lambda s, tools, **kwargs: {**s, "available_tools": tools}, ) validated: list = [] monkeypatch.setattr(cli_mod, "validate_all_tools", lambda s: validated.append(s)) @@ -2751,7 +2946,7 @@ def test_skip_validate_skips_single_tool_validation(self, monkeypatch): state = {**MINIMAL_STATE, "workspace": "https://first.com"} monkeypatch.setattr(cli_mod, "configure_shared_state", lambda *a, **k: state) - monkeypatch.setattr(cli_mod, "configure_single_tool", lambda t, s: s) + monkeypatch.setattr(cli_mod, "configure_single_tool", lambda t, s, **kwargs: s) installed: list = [] monkeypatch.setattr( cli_mod, diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index 80860b2a..d3f5a43f 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -133,7 +133,7 @@ def deny_write(path, text): monkeypatch.setattr(managed_files, "_sudo_replace", deny_write) - with pytest.raises(RuntimeError, match="could not update"): + with pytest.raises(RuntimeError, match="Could not synchronize"): managed_files.reconcile_managed_file( path, '{"ucode": true}\n', @@ -238,6 +238,25 @@ def test_verified_check_uses_fingerprint(self, tmp_path): path.write_text("changed-content", encoding="utf-8") assert managed_files.managed_file_is_verified(state, "claude", path) is False + def test_matches_last_applied_snapshot(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + monkeypatch.setattr( + managed_files, + "_sudo_replace", + lambda target, text: target.write_text(text, encoding="utf-8"), + ) + managed_files.reconcile_managed_file( + path, + '{"ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + ) + + assert managed_files.managed_file_matches_last_applied("claude", path, path.read_text()) + path.write_text('{"ucode": false}\n', encoding="utf-8") + assert not managed_files.managed_file_matches_last_applied("claude", path, path.read_text()) + def test_revert_restores_exact_original(self, tmp_path, backup_dir, monkeypatch): path = tmp_path / "managed.json" path.write_text('{"enterprise": true}\n', encoding="utf-8") @@ -291,6 +310,37 @@ def test_revert_removes_file_created_by_ucode(self, tmp_path, backup_dir, monkey assert result == "removed" assert not path.exists() + def test_revert_permission_failure_retains_backup(self, tmp_path, backup_dir, monkeypatch): + path = tmp_path / "managed.json" + monkeypatch.setattr( + managed_files, + "_sudo_replace", + lambda target, text: target.write_text(text, encoding="utf-8"), + ) + managed_files.reconcile_managed_file( + path, + '{"ucode": true}\n', + tool="claude", + display="Claude Code", + owned_paths=[["ucode"]], + ) + monkeypatch.setattr( + managed_files, + "_sudo_remove", + lambda target: (_ for _ in ()).throw(PermissionError("denied")), + ) + + with pytest.raises(RuntimeError, match="backup was retained"): + managed_files.revert_managed_file( + "claude", + display="Claude Code", + parser=json.loads, + dumper=lambda doc: json.dumps(doc) + "\n", + ) + + assert managed_files.managed_file_backup_available("claude") is True + assert path.exists() + def test_revert_preserves_external_changes(self, tmp_path, backup_dir, monkeypatch): path = tmp_path / "managed.json" path.write_text('{"enterprise": "original"}\n', encoding="utf-8") diff --git a/tests/test_managed_resolve.py b/tests/test_managed_resolve.py index cf5d5cdb..ff89cd5d 100644 --- a/tests/test_managed_resolve.py +++ b/tests/test_managed_resolve.py @@ -193,20 +193,13 @@ class TestStateFileIsNotRewritten: @pytest.fixture def real_state_file(self, tmp_path, monkeypatch): - """Redirect state.json and both Claude settings files into tmp_path, unstubbed.""" + """Redirect state.json and Claude's local settings file into tmp_path, unstubbed.""" monkeypatch.setattr(config_io, "APP_DIR", tmp_path) monkeypatch.setattr(state_mod, "STATE_PATH", tmp_path / "state.json") monkeypatch.setattr(claude, "CLAUDE_SETTINGS_PATH", tmp_path / "ucode-settings.json") monkeypatch.setattr(claude, "CLAUDE_BACKUP_PATH", tmp_path / "backup.json") managed_settings_path = tmp_path / "managed-settings.json" monkeypatch.setattr(claude, "_managed_settings_path", lambda: managed_settings_path) - monkeypatch.setattr(claude, "managed_writes_allowed", lambda: True) - - def reconcile_managed_file(path, desired_text, **kwargs): - path.write_text(desired_text, encoding="utf-8") - return "written" - - monkeypatch.setattr(claude, "reconcile_managed_file", reconcile_managed_file) # Seed a developer whose own opus choice differs from the manifest's. state_mod.save_state( { @@ -240,8 +233,7 @@ def test_settings_file_gets_the_managed_model(self, real_state_file): env = json.loads((real_state_file / "ucode-settings.json").read_text())["env"] assert env["ANTHROPIC_DEFAULT_OPUS_MODEL"].startswith("system.ai.claude-opus-5") - managed_env = json.loads((real_state_file / "managed-settings.json").read_text())["env"] - assert managed_env["ANTHROPIC_DEFAULT_OPUS_MODEL"].startswith("system.ai.claude-opus-5") + assert not (real_state_file / "managed-settings.json").exists() def test_overlay_bookkeeping_never_lands_on_disk(self, real_state_file): resolved_state = resolve_state(MANAGED, state_mod.load_state(), "claude")