From 5d6f2bd6ac70606421f3b90d83c09442eec1c176 Mon Sep 17 00:00:00 2001 From: John Speaks Date: Tue, 20 Oct 2026 14:01:01 +0000 Subject: [PATCH 1/2] [AIGTWY] Add ug configure --global/--user scope --user skips the OS-managed Claude Code and Codex settings like UG_SKIP_OS_SETTINGS, so no sudo is needed and ug relies on the per-user files it already passes at launch. It fails while an OS-managed file for the agent exists, and the choice is saved after a successful configure. Co-authored-by: Isaac --- README.md | 6 +++ docs/os-managed-settings-design.md | 8 +++ src/ucode/agents/__init__.py | 20 ++++++- src/ucode/agents/claude.py | 2 +- src/ucode/agents/codex.py | 7 ++- src/ucode/cli.py | 29 +++++++++- src/ucode/managed_files.py | 20 +++++-- src/ucode/state.py | 22 +++++++- tests/conftest.py | 1 + tests/test_agent_claude.py | 28 ++++++++++ tests/test_agent_codex.py | 26 +++++++++ tests/test_agents_init.py | 37 +++++++++++++ tests/test_cli.py | 87 ++++++++++++++++++++++++++++++ tests/test_managed_files.py | 11 ++++ 14 files changed, 296 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index b5293c035..5d6dc9563 100644 --- a/README.md +++ b/README.md @@ -108,8 +108,14 @@ ug configure ug configure --agents claude,codex ug configure --workspace https://first.databricks.com ug configure --profile DEFAULT --agents claude,codex +ug configure --user ``` +`--global` (the default) writes the machine-wide Claude Code and Codex managed settings with sudo. +`--user` skips them, like `UG_SKIP_OS_SETTINGS`, and needs no sudo; use it when you launch through +`ug`. It fails while a managed file for the agent exists, since managed settings take precedence. +The choice is saved after a successful configure, until `ug revert` or the next `--global`/`--user`. + Available coding agents are `codex`, `claude`, `gemini`, `opencode`, `copilot`, and `pi`. `cursor` can be included in `--agents` for MCP-only setup; Cursor models still run through your Cursor account. diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index d734ef1fb..2762eca2d 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -27,6 +27,14 @@ for a password. If that repair is denied, the agent does not launch. The local file is always written. The OS-managed file is additionally reconciled during interactive configuration, except for Claude subscription relay. +## Per-user Scope + +`ug configure --user` (saved as `config_scope` in `~/.ucode/state.json`, default `global`) skips the +OS-managed files like `UG_SKIP_OS_SETTINGS`: no administrator permission, and managed MCP servers +register in user scope. Because OS-managed settings take precedence over user settings, `--user` +fails for Claude Code or Codex while their OS-managed file exists, whoever wrote it; `ug revert`, +removal, or `--global` resolves it. `ug revert` resets the scope to `global`. + ## Interactive Detection Standard input determines whether UG may request administrator permission interactively. Standard diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index b148b496a..d1e2a7da4 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -27,7 +27,7 @@ ) from ucode.env import env_bool from ucode.managed_config import refresh_managed_config -from ucode.managed_files import managed_write_batch +from ucode.managed_files import config_scope, managed_write_batch from ucode.os_compatibility import subprocess_cross_os from ucode.state import get_model_location, get_provider_service, load_state, save_state from ucode.telemetry import agent_version @@ -414,6 +414,23 @@ def resolve_gemini_provider_model( ) +def _refuse_user_scope_over_managed_file(tool: str) -> None: + if config_scope() != "user": + return + if tool == AGENT_CLAUDE: + display, path = "Claude Code", claude._managed_settings_path() + elif tool == AGENT_CODEX: + display, path = "Codex", codex.codex_managed_config_path() + else: + return + if path is not None and path.exists(): + raise RuntimeError( + f"`ug configure --user` can't be used while {display} managed settings exist at " + f"{path}; they take precedence over user settings. Run `ug revert` if ug wrote them, " + "ask your administrator to remove them, or use `ug configure --global`." + ) + + def configure_tool( tool: str, state: dict, @@ -427,6 +444,7 @@ def configure_tool( parent_schema: str | None = None, picker_catalog: AnthropicModelCatalog | None = None, ) -> dict: + _refuse_user_scope_over_managed_file(tool) result: dict | tuple[dict, str] if tool == AGENT_CODEX: result = codex.write_tool_config( diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 93b5974b6..577631fcc 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -998,7 +998,7 @@ def reconcile_managed_mcp( user-scope fallback), so an admin's ``agent_native_settings`` MCP allowlist keeps admitting them. """ path = _managed_settings_path() - if path is None or not managed_writes_allowed(): + if path is None or managed_settings_skipped() or not managed_writes_allowed(): return False if path.is_symlink(): raise RuntimeError( diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 7a5601384..1fa868ef4 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -947,7 +947,12 @@ def reconcile_managed_mcp(state: dict, servers: dict[str, dict]) -> bool: platform or a non-interactive run), so the caller routes those servers to the user-scope registration instead.""" path = codex_managed_config_path() - if path is None or path.name == CODEX_REQUIREMENTS_FILE or not managed_writes_allowed(): + if ( + path is None + or path.name == CODEX_REQUIREMENTS_FILE + or managed_settings_skipped() + or not managed_writes_allowed() + ): return False if path.is_symlink(): raise RuntimeError( diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 4f81959a4..91660000f 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -99,7 +99,7 @@ normalize_managed_config, refresh_managed_config, ) -from ucode.managed_files import managed_write_session +from ucode.managed_files import managed_write_session, set_config_scope from ucode.managed_resolve import ( managed_claude_family_models, managed_default_model, @@ -163,9 +163,11 @@ get_model_location, get_provider_service, is_self_managed, + load_config_scope, load_full_state, load_state, remove_self_managed_agent, + save_config_scope, save_state, set_current_workspace, set_model_location, @@ -3643,6 +3645,8 @@ def default( workspace: WorkspaceOption = None, ) -> None: """Configure and launch coding agents through Databricks AI Gateway.""" + if ctx.invoked_subcommand != "revert": + set_config_scope(load_config_scope()) if ctx.invoked_subcommand is not None: return set_dry_run(dry_run) @@ -4167,6 +4171,22 @@ def configure( "compatibility. Accepted for backward compatibility.", ), ] = False, + global_scope: Annotated[ + bool, + typer.Option( + "--global", + help="Write machine-wide Claude Code and Codex managed settings with sudo (default). " + "The choice is saved for later commands.", + ), + ] = False, + user_scope: Annotated[ + bool, + typer.Option( + "--user", + help="Skip the machine-wide Claude Code and Codex managed settings, without sudo. " + "The choice is saved for later commands.", + ), + ] = False, verbose: Annotated[ str, typer.Option( @@ -4185,6 +4205,11 @@ def configure( set_dry_run(dry_run) set_verbosity(verbose) try: + if global_scope and user_scope: + raise RuntimeError("Use either --global or --user, not both.") + scope = "user" if user_scope else "global" + if global_scope or user_scope: + set_config_scope(scope) custom_oauth = _custom_oauth_config(client_id, redirect_url, scopes) if custom_oauth is not None and use_pat: raise RuntimeError("--client-id cannot be combined with --use-pat.") @@ -4334,6 +4359,8 @@ def configure( and prompt_yes_no("Configure MCP servers now?") ): configure_mcp_command() + if global_scope or user_scope: + save_config_scope(scope) except typer.Exit: # `typer.Exit` subclasses RuntimeError, so it has to be re-raised ahead of the handler # below. Otherwise a clean exit (e.g. `_reject_configure_under_managed_config` under a diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 02999fb9a..d7f135c40 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -36,6 +36,9 @@ _SUDO = "/usr/bin/sudo" # Opt out of all OS-managed settings work (read, conflict check, and the privileged sudo write). SKIP_OS_SETTINGS_ENV = "UG_SKIP_OS_SETTINGS" +CONFIG_SCOPES = ("global", "user") +DEFAULT_CONFIG_SCOPE = "global" +_config_scope = DEFAULT_CONFIG_SCOPE MANAGED_BACKUP_DIR = APP_DIR / "managed-backups" MANAGED_BACKUP_MANIFEST_PATH = MANAGED_BACKUP_DIR / "manifest.json" MANAGED_FINGERPRINT_VERSION = 1 @@ -92,8 +95,19 @@ def current_os() -> OS: return OS.OTHER +def set_config_scope(scope: str) -> None: + global _config_scope + if scope not in CONFIG_SCOPES: + raise ValueError(f"Unknown config scope {scope!r}; expected one of {CONFIG_SCOPES}.") + _config_scope = scope + + +def config_scope() -> str: + return _config_scope + + def managed_settings_skipped() -> bool: - """Whether the developer opted out of OS-managed settings via ``UG_SKIP_OS_SETTINGS``. + """Whether OS-managed settings are opted out via ``UG_SKIP_OS_SETTINGS`` or the ``user`` scope. Set the variable to ``1``/``true``/``yes``/``on`` to make ucode neither read nor update the root-owned managed files (Claude Code ``managed-settings.json``, Codex ``managed_config.toml`` @@ -102,9 +116,9 @@ def managed_settings_skipped() -> bool: always launches through ``ug`` and never needs a bare ``claude``/``codex`` to reach the gateway. Skips the privileged ``sudo`` step entirely, so an interactive run is never prompted for a password and never hits the "OS-managed settings could not be updated" failure on hosts where - that write is blocked. + that write is blocked. ``ug configure --user`` has the same effect. """ - return env_bool(SKIP_OS_SETTINGS_ENV) + return env_bool(SKIP_OS_SETTINGS_ENV) or _config_scope == "user" def managed_files_supported() -> bool: diff --git a/src/ucode/state.py b/src/ucode/state.py index 925f62307..75eb1d367 100644 --- a/src/ucode/state.py +++ b/src/ucode/state.py @@ -17,9 +17,11 @@ build_auth_token_argv, build_shared_base_urls, ) +from ucode.managed_files import CONFIG_SCOPES, DEFAULT_CONFIG_SCOPE STATE_PATH = APP_DIR / "state.json" STATE_VERSION = 3 +CONFIG_SCOPE_KEY = "config_scope" # Transient key holding the developer's own values for whatever a managed config layered over them. # Present only in memory: the layered values render the agent settings files, while `save_state` # restores what's under it so `state.json` keeps recording the developer's own configuration. @@ -41,6 +43,23 @@ def load_full_state() -> dict: return data +def load_config_scope() -> str: + scope = load_full_state().get(CONFIG_SCOPE_KEY) + return scope if scope in CONFIG_SCOPES else DEFAULT_CONFIG_SCOPE + + +def save_config_scope(scope: str) -> None: + if is_dry_run() or load_config_scope() == scope: + return + full = load_full_state() + full[CONFIG_SCOPE_KEY] = scope + try: + APP_DIR.mkdir(parents=True, exist_ok=True) + STATE_PATH.write_text(json.dumps(full, indent=2), encoding="utf-8") + except OSError as exc: + raise RuntimeError(f"Failed to write state file: {STATE_PATH}") from exc + + def load_state() -> dict: """Load the current workspace's state as a flat dict.""" full = load_full_state() @@ -240,8 +259,9 @@ def build_agent_state(state: dict) -> dict[str, dict]: def clear_state() -> None: - """Remove the current workspace entry from state.""" + """Remove the current workspace entry and the config scope from state.""" full = load_full_state() + full.pop(CONFIG_SCOPE_KEY, None) workspace = full.get("current_workspace") if workspace: full.get("workspaces", {}).pop(workspace, None) diff --git a/tests/conftest.py b/tests/conftest.py index c0dde1460..fb5901ddd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,6 +56,7 @@ def _isolate_ucode_state(tmp_path, monkeypatch): monkeypatch.setattr( managed_files_mod, "MANAGED_BACKUP_MANIFEST_PATH", backup_dir / "manifest.json" ) + monkeypatch.setattr(managed_files_mod, "_config_scope", "global") monkeypatch.setattr(codex_mod, "codex_managed_config_path", lambda: None) monkeypatch.setattr( codex_mod, "CODEX_MODEL_CATALOG_PATH", state_dir / "codex-model-catalog.json" diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 080991dba..73ebd388d 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -1130,6 +1130,19 @@ def test_skip_env_errors_on_stale_managed_override(self, monkeypatch): assert "stale" in message assert "ug revert" in message + def test_user_scope_bypasses_managed_file(self, monkeypatch): + private_writes: list = [] + managed_writes: list = [] + self._patch(monkeypatch, private_writes, managed_writes) + managed_files.set_config_scope("user") + monkeypatch.setattr( + claude, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") + ) + state = {"workspace": WS, "codex_models": []} + claude.write_tool_config(state, "databricks-claude-sonnet-4") + assert str(claude.CLAUDE_SETTINGS_PATH) in [p for p, _ in private_writes] + assert managed_writes == [] + def test_skip_env_no_error_without_stale_override(self, monkeypatch): private_writes: list = [] managed_writes: list = [] @@ -4140,6 +4153,21 @@ def test_admin_mcp_allowlist_admits_every_server_ug_registers(self, monkeypatch) ] assert ["allowedMcpServers"] in captured["owned_paths"] + def test_user_scope_returns_false_without_touching_managed_file(self, monkeypatch): + monkeypatch.setattr(claude, "_managed_settings_path", lambda: FAKE_MANAGED_PATH) + monkeypatch.setattr(claude, "managed_writes_allowed", lambda: True) + monkeypatch.setattr( + claude, "read_managed_file", lambda path: pytest.fail("must not read managed") + ) + monkeypatch.setattr( + claude, "reconcile_managed_file", lambda *a, **k: pytest.fail("must not write") + ) + managed_files.set_config_scope("user") + used = claude.reconcile_managed_mcp( + {}, {"system-ai-github": claude.managed_mcp_entry(GH_URL)} + ) + assert used is False + def test_mcp_allowlist_not_from_agent_native_settings_is_left_alone(self, monkeypatch): captured: dict = {} existing = json.dumps({"allowedMcpServers": [{"serverName": "jira-internal"}]}) diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 296c58a0f..8093ca0de 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -1784,6 +1784,18 @@ def test_skip_env_errors_on_stale_managed_override(self, tmp_path, monkeypatch): assert "stale" in message assert "ug revert" in message + def test_user_scope_bypasses_managed_config(self, tmp_path, monkeypatch): + config_path, managed_path = self._patch(tmp_path, monkeypatch) + managed_files.set_config_scope("user") + monkeypatch.setattr( + codex, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") + ) + + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + + assert read_toml_safe(config_path)["model_provider"] == "Databricks" + assert not managed_path.exists() + def test_skip_env_no_error_without_stale_override(self, tmp_path, monkeypatch): config_path, managed_path = self._patch(tmp_path, monkeypatch) monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") @@ -2314,6 +2326,20 @@ def test_non_interactive_returns_false_without_writing(self, monkeypatch): ) assert codex.reconcile_managed_mcp({}, {"s": codex.managed_mcp_entry(self.ARGV)}) is False + def test_user_scope_returns_false_without_touching_managed_file(self, monkeypatch): + monkeypatch.setattr( + codex, "codex_managed_config_path", lambda: Path("/etc/codex/managed_config.toml") + ) + monkeypatch.setattr(codex, "managed_writes_allowed", lambda: True) + monkeypatch.setattr( + codex, "read_managed_file", lambda path: pytest.fail("must not read managed") + ) + monkeypatch.setattr( + codex, "reconcile_managed_file", lambda *a, **k: pytest.fail("must not write") + ) + managed_files.set_config_scope("user") + assert codex.reconcile_managed_mcp({}, {"s": codex.managed_mcp_entry(self.ARGV)}) is False + def test_preserves_prior_verification_scope(self, monkeypatch): # An MCP-only write must refresh the fingerprint without downgrading the model reconcile's # scope (e.g. local-compatible). diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index 47b4acacb..ea3b53247 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -13,6 +13,7 @@ import ucode.agents as agents_mod import ucode.databricks as db_mod +from ucode import managed_files from ucode.agents import ( DEFAULT_TOOL, TOOL_SPECS, @@ -1211,3 +1212,39 @@ def test_appends_os_managed_file_recorded_in_state(self): paths = configured_paths("codex", state) assert str(CODEX_CONFIG_PATH).replace(str(CODEX_CONFIG_PATH.home()), "~", 1) in paths assert "/etc/codex/managed_config.toml" in paths + + +class TestConfigureToolUserScope: + @pytest.mark.parametrize( + "tool,path_owner,path_attr,writer", + [ + ("claude", "claude", "_managed_settings_path", "write_tool_config"), + ("codex", "codex", "codex_managed_config_path", "write_tool_config"), + ], + ) + @pytest.mark.parametrize("exists", [True, False]) + def test_managed_file_blocks_user_scope( + self, monkeypatch, tmp_path, tool, path_owner, path_attr, writer, exists + ): + managed = tmp_path / "managed" + if exists: + managed.write_text("x", encoding="utf-8") + module = getattr(agents_mod, path_owner) + monkeypatch.setattr(module, path_attr, lambda: managed) + written: list = [] + monkeypatch.setattr(module, writer, lambda state, *a, **kw: written.append(tool) or state) + managed_files.set_config_scope("user") + + if exists: + with pytest.raises(RuntimeError, match=r"--user.*managed settings exist.*--global"): + agents_mod.configure_tool(tool, {}, "m") + assert written == [] + else: + agents_mod.configure_tool(tool, {}, "m") + assert written == [tool] + + def test_no_managed_path_is_not_checked(self, monkeypatch): + monkeypatch.setattr(agents_mod.codex, "codex_managed_config_path", lambda: None) + monkeypatch.setattr(agents_mod.codex, "write_tool_config", lambda state, *a, **kw: state) + managed_files.set_config_scope("user") + agents_mod.configure_tool("codex", {}, "m") diff --git a/tests/test_cli.py b/tests/test_cli.py index 81acc425b..dbab754fe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5138,6 +5138,93 @@ def test_profile_and_workspace_are_mutually_exclusive(self): mock_cfg.assert_not_called() +class TestConfigureScopeFlags: + """`ug configure --global/--user` picks, saves, and reapplies the managed-settings scope.""" + + ARGS = ["configure", "--agents", "claude,codex", "--workspace", "https://first.databricks.com"] + + @staticmethod + def _configure(extra=(), *, args=None): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary"), + patch("ucode.cli.configure_workspace_command") as mock_cfg, + ): + result = runner.invoke(app, [*(args or TestConfigureScopeFlags.ARGS), *extra]) + return result, mock_cfg + + def test_default_is_global_and_not_persisted(self): + result, _ = self._configure() + assert result.exit_code == 0, result.output + assert managed_files.config_scope() == "global" + assert "config_scope" not in cli_mod.load_full_state() + + def test_user_flag_sets_and_persists_scope(self): + result, mock_cfg = self._configure(["--user"]) + assert result.exit_code == 0, result.output + assert managed_files.config_scope() == "user" + assert cli_mod.load_full_state()["config_scope"] == "user" + assert "user_scope" not in mock_cfg.call_args.kwargs + + def test_global_flag_overrides_saved_user_scope(self): + self._configure(["--user"]) + result, _ = self._configure(["--global"]) + assert result.exit_code == 0, result.output + assert managed_files.config_scope() == "global" + assert cli_mod.load_full_state()["config_scope"] == "global" + + def test_saved_scope_is_reused_without_flag(self): + self._configure(["--user"]) + managed_files.set_config_scope("global") + result, _ = self._configure() + assert result.exit_code == 0, result.output + assert managed_files.config_scope() == "user" + assert cli_mod.load_full_state()["config_scope"] == "user" + + def test_failed_configure_does_not_persist_scope(self): + with ( + patch("ucode.cli.install_databricks_cli"), + patch("ucode.cli.install_tool_binary"), + patch("ucode.cli.configure_workspace_command", side_effect=RuntimeError("refused")), + ): + result = runner.invoke(app, [*self.ARGS, "--user"]) + assert result.exit_code == 1 + assert managed_files.config_scope() == "user" + assert "config_scope" not in cli_mod.load_full_state() + + def test_flags_are_mutually_exclusive(self): + result, mock_cfg = self._configure(["--global", "--user"]) + assert result.exit_code == 1 + assert "Use either --global or --user, not both." in _strip_ansi(result.output) + mock_cfg.assert_not_called() + assert "config_scope" not in cli_mod.load_full_state() + + def test_dry_run_applies_but_does_not_persist(self): + result, _ = self._configure(["--user", "--dry-run"]) + assert result.exit_code == 0, result.output + assert managed_files.config_scope() == "user" + assert "config_scope" not in cli_mod.load_full_state() + + def test_saved_scope_applies_to_other_commands(self): + self._configure(["--user"]) + managed_files.set_config_scope("global") + runner.invoke(app, ["agents", "--help"]) + assert managed_files.config_scope() == "user" + + def test_revert_does_not_apply_saved_user_scope(self): + self._configure(["--user"]) + managed_files.set_config_scope("global") + with patch("ucode.cli.revert", return_value=0): + runner.invoke(app, ["revert"]) + assert managed_files.config_scope() == "global" + + def test_clear_state_resets_scope_to_default(self): + self._configure(["--user"]) + cli_mod.clear_state() + assert "config_scope" not in cli_mod.load_full_state() + assert cli_mod.load_config_scope() == "global" + + class TestConfigureSharedStateUsePat: """--use-pat reads the profile's PAT from ~/.databrickscfg, exports it as DATABRICKS_BEARER, persists the mode, and never opens a browser.""" diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index 8fbef92de..51292ae6a 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -736,6 +736,17 @@ def test_raise_on_skipped_managed_overrides_reports_stale_leaves(self): assert "stale" in message assert "ug revert" in message + def test_user_scope_skips_managed_settings_and_files(self, monkeypatch): + monkeypatch.delenv(managed_files.SKIP_OS_SETTINGS_ENV, raising=False) + monkeypatch.setattr(managed_files, "_config_scope", "user") + assert managed_files.managed_settings_skipped() is True + monkeypatch.setattr(managed_files, "managed_files_supported", _REAL_MANAGED_FILES_SUPPORTED) + assert managed_files.managed_files_supported() is False + + def test_set_config_scope_rejects_unknown_scope(self): + with pytest.raises(ValueError, match="Unknown config scope"): + managed_files.set_config_scope("system") + def test_raise_on_skipped_managed_overrides_silent_without_conflicts(self): # No conflicts means nothing stale overrides the user config, so this must not raise. managed_files.raise_on_skipped_managed_overrides( From 9aa96985d8c079a914dbdf2aec3a0503ad4238b2 Mon Sep 17 00:00:00 2001 From: John Speaks Date: Wed, 21 Oct 2026 14:31:29 +0000 Subject: [PATCH 2/2] Refuse ug configure --user before configuring any agent; launches use present files Co-authored-by: Isaac --- README.md | 6 ++-- docs/os-managed-settings-design.md | 9 +++-- src/ucode/agents/__init__.py | 29 ++++++++-------- src/ucode/agents/claude.py | 20 +++++++---- src/ucode/agents/codex.py | 16 +++++---- src/ucode/cli.py | 10 +++--- tests/test_agent_claude.py | 23 +++++++----- tests/test_agent_codex.py | 22 ++++++++---- tests/test_agents_init.py | 54 +++++++++++++++++++--------- tests/test_cli.py | 56 ++++++++++++++++++++++-------- tests/test_state.py | 7 ++++ 11 files changed, 169 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index 5d6dc9563..3843d03f6 100644 --- a/README.md +++ b/README.md @@ -113,8 +113,10 @@ ug configure --user `--global` (the default) writes the machine-wide Claude Code and Codex managed settings with sudo. `--user` skips them, like `UG_SKIP_OS_SETTINGS`, and needs no sudo; use it when you launch through -`ug`. It fails while a managed file for the agent exists, since managed settings take precedence. -The choice is saved after a successful configure, until `ug revert` or the next `--global`/`--user`. +`ug`. Configure fails while a managed file for the agent exists, since managed settings take +precedence; later launches use whatever files are present. +The choice is saved for launches after a successful configure; `ug configure` without `--user` +and `ug revert` return to `--global`. Available coding agents are `codex`, `claude`, `gemini`, `opencode`, `copilot`, and `pi`. `cursor` can be included in `--agents` for MCP-only setup; diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index 2762eca2d..463a7fd2d 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -31,9 +31,12 @@ configuration, except for Claude subscription relay. `ug configure --user` (saved as `config_scope` in `~/.ucode/state.json`, default `global`) skips the OS-managed files like `UG_SKIP_OS_SETTINGS`: no administrator permission, and managed MCP servers -register in user scope. Because OS-managed settings take precedence over user settings, `--user` -fails for Claude Code or Codex while their OS-managed file exists, whoever wrote it; `ug revert`, -removal, or `--global` resolves it. `ug revert` resets the scope to `global`. +register in user scope. Because OS-managed settings take precedence over user settings, +`ug configure --user` fails before configuring any agent while a selected agent's OS-managed file +exists (including Windows `requirements.toml`), whoever wrote it; `ug revert`, removal, or +`--global` resolves it. Launches don't check: a managed file added later simply takes precedence, +without the `UG_SKIP_OS_SETTINGS` stale-override error. `ug configure` without `--user` and +`ug revert` reset the scope to `global`. ## Interactive Detection diff --git a/src/ucode/agents/__init__.py b/src/ucode/agents/__init__.py index d1e2a7da4..8accbb1a6 100644 --- a/src/ucode/agents/__init__.py +++ b/src/ucode/agents/__init__.py @@ -414,21 +414,23 @@ def resolve_gemini_provider_model( ) -def _refuse_user_scope_over_managed_file(tool: str) -> None: +def refuse_user_scope_over_managed_files(tools: list[str]) -> None: + """Run by ``ug configure`` before any agent is configured; launches use the files present.""" if config_scope() != "user": return - if tool == AGENT_CLAUDE: - display, path = "Claude Code", claude._managed_settings_path() - elif tool == AGENT_CODEX: - display, path = "Codex", codex.codex_managed_config_path() - else: - return - if path is not None and path.exists(): - raise RuntimeError( - f"`ug configure --user` can't be used while {display} managed settings exist at " - f"{path}; they take precedence over user settings. Run `ug revert` if ug wrote them, " - "ask your administrator to remove them, or use `ug configure --global`." - ) + for tool in tools: + if tool == AGENT_CLAUDE: + display, path = "Claude Code", claude.managed_settings_path() + elif tool == AGENT_CODEX: + display, path = "Codex", codex.codex_managed_config_path() + else: + continue + if path is not None and path.exists(): + raise RuntimeError( + f"`ug configure --user` can't be used while {display} managed settings exist at " + f"{path}; they take precedence over user settings. Run `ug revert` if ug wrote " + "them, ask your administrator to remove them, or use `ug configure --global`." + ) def configure_tool( @@ -444,7 +446,6 @@ def configure_tool( parent_schema: str | None = None, picker_catalog: AnthropicModelCatalog | None = None, ) -> dict: - _refuse_user_scope_over_managed_file(tool) result: dict | tuple[dict, str] if tool == AGENT_CODEX: result = codex.write_tool_config( diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 577631fcc..e642c5663 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -59,6 +59,7 @@ SettingsPassthrough, apply_settings_passthrough, check_skipped_managed_overrides, + config_scope, current_os, managed_file_conflicts, managed_file_is_verified, @@ -278,6 +279,10 @@ def configured_paths(state: dict) -> list[str]: return [str(CLAUDE_SETTINGS_PATH)] +def managed_settings_path() -> Path | None: + return _managed_settings_path() + + def _managed_settings_path() -> Path | None: """OS-specific location of Claude Code's enterprise managed-settings.json. Returns None on unsupported platforms.""" @@ -1874,13 +1879,14 @@ def _reconcile_managed_settings( during `ucode claude`, so a bare `claude` could not reach the gateway anyway. """ if managed_settings_skipped(): - check_skipped_managed_overrides( - _managed_settings_path(), - parser=_parse_managed_settings, - compose=compose, - owned_paths=owned_paths, - display="Claude Code", - ) + if config_scope() != "user": + check_skipped_managed_overrides( + _managed_settings_path(), + parser=_parse_managed_settings, + compose=compose, + owned_paths=owned_paths, + display="Claude Code", + ) return path = _managed_settings_path() if path is None: diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 1fa868ef4..141d3c27d 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -67,6 +67,7 @@ OS, ManagedFileWriteUnavailable, check_skipped_managed_overrides, + config_scope, current_os, managed_file_conflicts, managed_file_is_verified, @@ -764,13 +765,14 @@ def _reconcile_managed_config(state: dict, compose: Callable[[dict], dict]) -> N if path is not None and path.name == CODEX_REQUIREMENTS_FILE: compose = _as_requirements(compose) if managed_settings_skipped(): - check_skipped_managed_overrides( - path, - parser=_parse_managed_config, - compose=compose, - owned_paths=MANAGED_KEYS, - display="Codex", - ) + if config_scope() != "user": + check_skipped_managed_overrides( + path, + parser=_parse_managed_config, + compose=compose, + owned_paths=MANAGED_KEYS, + display="Codex", + ) return if path is None: print_warning_err( diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 91660000f..0d406cc28 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -33,6 +33,7 @@ install_databricks_ai_tools_for_agents, install_tool_binary, normalize_tool, + refuse_user_scope_over_managed_files, resolve_gemini_provider_model, resolve_launch_model, resolve_provider_models, @@ -976,6 +977,7 @@ def _configure_workspace_command( ) -> int: if tool is not None and selected_tools is not None: raise RuntimeError("Use either --agent or --agents, not both.") + refuse_user_scope_over_managed_files([tool] if tool else selected_tools or []) # The Databricks-vs-Model-Provider-Service picker is shown only on the fully # interactive path (`ug configure` with no --agent/--agents). Naming agents @@ -1013,6 +1015,7 @@ def _configure_workspace_command( managed, _ = refresh_managed_config(state, force_refresh=True) managed_tools = managed_enabled_tools(managed) if managed is not None else [] if managed is not None and managed_tools: + refuse_user_scope_over_managed_files(managed_tools) configured_tools: list[str] = [] for tool_name in managed_tools: resolved = resolve_state(managed, state, tool_name) @@ -1091,6 +1094,7 @@ def _configure_workspace_command( if not picked: print_note("No coding agents selected — nothing to configure.") return 0 + refuse_user_scope_over_managed_files(picked) for tool_name in picked: install_tool_binary( @@ -4208,8 +4212,7 @@ def configure( if global_scope and user_scope: raise RuntimeError("Use either --global or --user, not both.") scope = "user" if user_scope else "global" - if global_scope or user_scope: - set_config_scope(scope) + set_config_scope(scope) custom_oauth = _custom_oauth_config(client_id, redirect_url, scopes) if custom_oauth is not None and use_pat: raise RuntimeError("--client-id cannot be combined with --use-pat.") @@ -4359,8 +4362,7 @@ def configure( and prompt_yes_no("Configure MCP servers now?") ): configure_mcp_command() - if global_scope or user_scope: - save_config_scope(scope) + save_config_scope(scope) except typer.Exit: # `typer.Exit` subclasses RuntimeError, so it has to be re-raised ahead of the handler # below. Otherwise a clean exit (e.g. `_reject_configure_under_managed_config` under a diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 73ebd388d..5f3874f47 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -1088,11 +1088,15 @@ def test_writes_managed_file_by_default(self, monkeypatch): assert "modelPicker" not in private_writes[0][1] assert "modelPicker" not in json.loads(managed_writes[0][1]) - def test_skip_env_bypasses_managed_file(self, monkeypatch): + @pytest.mark.parametrize("opt_out", ["env", "user_scope"]) + def test_skip_env_bypasses_managed_file(self, monkeypatch, opt_out): private_writes: list = [] managed_writes: list = [] self._patch(monkeypatch, private_writes, managed_writes) - monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") + if opt_out == "env": + monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") + else: + managed_files.set_config_scope("user") # A privileged write must never be attempted when the developer opted out. monkeypatch.setattr( claude, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") @@ -1130,17 +1134,20 @@ def test_skip_env_errors_on_stale_managed_override(self, monkeypatch): assert "stale" in message assert "ug revert" in message - def test_user_scope_bypasses_managed_file(self, monkeypatch): + def test_user_scope_launches_over_stale_managed_override(self, monkeypatch): private_writes: list = [] managed_writes: list = [] - self._patch(monkeypatch, private_writes, managed_writes) + existing = { + str(FAKE_MANAGED_PATH): { + "env": { + "ANTHROPIC_BASE_URL": "https://old-workspace.databricks.com/ai-gateway/anthropic" + } + } + } + self._patch(monkeypatch, private_writes, managed_writes, existing) managed_files.set_config_scope("user") - monkeypatch.setattr( - claude, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") - ) state = {"workspace": WS, "codex_models": []} claude.write_tool_config(state, "databricks-claude-sonnet-4") - assert str(claude.CLAUDE_SETTINGS_PATH) in [p for p, _ in private_writes] assert managed_writes == [] def test_skip_env_no_error_without_stale_override(self, monkeypatch): diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index 8093ca0de..ca9a2f53d 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -1744,9 +1744,13 @@ def test_windows_writes_requirements_shaped_config(self, tmp_path, monkeypatch): assert profile["model"] == "gpt-5" assert "env_http_headers" not in profile["model_providers"]["Databricks"] - def test_skip_env_bypasses_managed_config(self, tmp_path, monkeypatch): + @pytest.mark.parametrize("opt_out", ["env", "user_scope"]) + def test_skip_env_bypasses_managed_config(self, tmp_path, monkeypatch, opt_out): config_path, managed_path = self._patch(tmp_path, monkeypatch) - monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") + if opt_out == "env": + monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") + else: + managed_files.set_config_scope("user") # A privileged write must never be attempted when the developer opted out. monkeypatch.setattr( codex, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") @@ -1784,17 +1788,21 @@ def test_skip_env_errors_on_stale_managed_override(self, tmp_path, monkeypatch): assert "stale" in message assert "ug revert" in message - def test_user_scope_bypasses_managed_config(self, tmp_path, monkeypatch): + def test_user_scope_launches_over_stale_managed_override(self, tmp_path, monkeypatch): config_path, managed_path = self._patch(tmp_path, monkeypatch) - managed_files.set_config_scope("user") - monkeypatch.setattr( - codex, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") + managed_path.parent.mkdir(parents=True, exist_ok=True) + managed_path.write_text( + 'model_provider = "Databricks"\n' + "[model_providers.Databricks]\n" + 'base_url = "https://old-workspace.databricks.com/ai-gateway/openai/v1"\n', + encoding="utf-8", ) + managed_files.set_config_scope("user") codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) assert read_toml_safe(config_path)["model_provider"] == "Databricks" - assert not managed_path.exists() + assert "old-workspace" in managed_path.read_text(encoding="utf-8") def test_skip_env_no_error_without_stale_override(self, tmp_path, monkeypatch): config_path, managed_path = self._patch(tmp_path, monkeypatch) diff --git a/tests/test_agents_init.py b/tests/test_agents_init.py index ea3b53247..c90fa4148 100644 --- a/tests/test_agents_init.py +++ b/tests/test_agents_init.py @@ -1216,35 +1216,55 @@ def test_appends_os_managed_file_recorded_in_state(self): class TestConfigureToolUserScope: @pytest.mark.parametrize( - "tool,path_owner,path_attr,writer", - [ - ("claude", "claude", "_managed_settings_path", "write_tool_config"), - ("codex", "codex", "codex_managed_config_path", "write_tool_config"), - ], + "tool,path_attr", + [("claude", "_managed_settings_path"), ("codex", "codex_managed_config_path")], ) @pytest.mark.parametrize("exists", [True, False]) - def test_managed_file_blocks_user_scope( - self, monkeypatch, tmp_path, tool, path_owner, path_attr, writer, exists - ): + def test_managed_file_blocks_user_scope(self, monkeypatch, tmp_path, tool, path_attr, exists): managed = tmp_path / "managed" if exists: managed.write_text("x", encoding="utf-8") - module = getattr(agents_mod, path_owner) + module = getattr(agents_mod, tool) monkeypatch.setattr(module, path_attr, lambda: managed) - written: list = [] - monkeypatch.setattr(module, writer, lambda state, *a, **kw: written.append(tool) or state) managed_files.set_config_scope("user") if exists: with pytest.raises(RuntimeError, match=r"--user.*managed settings exist.*--global"): - agents_mod.configure_tool(tool, {}, "m") - assert written == [] + agents_mod.refuse_user_scope_over_managed_files([tool]) else: - agents_mod.configure_tool(tool, {}, "m") - assert written == [tool] + agents_mod.refuse_user_scope_over_managed_files([tool]) + + def test_windows_codex_requirements_blocks_user_scope(self, monkeypatch, tmp_path): + import ucode.codex_config as codex_config + from ucode.os_compatibility import system_cross_os + + requirements = tmp_path / "OpenAI" / "Codex" / managed_files.CODEX_REQUIREMENTS_FILE + requirements.parent.mkdir(parents=True) + requirements.write_text('model_provider = "Databricks"\n', encoding="utf-8") + monkeypatch.setattr(codex_config, "current_os", lambda: managed_files.OS.WINDOWS) + monkeypatch.setattr(system_cross_os, "windows_program_data_dir", lambda: tmp_path) + monkeypatch.setattr( + agents_mod.codex, "codex_managed_config_path", codex_config.codex_managed_config_path + ) + managed_files.set_config_scope("user") + with pytest.raises(RuntimeError, match="requirements.toml"): + agents_mod.refuse_user_scope_over_managed_files(["codex"]) def test_no_managed_path_is_not_checked(self, monkeypatch): monkeypatch.setattr(agents_mod.codex, "codex_managed_config_path", lambda: None) - monkeypatch.setattr(agents_mod.codex, "write_tool_config", lambda state, *a, **kw: state) managed_files.set_config_scope("user") - agents_mod.configure_tool("codex", {}, "m") + agents_mod.refuse_user_scope_over_managed_files(["codex"]) + + def test_launch_does_not_refuse_over_managed_file(self, monkeypatch, tmp_path): + managed = tmp_path / "managed" + managed.write_text("x", encoding="utf-8") + monkeypatch.setattr(agents_mod.claude, "_managed_settings_path", lambda: managed) + written: list = [] + monkeypatch.setattr( + agents_mod.claude, + "write_tool_config", + lambda state, *a, **kw: written.append(1) or state, + ) + managed_files.set_config_scope("user") + agents_mod.configure_tool("claude", {}, "m") + assert written == [1] diff --git a/tests/test_cli.py b/tests/test_cli.py index dbab754fe..a90f1c98a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5173,25 +5173,59 @@ def test_global_flag_overrides_saved_user_scope(self): assert managed_files.config_scope() == "global" assert cli_mod.load_full_state()["config_scope"] == "global" - def test_saved_scope_is_reused_without_flag(self): + def test_configure_without_flag_resets_saved_user_scope(self): self._configure(["--user"]) - managed_files.set_config_scope("global") result, _ = self._configure() assert result.exit_code == 0, result.output - assert managed_files.config_scope() == "user" - assert cli_mod.load_full_state()["config_scope"] == "user" + assert managed_files.config_scope() == "global" + assert cli_mod.load_full_state()["config_scope"] == "global" - def test_failed_configure_does_not_persist_scope(self): + def test_managed_file_refuses_before_any_agent_is_configured(self, tmp_path): + codex_managed = tmp_path / "managed_config.toml" + codex_managed.write_text("x", encoding="utf-8") with ( patch("ucode.cli.install_databricks_cli"), - patch("ucode.cli.install_tool_binary"), - patch("ucode.cli.configure_workspace_command", side_effect=RuntimeError("refused")), + patch("ucode.agents.claude._managed_settings_path", return_value=None), + patch("ucode.agents.codex.codex_managed_config_path", return_value=codex_managed), + patch("ucode.cli._configure_shared_workspace_states") as mock_login, + patch("ucode.cli.configure_selected_tools") as mock_selected, ): result = runner.invoke(app, [*self.ARGS, "--user"]) assert result.exit_code == 1 - assert managed_files.config_scope() == "user" + assert "managed settings exist" in _strip_ansi(result.output) + mock_login.assert_not_called() + mock_selected.assert_not_called() assert "config_scope" not in cli_mod.load_full_state() + @pytest.mark.parametrize("managed", [True, False], ids=["managed_config", "picker"]) + def test_workspace_paths_refuse_before_configuring(self, tmp_path, managed): + claude_managed = tmp_path / "managed-settings.json" + claude_managed.write_text("{}", encoding="utf-8") + managed_files.set_config_scope("user") + with ( + patch("ucode.agents.claude._managed_settings_path", return_value=claude_managed), + patch( + "ucode.cli._configure_shared_workspace_states", + return_value=[{"workspace": "https://first.databricks.com"}], + ), + patch("ucode.cli.save_state"), + patch( + "ucode.cli.refresh_managed_config", + return_value=({"enabled": True} if managed else None, None), + ), + patch("ucode.cli.managed_enabled_tools", return_value=["claude"]), + patch("ucode.cli.check_gateway_endpoint", return_value=True), + patch("ucode.cli.prompt_for_tools", return_value=["claude"]), + patch("ucode.cli.install_tool_binary") as mock_install, + patch("ucode.cli.configure_selected_tools") as mock_selected, + ): + with pytest.raises(RuntimeError, match="managed settings exist"): + cli_mod._configure_workspace_command( + workspaces=[("https://first.databricks.com", None)] + ) + mock_install.assert_not_called() + mock_selected.assert_not_called() + def test_flags_are_mutually_exclusive(self): result, mock_cfg = self._configure(["--global", "--user"]) assert result.exit_code == 1 @@ -5218,12 +5252,6 @@ def test_revert_does_not_apply_saved_user_scope(self): runner.invoke(app, ["revert"]) assert managed_files.config_scope() == "global" - def test_clear_state_resets_scope_to_default(self): - self._configure(["--user"]) - cli_mod.clear_state() - assert "config_scope" not in cli_mod.load_full_state() - assert cli_mod.load_config_scope() == "global" - class TestConfigureSharedStateUsePat: """--use-pat reads the profile's PAT from ~/.databrickscfg, exports it as diff --git a/tests/test_state.py b/tests/test_state.py index fcc6cbfa2..098d2b829 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -19,10 +19,12 @@ get_provider_service, hydrate_state, is_self_managed, + load_config_scope, load_full_state, load_state, mark_tool_managed, remove_self_managed_agent, + save_config_scope, save_state, self_managed_agents, set_applied_managed_update_time, @@ -165,6 +167,11 @@ def test_clears_current_workspace(self): assert full["current_workspace"] is None assert FAKE_WS not in full.get("workspaces", {}) + def test_resets_config_scope(self): + save_config_scope("user") + clear_state() + assert load_config_scope() == "global" + def test_clear_when_no_state_is_noop(self): clear_state() # should not raise