diff --git a/README.md b/README.md index 3e18b02c..9b390cae 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,16 @@ ucode skill add --location main.default --skills my-skill,other-skill ucode skill add --skills main.default.my-skill,main.default.other-skill ``` +#### Remove shared skill MCP scopes + +`ucode skill remove --mcp` interactively removes developer-configured schemas from the shared +skills MCP scope. Administrator-managed schemas are not offered, and the schema-less utility +connection remains registered after its last schema is removed. + +```bash +ucode skill remove --mcp +``` + ### Managed config for a workspace (admins) Author the coding config your developers pick up automatically, instead of asking each of them to @@ -386,6 +396,7 @@ The output looks like: | `ucode skill add --location main.default --mcp --agents claude` | Set up selected agents if needed and add schemas only to their MCP scopes | | `ucode skill add --location main.default` | Download a schema's skills to disk without removing existing downloads | | `ucode skill add --skills main.default.my-skill` | Download a named subset of skills (bare names need `--location`; fully-qualified names stand alone) | +| `ucode skill remove --mcp` | Interactively remove developer schemas from the shared skills MCP scope | | `ucode setup` | Author the managed config's agents and models (workspace admins only) | | `ucode setup mcps` | Add or change the managed config's MCP servers | | `ucode setup skills [--location a.b,c.d]` | Add or change the managed config's skills | diff --git a/src/ucode/cli.py b/src/ucode/cli.py index 748aa40e..95e73c6b 100644 --- a/src/ucode/cli.py +++ b/src/ucode/cli.py @@ -106,6 +106,7 @@ configure_skills_mcp_command, purge_cross_workspace_mcp_residue, remove_mcp_command, + remove_skills_command, revert_mcp_configs, skill_locations_for_client, ) @@ -1119,9 +1120,7 @@ def status() -> int: print_note( "Use `ucode configure mcp` to add Databricks MCP servers to configured coding tools." ) - print_note( - "Use `ucode configure skills` to set up Unity Catalog Skills for configured coding tools." - ) + print_note("Use `ucode skill add` and `ucode skill remove --mcp` to manage UC Skills.") print_note("Use `ucode configure tracing` to log coding sessions to an MLflow experiment.") print_note("Use `ucode revert` to clear managed configs and restore prior files.") return 0 @@ -1475,6 +1474,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 shared 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 9abe4dca..18a4bd67 100644 --- a/src/ucode/mcp.py +++ b/src/ucode/mcp.py @@ -2385,3 +2385,98 @@ def add_skills_command(locations: list[str], agents: set[str] | None = None) -> location_overrides=overrides, ) return 0 + + +def _prompt_for_skill_removal( + locations_by_client: dict[str, list[str]], +) -> list[str] | None: + """Prompt for skill locations, showing which clients receive each one.""" + 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: + """Interactively remove developer schemas from every client's effective scope.""" + state = load_state() + workspace, profile, clients = setup_mcp_clients( + state, + "Remove Skills MCP", + require_auth=False, + action_note="Removing from", + ) + entry = _skills_entry(list(state.get("mcp_servers") or [])) + managed = { + location + for location in (state.get("managed_skill_locations") or []) + if isinstance(location, str) and location + } + default = _skill_mcp_locations(state) + overrides = _skill_location_overrides(entry) + locations_by_client = { + client: [ + location + for location in skill_locations_for_client(entry, client) + if location not in managed + ] + for client in clients + } + if not any(locations_by_client.values()): + print_note("No developer skill schemas are configured to remove.") + return 0 + selection = _prompt_for_skill_removal(locations_by_client) + if selection is None: + return 0 + if not selection: + print_note("No skill schemas selected.") + return 0 + + remove_locations = set(selection) + new_default = [location for location in default if location not in remove_locations] + remaining_overrides: dict[str, list[str]] = {} + for client, client_locations in overrides.items(): + remaining = [ + location for location in client_locations if location not in remove_locations + ] + if remaining: + remaining_overrides[client] = remaining + + _update_skills_mcp( + state, + workspace, + profile, + clients, + new_default, + location_overrides=remaining_overrides, + ) + 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 47f7c901..78d1036f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1251,6 +1251,23 @@ def test_agents_is_rejected_for_download_mode(self): mock_download.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 TestApplyManagedSkills: """The launch path both registers the skills MCP connection and downloads bundles to disk.""" diff --git a/tests/test_mcp.py b/tests/test_mcp.py index a016434f..a347b31b 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -2684,6 +2684,74 @@ def test_agents_persists_explicit_scope_even_when_it_matches_default(self, monke assert configured == [] +class TestRemoveSkillsCommand: + def _state(self): + return { + "workspace": WS, + "available_tools": ["claude", "codex"], + "mcp_servers": mcp._resolve_skills_mcp_servers( + WS, ["claude", "codex"], ["A.a", "B.b"], [] + ), + } + + 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_unscoped_remove_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 configured == [ + ("claude", f"{WS}/ai-gateway/skills/"), + ("codex", f"{WS}/ai-gateway/skills/"), + ] + + def test_unscoped_remove_drops_an_empty_client_additions_list(self, monkeypatch): + state = self._state() + state["mcp_servers"] = mcp._resolve_skills_mcp_servers( + WS, + ["claude", "codex"], + ["A.a"], + [], + {"claude": ["C.c"]}, + ) + self._stub(monkeypatch, state, ["C.c"]) + + assert mcp.remove_skills_command() == 0 + + entry = _find_skills(state["mcp_servers"])[0] + assert mcp.SKILL_LOCATION_OVERRIDES_KEY not in entry + + def test_managed_locations_are_not_offered(self, monkeypatch): + state = self._state() + state["managed_skill_locations"] = ["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": ["B.b"], "codex": ["B.b"]} + + class TestRegisterSchemalessSkillsConnection: def _stub(self, monkeypatch): saved_states: list[dict] = []