diff --git a/src/ucode/cli.py b/src/ucode/cli.py index b23c27b2..71afbb4d 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -114,6 +114,7 @@ configured_mcp_clients, purge_cross_workspace_mcp_residue, remove_mcp_command, + remove_skills_command, revert_mcp_configs, skill_locations_for_client, ) @@ -1044,6 +1045,7 @@ def status() -> int: print_note( "Use `ug configure skills` to set up Unity Catalog Skills for configured coding tools." ) + print_note("Use `ug skill add` and `ug skill remove --mcp` to manage UC Skills.") print_note("Use `ug configure tracing` to log coding sessions to an MLflow experiment.") print_note("Use `ug revert` to clear managed configs and restore prior files.") return 0 @@ -1389,6 +1391,32 @@ def skills_add( raise typer.Exit(130) from None +@skill_app.command("remove") +def skills_remove( + mcp: Annotated[ + bool, + typer.Option( + "--mcp", + help="Remove schemas from the skills MCP connection instead of downloaded files.", + ), + ] = False, +) -> None: + """Interactively remove Skill schemas from the skills MCP connection.""" + try: + if not mcp: + raise RuntimeError( + "Removing downloaded skills is not supported yet. Pass --mcp to remove " + "schemas from the skills MCP connection." + ) + remove_skills_command() + except RuntimeError as exc: + print_err(str(exc)) + raise typer.Exit(1) from None + except KeyboardInterrupt: + print_err("Interrupted.") + raise typer.Exit(130) from None + + @app.command("mcp-proxy", hidden=True) def mcp_proxy_cmd( url: Annotated[ diff --git a/src/ucode/mcp.py b/src/ucode/mcp.py index 33dd0793..94e3d34e 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2328,3 +2328,78 @@ def add_skills_command(locations: list[str], agents: set[str] | None = None) -> ) _update_skills_mcp(state, workspace, profile, clients, locations_by_client) return 0 + + +def _prompt_for_skill_removal(locations_by_client: dict[str, list[str]]) -> list[str] | None: + """Checklist of skill schemas to remove, each annotated with the clients it's scoped to. + Returns the selected locations, ``None`` if cancelled (Ctrl-C), or ``[]`` if nothing checked.""" + choices: list[questionary.Choice | questionary.Separator] = [] + ordered_locations = list( + dict.fromkeys( + location for locations in locations_by_client.values() for location in locations + ) + ) + for location in ordered_locations: + displays = [ + str(MCP_CLIENTS[client]["display"]) + for client, locations in locations_by_client.items() + if location in locations + ] + choices.append( + questionary.Choice( + title=f"{location} ({', '.join(displays)})", + value=location, + checked=False, + ) + ) + if not choices: + return [] + selection = _scrolling_checkbox( + "Remove skill schemas:", + choices=choices, + style=_picker_style(), + instruction="(space to toggle, ctrl-a all, enter to remove, type to filter)", + ).ask() + if selection is None: + return None + return [str(value) for value in selection] + + +def remove_skills_command() -> int: + """`ucode skill remove --mcp`: interactively drop skill schemas from every configured client. + + Shows the schemas currently in each configured client's skills scope and removes the ones you + select from every client that has them. It never adds or reconfigures anything, and needs no + Databricks auth.""" + state = load_state() + workspace, profile, clients = setup_mcp_clients( + state, + "Remove Skills MCP", + require_auth=False, + action_note="Removing from", + ) + locations_by_client = _skill_locations_by_client_from_state(state) + offered = {client: locations_by_client.get(client, []) for client in clients} + if not any(offered.values()): + print_note("No skill schemas are configured to remove.") + return 0 + + selection = _prompt_for_skill_removal(offered) + if selection is None: + return 0 + if not selection: + print_note("No skill schemas selected.") + return 0 + + remove_locations = set(selection) + for client in clients: + locations_by_client[client] = [ + location + for location in locations_by_client.get(client, []) + if location not in remove_locations + ] + _update_skills_mcp(state, workspace, profile, clients, locations_by_client, print_summary=False) + print_success( + f"Removed {len(remove_locations)} skill schema{'s' if len(remove_locations) != 1 else ''}." + ) + return 0 diff --git a/tests/test_cli.py b/tests/test_cli.py index c1ea4f2e..5270a360 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1598,6 +1598,23 @@ def test_all_configured_skips_bootstrap(self): mock_cfg.assert_not_called() +class TestSkillsRemoveCommand: + def test_requires_mcp_until_download_removal_is_supported(self): + with patch("ucode.cli.remove_skills_command") as remove: + result = runner.invoke(app, ["skill", "remove"]) + + assert result.exit_code == 1 + assert "Removing downloaded skills is not supported yet" in _strip_ansi(result.output) + remove.assert_not_called() + + def test_mcp_remove_dispatches_global_removal(self): + with patch("ucode.cli.remove_skills_command") as remove: + result = runner.invoke(app, ["skill", "remove", "--mcp"]) + + assert result.exit_code == 0, result.output + remove.assert_called_once_with() + + class TestManagedSkillsOnLaunch: """Managed skills are delivered by download only: the launch path downloads them and never registers them on the skills MCP connection (only a developer's own `skill add --mcp` schemas diff --git a/tests/test_mcp.py b/tests/test_mcp.py index cddf1f7f..18f93748 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2314,6 +2314,86 @@ def test_agents_add_matching_existing_scope_is_a_noop(self, monkeypatch): assert configured == [] +class TestRemoveSkillsCommand: + def _state(self, by_client=None): + by_client = by_client or _by_client(["claude", "codex"], ["A.a", "B.b"]) + return { + "workspace": WS, + "available_tools": ["claude", "codex"], + "mcp_servers": mcp._resolve_skills_mcp_servers(WS, list(by_client), by_client, []), + } + + def _stub(self, monkeypatch, state, selection): + configured: list[tuple[str, str]] = [] + _stub_location_base(monkeypatch, state) + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"]) + monkeypatch.setattr(mcp, "_prompt_for_skill_removal", lambda scopes: selection) + monkeypatch.setattr( + mcp, + "configure_client_mcp_server", + lambda client, name, url, *a, **kw: configured.append((client, url)) or [], + ) + monkeypatch.setattr(mcp, "save_state", lambda s: None) + return configured + + def test_removes_selected_schema_from_every_client(self, monkeypatch): + state = self._state() + configured = self._stub(monkeypatch, state, ["A.a"]) + + assert mcp.remove_skills_command() == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert mcp.skill_locations_for_client(entry, "claude") == ["B.b"] + assert mcp.skill_locations_for_client(entry, "codex") == ["B.b"] + assert sorted(configured) == [ + ("claude", f"{WS}/ai-gateway/skills/?schema=B.b"), + ("codex", f"{WS}/ai-gateway/skills/?schema=B.b"), + ] + + def test_removing_all_schemas_keeps_schemaless_connection(self, monkeypatch): + state = self._state() + configured = self._stub(monkeypatch, state, ["A.a", "B.b"]) + + assert mcp.remove_skills_command() == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert entry["skill_locations"] == [] + assert sorted(configured) == [ + ("claude", f"{WS}/ai-gateway/skills/"), + ("codex", f"{WS}/ai-gateway/skills/"), + ] + + def test_nothing_configured_is_a_noop(self, monkeypatch): + state = { + "workspace": WS, + "available_tools": ["claude", "codex"], + "mcp_servers": mcp._resolve_skills_mcp_servers(WS, ["claude", "codex"], {}, []), + } + captured: dict[str, bool] = {} + _stub_location_base(monkeypatch, state) + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"]) + monkeypatch.setattr( + mcp, "_prompt_for_skill_removal", lambda scopes: captured.setdefault("called", True) + ) + + assert mcp.remove_skills_command() == 0 + assert "called" not in captured + + def test_offers_each_client_scope_to_the_picker(self, monkeypatch): + state = self._state({"claude": ["A.a", "B.b"], "codex": ["A.a"]}) + captured: dict[str, dict[str, list[str]]] = {} + _stub_location_base(monkeypatch, state) + monkeypatch.setattr(mcp, "available_mcp_clients", lambda: ["claude", "codex"]) + monkeypatch.setattr( + mcp, + "_prompt_for_skill_removal", + lambda scopes: captured.setdefault("scopes", scopes) and None, + ) + + assert mcp.remove_skills_command() == 0 + assert captured["scopes"] == {"claude": ["A.a", "B.b"], "codex": ["A.a"]} + + class TestRegisterSchemalessSkillsConnection: def _stub(self, monkeypatch): saved_states: list[dict] = []