diff --git a/src/ucode/managed_wizard.py b/src/ucode/managed_wizard.py index da68ba4..d1d40c1 100644 --- a/src/ucode/managed_wizard.py +++ b/src/ucode/managed_wizard.py @@ -22,6 +22,7 @@ from ucode.databricks import ( ANTHROPIC_FAMILIES, create_coding_agent_config, + delete_coding_agent_config, discover_claude_models_unbucketed, ensure_databricks_auth, get_databricks_token, @@ -674,8 +675,12 @@ def _require_admin(workspace: str, token: str) -> None: print_success("Admin permissions verified") -def _warn_on_existing_config(workspace: str, token: str) -> None: - """Warn when the workspace already has a published config that `ucode apply` would replace. +def _handle_existing_config(workspace: str, token: str) -> bool: + """Decide what to do when the workspace already has a published config. + + Returns True to keep authoring a new config (the wizard continues; publishing later replaces the + existing one) and False to stop (no config exists, the check failed, or the admin chose to delete + the existing one instead of authoring a replacement). Deliberately doesn't itemize what the existing config holds. The admin doesn't need an inventory to act on this — the instruction is the same either way ("include everything you want to keep") @@ -685,14 +690,58 @@ def _warn_on_existing_config(workspace: str, token: str) -> None: existing, reason = get_managed_config(workspace, token) if reason is not None: print_note(f"Could not check for an existing config: {reason}") - return + return True if existing is None: - return + return True + print_warning( "This workspace already has a managed configuration — one config covers every agent, MCP " - "server, skill, tracing table, and budget policy for the whole workspace. Publishing " - "replaces all of it, so make sure this run includes everything you want to keep." + "server, skill, tracing table, and budget policy for the whole workspace." + ) + choice = prompt_for_selection( + "What would you like to do?", + [ + ("create", "Author a new config (replaces the existing one when you publish)"), + ("delete", "Delete the existing config (removes it from the workspace, leaves none)"), + ], ) + if choice is None: + raise KeyboardInterrupt + if choice == "create": + print_note("Make sure this run includes everything you want to keep.") + return True + + _delete_existing_config(workspace, token, existing) + return False + + +def _delete_existing_config(workspace: str, token: str, existing: dict) -> None: + """Delete the workspace's published config after confirming. Raises RuntimeError on failure. + + Deleting leaves the workspace with no managed config, so every developer falls back to their own + settings on their next ucode run — confirm before doing it, and honor ``--dry-run``. + """ + name = existing.get("name") + if not isinstance(name, str): + raise RuntimeError( + "This workspace has a managed config but the API didn't return its resource name, so " + "ucode can't delete it. Delete it in the workspace directly." + ) + print_warning( + "Deleting removes the managed config entirely. Every developer falls back to their own " + "settings on their next ucode run." + ) + if not prompt_yes_no_default("Delete the existing managed config?", default=False): + print_note("Nothing was deleted.") + return + if is_dry_run(): + print_success("Dry run: the config was not deleted.") + return + with spinner("Deleting the managed config..."): + delete_reason = delete_coding_agent_config(workspace, token, name) + if delete_reason is not None: + raise RuntimeError(f"Could not delete the managed config on {workspace}: {delete_reason}.") + print_success(f"Deleted the managed config from {workspace}") def setup_from_file(path: str) -> int: @@ -771,7 +820,8 @@ def setup_command(from_file: str | None = None) -> int: token = get_databricks_token(workspace, profile) _require_admin(workspace, token) - _warn_on_existing_config(workspace, token) + if not _handle_existing_config(workspace, token): + return 0 # Discover the workspace's models and gateway URLs. This also logs in and persists local state, # which is what lets the admin dry-run the config on their own machine afterwards. diff --git a/tests/test_managed_wizard.py b/tests/test_managed_wizard.py index 27ebe69..f5e9257 100644 --- a/tests/test_managed_wizard.py +++ b/tests/test_managed_wizard.py @@ -180,59 +180,134 @@ def test_unverifiable_check_warns_and_continues(self): assert warn.called -class TestExistingConfigWarning: - @staticmethod - def _warn(existing: dict) -> str: +class TestExistingConfigHandling: + RICH_CONFIG = { + "name": "coding-agent-configs/abc", + "enabled_agents": {"claude": {}, "opencode": {}, "pi": {}}, + "mcp_servers": [{"name": "a", "type": "sql"}], + "skills": {"names": ["main.default"]}, + "tracing_table": "main.default.traces", + "budget_policy": {"display_name": "lillys_budget", "budget_id": "abc"}, + } + + def test_continue_when_no_config_exists(self): + # Nothing published, so there is no prompt — the wizard just proceeds. with ( - patch.object(wizard, "get_managed_config", return_value=(existing, None)), + patch.object(wizard, "get_managed_config", return_value=(None, None)), + patch.object(wizard, "prompt_for_selection") as select, patch.object(wizard, "print_warning") as warn, ): - wizard._warn_on_existing_config(WORKSPACE, "token") - assert warn.called - return warn.call_args[0][0] + assert wizard._handle_existing_config(WORKSPACE, "token") is True + assert not select.called + assert not warn.called - def test_warns_that_publishing_replaces_the_whole_config(self): - message = self._warn({"enabled_agents": {"claude": {}, "codex": {}}}) - # There is one config per workspace covering everything, so the warning says that rather - # than reading like a per-agent notice. - assert "one config covers every agent" in message - assert "replaces all of it" in message - assert "everything you want to keep" in message + def test_read_failure_continues_with_a_note(self): + # Can't check isn't the same as "there is one"; don't imply data loss or block the wizard. + with ( + patch.object(wizard, "get_managed_config", return_value=(None, "HTTP 403 Forbidden")), + patch.object(wizard, "prompt_for_selection") as select, + patch.object(wizard, "print_note") as note, + ): + assert wizard._handle_existing_config(WORKSPACE, "token") is True + assert not select.called + assert note.called + + def test_choosing_create_continues_authoring(self): + with ( + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "x", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value="create"), + ): + assert wizard._handle_existing_config(WORKSPACE, "token") is True def test_warning_does_not_itemize_the_existing_config(self): - # The message is the same whatever the config holds: an inventory doesn't change what the + # The warning is the same whatever the config holds: an inventory doesn't change what the # admin should do, and `ucode setup show` prints the real thing for comparison. - rich = self._warn( - { - "enabled_agents": {"claude": {}, "opencode": {}, "pi": {}}, - "mcp_servers": [{"name": "a", "type": "sql"}], - "skills": {"names": ["main.default"]}, - "tracing_table": "main.default.traces", - "budget_policy": {"display_name": "lillys_budget", "budget_id": "abc"}, - } - ) - assert rich == self._warn({"enabled_agents": {}}) + with ( + patch.object(wizard, "get_managed_config", return_value=(self.RICH_CONFIG, None)), + patch.object(wizard, "prompt_for_selection", return_value="create"), + patch.object(wizard, "print_warning") as warn, + ): + wizard._handle_existing_config(WORKSPACE, "token") + message = warn.call_args[0][0] + assert "one config covers every agent" in message for leaked in ("Claude Code", "OpenCode", "lillys_budget", "main.default"): - assert leaked not in rich, leaked + assert leaked not in message, leaked - def test_silent_when_no_config_exists(self): + def test_choosing_delete_stops_and_deletes(self): with ( - patch.object(wizard, "get_managed_config", return_value=(None, None)), - patch.object(wizard, "print_warning") as warn, + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "cfg/1", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value="delete"), + patch.object(wizard, "prompt_yes_no_default", return_value=True), + patch.object(wizard, "is_dry_run", return_value=False), + patch.object(wizard, "delete_coding_agent_config", return_value=None) as delete, ): - wizard._warn_on_existing_config(WORKSPACE, "token") - assert not warn.called + assert wizard._handle_existing_config(WORKSPACE, "token") is False + delete.assert_called_once_with(WORKSPACE, "token", "cfg/1") - def test_read_failure_is_a_note_not_a_warning(self): - # Can't check isn't the same as "there is one"; don't imply data loss. + def test_delete_declined_leaves_config_intact(self): with ( - patch.object(wizard, "get_managed_config", return_value=(None, "HTTP 403 Forbidden")), - patch.object(wizard, "print_warning") as warn, - patch.object(wizard, "print_note") as note, + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "cfg/1", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value="delete"), + patch.object(wizard, "prompt_yes_no_default", return_value=False), + patch.object(wizard, "delete_coding_agent_config") as delete, ): - wizard._warn_on_existing_config(WORKSPACE, "token") - assert not warn.called - assert note.called + # Still stops the wizard: the admin chose the delete path, not the author path. + assert wizard._handle_existing_config(WORKSPACE, "token") is False + assert not delete.called + + def test_delete_honors_dry_run(self): + with ( + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "cfg/1", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value="delete"), + patch.object(wizard, "prompt_yes_no_default", return_value=True), + patch.object(wizard, "is_dry_run", return_value=True), + patch.object(wizard, "delete_coding_agent_config") as delete, + ): + assert wizard._handle_existing_config(WORKSPACE, "token") is False + assert not delete.called + + def test_delete_failure_raises(self): + with ( + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "cfg/1", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value="delete"), + patch.object(wizard, "prompt_yes_no_default", return_value=True), + patch.object(wizard, "is_dry_run", return_value=False), + patch.object(wizard, "delete_coding_agent_config", return_value="HTTP 500"), + pytest.raises(RuntimeError, match="Could not delete"), + ): + wizard._handle_existing_config(WORKSPACE, "token") + + def test_cancelling_the_picker_aborts(self): + with ( + patch.object( + wizard, + "get_managed_config", + return_value=({"name": "x", "enabled_agents": {}}, None), + ), + patch.object(wizard, "prompt_for_selection", return_value=None), + pytest.raises(KeyboardInterrupt), + ): + wizard._handle_existing_config(WORKSPACE, "token") class TestModelPrompting: