diff --git a/README.md b/README.md index 5ad3dd328..df4b152a3 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,8 @@ 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`. Configure fails while a managed file for the agent exists, since managed settings take +`ug`. `UG_SKIP_OS_SETTINGS=1` neither writes nor checks the OS-managed files: ug trusts whatever is +there and the agents apply their native precedence. 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`. diff --git a/docs/os-managed-settings-design.md b/docs/os-managed-settings-design.md index 857f59ef2..eba0d91ac 100644 --- a/docs/os-managed-settings-design.md +++ b/docs/os-managed-settings-design.md @@ -34,10 +34,11 @@ OS-managed files like `UG_SKIP_OS_SETTINGS`: no administrator permission, and ma 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`. `ug configure --user` also fails when the selected -workspace publishes a managed config, since that config must be installed machine-wide. +`--global` resolves it. Launches don't check: a managed file added later simply takes precedence. +`UG_SKIP_OS_SETTINGS` doesn't check either: ug trusts the OS-managed files present and leaves them +untouched. `ug configure` without `--user` and `ug revert` reset the scope to `global`. +`ug configure --user` also fails when the selected workspace publishes a managed config, since that +config must be installed machine-wide. ## Interactive Detection diff --git a/src/ucode/agents/claude.py b/src/ucode/agents/claude.py index 250439294..47ab12ab1 100644 --- a/src/ucode/agents/claude.py +++ b/src/ucode/agents/claude.py @@ -58,8 +58,6 @@ ManagedFileWriteUnavailable, SettingsPassthrough, apply_settings_passthrough, - check_skipped_managed_overrides, - config_scope, current_os, managed_file_conflicts, managed_file_is_verified, @@ -1880,14 +1878,7 @@ def _reconcile_managed_settings( during `ucode claude`, so a bare `claude` could not reach the gateway anyway. """ if managed_settings_skipped(): - if config_scope() != "user": - check_skipped_managed_overrides( - _managed_settings_path(), - parser=_parse_managed_settings, - compose=compose, - owned_paths=owned_paths, - display="Claude Code", - ) + # Opted out: trust whatever OS-managed file exists and leave it untouched. return path = _managed_settings_path() if path is None: diff --git a/src/ucode/agents/codex.py b/src/ucode/agents/codex.py index 5c5794ef5..99ece5d21 100644 --- a/src/ucode/agents/codex.py +++ b/src/ucode/agents/codex.py @@ -68,8 +68,6 @@ CODEX_REQUIREMENTS_FILE, OS, ManagedFileWriteUnavailable, - check_skipped_managed_overrides, - config_scope, current_os, managed_file_conflicts, managed_file_is_verified, @@ -900,14 +898,7 @@ 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(): - if config_scope() != "user": - check_skipped_managed_overrides( - path, - parser=_parse_managed_config, - compose=compose, - owned_paths=MANAGED_KEYS, - display="Codex", - ) + # Opted out: trust whatever OS-managed file exists and leave it untouched. return if path is None: print_warning_err( diff --git a/src/ucode/managed_files.py b/src/ucode/managed_files.py index 6e6d780e2..91b4cce59 100644 --- a/src/ucode/managed_files.py +++ b/src/ucode/managed_files.py @@ -116,7 +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. ``ug configure --user`` has the same effect. + that write is blocked. ug also trusts whatever OS-managed files exist and doesn't check them + against its own settings; the agents apply their native precedence. ``ug configure --user`` has + the same effect. """ return env_bool(SKIP_OS_SETTINGS_ENV) or _config_scope == "user" @@ -432,55 +434,6 @@ def managed_file_conflicts( return conflicts -def raise_on_skipped_managed_overrides( - path: Path, - conflicts: list[str], - *, - display: str, -) -> None: - """Fail when an opt-out left a stale OS-managed file overriding the new user config.""" - if not conflicts: - return - raise RuntimeError( - f"the OS-managed {display} settings at {path} are stale and conflict with your per-user settings." - f" {SKIP_OS_SETTINGS_ENV} is set, so the OS-managed settings were not modified. Run `ug revert` " - f"from an interactive terminal or remove {path}" - ) - - -def check_skipped_managed_overrides( - path: Path | None, - *, - parser: ManagedParser, - compose: Callable[[dict], dict], - owned_paths: list[list[str]], - display: str, -) -> None: - """When opting out, fail if a stale OS-managed file still overrides the new user config. - - `UG_SKIP_OS_SETTINGS` leaves the root-owned managed file untouched. A file from an earlier - workspace keeps its highest-precedence gateway values, so refuse to proceed when the conflicting - keys would let a bare agent silently reach the old workspace. Reads only (no sudo); best-effort, - so an unsupported platform, symlink, or unparseable file is left to the normal reconcile path. - """ - if path is None or path.is_symlink(): - return - current_text = read_managed_file(path) - if current_text is None: - return - try: - existing = parser(current_text) - except RuntimeError: - return - # `compose` mutates its argument in place, so snapshot the on-disk state before composing. - managed_before = deepcopy(existing) - raise_on_skipped_managed_overrides( - path, - managed_file_conflicts(managed_before, compose(existing), owned_paths), - display=display, - ) - - def _unwrap(value: object) -> object: """Return a plain-Python view of a value, unwrapping tomlkit items so it compares by content.""" unwrap = getattr(value, "unwrap", None) diff --git a/tests/test_agent_claude.py b/tests/test_agent_claude.py index 23476d883..3dbe63784 100644 --- a/tests/test_agent_claude.py +++ b/tests/test_agent_claude.py @@ -1110,10 +1110,10 @@ def test_skip_env_bypasses_managed_file(self, monkeypatch, opt_out): assert str(claude.CLAUDE_SETTINGS_PATH) in [p for p, _ in private_writes] assert managed_writes == [] - def test_skip_env_errors_on_stale_managed_override(self, monkeypatch): + def test_skip_env_trusts_a_stale_managed_file(self, monkeypatch): + """The opt-out trusts whatever OS-managed file exists: no check, no write, no error.""" private_writes: list = [] managed_writes: list = [] - # A managed file left from an earlier workspace still points at that workspace's gateway. existing = { str(FAKE_MANAGED_PATH): { "env": { @@ -1127,15 +1127,10 @@ def test_skip_env_errors_on_stale_managed_override(self, monkeypatch): claude, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") ) state = {"workspace": WS, "codex_models": []} - # A stale override silently defeats the reconfigure, so launching must fail loudly. - with pytest.raises(RuntimeError) as excinfo: - claude.write_tool_config(state, "databricks-claude-sonnet-4") + 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 == [] - message = str(excinfo.value) - assert managed_files.SKIP_OS_SETTINGS_ENV in message - assert "stale" in message - assert "ug revert" in message def test_user_scope_launches_over_stale_managed_override(self, monkeypatch): private_writes: list = [] diff --git a/tests/test_agent_codex.py b/tests/test_agent_codex.py index b76a4dd6b..2885ab27c 100644 --- a/tests/test_agent_codex.py +++ b/tests/test_agent_codex.py @@ -1771,31 +1771,25 @@ def test_skip_env_bypasses_managed_config(self, tmp_path, monkeypatch, opt_out): assert read_toml_safe(config_path)["model_provider"] == "Databricks" assert not managed_path.exists() - def test_skip_env_errors_on_stale_managed_override(self, tmp_path, monkeypatch): + def test_skip_env_trusts_a_stale_managed_file(self, tmp_path, monkeypatch): + """The opt-out trusts whatever OS-managed file exists: no check, no write, no error.""" config_path, managed_path = self._patch(tmp_path, monkeypatch) - # A managed file left from an earlier workspace still points at that workspace's gateway. managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_text( + stale = ( 'model_provider = "Databricks"\n' "[model_providers.Databricks]\n" - 'base_url = "https://old-workspace.databricks.com/ai-gateway/openai/v1"\n', - encoding="utf-8", + 'base_url = "https://old-workspace.databricks.com/ai-gateway/openai/v1"\n' ) + managed_path.write_text(stale, encoding="utf-8") monkeypatch.setenv(managed_files.SKIP_OS_SETTINGS_ENV, "1") monkeypatch.setattr( codex, "reconcile_managed_file", lambda *a, **kw: pytest.fail("must not write managed") ) - # A stale override silently defeats the reconfigure, so launching must fail loudly. - with pytest.raises(RuntimeError) as excinfo: - codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) + codex.write_tool_config({"workspace": WS, "codex_models": ["gpt-5"]}) - # The stale managed file is left in place and the user is told how to clear it. - assert managed_path.exists() - message = str(excinfo.value) - assert managed_files.SKIP_OS_SETTINGS_ENV in message - assert "stale" in message - assert "ug revert" in message + assert config_path.exists() + assert managed_path.read_text(encoding="utf-8") == stale def test_user_scope_launches_over_stale_managed_override(self, tmp_path, monkeypatch): config_path, managed_path = self._patch(tmp_path, monkeypatch) diff --git a/tests/test_managed_files.py b/tests/test_managed_files.py index 8c36c53e4..8e8742bdf 100644 --- a/tests/test_managed_files.py +++ b/tests/test_managed_files.py @@ -724,37 +724,6 @@ def test_skip_prevents_privileged_write(self, monkeypatch): ) assert result == "unsupported" - def test_raise_on_skipped_managed_overrides_reports_stale_leaves(self): - with pytest.raises(RuntimeError) as excinfo: - managed_files.raise_on_skipped_managed_overrides( - Path("/etc/claude-code/managed-settings.json"), - ["env.ANTHROPIC_BASE_URL"], - display="Claude Code", - ) - message = str(excinfo.value) - assert managed_files.SKIP_OS_SETTINGS_ENV in message - 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( - Path("/etc/claude-code/managed-settings.json"), - [], - display="Claude Code", - ) - class TestWindowsManagedWrites: """Windows has no sudo: an elevated process writes Program Files directly, others can't."""